[Qemu-devel] [PULL 01/19] vfio/pci: Cleanup RTL8168 quirk and tracing

[Alex Williamson <alex.williamson@redhat.com>]

There's quite a bit of cleanup that can be done to the RTL8168 quirk,
as well as the tracing to prevent a spew of uninteresting accesses
for anything else the driver might choose to use the window registers
for besides the MSI-X table.  There should be no functional change,
but it's now possible to get compact and useful traces by enabling
vfio_rtl8168_quirk*, ex:

vfio_rtl8168_quirk_write 0000:04:00.0 [address]: 0x1f000
vfio_rtl8168_quirk_read 0000:04:00.0 [address]: 0x8001f000
vfio_rtl8168_quirk_read 0000:04:00.0 [data]: 0xfee0100c
vfio_rtl8168_quirk_write 0000:04:00.0 [address]: 0x1f004
vfio_rtl8168_quirk_read 0000:04:00.0 [address]: 0x8001f004
vfio_rtl8168_quirk_read 0000:04:00.0 [data]: 0x0
vfio_rtl8168_quirk_write 0000:04:00.0 [address]: 0x1f008
vfio_rtl8168_quirk_read 0000:04:00.0 [address]: 0x8001f008
vfio_rtl8168_quirk_read 0000:04:00.0 [data]: 0x49b1
vfio_rtl8168_quirk_write 0000:04:00.0 [address]: 0x1f00c
vfio_rtl8168_quirk_read 0000:04:00.0 [address]: 0x8001f00c
vfio_rtl8168_quirk_read 0000:04:00.0 [data]: 0x0

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   88 ++++++++++++++++++++++++---------------------------------
 trace-events  |   10 +++---
 2 files changed, 41 insertions(+), 57 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 73d34b9..77f92f1 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1508,44 +1508,29 @@ static uint64_t vfio_rtl8168_window_quirk_read(void *opaque,
 {
     VFIOQuirk *quirk = opaque;
     VFIOPCIDevice *vdev = quirk->vdev;
+    uint64_t val = 0;
+
+    if (!quirk->data.flags) { /* Non-MSI-X table access */
+        return vfio_region_read(&vdev->bars[quirk->data.bar].region,
+                                addr + 0x70, size);
+    }
 
     switch (addr) {
     case 4: /* address */
-        if (quirk->data.flags) {
-            trace_vfio_rtl8168_window_quirk_read_fake(
-                    memory_region_name(&quirk->mem),
-                    vdev->vbasedev.name);
-
-            return quirk->data.address_match ^ 0x80000000U;
-        }
+        val = quirk->data.address_match ^ 0x80000000U; /* latch/complete */
         break;
     case 0: /* data */
-        if (quirk->data.flags) {
-            uint64_t val;
-
-            trace_vfio_rtl8168_window_quirk_read_table(
-                    memory_region_name(&quirk->mem),
-                    vdev->vbasedev.name);
-
-            if (!(vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX)) {
-                return 0;
-            }
-
+        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);
-            return val;
+                                (hwaddr)(quirk->data.address_match & 0xfff),
+                                &val, size, MEMTXATTRS_UNSPECIFIED);
         }
+        break;
     }
 
-    trace_vfio_rtl8168_window_quirk_read_direct(memory_region_name(&quirk->mem),
-                                                vdev->vbasedev.name);
-
-    return vfio_region_read(&vdev->bars[quirk->data.bar].region,
-                            addr + 0x70, size);
+    trace_vfio_rtl8168_quirk_read(vdev->vbasedev.name,
+                                  addr ? "address" : "data", val);
+    return val;
 }
 
 static void vfio_rtl8168_window_quirk_write(void *opaque, hwaddr addr,
@@ -1556,36 +1541,36 @@ static void vfio_rtl8168_window_quirk_write(void *opaque, hwaddr addr,
 
     switch (addr) {
     case 4: /* address */
-        if ((data & 0x7fff0000) == 0x10000) {
-            if (data & 0x80000000U &&
-                vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX) {
-
-                trace_vfio_rtl8168_window_quirk_write_table(
-                        memory_region_name(&quirk->mem),
-                        vdev->vbasedev.name);
-
-                memory_region_dispatch_write(&vdev->pdev.msix_table_mmio,
-                                             (hwaddr)(data & 0xfff),
-                                             (uint64_t)quirk->data.address_mask,
-                                             size, MEMTXATTRS_UNSPECIFIED);
-            }
-
-            quirk->data.flags = 1;
+        if ((data & 0x7fff0000) == 0x10000) { /* MSI-X table */
+            quirk->data.flags = 1; /* Activate reads */
             quirk->data.address_match = data;
 
-            return;
+            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 */
+            }
+        } else {
+            quirk->data.flags = 0; /* De-activate reads, non-MSI-X */
         }
-        quirk->data.flags = 0;
         break;
     case 0: /* data */
         quirk->data.address_mask = data;
         break;
     }
 
-    trace_vfio_rtl8168_window_quirk_write_direct(
-            memory_region_name(&quirk->mem),
-            vdev->vbasedev.name);
-
     vfio_region_write(&vdev->bars[quirk->data.bar].region,
                       addr + 0x70, data, size);
 }
@@ -1622,8 +1607,9 @@ static void vfio_probe_rtl8168_bar2_window_quirk(VFIOPCIDevice *vdev, int nr)
 
     QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, quirk, next);
 
-    trace_vfio_probe_rtl8168_bar2_window_quirk(vdev->vbasedev.name);
+    trace_vfio_rtl8168_quirk_enable(vdev->vbasedev.name);
 }
+
 /*
  * Trap the BAR2 MMIO window to config space as well.
  */
diff --git a/trace-events b/trace-events
index 88a2f14..eb00342 100644
--- a/trace-events
+++ b/trace-events
@@ -1554,12 +1554,10 @@ vfio_ati_3c3_quirk_read(uint64_t data) " (0x3c3, 1) = 0x%"PRIx64
 vfio_vga_probe_ati_3c3_quirk(const char *name) "Enabled ATI/AMD quirk 0x3c3 BAR4for device %s"
 vfio_probe_ati_bar4_window_quirk(const char *name) "Enabled ATI/AMD BAR4 window quirk for device %s"
 #issue with )
-vfio_rtl8168_window_quirk_read_fake(const char *region_name, const char *name) "%s fake read(%s"
-vfio_rtl8168_window_quirk_read_table(const char *region_name, const char *name) "%s MSI-X table read(%s"
-vfio_rtl8168_window_quirk_read_direct(const char *region_name, const char *name) "%s direct read(%s"
-vfio_rtl8168_window_quirk_write_table(const char *region_name, const char *name) "%s MSI-X table write(%s"
-vfio_rtl8168_window_quirk_write_direct(const char *region_name, const char *name) "%s direct write(%s"
-vfio_probe_rtl8168_bar2_window_quirk(const char *name) "Enabled RTL8168 BAR2 window quirk for device %s"
+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_nvidia_3d0_quirk_read(int size, uint64_t data) " (0x3d0, %d) = 0x%"PRIx64
 vfio_nvidia_3d0_quirk_write(uint64_t data, int size) " (0x3d0, 0x%"PRIx64", %d)"