* [Qemu-devel] [PATCH 1/2 v4] qdev-monitor: set DeviceState opts before calling realize
2014-02-25 23:47 [Qemu-devel] [PATCH 0/2 v4] vfio: blacklist loading of unstable roms Bandan Das
@ 2014-02-25 23:47 ` Bandan Das
2014-02-25 23:47 ` [Qemu-devel] [PATCH 2/2 v4] vfio: blacklist loading of unstable roms Bandan Das
2014-02-26 18:11 ` [Qemu-devel] [PATCH 0/2 " Alex Williamson
2 siblings, 0 replies; 4+ messages in thread
From: Bandan Das @ 2014-02-25 23:47 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Alex Williamson, Andreas Färber,
Michael S. Tsirkin
Setting opts before the realize property is set allows the
following patch to make decisions based on whether the user
specified "rombar". This also avoids having to create a new
tristate property especially for this purpose
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Bandan Das <bsd@redhat.com>
---
qdev-monitor.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 3a7dc0d..6673e3c 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -548,16 +548,18 @@ DeviceState *qdev_device_add(QemuOpts *opts)
OBJECT(dev), NULL);
g_free(name);
}
+
+ dev->opts = opts;
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err != NULL) {
qerror_report_err(err);
error_free(err);
+ dev->opts = NULL;
object_unparent(OBJECT(dev));
object_unref(OBJECT(dev));
qerror_report(QERR_DEVICE_INIT_FAILED, driver);
return NULL;
}
- dev->opts = opts;
return dev;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Qemu-devel] [PATCH 2/2 v4] vfio: blacklist loading of unstable roms
2014-02-25 23:47 [Qemu-devel] [PATCH 0/2 v4] vfio: blacklist loading of unstable roms Bandan Das
2014-02-25 23:47 ` [Qemu-devel] [PATCH 1/2 v4] qdev-monitor: set DeviceState opts before calling realize Bandan Das
@ 2014-02-25 23:47 ` Bandan Das
2014-02-26 18:11 ` [Qemu-devel] [PATCH 0/2 " Alex Williamson
2 siblings, 0 replies; 4+ messages in thread
From: Bandan Das @ 2014-02-25 23:47 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Alex Williamson, Andreas Färber,
Michael S. 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
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 <bsd@redhat.com>
---
hw/misc/vfio.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 8db182f..3a854f9 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -209,6 +209,29 @@ typedef struct VFIOGroup {
QLIST_ENTRY(VFIOGroup) container_next;
} VFIOGroup;
+typedef struct VFIORomBlacklistEntry {
+ uint16_t vendor_id;
+ uint16_t device_id;
+} VFIORomBlacklistEntry;
+
+/*
+ * 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 VFIORomBlacklistEntry romblacklist[] = {
+ /* Broadcom BCM 57810 */
+ { 0x14e4, 0x168e }
+};
+
#define MSIX_CAP_LENGTH 12
static QLIST_HEAD(, VFIOContainer)
@@ -1197,13 +1220,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 +1280,26 @@ static void vfio_pci_size_rom(VFIODevice *vdev)
return;
}
+ if (vfio_blacklist_opt_rom(vdev)) {
+ 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);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH 0/2 v4] vfio: blacklist loading of unstable roms
2014-02-25 23:47 [Qemu-devel] [PATCH 0/2 v4] vfio: blacklist loading of unstable roms Bandan Das
2014-02-25 23:47 ` [Qemu-devel] [PATCH 1/2 v4] qdev-monitor: set DeviceState opts before calling realize Bandan Das
2014-02-25 23:47 ` [Qemu-devel] [PATCH 2/2 v4] vfio: blacklist loading of unstable roms Bandan Das
@ 2014-02-26 18:11 ` Alex Williamson
2 siblings, 0 replies; 4+ messages in thread
From: Alex Williamson @ 2014-02-26 18:11 UTC (permalink / raw)
To: Bandan Das
Cc: Markus Armbruster, qemu-devel, Andreas Färber,
Michael S. Tsirkin
On Tue, 2014-02-25 at 18:47 -0500, Bandan Das wrote:
> v4:
> 1. Short Description of the issue in comments, pointer
> to qemu bug tracker in 2/2
> 2. Remove unnecessary check on rom_bar in vfio_pci_size_rom 2/2
> 3. No changes to 1/2 - Andreas, I have retained your "Reviewed-by"
> since the code hasn't changed.
>
> v3:
> 1. Change the "force" logic to depend on whether the user specified rombar,
> eliminate dependence on multiple values of rom_bar
> 2. Avoid printing the informational message if there's no rom
> 3. Minor changes to commit messages
>
> v2:
> 1. Break up into two patches separating the infrastructural changes
> 2. Change vendor_id and device_id type to uint16_t
> 3. Rename struct for blacklisted devid and vendorid to more meaningful names
> 4. Remove unnecessary rom_quirk variable and just call vfio_blacklist_opt_rom()
> in vfio_pci_size_rom()
>
> Bandan Das (2):
> qdev-monitor: set DeviceState opts before calling realize
> vfio: blacklist loading of unstable roms
>
> hw/misc/vfio.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qdev-monitor.c | 4 +++-
> 2 files changed, 76 insertions(+), 1 deletion(-)
>
Applied, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread