From cf10a5b18f4eb25004cffde15c770dadaa3c4bde Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:32 +0200 Subject: pci-assign: accept Error from monitor_handle_fd_param2() Propagate any errors in monitor fd handling up to get_real_device(), and report them there. We'll continue the propagation upwards when get_real_device() becomes a leaf itself (when none of its callees will report errors internally any longer when detecting and returning an error). Signed-off-by: Laszlo Ersek eviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index a825871..bfce97f 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -541,6 +541,7 @@ static int get_real_device(AssignedDevice *pci_dev) uint16_t id; PCIRegion *rp; PCIDevRegions *dev = &pci_dev->real_device; + Error *local_err = NULL; dev->region_number = 0; @@ -551,8 +552,12 @@ static int get_real_device(AssignedDevice *pci_dev) snprintf(name, sizeof(name), "%sconfig", dir); if (pci_dev->configfd_name && *pci_dev->configfd_name) { - dev->config_fd = monitor_handle_fd_param(cur_mon, pci_dev->configfd_name); - if (dev->config_fd < 0) { + dev->config_fd = monitor_handle_fd_param2(cur_mon, + pci_dev->configfd_name, + &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); return 1; } } else { -- cgit v1.1 From 4951013ff58a87e7f74393c6c6c2f964ee59de47 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:33 +0200 Subject: pci-assign: make assign_failed_examine() just format the cause This allows us to report the entire error with one error_report() call, easing future error propagation. Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index bfce97f..6b8db25 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -731,7 +731,12 @@ static void free_assigned_device(AssignedDevice *dev) free_msi_virqs(dev); } -static void assign_failed_examine(AssignedDevice *dev) +/* This function tries to determine the cause of the PCI assignment failure. It + * always returns the cause as a dynamically allocated, human readable string. + * If the function fails to determine the cause for any internal reason, then + * the returned string will state that fact. + */ +static char *assign_failed_examine(const AssignedDevice *dev) { char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns; uint16_t vendor_id, device_id; @@ -761,8 +766,8 @@ static void assign_failed_examine(AssignedDevice *dev) goto fail; } - error_printf("*** The driver '%s' is occupying your device " - "%04x:%02x:%02x.%x.\n" + return g_strdup_printf( + "*** The driver '%s' is occupying your device %04x:%02x:%02x.%x.\n" "***\n" "*** You can try the following commands to free it:\n" "***\n" @@ -778,10 +783,8 @@ static void assign_failed_examine(AssignedDevice *dev) ns, dev->host.domain, dev->host.bus, dev->host.slot, dev->host.function, vendor_id, device_id); - return; - fail: - error_report("Couldn't find out why."); + return g_strdup("Couldn't find out why."); } static int assign_device(AssignedDevice *dev) @@ -810,14 +813,19 @@ static int assign_device(AssignedDevice *dev) r = kvm_device_pci_assign(kvm_state, &dev->host, flags, &dev->dev_id); if (r < 0) { - error_report("Failed to assign device \"%s\" : %s", - dev->dev.qdev.id, strerror(-r)); - switch (r) { - case -EBUSY: - assign_failed_examine(dev); + case -EBUSY: { + char *cause; + + cause = assign_failed_examine(dev); + error_report("Failed to assign device \"%s\" : %s\n%s", + dev->dev.qdev.id, strerror(-r), cause); + g_free(cause); break; + } default: + error_report("Failed to assign device \"%s\" : %s", + dev->dev.qdev.id, strerror(-r)); break; } } -- cgit v1.1 From bcdcf75d6230fcd5a8a6578c5cf15e7c643328e8 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:34 +0200 Subject: pci-assign: propagate errors from get_real_id() get_real_id() has two thin wrappers (and no other callers), get_real_vendor_id() and get_real_device_id(); it's easiest to convert them in one fell swoop. Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 6b8db25..997ef09 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -499,7 +499,8 @@ static int assigned_dev_register_regions(PCIRegion *io_regions, return 0; } -static int get_real_id(const char *devpath, const char *idname, uint16_t *val) +static void get_real_id(const char *devpath, const char *idname, uint16_t *val, + Error **errp) { FILE *f; char name[128]; @@ -508,34 +509,33 @@ static int get_real_id(const char *devpath, const char *idname, uint16_t *val) snprintf(name, sizeof(name), "%s%s", devpath, idname); f = fopen(name, "r"); if (f == NULL) { - error_report("%s: %s: %m", __func__, name); - return -1; + error_setg_file_open(errp, errno, name); + return; } if (fscanf(f, "%li\n", &id) == 1) { *val = id; } else { - fclose(f); - return -1; + error_setg(errp, "Failed to parse contents of '%s'", name); } fclose(f); - - return 0; } -static int get_real_vendor_id(const char *devpath, uint16_t *val) +static void get_real_vendor_id(const char *devpath, uint16_t *val, + Error **errp) { - return get_real_id(devpath, "vendor", val); + get_real_id(devpath, "vendor", val, errp); } -static int get_real_device_id(const char *devpath, uint16_t *val) +static void get_real_device_id(const char *devpath, uint16_t *val, + Error **errp) { - return get_real_id(devpath, "device", val); + get_real_id(devpath, "device", val, errp); } static int get_real_device(AssignedDevice *pci_dev) { char dir[128], name[128]; - int fd, r = 0, v; + int fd, r = 0; FILE *f; uint64_t start, end, size, flags; uint16_t id; @@ -639,16 +639,20 @@ again: fclose(f); /* read and fill vendor ID */ - v = get_real_vendor_id(dir, &id); - if (v) { + get_real_vendor_id(dir, &id, &local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return 1; } pci_dev->dev.config[0] = id & 0xff; pci_dev->dev.config[1] = (id & 0xff00) >> 8; /* read and fill device ID */ - v = get_real_device_id(dir, &id); - if (v) { + get_real_device_id(dir, &id, &local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return 1; } pci_dev->dev.config[2] = id & 0xff; @@ -741,6 +745,7 @@ static char *assign_failed_examine(const AssignedDevice *dev) char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns; uint16_t vendor_id, device_id; int r; + Error *local_err = NULL; snprintf(dir, sizeof(dir), "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/", dev->host.domain, dev->host.bus, dev->host.slot, @@ -761,8 +766,12 @@ static char *assign_failed_examine(const AssignedDevice *dev) ns++; - if (get_real_vendor_id(dir, &vendor_id) || - get_real_device_id(dir, &device_id)) { + if ((get_real_vendor_id(dir, &vendor_id, &local_err), local_err) || + (get_real_device_id(dir, &device_id, &local_err), local_err)) { + /* We're already analyzing an assignment error, so we suppress this + * one just like the others above. + */ + error_free(local_err); goto fail; } -- cgit v1.1 From 665f119fbad97c05c2603673ac6b2dcbf0d0e9e1 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:35 +0200 Subject: pci-assign: propagate Error from check_irqchip_in_kernel() Rename check_irqchip_in_kernel() to verify_irqchip_in_kernel(), so that the name reflects our expectation better. Rather than returning a bool, make it do nothing or set an Error. Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 997ef09..b4696aa 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -841,14 +841,12 @@ static int assign_device(AssignedDevice *dev) return r; } -static bool check_irqchip_in_kernel(void) +static void verify_irqchip_in_kernel(Error **errp) { if (kvm_irqchip_in_kernel()) { - return true; + return; } - error_report("pci-assign: error: requires KVM with in-kernel irqchip " - "enabled"); - return false; + error_setg(errp, "pci-assign requires KVM with in-kernel irqchip enabled"); } static int assign_intx(AssignedDevice *dev) @@ -857,6 +855,7 @@ static int assign_intx(AssignedDevice *dev) PCIINTxRoute intx_route; bool intx_host_msi; int r; + Error *local_err = NULL; /* Interrupt PIN 0 means don't use INTx */ if (assigned_dev_pci_read_byte(&dev->dev, PCI_INTERRUPT_PIN) == 0) { @@ -864,7 +863,10 @@ static int assign_intx(AssignedDevice *dev) return 0; } - if (!check_irqchip_in_kernel()) { + verify_irqchip_in_kernel(&local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return -ENOTSUP; } @@ -1241,6 +1243,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); PCIRegion *pci_region = dev->real_device.regions; int ret, pos; + Error *local_err = NULL; /* Clear initial capabilities pointer and status copied from hw */ pci_set_byte(pci_dev->config + PCI_CAPABILITY_LIST, 0); @@ -1252,7 +1255,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) * MSI capability is the 1st capability in capability config */ pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0); if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) { - if (!check_irqchip_in_kernel()) { + verify_irqchip_in_kernel(&local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return -ENOTSUP; } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI; @@ -1281,7 +1287,10 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) int bar_nr; uint32_t msix_table_entry; - if (!check_irqchip_in_kernel()) { + verify_irqchip_in_kernel(&local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return -ENOTSUP; } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX; -- cgit v1.1 From cd9aa33e2cab11fb89071dc2f48550431406e524 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:36 +0200 Subject: pci: add Error-propagating pci_add_capability2() ... and rebase pci_add_capability() to it. Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/pci/pci.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 517ff2a..22fe5ee 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2013,12 +2013,32 @@ static void pci_del_option_rom(PCIDevice *pdev) int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size) { + int ret; + Error *local_err = NULL; + + ret = pci_add_capability2(pdev, cap_id, offset, size, &local_err); + if (local_err) { + assert(ret < 0); + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); + } else { + /* success implies a positive offset in config space */ + assert(ret > 0); + } + return ret; +} + +int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id, + uint8_t offset, uint8_t size, + Error **errp) +{ uint8_t *config; int i, overlapping_cap; if (!offset) { offset = pci_find_space(pdev, size); if (!offset) { + error_setg(errp, "out of PCI config space"); return -ENOSPC; } } else { @@ -2029,12 +2049,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, for (i = offset; i < offset + size; i++) { overlapping_cap = pci_find_capability_at_offset(pdev, i); if (overlapping_cap) { - fprintf(stderr, "ERROR: %s:%02x:%02x.%x " - "Attempt to add PCI capability %x at offset " - "%x overlaps existing capability %x at offset %x\n", - pci_root_bus_path(pdev), pci_bus_num(pdev->bus), - PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), - cap_id, offset, overlapping_cap, i); + error_setg(errp, "%s:%02x:%02x.%x " + "Attempt to add PCI capability %x at offset " + "%x overlaps existing capability %x at offset %x", + pci_root_bus_path(pdev), pci_bus_num(pdev->bus), + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), + cap_id, offset, overlapping_cap, i); return -EINVAL; } } -- cgit v1.1 From 42ee4194f2ac7ff16f8e5cb7900c7d35d483f974 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:37 +0200 Subject: pci-assign: accept Error from pci_add_capability2() Propagate any errors while adding PCI capabilities to assigned_device_pci_cap_init(). We'll continue the propagation upwards when assigned_device_pci_cap_init() becomes a leaf itself (when none of its callees will report errors internally any longer when detecting and returning them). Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index b4696aa..f91d4fb 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -1263,8 +1263,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI; /* Only 32-bit/no-mask currently supported */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10, + &local_err); if (ret < 0) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return ret; } pci_dev->msi_cap = pos; @@ -1294,8 +1297,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return -ENOTSUP; } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX; - ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12, + &local_err); if (ret < 0) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return ret; } pci_dev->msix_cap = pos; @@ -1322,8 +1328,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) if (pos) { uint16_t pmc; - ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF, + &local_err); if (ret < 0) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return ret; } @@ -1388,8 +1397,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return -EINVAL; } - ret = pci_add_capability(pci_dev, PCI_CAP_ID_EXP, pos, size); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_EXP, pos, size, + &local_err); if (ret < 0) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return ret; } @@ -1462,8 +1474,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) uint32_t status; /* Only expose the minimum, 8 byte capability */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PCIX, pos, 8, + &local_err); if (ret < 0) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return ret; } @@ -1488,8 +1503,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VPD, 0); if (pos) { /* Direct R/W passthrough */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VPD, pos, 8, + &local_err); if (ret < 0) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return ret; } @@ -1504,8 +1522,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pos += PCI_CAP_LIST_NEXT) { uint8_t len = pci_get_byte(pci_dev->config + pos + PCI_CAP_FLAGS); /* Direct R/W passthrough */ - ret = pci_add_capability(pci_dev, PCI_CAP_ID_VNDR, pos, len); + ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VNDR, pos, len, + &local_err); if (ret < 0) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return ret; } -- cgit v1.1 From f3455d47046f604f1ca7353dd519ea2d2e5fa798 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:38 +0200 Subject: pci-assign: assignment should fail if we can't read config space assigned_initfn() get_real_device() read() Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 1 + 1 file changed, 1 insertion(+) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index f91d4fb..e89bb6a 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -576,6 +576,7 @@ again: goto again; } error_report("%s: read failed, errno = %d", __func__, errno); + return 1; } /* Restore or clear multifunction, this is always controlled by qemu */ -- cgit v1.1 From 5b877045d31e25aaf089f595c6a3fe8aedb8539a Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:39 +0200 Subject: pci-assign: propagate errors from get_real_device() Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index e89bb6a..c6d1094 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -532,7 +532,7 @@ static void get_real_device_id(const char *devpath, uint16_t *val, get_real_id(devpath, "device", val, errp); } -static int get_real_device(AssignedDevice *pci_dev) +static void get_real_device(AssignedDevice *pci_dev, Error **errp) { char dir[128], name[128]; int fd, r = 0; @@ -556,16 +556,15 @@ static int get_real_device(AssignedDevice *pci_dev) pci_dev->configfd_name, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); - return 1; + error_propagate(errp, local_err); + return; } } else { dev->config_fd = open(name, O_RDWR); if (dev->config_fd == -1) { - error_report("%s: %s: %m", __func__, name); - return 1; + error_setg_file_open(errp, errno, name); + return; } } again: @@ -575,8 +574,10 @@ again: if (errno == EINTR || errno == EAGAIN) { goto again; } - error_report("%s: read failed, errno = %d", __func__, errno); - return 1; + error_setg_errno(errp, errno, "read(\"%s\")", + (pci_dev->configfd_name && *pci_dev->configfd_name) ? + pci_dev->configfd_name : name); + return; } /* Restore or clear multifunction, this is always controlled by qemu */ @@ -596,8 +597,8 @@ again: f = fopen(name, "r"); if (f == NULL) { - error_report("%s: %s: %m", __func__, name); - return 1; + error_setg_file_open(errp, errno, name); + return; } for (r = 0; r < PCI_ROM_SLOT; r++) { @@ -642,9 +643,8 @@ again: /* read and fill vendor ID */ get_real_vendor_id(dir, &id, &local_err); if (local_err) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); - return 1; + error_propagate(errp, local_err); + return; } pci_dev->dev.config[0] = id & 0xff; pci_dev->dev.config[1] = (id & 0xff00) >> 8; @@ -652,9 +652,8 @@ again: /* read and fill device ID */ get_real_device_id(dir, &id, &local_err); if (local_err) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); - return 1; + error_propagate(errp, local_err); + return; } pci_dev->dev.config[2] = id & 0xff; pci_dev->dev.config[3] = (id & 0xff00) >> 8; @@ -663,7 +662,6 @@ again: PCI_COMMAND_MASTER | PCI_COMMAND_INTX_DISABLE); dev->region_number = r; - return 0; } static void free_msi_virqs(AssignedDevice *dev) @@ -1751,6 +1749,7 @@ static int assigned_initfn(struct PCIDevice *pci_dev) AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); uint8_t e_intx; int r; + Error *local_err = NULL; if (!kvm_enabled()) { error_report("pci-assign: error: requires KVM support"); @@ -1783,9 +1782,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev) memcpy(dev->emulate_config_write, dev->emulate_config_read, sizeof(dev->emulate_config_read)); - if (get_real_device(dev)) { - error_report("pci-assign: Error: Couldn't get real device (%s)!", - dev->dev.qdev.id); + get_real_device(dev, &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); goto out; } -- cgit v1.1 From 64135217a75ac6c4e672ed67cb8fee7a4c16ada4 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:40 +0200 Subject: pci-assign: propagate errors from assigned_device_pci_cap_init() Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index c6d1094..2de6559 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -1237,7 +1237,7 @@ static void assigned_dev_setup_cap_read(AssignedDevice *dev, uint32_t offset, assigned_dev_emulate_config_read(dev, offset + PCI_CAP_LIST_NEXT, 1); } -static int assigned_device_pci_cap_init(PCIDevice *pci_dev) +static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp) { AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); PCIRegion *pci_region = dev->real_device.regions; @@ -1256,8 +1256,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) { verify_irqchip_in_kernel(&local_err); if (local_err) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return -ENOTSUP; } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI; @@ -1265,8 +1264,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10, &local_err); if (ret < 0) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return ret; } pci_dev->msi_cap = pos; @@ -1291,16 +1289,14 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) verify_irqchip_in_kernel(&local_err); if (local_err) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return -ENOTSUP; } dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX; ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12, &local_err); if (ret < 0) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return ret; } pci_dev->msix_cap = pos; @@ -1330,8 +1326,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF, &local_err); if (ret < 0) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -1369,8 +1364,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) */ size = MIN(0x3c, PCI_CONFIG_SPACE_SIZE - pos); if (size < 0x34) { - error_report("%s: Invalid size PCIe cap-id 0x%x", - __func__, PCI_CAP_ID_EXP); + error_setg(errp, "Invalid size PCIe cap-id 0x%x", + PCI_CAP_ID_EXP); return -EINVAL; } else if (size != 0x3c) { error_report("WARNING, %s: PCIe cap-id 0x%x has " @@ -1391,16 +1386,15 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) } if (size == 0) { - error_report("%s: Unsupported PCI express capability version %d", - __func__, version); + error_setg(errp, "Unsupported PCI express capability version %d", + version); return -EINVAL; } ret = pci_add_capability2(pci_dev, PCI_CAP_ID_EXP, pos, size, &local_err); if (ret < 0) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -1410,8 +1404,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) type = (type & PCI_EXP_FLAGS_TYPE) >> 4; if (type != PCI_EXP_TYPE_ENDPOINT && type != PCI_EXP_TYPE_LEG_END && type != PCI_EXP_TYPE_RC_END) { - error_report("Device assignment only supports endpoint assignment," - " device type %d", type); + error_setg(errp, "Device assignment only supports endpoint " + "assignment, device type %d", type); return -EINVAL; } @@ -1476,8 +1470,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PCIX, pos, 8, &local_err); if (ret < 0) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -1505,8 +1498,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VPD, pos, 8, &local_err); if (ret < 0) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -1524,8 +1516,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VNDR, pos, len, &local_err); if (ret < 0) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return ret; } @@ -1789,7 +1780,9 @@ static int assigned_initfn(struct PCIDevice *pci_dev) goto out; } - if (assigned_device_pci_cap_init(pci_dev) < 0) { + if (assigned_device_pci_cap_init(pci_dev, &local_err) < 0) { + qerror_report_err(local_err); + error_free(local_err); goto out; } -- cgit v1.1 From 7a98593b34d3e19d77c43a69dd47f2387250d67d Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:41 +0200 Subject: pci-assign: propagate errors from assigned_dev_register_msix_mmio() The return type is also changed from "int" to "void", because it was used in a success vs. failure sense only (the caller didn't distinguish error codes from each other, and even assigned_dev_register_msix_mmio() masked mmap()'s errno values with a common -EFAULT). Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 2de6559..3a904e8 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -1644,20 +1644,19 @@ static void assigned_dev_msix_reset(AssignedDevice *dev) } } -static int assigned_dev_register_msix_mmio(AssignedDevice *dev) +static void assigned_dev_register_msix_mmio(AssignedDevice *dev, Error **errp) { dev->msix_table = mmap(NULL, MSIX_PAGE_SIZE, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, 0, 0); if (dev->msix_table == MAP_FAILED) { - error_report("fail allocate msix_table! %s", strerror(errno)); - return -EFAULT; + error_setg_errno(errp, errno, "failed to allocate msix_table"); + return; } assigned_dev_msix_reset(dev); memory_region_init_io(&dev->mmio, OBJECT(dev), &assigned_dev_msix_mmio_ops, dev, "assigned-dev-msix", MSIX_PAGE_SIZE); - return 0; } static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev) @@ -1788,7 +1787,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev) /* intercept MSI-X entry page in the MMIO */ if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) { - if (assigned_dev_register_msix_mmio(dev)) { + assigned_dev_register_msix_mmio(dev, &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); goto out; } } -- cgit v1.1 From 7d9cb533f5a475c3d87b8c7c3fd65b2b6530cdac Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:42 +0200 Subject: pci-assign: propagate errors from assigned_dev_register_regions() Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 3a904e8..9aa92a1 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -394,9 +394,10 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) return 0; } -static int assigned_dev_register_regions(PCIRegion *io_regions, - unsigned long regions_num, - AssignedDevice *pci_dev) +static void assigned_dev_register_regions(PCIRegion *io_regions, + unsigned long regions_num, + AssignedDevice *pci_dev, + Error **errp) { uint32_t i; PCIRegion *cur_region = io_regions; @@ -425,9 +426,9 @@ static int assigned_dev_register_regions(PCIRegion *io_regions, if (pci_dev->v_addrs[i].u.r_virtbase == MAP_FAILED) { pci_dev->v_addrs[i].u.r_virtbase = NULL; - error_report("%s: Error: Couldn't mmap 0x%" PRIx64 "!", - __func__, cur_region->base_addr); - return -1; + error_setg_errno(errp, errno, "Couldn't mmap 0x%" PRIx64 "!", + cur_region->base_addr); + return; } pci_dev->v_addrs[i].r_size = cur_region->size; @@ -496,7 +497,6 @@ static int assigned_dev_register_regions(PCIRegion *io_regions, } /* success */ - return 0; } static void get_real_id(const char *devpath, const char *idname, uint16_t *val, @@ -1796,9 +1796,12 @@ static int assigned_initfn(struct PCIDevice *pci_dev) } /* handle real device's MMIO/PIO BARs */ - if (assigned_dev_register_regions(dev->real_device.regions, - dev->real_device.region_number, - dev)) { + assigned_dev_register_regions(dev->real_device.regions, + dev->real_device.region_number, dev, + &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); goto out; } -- cgit v1.1 From 6877cff044cdf6da66885eab62363baf98bb39ee Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:43 +0200 Subject: pci-assign: propagate errors from assign_device() Also, change the return type to "void"; the function is static (with a sole caller) and the negative errno values are not distinguished from each other. Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 9aa92a1..0fedca8 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -795,7 +795,7 @@ fail: return g_strdup("Couldn't find out why."); } -static int assign_device(AssignedDevice *dev) +static void assign_device(AssignedDevice *dev, Error **errp) { uint32_t flags = KVM_DEV_ASSIGN_ENABLE_IOMMU; int r; @@ -803,15 +803,15 @@ static int assign_device(AssignedDevice *dev) /* Only pass non-zero PCI segment to capable module */ if (!kvm_check_extension(kvm_state, KVM_CAP_PCI_SEGMENT) && dev->host.domain) { - error_report("Can't assign device inside non-zero PCI segment " - "as this KVM module doesn't support it."); - return -ENODEV; + error_setg(errp, "Can't assign device inside non-zero PCI segment " + "as this KVM module doesn't support it."); + return; } if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) { - error_report("No IOMMU found. Unable to assign device \"%s\"", - dev->dev.qdev.id); - return -ENODEV; + error_setg(errp, "No IOMMU found. Unable to assign device \"%s\"", + dev->dev.qdev.id); + return; } if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK && @@ -826,18 +826,17 @@ static int assign_device(AssignedDevice *dev) char *cause; cause = assign_failed_examine(dev); - error_report("Failed to assign device \"%s\" : %s\n%s", - dev->dev.qdev.id, strerror(-r), cause); + error_setg_errno(errp, -r, "Failed to assign device \"%s\"\n%s", + dev->dev.qdev.id, cause); g_free(cause); break; } default: - error_report("Failed to assign device \"%s\" : %s", - dev->dev.qdev.id, strerror(-r)); + error_setg_errno(errp, -r, "Failed to assign device \"%s\"", + dev->dev.qdev.id); break; } } - return r; } static void verify_irqchip_in_kernel(Error **errp) @@ -1812,8 +1811,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev) dev->intx_route.irq = -1; /* assign device to guest */ - r = assign_device(dev); - if (r < 0) { + assign_device(dev, &local_err); + if (local_err) { + qerror_report_err(local_err); + error_free(local_err); goto out; } -- cgit v1.1 From ef47827ac4da5217243e1fec7ec75c1924eb4f40 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:44 +0200 Subject: pci-assign: propagate errors from assign_intx() Among the callers, only assigned_initfn() should set the monitor's stored error. Other callers may run in contexts where the monitor's stored error makes no sense. For example: assigned_dev_pci_write_config() assigned_dev_update_msix() assign_intx() Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 0fedca8..6891729 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -847,7 +847,7 @@ static void verify_irqchip_in_kernel(Error **errp) error_setg(errp, "pci-assign requires KVM with in-kernel irqchip enabled"); } -static int assign_intx(AssignedDevice *dev) +static int assign_intx(AssignedDevice *dev, Error **errp) { AssignedIRQType new_type; PCIINTxRoute intx_route; @@ -863,8 +863,7 @@ static int assign_intx(AssignedDevice *dev) verify_irqchip_in_kernel(&local_err); if (local_err) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); + error_propagate(errp, local_err); return -ENOTSUP; } @@ -927,10 +926,11 @@ retry: dev->features |= ASSIGNED_DEVICE_PREFER_MSI_MASK; goto retry; } - error_report("Failed to assign irq for \"%s\": %s", - dev->dev.qdev.id, strerror(-r)); - error_report("Perhaps you are assigning a device " - "that shares an IRQ with another device?"); + error_setg_errno(errp, -r, + "Failed to assign irq for \"%s\"\n" + "Perhaps you are assigning a device " + "that shares an IRQ with another device?", + dev->dev.qdev.id); return r; } @@ -956,8 +956,11 @@ static void assigned_dev_update_irq_routing(PCIDevice *dev) Error *err = NULL; int r; - r = assign_intx(assigned_dev); + r = assign_intx(assigned_dev, &err); if (r < 0) { + error_report("%s", error_get_pretty(err)); + error_free(err); + err = NULL; qdev_unplug(&dev->qdev, &err); assert(!err); } @@ -1008,7 +1011,13 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev) assigned_dev->intx_route.irq = -1; assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSI; } else { - assign_intx(assigned_dev); + Error *local_err = NULL; + + assign_intx(assigned_dev, &local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); + } } } @@ -1150,7 +1159,13 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev) assigned_dev->intx_route.irq = -1; assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSIX; } else { - assign_intx(assigned_dev); + Error *local_err = NULL; + + assign_intx(assigned_dev, &local_err); + if (local_err) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); + } } } @@ -1819,8 +1834,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev) } /* assign legacy INTx to the device */ - r = assign_intx(dev); + r = assign_intx(dev, &local_err); if (r < 0) { + qerror_report_err(local_err); + error_free(local_err); goto assigned_out; } -- cgit v1.1 From 636713bad4240836947c04c26e1cd001d3bcbff1 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 10 Apr 2014 10:24:45 +0200 Subject: pci-assign: assigned_initfn(): set monitor error in common error handler Signed-off-by: Laszlo Ersek Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- hw/i386/kvm/pci-assign.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 6891729..e55421a 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -1756,14 +1756,14 @@ static int assigned_initfn(struct PCIDevice *pci_dev) Error *local_err = NULL; if (!kvm_enabled()) { - error_report("pci-assign: error: requires KVM support"); - return -1; + error_setg(&local_err, "pci-assign requires KVM support"); + goto exit_with_error; } if (!dev->host.domain && !dev->host.bus && !dev->host.slot && !dev->host.function) { - error_report("pci-assign: error: no host device specified"); - return -1; + error_setg(&local_err, "no host device specified"); + goto exit_with_error; } /* @@ -1788,14 +1788,10 @@ static int assigned_initfn(struct PCIDevice *pci_dev) get_real_device(dev, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); goto out; } if (assigned_device_pci_cap_init(pci_dev, &local_err) < 0) { - qerror_report_err(local_err); - error_free(local_err); goto out; } @@ -1803,8 +1799,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev) if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) { assigned_dev_register_msix_mmio(dev, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); goto out; } } @@ -1814,8 +1808,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev) dev->real_device.region_number, dev, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); goto out; } @@ -1828,16 +1820,12 @@ static int assigned_initfn(struct PCIDevice *pci_dev) /* assign device to guest */ assign_device(dev, &local_err); if (local_err) { - qerror_report_err(local_err); - error_free(local_err); goto out; } /* assign legacy INTx to the device */ r = assign_intx(dev, &local_err); if (r < 0) { - qerror_report_err(local_err); - error_free(local_err); goto assigned_out; } @@ -1849,8 +1837,14 @@ static int assigned_initfn(struct PCIDevice *pci_dev) assigned_out: deassign_device(dev); + out: free_assigned_device(dev); + +exit_with_error: + assert(local_err); + qerror_report_err(local_err); + error_free(local_err); return -1; } -- cgit v1.1