public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes
@ 2011-04-23 10:05 Jan Kiszka
  2011-04-23 10:05 ` [PATCH 1/3] qemu-kvm: pci-assign: Clean up free_assigned_device Jan Kiszka
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Jan Kiszka @ 2011-04-23 10:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

The promised cleanups.



Jan Kiszka (3):
  qemu-kvm: pci-assign: Clean up free_assigned_device
  qemu-kvm: pci-assign: Remove dead code from assigned_dev_iomem_map
  qemu-kvm: pci-assign: Consolidate and fix slow mmio region mappings

 hw/device-assignment.c |  139 ++++++++++++++++++-----------------------------
 1 files changed, 53 insertions(+), 86 deletions(-)


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

* [PATCH 1/3] qemu-kvm: pci-assign: Clean up free_assigned_device
  2011-04-23 10:05 [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Jan Kiszka
@ 2011-04-23 10:05 ` Jan Kiszka
  2011-04-23 10:05 ` [PATCH 2/3] qemu-kvm: pci-assign: Remove dead code from assigned_dev_iomem_map Jan Kiszka
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2011-04-23 10:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

From: Jan Kiszka <jan.kiszka@siemens.com>

The dev argument is never NULL for all free_assigned_device use cases.
So remove the check and one indention level.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   70 +++++++++++++++++++++++------------------------
 1 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 4997b6e..50a7485 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -801,52 +801,50 @@ static void free_dev_irq_entries(AssignedDevice *dev)
 
 static void free_assigned_device(AssignedDevice *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];
+    int i;
 
-            if (!pci_region->valid)
-                continue;
+    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->type & IORESOURCE_IO) {
-                if (pci_region->resource_fd < 0) {
-                    kvm_remove_ioperm_data(region->u.r_baseport,
-                                           region->r_size);
+        if (!pci_region->valid) {
+            continue;
+        }
+        if (pci_region->type & IORESOURCE_IO) {
+            if (pci_region->resource_fd < 0) {
+                kvm_remove_ioperm_data(region->u.r_baseport, region->r_size);
+            }
+        } else if (pci_region->type & IORESOURCE_MEM) {
+            if (region->u.r_virtbase) {
+                if (region->memory_index) {
+                    cpu_register_physical_memory(region->e_physbase,
+                                                 region->e_size,
+                                                 IO_MEM_UNASSIGNED);
+                    qemu_ram_unmap(region->memory_index);
                 }
-            } else if (pci_region->type & IORESOURCE_MEM) {
-                if (region->u.r_virtbase) {
-                    if (region->memory_index) {
-                        cpu_register_physical_memory(region->e_physbase,
-                                                     region->e_size,
-                                                     IO_MEM_UNASSIGNED);
-                        qemu_ram_unmap(region->memory_index);
-                    }
-                    if (munmap(region->u.r_virtbase,
-                               (pci_region->size + 0xFFF) & 0xFFFFF000))
-                        fprintf(stderr,
-				"Failed to unmap assigned device region: %s\n",
-				strerror(errno));
+                if (munmap(region->u.r_virtbase,
+                           (pci_region->size + 0xFFF) & 0xFFFFF000)) {
+                    fprintf(stderr,
+                            "Failed to unmap assigned device region: %s\n",
+                            strerror(errno));
                 }
             }
-            if (pci_region->resource_fd >= 0) {
-                close(pci_region->resource_fd);
-            }
         }
-
-        if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX)
-            assigned_dev_unregister_msix_mmio(dev);
-
-        if (dev->real_device.config_fd >= 0) {
-            close(dev->real_device.config_fd);
+        if (pci_region->resource_fd >= 0) {
+            close(pci_region->resource_fd);
         }
+    }
+
+    if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
+        assigned_dev_unregister_msix_mmio(dev);
+    }
+    if (dev->real_device.config_fd >= 0) {
+        close(dev->real_device.config_fd);
+    }
 
 #ifdef KVM_CAP_IRQ_ROUTING
-        free_dev_irq_entries(dev);
+    free_dev_irq_entries(dev);
 #endif
-    }
 }
 
 static uint32_t calc_assigned_dev_id(uint16_t seg, uint8_t bus, uint8_t devfn)
-- 
1.7.1


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

* [PATCH 2/3] qemu-kvm: pci-assign: Remove dead code from assigned_dev_iomem_map
  2011-04-23 10:05 [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Jan Kiszka
  2011-04-23 10:05 ` [PATCH 1/3] qemu-kvm: pci-assign: Clean up free_assigned_device Jan Kiszka
@ 2011-04-23 10:05 ` Jan Kiszka
  2011-04-23 10:05 ` [PATCH 3/3] qemu-kvm: pci-assign: Consolidate and fix slow mmio region mappings Jan Kiszka
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2011-04-23 10:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

From: Jan Kiszka <jan.kiszka@siemens.com>

