All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17
@ 2016-10-17 19:51 Alex Williamson
  2016-10-17 19:51 ` [Qemu-devel] [PULL 01/19] vfio/pci: Use local error object in vfio_initfn Alex Williamson
                   ` (19 more replies)
  0 siblings, 20 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:51 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 0975b8b823a888d474fa33821dfe84e6904db197:

  Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging (2016-10-17 16:17:51 +0100)

are available in the git repository at:


  git://github.com/awilliam/qemu-vfio.git tags/vfio-updates-20161017.0

for you to fetch changes up to 893bfc3cc893ed36cedc364e99cf483e9b08c294:

  vfio: fix duplicate function call (2016-10-17 10:58:03 -0600)

----------------------------------------------------------------
VFIO updates 2016-10-17

 - Convert to realize & improve error reporting (Eric Auger)
 - RTL quirk bug fix (Thorsten Kohfeldt)
 - Skip duplicate pre/post reset (Cao jin)

----------------------------------------------------------------
Cao jin (1):
      vfio: fix duplicate function call

Eric Auger (17):
      vfio/pci: Use local error object in vfio_initfn
      vfio/pci: Pass an error object to vfio_populate_vga
      vfio/pci: Pass an error object to vfio_populate_device
      vfio/pci: Pass an error object to vfio_msix_early_setup
      vfio/pci: Pass an error object to vfio_intx_enable
      vfio/pci: Pass an error object to vfio_add_capabilities
      vfio/pci: Pass an error object to vfio_pci_igd_opregion_init
      vfio: Pass an Error object to vfio_connect_container
      vfio: Pass an error object to vfio_get_group
      vfio: Pass an error object to vfio_get_device
      vfio/platform: Pass an error object to vfio_populate_device
      vfio/platform: fix a wrong returned value in vfio_populate_device
      vfio/platform: Pass an error object to vfio_base_device_init
      vfio/pci: Conversion to realize
      vfio/pci: Remove vfio_msix_early_setup returned value
      vfio/pci: Remove vfio_populate_device returned value
      vfio/pci: Handle host oversight

Thorsten Kohfeldt (1):
      vfio/pci: Fix vfio_rtl8168_quirk_data_read address offset

 hw/vfio/common.c              |  69 ++++++-----
 hw/vfio/pci-quirks.c          |  16 +--
 hw/vfio/pci.c                 | 279 ++++++++++++++++++++++++------------------
 hw/vfio/pci.h                 |   5 +-
 hw/vfio/platform.c            |  66 +++++-----
 hw/vfio/trace-events          |   2 +-
 include/hw/vfio/vfio-common.h |   7 +-
 7 files changed, 253 insertions(+), 191 deletions(-)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 01/19] vfio/pci: Use local error object in vfio_initfn
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
@ 2016-10-17 19:51 ` Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 02/19] vfio/pci: Pass an error object to vfio_populate_vga Alex Williamson
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:51 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

To prepare for migration to realize, let's use a local error
object in vfio_initfn. Also let's use the same error prefix for all
error messages.

On top of the 1-1 conversion, we start using a common error prefix for
all error messages. We also introduce a similar warning prefix which will
be used later on.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c                 |   72 ++++++++++++++++++++++++-----------------
 include/hw/vfio/vfio-common.h |    3 ++
 2 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index a5a620a..417bf7f 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2493,6 +2493,7 @@ static int vfio_initfn(PCIDevice *pdev)
     VFIODevice *vbasedev_iter;
     VFIOGroup *group;
     char *tmp, group_path[PATH_MAX], *group_name;
+    Error *err = NULL;
     ssize_t len;
     struct stat st;
     int groupid;
@@ -2506,9 +2507,9 @@ static int vfio_initfn(PCIDevice *pdev)
     }
 
     if (stat(vdev->vbasedev.sysfsdev, &st) < 0) {
-        error_report("vfio: error: no such host device: %s",
-                     vdev->vbasedev.sysfsdev);
-        return -errno;
+        error_setg_errno(&err, errno, "no such host device");
+        ret = -errno;
+        goto error;
     }
 
     vdev->vbasedev.name = g_strdup(basename(vdev->vbasedev.sysfsdev));
@@ -2520,40 +2521,43 @@ static int vfio_initfn(PCIDevice *pdev)
     g_free(tmp);
 
     if (len <= 0 || len >= sizeof(group_path)) {
-        error_report("vfio: error no iommu_group for device");
-        return len < 0 ? -errno : -ENAMETOOLONG;
+        ret = len < 0 ? -errno : -ENAMETOOLONG;
+        error_setg_errno(&err, -ret, "no iommu_group found");
+        goto error;
     }
 
     group_path[len] = 0;
 
     group_name = basename(group_path);
     if (sscanf(group_name, "%d", &groupid) != 1) {
-        error_report("vfio: error reading %s: %m", group_path);
-        return -errno;
+        error_setg_errno(&err, errno, "failed to read %s", group_path);
+        ret = -errno;
+        goto error;
     }
 
     trace_vfio_initfn(vdev->vbasedev.name, groupid);
 
     group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev));
     if (!group) {
-        error_report("vfio: failed to get group %d", groupid);
-        return -ENOENT;
+        error_setg(&err, "failed to get group %d", groupid);
+        ret = -ENOENT;
+        goto error;
     }
 
     QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
         if (strcmp(vbasedev_iter->name, vdev->vbasedev.name) == 0) {
-            error_report("vfio: error: device %s is already attached",
-                         vdev->vbasedev.name);
+            error_setg(&err, "device is already attached");
             vfio_put_group(group);
-            return -EBUSY;
+            ret = -EBUSY;
+            goto error;
         }
     }
 
     ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev);
     if (ret) {
-        error_report("vfio: failed to get device %s", vdev->vbasedev.name);
+        error_setg_errno(&err, -ret, "failed to get device");
         vfio_put_group(group);
-        return ret;
+        goto error;
     }
 
     ret = vfio_populate_device(vdev);
@@ -2567,8 +2571,8 @@ static int vfio_initfn(PCIDevice *pdev)
                 vdev->config_offset);
     if (ret < (int)MIN(pci_config_size(&vdev->pdev), vdev->config_size)) {
         ret = ret < 0 ? -errno : -EFAULT;
-        error_report("vfio: Failed to read device config space");
-        return ret;
+        error_setg_errno(&err, -ret, "failed to read device config space");
+        goto error;
     }
 
     /* vfio emulates a lot for us, but some bits need extra love */
@@ -2584,8 +2588,9 @@ static int vfio_initfn(PCIDevice *pdev)
      */
     if (vdev->vendor_id != PCI_ANY_ID) {
         if (vdev->vendor_id >= 0xffff) {
-            error_report("vfio: Invalid PCI vendor ID provided");
-            return -EINVAL;
+            error_setg(&err, "invalid PCI vendor ID provided");
+            ret = -EINVAL;
+            goto error;
         }
         vfio_add_emulated_word(vdev, PCI_VENDOR_ID, vdev->vendor_id, ~0);
         trace_vfio_pci_emulated_vendor_id(vdev->vbasedev.name, vdev->vendor_id);
@@ -2595,8 +2600,9 @@ static int vfio_initfn(PCIDevice *pdev)
 
     if (vdev->device_id != PCI_ANY_ID) {
         if (vdev->device_id > 0xffff) {
-            error_report("vfio: Invalid PCI device ID provided");
-            return -EINVAL;
+            error_setg(&err, "invalid PCI device ID provided");
+            ret = -EINVAL;
+            goto error;
         }
         vfio_add_emulated_word(vdev, PCI_DEVICE_ID, vdev->device_id, ~0);
         trace_vfio_pci_emulated_device_id(vdev->vbasedev.name, vdev->device_id);
@@ -2606,8 +2612,9 @@ static int vfio_initfn(PCIDevice *pdev)
 
     if (vdev->sub_vendor_id != PCI_ANY_ID) {
         if (vdev->sub_vendor_id > 0xffff) {
-            error_report("vfio: Invalid PCI subsystem vendor ID provided");
-            return -EINVAL;
+            error_setg(&err, "invalid PCI subsystem vendor ID provided");
+            ret = -EINVAL;
+            goto error;
         }
         vfio_add_emulated_word(vdev, PCI_SUBSYSTEM_VENDOR_ID,
                                vdev->sub_vendor_id, ~0);
@@ -2617,8 +2624,9 @@ static int vfio_initfn(PCIDevice *pdev)
 
     if (vdev->sub_device_id != PCI_ANY_ID) {
         if (vdev->sub_device_id > 0xffff) {
-            error_report("vfio: Invalid PCI subsystem device ID provided");
-            return -EINVAL;
+            error_setg(&err, "invalid PCI subsystem device ID provided");
+            ret = -EINVAL;
+            goto error;
         }
         vfio_add_emulated_word(vdev, PCI_SUBSYSTEM_ID, vdev->sub_device_id, ~0);
         trace_vfio_pci_emulated_sub_device_id(vdev->vbasedev.name,
@@ -2671,8 +2679,9 @@ static int vfio_initfn(PCIDevice *pdev)
         struct vfio_region_info *opregion;
 
         if (vdev->pdev.qdev.hotplugged) {
-            error_report("Cannot support IGD OpRegion feature on hotplugged "
-                         "device %s", vdev->vbasedev.name);
+            error_setg(&err,
+                       "cannot support IGD OpRegion feature on hotplugged "
+                       "device");
             ret = -EINVAL;
             goto out_teardown;
         }
@@ -2681,16 +2690,15 @@ static int vfio_initfn(PCIDevice *pdev)
                         VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
                         VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
         if (ret) {
-            error_report("Device %s does not support requested IGD OpRegion "
-                         "feature", vdev->vbasedev.name);
+            error_setg_errno(&err, -ret,
+                             "does not support requested IGD OpRegion feature");
             goto out_teardown;
         }
 
         ret = vfio_pci_igd_opregion_init(vdev, opregion);
         g_free(opregion);
         if (ret) {
-            error_report("Device %s IGD OpRegion initialization failed",
-                         vdev->vbasedev.name);
+            error_setg_errno(&err, -ret, "IGD OpRegion initialization failed");
             goto out_teardown;
         }
     }
@@ -2726,6 +2734,10 @@ out_teardown:
     pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
     vfio_teardown_msi(vdev);
     vfio_bars_exit(vdev);
+error:
+    if (err) {
+        error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
+    }
     return ret;
 }
 
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index c17602e..b26b6cf 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -30,6 +30,9 @@
 #include <linux/vfio.h>
 #endif
 
+#define ERR_PREFIX "vfio error: %s: "
+#define WARN_PREFIX "vfio warning: %s: "
+
 /*#define DEBUG_VFIO*/
 #ifdef DEBUG_VFIO
 #define DPRINTF(fmt, ...) \

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 02/19] vfio/pci: Pass an error object to vfio_populate_vga
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
  2016-10-17 19:51 ` [Qemu-devel] [PULL 01/19] vfio/pci: Use local error object in vfio_initfn Alex Williamson
@ 2016-10-17 19:52 ` Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 03/19] vfio/pci: Pass an error object to vfio_populate_device Alex Williamson
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for the same operation in
vfio_populate_device. Eventually this contributes to the migration
to VFIO-PCI realize.

We now report an error on vfio_get_region_info failure.

vfio_probe_igd_bar4_quirk is not involved in the migration to realize
and simply calls error_reportf_err.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci-quirks.c |    4 +++-
 hw/vfio/pci.c        |   19 ++++++++++++-------
 hw/vfio/pci.h        |    2 +-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index bec694c..806ea5d 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1363,6 +1363,7 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     uint64_t *bdsm_size;
     uint32_t gmch;
     uint16_t cmd_orig, cmd;
+    Error *err = NULL;
 
     /*
      * This must be an Intel VGA device at address 00:02.0 for us to even
@@ -1464,7 +1465,8 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
      * try to enable it.  Probably shouldn't be using legacy mode without VGA,
      * but also no point in us enabling VGA if disabled in hardware.
      */
-    if (!(gmch & 0x2) && !vdev->vga && vfio_populate_vga(vdev)) {
+    if (!(gmch & 0x2) && !vdev->vga && vfio_populate_vga(vdev, &err)) {
+        error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
         error_report("IGD device %s failed to enable VGA access, "
                      "legacy mode disabled", vdev->vbasedev.name);
         goto out;
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 417bf7f..9645a77 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2134,7 +2134,7 @@ static VFIODeviceOps vfio_pci_ops = {
     .vfio_eoi = vfio_intx_eoi,
 };
 
-int vfio_populate_vga(VFIOPCIDevice *vdev)
+int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
 {
     VFIODevice *vbasedev = &vdev->vbasedev;
     struct vfio_region_info *reg_info;
@@ -2142,15 +2142,18 @@ int vfio_populate_vga(VFIOPCIDevice *vdev)
 
     ret = vfio_get_region_info(vbasedev, VFIO_PCI_VGA_REGION_INDEX, &reg_info);
     if (ret) {
+        error_setg_errno(errp, -ret,
+                         "failed getting region info for VGA region index %d",
+                         VFIO_PCI_VGA_REGION_INDEX);
         return ret;
     }
 
     if (!(reg_info->flags & VFIO_REGION_INFO_FLAG_READ) ||
         !(reg_info->flags & VFIO_REGION_INFO_FLAG_WRITE) ||
         reg_info->size < 0xbffff + 1) {
-        error_report("vfio: Unexpected VGA info, flags 0x%lx, size 0x%lx",
-                     (unsigned long)reg_info->flags,
-                     (unsigned long)reg_info->size);
+        error_setg(errp, "unexpected VGA info, flags 0x%lx, size 0x%lx",
+                   (unsigned long)reg_info->flags,
+                   (unsigned long)reg_info->size);
         g_free(reg_info);
         return -EINVAL;
     }
@@ -2205,6 +2208,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
     struct vfio_region_info *reg_info;
     struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
     int i, ret = -1;
+    Error *err = NULL;
 
     /* Sanity check device */
     if (!(vbasedev->flags & VFIO_DEVICE_FLAGS_PCI)) {
@@ -2259,10 +2263,11 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
     g_free(reg_info);
 
     if (vdev->features & VFIO_FEATURE_ENABLE_VGA) {
-        ret = vfio_populate_vga(vdev);
+        ret = vfio_populate_vga(vdev, &err);
         if (ret) {
-            error_report(
-                "vfio: Device does not support requested feature x-vga");
+            error_append_hint(&err, "device does not support "
+                              "requested feature x-vga\n");
+            error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
             goto error;
         }
     }
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 7d482d9..87a62f9 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -161,7 +161,7 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr);
 void vfio_bar_quirk_finalize(VFIOPCIDevice *vdev, int nr);
 void vfio_setup_resetfn_quirk(VFIOPCIDevice *vdev);
 
-int vfio_populate_vga(VFIOPCIDevice *vdev);
+int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp);
 
 int vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
                                struct vfio_region_info *info);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 03/19] vfio/pci: Pass an error object to vfio_populate_device
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
  2016-10-17 19:51 ` [Qemu-devel] [PULL 01/19] vfio/pci: Use local error object in vfio_initfn Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 02/19] vfio/pci: Pass an error object to vfio_populate_vga Alex Williamson
@ 2016-10-17 19:52 ` Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 04/19] vfio/pci: Pass an error object to vfio_msix_early_setup Alex Williamson
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for migration to VFIO-PCI realize.
The returned value will be removed later on.

The case where error recovery cannot be enabled is not converted into
an error object but directly reported through error_report, as before.
Populating an error instead would cause the future realize function to
fail, which is not wanted.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9645a77..46e3cb8 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2202,28 +2202,27 @@ int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
     return 0;
 }
 
-static int vfio_populate_device(VFIOPCIDevice *vdev)
+static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
 {
     VFIODevice *vbasedev = &vdev->vbasedev;
     struct vfio_region_info *reg_info;
     struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
     int i, ret = -1;
-    Error *err = NULL;
 
     /* Sanity check device */
     if (!(vbasedev->flags & VFIO_DEVICE_FLAGS_PCI)) {
-        error_report("vfio: Um, this isn't a PCI device");
+        error_setg(errp, "this isn't a PCI device");
         goto error;
     }
 
     if (vbasedev->num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
-        error_report("vfio: unexpected number of io regions %u",
-                     vbasedev->num_regions);
+        error_setg(errp, "unexpected number of io regions %u",
+                   vbasedev->num_regions);
         goto error;
     }
 
     if (vbasedev->num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
-        error_report("vfio: unexpected number of irqs %u", vbasedev->num_irqs);
+        error_setg(errp, "unexpected number of irqs %u", vbasedev->num_irqs);
         goto error;
     }
 
@@ -2235,7 +2234,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
         g_free(name);
 
         if (ret) {
-            error_report("vfio: Error getting region %d info: %m", i);
+            error_setg_errno(errp, -ret, "failed to get region %d info", i);
             goto error;
         }
 
@@ -2245,7 +2244,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
     ret = vfio_get_region_info(vbasedev,
                                VFIO_PCI_CONFIG_REGION_INDEX, &reg_info);
     if (ret) {
-        error_report("vfio: Error getting config info: %m");
+        error_setg_errno(errp, -ret, "failed to get config info");
         goto error;
     }
 
@@ -2263,11 +2262,10 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
     g_free(reg_info);
 
     if (vdev->features & VFIO_FEATURE_ENABLE_VGA) {
-        ret = vfio_populate_vga(vdev, &err);
+        ret = vfio_populate_vga(vdev, errp);
         if (ret) {
-            error_append_hint(&err, "device does not support "
+            error_append_hint(errp, "device does not support "
                               "requested feature x-vga\n");
-            error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
             goto error;
         }
     }
@@ -2282,7 +2280,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev)
     } else if (irq_info.count == 1) {
         vdev->pci_aer = true;
     } else {
-        error_report("vfio: %s "
+        error_report(WARN_PREFIX
                      "Could not enable error recovery for the device",
                      vbasedev->name);
     }
@@ -2565,9 +2563,9 @@ static int vfio_initfn(PCIDevice *pdev)
         goto error;
     }
 
-    ret = vfio_populate_device(vdev);
+    ret = vfio_populate_device(vdev, &err);
     if (ret) {
-        return ret;
+        goto error;
     }
 
     /* Get a copy of config space */

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 04/19] vfio/pci: Pass an error object to vfio_msix_early_setup
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (2 preceding siblings ...)
  2016-10-17 19:52 ` [Qemu-devel] [PULL 03/19] vfio/pci: Pass an error object to vfio_populate_device Alex Williamson
@ 2016-10-17 19:52 ` Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 05/19] vfio/pci: Pass an error object to vfio_intx_enable Alex Williamson
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for migration to VFIO-PCI realize.
The returned value will be removed later on.

We now format an error in case of reading failure for
- the MSIX flags
- the MSIX table,
- the MSIX PBA.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 46e3cb8..02e92b0 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1277,7 +1277,7 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
  * need to first look for where the MSI-X table lives.  So we
  * unfortunately split MSI-X setup across two functions.
  */
-static int vfio_msix_early_setup(VFIOPCIDevice *vdev)
+static int vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
 {
     uint8_t pos;
     uint16_t ctrl;
@@ -1292,16 +1292,19 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev)
 
     if (pread(fd, &ctrl, sizeof(ctrl),
               vdev->config_offset + pos + PCI_MSIX_FLAGS) != sizeof(ctrl)) {
+        error_setg_errno(errp, errno, "failed to read PCI MSIX FLAGS");
         return -errno;
     }
 
     if (pread(fd, &table, sizeof(table),
               vdev->config_offset + pos + PCI_MSIX_TABLE) != sizeof(table)) {
+        error_setg_errno(errp, errno, "failed to read PCI MSIX TABLE");
         return -errno;
     }
 
     if (pread(fd, &pba, sizeof(pba),
               vdev->config_offset + pos + PCI_MSIX_PBA) != sizeof(pba)) {
+        error_setg_errno(errp, errno, "failed to read PCI MSIX PBA");
         return -errno;
     }
 
@@ -1332,8 +1335,8 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev)
             (vdev->device_id & 0xff00) == 0x5800) {
             msix->pba_offset = 0x1000;
         } else {
-            error_report("vfio: Hardware reports invalid configuration, "
-                         "MSIX PBA outside of specified BAR");
+            error_setg(errp, "hardware reports invalid configuration, "
+                       "MSIX PBA outside of specified BAR");
             g_free(msix);
             return -EINVAL;
         }
@@ -2657,9 +2660,9 @@ static int vfio_initfn(PCIDevice *pdev)
 
     vfio_pci_size_rom(vdev);
 
-    ret = vfio_msix_early_setup(vdev);
+    ret = vfio_msix_early_setup(vdev, &err);
     if (ret) {
-        return ret;
+        goto error;
     }
 
     vfio_bars_setup(vdev);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 05/19] vfio/pci: Pass an error object to vfio_intx_enable
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (3 preceding siblings ...)
  2016-10-17 19:52 ` [Qemu-devel] [PULL 04/19] vfio/pci: Pass an error object to vfio_msix_early_setup Alex Williamson
@ 2016-10-17 19:52 ` Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 06/19] vfio/pci: Pass an error object to vfio_add_capabilities Alex Williamson
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for migration to VFIO-PCI realize.

The error object is propagated down to vfio_intx_enable_kvm().

The three other callers, vfio_intx_enable_kvm(), vfio_msi_disable_common()
and vfio_pci_post_reset() do not propagate the error and simply call
error_reportf_err() with the ERR_PREFIX formatting.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 02e92b0..42161c8 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -100,7 +100,7 @@ static void vfio_intx_eoi(VFIODevice *vbasedev)
     vfio_unmask_single_irqindex(vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
 }
 
-static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev)
+static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev, Error **errp)
 {
 #ifdef CONFIG_KVM
     struct kvm_irqfd irqfd = {
@@ -126,7 +126,7 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev)
 
     /* Get an eventfd for resample/unmask */
     if (event_notifier_init(&vdev->intx.unmask, 0)) {
-        error_report("vfio: Error: event_notifier_init failed eoi");
+        error_setg(errp, "event_notifier_init failed eoi");
         goto fail;
     }
 
@@ -134,7 +134,7 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev)
     irqfd.resamplefd = event_notifier_get_fd(&vdev->intx.unmask);
 
     if (kvm_vm_ioctl(kvm_state, KVM_IRQFD, &irqfd)) {
-        error_report("vfio: Error: Failed to setup resample irqfd: %m");
+        error_setg_errno(errp, errno, "failed to setup resample irqfd");
         goto fail_irqfd;
     }
 
@@ -153,7 +153,7 @@ static void vfio_intx_enable_kvm(VFIOPCIDevice *vdev)
     ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
     g_free(irq_set);
     if (ret) {
-        error_report("vfio: Error: Failed to setup INTx unmask fd: %m");
+        error_setg_errno(errp, -ret, "failed to setup INTx unmask fd");
         goto fail_vfio;
     }
 
@@ -222,6 +222,7 @@ static void vfio_intx_update(PCIDevice *pdev)
 {
     VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
     PCIINTxRoute route;
+    Error *err = NULL;
 
     if (vdev->interrupt != VFIO_INT_INTx) {
         return;
@@ -244,18 +245,22 @@ static void vfio_intx_update(PCIDevice *pdev)
         return;
     }
 
-    vfio_intx_enable_kvm(vdev);
+    vfio_intx_enable_kvm(vdev, &err);
+    if (err) {
+        error_reportf_err(err, WARN_PREFIX, vdev->vbasedev.name);
+    }
 
     /* Re-enable the interrupt in cased we missed an EOI */
     vfio_intx_eoi(&vdev->vbasedev);
 }
 
-static int vfio_intx_enable(VFIOPCIDevice *vdev)
+static int vfio_intx_enable(VFIOPCIDevice *vdev, Error **errp)
 {
     uint8_t pin = vfio_pci_read_config(&vdev->pdev, PCI_INTERRUPT_PIN, 1);
     int ret, argsz;
     struct vfio_irq_set *irq_set;
     int32_t *pfd;
+    Error *err = NULL;
 
     if (!pin) {
         return 0;
@@ -279,7 +284,7 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev)
 
     ret = event_notifier_init(&vdev->intx.interrupt, 0);
     if (ret) {
-        error_report("vfio: Error: event_notifier_init failed");
+        error_setg_errno(errp, -ret, "event_notifier_init failed");
         return ret;
     }
 
@@ -299,13 +304,16 @@ static int vfio_intx_enable(VFIOPCIDevice *vdev)
     ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
     g_free(irq_set);
     if (ret) {
-        error_report("vfio: Error: Failed to setup INTx fd: %m");
+        error_setg_errno(errp, -ret, "failed to setup INTx fd");
         qemu_set_fd_handler(*pfd, NULL, NULL, vdev);
         event_notifier_cleanup(&vdev->intx.interrupt);
         return -errno;
     }
 
-    vfio_intx_enable_kvm(vdev);
+    vfio_intx_enable_kvm(vdev, &err);
+    if (err) {
+        error_reportf_err(err, WARN_PREFIX, vdev->vbasedev.name);
+    }
 
     vdev->interrupt = VFIO_INT_INTx;
 
@@ -707,6 +715,7 @@ retry:
 
 static void vfio_msi_disable_common(VFIOPCIDevice *vdev)
 {
+    Error *err = NULL;
     int i;
 
     for (i = 0; i < vdev->nr_vectors; i++) {
@@ -726,7 +735,10 @@ static void vfio_msi_disable_common(VFIOPCIDevice *vdev)
     vdev->nr_vectors = 0;
     vdev->interrupt = VFIO_INT_NONE;
 
-    vfio_intx_enable(vdev);
+    vfio_intx_enable(vdev, &err);
+    if (err) {
+        error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
+    }
 }
 
 static void vfio_msix_disable(VFIOPCIDevice *vdev)
@@ -1908,7 +1920,12 @@ static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
 
 static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
 {
-    vfio_intx_enable(vdev);
+    Error *err = NULL;
+
+    vfio_intx_enable(vdev, &err);
+    if (err) {
+        error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
+    }
 }
 
 static bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
@@ -2724,7 +2741,7 @@ static int vfio_initfn(PCIDevice *pdev)
         vdev->intx.mmap_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
                                                   vfio_intx_mmap_enable, vdev);
         pci_device_set_intx_routing_notifier(&vdev->pdev, vfio_intx_update);
-        ret = vfio_intx_enable(vdev);
+        ret = vfio_intx_enable(vdev, &err);
         if (ret) {
             goto out_teardown;
         }

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 06/19] vfio/pci: Pass an error object to vfio_add_capabilities
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (4 preceding siblings ...)
  2016-10-17 19:52 ` [Qemu-devel] [PULL 05/19] vfio/pci: Pass an error object to vfio_intx_enable Alex Williamson
@ 2016-10-17 19:52 ` Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 07/19] vfio/pci: Pass an error object to vfio_pci_igd_opregion_init Alex Williamson
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for migration to VFIO-PCI realize.
The error is cascaded downto vfio_add_std_cap and then vfio_msi(x)_setup,
vfio_setup_pcie_cap.

vfio_add_ext_cap does not return anything else than 0 so let's transform
it into a void function.

Also use pci_add_capability2 which takes an error object.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   59 ++++++++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 42161c8..e2cf6ac 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1180,7 +1180,7 @@ static void vfio_disable_interrupts(VFIOPCIDevice *vdev)
     }
 }
 
-static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
+static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
 {
     uint16_t ctrl;
     bool msi_64bit, msi_maskbit;
@@ -1189,6 +1189,7 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
 
     if (pread(vdev->vbasedev.fd, &ctrl, sizeof(ctrl),
               vdev->config_offset + pos + PCI_CAP_FLAGS) != sizeof(ctrl)) {
+        error_setg_errno(errp, errno, "failed reading MSI PCI_CAP_FLAGS");
         return -errno;
     }
     ctrl = le16_to_cpu(ctrl);
@@ -1204,8 +1205,8 @@ static int vfio_msi_setup(VFIOPCIDevice *vdev, int pos)
         if (ret == -ENOTSUP) {
             return 0;
         }
-        error_prepend(&err, "vfio: msi_init failed: ");
-        error_report_err(err);
+        error_prepend(&err, "msi_init failed: ");
+        error_propagate(errp, err);
         return ret;
     }
     vdev->msi_cap_size = 0xa + (msi_maskbit ? 0xa : 0) + (msi_64bit ? 0x4 : 0);
@@ -1363,7 +1364,7 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
     return 0;
 }
 
-static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos)
+static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
 {
     int ret;
 
@@ -1378,7 +1379,7 @@ static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos)
         if (ret == -ENOTSUP) {
             return 0;
         }
-        error_report("vfio: msix_init failed");
+        error_setg(errp, "msix_init failed");
         return ret;
     }
 
@@ -1563,7 +1564,8 @@ static void vfio_add_emulated_long(VFIOPCIDevice *vdev, int pos,
     vfio_set_long_bits(vdev->emulated_config_bits + pos, mask, mask);
 }
 
-static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size)
+static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
+                               Error **errp)
 {
     uint16_t flags;
     uint8_t type;
@@ -1575,8 +1577,8 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size)
         type != PCI_EXP_TYPE_LEG_END &&
         type != PCI_EXP_TYPE_RC_END) {
 
-        error_report("vfio: Assignment of PCIe type 0x%x "
-                     "devices is not currently supported", type);
+        error_setg(errp, "assignment of PCIe type 0x%x "
+                   "devices is not currently supported", type);
         return -EINVAL;
     }
 
@@ -1710,7 +1712,7 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, uint8_t pos)
     }
 }
 
-static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
+static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
 {
     PCIDevice *pdev = &vdev->pdev;
     uint8_t cap_id, next, size;
@@ -1735,9 +1737,9 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
      * will be changed as we unwind the stack.
      */
     if (next) {
-        ret = vfio_add_std_cap(vdev, next);
+        ret = vfio_add_std_cap(vdev, next, errp);
         if (ret) {
-            return ret;
+            goto out;
         }
     } else {
         /* Begin the rebuild, use QEMU emulated list bits */
@@ -1751,40 +1753,40 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 
     switch (cap_id) {
     case PCI_CAP_ID_MSI:
-        ret = vfio_msi_setup(vdev, pos);
+        ret = vfio_msi_setup(vdev, pos, errp);
         break;
     case PCI_CAP_ID_EXP:
         vfio_check_pcie_flr(vdev, pos);
-        ret = vfio_setup_pcie_cap(vdev, pos, size);
+        ret = vfio_setup_pcie_cap(vdev, pos, size, errp);
         break;
     case PCI_CAP_ID_MSIX:
-        ret = vfio_msix_setup(vdev, pos);
+        ret = vfio_msix_setup(vdev, pos, errp);
         break;
     case PCI_CAP_ID_PM:
         vfio_check_pm_reset(vdev, pos);
         vdev->pm_cap = pos;
-        ret = pci_add_capability(pdev, cap_id, pos, size);
+        ret = pci_add_capability2(pdev, cap_id, pos, size, errp);
         break;
     case PCI_CAP_ID_AF:
         vfio_check_af_flr(vdev, pos);
-        ret = pci_add_capability(pdev, cap_id, pos, size);
+        ret = pci_add_capability2(pdev, cap_id, pos, size, errp);
         break;
     default:
-        ret = pci_add_capability(pdev, cap_id, pos, size);
+        ret = pci_add_capability2(pdev, cap_id, pos, size, errp);
         break;
     }
-
+out:
     if (ret < 0) {
-        error_report("vfio: %s Error adding PCI capability "
-                     "0x%x[0x%x]@0x%x: %d", vdev->vbasedev.name,
-                     cap_id, size, pos, ret);
+        error_prepend(errp,
+                      "failed to add PCI capability 0x%x[0x%x]@0x%x: ",
+                      cap_id, size, pos);
         return ret;
     }
 
     return 0;
 }
 
-static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
+static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
 {
     PCIDevice *pdev = &vdev->pdev;
     uint32_t header;
@@ -1795,7 +1797,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
     /* Only add extended caps if we have them and the guest can see them */
     if (!pci_is_express(pdev) || !pci_bus_is_express(pdev->bus) ||
         !pci_get_long(pdev->config + PCI_CONFIG_SPACE_SIZE)) {
-        return 0;
+        return;
     }
 
     /*
@@ -1860,10 +1862,10 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
     }
 
     g_free(config);
-    return 0;
+    return;
 }
 
-static int vfio_add_capabilities(VFIOPCIDevice *vdev)
+static int vfio_add_capabilities(VFIOPCIDevice *vdev, Error **errp)
 {
     PCIDevice *pdev = &vdev->pdev;
     int ret;
@@ -1873,12 +1875,13 @@ static int vfio_add_capabilities(VFIOPCIDevice *vdev)
         return 0; /* Nothing to add */
     }
 
-    ret = vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
+    ret = vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST], errp);
     if (ret) {
         return ret;
     }
 
-    return vfio_add_ext_cap(vdev);
+    vfio_add_ext_cap(vdev);
+    return 0;
 }
 
 static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
@@ -2684,7 +2687,7 @@ static int vfio_initfn(PCIDevice *pdev)
 
     vfio_bars_setup(vdev);
 
-    ret = vfio_add_capabilities(vdev);
+    ret = vfio_add_capabilities(vdev, &err);
     if (ret) {
         goto out_teardown;
     }

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 07/19] vfio/pci: Pass an error object to vfio_pci_igd_opregion_init
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (5 preceding siblings ...)
  2016-10-17 19:52 ` [Qemu-devel] [PULL 06/19] vfio/pci: Pass an error object to vfio_add_capabilities Alex Williamson
@ 2016-10-17 19:52 ` Alex Williamson
  2016-10-17 19:52 ` [Qemu-devel] [PULL 08/19] vfio: Pass an Error object to vfio_connect_container Alex Williamson
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for migration to VFIO-PCI realize.

In vfio_probe_igd_bar4_quirk, simply report the error.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci-quirks.c |   10 +++++-----
 hw/vfio/pci.c        |    3 +--
 hw/vfio/pci.h        |    3 ++-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 806ea5d..2cbda08 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1056,7 +1056,7 @@ typedef struct VFIOIGDQuirk {
  * of the IGD device.
  */
 int vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
-                               struct vfio_region_info *info)
+                               struct vfio_region_info *info, Error **errp)
 {
     int ret;
 
@@ -1064,7 +1064,7 @@ int vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
     ret = pread(vdev->vbasedev.fd, vdev->igd_opregion,
                 info->size, info->offset);
     if (ret != info->size) {
-        error_report("vfio: Error reading IGD OpRegion");
+        error_setg(errp, "failed to read IGD OpRegion");
         g_free(vdev->igd_opregion);
         vdev->igd_opregion = NULL;
         return -EINVAL;
@@ -1489,10 +1489,10 @@ static void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
     }
 
     /* Setup OpRegion access */
-    ret = vfio_pci_igd_opregion_init(vdev, opregion);
+    ret = vfio_pci_igd_opregion_init(vdev, opregion, &err);
     if (ret) {
-        error_report("IGD device %s failed to setup OpRegion, "
-                     "legacy mode disabled", vdev->vbasedev.name);
+        error_append_hint(&err, "IGD legacy mode disabled\n");
+        error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
         goto out;
     }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e2cf6ac..3d84126 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2721,10 +2721,9 @@ static int vfio_initfn(PCIDevice *pdev)
             goto out_teardown;
         }
 
-        ret = vfio_pci_igd_opregion_init(vdev, opregion);
+        ret = vfio_pci_igd_opregion_init(vdev, opregion, &err);
         g_free(opregion);
         if (ret) {
-            error_setg_errno(&err, -ret, "IGD OpRegion initialization failed");
             goto out_teardown;
         }
     }
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 87a62f9..a8366bb 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -164,6 +164,7 @@ void vfio_setup_resetfn_quirk(VFIOPCIDevice *vdev);
 int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp);
 
 int vfio_pci_igd_opregion_init(VFIOPCIDevice *vdev,
-                               struct vfio_region_info *info);
+                               struct vfio_region_info *info,
+                               Error **errp);
 
 #endif /* HW_VFIO_VFIO_PCI_H */

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 08/19] vfio: Pass an Error object to vfio_connect_container
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (6 preceding siblings ...)
  2016-10-17 19:52 ` [Qemu-devel] [PULL 07/19] vfio/pci: Pass an error object to vfio_pci_igd_opregion_init Alex Williamson
@ 2016-10-17 19:52 ` Alex Williamson
  2016-10-17 19:53 ` [Qemu-devel] [PULL 09/19] vfio: Pass an error object to vfio_get_group Alex Williamson
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

The error is currently simply reported in vfio_get_group. Don't
bother too much with the prefix which will be handled at upper level,
later on.

Also return an error value in case container->error is not 0 and
the container is teared down.

On vfio_spapr_remove_window failure, we also report an error whereas
it was silent before.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/common.c |   40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 29188a1..85a7759 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -34,6 +34,7 @@
 #include "qemu/range.h"
 #include "sysemu/kvm.h"
 #include "trace.h"
+#include "qapi/error.h"
 
 struct vfio_group_head vfio_group_list =
     QLIST_HEAD_INITIALIZER(vfio_group_list);
@@ -900,7 +901,8 @@ static void vfio_put_address_space(VFIOAddressSpace *space)
     }
 }
 
-static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
+static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
+                                  Error **errp)
 {
     VFIOContainer *container;
     int ret, fd;
@@ -918,15 +920,15 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
 
     fd = qemu_open("/dev/vfio/vfio", O_RDWR);
     if (fd < 0) {
-        error_report("vfio: failed to open /dev/vfio/vfio: %m");
+        error_setg_errno(errp, errno, "failed to open /dev/vfio/vfio");
         ret = -errno;
         goto put_space_exit;
     }
 
     ret = ioctl(fd, VFIO_GET_API_VERSION);
     if (ret != VFIO_API_VERSION) {
-        error_report("vfio: supported vfio version: %d, "
-                     "reported version: %d", VFIO_API_VERSION, ret);
+        error_setg(errp, "supported vfio version: %d, "
+                   "reported version: %d", VFIO_API_VERSION, ret);
         ret = -EINVAL;
         goto close_fd_exit;
     }
@@ -941,7 +943,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
 
         ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
         if (ret) {
-            error_report("vfio: failed to set group container: %m");
+            error_setg_errno(errp, errno, "failed to set group container");
             ret = -errno;
             goto free_container_exit;
         }
@@ -949,7 +951,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
         container->iommu_type = v2 ? VFIO_TYPE1v2_IOMMU : VFIO_TYPE1_IOMMU;
         ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
         if (ret) {
-            error_report("vfio: failed to set iommu for container: %m");
+            error_setg_errno(errp, errno, "failed to set iommu for container");
             ret = -errno;
             goto free_container_exit;
         }
@@ -976,7 +978,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
 
         ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
         if (ret) {
-            error_report("vfio: failed to set group container: %m");
+            error_setg_errno(errp, errno, "failed to set group container");
             ret = -errno;
             goto free_container_exit;
         }
@@ -984,7 +986,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
             v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
         ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
         if (ret) {
-            error_report("vfio: failed to set iommu for container: %m");
+            error_setg_errno(errp, errno, "failed to set iommu for container");
             ret = -errno;
             goto free_container_exit;
         }
@@ -997,7 +999,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
         if (!v2) {
             ret = ioctl(fd, VFIO_IOMMU_ENABLE);
             if (ret) {
-                error_report("vfio: failed to enable container: %m");
+                error_setg_errno(errp, errno, "failed to enable container");
                 ret = -errno;
                 goto free_container_exit;
             }
@@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
                                      &address_space_memory);
             if (container->error) {
                 memory_listener_unregister(&container->prereg_listener);
-                error_report("vfio: RAM memory listener initialization failed for container");
+                ret = container->error;
+                error_setg(errp,
+                    "RAM memory listener initialization failed for container");
                 goto free_container_exit;
             }
         }
@@ -1016,7 +1020,8 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
         info.argsz = sizeof(info);
         ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
         if (ret) {
-            error_report("vfio: VFIO_IOMMU_SPAPR_TCE_GET_INFO failed: %m");
+            error_setg_errno(errp, errno,
+                             "VFIO_IOMMU_SPAPR_TCE_GET_INFO failed");
             ret = -errno;
             if (v2) {
                 memory_listener_unregister(&container->prereg_listener);
@@ -1033,6 +1038,8 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
              */
             ret = vfio_spapr_remove_window(container, info.dma32_window_start);
             if (ret) {
+                error_setg_errno(errp, -ret,
+                                 "failed to remove existing window");
                 goto free_container_exit;
             }
         } else {
@@ -1043,7 +1050,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
                               0x1000);
         }
     } else {
-        error_report("vfio: No available IOMMU models");
+        error_setg(errp, "No available IOMMU models");
         ret = -EINVAL;
         goto free_container_exit;
     }
@@ -1054,7 +1061,8 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as)
 
     if (container->error) {
         ret = container->error;
-        error_report("vfio: memory listener initialization failed for container");
+        error_setg_errno(errp, -ret,
+                         "memory listener initialization failed for container");
         goto listener_release_exit;
     }
 
@@ -1120,6 +1128,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as)
     VFIOGroup *group;
     char path[32];
     struct vfio_group_status status = { .argsz = sizeof(status) };
+    Error *err = NULL;
 
     QLIST_FOREACH(group, &vfio_group_list, next) {
         if (group->groupid == groupid) {
@@ -1158,8 +1167,9 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as)
     group->groupid = groupid;
     QLIST_INIT(&group->device_list);
 
-    if (vfio_connect_container(group, as)) {
-        error_report("vfio: failed to setup container for group %d", groupid);
+    if (vfio_connect_container(group, as, &err)) {
+        error_reportf_err(err, "vfio: failed to setup container for group %d",
+                          groupid);
         goto close_fd_exit;
     }
 

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 09/19] vfio: Pass an error object to vfio_get_group
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (7 preceding siblings ...)
  2016-10-17 19:52 ` [Qemu-devel] [PULL 08/19] vfio: Pass an Error object to vfio_connect_container Alex Williamson
@ 2016-10-17 19:53 ` Alex Williamson
  2016-10-17 19:53 ` [Qemu-devel] [PULL 10/19] vfio: Pass an error object to vfio_get_device Alex Williamson
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for migration to VFIO-PCI realize.

For the time being let's just simply report the error in
vfio platform's vfio_base_device_init(). A subsequent patch will
duly propagate the error up to vfio_platform_realize.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/common.c              |   24 ++++++++++++------------
 hw/vfio/pci.c                 |    3 +--
 hw/vfio/platform.c            |   11 ++++++++---
 include/hw/vfio/vfio-common.h |    2 +-
 4 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 85a7759..90b1ebb 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1123,12 +1123,11 @@ static void vfio_disconnect_container(VFIOGroup *group)
     }
 }
 
-VFIOGroup *vfio_get_group(int groupid, AddressSpace *as)
+VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
 {
     VFIOGroup *group;
     char path[32];
     struct vfio_group_status status = { .argsz = sizeof(status) };
-    Error *err = NULL;
 
     QLIST_FOREACH(group, &vfio_group_list, next) {
         if (group->groupid == groupid) {
@@ -1136,8 +1135,8 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as)
             if (group->container->space->as == as) {
                 return group;
             } else {
-                error_report("vfio: group %d used in multiple address spaces",
-                             group->groupid);
+                error_setg(errp, "group %d used in multiple address spaces",
+                           group->groupid);
                 return NULL;
             }
         }
@@ -1148,28 +1147,29 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as)
     snprintf(path, sizeof(path), "/dev/vfio/%d", groupid);
     group->fd = qemu_open(path, O_RDWR);
     if (group->fd < 0) {
-        error_report("vfio: error opening %s: %m", path);
+        error_setg_errno(errp, errno, "failed to open %s", path);
         goto free_group_exit;
     }
 
     if (ioctl(group->fd, VFIO_GROUP_GET_STATUS, &status)) {
-        error_report("vfio: error getting group status: %m");
+        error_setg_errno(errp, errno, "failed to get group %d status", groupid);
         goto close_fd_exit;
     }
 
     if (!(status.flags & VFIO_GROUP_FLAGS_VIABLE)) {
-        error_report("vfio: error, group %d is not viable, please ensure "
-                     "all devices within the iommu_group are bound to their "
-                     "vfio bus driver.", groupid);
+        error_setg(errp, "group %d is not viable", groupid);
+        error_append_hint(errp,
+                          "Please ensure all devices within the iommu_group "
+                          "are bound to their vfio bus driver.\n");
         goto close_fd_exit;
     }
 
     group->groupid = groupid;
     QLIST_INIT(&group->device_list);
 
-    if (vfio_connect_container(group, as, &err)) {
-        error_reportf_err(err, "vfio: failed to setup container for group %d",
-                          groupid);
+    if (vfio_connect_container(group, as, errp)) {
+        error_prepend(errp, "failed to setup container for group %d: ",
+                      groupid);
         goto close_fd_exit;
     }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 3d84126..fdb0616 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2563,9 +2563,8 @@ static int vfio_initfn(PCIDevice *pdev)
 
     trace_vfio_initfn(vdev->vbasedev.name, groupid);
 
-    group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev));
+    group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), &err);
     if (!group) {
-        error_setg(&err, "failed to get group %d", groupid);
         ret = -ENOENT;
         goto error;
     }
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index a559e7b..7bf525b 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -552,6 +552,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
     ssize_t len;
     struct stat st;
     int groupid;
+    Error *err = NULL;
     int ret;
 
     /* @sysfsdev takes precedence over @host */
@@ -592,10 +593,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
 
     trace_vfio_platform_base_device_init(vbasedev->name, groupid);
 
-    group = vfio_get_group(groupid, &address_space_memory);
+    group = vfio_get_group(groupid, &address_space_memory, &err);
     if (!group) {
-        error_report("vfio: failed to get group %d", groupid);
-        return -ENOENT;
+        ret = -ENOENT;
+        goto error;
     }
 
     QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
@@ -619,6 +620,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
         vfio_put_group(group);
     }
 
+error:
+    if (err) {
+        error_reportf_err(err, ERR_PREFIX, vbasedev->name);
+    }
     return ret;
 }
 
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index b26b6cf..286fa31 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -155,7 +155,7 @@ void vfio_region_mmaps_set_enabled(VFIORegion *region, bool enabled);
 void vfio_region_exit(VFIORegion *region);
 void vfio_region_finalize(VFIORegion *region);
 void vfio_reset_handler(void *opaque);
-VFIOGroup *vfio_get_group(int groupid, AddressSpace *as);
+VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp);
 void vfio_put_group(VFIOGroup *group);
 int vfio_get_device(VFIOGroup *group, const char *name,
                     VFIODevice *vbasedev);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 10/19] vfio: Pass an error object to vfio_get_device
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (8 preceding siblings ...)
  2016-10-17 19:53 ` [Qemu-devel] [PULL 09/19] vfio: Pass an error object to vfio_get_group Alex Williamson
@ 2016-10-17 19:53 ` Alex Williamson
  2016-10-17 19:53 ` [Qemu-devel] [PULL 11/19] vfio/platform: Pass an error object to vfio_populate_device Alex Williamson
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Pass an error object to prepare for migration to VFIO-PCI realize.

In vfio platform vfio_base_device_init we currently just report the
error. Subsequent patches will propagate the error up to the realize
function.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/common.c              |   13 +++++++------
 hw/vfio/pci.c                 |    3 +--
 hw/vfio/platform.c            |    5 ++---
 include/hw/vfio/vfio-common.h |    2 +-
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 90b1ebb..9505fb3 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1211,23 +1211,24 @@ void vfio_put_group(VFIOGroup *group)
 }
 
 int vfio_get_device(VFIOGroup *group, const char *name,
-                       VFIODevice *vbasedev)
+                    VFIODevice *vbasedev, Error **errp)
 {
     struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) };
     int ret, fd;
 
     fd = ioctl(group->fd, VFIO_GROUP_GET_DEVICE_FD, name);
     if (fd < 0) {
-        error_report("vfio: error getting device %s from group %d: %m",
-                     name, group->groupid);
-        error_printf("Verify all devices in group %d are bound to vfio-<bus> "
-                     "or pci-stub and not already in use\n", group->groupid);
+        error_setg_errno(errp, errno, "error getting device from group %d",
+                         group->groupid);
+        error_append_hint(errp,
+                      "Verify all devices in group %d are bound to vfio-<bus> "
+                      "or pci-stub and not already in use\n", group->groupid);
         return fd;
     }
 
     ret = ioctl(fd, VFIO_DEVICE_GET_INFO, &dev_info);
     if (ret) {
-        error_report("vfio: error getting device info: %m");
+        error_setg_errno(errp, errno, "error getting device info");
         close(fd);
         return ret;
     }
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index fdb0616..0ba0711 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2578,9 +2578,8 @@ static int vfio_initfn(PCIDevice *pdev)
         }
     }
 
-    ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev);
+    ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev, &err);
     if (ret) {
-        error_setg_errno(&err, -ret, "failed to get device");
         vfio_put_group(group);
         goto error;
     }
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 7bf525b..9014ea7 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -607,11 +607,10 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
             return -EBUSY;
         }
     }
-    ret = vfio_get_device(group, vbasedev->name, vbasedev);
+    ret = vfio_get_device(group, vbasedev->name, vbasedev, &err);
     if (ret) {
-        error_report("vfio: failed to get device %s", vbasedev->name);
         vfio_put_group(group);
-        return ret;
+        goto error;
     }
 
     ret = vfio_populate_device(vbasedev);
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 286fa31..c582de1 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -158,7 +158,7 @@ void vfio_reset_handler(void *opaque);
 VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp);
 void vfio_put_group(VFIOGroup *group);
 int vfio_get_device(VFIOGroup *group, const char *name,
-                    VFIODevice *vbasedev);
+                    VFIODevice *vbasedev, Error **errp);
 
 extern const MemoryRegionOps vfio_region_ops;
 extern QLIST_HEAD(vfio_group_head, VFIOGroup) vfio_group_list;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 11/19] vfio/platform: Pass an error object to vfio_populate_device
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (9 preceding siblings ...)
  2016-10-17 19:53 ` [Qemu-devel] [PULL 10/19] vfio: Pass an error object to vfio_get_device Alex Williamson
@ 2016-10-17 19:53 ` Alex Williamson
  2016-10-17 19:53 ` [Qemu-devel] [PULL 12/19] vfio/platform: fix a wrong returned value in vfio_populate_device Alex Williamson
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

Propagate the vfio_populate_device errors up to vfio_base_device_init.
The error object also is passed to vfio_init_intp. At the moment we
only report the error. Subsequent patches will propagate the error
up to the realize function.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/platform.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 9014ea7..1a35da0 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -44,9 +44,10 @@ static inline bool vfio_irq_is_automasked(VFIOINTp *intp)
  * and add it into the list of IRQs
  * @vbasedev: the VFIO device handle
  * @info: irq info struct retrieved from VFIO driver
+ * @errp: error object
  */
 static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
-                                struct vfio_irq_info info)
+                                struct vfio_irq_info info, Error **errp)
 {
     int ret;
     VFIOPlatformDevice *vdev =
@@ -69,7 +70,8 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
     if (ret) {
         g_free(intp->interrupt);
         g_free(intp);
-        error_report("vfio: Error: trigger event_notifier_init failed ");
+        error_setg_errno(errp, -ret,
+                         "failed to initialize trigger eventd notifier");
         return NULL;
     }
     if (vfio_irq_is_automasked(intp)) {
@@ -80,7 +82,8 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev,
             g_free(intp->interrupt);
             g_free(intp->unmask);
             g_free(intp);
-            error_report("vfio: Error: resamplefd event_notifier_init failed");
+            error_setg_errno(errp, -ret,
+                             "failed to initialize resample eventd notifier");
             return NULL;
         }
     }
@@ -456,9 +459,10 @@ static int vfio_platform_hot_reset_multi(VFIODevice *vbasedev)
  * vfio_populate_device - Allocate and populate MMIO region
  * and IRQ structs according to driver returned information
  * @vbasedev: the VFIO device handle
+ * @errp: error object
  *
  */
-static int vfio_populate_device(VFIODevice *vbasedev)
+static int vfio_populate_device(VFIODevice *vbasedev, Error **errp)
 {
     VFIOINTp *intp, *tmp;
     int i, ret = -1;
@@ -466,7 +470,7 @@ static int vfio_populate_device(VFIODevice *vbasedev)
         container_of(vbasedev, VFIOPlatformDevice, vbasedev);
 
     if (!(vbasedev->flags & VFIO_DEVICE_FLAGS_PLATFORM)) {
-        error_report("vfio: Um, this isn't a platform device");
+        error_setg(errp, "this isn't a platform device");
         return ret;
     }
 
@@ -480,7 +484,7 @@ static int vfio_populate_device(VFIODevice *vbasedev)
                                 vdev->regions[i], i, name);
         g_free(name);
         if (ret) {
-            error_report("vfio: Error getting region %d info: %m", i);
+            error_setg_errno(errp, -ret, "failed to get region %d info", i);
             goto reg_error;
         }
     }
@@ -496,16 +500,14 @@ static int vfio_populate_device(VFIODevice *vbasedev)
         irq.index = i;
         ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
         if (ret) {
-            error_report("vfio: error getting device %s irq info",
-                         vbasedev->name);
+            error_setg_errno(errp, -ret, "failed to get device irq info");
             goto irq_err;
         } else {
             trace_vfio_platform_populate_interrupts(irq.index,
                                                     irq.count,
                                                     irq.flags);
-            intp = vfio_init_intp(vbasedev, irq);
+            intp = vfio_init_intp(vbasedev, irq, errp);
             if (!intp) {
-                error_report("vfio: Error installing IRQ %d up", i);
                 goto irq_err;
             }
         }
@@ -613,9 +615,8 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
         goto error;
     }
 
-    ret = vfio_populate_device(vbasedev);
+    ret = vfio_populate_device(vbasedev, &err);
     if (ret) {
-        error_report("vfio: failed to populate device %s", vbasedev->name);
         vfio_put_group(group);
     }
 

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 12/19] vfio/platform: fix a wrong returned value in vfio_populate_device
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (10 preceding siblings ...)
  2016-10-17 19:53 ` [Qemu-devel] [PULL 11/19] vfio/platform: Pass an error object to vfio_populate_device Alex Williamson
@ 2016-10-17 19:53 ` Alex Williamson
  2016-10-17 19:53 ` [Qemu-devel] [PULL 13/19] vfio/platform: Pass an error object to vfio_base_device_init Alex Williamson
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

In case the vfio_init_intp fails we currently do not return an
error value. This patch fixes the bug. The returned value is not
explicit but in practice the error object is the one used to
report the error to the end-user and the actual returned error
value is not used.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/platform.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 1a35da0..484e31f 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -508,6 +508,7 @@ static int vfio_populate_device(VFIODevice *vbasedev, Error **errp)
                                                     irq.flags);
             intp = vfio_init_intp(vbasedev, irq, errp);
             if (!intp) {
+                ret = -1;
                 goto irq_err;
             }
         }

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 13/19] vfio/platform: Pass an error object to vfio_base_device_init
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (11 preceding siblings ...)
  2016-10-17 19:53 ` [Qemu-devel] [PULL 12/19] vfio/platform: fix a wrong returned value in vfio_populate_device Alex Williamson
@ 2016-10-17 19:53 ` Alex Williamson
  2016-10-17 19:53 ` [Qemu-devel] [PULL 14/19] vfio/pci: Conversion to realize Alex Williamson
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

This patch propagates errors encountered during vfio_base_device_init
up to the realize function.

In case the host value is not set or badly formed we now report an
error.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/platform.c |   50 +++++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 484e31f..a4663c9 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -541,13 +541,14 @@ static VFIODeviceOps vfio_platform_ops = {
 /**
  * vfio_base_device_init - perform preliminary VFIO setup
  * @vbasedev: the VFIO device handle
+ * @errp: error object
  *
  * Implement the VFIO command sequence that allows to discover
  * assigned device resources: group extraction, device
  * fd retrieval, resource query.
  * Precondition: the device name must be initialized
  */
-static int vfio_base_device_init(VFIODevice *vbasedev)
+static int vfio_base_device_init(VFIODevice *vbasedev, Error **errp)
 {
     VFIOGroup *group;
     VFIODevice *vbasedev_iter;
@@ -555,7 +556,6 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
     ssize_t len;
     struct stat st;
     int groupid;
-    Error *err = NULL;
     int ret;
 
     /* @sysfsdev takes precedence over @host */
@@ -564,6 +564,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
         vbasedev->name = g_strdup(basename(vbasedev->sysfsdev));
     } else {
         if (!vbasedev->name || strchr(vbasedev->name, '/')) {
+            error_setg(errp, "wrong host device name");
             return -EINVAL;
         }
 
@@ -572,8 +573,8 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
     }
 
     if (stat(vbasedev->sysfsdev, &st) < 0) {
-        error_report("vfio: error: no such host device: %s",
-                     vbasedev->sysfsdev);
+        error_setg_errno(errp, errno,
+                         "failed to get the sysfs host device file status");
         return -errno;
     }
 
@@ -582,49 +583,44 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
     g_free(tmp);
 
     if (len < 0 || len >= sizeof(group_path)) {
-        error_report("vfio: error no iommu_group for device");
-        return len < 0 ? -errno : -ENAMETOOLONG;
+        ret = len < 0 ? -errno : -ENAMETOOLONG;
+        error_setg_errno(errp, -ret, "no iommu_group found");
+        return ret;
     }
 
     group_path[len] = 0;
 
     group_name = basename(group_path);
     if (sscanf(group_name, "%d", &groupid) != 1) {
-        error_report("vfio: error reading %s: %m", group_path);
+        error_setg_errno(errp, errno, "failed to read %s", group_path);
         return -errno;
     }
 
     trace_vfio_platform_base_device_init(vbasedev->name, groupid);
 
-    group = vfio_get_group(groupid, &address_space_memory, &err);
+    group = vfio_get_group(groupid, &address_space_memory, errp);
     if (!group) {
-        ret = -ENOENT;
-        goto error;
+        return -ENOENT;
     }
 
     QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
         if (strcmp(vbasedev_iter->name, vbasedev->name) == 0) {
-            error_report("vfio: error: device %s is already attached",
-                         vbasedev->name);
+            error_setg(errp, "device is already attached");
             vfio_put_group(group);
             return -EBUSY;
         }
     }
-    ret = vfio_get_device(group, vbasedev->name, vbasedev, &err);
+    ret = vfio_get_device(group, vbasedev->name, vbasedev, errp);
     if (ret) {
         vfio_put_group(group);
-        goto error;
+        return ret;
     }
 
-    ret = vfio_populate_device(vbasedev, &err);
+    ret = vfio_populate_device(vbasedev, errp);
     if (ret) {
         vfio_put_group(group);
     }
 
-error:
-    if (err) {
-        error_reportf_err(err, ERR_PREFIX, vbasedev->name);
-    }
     return ret;
 }
 
@@ -650,11 +646,9 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
                                 vbasedev->sysfsdev : vbasedev->name,
                                 vdev->compat);
 
-    ret = vfio_base_device_init(vbasedev);
+    ret = vfio_base_device_init(vbasedev, errp);
     if (ret) {
-        error_setg(errp, "vfio: vfio_base_device_init failed for %s",
-                   vbasedev->name);
-        return;
+        goto out;
     }
 
     for (i = 0; i < vbasedev->num_regions; i++) {
@@ -664,6 +658,16 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
         }
         sysbus_init_mmio(sbdev, vdev->regions[i]->mem);
     }
