From 426ec9049e29397af95f8c28e020590bd1fe57dd Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:56 -0600 Subject: [PATCH 01/19] vfio/pci: Use local error object in vfio_initfn 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 a5a620a0c4..417bf7f87c 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 c17602e533..b26b6cf753 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -30,6 +30,9 @@ #include #endif +#define ERR_PREFIX "vfio error: %s: " +#define WARN_PREFIX "vfio warning: %s: " + /*#define DEBUG_VFIO*/ #ifdef DEBUG_VFIO #define DPRINTF(fmt, ...) \ From cde4279baa246fdfadb06d021e3b8d0a0234879f Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:57 -0600 Subject: [PATCH 02/19] vfio/pci: Pass an error object to vfio_populate_vga 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 bec694c8d8..806ea5d21a 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 417bf7f87c..9645a77fb6 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, ®_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 7d482d9d21..87a62f921a 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); From 2312d907ddcdeffa45350e5960be46d6105b8ec2 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:57 -0600 Subject: [PATCH 03/19] vfio/pci: Pass an error object to vfio_populate_device 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 9645a77fb6..46e3cb8378 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, ®_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 */ From 008d0e2d7bb1bb8c0393d5cc7501a05c2bc4ed81 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:58 -0600 Subject: [PATCH 04/19] vfio/pci: Pass an error object to vfio_msix_early_setup 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 46e3cb8378..02e92b0f6e 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); From 7dfb34247e231e1f317e3558af96c90f06fef8a5 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:58 -0600 Subject: [PATCH 05/19] vfio/pci: Pass an error object to vfio_intx_enable 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 02e92b0f6e..42161c8a7d 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; } From 7ef165b9a8d903babddaf9c61c54ae9b6e739604 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:58 -0600 Subject: [PATCH 06/19] vfio/pci: Pass an error object to vfio_add_capabilities 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 42161c8a7d..e2cf6acb6a 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; } From 7237011d0596e57f42d2a4e0556034b8d13533fe Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:59 -0600 Subject: [PATCH 07/19] vfio/pci: Pass an error object to vfio_pci_igd_opregion_init 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 806ea5d21a..2cbda0844e 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 e2cf6acb6a..3d84126e87 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 87a62f921a..a8366bb2a7 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 */ From 01905f58f166646619c35a2ebfc3ca3ed4cad62d Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:59 -0600 Subject: [PATCH 08/19] vfio: Pass an Error object to vfio_connect_container 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 29188a12fc..85a7759a29 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; } From 1b808d5be070e9d07e5d0e5b825a31a0cf87001d Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:57:59 -0600 Subject: [PATCH 09/19] vfio: Pass an error object to vfio_get_group 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 85a7759a29..90b1ebba2c 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 3d84126e87..fdb0616f8d 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 a559e7b659..7bf525b918 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 b26b6cf753..286fa31290 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); From 59f7d6743ccbe17587e491dc5d79cad8bf31f76a Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:00 -0600 Subject: [PATCH 10/19] vfio: Pass an error object to vfio_get_device 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 90b1ebba2c..9505fb3040 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- " - "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- " + "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 fdb0616f8d..0ba071172b 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 7bf525b918..9014ea7402 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 286fa31290..c582de18c9 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; From 5ff7419d4c7706fc3f625c0fd48cae191d2cecbc Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:00 -0600 Subject: [PATCH 11/19] vfio/platform: Pass an error object to vfio_populate_device 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 9014ea7402..1a35da04c3 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); } From 0d84f47bff2f98d0cea5fb27e9b62225ccebd70a Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:00 -0600 Subject: [PATCH 12/19] vfio/platform: fix a wrong returned value in vfio_populate_device 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- hw/vfio/platform.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index 1a35da04c3..484e31f233 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; } } From 9bdbfbd50dfcc23252224112e8cbb720a8535d40 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:01 -0600 Subject: [PATCH 13/19] vfio/platform: Pass an error object to vfio_base_device_init 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 484e31f233..a4663c918e 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 = { From 1a22aca1d0071bdc4abc1a72894a13f0076b2404 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:01 -0600 Subject: [PATCH 14/19] vfio/pci: Conversion to realize 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 0ba071172b..d9652c2c9d 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 da133221de..ef81609b98 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" From ec3bcf424edb76e00b6ff545360329c4cedf9953 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:01 -0600 Subject: [PATCH 15/19] vfio/pci: Remove vfio_msix_early_setup returned value 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 d9652c2c9d..f063c65cde 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; } From e04cff9d975389b83d59b89cc3904b20ae0b4ce1 Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:02 -0600 Subject: [PATCH 16/19] vfio/pci: Remove vfio_populate_device returned value 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- 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 f063c65cde..6d013249fc 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, ®_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; } From 4a946268504e72fe0c547b9dda97adbe277a585f Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Mon, 17 Oct 2016 10:58:02 -0600 Subject: [PATCH 17/19] vfio/pci: Handle host oversight 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 Reviewed-by: Markus Armbruster Signed-off-by: Alex Williamson --- hw/vfio/pci.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index 6d013249fc..fef436a1de 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[] = { From 31e6a7b17b35711eb44f0e686b5ba68d15bfe4c1 Mon Sep 17 00:00:00 2001 From: Thorsten Kohfeldt Date: Mon, 17 Oct 2016 10:58:02 -0600 Subject: [PATCH 18/19] vfio/pci: Fix vfio_rtl8168_quirk_data_read address offset 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 Signed-off-by: Alex Williamson --- 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 2cbda0844e..811eecd1b4 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; From 893bfc3cc893ed36cedc364e99cf483e9b08c294 Mon Sep 17 00:00:00 2001 From: Cao jin Date: Mon, 17 Oct 2016 10:58:03 -0600 Subject: [PATCH 19/19] vfio: fix duplicate function call 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 Signed-off-by: Alex Williamson --- 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 fef436a1de..65d30fdef9 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;