[Qemu-devel] [PATCH 07/17] vfio/pci: Cleanup ROM blacklist quirk

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

Create a vendor:device ID helper that we'll also use as we rework the
rest of the quirks.  Re-reading the config entries, even if we get
more blacklist entries, is trivial overhead and only incurred during
device setup.  There's no need to typedef the blacklist structure,
it's a static private data type used once.  The elements get bumped
up to uint32_t to avoid future maintenance issues if PCI_ANY_ID gets
used for a blacklist entry (avoiding an actual hardware match).  Our
test loop is also crying out to be simplified as a for loop.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci-quirks.c |   40 +++++++++++++++++++++++++---------------
 hw/vfio/pci.h        |    5 -----
 trace-events         |    3 +++
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 1f9a809..17e300a 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -14,6 +14,19 @@
 #include "trace.h"
 #include "qemu/range.h"
 
+#define PCI_ANY_ID (~0)
+
+/* Use uin32_t for vendor & device so PCI_ANY_ID expands and cannot match hw */
+static bool vfio_pci_is(VFIOPCIDevice *vdev, uint32_t vendor, uint32_t device)
+{
+    PCIDevice *pdev = &vdev->pdev;
+
+    return (vendor == PCI_ANY_ID ||
+            vendor == pci_get_word(pdev->config + PCI_VENDOR_ID)) &&
+           (device == PCI_ANY_ID ||
+            device == pci_get_word(pdev->config + PCI_DEVICE_ID));
+}
+
 /*
  * List of device ids/vendor ids for which to disable
  * option rom loading. This avoids the guest hangs during rom
@@ -27,28 +40,25 @@
  * your card/environment details and information that could
  * help in debugging to the bug tracking this issue
  */
-static const VFIORomBlacklistEntry romblacklist[] = {
-    /* Broadcom BCM 57810 */
-    { 0x14e4, 0x168e }
+static const struct {
+    uint32_t vendor;
+    uint32_t device;
+} romblacklist[] = {
+    { 0x14e4, 0x168e }, /* Broadcom BCM 57810 */
 };
 
 bool vfio_blacklist_opt_rom(VFIOPCIDevice *vdev)
 {
-    PCIDevice *pdev = &vdev->pdev;
-    uint16_t vendor_id, device_id;
-    int count = 0;
-
-    vendor_id = pci_get_word(pdev->config + PCI_VENDOR_ID);
-    device_id = pci_get_word(pdev->config + PCI_DEVICE_ID);
+    int i;
 
-    while (count < ARRAY_SIZE(romblacklist)) {
-        if (romblacklist[count].vendor_id == vendor_id &&
-            romblacklist[count].device_id == device_id) {
-                return true;
+    for (i = 0 ; i < ARRAY_SIZE(romblacklist); i++) {
+        if (vfio_pci_is(vdev, romblacklist[i].vendor, romblacklist[i].device)) {
+            trace_vfio_quirk_rom_blacklisted(vdev->vbasedev.name,
+                                             romblacklist[i].vendor,
+                                             romblacklist[i].device);
+            return true;
         }
-        count++;
     }
-
     return false;
 }
 
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index ff94929..f6dbe7f 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -150,11 +150,6 @@ typedef struct VFIOPCIDevice {
     bool no_kvm_msix;
 } VFIOPCIDevice;
 
-typedef struct VFIORomBlacklistEntry {
-    uint16_t vendor_id;
-    uint16_t device_id;
-} VFIORomBlacklistEntry;
-
 uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len);
 void vfio_pci_write_config(PCIDevice *pdev,
                            uint32_t addr, uint32_t val, int len);
diff --git a/trace-events b/trace-events
index 67a5792..4b19d43 100644
--- a/trace-events
+++ b/trace-events
@@ -1572,6 +1572,9 @@ vfio_pci_reset(const char *name) " (%s)"
 vfio_pci_reset_flr(const char *name) "%s FLR/VFIO_DEVICE_RESET"
 vfio_pci_reset_pm(const char *name) "%s PCI PM Reset"
 
+# hw/vfio/pci-quirks.
+vfio_quirk_rom_blacklisted(const char *name, uint16_t vid, uint16_t did) "%s %04x:%04x"
+
 # 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)"
 vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t data) " (%s:region%d+0x%"PRIx64", %d) = 0x%"PRIx64