[Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Hervé Poussineau]

These proposed patches aim at removing the .old_portio member of
MemoryRegionOps structure, and replacing their usage by .read/.write
handlers.

That way, faked I/O address space can be removed from architectures
which don't have it (MIPS, PowerPC...), and commits like
a178274efabcbbc5d44805b51def874e47051325 ("PPC: pseries: Remove hack
for PIO window") can be reapplied.

The first 4 patches do the cleanup and remove the old_portio handler
in MemoryRegion structure.
The last 4 patches are simplifications, now that Portio handlers
can be called from memory core without limitation.

Changes since v1:
- handling of Portio has been moved to ioport.c (instead of isa-bus.c)
  This prevents creating an ISA bus on machines which don't have one
- added last 4 patches to see benefits of removing old_portio

Alexander Graf (1):
  PPC: pseries: Remove hack for PIO window

Hervé Poussineau (7):
  isa: fix documentation of isa_register_portio_list
  memory: handle old_portio accesses in MMIO path
  ioport: register memory regions for I/O port lists
  memory: remove code dealing with old_portio
  ioport: reimplement cpu_in/cpu_out using address_space_rw
  ppc: simplify access to PReP I/O region
  isa_mmio: simplify access to system I/O region

 hw/isa/isa_mmio.c           |   42 +++------
 hw/ppc/prep.c               |   65 ++-----------
 hw/ppc/spapr_pci.c          |   44 +--------
 include/exec/ioport.h       |    3 +-
 include/exec/memory.h       |    4 -
 include/hw/isa/isa.h        |    2 +-
 include/hw/pci-host/spapr.h |    2 +-
 ioport.c                    |  215 ++++++++++++++++++++++---------------------
 memory.c                    |   44 ---------
 9 files changed, 137 insertions(+), 284 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v2 1/8] isa: fix documentation of isa_register_portio_list

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 include/hw/isa/isa.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 82da37c..556790c 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -73,7 +73,7 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start);
  * @dev: the ISADevice against which these are registered; may be NULL.
  * @start: the base I/O port against which the portio->offset is applied.
  * @portio: the ports, sorted by offset.
- * @opaque: passed into the old_portio callbacks.
+ * @opaque: passed into the portio callbacks.
  * @name: passed into memory_region_init_io.
  */
 void isa_register_portio_list(ISADevice *dev, uint16_t start,
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 2/8] memory: handle old_portio accesses in MMIO path

[Hervé Poussineau]

This patch is only temporary, and will be removed soon.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 memory.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/memory.c b/memory.c
index 5cb8f4a..f27167c 100644
--- a/memory.c
+++ b/memory.c
@@ -933,6 +933,14 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
                                   mr->ops->impl.min_access_size,
                                   mr->ops->impl.max_access_size,
                                   memory_region_read_accessor, mr);
+    } else if (mr->ops->old_portio) {
+        MemoryRegionIORange mrio;
+        MemoryRegionSection mrs = memory_region_find(get_system_io(), addr,
+                                                     size);
+        mrio.mr = mr;
+        mrio.offset = mrs.offset_within_region;
+        memory_region_iorange_read(&mrio.iorange, addr - mrio.offset, size,
+                                   &data);
     } else {
         access_with_adjusted_size(addr, &data, size, 1, 4,
                                   memory_region_oldmmio_read_accessor, mr);
@@ -994,6 +1002,14 @@ static bool memory_region_dispatch_write(MemoryRegion *mr,
                                   mr->ops->impl.min_access_size,
                                   mr->ops->impl.max_access_size,
                                   memory_region_write_accessor, mr);
+    } else if (mr->ops->old_portio) {
+        MemoryRegionIORange mrio;
+        MemoryRegionSection mrs = memory_region_find(get_system_io(), addr,
+                                                     size);
+        mrio.mr = mr;
+        mrio.offset = mrs.offset_within_region;
+        memory_region_iorange_write(&mrio.iorange, addr - mrio.offset, size,
+                                    data);
     } else {
         access_with_adjusted_size(addr, &data, size, 1, 4,
                                   memory_region_oldmmio_write_accessor, mr);
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 3/8] ioport: register memory regions for I/O port lists

[Hervé Poussineau]

As I/O ports are now memory regions, they can be seen in 'info qtree'.
This also removes the last use of MemoryRegionOps.old_portio field.

Some of the code has been extracted from memory.c

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 include/exec/ioport.h |    3 +-
 ioport.c              |  132 +++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 107 insertions(+), 28 deletions(-)

diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index fc28350..398fe14 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -55,13 +55,14 @@ uint32_t cpu_inl(pio_addr_t addr);
 
 struct MemoryRegion;
 struct MemoryRegionPortio;
+struct PortioListState;
 
 typedef struct PortioList {
     const struct MemoryRegionPortio *ports;
     struct MemoryRegion *address_space;
     unsigned nr;
     struct MemoryRegion **regions;
-    struct MemoryRegion **aliases;
+    struct PortioListState **states;
     void *opaque;
     const char *name;
 } PortioList;
