[Qemu-devel] [PATCH] pci-assign: add a way to blacklist loading of unstable roms

[Bandan Das]

commit 4b9430294ed added an option in vfio to blacklist
roms that are known to be unstable. Add a similar mechanism
for pci-assign as well. The default behavior is to disable
loading but can be overriden by specifying rombar or romfile

Signed-off-by: Bandan Das <address@hidden>
---
Note: ignored checkpatch reported error-
ERROR: need consistent spacing around '*' (ctx:WxV)

 hw/i386/kvm/pci-assign.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index a825871..42618dd 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -59,6 +59,29 @@
 #define DEBUG(fmt, ...)
 #endif
 
+typedef struct PCIRomBlacklistEntry {
+    uint16_t vendor_id;
+    uint16_t device_id;
+} PCIRomBlacklistEntry;
+
+/*
+ * List of device ids/vendor ids for which to disable
+ * option rom loading. This avoids the guest hangs during rom
+ * execution as noticed with the BCM 57810 card for lack of a
+ * more better way to handle such issues.
+ * The  user can still override by specifying a romfile or
+ * rombar=1.
+ * Please see https://bugs.launchpad.net/qemu/+bug/1284874
+ * for an analysis of the 57810 card hang. When adding
+ * a new vendor id/device id combination below, please also add
+ * your card/environment details and information that could
+ * help in debugging to the bug tracking this issue
+ */
+static const PCIRomBlacklistEntry romblacklist[] = {
+    /* Broadcom BCM 57810 */
+    { 0x14e4, 0x168e }
+};
+
 typedef struct PCIRegion {
     int type;           /* Memory or port I/O */
     int valid;
@@ -1834,6 +1857,26 @@ static void assign_register_types(void)
 
 type_init(assign_register_types)
 
+static bool blacklist_opt_rom(AssignedDevice *adev)
+{
+    PCIDevice *pdev = &adev->dev;
+    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);
+
+    while (count < ARRAY_SIZE(romblacklist)) {
+        if (romblacklist[count].vendor_id == vendor_id &&
+            romblacklist[count].device_id == device_id) {
+                return true;
+        }
+        count++;
+    }
+
+    return false;
+}
+
 /*
  * Scan the assigned devices for the devices that have an option ROM, and then
  * load the corresponding ROM data to RAM. If an error occurs while loading an
@@ -1846,9 +1889,19 @@ static void assigned_dev_load_option_rom(AssignedDevice 
*dev)
     uint8_t val;
     struct stat st;
     void *ptr;
+    DeviceState *devstate = DEVICE(dev);
 
     /* If loading ROM from file, pci handles it */
     if (dev->dev.romfile || !dev->dev.rom_bar) {
+        /* Since pci handles romfile, just print a message and return */
+        if (blacklist_opt_rom(dev) && dev->dev.romfile) {
+            error_printf("Warning : Device at %04x:%02x:%02x.%x "
+                         "is known to cause system instability issues during "
+                         "option rom execution. "
+                         "Proceeding anyway since user specified romfile\n",
+                         dev->host.domain, dev->host.bus, dev->host.slot,
+                         dev->host.function);
+        }
         return;
     }
 
@@ -1866,6 +1919,26 @@ static void assigned_dev_load_option_rom(AssignedDevice 
*dev)
         return;
     }
 
+    if (blacklist_opt_rom(dev)) {
+        if (devstate->opts && qemu_opt_get(devstate->opts, "rombar")) {
+            error_printf("Warning : Device at %04x:%02x:%02x.%x "
+                         "is known to cause system instability issues during "
+                         "option rom execution. "
+                         "Proceeding anyway since user specified non zero 
value for "
+                         "rombar\n",
+                         dev->host.domain, dev->host.bus, dev->host.slot,
+                         dev->host.function);
+        } else {
+            error_printf("Warning : Rom loading for device at "
+                         "%04x:%02x:%02x.%x has been disabled due to "
+                         "system instability issues. "
+                         "Specify rombar=1 or romfile to force\n",
+                         dev->host.domain, dev->host.bus, dev->host.slot,
+                         dev->host.function);
+            return;
+        }
+    }
+
     /* Write "1" to the ROM file to enable it */
     fp = fopen(rom_file, "r+");
     if (fp == NULL) {
-- 
1.8.3.1