+out:
+    if (!ret) {
+        return;
+    }
+
+    if (vdev->vbasedev.name) {
+        error_prepend(errp, ERR_PREFIX, vdev->vbasedev.name);
+    } else {
+        error_prepend(errp, "vfio error: ");
+    }
 }
 
 static const VMStateDescription vfio_platform_vmstate = {

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 14/19] vfio/pci: Conversion to realize
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (12 preceding siblings ...)
  2016-10-17 19:53 ` [Qemu-devel] [PULL 13/19] vfio/platform: Pass an error object to vfio_base_device_init Alex Williamson
@ 2016-10-17 19:53 ` Alex Williamson
  2016-10-17 19:53 ` [Qemu-devel] [PULL 15/19] vfio/pci: Remove vfio_msix_early_setup returned value Alex Williamson
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

This patch converts VFIO PCI to realize function.

Also original initfn errors now are propagated using QEMU
error objects. All errors are formatted with the same pattern:
"vfio: %s: the error description"

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c        |   64 ++++++++++++++++++++------------------------------
 hw/vfio/trace-events |    2 +-
 2 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 0ba0711..d9652c2 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2513,13 +2513,12 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice *vdev)
     vdev->req_enabled = false;
 }
 
-static int vfio_initfn(PCIDevice *pdev)
+static void vfio_realize(PCIDevice *pdev, Error **errp)
 {
     VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
     VFIODevice *vbasedev_iter;
     VFIOGroup *group;
     char *tmp, group_path[PATH_MAX], *group_name;
-    Error *err = NULL;
     ssize_t len;
     struct stat st;
     int groupid;
@@ -2533,9 +2532,9 @@ static int vfio_initfn(PCIDevice *pdev)
     }
 
     if (stat(vdev->vbasedev.sysfsdev, &st) < 0) {
-        error_setg_errno(&err, errno, "no such host device");
-        ret = -errno;
-        goto error;
+        error_setg_errno(errp, errno, "no such host device");
+        error_prepend(errp, ERR_PREFIX, vdev->vbasedev.sysfsdev);
+        return;
     }
 
     vdev->vbasedev.name = g_strdup(basename(vdev->vbasedev.sysfsdev));
@@ -2547,8 +2546,8 @@ static int vfio_initfn(PCIDevice *pdev)
     g_free(tmp);
 
     if (len <= 0 || len >= sizeof(group_path)) {
-        ret = len < 0 ? -errno : -ENAMETOOLONG;
-        error_setg_errno(&err, -ret, "no iommu_group found");
+        error_setg_errno(errp, len < 0 ? errno : ENAMETOOLONG,
+                         "no iommu_group found");
         goto error;
     }
 
@@ -2556,35 +2555,32 @@ static int vfio_initfn(PCIDevice *pdev)
 
     group_name = basename(group_path);
     if (sscanf(group_name, "%d", &groupid) != 1) {
-        error_setg_errno(&err, errno, "failed to read %s", group_path);
-        ret = -errno;
+        error_setg_errno(errp, errno, "failed to read %s", group_path);
         goto error;
     }
 
-    trace_vfio_initfn(vdev->vbasedev.name, groupid);
+    trace_vfio_realize(vdev->vbasedev.name, groupid);
 
-    group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), &err);
+    group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), errp);
     if (!group) {
-        ret = -ENOENT;
         goto error;
     }
 
     QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
         if (strcmp(vbasedev_iter->name, vdev->vbasedev.name) == 0) {
-            error_setg(&err, "device is already attached");
+            error_setg(errp, "device is already attached");
             vfio_put_group(group);
-            ret = -EBUSY;
             goto error;
         }
     }
 
-    ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev, &err);
+    ret = vfio_get_device(group, vdev->vbasedev.name, &vdev->vbasedev, errp);
     if (ret) {
         vfio_put_group(group);
         goto error;
     }
 
-    ret = vfio_populate_device(vdev, &err);
+    ret = vfio_populate_device(vdev, errp);
     if (ret) {
         goto error;
     }
@@ -2595,7 +2591,7 @@ static int vfio_initfn(PCIDevice *pdev)
                 vdev->config_offset);
     if (ret < (int)MIN(pci_config_size(&vdev->pdev), vdev->config_size)) {
         ret = ret < 0 ? -errno : -EFAULT;
-        error_setg_errno(&err, -ret, "failed to read device config space");
+        error_setg_errno(errp, -ret, "failed to read device config space");
         goto error;
     }
 
@@ -2612,8 +2608,7 @@ static int vfio_initfn(PCIDevice *pdev)
      */
     if (vdev->vendor_id != PCI_ANY_ID) {
         if (vdev->vendor_id >= 0xffff) {
-            error_setg(&err, "invalid PCI vendor ID provided");
-            ret = -EINVAL;
+            error_setg(errp, "invalid PCI vendor ID provided");
             goto error;
         }
         vfio_add_emulated_word(vdev, PCI_VENDOR_ID, vdev->vendor_id, ~0);
@@ -2624,8 +2619,7 @@ static int vfio_initfn(PCIDevice *pdev)
 
     if (vdev->device_id != PCI_ANY_ID) {
         if (vdev->device_id > 0xffff) {
-            error_setg(&err, "invalid PCI device ID provided");
-            ret = -EINVAL;
+            error_setg(errp, "invalid PCI device ID provided");
             goto error;
         }
         vfio_add_emulated_word(vdev, PCI_DEVICE_ID, vdev->device_id, ~0);
@@ -2636,8 +2630,7 @@ static int vfio_initfn(PCIDevice *pdev)
 
     if (vdev->sub_vendor_id != PCI_ANY_ID) {
         if (vdev->sub_vendor_id > 0xffff) {
-            error_setg(&err, "invalid PCI subsystem vendor ID provided");
-            ret = -EINVAL;
+            error_setg(errp, "invalid PCI subsystem vendor ID provided");
             goto error;
         }
         vfio_add_emulated_word(vdev, PCI_SUBSYSTEM_VENDOR_ID,
@@ -2648,8 +2641,7 @@ static int vfio_initfn(PCIDevice *pdev)
 
     if (vdev->sub_device_id != PCI_ANY_ID) {
         if (vdev->sub_device_id > 0xffff) {
-            error_setg(&err, "invalid PCI subsystem device ID provided");
-            ret = -EINVAL;
+            error_setg(errp, "invalid PCI subsystem device ID provided");
             goto error;
         }
         vfio_add_emulated_word(vdev, PCI_SUBSYSTEM_ID, vdev->sub_device_id, ~0);
@@ -2678,14 +2670,14 @@ static int vfio_initfn(PCIDevice *pdev)
 
     vfio_pci_size_rom(vdev);
 
-    ret = vfio_msix_early_setup(vdev, &err);
+    ret = vfio_msix_early_setup(vdev, errp);
     if (ret) {
         goto error;
     }
 
     vfio_bars_setup(vdev);
 
-    ret = vfio_add_capabilities(vdev, &err);
+    ret = vfio_add_capabilities(vdev, errp);
     if (ret) {
         goto out_teardown;
     }
@@ -2703,10 +2695,9 @@ static int vfio_initfn(PCIDevice *pdev)
         struct vfio_region_info *opregion;
 
         if (vdev->pdev.qdev.hotplugged) {
-            error_setg(&err,
+            error_setg(errp,
                        "cannot support IGD OpRegion feature on hotplugged "
                        "device");
-            ret = -EINVAL;
             goto out_teardown;
         }
 
@@ -2714,12 +2705,12 @@ static int vfio_initfn(PCIDevice *pdev)
                         VFIO_REGION_TYPE_PCI_VENDOR_TYPE | PCI_VENDOR_ID_INTEL,
                         VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, &opregion);
         if (ret) {
-            error_setg_errno(&err, -ret,
+            error_setg_errno(errp, -ret,
                              "does not support requested IGD OpRegion feature");
             goto out_teardown;
         }
 
-        ret = vfio_pci_igd_opregion_init(vdev, opregion, &err);
+        ret = vfio_pci_igd_opregion_init(vdev, opregion, errp);
         g_free(opregion);
         if (ret) {
             goto out_teardown;
@@ -2741,7 +2732,7 @@ static int vfio_initfn(PCIDevice *pdev)
         vdev->intx.mmap_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
                                                   vfio_intx_mmap_enable, vdev);
         pci_device_set_intx_routing_notifier(&vdev->pdev, vfio_intx_update);
-        ret = vfio_intx_enable(vdev, &err);
+        ret = vfio_intx_enable(vdev, errp);
         if (ret) {
             goto out_teardown;
         }
@@ -2751,17 +2742,14 @@ static int vfio_initfn(PCIDevice *pdev)
     vfio_register_req_notifier(vdev);
     vfio_setup_resetfn_quirk(vdev);
 
-    return 0;
+    return;
 
 out_teardown:
     pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
     vfio_teardown_msi(vdev);
     vfio_bars_exit(vdev);
 error:
-    if (err) {
-        error_reportf_err(err, ERR_PREFIX, vdev->vbasedev.name);
-    }
-    return ret;
+    error_prepend(errp, ERR_PREFIX, vdev->vbasedev.name);
 }
 
 static void vfio_instance_finalize(Object *obj)
@@ -2890,7 +2878,7 @@ static void vfio_pci_dev_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vfio_pci_vmstate;
     dc->desc = "VFIO-based PCI device assignment";
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-    pdc->init = vfio_initfn;
+    pdc->realize = vfio_realize;
     pdc->exit = vfio_exitfn;
     pdc->config_read = vfio_pci_read_config;
     pdc->config_write = vfio_pci_write_config;
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index da13322..ef81609 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -36,7 +36,7 @@ vfio_pci_hot_reset_dep_devices(int domain, int bus, int slot, int function, int
 vfio_pci_hot_reset_result(const char *name, const char *result) "%s hot reset: %s"
 vfio_populate_device_config(const char *name, unsigned long size, unsigned long offset, unsigned long flags) "Device %s config:\n  size: 0x%lx, offset: 0x%lx, flags: 0x%lx"
 vfio_populate_device_get_irq_info_failure(void) "VFIO_DEVICE_GET_IRQ_INFO failure: %m"
-vfio_initfn(const char *name, int group_id) " (%s) group %d"
+vfio_realize(const char *name, int group_id) " (%s) group %d"
 vfio_add_ext_cap_dropped(const char *name, uint16_t cap, uint16_t offset) "%s %x@%x"
 vfio_pci_reset(const char *name) " (%s)"
 vfio_pci_reset_flr(const char *name) "%s FLR/VFIO_DEVICE_RESET"

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 15/19] vfio/pci: Remove vfio_msix_early_setup returned value
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (13 preceding siblings ...)
  2016-10-17 19:53 ` [Qemu-devel] [PULL 14/19] vfio/pci: Conversion to realize Alex Williamson
@ 2016-10-17 19:53 ` Alex Williamson
  2016-10-17 19:54 ` [Qemu-devel] [PULL 16/19] vfio/pci: Remove vfio_populate_device " Alex Williamson
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

The returned value is not used anymore by the caller, vfio_realize,
since the error now is stored in the error object. So let's remove it.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d9652c2..f063c65 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1290,7 +1290,7 @@ static void vfio_pci_fixup_msix_region(VFIOPCIDevice *vdev)
  * need to first look for where the MSI-X table lives.  So we
  * unfortunately split MSI-X setup across two functions.
  */
-static int vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
+static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
 {
     uint8_t pos;
     uint16_t ctrl;
@@ -1300,25 +1300,25 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
 
     pos = pci_find_capability(&vdev->pdev, PCI_CAP_ID_MSIX);
     if (!pos) {
-        return 0;
+        return;
     }
 
     if (pread(fd, &ctrl, sizeof(ctrl),
               vdev->config_offset + pos + PCI_MSIX_FLAGS) != sizeof(ctrl)) {
         error_setg_errno(errp, errno, "failed to read PCI MSIX FLAGS");
-        return -errno;
+        return;
     }
 
     if (pread(fd, &table, sizeof(table),
               vdev->config_offset + pos + PCI_MSIX_TABLE) != sizeof(table)) {
         error_setg_errno(errp, errno, "failed to read PCI MSIX TABLE");
-        return -errno;
+        return;
     }
 
     if (pread(fd, &pba, sizeof(pba),
               vdev->config_offset + pos + PCI_MSIX_PBA) != sizeof(pba)) {
         error_setg_errno(errp, errno, "failed to read PCI MSIX PBA");
-        return -errno;
+        return;
     }
 
     ctrl = le16_to_cpu(ctrl);
@@ -1351,7 +1351,7 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
             error_setg(errp, "hardware reports invalid configuration, "
                        "MSIX PBA outside of specified BAR");
             g_free(msix);
-            return -EINVAL;
+            return;
         }
     }
 
@@ -1360,8 +1360,6 @@ static int vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
     vdev->msix = msix;
 
     vfio_pci_fixup_msix_region(vdev);
-
-    return 0;
 }
 
 static int vfio_msix_setup(VFIOPCIDevice *vdev, int pos, Error **errp)
@@ -2519,6 +2517,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     VFIODevice *vbasedev_iter;
     VFIOGroup *group;
     char *tmp, group_path[PATH_MAX], *group_name;
+    Error *err = NULL;
     ssize_t len;
     struct stat st;
     int groupid;
@@ -2670,8 +2669,9 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 
     vfio_pci_size_rom(vdev);
 
-    ret = vfio_msix_early_setup(vdev, errp);
-    if (ret) {
+    vfio_msix_early_setup(vdev, &err);
+    if (err) {
+        error_propagate(errp, err);
         goto error;
     }
 

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 16/19] vfio/pci: Remove vfio_populate_device returned value
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (14 preceding siblings ...)
  2016-10-17 19:53 ` [Qemu-devel] [PULL 15/19] vfio/pci: Remove vfio_msix_early_setup returned value Alex Williamson
@ 2016-10-17 19:54 ` Alex Williamson
  2016-10-17 19:54 ` [Qemu-devel] [PULL 17/19] vfio/pci: Handle host oversight Alex Williamson
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

The returned value (either -errno or -1) is not used anymore by the caller,
vfio_realize, since the error now is stored in the error object. So let's
remove it.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index f063c65..6d01324 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2223,7 +2223,7 @@ int vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
     return 0;
 }
 
-static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
+static void vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
 {
     VFIODevice *vbasedev = &vdev->vbasedev;
     struct vfio_region_info *reg_info;
@@ -2233,18 +2233,18 @@ static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
     /* Sanity check device */
     if (!(vbasedev->flags & VFIO_DEVICE_FLAGS_PCI)) {
         error_setg(errp, "this isn't a PCI device");
-        goto error;
+        return;
     }
 
     if (vbasedev->num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
         error_setg(errp, "unexpected number of io regions %u",
                    vbasedev->num_regions);
-        goto error;
+        return;
     }
 
     if (vbasedev->num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
         error_setg(errp, "unexpected number of irqs %u", vbasedev->num_irqs);
-        goto error;
+        return;
     }
 
     for (i = VFIO_PCI_BAR0_REGION_INDEX; i < VFIO_PCI_ROM_REGION_INDEX; i++) {
@@ -2256,7 +2256,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
 
         if (ret) {
             error_setg_errno(errp, -ret, "failed to get region %d info", i);
-            goto error;
+            return;
         }
 
         QLIST_INIT(&vdev->bars[i].quirks);
@@ -2266,7 +2266,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
                                VFIO_PCI_CONFIG_REGION_INDEX, &reg_info);
     if (ret) {
         error_setg_errno(errp, -ret, "failed to get config info");
-        goto error;
+        return;
     }
 
     trace_vfio_populate_device_config(vdev->vbasedev.name,
@@ -2287,7 +2287,7 @@ static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
         if (ret) {
             error_append_hint(errp, "device does not support "
                               "requested feature x-vga\n");
-            goto error;
+            return;
         }
     }
 
@@ -2297,7 +2297,6 @@ static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
     if (ret) {
         /* This can fail for an old kernel or legacy PCI dev */
         trace_vfio_populate_device_get_irq_info_failure();
-        ret = 0;
     } else if (irq_info.count == 1) {
         vdev->pci_aer = true;
     } else {
@@ -2305,9 +2304,6 @@ static int vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
                      "Could not enable error recovery for the device",
                      vbasedev->name);
     }
-
-error:
-    return ret;
 }
 
 static void vfio_put_device(VFIOPCIDevice *vdev)
@@ -2579,8 +2575,9 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
         goto error;
     }
 
-    ret = vfio_populate_device(vdev, errp);
-    if (ret) {
+    vfio_populate_device(vdev, &err);
+    if (err) {
+        error_propagate(errp, err);
         goto error;
     }
 

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 17/19] vfio/pci: Handle host oversight
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (15 preceding siblings ...)
  2016-10-17 19:54 ` [Qemu-devel] [PULL 16/19] vfio/pci: Remove vfio_populate_device " Alex Williamson
@ 2016-10-17 19:54 ` Alex Williamson
  2016-10-17 19:54 ` [Qemu-devel] [PULL 18/19] vfio/pci: Fix vfio_rtl8168_quirk_data_read address offset Alex Williamson
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Eric Auger, Markus Armbruster

From: Eric Auger <eric.auger@redhat.com>

In case the end-user calls qemu with -vfio-pci option without passing
either sysfsdev or host property value, the device is interpreted as
0000:00:00.0. Let's create a specific error message to guide the end-user.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 6d01324..fef436a 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2520,6 +2520,13 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
     int i, ret;
 
     if (!vdev->vbasedev.sysfsdev) {
+        if (!(~vdev->host.domain || ~vdev->host.bus ||
+              ~vdev->host.slot || ~vdev->host.function)) {
+            error_setg(errp, "No provided host device");
+            error_append_hint(errp, "Use -vfio-pci,host=DDDD:BB:DD.F "
+                              "or -vfio-pci,sysfsdev=PATH_TO_DEVICE\n");
+            return;
+        }
         vdev->vbasedev.sysfsdev =
             g_strdup_printf("/sys/bus/pci/devices/%04x:%02x:%02x.%01x",
                             vdev->host.domain, vdev->host.bus,
@@ -2828,6 +2835,10 @@ static void vfio_instance_init(Object *obj)
     device_add_bootindex_property(obj, &vdev->bootindex,
                                   "bootindex", NULL,
                                   &pci_dev->qdev, NULL);
+    vdev->host.domain = ~0U;
+    vdev->host.bus = ~0U;
+    vdev->host.slot = ~0U;
+    vdev->host.function = ~0U;
 }
 
 static Property vfio_pci_dev_properties[] = {

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 18/19] vfio/pci: Fix vfio_rtl8168_quirk_data_read address offset
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (16 preceding siblings ...)
  2016-10-17 19:54 ` [Qemu-devel] [PULL 17/19] vfio/pci: Handle host oversight Alex Williamson
@ 2016-10-17 19:54 ` Alex Williamson
  2016-10-17 19:54 ` [Qemu-devel] [PULL 19/19] vfio: fix duplicate function call Alex Williamson
  2016-10-18 11:34 ` [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Peter Maydell
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thorsten Kohfeldt

From: Thorsten Kohfeldt <thorsten.kohfeldt@gmx.de>

Introductory comment for rtl8168 VFIO MSI-X quirk states:
At BAR2 offset 0x70 there is a dword data register,
         offset 0x74 is a dword address register.
vfio: vfio_bar_read(0000:05:00.0:BAR2+0x70, 4) = 0xfee00398 // read data

Thus, correct offset for data read is 0x70,
but function vfio_rtl8168_quirk_data_read() wrongfully uses offset 0x74.

Signed-off-by: Thorsten Kohfeldt <thorsten.kohfeldt@gmx.de>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci-quirks.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 2cbda08..811eecd 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -898,7 +898,7 @@ static uint64_t vfio_rtl8168_quirk_data_read(void *opaque,
 {
     VFIOrtl8168Quirk *rtl = opaque;
     VFIOPCIDevice *vdev = rtl->vdev;
-    uint64_t data = vfio_region_read(&vdev->bars[2].region, addr + 0x74, size);
+    uint64_t data = vfio_region_read(&vdev->bars[2].region, addr + 0x70, size);
 
     if (rtl->enabled && (vdev->pdev.cap_present & QEMU_PCI_CAP_MSIX)) {
         hwaddr offset = rtl->addr & 0xfff;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Qemu-devel] [PULL 19/19] vfio: fix duplicate function call
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (17 preceding siblings ...)
  2016-10-17 19:54 ` [Qemu-devel] [PULL 18/19] vfio/pci: Fix vfio_rtl8168_quirk_data_read address offset Alex Williamson
@ 2016-10-17 19:54 ` Alex Williamson
  2016-10-18 11:34 ` [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Peter Maydell
  19 siblings, 0 replies; 21+ messages in thread
From: Alex Williamson @ 2016-10-17 19:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: Cao jin

From: Cao jin <caoj.fnst@cn.fujitsu.com>

When vfio device is reset(encounter FLR, or bus reset), if need to do
bus reset(vfio_pci_hot_reset_one is called), vfio_pci_pre_reset &
vfio_pci_post_reset will be called twice.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index fef436a..65d30fd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1951,7 +1951,9 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 
     trace_vfio_pci_hot_reset(vdev->vbasedev.name, single ? "one" : "multi");
 
-    vfio_pci_pre_reset(vdev);
+    if (!single) {
+        vfio_pci_pre_reset(vdev);
+    }
     vdev->vbasedev.needs_reset = false;
 
     info = g_malloc0(sizeof(*info));
@@ -2109,7 +2111,9 @@ out:
         }
     }
 out_single:
-    vfio_pci_post_reset(vdev);
+    if (!single) {
+        vfio_pci_post_reset(vdev);
+    }
     g_free(info);
 
     return ret;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17
  2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
                   ` (18 preceding siblings ...)
  2016-10-17 19:54 ` [Qemu-devel] [PULL 19/19] vfio: fix duplicate function call Alex Williamson
@ 2016-10-18 11:34 ` Peter Maydell
  19 siblings, 0 replies; 21+ messages in thread
From: Peter Maydell @ 2016-10-18 11:34 UTC (permalink / raw)
  To: Alex Williamson; +Cc: QEMU Developers

On 17 October 2016 at 20:51, Alex Williamson <alex.williamson@redhat.com> wrote:
> The following changes since commit 0975b8b823a888d474fa33821dfe84e6904db197:
>
>   Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging (2016-10-17 16:17:51 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/awilliam/qemu-vfio.git tags/vfio-updates-20161017.0
>
> for you to fetch changes up to 893bfc3cc893ed36cedc364e99cf483e9b08c294:
>
>   vfio: fix duplicate function call (2016-10-17 10:58:03 -0600)
>
> ----------------------------------------------------------------
> VFIO updates 2016-10-17
>
>  - Convert to realize & improve error reporting (Eric Auger)
>  - RTL quirk bug fix (Thorsten Kohfeldt)
>  - Skip duplicate pre/post reset (Cao jin)
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2016-10-18 11:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-17 19:51 [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Alex Williamson
2016-10-17 19:51 ` [Qemu-devel] [PULL 01/19] vfio/pci: Use local error object in vfio_initfn Alex Williamson
2016-10-17 19:52 ` [Qemu-devel] [PULL 02/19] vfio/pci: Pass an error object to vfio_populate_vga Alex Williamson
2016-10-17 19:52 ` [Qemu-devel] [PULL 03/19] vfio/pci: Pass an error object to vfio_populate_device Alex Williamson
2016-10-17 19:52 ` [Qemu-devel] [PULL 04/19] vfio/pci: Pass an error object to vfio_msix_early_setup Alex Williamson
2016-10-17 19:52 ` [Qemu-devel] [PULL 05/19] vfio/pci: Pass an error object to vfio_intx_enable Alex Williamson
2016-10-17 19:52 ` [Qemu-devel] [PULL 06/19] vfio/pci: Pass an error object to vfio_add_capabilities Alex Williamson
2016-10-17 19:52 ` [Qemu-devel] [PULL 07/19] vfio/pci: Pass an error object to vfio_pci_igd_opregion_init Alex Williamson
2016-10-17 19:52 ` [Qemu-devel] [PULL 08/19] vfio: Pass an Error object to vfio_connect_container Alex Williamson
2016-10-17 19:53 ` [Qemu-devel] [PULL 09/19] vfio: Pass an error object to vfio_get_group Alex Williamson
2016-10-17 19:53 ` [Qemu-devel] [PULL 10/19] vfio: Pass an error object to vfio_get_device Alex Williamson
2016-10-17 19:53 ` [Qemu-devel] [PULL 11/19] vfio/platform: Pass an error object to vfio_populate_device Alex Williamson
2016-10-17 19:53 ` [Qemu-devel] [PULL 12/19] vfio/platform: fix a wrong returned value in vfio_populate_device Alex Williamson
2016-10-17 19:53 ` [Qemu-devel] [PULL 13/19] vfio/platform: Pass an error object to vfio_base_device_init Alex Williamson
2016-10-17 19:53 ` [Qemu-devel] [PULL 14/19] vfio/pci: Conversion to realize Alex Williamson
2016-10-17 19:53 ` [Qemu-devel] [PULL 15/19] vfio/pci: Remove vfio_msix_early_setup returned value Alex Williamson
2016-10-17 19:54 ` [Qemu-devel] [PULL 16/19] vfio/pci: Remove vfio_populate_device " Alex Williamson
2016-10-17 19:54 ` [Qemu-devel] [PULL 17/19] vfio/pci: Handle host oversight Alex Williamson
2016-10-17 19:54 ` [Qemu-devel] [PULL 18/19] vfio/pci: Fix vfio_rtl8168_quirk_data_read address offset Alex Williamson
2016-10-17 19:54 ` [Qemu-devel] [PULL 19/19] vfio: fix duplicate function call Alex Williamson
2016-10-18 11:34 ` [Qemu-devel] [PULL 00/19] VFIO updates 2016-10-17 Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.