[Qemu-devel] [PATCH 0/2] Unbreak aliases of I/O regions (and -vga std)

[Qemu-devel] [PATCH 0/2] Unbreak aliases of I/O regions (and -vga std)

[Avi Kivity]

Aliases to memory regions in the I/O address space are broken; this affects
-vga std.  These two patches fix the problem.

Also available in

  git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/urgent

Avi Kivity (2):
  ioport: add destructor method to IORange
  memory: fix I/O port aliases

 exec.c    |    8 ++++++--
 ioport.c  |   15 +++++++++++++++
 ioport.h  |    1 +
 iorange.h |    1 +
 memory.c  |   26 ++++++++++++++++++++------
 memory.h  |    9 ++++++++-
 6 files changed, 51 insertions(+), 9 deletions(-)

-- 
1.7.9

[Qemu-devel] [PATCH 1/2] ioport: add destructor method to IORange

[Avi Kivity]

Previously all callers had a containing object with a destructor that
could be used to trigger cleanup of the IORange objects (typically
just freeing the containing object), but a forthcoming memory API
change doesn't fit this pattern.  Rather than setting up a new global
table, extend the ioport system to support destructors.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 ioport.c  |   15 +++++++++++++++
 ioport.h  |    1 +
 iorange.h |    1 +
 3 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/ioport.c b/ioport.c
index 8a474d3..78a3b89 100644
--- a/ioport.c
+++ b/ioport.c
@@ -52,6 +52,7 @@
 static void *ioport_opaque[MAX_IOPORTS];
 static IOPortReadFunc *ioport_read_table[3][MAX_IOPORTS];
 static IOPortWriteFunc *ioport_write_table[3][MAX_IOPORTS];
+static IOPortDestructor *ioport_destructor_table[MAX_IOPORTS];
 
 static IOPortReadFunc default_ioport_readb, default_ioport_readw, default_ioport_readl;
 static IOPortWriteFunc default_ioport_writeb, default_ioport_writew, default_ioport_writel;
@@ -225,6 +226,15 @@ static void ioport_writel_thunk(void *opaque, uint32_t addr, uint32_t data)
     ioport->ops->write(ioport, addr - ioport->base, 4, data);
 }
 
+static void iorange_destructor_thunk(void *opaque)
+{
+    IORange *iorange = opaque;
+
+    if (iorange->ops->destructor) {
+        iorange->ops->destructor(iorange);
+    }
+}
+
 void ioport_register(IORange *ioport)
 {
     register_ioport_read(ioport->base, ioport->len, 1,
@@ -239,12 +249,17 @@ void ioport_register(IORange *ioport)
                           ioport_writew_thunk, ioport);
     register_ioport_write(ioport->base, ioport->len, 4,
                           ioport_writel_thunk, ioport);
+    ioport_destructor_table[ioport->base] = iorange_destructor_thunk;
 }
 
 void isa_unassign_ioport(pio_addr_t start, int length)
 {
     int i;
 
+    if (ioport_destructor_table[start]) {
+        ioport_destructor_table[start](ioport_opaque[start]);
+        ioport_destructor_table[start] = NULL;
+    }
     for(i = start; i < start + length; i++) {
         ioport_read_table[0][i] = NULL;
         ioport_read_table[1][i] = NULL;
diff --git a/ioport.h b/ioport.h
index ab29c89..23441cb 100644
--- a/ioport.h
+++ b/ioport.h
@@ -36,6 +36,7 @@ typedef uint32_t pio_addr_t;
 /* These should really be in isa.h, but are here to make pc.h happy.  */
 typedef void (IOPortWriteFunc)(void *opaque, uint32_t address, uint32_t data);
 typedef uint32_t (IOPortReadFunc)(void *opaque, uint32_t address);
+typedef void (IOPortDestructor)(void *opaque);
 
 void ioport_register(IORange *iorange);
 int register_ioport_read(pio_addr_t start, int length, int size,
diff --git a/iorange.h b/iorange.h
index 9783168..cd980a8 100644
--- a/iorange.h
+++ b/iorange.h
@@ -11,6 +11,7 @@ struct IORangeOps {
                  uint64_t *data);
     void (*write)(IORange *iorange, uint64_t offset, unsigned width,
                   uint64_t data);
+    void (*destructor)(IORange *iorange);
 };
 
 struct IORange {
-- 
1.7.9

[Qemu-devel] [PATCH 2/2] memory: fix I/O port aliases

[Avi Kivity]

Commit e58ac72b6a0 ("ioport: change portio_list not to use
memory_region_set_offset()") started using aliases of I/O memory
regions.  Since the IORange used for the I/O was contained in the
target region, the alias information (specifically, the offset
into the region) was lost.  This broke -vga std.

Fix by allocating an independent object to hold the IORange and
also the new offset.

Note that I/O memory regions were conceptually broken wrt aliases
in a different way: an alias can cause the same region to appear
twice in an address space, but we had just one IORange to service it.
This patch fixes that problem as well, since we can now have multiple
IORange/MemoryRegion associations.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec.c   |    8 ++++++--
 memory.c |   26 ++++++++++++++++++++------
 memory.h |    9 ++++++++-
 3 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/exec.c b/exec.c
index 3ce3539..1e5bbd6 100644
--- a/exec.c
+++ b/exec.c
@@ -3668,9 +3668,13 @@ static void io_commit(MemoryListener *listener)
 static void io_region_add(MemoryListener *listener,
                           MemoryRegionSection *section)
 {
-    iorange_init(&section->mr->iorange, &memory_region_iorange_ops,
+    MemoryRegionIORange *mrio = g_new(MemoryRegionIORange, 1);
+
+    mrio->mr = section->mr;
+    mrio->offset = section->offset_within_region;
+    iorange_init(&mrio->iorange, &memory_region_iorange_ops,
                  section->offset_within_address_space, section->size);
-    ioport_register(&section->mr->iorange);
+    ioport_register(&mrio->iorange);
 }
 
 static void io_region_del(MemoryListener *listener,
diff --git a/memory.c b/memory.c
index 6565e2e..4c3dc49 100644
--- a/memory.c
+++ b/memory.c
@@ -382,16 +382,20 @@ static void memory_region_iorange_read(IORange *iorange,
                                        unsigned width,
                                        uint64_t *data)
 {
-    MemoryRegion *mr = container_of(iorange, MemoryRegion, iorange);
+    MemoryRegionIORange *mrio
+        = container_of(iorange, MemoryRegionIORange, iorange);
+    MemoryRegion *mr = mrio->mr;
 
+    offset += mrio->offset;
     if (mr->ops->old_portio) {
-        const MemoryRegionPortio *mrp = find_portio(mr, offset, width, false);
+        const MemoryRegionPortio *mrp = find_portio(mr, offset - mrio->offset,
+                                                    width, false);
 
         *data = ((uint64_t)1 << (width * 8)) - 1;
         if (mrp) {
             *data = mrp->read(mr->opaque, offset);
         } else if (width == 2) {
-            mrp = find_portio(mr, offset, 1, false);
+            mrp = find_portio(mr, offset - mrio->offset, 1, false);
             assert(mrp);
             *data = mrp->read(mr->opaque, offset) |
                     (mrp->read(mr->opaque, offset + 1) << 8);
@@ -410,15 +414,19 @@ static void memory_region_iorange_write(IORange *iorange,
                                         unsigned width,
                                         uint64_t data)
 {
-    MemoryRegion *mr = container_of(iorange, MemoryRegion, iorange);
+    MemoryRegionIORange *mrio
+        = container_of(iorange, MemoryRegionIORange, iorange);
+    MemoryRegion *mr = mrio->mr;
 
+    offset += mrio->offset;
     if (mr->ops->old_portio) {
-        const MemoryRegionPortio *mrp = find_portio(mr, offset, width, true);
+        const MemoryRegionPortio *mrp = find_portio(mr, offset - mrio->offset,
+                                                    width, true);
 
         if (mrp) {
             mrp->write(mr->opaque, offset, data);
         } else if (width == 2) {
-            mrp = find_portio(mr, offset, 1, false);
+            mrp = find_portio(mr, offset - mrio->offset, 1, false);
             assert(mrp);
             mrp->write(mr->opaque, offset, data & 0xff);
             mrp->write(mr->opaque, offset + 1, data >> 8);
@@ -431,9 +439,15 @@ static void memory_region_iorange_write(IORange *iorange,
                               memory_region_write_accessor, mr);
 }
 
+static void memory_region_iorange_destructor(IORange *iorange)
+{
+    g_free(container_of(iorange, MemoryRegionIORange, iorange));
+}
+
 const IORangeOps memory_region_iorange_ops = {
     .read = memory_region_iorange_read,
     .write = memory_region_iorange_write,
+    .destructor = memory_region_iorange_destructor,
 };
 
 static AddressSpace address_space_io;
diff --git a/memory.h b/memory.h
index b7bccd1..53ff62b 100644
--- a/memory.h
+++ b/memory.h
@@ -43,6 +43,14 @@ struct MemoryRegionMmio {
     CPUWriteMemoryFunc *write[3];
 };
 
+/* Internal use; thunks between old-style IORange and MemoryRegions. */
+typedef struct MemoryRegionIORange MemoryRegionIORange;
+struct MemoryRegionIORange {
+    IORange iorange;
+    MemoryRegion *mr;
+    target_phys_addr_t offset;
+};
+
 /*
  * Memory region callbacks
  */
@@ -117,7 +125,6 @@ struct MemoryRegion {
     target_phys_addr_t addr;
     void (*destructor)(MemoryRegion *mr);
     ram_addr_t ram_addr;
-    IORange iorange;
     bool subpage;
     bool terminates;
     bool readable;
-- 
1.7.9

Re: [Qemu-devel] [PATCH 0/2] Unbreak aliases of I/O regions (and -vga std)

[Blue Swirl]

On Mon, Mar 5, 2012 at 15:51, Avi Kivity <avi@redhat.com> wrote:
> Aliases to memory regions in the I/O address space are broken; this affects
> -vga std.  These two patches fix the problem.
>
> Also available in
>
>  git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/urgent

Thanks, pulled.

> Avi Kivity (2):
>  ioport: add destructor method to IORange
>  memory: fix I/O port aliases
>
>  exec.c    |    8 ++++++--
>  ioport.c  |   15 +++++++++++++++
>  ioport.h  |    1 +
>  iorange.h |    1 +
>  memory.c  |   26 ++++++++++++++++++++------
>  memory.h  |    9 ++++++++-
>  6 files changed, 51 insertions(+), 9 deletions(-)
>
> --
> 1.7.9
>
>