diff --git a/ioport.c b/ioport.c
index a0ac2a0..d26d3e1 100644
--- a/ioport.c
+++ b/ioport.c
@@ -28,6 +28,7 @@
 #include "exec/ioport.h"
 #include "trace.h"
 #include "exec/memory.h"
+#include "qemu/host-utils.h"
 
 /***********************************************************/
 /* IO Port */
@@ -330,6 +331,12 @@ uint32_t cpu_inl(pio_addr_t addr)
     return val;
 }
 
+typedef struct PortioListState {
+    const MemoryRegionPortio *portio;
+    uint32_t offset;
+    void *opaque;
+} PortioListState;
+
 void portio_list_init(PortioList *piolist,
                       const MemoryRegionPortio *callbacks,
                       void *opaque, const char *name)
@@ -343,16 +350,95 @@ void portio_list_init(PortioList *piolist,
     piolist->ports = callbacks;
     piolist->nr = 0;
     piolist->regions = g_new0(MemoryRegion *, n);
-    piolist->aliases = g_new0(MemoryRegion *, n);
+    piolist->states = g_new0(PortioListState *, n);
     piolist->address_space = NULL;
     piolist->opaque = opaque;
     piolist->name = name;
 }
 
+static const MemoryRegionPortio *portio_find(const MemoryRegionPortio *mrp,
+                                             uint64_t offset,
+                                             unsigned int width, bool write)
+{
+    for (; mrp->size; ++mrp) {
+        if (offset >= mrp->offset && offset < mrp->offset + mrp->len
+            && width == mrp->size
+            && (write ? (bool)mrp->write : (bool)mrp->read)) {
+            return mrp;
+        }
+    }
+    return NULL;
+}
+
+static uint64_t portio_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    const PortioListState *s = opaque;
+    const MemoryRegionPortio *mrp;
+    uint16_t data;
+
+    addr += s->portio->offset;
+    mrp = portio_find(s->portio, addr, size, false);
+    if (!mrp && size == 2) {
+        mrp = portio_find(s->portio, addr, 1, false);
+        assert(mrp);
+        data = mrp->read(s->opaque, addr) |
+              (mrp->read(s->opaque, addr + 1) << 8);
+        return data;
+    }
+    assert(mrp);
+    return mrp->read(s->opaque, addr);
+}
+
+static void portio_write(void *opaque, hwaddr addr, uint64_t data,
+                         unsigned int size)
+{
+    const PortioListState *s = opaque;
+    const MemoryRegionPortio *mrp;
+
+    addr += s->offset;
+    mrp = portio_find(s->portio, addr, size, true);
+    if (!mrp && size == 2) {
+        mrp = portio_find(s->portio, addr, 1, true);
+        assert(mrp);
+        mrp->write(s->opaque, addr, data & 0xff);
+        mrp->write(s->opaque, addr + 1, data >> 8);
+        return;
+    }
+    assert(mrp);
+    mrp->write(s->opaque, addr, data);
+}
+
+static bool portio_accepts(void *opaque, hwaddr addr, unsigned int size,
+                           bool is_write)
+{
+    const PortioListState *s = opaque;
+    const MemoryRegionPortio *mrp;
+
+    addr += s->offset;
+    mrp = portio_find(s->portio, addr, size, is_write);
+    if (!mrp && size == 2) {
+        mrp = portio_find(s->portio, addr, 1, is_write);
+    }
+    if (!mrp) {
+        LOG_UNUSED_IOPORT("unused %s%c: port=0x%04" HWADDR_PRIx "\n",
+                           is_write ? "out" : "in",
+                           size == 1 ? 'b' : size == 2 ? 'w' : 'l',
+                           addr);
+    }
+    return mrp;
+}
+
+const MemoryRegionOps portio_ops = {
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .read = portio_read,
+    .write = portio_write,
+    .valid.accepts = portio_accepts,
+};
+
 void portio_list_destroy(PortioList *piolist)
 {
     g_free(piolist->regions);
-    g_free(piolist->aliases);
+    g_free(piolist->states);
 }
 
 static void portio_list_add_1(PortioList *piolist,
@@ -361,8 +447,8 @@ static void portio_list_add_1(PortioList *piolist,
                               unsigned off_low, unsigned off_high)
 {
     MemoryRegionPortio *pio;
-    MemoryRegionOps *ops;
-    MemoryRegion *region, *alias;
+    MemoryRegion *region = g_new(MemoryRegion, 1);
+    PortioListState *s = g_new(PortioListState, 1);
     unsigned i;
 
     /* Copy the sub-list and null-terminate it.  */
@@ -370,28 +456,20 @@ static void portio_list_add_1(PortioList *piolist,
     memcpy(pio, pio_init, sizeof(MemoryRegionPortio) * count);
     memset(pio + count, 0, sizeof(MemoryRegionPortio));
 
-    /* Adjust the offsets to all be zero-based for the region.  */
+    /* Adjust the offsets to all be address-based for the region. */
     for (i = 0; i < count; ++i) {
-        pio[i].offset -= off_low;
+        pio[i].offset += start;
     }
 
-    ops = g_new0(MemoryRegionOps, 1);
-    ops->old_portio = pio;
-
-    region = g_new(MemoryRegion, 1);
-    alias = g_new(MemoryRegion, 1);
-    /*
-     * Use an alias so that the callback is called with an absolute address,
-     * rather than an offset relative to to start + off_low.
-     */
-    memory_region_init_io(region, ops, piolist->opaque, piolist->name,
-                          INT64_MAX);
-    memory_region_init_alias(alias, piolist->name,
-                             region, start + off_low, off_high - off_low);
+    s->offset = start + off_low;
+    s->portio = pio;
+    s->opaque = piolist->opaque;
+    memory_region_init_io(region, &portio_ops, s, piolist->name,
+                          off_high - off_low);
     memory_region_add_subregion(piolist->address_space,
-                                start + off_low, alias);
+                                s->offset, region);
     piolist->regions[piolist->nr] = region;
-    piolist->aliases[piolist->nr] = alias;
+    piolist->states[piolist->nr] = s;
     ++piolist->nr;
 }
 
@@ -434,19 +512,19 @@ void portio_list_add(PortioList *piolist,
 
 void portio_list_del(PortioList *piolist)
 {
-    MemoryRegion *mr, *alias;
+    MemoryRegion *mr;
+    PortioListState *s;
     unsigned i;
 
     for (i = 0; i < piolist->nr; ++i) {
         mr = piolist->regions[i];
-        alias = piolist->aliases[i];
-        memory_region_del_subregion(piolist->address_space, alias);
-        memory_region_destroy(alias);
+        s = piolist->states[i];
+        memory_region_del_subregion(piolist->address_space, mr);
         memory_region_destroy(mr);
         g_free((MemoryRegionOps *)mr->ops);
         g_free(mr);
-        g_free(alias);
+        g_free(s);
         piolist->regions[i] = NULL;
-        piolist->aliases[i] = NULL;
+        piolist->states[i] = NULL;
     }
 }
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 4/8] memory: remove code dealing with old_portio

[Hervé Poussineau]

Last user of old_portio callbacks has been removed in previous commit.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 include/exec/memory.h |    4 ----
 memory.c              |   60 -------------------------------------------------
 2 files changed, 64 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index d53a6a1..3c225c9 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -105,10 +105,6 @@ struct MemoryRegionOps {
          bool unaligned;
     } impl;
 
-    /* If .read and .write are not present, old_portio may be used for
-     * backwards compatibility with old portio registration
-     */
-    const MemoryRegionPortio *old_portio;
     /* If .read and .write are not present, old_mmio may be used for
      * backwards compatibility with old mmio registration
      */
diff --git a/memory.c b/memory.c
index f27167c..8ac9e66 100644
--- a/memory.c
+++ b/memory.c
@@ -401,21 +401,6 @@ static void access_with_adjusted_size(hwaddr addr,
     }
 }
 
-static const MemoryRegionPortio *find_portio(MemoryRegion *mr, uint64_t offset,
-                                             unsigned width, bool write)
-{
-    const MemoryRegionPortio *mrp;
-
-    for (mrp = mr->ops->old_portio; mrp->size; ++mrp) {
-        if (offset >= mrp->offset && offset < mrp->offset + mrp->len
-            && width == mrp->size
-            && (write ? (bool)mrp->write : (bool)mrp->read)) {
-            return mrp;
-        }
-    }
-    return NULL;
-}
-
 static void memory_region_iorange_read(IORange *iorange,
                                        uint64_t offset,
                                        unsigned width,
@@ -426,21 +411,6 @@ static void memory_region_iorange_read(IORange *iorange,
     MemoryRegion *mr = mrio->mr;
 
     offset += mrio->offset;
-    if (mr->ops->old_portio) {
-        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 - mrio->offset, 1, false);
-            assert(mrp);
-            *data = mrp->read(mr->opaque, offset) |
-                    (mrp->read(mr->opaque, offset + 1) << 8);
-        }
-        return;
-    }
     *data = 0;
     access_with_adjusted_size(offset, data, width,
                               mr->ops->impl.min_access_size,
@@ -458,20 +428,6 @@ static void memory_region_iorange_write(IORange *iorange,
     MemoryRegion *mr = mrio->mr;
 
     offset += mrio->offset;
-    if (mr->ops->old_portio) {
-        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 - mrio->offset, 1, true);
-            assert(mrp);
-            mrp->write(mr->opaque, offset, data & 0xff);
-            mrp->write(mr->opaque, offset + 1, data >> 8);
-        }
-        return;
-    }
     access_with_adjusted_size(offset, &data, width,
                               mr->ops->impl.min_access_size,
                               mr->ops->impl.max_access_size,
@@ -933,14 +889,6 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
                                   mr->ops->impl.min_access_size,
                                   mr->ops->impl.max_access_size,
                                   memory_region_read_accessor, mr);
-    } else if (mr->ops->old_portio) {
-        MemoryRegionIORange mrio;
-        MemoryRegionSection mrs = memory_region_find(get_system_io(), addr,
-                                                     size);
-        mrio.mr = mr;
-        mrio.offset = mrs.offset_within_region;
-        memory_region_iorange_read(&mrio.iorange, addr - mrio.offset, size,
-                                   &data);
     } else {
         access_with_adjusted_size(addr, &data, size, 1, 4,
                                   memory_region_oldmmio_read_accessor, mr);
@@ -1002,14 +950,6 @@ static bool memory_region_dispatch_write(MemoryRegion *mr,
                                   mr->ops->impl.min_access_size,
                                   mr->ops->impl.max_access_size,
                                   memory_region_write_accessor, mr);
-    } else if (mr->ops->old_portio) {
-        MemoryRegionIORange mrio;
-        MemoryRegionSection mrs = memory_region_find(get_system_io(), addr,
-                                                     size);
-        mrio.mr = mr;
-        mrio.offset = mrs.offset_within_region;
-        memory_region_iorange_write(&mrio.iorange, addr - mrio.offset, size,
-                                    data);
     } else {
         access_with_adjusted_size(addr, &data, size, 1, 4,
                                   memory_region_oldmmio_write_accessor, mr);
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 5/8] ioport: reimplement cpu_in/cpu_out using address_space_rw

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 ioport.c |   83 ++++++--------------------------------------------------------
 1 file changed, 7 insertions(+), 76 deletions(-)

diff --git a/ioport.c b/ioport.c
index d26d3e1..7bc9d8f 100644
--- a/ioport.c
+++ b/ioport.c
@@ -28,6 +28,7 @@
 #include "exec/ioport.h"
 #include "trace.h"
 #include "exec/memory.h"
+#include "exec/address-spaces.h"
 #include "qemu/host-utils.h"
 
 /***********************************************************/
@@ -55,76 +56,6 @@ 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;
-
-static uint32_t ioport_read(int index, uint32_t address)
-{
-    static IOPortReadFunc * const default_func[3] = {
-        default_ioport_readb,
-        default_ioport_readw,
-        default_ioport_readl
-    };
-    IOPortReadFunc *func = ioport_read_table[index][address];
-    if (!func)
-        func = default_func[index];
-    return func(ioport_opaque[address], address);
-}
-
-static void ioport_write(int index, uint32_t address, uint32_t data)
-{
-    static IOPortWriteFunc * const default_func[3] = {
-        default_ioport_writeb,
-        default_ioport_writew,
-        default_ioport_writel
-    };
-    IOPortWriteFunc *func = ioport_write_table[index][address];
-    if (!func)
-        func = default_func[index];
-    func(ioport_opaque[address], address, data);
-}
-
-static uint32_t default_ioport_readb(void *opaque, uint32_t address)
-{
-    LOG_UNUSED_IOPORT("unused inb: port=0x%04"PRIx32"\n", address);
-    return 0xff;
-}
-
-static void default_ioport_writeb(void *opaque, uint32_t address, uint32_t data)
-{
-    LOG_UNUSED_IOPORT("unused outb: port=0x%04"PRIx32" data=0x%02"PRIx32"\n",
-                      address, data);
-}
-
-/* default is to make two byte accesses */
-static uint32_t default_ioport_readw(void *opaque, uint32_t address)
-{
-    uint32_t data;
-    data = ioport_read(0, address);
-    address = (address + 1) & IOPORTS_MASK;
-    data |= ioport_read(0, address) << 8;
-    return data;
-}
-
-static void default_ioport_writew(void *opaque, uint32_t address, uint32_t data)
-{
-    ioport_write(0, address, data & 0xff);
-    address = (address + 1) & IOPORTS_MASK;
-    ioport_write(0, address, (data >> 8) & 0xff);
-}
-
-static uint32_t default_ioport_readl(void *opaque, uint32_t address)
-{
-    LOG_UNUSED_IOPORT("unused inl: port=0x%04"PRIx32"\n", address);
-    return 0xffffffff;
-}
-
-static void default_ioport_writel(void *opaque, uint32_t address, uint32_t data)
-{
-    LOG_UNUSED_IOPORT("unused outl: port=0x%04"PRIx32" data=0x%02"PRIx32"\n",
-                      address, data);
-}
-
 static int ioport_bsize(int size, int *bsize)
 {
     if (size == 1) {
@@ -287,27 +218,27 @@ void cpu_outb(pio_addr_t addr, uint8_t val)
 {
     LOG_IOPORT("outb: %04"FMT_pioaddr" %02"PRIx8"\n", addr, val);
     trace_cpu_out(addr, val);
-    ioport_write(0, addr, val);
+    address_space_write(&address_space_io, addr, &val, 1);
 }
 
 void cpu_outw(pio_addr_t addr, uint16_t val)
 {
     LOG_IOPORT("outw: %04"FMT_pioaddr" %04"PRIx16"\n", addr, val);
     trace_cpu_out(addr, val);
-    ioport_write(1, addr, val);
+    address_space_write(&address_space_io, addr, (uint8_t *)&val, 2);
 }
 
 void cpu_outl(pio_addr_t addr, uint32_t val)
 {
     LOG_IOPORT("outl: %04"FMT_pioaddr" %08"PRIx32"\n", addr, val);
     trace_cpu_out(addr, val);
-    ioport_write(2, addr, val);
+    address_space_write(&address_space_io, addr, (uint8_t *)&val, 4);
 }
 
 uint8_t cpu_inb(pio_addr_t addr)
 {
     uint8_t val;
-    val = ioport_read(0, addr);
+    address_space_read(&address_space_io, addr, &val, 1);
     trace_cpu_in(addr, val);
     LOG_IOPORT("inb : %04"FMT_pioaddr" %02"PRIx8"\n", addr, val);
     return val;
@@ -316,7 +247,7 @@ uint8_t cpu_inb(pio_addr_t addr)
 uint16_t cpu_inw(pio_addr_t addr)
 {
     uint16_t val;
-    val = ioport_read(1, addr);
+    address_space_read(&address_space_io, addr, (uint8_t *)&val, 2);
     trace_cpu_in(addr, val);
     LOG_IOPORT("inw : %04"FMT_pioaddr" %04"PRIx16"\n", addr, val);
     return val;
@@ -325,7 +256,7 @@ uint16_t cpu_inw(pio_addr_t addr)
 uint32_t cpu_inl(pio_addr_t addr)
 {
     uint32_t val;
-    val = ioport_read(2, addr);
+    address_space_read(&address_space_io, addr, (uint8_t *)&val, 4);
     trace_cpu_in(addr, val);
     LOG_IOPORT("inl : %04"FMT_pioaddr" %08"PRIx32"\n", addr, val);
     return val;
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 6/8] ppc: simplify access to PReP I/O region

[Hervé Poussineau]

We can now simply use address_space_rw instead of the more convoluted
cpu_in() and cpu_out() functions.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/ppc/prep.c |   65 ++++++++-------------------------------------------------
 1 file changed, 9 insertions(+), 56 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index be8a50e..bb77e1f 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -341,75 +341,28 @@ static inline hwaddr prep_IO_address(sysctrl_t *sysctrl,
     return addr;
 }
 
-static void PPC_prep_io_writeb (void *opaque, hwaddr addr,
-                                uint32_t value)
+static uint64_t PPC_prep_io_read(void *opaque, hwaddr addr, unsigned int size)
 {
     sysctrl_t *sysctrl = opaque;
+    uint64_t data = 0;
 
     addr = prep_IO_address(sysctrl, addr);
-    cpu_outb(addr, value);
+    address_space_read(&address_space_io, addr, (uint8_t *)&data, size);
+    return data;
 }
 
-static uint32_t PPC_prep_io_readb (void *opaque, hwaddr addr)
+static void PPC_prep_io_write(void *opaque, hwaddr addr, uint64_t data,
+                              unsigned int size)
 {
     sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
 
     addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inb(addr);
-
-    return ret;
-}
-
-static void PPC_prep_io_writew (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " => 0x%08" PRIx32 "\n", addr, value);
-    cpu_outw(addr, value);
-}
-
-static uint32_t PPC_prep_io_readw (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inw(addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " <= 0x%08" PRIx32 "\n", addr, ret);
-
-    return ret;
-}
-
-static void PPC_prep_io_writel (void *opaque, hwaddr addr,
-                                uint32_t value)
-{
-    sysctrl_t *sysctrl = opaque;
-
-    addr = prep_IO_address(sysctrl, addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " => 0x%08" PRIx32 "\n", addr, value);
-    cpu_outl(addr, value);
-}
-
-static uint32_t PPC_prep_io_readl (void *opaque, hwaddr addr)
-{
-    sysctrl_t *sysctrl = opaque;
-    uint32_t ret;
-
-    addr = prep_IO_address(sysctrl, addr);
-    ret = cpu_inl(addr);
-    PPC_IO_DPRINTF("0x" TARGET_FMT_plx " <= 0x%08" PRIx32 "\n", addr, ret);
-
-    return ret;
+    address_space_write(&address_space_io, addr, (uint8_t *)&data, size);
 }
 
 static const MemoryRegionOps PPC_prep_io_ops = {
-    .old_mmio = {
-        .read = { PPC_prep_io_readb, PPC_prep_io_readw, PPC_prep_io_readl },
-        .write = { PPC_prep_io_writeb, PPC_prep_io_writew, PPC_prep_io_writel },
-    },
+    .read = PPC_prep_io_read,
+    .write = PPC_prep_io_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 7/8] PPC: pseries: Remove hack for PIO window

[Hervé Poussineau]

From: Alexander Graf <agraf@suse.de>

Now that all users of old_portio are gone, we can remove the hack
that enabled us to support them.

This is the same commit as a178274efabcbbc5d44805b51def874e47051325,
which has been reverted in a3cfa18eb075c7ef78358ca1956fe7b01caa1724.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/ppc/spapr_pci.c          |   44 +------------------------------------------
 include/hw/pci-host/spapr.h |    2 +-
 2 files changed, 2 insertions(+), 44 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 62ff323..161d570 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -439,43 +439,6 @@ static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
     qemu_set_irq(spapr_phb_lsi_qirq(phb, irq_num), level);
 }
 
-static uint64_t spapr_io_read(void *opaque, hwaddr addr,
-                              unsigned size)
-{
-    switch (size) {
-    case 1:
-        return cpu_inb(addr);
-    case 2:
-        return cpu_inw(addr);
-    case 4:
-        return cpu_inl(addr);
-    }
-    assert(0);
-}
-
-static void spapr_io_write(void *opaque, hwaddr addr,
-                           uint64_t data, unsigned size)
-{
-    switch (size) {
-    case 1:
-        cpu_outb(addr, data);
-        return;
-    case 2:
-        cpu_outw(addr, data);
-        return;
-    case 4:
-        cpu_outl(addr, data);
-        return;
-    }
-    assert(0);
-}
-
-static const MemoryRegionOps spapr_io_ops = {
-    .endianness = DEVICE_LITTLE_ENDIAN,
-    .read = spapr_io_read,
-    .write = spapr_io_write
-};
-
 /*
  * MSI/MSIX memory region implementation.
  * The handler handles both MSI and MSIX.
@@ -599,14 +562,9 @@ static int spapr_phb_init(SysBusDevice *s)
      * old_portion are updated */
     sprintf(namebuf, "%s.io", sphb->dtbusname);
     memory_region_init(&sphb->iospace, namebuf, SPAPR_PCI_IO_WIN_SIZE);
-    /* FIXME: fix to support multiple PHBs */
-    memory_region_add_subregion(get_system_io(), 0, &sphb->iospace);
 
-    sprintf(namebuf, "%s.io-alias", sphb->dtbusname);
-    memory_region_init_io(&sphb->iowindow, &spapr_io_ops, sphb,
-                          namebuf, SPAPR_PCI_IO_WIN_SIZE);
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
-                                &sphb->iowindow);
+                                &sphb->iospace);
 
     /* As MSI/MSIX interrupts trigger by writing at MSI/MSIX vectors,
      * we need to allocate some memory to catch those writes coming
diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
index b21080c..dabef16 100644
--- a/include/hw/pci-host/spapr.h
+++ b/include/hw/pci-host/spapr.h
@@ -44,7 +44,7 @@ typedef struct sPAPRPHBState {
     MemoryRegion memspace, iospace;
     hwaddr mem_win_addr, mem_win_size, io_win_addr, io_win_size;
     hwaddr msi_win_addr;
-    MemoryRegion memwindow, iowindow, msiwindow;
+    MemoryRegion memwindow, msiwindow;
 
     uint32_t dma_liobn;
     uint64_t dma_window_start;
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 8/8] isa_mmio: simplify access to system I/O region

[Hervé Poussineau]

We can now simply use address_space_rw instead of the more convoluted
cpu_in() and cpu_out() functions.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/isa/isa_mmio.c |   42 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 31 deletions(-)

diff --git a/hw/isa/isa_mmio.c b/hw/isa/isa_mmio.c
index d4dbf13..5bf8878 100644
--- a/hw/isa/isa_mmio.c
+++ b/hw/isa/isa_mmio.c
@@ -26,44 +26,24 @@
 #include "hw/isa/isa.h"
 #include "exec/address-spaces.h"
 
-static void isa_mmio_writeb (void *opaque, hwaddr addr,
-                                  uint32_t val)
+static void isa_mmio_write(void *opaque, hwaddr addr, uint64_t data,
+                           unsigned int size)
 {
-    cpu_outb(addr & IOPORTS_MASK, val);
+    addr &= IOPORTS_MASK;
+    address_space_write(&address_space_io, addr, (uint8_t *)&data, size);
 }
 
-static void isa_mmio_writew(void *opaque, hwaddr addr,
-                               uint32_t val)
+static uint64_t isa_mmio_read(void *opaque, hwaddr addr, unsigned int size)
 {
-    cpu_outw(addr & IOPORTS_MASK, val);
-}
-
-static void isa_mmio_writel(void *opaque, hwaddr addr,
-                               uint32_t val)
-{
-    cpu_outl(addr & IOPORTS_MASK, val);
-}
-
-static uint32_t isa_mmio_readb (void *opaque, hwaddr addr)
-{
-    return cpu_inb(addr & IOPORTS_MASK);
-}
-
-static uint32_t isa_mmio_readw(void *opaque, hwaddr addr)
-{
-    return cpu_inw(addr & IOPORTS_MASK);
-}
-
-static uint32_t isa_mmio_readl(void *opaque, hwaddr addr)
-{
-    return cpu_inl(addr & IOPORTS_MASK);
+    uint64_t data = 0;
+    addr &= IOPORTS_MASK;
+    address_space_read(&address_space_io, addr, (uint8_t *)&data, size);
+    return data;
 }
 
 static const MemoryRegionOps isa_mmio_ops = {
-    .old_mmio = {
-        .write = { isa_mmio_writeb, isa_mmio_writew, isa_mmio_writel },
-        .read = { isa_mmio_readb, isa_mmio_readw, isa_mmio_readl, },
-    },
+    .write = isa_mmio_write,
+    .read = isa_mmio_read,
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Hervé Poussineau]

Hervé Poussineau a écrit :
> These proposed patches aim at removing the .old_portio member of
> MemoryRegionOps structure, and replacing their usage by .read/.write
> handlers.

Ping.

> That way, faked I/O address space can be removed from architectures
> which don't have it (MIPS, PowerPC...), and commits like
> a178274efabcbbc5d44805b51def874e47051325 ("PPC: pseries: Remove hack
> for PIO window") can be reapplied.
> 
> The first 4 patches do the cleanup and remove the old_portio handler
> in MemoryRegion structure.
> The last 4 patches are simplifications, now that Portio handlers
> can be called from memory core without limitation.
> 
> Changes since v1:
> - handling of Portio has been moved to ioport.c (instead of isa-bus.c)
>   This prevents creating an ISA bus on machines which don't have one
> - added last 4 patches to see benefits of removing old_portio
> 
> Alexander Graf (1):
>   PPC: pseries: Remove hack for PIO window
> 
> Hervé Poussineau (7):
>   isa: fix documentation of isa_register_portio_list
>   memory: handle old_portio accesses in MMIO path
>   ioport: register memory regions for I/O port lists
>   memory: remove code dealing with old_portio
>   ioport: reimplement cpu_in/cpu_out using address_space_rw
>   ppc: simplify access to PReP I/O region
>   isa_mmio: simplify access to system I/O region
> 
>  hw/isa/isa_mmio.c           |   42 +++------
>  hw/ppc/prep.c               |   65 ++-----------
>  hw/ppc/spapr_pci.c          |   44 +--------
>  include/exec/ioport.h       |    3 +-
>  include/exec/memory.h       |    4 -
>  include/hw/isa/isa.h        |    2 +-
>  include/hw/pci-host/spapr.h |    2 +-
>  ioport.c                    |  215 ++++++++++++++++++++++---------------------
>  memory.c                    |   44 ---------
>  9 files changed, 137 insertions(+), 284 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Paolo Bonzini]

Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
> Hervé Poussineau a écrit :
>> These proposed patches aim at removing the .old_portio member of
>> MemoryRegionOps structure, and replacing their usage by .read/.write
>> handlers.
> 
> Ping.

Jan has patches that do something similar, so I was hoping he'd look at it.

Jan, are you back from vacation? :)

Paolo

Re: [Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Jan Kiszka]

On 2013-06-17 09:32, Paolo Bonzini wrote:
> Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
>> Hervé Poussineau a écrit :
>>> These proposed patches aim at removing the .old_portio member of
>>> MemoryRegionOps structure, and replacing their usage by .read/.write
>>> handlers.
>>
>> Ping.
> 
> Jan has patches that do something similar, so I was hoping he'd look at it.
> 
> Jan, are you back from vacation? :)

Yes, and that is the problem. ;)

>From a quick glance, I'm a bit skeptical, Hervé, that your patches are
addressing all corner cases like mine. Did you see
http://thread.gmane.org/gmane.comp.emulators.qemu/210188?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Hervé Poussineau]

Jan Kiszka a écrit :
> On 2013-06-17 09:32, Paolo Bonzini wrote:
>> Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
>>> Hervé Poussineau a écrit :
>>>> These proposed patches aim at removing the .old_portio member of
>>>> MemoryRegionOps structure, and replacing their usage by .read/.write
>>>> handlers.
>>> Ping.
>> Jan has patches that do something similar, so I was hoping he'd look at it.
>>
>> Jan, are you back from vacation? :)
> 
> Yes, and that is the problem. ;)
> 
>>From a quick glance, I'm a bit skeptical, Hervé, that your patches are
> addressing all corner cases like mine. Did you see
> http://thread.gmane.org/gmane.comp.emulators.qemu/210188?
> 
> Jan
> 

My patches are less intrusive than yours, because they are probably less 
complex. They don't change subpage handling, they don't remove the 
register_ioport_*, and they don't move ioport handling to memory core.

However, my patches do not add a new base address field in MemoryRegion, 
and also simplify cpu_in/out to be simply a call to 
address_space_read/write (like yours).

I don't really care whatever way is chosen. I'm only interested to be 
able to put I/O address space into memory space, so I can improve PReP 
emulation.

Hervé

Re: [Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Jan Kiszka]

On 2013-06-17 22:39, Hervé Poussineau wrote:
> Jan Kiszka a écrit :
>> On 2013-06-17 09:32, Paolo Bonzini wrote:
>>> Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
>>>> Hervé Poussineau a écrit :
>>>>> These proposed patches aim at removing the .old_portio member of
>>>>> MemoryRegionOps structure, and replacing their usage by .read/.write
>>>>> handlers.
>>>> Ping.
>>> Jan has patches that do something similar, so I was hoping he'd look at it.
>>>
>>> Jan, are you back from vacation? :)
>>
>> Yes, and that is the problem. ;)
>>
>> >From a quick glance, I'm a bit skeptical, Hervé, that your patches are
>> addressing all corner cases like mine. Did you see
>> http://thread.gmane.org/gmane.comp.emulators.qemu/210188?
>>
>> Jan
>>
> 
> My patches are less intrusive than yours, because they are probably less 
> complex. They don't change subpage handling, they don't remove the 
> register_ioport_*, and they don't move ioport handling to memory core.
> 
> However, my patches do not add a new base address field in MemoryRegion, 
> and also simplify cpu_in/out to be simply a call to 
> address_space_read/write (like yours).
> 
> I don't really care whatever way is chosen. I'm only interested to be 
> able to put I/O address space into memory space, so I can improve PReP 
> emulation.

Refactorings like the subpage changes are required to break up the BQL
also for PIO dispatching. So we need the complete rework. But, of
course, I'm open for improvement suggestions.

I'm planning to rebase my series on top of Paolo's changes soon and will
then post. Would you mind rebasing what you need additionally on top of
that?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Hervé Poussineau]

Jan Kiszka a écrit :
> On 2013-06-17 22:39, Hervé Poussineau wrote:
>> Jan Kiszka a écrit :
>>> On 2013-06-17 09:32, Paolo Bonzini wrote:
>>>> Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
>>>>> Hervé Poussineau a écrit :
>>>>>> These proposed patches aim at removing the .old_portio member of
>>>>>> MemoryRegionOps structure, and replacing their usage by .read/.write
>>>>>> handlers.
>>>>> Ping.
>>>> Jan has patches that do something similar, so I was hoping he'd look at it.
>>>>
>>>> Jan, are you back from vacation? :)
>>> Yes, and that is the problem. ;)
>>>
>>> >From a quick glance, I'm a bit skeptical, Hervé, that your patches are
>>> addressing all corner cases like mine. Did you see
>>> http://thread.gmane.org/gmane.comp.emulators.qemu/210188?
>>>
>>> Jan
>>>
>> My patches are less intrusive than yours, because they are probably less 
>> complex. They don't change subpage handling, they don't remove the 
>> register_ioport_*, and they don't move ioport handling to memory core.
>>
>> However, my patches do not add a new base address field in MemoryRegion, 
>> and also simplify cpu_in/out to be simply a call to 
>> address_space_read/write (like yours).
>>
>> I don't really care whatever way is chosen. I'm only interested to be 
>> able to put I/O address space into memory space, so I can improve PReP 
>> emulation.
> 
> Refactorings like the subpage changes are required to break up the BQL
> also for PIO dispatching. So we need the complete rework. But, of
> course, I'm open for improvement suggestions.
> 
> I'm planning to rebase my series on top of Paolo's changes soon and will
> then post. Would you mind rebasing what you need additionally on top of
> that?

Not a problem.
Moreover, with your patches, if devices registering ports with portio_* 
functions can be added in the system memory address space, it will be 
enough for me, and I'll happily drop my patches.

Hervé

Re: [Qemu-devel] [PATCH v2 0/8] memory: remove old_portio usage

[Jan Kiszka]

On 2013-06-18 20:09, Hervé Poussineau wrote:
> Jan Kiszka a écrit :
>> On 2013-06-17 22:39, Hervé Poussineau wrote:
>>> Jan Kiszka a écrit :
>>>> On 2013-06-17 09:32, Paolo Bonzini wrote:
>>>>> Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
>>>>>> Hervé Poussineau a écrit :
>>>>>>> These proposed patches aim at removing the .old_portio member of
>>>>>>> MemoryRegionOps structure, and replacing their usage by .read/.write
>>>>>>> handlers.
>>>>>> Ping.
>>>>> Jan has patches that do something similar, so I was hoping he'd look at it.
>>>>>
>>>>> Jan, are you back from vacation? :)
>>>> Yes, and that is the problem. ;)
>>>>
>>>> >From a quick glance, I'm a bit skeptical, Hervé, that your patches are
>>>> addressing all corner cases like mine. Did you see
>>>> http://thread.gmane.org/gmane.comp.emulators.qemu/210188?
>>>>
>>>> Jan
>>>>
>>> My patches are less intrusive than yours, because they are probably less 
>>> complex. They don't change subpage handling, they don't remove the 
>>> register_ioport_*, and they don't move ioport handling to memory core.
>>>
>>> However, my patches do not add a new base address field in MemoryRegion, 
>>> and also simplify cpu_in/out to be simply a call to 
>>> address_space_read/write (like yours).
>>>
>>> I don't really care whatever way is chosen. I'm only interested to be 
>>> able to put I/O address space into memory space, so I can improve PReP 
>>> emulation.
>>
>> Refactorings like the subpage changes are required to break up the BQL
>> also for PIO dispatching. So we need the complete rework. But, of
>> course, I'm open for improvement suggestions.
>>
>> I'm planning to rebase my series on top of Paolo's changes soon and will
>> then post. Would you mind rebasing what you need additionally on top of
>> that?
> 
> Not a problem.
> Moreover, with your patches, if devices registering ports with portio_* 
> functions can be added in the system memory address space, it will be 
> enough for me, and I'll happily drop my patches.

Sorry, forgot to CC you. Please check
http://thread.gmane.org/gmane.comp.emulators.qemu/218193.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux