* [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer @ 2008-11-28 17:10 Mark McLoughlin 2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin 2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin 0 siblings, 2 replies; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin add_assigned_device() returns a pointer, so don't check the return value is less than zero. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/ipf.c | 2 +- qemu/hw/pc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qemu/hw/ipf.c b/qemu/hw/ipf.c index 37f2de7..c0ac9eb 100644 --- a/qemu/hw/ipf.c +++ b/qemu/hw/ipf.c @@ -650,7 +650,7 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, if (kvm_enabled()) { int i; for (i = 0; i < assigned_devices_index; i++) { - if (add_assigned_device(assigned_devices[i]) < 0) { + if (!add_assigned_device(assigned_devices[i])) { fprintf(stderr, "Warning: could not add assigned device %s\n", assigned_devices[i]); } diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c index 7d296c4..6f3339f 100644 --- a/qemu/hw/pc.c +++ b/qemu/hw/pc.c @@ -1187,7 +1187,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size, if (kvm_enabled()) { int i; for (i = 0; i < assigned_devices_index; i++) { - if (add_assigned_device(assigned_devices[i]) < 0) { + if (!add_assigned_device(assigned_devices[i])) { fprintf(stderr, "Warning: could not add assigned device %s\n", assigned_devices[i]); } -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() 2008-11-28 17:10 [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin 2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin 1 sibling, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Merge two copies of the same code and cleanup with no functional changes. Signed-off-by: Mark McLoughlin <markmc@redhat.com> fix error messages Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 32 +++++++++++++++++++++----------- qemu/hw/device-assignment.h | 2 +- qemu/hw/ipf.c | 16 ++-------------- qemu/hw/pc.c | 16 ++-------------- 4 files changed, 26 insertions(+), 40 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index 9a790c6..eb2a73a 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -554,17 +554,6 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) return &dev->dev; } -int init_all_assigned_devices(PCIBus *bus) -{ - struct AssignedDevInfo *adev; - - LIST_FOREACH(adev, &adev_head, next) - if (init_assigned_device(adev, bus) == NULL) - return -1; - - return 0; -} - /* * Syntax to assign device: * @@ -619,3 +608,24 @@ bad: qemu_free(adev); return NULL; } + +void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices) +{ + int i; + + for (i = 0; i < n_devices; i++) { + struct AssignedDevInfo *adev; + + adev = add_assigned_device(devices[i]); + if (!adev) { + fprintf(stderr, "Could not add assigned device %s\n", devices[i]); + continue; + } + + if (!init_assigned_device(adev, bus)) { + fprintf(stderr, "Failed to initialize assigned device %s\n", + devices[i]); + exit(1); + } + } +} diff --git a/qemu/hw/device-assignment.h b/qemu/hw/device-assignment.h index d6caa67..5a01d98 100644 --- a/qemu/hw/device-assignment.h +++ b/qemu/hw/device-assignment.h @@ -96,7 +96,7 @@ struct AssignedDevInfo { PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus); AssignedDevInfo *add_assigned_device(const char *arg); -int init_all_assigned_devices(PCIBus *bus); +void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices); #define MAX_DEV_ASSIGN_CMDLINE 8 diff --git a/qemu/hw/ipf.c b/qemu/hw/ipf.c index c0ac9eb..3e24c98 100644 --- a/qemu/hw/ipf.c +++ b/qemu/hw/ipf.c @@ -647,20 +647,8 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size, } #ifdef USE_KVM_DEVICE_ASSIGNMENT - if (kvm_enabled()) { - int i; - for (i = 0; i < assigned_devices_index; i++) { - if (!add_assigned_device(assigned_devices[i])) { - fprintf(stderr, "Warning: could not add assigned device %s\n", - assigned_devices[i]); - } - } - - if (init_all_assigned_devices(pci_bus)) { - fprintf(stderr, "Failed to initialize assigned devices\n"); - exit (1); - } - } + if (kvm_enabled()) + add_assigned_devices(pci_bus, assigned_devices, assigned_devices_index); #endif /* USE_KVM_DEVICE_ASSIGNMENT */ } diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c index 6f3339f..6de460c 100644 --- a/qemu/hw/pc.c +++ b/qemu/hw/pc.c @@ -1184,20 +1184,8 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size, virtio_balloon_init(pci_bus); #ifdef USE_KVM_DEVICE_ASSIGNMENT - if (kvm_enabled()) { - int i; - for (i = 0; i < assigned_devices_index; i++) { - if (!add_assigned_device(assigned_devices[i])) { - fprintf(stderr, "Warning: could not add assigned device %s\n", - assigned_devices[i]); - } - } - - if (init_all_assigned_devices(pci_bus)) { - fprintf(stderr, "Failed to initialize assigned devices\n"); - exit (1); - } - } + if (kvm_enabled()) + add_assigned_devices(pci_bus, assigned_devices, assigned_devices_index); #endif /* USE_KVM_DEVICE_ASSIGNMENT */ } -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails 2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin 2008-12-10 10:23 ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin 0 siblings, 2 replies; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin It's standard practice in qemu to exit if command line parameter fails, so do that here too. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index eb2a73a..8fbd66c 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -620,6 +620,7 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices) if (!adev) { fprintf(stderr, "Could not add assigned device %s\n", devices[i]); continue; + exit(1); } if (!init_assigned_device(adev, bus)) { -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails 2008-11-28 17:10 ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting Mark McLoughlin 2008-12-10 10:23 ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin 1 sibling, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Fixes a segfault in assigned_dev_update_irq() if assigning a device previously failed. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index 8fbd66c..b5cf6ee 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -550,8 +550,11 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) } adev->assigned_dev = dev; - out: return &dev->dev; + +out: + pci_unregister_device(&dev->dev); + return NULL; } /* -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting 2008-11-28 17:10 ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin 0 siblings, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin If kvm_assign_pci_device(), there's no need for us to print two lines of error messages. The string translation of errno is very useful though, so include that in the message using strerror(). Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index b5cf6ee..2b2ef68 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -540,12 +540,11 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) if (r && !adev->disable_iommu) assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU; #endif - + r = kvm_assign_pci_device(kvm_context, &assigned_dev_data); if (r < 0) { - fprintf(stderr, "Could not notify kernel about " - "assigned device \"%s\"\n", adev->name); - perror("register_real_device"); + fprintf(stderr, "Failed to assign device \"%s\" : %s\n", + adev->name, strerror(-r)); goto out; } -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages 2008-11-28 17:10 ` [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin 2008-11-28 19:05 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau 0 siblings, 2 replies; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Replace perror() usage with sane error message. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index 2b2ef68..b39617a 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -473,9 +473,10 @@ void assigned_dev_update_irq(PCIDevice *d) assigned_irq_data.host_irq = assigned_dev->real_device.irq; r = kvm_assign_irq(kvm_context, &assigned_irq_data); if (r < 0) { - perror("assigned_dev_update_irq"); - fprintf(stderr, "Are you assigning a device " - "that shares IRQ with some other device?\n"); + fprintf(stderr, "Failed to assign irq for \"%s\": %s\n", + adev->name, strerror(-r)); + fprintf(stderr, "Perhaps you re you assigning a device " + "that shares IRQ with another device?\n"); pci_unregister_device(&assigned_dev->dev); /* FIXME: Delete node from list */ continue; -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails 2008-11-28 17:10 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device() Mark McLoughlin 2008-11-28 19:05 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau 1 sibling, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Fix blatant issue where we leave a freed assigned device node on the list if irq assignment fails. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index b39617a..fde17ac 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -449,12 +449,14 @@ static uint32_t calc_assigned_dev_id(uint8_t bus, uint8_t devfn) */ void assigned_dev_update_irq(PCIDevice *d) { - int irq, r; - AssignedDevice *assigned_dev; AssignedDevInfo *adev; - LIST_FOREACH(adev, &adev_head, next) { - assigned_dev = adev->assigned_dev; + adev = LIST_FIRST(&adev_head); + while (adev) { + AssignedDevInfo *next = LIST_NEXT(adev, next); + AssignedDevice *assigned_dev = adev->assigned_dev; + int irq, r; + irq = pci_map_irq(&assigned_dev->dev, assigned_dev->intpin); irq = piix_get_irq(irq); @@ -477,12 +479,15 @@ void assigned_dev_update_irq(PCIDevice *d) adev->name, strerror(-r)); fprintf(stderr, "Perhaps you re you assigning a device " "that shares IRQ with another device?\n"); + LIST_REMOVE(adev, next); pci_unregister_device(&assigned_dev->dev); - /* FIXME: Delete node from list */ + adev = next; continue; } assigned_dev->girq = irq; } + + adev = next; } } -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device() 2008-11-28 17:10 ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails Mark McLoughlin 0 siblings, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Add a function to fully free the AssignedDevice and AssignedDevInfo and remove from the list. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 16 ++++++++++++++-- qemu/hw/device-assignment.h | 1 + 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index fde17ac..2836059 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -439,6 +439,19 @@ again: static LIST_HEAD(, AssignedDevInfo) adev_head; +void free_assigned_device(AssignedDevInfo *adev) +{ + AssignedDevice *dev = adev->assigned_dev; + + if (dev) { + pci_unregister_device(&dev->dev); + adev->assigned_dev = dev = NULL; + } + + LIST_REMOVE(adev, next); + qemu_free(adev); +} + static uint32_t calc_assigned_dev_id(uint8_t bus, uint8_t devfn) { return (uint32_t)bus << 8 | (uint32_t)devfn; @@ -479,8 +492,7 @@ void assigned_dev_update_irq(PCIDevice *d) adev->name, strerror(-r)); fprintf(stderr, "Perhaps you re you assigning a device " "that shares IRQ with another device?\n"); - LIST_REMOVE(adev, next); - pci_unregister_device(&assigned_dev->dev); + free_assigned_device(adev); adev = next; continue; } diff --git a/qemu/hw/device-assignment.h b/qemu/hw/device-assignment.h index 5a01d98..c8c47d3 100644 --- a/qemu/hw/device-assignment.h +++ b/qemu/hw/device-assignment.h @@ -94,6 +94,7 @@ struct AssignedDevInfo { int disable_iommu; }; +void free_assigned_device(AssignedDevInfo *adev); PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus); AssignedDevInfo *add_assigned_device(const char *arg); void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices); -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails 2008-11-28 17:10 ` [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device() Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd Mark McLoughlin 0 siblings, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-hotplug.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/qemu/hw/device-hotplug.c b/qemu/hw/device-hotplug.c index ba1b161..0bcac60 100644 --- a/qemu/hw/device-hotplug.c +++ b/qemu/hw/device-hotplug.c @@ -48,6 +48,11 @@ static PCIDevice *qemu_system_hot_assign_device(const char *opts, int bus_nr) } ret = init_assigned_device(adev, pci_bus); + if (ret == NULL) { + term_printf("Failed to assign device\n"); + free_assigned_device(adev); + return NULL; + } term_printf("Registered host PCI device %02x:%02x.%1x " "(\"%s\") as guest device %02x:%02x.%1x\n", -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd 2008-11-28 17:10 ` [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions Mark McLoughlin 0 siblings, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index 2836059..5f803b1 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -444,6 +444,11 @@ void free_assigned_device(AssignedDevInfo *adev) AssignedDevice *dev = adev->assigned_dev; if (dev) { + if (dev->real_device.config_fd) { + close(dev->real_device.config_fd); + dev->real_device.config_fd = 0; + } + pci_unregister_device(&dev->dev); adev->assigned_dev = dev = NULL; } -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions 2008-11-28 17:10 ` [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 2008-11-28 17:10 ` [PATCH 12/12] kvm: qemu: device-assignment: init_assigned_device() error handling Mark McLoughlin 0 siblings, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Undo the effects of assigned_dev_register_regions(); made unneccessarily difficult by the twisty data structures. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index 5f803b1..1964fbc 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -330,6 +330,7 @@ static int assigned_dev_register_regions(PCIRegion *io_regions, cur_region->resource_fd, (off_t) 0); if (pci_dev->v_addrs[i].u.r_virtbase == MAP_FAILED) { + pci_dev->v_addrs[i].u.r_virtbase = NULL; fprintf(stderr, "%s: Error: Couldn't mmap 0x%x!" "\n", __func__, (uint32_t) (cur_region->base_addr)); @@ -444,6 +445,25 @@ void free_assigned_device(AssignedDevInfo *adev) AssignedDevice *dev = adev->assigned_dev; if (dev) { + int i; + + for (i = 0; i < dev->real_device.region_number; i++) { + PCIRegion *pci_region = &dev->real_device.regions[i]; + AssignedDevRegion *region = &dev->v_addrs[i]; + + if (!pci_region->valid || !(pci_region->type & IORESOURCE_MEM)) + continue; + + if (region->u.r_virtbase) { + int ret = munmap(region->u.r_virtbase, + (pci_region->size + 0xFFF) & 0xFFFFF000); + if (ret != 0) + fprintf(stderr, + "Failed to unmap assigned device region: %s\n", + strerror(errno)); + } + } + if (dev->real_device.config_fd) { close(dev->real_device.config_fd); dev->real_device.config_fd = 0; -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 12/12] kvm: qemu: device-assignment: init_assigned_device() error handling 2008-11-28 17:10 ` [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions Mark McLoughlin @ 2008-11-28 17:10 ` Mark McLoughlin 0 siblings, 0 replies; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm, Mark McLoughlin Associate the AssignedDevice with the AssignedDevInfo as soon as we have allocated it and don't try and free it in init_assigned_device() if an error occurs. This ensures that the mmio region is unmapped by free_assigned_device() if init_assigned_device() fails. Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 13 +++++-------- 1 files changed, 5 insertions(+), 8 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index 1964fbc..70640c9 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -548,17 +548,19 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) return NULL; } + adev->assigned_dev = dev; + if (get_real_device(dev, adev->bus, adev->dev, adev->func)) { fprintf(stderr, "%s: Error: Couldn't get real device (%s)!\n", __func__, adev->name); - goto out; + return NULL; } /* handle real device's MMIO/PIO BARs */ if (assigned_dev_register_regions(dev->real_device.regions, dev->real_device.region_number, dev)) - goto out; + return NULL; /* handle interrupt routing */ e_device = (dev->dev.devfn >> 3) & 0x1f; @@ -588,15 +590,10 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) if (r < 0) { fprintf(stderr, "Failed to assign device \"%s\" : %s\n", adev->name, strerror(-r)); - goto out; + return NULL; } - adev->assigned_dev = dev; return &dev->dev; - -out: - pci_unregister_device(&dev->dev); - return NULL; } /* -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages 2008-11-28 17:10 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin 2008-11-28 17:10 ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin @ 2008-11-28 19:05 ` John Rousseau 2008-11-30 10:47 ` Avi Kivity 1 sibling, 1 reply; 18+ messages in thread From: John Rousseau @ 2008-11-28 19:05 UTC (permalink / raw) To: Mark McLoughlin; +Cc: Avi Kivity, kvm Mark McLoughlin wrote: > Replace perror() usage with sane error message. > > Signed-off-by: Mark McLoughlin <markmc@redhat.com> > --- > qemu/hw/device-assignment.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c > index 2b2ef68..b39617a 100644 > --- a/qemu/hw/device-assignment.c > +++ b/qemu/hw/device-assignment.c > @@ -473,9 +473,10 @@ void assigned_dev_update_irq(PCIDevice *d) > assigned_irq_data.host_irq = assigned_dev->real_device.irq; > r = kvm_assign_irq(kvm_context, &assigned_irq_data); > if (r < 0) { > - perror("assigned_dev_update_irq"); > - fprintf(stderr, "Are you assigning a device " > - "that shares IRQ with some other device?\n"); > + fprintf(stderr, "Failed to assign irq for \"%s\": %s\n", > + adev->name, strerror(-r)); > + fprintf(stderr, "Perhaps you re you assigning a device " Typo. > + "that shares IRQ with another device?\n"); > pci_unregister_device(&assigned_dev->dev); > /* FIXME: Delete node from list */ > continue; -John ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages 2008-11-28 19:05 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau @ 2008-11-30 10:47 ` Avi Kivity 0 siblings, 0 replies; 18+ messages in thread From: Avi Kivity @ 2008-11-30 10:47 UTC (permalink / raw) To: John Rousseau; +Cc: Mark McLoughlin, kvm John Rousseau wrote: >> - perror("assigned_dev_update_irq"); >> - fprintf(stderr, "Are you assigning a device " >> - "that shares IRQ with some other device?\n"); >> + fprintf(stderr, "Failed to assign irq for \"%s\": >> %s\n", >> + adev->name, strerror(-r)); >> + fprintf(stderr, "Perhaps you re you assigning a >> device " > > Typo. Thanks, updated. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails 2008-11-28 17:10 ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin 2008-11-28 17:10 ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin @ 2008-12-10 10:23 ` Mark McLoughlin 2008-12-10 10:28 ` Avi Kivity 1 sibling, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-12-10 10:23 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm On Fri, 2008-11-28 at 17:10 +0000, Mark McLoughlin wrote: > It's standard practice in qemu to exit if command line parameter > fails, so do that here too. > > Signed-off-by: Mark McLoughlin <markmc@redhat.com> > --- > qemu/hw/device-assignment.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c > index eb2a73a..8fbd66c 100644 > --- a/qemu/hw/device-assignment.c > +++ b/qemu/hw/device-assignment.c > @@ -620,6 +620,7 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices) > if (!adev) { > fprintf(stderr, "Could not add assigned device %s\n", devices[i]); > continue; > + exit(1); Um, that's a rather embarrassing thinko. Cheers, Mark. From: Mark McLoughlin <markmc@redhat.com> Subject: [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Signed-off-by: Mark McLoughlin <markmc@redhat.com> --- qemu/hw/device-assignment.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index 4a38a22..7a66665 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -668,7 +668,6 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices) adev = add_assigned_device(devices[i]); if (!adev) { fprintf(stderr, "Could not add assigned device %s\n", devices[i]); - continue; exit(1); } -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails 2008-12-10 10:23 ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin @ 2008-12-10 10:28 ` Avi Kivity 0 siblings, 0 replies; 18+ messages in thread From: Avi Kivity @ 2008-12-10 10:28 UTC (permalink / raw) To: Mark McLoughlin; +Cc: kvm Mark McLoughlin wrote: >> fprintf(stderr, "Could not add assigned device %s\n", devices[i]); >> continue; >> + exit(1); >> > > Um, that's a rather embarrassing thinko. > For the reviewer as well; applied. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer 2008-11-28 17:10 [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin 2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin @ 2008-11-28 17:14 ` Mark McLoughlin 2008-11-30 10:42 ` Avi Kivity 1 sibling, 1 reply; 18+ messages in thread From: Mark McLoughlin @ 2008-11-28 17:14 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm Hi Avi, Sorry ... forgot to 'git send-email --compose' :-) The patchset is just a bunch of fixes to the device-assignment error handling code. Shouldn't be anything too controversial there. Cheers, Mark. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer 2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin @ 2008-11-30 10:42 ` Avi Kivity 0 siblings, 0 replies; 18+ messages in thread From: Avi Kivity @ 2008-11-30 10:42 UTC (permalink / raw) To: Mark McLoughlin; +Cc: kvm Mark McLoughlin wrote: > Hi Avi, > > Sorry ... forgot to 'git send-email --compose' :-) > > The patchset is just a bunch of fixes to the device-assignment error > handling code. Shouldn't be anything too controversial there. > No, all applied. Patch 2 looks like two patches munged together; I applied it anyway as it wasn't worth the effort to respin. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2008-12-10 10:28 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-11-28 17:10 [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin 2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin 2008-11-28 17:10 ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin 2008-11-28 17:10 ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin 2008-11-28 17:10 ` [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting Mark McLoughlin 2008-11-28 17:10 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin 2008-11-28 17:10 ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin 2008-11-28 17:10 ` [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device() Mark McLoughlin 2008-11-28 17:10 ` [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails Mark McLoughlin 2008-11-28 17:10 ` [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd Mark McLoughlin 2008-11-28 17:10 ` [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions Mark McLoughlin 2008-11-28 17:10 ` [PATCH 12/12] kvm: qemu: device-assignment: init_assigned_device() error handling Mark McLoughlin 2008-11-28 19:05 ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau 2008-11-30 10:47 ` Avi Kivity 2008-12-10 10:23 ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin 2008-12-10 10:28 ` Avi Kivity 2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin 2008-11-30 10:42 ` Avi Kivity
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).