vfio/pci: Rework RTL8168 quirk

Another rework of this quirk, this time to update to the new quirk
structure.  We can handle the address and data registers with
separate MemoryRegions and a quirk specific data structure, making the
code much more understandable.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

2 files changed, 105 insertions, 80 deletions

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 438b082..b3bb2f8 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -784,81 +784,63 @@ static void vfio_probe_nvidia_bar0_1800_quirk(VFIOPCIDevice *vdev, int nr)
  * vfio: vfio_bar_write(0000:05:00.0:BAR2+0x74, 0x8001f000, 4) // do write
  * vfio: vfio_bar_read(0000:05:00.0:BAR2+0x74, 4) = 0x1f000 // complete
  */
-static uint64_t vfio_rtl8168_window_quirk_read(void *opaque,
-                                               hwaddr addr, unsigned size)
-{
-    VFIOLegacyQuirk *quirk = opaque;
-    VFIOPCIDevice *vdev = quirk->vdev;
-    uint64_t val = 0;
+typedef struct VFIOrtl8168Quirk {
+    VFIOPCIDevice *vdev;
+    uint32_t addr;
+    uint32_t data;
+    bool enabled;
+} VFIOrtl8168Quirk;
 
-    if (!quirk->data.flags) { /* Non-MSI-X table access */
-        return vfio_region_read(&vdev->bars[quirk->data.bar].region,
-                                addr + 0x70, size);
-    }
+static uint64_t vfio_rtl8168_quirk_address_read(void *opaque,
+                                                hwaddr addr, unsigned size)
+{
+    VFIOrtl8168Quirk *rtl = opaque;
+    VFIOPCIDevice *vdev = rtl->vdev;
+    uint64_t data = vfio_region_read(&vdev->bars[2].region, addr + 0x74, size);
 
-    switch (addr) {
-    case 4: /* address */
-        val = quirk->data.address_match ^ 0x80000000U; /* latch/complete */
-        break;
-    case 0: /* data */
-        if ((vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX)) {
-            memory_region_dispatch_read(&vdev->pdev.msix_table_mmio,
-                                (hwaddr)(quirk->data.address_match & 0xfff),
-                                &val, size, MEMTXATTRS_UNSPECIFIED);
-        }
-        break;
+    if (rtl->enabled) {
+        data = rtl->addr ^ 0x80000000U; /* latch/complete */
+        trace_vfio_quirk_rtl8168_fake_latch(vdev->vbasedev.name, data);
     }
 
-    trace_vfio_rtl8168_quirk_read(vdev->vbasedev.name,
-                                  addr ? "address" : "data", val);
-    return val;
+    return data;
 }
 
-static void vfio_rtl8168_window_quirk_write(void *opaque, hwaddr addr,
-                                            uint64_t data, unsigned size)
+static void vfio_rtl8168_quirk_address_write(void *opaque, hwaddr addr,
+                                             uint64_t data, unsigned size)
 {
-    VFIOLegacyQuirk *quirk = opaque;
-    VFIOPCIDevice *vdev = quirk->vdev;
+    VFIOrtl8168Quirk *rtl = opaque;
+    VFIOPCIDevice *vdev = rtl->vdev;
 
-    switch (addr) {
-    case 4: /* address */
-        if ((data & 0x7fff0000) == 0x10000) { /* MSI-X table */
-            quirk->data.flags = 1; /* Activate reads */
-            quirk->data.address_match = data;
-
-            trace_vfio_rtl8168_quirk_write(vdev->vbasedev.name, data);
-
-            if (data & 0x80000000U) { /* Do write */
-                if (vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX) {
-                    hwaddr offset = data & 0xfff;
-                    uint64_t val = quirk->data.address_mask;
-
-                    trace_vfio_rtl8168_quirk_msix(vdev->vbasedev.name,
-                                                  (uint16_t)offset, val);
-
-                    /* Write to the proper guest MSI-X table instead */
-                    memory_region_dispatch_write(&vdev->pdev.msix_table_mmio,
-                                                 offset, val, size,
-                                                 MEMTXATTRS_UNSPECIFIED);
-                }
-                return; /* Do not write guest MSI-X data to hardware */
+    rtl->enabled = false;
+
+    if ((data & 0x7fff0000) == 0x10000) { /* MSI-X table */
+        rtl->enabled = true;
+        rtl->addr = (uint32_t)data;
+
+        if (data & 0x80000000U) { /* Do write */
+            if (vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX) {
+                hwaddr offset = data & 0xfff;
+                uint64_t val = rtl->data;
+
+                trace_vfio_quirk_rtl8168_msix_write(vdev->vbasedev.name,
+                                                    (uint16_t)offset, val);
+
+                /* Write to the proper guest MSI-X table instead */
+                memory_region_dispatch_write(&vdev->pdev.msix_table_mmio,
+                                             offset, val, size,
+                                             MEMTXATTRS_UNSPECIFIED);
             }
-        } else {
-            quirk->data.flags = 0; /* De-activate reads, non-MSI-X */
+            return; /* Do not write guest MSI-X data to hardware */
         }
-        break;
-    case 0: /* data */
-        quirk->data.address_mask = data;
-        break;
     }
 
-    vfio_region_write(&vdev->bars[quirk->data.bar].region,
-                      addr + 0x70, data, size);
+    vfio_region_write(&vdev->bars[2].region, addr + 0x74, data, size);
 }
 
-static const MemoryRegionOps vfio_rtl8168_window_quirk = {
-    .read = vfio_rtl8168_window_quirk_read,
-    .write = vfio_rtl8168_window_quirk_write,
+static const MemoryRegionOps vfio_rtl_address_quirk = {
+    .read = vfio_rtl8168_quirk_address_read,
+    .write = vfio_rtl8168_quirk_address_write,
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4,
@@ -867,32 +849,75 @@ static const MemoryRegionOps vfio_rtl8168_window_quirk = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static void vfio_probe_rtl8168_bar2_window_quirk(VFIOPCIDevice *vdev, int nr)
+static uint64_t vfio_rtl8168_quirk_data_read(void *opaque,
+                                             hwaddr addr, unsigned size)
+{
+    VFIOrtl8168Quirk *rtl = opaque;
+    VFIOPCIDevice *vdev = rtl->vdev;
+    uint64_t data = vfio_region_read(&vdev->bars[2].region, addr + 0x74, size);
+
+    if (rtl->enabled && (vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX)) {
+        hwaddr offset = rtl->addr & 0xfff;
+        memory_region_dispatch_read(&vdev->pdev.msix_table_mmio, offset,
+                                    &data, size, MEMTXATTRS_UNSPECIFIED);
+        trace_vfio_quirk_rtl8168_msix_read(vdev->vbasedev.name, offset, data);
+    }
+
+    return data;
+}
+
+static void vfio_rtl8168_quirk_data_write(void *opaque, hwaddr addr,
+                                          uint64_t data, unsigned size)
+{
+    VFIOrtl8168Quirk *rtl = opaque;
+    VFIOPCIDevice *vdev = rtl->vdev;
+
+    rtl->data = (uint32_t)data;
+
+    vfio_region_write(&vdev->bars[2].region, addr + 0x70, data, size);
+}
+
+static const MemoryRegionOps vfio_rtl_data_quirk = {
+    .read = vfio_rtl8168_quirk_data_read,
+    .write = vfio_rtl8168_quirk_data_write,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+        .unaligned = false,
+    },
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void vfio_probe_rtl8168_bar2_quirk(VFIOPCIDevice *vdev, int nr)
 {
-    PCIDevice *pdev = &vdev->pdev;
     VFIOQuirk *quirk;
-    VFIOLegacyQuirk *legacy;
+    VFIOrtl8168Quirk *rtl;
 
-    if (pci_get_word(pdev->config + PCI_VENDOR_ID) != PCI_VENDOR_ID_REALTEK ||
-        pci_get_word(pdev->config + PCI_DEVICE_ID) != 0x8168 || nr != 2) {
+    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_REALTEK, 0x8168) || nr != 2) {
         return;
     }
 
     quirk = g_malloc0(sizeof(*quirk));
-    quirk->data = legacy = g_malloc0(sizeof(*legacy));
-    quirk->mem = legacy->mem = g_malloc0_n(sizeof(MemoryRegion), 1);
-    quirk->nr_mem = 1;
-    legacy->vdev = vdev;
-    legacy->data.bar = nr;
+    quirk->mem = g_malloc0_n(sizeof(MemoryRegion), 2);
+    quirk->nr_mem = 2;
+    quirk->data = rtl = g_malloc0(sizeof(*rtl));
+    rtl->vdev = vdev;
+
+    memory_region_init_io(&quirk->mem[0], OBJECT(vdev),
+                          &vfio_rtl_address_quirk, rtl,
+                          "vfio-rtl8168-window-address-quirk", 4);
+    memory_region_add_subregion_overlap(&vdev->bars[nr].region.mem,
+                                        0x74, &quirk->mem[0], 1);
 
-    memory_region_init_io(quirk->mem, OBJECT(vdev), &vfio_rtl8168_window_quirk,
-                          legacy, "vfio-rtl8168-window-quirk", 8);
+    memory_region_init_io(&quirk->mem[1], OBJECT(vdev),
+                          &vfio_rtl_data_quirk, rtl,
+                          "vfio-rtl8168-window-data-quirk", 4);
     memory_region_add_subregion_overlap(&vdev->bars[nr].region.mem,
-                                        0x70, quirk->mem, 1);
+                                        0x70, &quirk->mem[1], 1);
 
     QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next);
 
-    trace_vfio_rtl8168_quirk_enable(vdev->vbasedev.name);
+    trace_vfio_quirk_rtl8168_probe(vdev->vbasedev.name);
 }
 
 /*
@@ -944,7 +969,7 @@ void vfio_bar_quirk_setup(VFIOPCIDevice *vdev, int nr)
     vfio_probe_nvidia_bar5_window_quirk(vdev, nr);
     vfio_probe_nvidia_bar0_88000_quirk(vdev, nr);
     vfio_probe_nvidia_bar0_1800_quirk(vdev, nr);
-    vfio_probe_rtl8168_bar2_window_quirk(vdev, nr);
+    vfio_probe_rtl8168_bar2_quirk(vdev, nr);
 }
 
 void vfio_bar_quirk_teardown(VFIOPCIDevice *vdev, int nr)
diff --git a/trace-events b/trace-events
index 3020b9a..b54411f 100644
--- a/trace-events
+++ b/trace-events
@@ -1552,10 +1552,6 @@ vfio_generic_quirk_read(const char * region_name, const char *name, int index, u
 vfio_generic_quirk_write(const char * region_name, const char *name, int index, uint64_t addr, uint64_t data, int size) "%s write(%s:BAR%d+0x%"PRIx64", 0x%"PRIx64", %d"
 vfio_probe_ati_bar4_window_quirk(const char *name) "Enabled ATI/AMD BAR4 window quirk for device %s"
 #issue with )
-vfio_rtl8168_quirk_read(const char *name, const char *type, uint64_t val) "%s [%s]: 0x%"PRIx64
-vfio_rtl8168_quirk_write(const char *name, uint64_t val) "%s [address]: 0x%"PRIx64
-vfio_rtl8168_quirk_msix(const char *name, uint16_t offset, uint64_t val) "%s MSI-X table write[0x%x]: 0x%"PRIx64
-vfio_rtl8168_quirk_enable(const char *name) "%s"
 vfio_probe_ati_bar2_4000_quirk(const char *name) "Enabled ATI/AMD BAR2 0x4000 quirk for device %s"
 vfio_probe_nvidia_bar5_window_quirk(const char *name) "Enabled NVIDIA BAR5 window quirk for device %s"
 vfio_probe_nvidia_bar0_88000_quirk(const char *name) "Enabled NVIDIA BAR0 0x88000 quirk for device %s"
@@ -1588,6 +1584,10 @@ vfio_quirk_nvidia_3d0_state(const char *name, const char *state) "%s %s"
 vfio_quirk_nvidia_3d0_read(const char *name, uint8_t offset, unsigned size, uint64_t val) " (%s, @0x%x, len=0x%x) %"PRIx64
 vfio_quirk_nvidia_3d0_write(const char *name, uint8_t offset, uint64_t data, unsigned size) "(%s, @0x%x, 0x%"PRIx64", len=0x%x)"
 vfio_quirk_nvidia_3d0_probe(const char *name) "%s"
+vfio_quirk_rtl8168_fake_latch(const char *name, uint64_t val) "%s 0x%"PRIx64
+vfio_quirk_rtl8168_msix_write(const char *name, uint16_t offset, uint64_t val) "%s MSI-X table write[0x%x]: 0x%"PRIx64
+vfio_quirk_rtl8168_msix_read(const char *name, uint16_t offset, uint64_t val) "%s MSI-X table read[0x%x]: 0x%"PRIx64
+vfio_quirk_rtl8168_probe(const char *name) "%s"
 
 # hw/vfio/vfio-common.c
 vfio_region_write(const char *name, int index, uint64_t addr, uint64_t data, unsigned size) " (%s:region%d+0x%"PRIx64", 0x%"PRIx64 ", %d)"