memory: remove old_portio usage

PATCH In review Hervé Poussineau 3 participants 16 messages Jun 8, 2013 – Jun 21, 2013

Share & tools

Hervé Poussineau

author

2013-06-08 17:43 UTC #0 lore raw

v2 cover memory: remove old_portio usage

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(-)

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:43 UTC #1 lore raw

PATCH v2 1/8 isa: fix documentation of isa_register_portio_list

Signed-off-by Hervé Poussineau <[email protected]>

---
 include/hw/isa/isa.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/hw/isa/isa.h +1 -1

index 82da37c..556790c 100644
@@ -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,

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:43 UTC #2 lore raw

PATCH v2 2/8 memory: handle old_portio accesses in MMIO path

In reply to Hervé Poussineau

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

Signed-off-by Hervé Poussineau <[email protected]>

---
 memory.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

memory.c +16 -0

index 5cb8f4a..f27167c 100644
@@ -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);

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:43 UTC #3 lore raw

PATCH v2 3/8 ioport: register memory regions for I/O port lists

In reply to 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 <[email protected]>

---
 include/exec/ioport.h |    3 +-
 ioport.c              |  132 +++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 107 insertions(+), 28 deletions(-)

include/exec/ioport.h +2 -1

index fc28350..398fe14 100644
@@ -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;

ioport.c +105 ~~-27~~

188 lines not on this page · raw message

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:43 UTC #4 lore raw

PATCH v2 4/8 memory: remove code dealing with old_portio

In reply to Hervé Poussineau

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

Signed-off-by Hervé Poussineau <[email protected]>

---
 include/exec/memory.h |    4 ----
 memory.c              |   60 -------------------------------------------------
 2 files changed, 64 deletions(-)

include/exec/memory.h +0 -4

index d53a6a1..3c225c9 100644
@@ -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
      */

memory.c +0 ~~-60~~

90 lines not on this page · raw message

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:43 UTC #5 lore raw

PATCH v2 5/8 ioport: reimplement cpu_in/cpu_out using address_space_rw

In reply to Hervé Poussineau

Signed-off-by Hervé Poussineau <[email protected]>

---
 ioport.c |   83 ++++++--------------------------------------------------------
 1 file changed, 7 insertions(+), 76 deletions(-)

ioport.c +7 ~~-76~~

index d26d3e1..7bc9d8f 100644
@@ -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;

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:43 UTC #6 lore raw

PATCH v2 6/8 ppc: simplify access to PReP I/O region

In reply to 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 <[email protected]>

---
 hw/ppc/prep.c |   65 ++++++++-------------------------------------------------
 1 file changed, 9 insertions(+), 56 deletions(-)

hw/ppc/prep.c +9 ~~-56~~

index be8a50e..bb77e1f 100644
@@ -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,
 };

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:43 UTC #7 lore raw

PATCH v2 7/8 PPC: pseries: Remove hack for PIO window

In reply to Hervé Poussineau

From: Alexander Graf <[email protected]>

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 <[email protected]>

---
 hw/ppc/spapr_pci.c          |   44 +------------------------------------------
 include/hw/pci-host/spapr.h |    2 +-
 2 files changed, 2 insertions(+), 44 deletions(-)

hw/ppc/spapr_pci.c +1 ~~-43~~

index 62ff323..161d570 100644
@@ -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

include/hw/pci-host/spapr.h +1 -1

index b21080c..dabef16 100644
@@ -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;

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-08 17:44 UTC #8 lore raw

PATCH v2 8/8 isa_mmio: simplify access to system I/O region

In reply to 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 <[email protected]>

---
 hw/isa/isa_mmio.c |   42 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 31 deletions(-)

hw/isa/isa_mmio.c +11 ~~-31~~

index d4dbf13..5bf8878 100644
@@ -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,
 };

Signature · 1 line

1.7.10.4

Hervé Poussineau

author

2013-06-16 18:20 UTC #9 lore raw

In reply to Hervé Poussineau

Quoted 3 lines · Hervé Poussineau

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

Ping.

Quoted 38 lines

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(-)

Paolo Bonzini

2013-06-17 07:32 UTC #10 lore raw

Il 16/06/2013 20:20, Hervé Poussineau ha scritto:

Quoted 6 lines

Quoted 3 lines · Hervé Poussineau
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

Jan Kiszka

2013-06-17 07:43 UTC #11 lore raw

Quoted 11 lines · Paolo Bonzini

Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
Quoted 6 lines
Quoted 3 lines · Hervé Poussineau
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. ;)

Quoted 1 line

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

Signature · 2 lines

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

Hervé Poussineau

author

2013-06-17 20:39 UTC #12 lore raw

Quoted 11 lines · Jan Kiszka

Quoted 9 lines · Paolo Bonzini
Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
Quoted 5 lines
Quoted 3 lines · Hervé Poussineau
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? :)

Quoted 8 lines

Yes, and that is the problem. ;)
Quoted 1 line
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é

Jan Kiszka

2013-06-18 14:50 UTC #13 lore raw

Quoted 25 lines · Hervé Poussineau

Quoted 11 lines · Jan Kiszka
Quoted 9 lines · Paolo Bonzini
Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
Quoted 5 lines
Quoted 3 lines · Hervé Poussineau
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? :)
Quoted 8 lines
Yes, and that is the problem. ;)
Quoted 1 line
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.

Quoted 7 lines

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

Signature · 2 lines

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

Hervé Poussineau

author

2013-06-18 18:09 UTC #14 lore raw

Quoted 26 lines · Jan Kiszka

Quoted 17 lines · Hervé Poussineau
Quoted 16 lines · Jan Kiszka
Quoted 9 lines · Paolo Bonzini
Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
Quoted 5 lines
Quoted 3 lines · Hervé Poussineau
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. ;)
Quoted 1 line
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?
Quoted 7 lines
Quoted 2 lines
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.

Quoted 7 lines

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é

Jan Kiszka

2013-06-21 18:12 UTC #15 lore raw

Quoted 37 lines · Hervé Poussineau

Quoted 36 lines · Jan Kiszka
Quoted 23 lines · Hervé Poussineau
Quoted 9 lines · Jan Kiszka
Quoted 8 lines · Paolo Bonzini
Il 16/06/2013 20:20, Hervé Poussineau ha scritto:
Quoted 5 lines
Quoted 3 lines · Hervé Poussineau
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.
Quoted 9 lines
Quoted 1 line
Jan, are you back from vacation? :)
Yes, and that is the problem. ;)
Quoted 1 line
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.
Quoted 7 lines
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.

Quoted 8 lines

Quoted 3 lines
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

Signature · 2 lines

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