ret is only set to 0 and then left untouched.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 50a7485..8a7cfcf 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -273,7 +273,6 @@ static void assigned_dev_iomem_map(PCIDevice *pci_dev, int region_num,
     AssignedDevice *r_dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     AssignedDevRegion *region = &r_dev->v_addrs[region_num];
     PCIRegion *real_region = &r_dev->real_device.regions[region_num];
-    int ret = 0;
 
     DEBUG("e_phys=%08" FMT_PCIBUS " r_virt=%p type=%d len=%08" FMT_PCIBUS " region_num=%d \n",
           e_phys, region->u.r_virtbase, type, e_size, region_num);
@@ -294,11 +293,6 @@ static void assigned_dev_iomem_map(PCIDevice *pci_dev, int region_num,
                     TARGET_PAGE_SIZE, r_dev->mmio_index);
         }
     }
-
-    if (ret != 0) {
-	fprintf(stderr, "%s: Error: create new mapping failed\n", __func__);
-	exit(1);
-    }
 }
 
 static void assigned_dev_ioport_map(PCIDevice *pci_dev, int region_num,
-- 
1.7.1


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

* [PATCH 3/3] qemu-kvm: pci-assign: Consolidate and fix slow mmio region mappings
  2011-04-23 10:05 [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Jan Kiszka
  2011-04-23 10:05 ` [PATCH 1/3] qemu-kvm: pci-assign: Clean up free_assigned_device Jan Kiszka
  2011-04-23 10:05 ` [PATCH 2/3] qemu-kvm: pci-assign: Remove dead code from assigned_dev_iomem_map Jan Kiszka
@ 2011-04-23 10:05 ` Jan Kiszka
  2011-04-25 16:43 ` [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Alex Williamson
  2011-04-27  7:51 ` Avi Kivity
  4 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2011-04-23 10:05 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

From: Jan Kiszka <jan.kiszka@siemens.com>

By registering the io-memory for slow regions already in
assigned_dev_register_regions we can achieve several cleanups and fixes:
 - use assigned_dev_iomem_map for both normal and slow mappings
 - release slow io-region on clean up
 - avoid unregistering zero-size regions (i.e. yet unmapped ones) which
   causes abort() these days

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   65 ++++++++++++++---------------------------------
 1 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 8a7cfcf..606d725 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -242,31 +242,6 @@ static CPUReadMemoryFunc * const slow_bar_read[] = {
     &slow_bar_readl
 };
 
-static void assigned_dev_iomem_map_slow(PCIDevice *pci_dev, int region_num,
-                                        pcibus_t e_phys, pcibus_t e_size,
-                                        int type)
-{
-    AssignedDevice *r_dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
-    AssignedDevRegion *region = &r_dev->v_addrs[region_num];
-    PCIRegion *real_region = &r_dev->real_device.regions[region_num];
-    int m;
-
-    DEBUG("%s", "slow map\n");
-    m = cpu_register_io_memory(slow_bar_read, slow_bar_write, region,
-                               DEVICE_NATIVE_ENDIAN);
-    cpu_register_physical_memory(e_phys, e_size, m);
-
-    /* MSI-X MMIO page */
-    if ((e_size > 0) &&
-        real_region->base_addr <= r_dev->msix_table_addr &&
-        real_region->base_addr + real_region->size >= r_dev->msix_table_addr) {
-        int offset = r_dev->msix_table_addr - real_region->base_addr;
-
-        cpu_register_physical_memory(e_phys + offset,
-                TARGET_PAGE_SIZE, r_dev->mmio_index);
-    }
-}
-
 static void assigned_dev_iomem_map(PCIDevice *pci_dev, int region_num,
                                    pcibus_t e_phys, pcibus_t e_size, int type)
 {
@@ -531,21 +506,10 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
 
         /* handle memory io regions */
         if (cur_region->type & IORESOURCE_MEM) {
-            int slow_map = 0;
             int t = cur_region->type & IORESOURCE_PREFETCH
                 ? PCI_BASE_ADDRESS_MEM_PREFETCH
                 : PCI_BASE_ADDRESS_SPACE_MEMORY;
 
-            if (cur_region->size & 0xFFF) {
-                fprintf(stderr, "PCI region %d at address 0x%llx "
-                        "has size 0x%x, which is not a multiple of 4K. "
-                        "You might experience some performance hit "
-                        "due to that.\n",
-                        i, (unsigned long long)cur_region->base_addr,
-                        cur_region->size);
-                slow_map = 1;
-            }
-
             /* map physical memory */
             pci_dev->v_addrs[i].e_physbase = cur_region->base_addr;
             pci_dev->v_addrs[i].u.r_virtbase = mmap(NULL, cur_region->size,
@@ -569,8 +533,18 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
             pci_dev->v_addrs[i].u.r_virtbase +=
                 (cur_region->base_addr & 0xFFF);
 
-
-            if (!slow_map) {
+            if (cur_region->size & 0xFFF) {
+                fprintf(stderr, "PCI region %d at address 0x%llx "
+                        "has size 0x%x, which is not a multiple of 4K. "
+                        "You might experience some performance hit "
+                        "due to that.\n",
+                        i, (unsigned long long)cur_region->base_addr,
+                        cur_region->size);
+                pci_dev->v_addrs[i].memory_index =
+                    cpu_register_io_memory(slow_bar_read, slow_bar_write,
+                                           &pci_dev->v_addrs[i],
+                                           DEVICE_NATIVE_ENDIAN);
+            } else {
                 void *virtbase = pci_dev->v_addrs[i].u.r_virtbase;
                 char name[32];
                 snprintf(name, sizeof(name), "%s.bar%d",
@@ -580,13 +554,10 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
                                                          &pci_dev->dev.qdev,
                                                          name, cur_region->size,
                                                          virtbase);
-            } else
-                pci_dev->v_addrs[i].memory_index = 0;
+            }
 
-            pci_register_bar((PCIDevice *) pci_dev, i,
-                             cur_region->size, t,
-                             slow_map ? assigned_dev_iomem_map_slow
-                                      : assigned_dev_iomem_map);
+            pci_register_bar((PCIDevice *) pci_dev, i, cur_region->size, t,
+                             assigned_dev_iomem_map);
             continue;
         } else {
             /* handle port io regions */
@@ -810,10 +781,14 @@ static void free_assigned_device(AssignedDevice *dev)
             }
         } else if (pci_region->type & IORESOURCE_MEM) {
             if (region->u.r_virtbase) {
-                if (region->memory_index) {
+                if (region->e_size > 0) {
                     cpu_register_physical_memory(region->e_physbase,
                                                  region->e_size,
                                                  IO_MEM_UNASSIGNED);
+                }
+                if (region->r_size & 0xFFF) {
+                    cpu_unregister_io_memory(region->memory_index);
+                } else {
                     qemu_ram_unmap(region->memory_index);
                 }
                 if (munmap(region->u.r_virtbase,
-- 
1.7.1


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

* Re: [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes
  2011-04-23 10:05 [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Jan Kiszka
                   ` (2 preceding siblings ...)
  2011-04-23 10:05 ` [PATCH 3/3] qemu-kvm: pci-assign: Consolidate and fix slow mmio region mappings Jan Kiszka
@ 2011-04-25 16:43 ` Alex Williamson
  2011-04-27  7:51 ` Avi Kivity
  4 siblings, 0 replies; 6+ messages in thread
From: Alex Williamson @ 2011-04-25 16:43 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On Sat, 2011-04-23 at 12:05 +0200, Jan Kiszka wrote:
> The promised cleanups.
> 
> 
> 
> Jan Kiszka (3):
>   qemu-kvm: pci-assign: Clean up free_assigned_device
>   qemu-kvm: pci-assign: Remove dead code from assigned_dev_iomem_map
>   qemu-kvm: pci-assign: Consolidate and fix slow mmio region mappings
> 
>  hw/device-assignment.c |  139 ++++++++++++++++++-----------------------------
>  1 files changed, 53 insertions(+), 86 deletions(-)
> 

Looks good to me.

Acked-by: Alex Williamson <alex.williamson@redhat.com>


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

* Re: [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes
  2011-04-23 10:05 [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Jan Kiszka
                   ` (3 preceding siblings ...)
  2011-04-25 16:43 ` [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Alex Williamson
@ 2011-04-27  7:51 ` Avi Kivity
  4 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2011-04-27  7:51 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm, Alex Williamson

On 04/23/2011 01:05 PM, Jan Kiszka wrote:
> The promised cleanups.
>
>
>
> Jan Kiszka (3):
>    qemu-kvm: pci-assign: Clean up free_assigned_device
>    qemu-kvm: pci-assign: Remove dead code from assigned_dev_iomem_map
>    qemu-kvm: pci-assign: Consolidate and fix slow mmio region mappings

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function


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

end of thread, other threads:[~2011-04-27  7:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-23 10:05 [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Jan Kiszka
2011-04-23 10:05 ` [PATCH 1/3] qemu-kvm: pci-assign: Clean up free_assigned_device Jan Kiszka
2011-04-23 10:05 ` [PATCH 2/3] qemu-kvm: pci-assign: Remove dead code from assigned_dev_iomem_map Jan Kiszka
2011-04-23 10:05 ` [PATCH 3/3] qemu-kvm: pci-assign: Consolidate and fix slow mmio region mappings Jan Kiszka
2011-04-25 16:43 ` [PATCH 0/3] qemu-kvm: pci-assign: Mapping fixes Alex Williamson
2011-04-27  7:51 ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox