[Qemu-devel] [PULL 0/4] vfio update and fix + pci-assign fix

[Qemu-devel] [PULL 0/4] vfio update and fix + pci-assign fix

[Alex Williamson]

The following changes since commit d5001cf787ad0514839a81d0f2e771e01e076e21:

  xilinx: Delete hw/include/xilinx.h (2014-02-26 14:54:45 +1000)

are available in the git repository at:

  git://github.com/awilliam/qemu-vfio.git tags/vfio-pci-for-qemu-20140226.0

for you to fetch changes up to 4b9430294ed406a00f045d825ada146aecf32309:

  vfio: blacklist loading of unstable roms (2014-02-26 10:33:45 -0700)

----------------------------------------------------------------
Updates include:
 - Coverify fixes for vfio & pci-assign (Markus)
 - VFIO blacklisting support for known brokwn PCI option ROMs (Bandan)

----------------------------------------------------------------
Bandan Das (2):
      qdev-monitor: set DeviceState opts before calling realize
      vfio: blacklist loading of unstable roms

Markus Armbruster (2):
      vfio: Fix overrun after readlink() fills buffer completely
      pci-assign: Fix potential read beyond buffer on -EBUSY

 hw/i386/kvm/pci-assign.c |  1 +
 hw/misc/vfio.c           | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
 qdev-monitor.c           |  4 ++-
 3 files changed, 80 insertions(+), 4 deletions(-)

[Qemu-devel] [PULL 1/4] vfio: Fix overrun after readlink() fills buffer completely

[Alex Williamson]

From: Markus Armbruster <armbru@redhat.com>

readlink() returns the number of bytes written to the buffer, and it
doesn't write a terminating null byte.  vfio_init() writes it itself.
Overruns the buffer when readlink() filled it completely.

Fix by treating readlink() filling the buffer completely as error,
like we do in pci-assign.c's assign_failed_examine().

Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/misc/vfio.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 8db182f..e669bbe 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -3681,10 +3681,10 @@ static int vfio_initfn(PCIDevice *pdev)
 
     strncat(path, "iommu_group", sizeof(path) - strlen(path) - 1);
 
-    len = readlink(path, iommu_group_path, PATH_MAX);
-    if (len <= 0) {
+    len = readlink(path, iommu_group_path, sizeof(path));
+    if (len <= 0 || len >= sizeof(path)) {
         error_report("vfio: error no iommu_group for device");
-        return -errno;
+        return len < 0 ? -errno : ENAMETOOLONG;
     }
 
     iommu_group_path[len] = 0;

[Qemu-devel] [PULL 2/4] pci-assign: Fix potential read beyond buffer on -EBUSY

[Alex Williamson]

From: Markus Armbruster <armbru@redhat.com>

readlink() doesn't write a terminating null byte.
assign_failed_examine() passes the unterminated string to strrchr().
Oops.  Terminate it.

Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/i386/kvm/pci-assign.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 9686801..a825871 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -743,6 +743,7 @@ static void assign_failed_examine(AssignedDevice *dev)
         goto fail;
     }
 
+    driver[r] = 0;
     ns = strrchr(driver, '/');
     if (!ns) {
         goto fail;

[Qemu-devel] [PULL 3/4] qdev-monitor: set DeviceState opts before calling realize

[Alex Williamson]

From: Bandan Das <bsd@redhat.com>

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>
Signed-off-by: Alex Williamson <alex.williamson@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;
 }
 

[Qemu-devel] [PULL 4/4] vfio: blacklist loading of unstable roms

[Alex Williamson]

From: Bandan Das <bsd@redhat.com>

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>
Signed-off-by: Alex Williamson <alex.williamson@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 e669bbe..c2c688c 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);
 

Re: [Qemu-devel] [PULL 0/4] vfio update and fix + pci-assign fix

[Alex Williamson]

On Wed, 2014-02-26 at 11:25 -0700, Alex Williamson wrote:
> The following changes since commit d5001cf787ad0514839a81d0f2e771e01e076e21:
> 
>   xilinx: Delete hw/include/xilinx.h (2014-02-26 14:54:45 +1000)
> 
> are available in the git repository at:
> 
>   git://github.com/awilliam/qemu-vfio.git tags/vfio-pci-for-qemu-20140226.0
> 
> for you to fetch changes up to 4b9430294ed406a00f045d825ada146aecf32309:
> 
>   vfio: blacklist loading of unstable roms (2014-02-26 10:33:45 -0700)
> 
> ----------------------------------------------------------------
> Updates include:
>  - Coverify fixes for vfio & pci-assign (Markus)
>  - VFIO blacklisting support for known brokwn PCI option ROMs (Bandan)
> 
> ----------------------------------------------------------------
> Bandan Das (2):
>       qdev-monitor: set DeviceState opts before calling realize
>       vfio: blacklist loading of unstable roms
> 
> Markus Armbruster (2):
>       vfio: Fix overrun after readlink() fills buffer completely
>       pci-assign: Fix potential read beyond buffer on -EBUSY
> 
>  hw/i386/kvm/pci-assign.c |  1 +
>  hw/misc/vfio.c           | 79 ++++++++++++++++++++++++++++++++++++++++++++++--
>  qdev-monitor.c           |  4 ++-
>  3 files changed, 80 insertions(+), 4 deletions(-)

Well, may qemu-pull alias was completely broken, but I trust the scripts
are scraping this from the list anyway.  Thanks,

Alex

Re: [Qemu-devel] [PULL 0/4] vfio update and fix + pci-assign fix

[Peter Maydell]

On 26 February 2014 18:25, Alex Williamson <alex.williamson@redhat.com> wrote:
> The following changes since commit d5001cf787ad0514839a81d0f2e771e01e076e21:
>
>   xilinx: Delete hw/include/xilinx.h (2014-02-26 14:54:45 +1000)
>
> are available in the git repository at:
>
>   git://github.com/awilliam/qemu-vfio.git tags/vfio-pci-for-qemu-20140226.0
>
> for you to fetch changes up to 4b9430294ed406a00f045d825ada146aecf32309:
>
>   vfio: blacklist loading of unstable roms (2014-02-26 10:33:45 -0700)

Applied, thanks.

-- PMM