From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39266) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WG9lR-0007Uw-CI for qemu-devel@nongnu.org; Wed, 19 Feb 2014 11:12:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WG9lK-0005wQ-Sn for qemu-devel@nongnu.org; Wed, 19 Feb 2014 11:12:49 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37911) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WG9lK-0005w6-Gu for qemu-devel@nongnu.org; Wed, 19 Feb 2014 11:12:42 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1JGCf0V001254 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 19 Feb 2014 11:12:41 -0500 From: Bandan Das Date: Wed, 19 Feb 2014 11:12:40 -0500 Message-ID: MIME-Version: 1.0 Content-Type: text/plain Subject: [Qemu-devel] vfio: blacklist loading of unstable roms List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Alex Williamson , Michael Tsirkin Certain cards such as the Broadcom BCM57810 have rom quirks that exhibit unstable system behavior duing device assignment. In the particular case of 57810, rom execution hangs and if a FLR follows, the device becomes inoperable until a power cycle. This is a simple change to disable rom loading for such cards. In terms of implementation change, rombar now has a default value of 2. Existing code shouldn't be affected by changing the default value of rombar since all relevant decisions only rely on whether rom_bar is zero or non-zero. The motivation behind this change is that in certain cases such as a firmware upgrade, the user might want to override this blacklisting behavior and can do so by running with rombar = 1. Same reasoning applies to running with romfile. Signed-off-by: Bandan Das --- hw/misc/vfio.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/pci/pci.c | 3 ++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c index 8db182f..f5021f4 100644 --- a/hw/misc/vfio.c +++ b/hw/misc/vfio.c @@ -209,6 +209,16 @@ typedef struct VFIOGroup { QLIST_ENTRY(VFIOGroup) container_next; } VFIOGroup; +typedef struct VFIORomQList { + unsigned int vendor_id; + unsigned int device_id; +} VFIORomQList; + +static const VFIORomQList romqdevlist[] = { + /* Broadcom BCM 57810 */ + { 0x14e4, 0x168e } +}; + #define MSIX_CAP_LENGTH 12 static QLIST_HEAD(, VFIOContainer) @@ -1197,16 +1207,69 @@ static const MemoryRegionOps vfio_rom_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; +static bool vfio_blacklist_opt_rom(VFIODevice *vdev) +{ + PCIDevice *pdev = &vdev->pdev; + unsigned int 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(romqdevlist)) { + if (romqdevlist[count].vendor_id == vendor_id && + romqdevlist[count].device_id == device_id) { + return true; + } + count++; + } + + return false; +} + static void vfio_pci_size_rom(VFIODevice *vdev) { uint32_t orig, size = cpu_to_le32((uint32_t)PCI_ROM_ADDRESS_MASK); off_t offset = vdev->config_offset + PCI_ROM_ADDRESS; char name[32]; + int rom_quirk = 0; + + if (vfio_blacklist_opt_rom(vdev)) { + rom_quirk = 1; + } if (vdev->pdev.romfile || !vdev->pdev.rom_bar) { + /* Since pci handles romfile, just print a message and return */ + if (rom_quirk && vdev->pdev.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", + vdev->host.domain, vdev->host.bus, vdev->host.slot, + vdev->host.function); + } return; } + if (rom_quirk && vdev->pdev.rom_bar) { + if (vdev->pdev.rom_bar == 1) { + 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 rombar=1\n", + vdev->host.domain, vdev->host.bus, vdev->host.slot, + vdev->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", + vdev->host.domain, vdev->host.bus, vdev->host.slot, + vdev->host.function); + return; + } + } + /* * Use the same size ROM BAR as the physical device. The contents * will get filled in later when the guest tries to read it. diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 4e0701d..65766d8 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -53,7 +53,8 @@ static void pci_bus_finalize(Object *obj); static Property pci_props[] = { DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), DEFINE_PROP_STRING("romfile", PCIDevice, romfile), - DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1), + /* 0 = disable, 1 = user requested (on), 2 = default (on) */ + DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 2), DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present, QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false), DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present, -- 1.8.3.1