From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52886) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WIK8q-0003CD-2B for qemu-devel@nongnu.org; Tue, 25 Feb 2014 10:42:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WIK8h-0003dj-DW for qemu-devel@nongnu.org; Tue, 25 Feb 2014 10:41:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40708) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WIK8h-0003dQ-4p for qemu-devel@nongnu.org; Tue, 25 Feb 2014 10:41:47 -0500 Message-ID: <1393342902.9111.259.camel@ul30vt.home> From: Alex Williamson Date: Tue, 25 Feb 2014 08:41:42 -0700 In-Reply-To: <1393302864-11348-3-git-send-email-bsd@redhat.com> References: <1393302864-11348-1-git-send-email-bsd@redhat.com> <1393302864-11348-3-git-send-email-bsd@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2 v3] vfio: blacklist loading of unstable roms List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bandan Das Cc: Markus Armbruster , "Michael S. Tsirkin" , qemu-devel@nongnu.org, Andreas =?ISO-8859-1?Q?F=E4rber?= On Mon, 2014-02-24 at 23:34 -0500, Bandan Das wrote: > 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 > change blacklists loading of rom for such cards unless the user > specifies a romfile or rombar=1 on the cmd line > > Signed-off-by: Bandan Das > --- > hw/misc/vfio.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c > index 8db182f..df3ceee 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 VFIORomBlacklistEntry { > + uint16_t vendor_id; > + uint16_t device_id; > +} VFIORomBlacklistEntry; > + > +static const VFIORomBlacklistEntry romblacklist[] = { > + /* Broadcom BCM 57810 */ > + { 0x14e4, 0x168e } > +}; > + Any progress on a bug reference or trying to extract a version from the ROM so we can compare against future ROMs? We can always file a new bug in launchpad for tracking if needed. > #define MSIX_CAP_LENGTH 12 > > static QLIST_HEAD(, VFIOContainer) > @@ -1197,13 +1207,43 @@ static const MemoryRegionOps vfio_rom_ops = { > .endianness = DEVICE_LITTLE_ENDIAN, > }; > > +static bool vfio_blacklist_opt_rom(VFIODevice *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); > + > + while (count < ARRAY_SIZE(romblacklist)) { > + if (romblacklist[count].vendor_id == vendor_id && > + romblacklist[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; > + DeviceState *dev = DEVICE(vdev); > char name[32]; > > if (vdev->pdev.romfile || !vdev->pdev.rom_bar) { > + /* Since pci handles romfile, just print a message and return */ > + if (vfio_blacklist_opt_rom(vdev) && 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; > } > > @@ -1227,6 +1267,26 @@ static void vfio_pci_size_rom(VFIODevice *vdev) > return; > } > > + if (vfio_blacklist_opt_rom(vdev) && vdev->pdev.rom_bar) { We would have taken the return above if !rom_bar, so that test is unnecessary here. Thanks, Alex > + if (dev->opts && qemu_opt_get(dev->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", > + 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; > + } > + } > + > DPRINTF("%04x:%02x:%02x.%x ROM size 0x%x\n", vdev->host.domain, > vdev->host.bus, vdev->host.slot, vdev->host.function, size); >