[Qemu-devel] [RFC 0/8] Remove old_portio usage

[Qemu-devel] [RFC 0/8] 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.

Patches 2 to 6 are already ready to be committed.
However, patches 7 and 8 depend of patch 1, where a better solution
needs to be found.

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.

Hervé Poussineau (8):
  pc: disable bochs bios debug ports (do not apply!)
  xen_platform: do not use old_portio-style callbacks
  uhci: do not use old_portio-style callbacks
  acpi-piix4: do not use old_portio-style callbacks
  vga/qxl: do not use portio_list_init/portio_list_add
  isa: use memory regions instead of portio_list_* functions
  ioport: remove now useless portio_list_* functions
  memory: remove old_portio-style callbacks support

 hw/acpi_piix4.c   |   91 +++++++++++++++++---------------------
 hw/isa-bus.c      |  127 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/isa.h          |    2 +-
 hw/pc.c           |    6 +++
 hw/qxl.c          |    4 +-
 hw/usb/hcd-uhci.c |   43 +++++++++++++-----
 hw/vga.c          |    8 +---
 hw/xen_platform.c |   21 +++++----
 ioport.c          |  121 --------------------------------------------------
 ioport.h          |   19 --------
 memory.c          |   44 -------------------
 memory.h          |    4 --
 12 files changed, 217 insertions(+), 273 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [RFC 1/8] pc: disable bochs bios debug ports (do not apply!)

[Hervé Poussineau]

This patch must not be applied as-is.
Some patches to use MemoryRegion have already been sent to
mailing list, but none has been applied yet.
---
 hw/pc.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/pc.c b/hw/pc.c
index b11e7c4..7f98955 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -524,6 +524,7 @@ static void handle_a20_line_change(void *opaque, int irq, int level)
     cpu_x86_set_a20(cpu, level);
 }
 
+#if 0
 /***********************************************************/
 /* Bochs BIOS debug ports */
 
@@ -551,6 +552,7 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
         exit((val << 1) | 1);
     }
 }
+#endif
 
 int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
 {
@@ -569,6 +571,7 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
     return index;
 }
 
+#if 0
 static const MemoryRegionPortio bochs_bios_portio_list[] = {
     { 0x500, 1, 1, .write = bochs_bios_write, }, /* 0x500 */
     { 0x501, 1, 1, .write = bochs_bios_write, }, /* 0x501 */
@@ -576,6 +579,7 @@ static const MemoryRegionPortio bochs_bios_portio_list[] = {
     { 0x8900, 1, 1, .write = bochs_bios_write, }, /* 0x8900 */
     PORTIO_END_OF_LIST(),
 };
+#endif
 
 static void *bochs_bios_init(void)
 {
@@ -584,11 +588,13 @@ static void *bochs_bios_init(void)
     size_t smbios_len;
     uint64_t *numa_fw_cfg;
     int i, j;
+#if 0
     PortioList *bochs_bios_port_list = g_new(PortioList, 1);
 
     portio_list_init(bochs_bios_port_list, bochs_bios_portio_list,
                      NULL, "bochs-bios");
     portio_list_add(bochs_bios_port_list, get_system_io(), 0x0);
+#endif
 
     fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
 
-- 
1.7.10.4

[Qemu-devel] [RFC 2/8] xen_platform: do not use old_portio-style callbacks

[Hervé Poussineau]

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

diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index a54e7a2..ad7cb06 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -280,7 +280,8 @@ static void platform_fixed_ioport_init(PCIXenPlatformState* s)
 
 /* Xen Platform PCI Device */
 
-static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr)
+static uint64_t xen_platform_ioport_readb(void *opaque, hwaddr addr,
+                                          unsigned int size)
 {
     if (addr == 0) {
         return platform_fixed_ioport_readb(opaque, 0);
@@ -289,30 +290,28 @@ static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr)
     }
 }
 
-static void xen_platform_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void xen_platform_ioport_writeb(void *opaque, hwaddr addr,
+                                       uint64_t val, unsigned int size)
 {
     PCIXenPlatformState *s = opaque;
 
     switch (addr) {
     case 0: /* Platform flags */
-        platform_fixed_ioport_writeb(opaque, 0, val);
+        platform_fixed_ioport_writeb(opaque, 0, (uint32_t)val);
         break;
     case 8:
-        log_writeb(s, val);
+        log_writeb(s, (uint32_t)val);
         break;
     default:
         break;
     }
 }
 
-static MemoryRegionPortio xen_pci_portio[] = {
-    { 0, 0x100, 1, .read = xen_platform_ioport_readb, },
-    { 0, 0x100, 1, .write = xen_platform_ioport_writeb, },
-    PORTIO_END_OF_LIST()
-};
-
 static const MemoryRegionOps xen_pci_io_ops = {
-    .old_portio = xen_pci_portio,
+    .read  = xen_platform_ioport_readb,
+    .write = xen_platform_ioport_writeb,
+    .impl.min_access_size = 1,
+    .impl.max_access_size = 1,
 };
 
 static void platform_ioport_bar_setup(PCIXenPlatformState *d)
-- 
1.7.10.4

[Qemu-devel] [RFC 3/8] uhci: do not use old_portio-style callbacks

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/usb/hcd-uhci.c |   43 +++++++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index d053791..cc47635 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1213,18 +1213,41 @@ static void uhci_frame_timer(void *opaque)
     qemu_mod_timer(s->frame_timer, s->expire_time);
 }
 
-static const MemoryRegionPortio uhci_portio[] = {
-    { 0, 32, 2, .write = uhci_ioport_writew, },
-    { 0, 32, 2, .read = uhci_ioport_readw, },
-    { 0, 32, 4, .write = uhci_ioport_writel, },
-    { 0, 32, 4, .read = uhci_ioport_readl, },
-    { 0, 32, 1, .write = uhci_ioport_writeb, },
-    { 0, 32, 1, .read = uhci_ioport_readb, },
-    PORTIO_END_OF_LIST()
-};
+static uint64_t uhci_ioport_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    switch (size) {
+    case 1:
+        return uhci_ioport_readb(opaque, (uint32_t)addr);
+    case 2:
+        return uhci_ioport_readw(opaque, (uint32_t)addr);
+    case 4:
+        return uhci_ioport_readl(opaque, (uint32_t)addr);
+    default:
+        return ~0UL;
+    }
+}
+
+static void uhci_ioport_write(void *opaque, hwaddr addr, uint64_t data,
+                              unsigned int size)
+{
+    switch (size) {
+    case 1:
+        uhci_ioport_writeb(opaque, (uint32_t)addr, (uint32_t)data);
+        break;
+    case 2:
+        uhci_ioport_writew(opaque, (uint32_t)addr, (uint32_t)data);
+        break;
+    case 4:
+        uhci_ioport_writel(opaque, (uint32_t)addr, (uint32_t)data);
+        break;
+    default:
+        break;
+    }
+}
 
 static const MemoryRegionOps uhci_ioport_ops = {
-    .old_portio = uhci_portio,
+    .read = uhci_ioport_read,
+    .write = uhci_ioport_write,
 };
 
 static USBPortOps uhci_port_ops = {
-- 
1.7.10.4

[Qemu-devel] [RFC 4/8] acpi-piix4: do not use old_portio-style callbacks

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/acpi_piix4.c |   91 ++++++++++++++++++++++++-------------------------------
 1 file changed, 40 insertions(+), 51 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0b5b0d3..22704af 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -526,68 +526,57 @@ static const MemoryRegionOps piix4_gpe_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static uint32_t pci_up_read(void *opaque, uint32_t addr)
+static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size)
 {
     PIIX4PMState *s = opaque;
-    uint32_t val;
-
-    /* Manufacture an "up" value to cause a device check on any hotplug
-     * slot with a device.  Extra device checks are harmless. */
-    val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
-
-    PIIX4_DPRINTF("pci_up_read %x\n", val);
-    return val;
-}
-
-static uint32_t pci_down_read(void *opaque, uint32_t addr)
-{
-    PIIX4PMState *s = opaque;
-    uint32_t val = s->pci0_status.down;
+    uint32_t val = 0;
+
+    switch (addr) {
+    case PCI_UP_BASE - PCI_HOTPLUG_ADDR:
+        /* Manufacture an "up" value to cause a device check on any hotplug
+         * slot with a device.  Extra device checks are harmless. */
+        val = s->pci0_slot_device_present & s->pci0_hotplug_enable;
+        PIIX4_DPRINTF("pci_up_read %x\n", val);
+        break;
+    case PCI_DOWN_BASE - PCI_HOTPLUG_ADDR:
+        val = s->pci0_status.down;
+        PIIX4_DPRINTF("pci_down_read %x\n", val);
+        break;
+    case PCI_EJ_BASE - PCI_HOTPLUG_ADDR:
+        /* No feature defined yet */
+        PIIX4_DPRINTF("pci_features_read %x\n", val);
+        break;
+    case PCI_RMV_BASE - PCI_HOTPLUG_ADDR:
+        val = s->pci0_hotplug_enable;
+        break;
+    default:
+        break;
+    }
 
-    PIIX4_DPRINTF("pci_down_read %x\n", val);
     return val;
 }
 
-static uint32_t pci_features_read(void *opaque, uint32_t addr)
-{
-    /* No feature defined yet */
-    PIIX4_DPRINTF("pci_features_read %x\n", 0);
-    return 0;
-}
-
-static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
-{
-    acpi_piix_eject_slot(opaque, val);
-
-    PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val);
-}
-
-static uint32_t pcirmv_read(void *opaque, uint32_t addr)
+static void pci_write(void *opaque, hwaddr addr, uint64_t data,
+                      unsigned int size)
 {
-    PIIX4PMState *s = opaque;
-
-    return s->pci0_hotplug_enable;
+    switch (addr) {
+    case PCI_EJ_BASE - PCI_HOTPLUG_ADDR:
+        acpi_piix_eject_slot(opaque, (uint32_t)data);
+        PIIX4_DPRINTF("pciej write %x <== % " PRIu64 "\n", addr, data);
+        break;
+    default:
+        break;
+    }
 }
 
 static const MemoryRegionOps piix4_pci_ops = {
-    .old_portio = (MemoryRegionPortio[]) {
-        {
-            .offset = PCI_UP_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
-            .read = pci_up_read,
-        },{
-            .offset = PCI_DOWN_BASE - PCI_HOTPLUG_ADDR, .len = 4, .size = 4,
-            .read = pci_down_read,
-        },{
-            .offset = PCI_EJ_BASE - PCI_HOTPLUG_ADDR,   .len = 4, .size = 4,
-            .read = pci_features_read,
-            .write = pciej_write,
-        },{
-            .offset = PCI_RMV_BASE - PCI_HOTPLUG_ADDR,  .len = 4, .size = 4,
-            .read = pcirmv_read,
-        },
-        PORTIO_END_OF_LIST()
+    .read = pci_read,
+    .write = pci_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
     },
-    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
-- 
1.7.10.4

[Qemu-devel] [RFC 5/8] vga/qxl: do not use portio_list_init/portio_list_add

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/qxl.c |    4 +---
 hw/vga.c |    8 ++------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 96887c4..7982524 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -2048,7 +2048,6 @@ static int qxl_init_primary(PCIDevice *dev)
 {
     PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
     VGACommonState *vga = &qxl->vga;
-    PortioList *qxl_vga_port_list = g_new(PortioList, 1);
     int rc;
 
     qxl->id = 0;
@@ -2056,8 +2055,7 @@ static int qxl_init_primary(PCIDevice *dev)
     vga->vram_size_mb = qxl->vga.vram_size >> 20;
     vga_common_init(vga);
     vga_init(vga, pci_address_space(dev), pci_address_space_io(dev), false);
-    portio_list_init(qxl_vga_port_list, qxl_vga_portio_list, vga, "vga");
-    portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0);
+    isa_register_portio_list(NULL, 0x3b0, qxl_vga_portio_list, vga, "vga");
 
     vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
                                    qxl_hw_screen_dump, qxl_hw_text_update, qxl);
diff --git a/hw/vga.c b/hw/vga.c
index c266161..ccdb997 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2349,8 +2349,6 @@ void vga_init(VGACommonState *s, MemoryRegion *address_space,
 {
     MemoryRegion *vga_io_memory;
     const MemoryRegionPortio *vga_ports, *vbe_ports;
-    PortioList *vga_port_list = g_new(PortioList, 1);
-    PortioList *vbe_port_list = g_new(PortioList, 1);
 
     qemu_register_reset(vga_reset, s);
 
@@ -2365,12 +2363,10 @@ void vga_init(VGACommonState *s, MemoryRegion *address_space,
                                         1);
     memory_region_set_coalescing(vga_io_memory);
     if (init_vga_ports) {
-        portio_list_init(vga_port_list, vga_ports, s, "vga");
-        portio_list_add(vga_port_list, address_space_io, 0x3b0);
+        isa_register_portio_list(NULL, 0x3b0, vga_ports, s, "vga");
     }
     if (vbe_ports) {
-        portio_list_init(vbe_port_list, vbe_ports, s, "vbe");
-        portio_list_add(vbe_port_list, address_space_io, 0x1ce);
+        isa_register_portio_list(NULL, 0x1ce, vbe_ports, s, "vbe");
     }
 }
 
-- 
1.7.10.4

[Qemu-devel] [RFC 6/8] isa: use memory regions instead of portio_list_* functions

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/isa-bus.c |  127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/isa.h     |    2 +-
 2 files changed, 125 insertions(+), 4 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 144a88e..d22f432 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -104,19 +104,140 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)
     isa_init_ioport(dev, start);
 }
 
+typedef struct PortioState {
+    const char *name; /* debug purposes */
+    uint16_t start;
+    uint16_t offset;
+    const MemoryRegionPortio *pio_start;
+    void *opaque;
+} PortioState;
+
+static const MemoryRegionPortio *portio_find(const MemoryRegionPortio *mrp,
+                                             uint64_t offset,
+                                             unsigned int width, bool write,
+                                             bool smaller)
+{
+    for (; mrp->size; ++mrp) {
+        if (offset >= mrp->offset && offset < mrp->offset + mrp->len
+            && (width == mrp->size || (smaller && 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 PortioState *s = opaque;
+    const MemoryRegionPortio *mrp;
+
+    addr += s->offset;
+    mrp = portio_find(s->pio_start, addr, size, false, false);
+    if (mrp) {
+        return mrp->read(s->opaque, s->start + addr);
+    } else if (size == 2) {
+        uint64_t data;
+        mrp = portio_find(s->pio_start, addr, 1, false, false);
+        assert(mrp);
+        data = mrp->read(s->opaque, s->start + addr) |
+              (mrp->read(s->opaque, s->start + addr + 1) << 8);
+        return data;
+    }
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid read from 0x%x size=%d",
+                  s->name, s->start + (int)addr, size);
+    return -1U;
+}
+
+static void portio_write(void *opaque, hwaddr addr, uint64_t data,
+                         unsigned int size)
+{
+    const PortioState *s = opaque;
+    const MemoryRegionPortio *mrp;
+
+    addr += s->offset;
+    mrp = portio_find(s->pio_start, addr, size, true, false);
+    if (mrp) {
+        mrp->write(s->opaque, s->start + addr, data);
+        return;
+    } else if (size == 2) {
+        mrp = portio_find(s->pio_start, addr, 1, true, false);
+        assert(mrp);
+        mrp->write(s->opaque, s->start + addr, data & 0xff);
+        mrp->write(s->opaque, s->start + addr + 1, data >> 8);
+        return;
+    }
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid write to 0x%x size=%d",
+                  s->name, s->start + (int)addr, size);
+}
+
+static bool portio_accepts(void *opaque, hwaddr addr, unsigned int size,
+                           bool is_write)
+{
+    const PortioState *s = opaque;
+    const MemoryRegionPortio *mrp;
+
+    addr += s->offset;
+    mrp = portio_find(s->pio_start, addr, size, is_write, true);
+    return (mrp != NULL);
+}
+
+const MemoryRegionOps portio_ops = {
+    .read = portio_read,
+    .write = portio_write,
+    .valid.accepts = portio_accepts,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void isa_register_portio_list_1(ISADevice *dev, uint16_t start,
+                                       uint16_t offset, uint16_t end,
+                                       const MemoryRegionPortio *pio_start,
+                                       void *opaque, const char *name)
+{
+    MemoryRegion *mr = g_new(MemoryRegion, 1);
+    PortioState *s = g_new(PortioState, 1);
+
+    s->name = name;
+    s->start = start;
+    s->offset = offset;
+    s->pio_start = pio_start;
+    s->opaque = opaque;
+    memory_region_init_io(mr, &portio_ops, s, name, end - offset);
+    memory_region_add_subregion(isa_address_space_io(dev),
+                                start + offset, mr);
+}
+
 void isa_register_portio_list(ISADevice *dev, uint16_t start,
                               const MemoryRegionPortio *pio_start,
                               void *opaque, const char *name)
 {
-    PortioList *piolist = g_new(PortioList, 1);
+    const MemoryRegionPortio *pio, *first;
+    uint16_t end;
 
     /* START is how we should treat DEV, regardless of the actual
        contents of the portio array.  This is how the old code
        actually handled e.g. the FDC device.  */
     isa_init_ioport(dev, start);
 
-    portio_list_init(piolist, pio_start, opaque, name);
-    portio_list_add(piolist, isabus->address_space_io, start);
+    assert(pio_start->size);
+
+    first = pio_start;
+    end = 0;
+    for (pio = pio_start; pio->size; pio++) {
+        assert(pio->offset >= first->offset);
+        if (pio->offset > first->offset + first->len) {
+            isa_register_portio_list_1(dev, start, first->offset, end,
+                                       pio_start, opaque, name);
+            first = pio;
+            end = 0;
+        }
+        if (pio->offset + pio->len > end) {
+            end = pio->offset + pio->len;
+        }
+    }
+
+    isa_register_portio_list_1(dev, start, first->offset, end,
+                               pio_start, opaque, name);
 }
 
 static int isa_qdev_init(DeviceState *qdev)
diff --git a/hw/isa.h b/hw/isa.h
index 9d719fa..d44d16d 100644
--- a/hw/isa.h
+++ b/hw/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] [RFC 7/8] ioport: remove now useless portio_list_* functions

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 ioport.c |  121 --------------------------------------------------------------
 ioport.h |   19 ----------
 2 files changed, 140 deletions(-)

diff --git a/ioport.c b/ioport.c
index 6e4ca0d..05f8026 100644
--- a/ioport.c
+++ b/ioport.c
@@ -329,124 +329,3 @@ uint32_t cpu_inl(pio_addr_t addr)
     LOG_IOPORT("inl : %04"FMT_pioaddr" %08"PRIx32"\n", addr, val);
     return val;
 }
-
-void portio_list_init(PortioList *piolist,
-                      const MemoryRegionPortio *callbacks,
-                      void *opaque, const char *name)
-{
-    unsigned n = 0;
-
-    while (callbacks[n].size) {
-        ++n;
-    }
-
-    piolist->ports = callbacks;
-    piolist->nr = 0;
-    piolist->regions = g_new0(MemoryRegion *, n);
-    piolist->aliases = g_new0(MemoryRegion *, n);
-    piolist->address_space = NULL;
-    piolist->opaque = opaque;
-    piolist->name = name;
-}
-
-void portio_list_destroy(PortioList *piolist)
-{
-    g_free(piolist->regions);
-    g_free(piolist->aliases);
-}
-
-static void portio_list_add_1(PortioList *piolist,
-                              const MemoryRegionPortio *pio_init,
-                              unsigned count, unsigned start,
-                              unsigned off_low, unsigned off_high)
-{
-    MemoryRegionPortio *pio;
-    MemoryRegionOps *ops;
-    MemoryRegion *region, *alias;
-    unsigned i;
-
-    /* Copy the sub-list and null-terminate it.  */
-    pio = g_new(MemoryRegionPortio, count + 1);
-    memcpy(pio, pio_init, sizeof(MemoryRegionPortio) * count);
-    memset(pio + count, 0, sizeof(MemoryRegionPortio));
-
-    /* Adjust the offsets to all be zero-based for the region.  */
-    for (i = 0; i < count; ++i) {
-        pio[i].offset -= off_low;
-    }
-
-    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);
-    memory_region_add_subregion(piolist->address_space,
-                                start + off_low, alias);
-    piolist->regions[piolist->nr] = region;
-    piolist->aliases[piolist->nr] = alias;
-    ++piolist->nr;
-}
-
-void portio_list_add(PortioList *piolist,
-                     MemoryRegion *address_space,
-                     uint32_t start)
-{
-    const MemoryRegionPortio *pio, *pio_start = piolist->ports;
-    unsigned int off_low, off_high, off_last, count;
-
-    piolist->address_space = address_space;
-
-    /* Handle the first entry specially.  */
-    off_last = off_low = pio_start->offset;
-    off_high = off_low + pio_start->len;
-    count = 1;
-
-    for (pio = pio_start + 1; pio->size != 0; pio++, count++) {
-        /* All entries must be sorted by offset.  */
-        assert(pio->offset >= off_last);
-        off_last = pio->offset;
-
-        /* If we see a hole, break the region.  */
-        if (off_last > off_high) {
-            portio_list_add_1(piolist, pio_start, count, start, off_low,
-                              off_high);
-            /* ... and start collecting anew.  */
-            pio_start = pio;
-            off_low = off_last;
-            off_high = off_low + pio->len;
-            count = 0;
-        } else if (off_last + pio->len > off_high) {
-            off_high = off_last + pio->len;
-        }
-    }
-
-    /* There will always be an open sub-list.  */
-    portio_list_add_1(piolist, pio_start, count, start, off_low, off_high);
-}
-
-void portio_list_del(PortioList *piolist)
-{
-    MemoryRegion *mr, *alias;
-    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);
-        memory_region_destroy(mr);
-        g_free((MemoryRegionOps *)mr->ops);
-        g_free(mr);
-        g_free(alias);
-        piolist->regions[i] = NULL;
-        piolist->aliases[i] = NULL;
-    }
-}
diff --git a/ioport.h b/ioport.h
index 23441cb..cb06bff 100644
--- a/ioport.h
+++ b/ioport.h
@@ -56,23 +56,4 @@ uint32_t cpu_inl(pio_addr_t addr);
 struct MemoryRegion;
 struct MemoryRegionPortio;
 
-typedef struct PortioList {
-    const struct MemoryRegionPortio *ports;
-    struct MemoryRegion *address_space;
-    unsigned nr;
-    struct MemoryRegion **regions;
-    struct MemoryRegion **aliases;
-    void *opaque;
-    const char *name;
-} PortioList;
-
-void portio_list_init(PortioList *piolist,
-                      const struct MemoryRegionPortio *callbacks,
-                      void *opaque, const char *name);
-void portio_list_destroy(PortioList *piolist);
-void portio_list_add(PortioList *piolist,
-                     struct MemoryRegion *address_space,
-                     uint32_t addr);
-void portio_list_del(PortioList *piolist);
-
 #endif /* IOPORT_H */
-- 
1.7.10.4

[Qemu-devel] [RFC 8/8] memory: remove old_portio-style callbacks support

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 memory.c |   44 --------------------------------------------
 memory.h |    4 ----
 2 files changed, 48 deletions(-)

diff --git a/memory.c b/memory.c
index 7419853..ea3bb8a 100644
--- a/memory.c
+++ b/memory.c
@@ -365,21 +365,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,
@@ -390,21 +375,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,
@@ -422,20 +392,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,
diff --git a/memory.h b/memory.h
index 9462bfd..bdde0d0 100644
--- a/memory.h
+++ b/memory.h
@@ -103,10 +103,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
      */
-- 
1.7.10.4

Re: [Qemu-devel] [RFC 0/8] Remove old_portio usage

[Andreas Färber]

Am 23.12.2012 16:32, schrieb Hervé Poussineau:
> These proposed patches aim at removing the .old_portio member of
> MemoryRegionOps structure, and replacing their usage by .read/.write
> handlers.
> 
> Patches 2 to 6 are already ready to be committed.

In lack of a memory maintainer, I'll volunteer for reviewing those
portio conversions.

> However, patches 7 and 8 depend of patch 1, where a better solution
> needs to be found.

As for finding a solution to the bochs debug ports, can you resubmit
this series with the commit'able patches first, marked PATCH, and append
a proposal merging Gerd's, Lucas' and your QOM'ified debug device? Then
we can get the easy parts merged, and Anthony can ack/nack/timeout the
rest. :)

Regards,
Andreas

> 
> 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.
> 
> Hervé Poussineau (8):
>   pc: disable bochs bios debug ports (do not apply!)
>   xen_platform: do not use old_portio-style callbacks
>   uhci: do not use old_portio-style callbacks
>   acpi-piix4: do not use old_portio-style callbacks
>   vga/qxl: do not use portio_list_init/portio_list_add
>   isa: use memory regions instead of portio_list_* functions
>   ioport: remove now useless portio_list_* functions
>   memory: remove old_portio-style callbacks support

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC 0/8] Remove old_portio usage

[Gerd Hoffmann]

  Hi,

> As for finding a solution to the bochs debug ports, can you resubmit
> this series with the commit'able patches first, marked PATCH, and append
> a proposal merging Gerd's, Lucas' and your QOM'ified debug device?

I'll try to get a updated patch series with the debug/test devices out
of the door tomorrow, so we can finally zap the hardcoded bochs debug ports.

Beside that I agree that it is a good idea to split the series into two,
with ready-to-go and rfc patches, so we can merge the ready ones while
discussing the other ones.

cheers,
  Gerd

Re: [Qemu-devel] [RFC 3/8] uhci: do not use old_portio-style callbacks

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 973 bytes --]

On 12/23/12 16:32, Hervé Poussineau wrote:
> -static const MemoryRegionPortio uhci_portio[] = {
> -    { 0, 32, 2, .write = uhci_ioport_writew, },
> -    { 0, 32, 2, .read = uhci_ioport_readw, },
> -    { 0, 32, 4, .write = uhci_ioport_writel, },
> -    { 0, 32, 4, .read = uhci_ioport_readl, },
> -    { 0, 32, 1, .write = uhci_ioport_writeb, },
> -    { 0, 32, 1, .read = uhci_ioport_readb, },
> -    PORTIO_END_OF_LIST()
> -};
> +static uint64_t uhci_ioport_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    switch (size) {
> +    case 1:
> +        return uhci_ioport_readb(opaque, (uint32_t)addr);
> +    case 2:
> +        return uhci_ioport_readw(opaque, (uint32_t)addr);
> +    case 4:
> +        return uhci_ioport_readl(opaque, (uint32_t)addr);
> +    default:
> +        return ~0UL;
> +    }
> +}

Aaaargh.  Please don't.  Offloading the size handling to the memory api
is better.  See attached patch.

cheers,
  Gerd

[-- Attachment #2: 0001-uhci-stop-using-portio-lists.patch --]
[-- Type: text/plain, Size: 5193 bytes --]

From 3f5810004dad2ef67cf067d96cf0b983166a454f Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 3 Jan 2013 12:29:41 +0100
Subject: [PATCH] uhci: stop using portio lists

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-uhci.c |  106 +++++++++++++++--------------------------------------
 trace-events      |    2 -
 2 files changed, 30 insertions(+), 78 deletions(-)

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 0cd68cf..60645aa 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -461,40 +461,11 @@ static const VMStateDescription vmstate_uhci = {
     }
 };
 
-static void uhci_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void uhci_port_write(void *opaque, hwaddr addr,
+                            uint64_t val, unsigned size)
 {
     UHCIState *s = opaque;
 
-    addr &= 0x1f;
-    switch(addr) {
-    case 0x0c:
-        s->sof_timing = val;
-        break;
-    }
-}
-
-static uint32_t uhci_ioport_readb(void *opaque, uint32_t addr)
-{
-    UHCIState *s = opaque;
-    uint32_t val;
-
-    addr &= 0x1f;
-    switch(addr) {
-    case 0x0c:
-        val = s->sof_timing;
-        break;
-    default:
-        val = 0xff;
-        break;
-    }
-    return val;
-}
-
-static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
-{
-    UHCIState *s = opaque;
-
-    addr &= 0x1f;
     trace_usb_uhci_mmio_writew(addr, val);
 
     switch(addr) {
@@ -543,6 +514,17 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
         if (s->status & UHCI_STS_HCHALTED)
             s->frnum = val & 0x7ff;
         break;
+    case 0x08:
+        s->fl_base_addr &= 0xffff0000;
+        s->fl_base_addr |= val & ~0xfff;
+        break;
+    case 0x0a:
+        s->fl_base_addr &= 0x0000ffff;
+        s->fl_base_addr |= (val << 16);
+        break;
+    case 0x0c:
+        s->sof_timing = val & 0xff;
+        break;
     case 0x10 ... 0x1f:
         {
             UHCIPort *port;
@@ -574,12 +556,11 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-static uint32_t uhci_ioport_readw(void *opaque, uint32_t addr)
+static uint64_t uhci_port_read(void *opaque, hwaddr addr, unsigned size)
 {
     UHCIState *s = opaque;
     uint32_t val;
 
-    addr &= 0x1f;
     switch(addr) {
     case 0x00:
         val = s->cmd;
@@ -593,6 +574,15 @@ static uint32_t uhci_ioport_readw(void *opaque, uint32_t addr)
     case 0x06:
         val = s->frnum;
         break;
+    case 0x08:
+        val = s->fl_base_addr & 0xffff;
+        break;
+    case 0x0a:
+        val = (s->fl_base_addr >> 16) & 0xffff;
+        break;
+    case 0x0c:
+        val = s->sof_timing;
+        break;
     case 0x10 ... 0x1f:
         {
             UHCIPort *port;
@@ -615,38 +605,6 @@ static uint32_t uhci_ioport_readw(void *opaque, uint32_t addr)
     return val;
 }
 
-static void uhci_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
-{
-    UHCIState *s = opaque;
-
-    addr &= 0x1f;
-    trace_usb_uhci_mmio_writel(addr, val);
-
-    switch(addr) {
-    case 0x08:
-        s->fl_base_addr = val & ~0xfff;
-        break;
-    }
-}
-
-static uint32_t uhci_ioport_readl(void *opaque, uint32_t addr)
-{
-    UHCIState *s = opaque;
-    uint32_t val;
-
-    addr &= 0x1f;
-    switch(addr) {
-    case 0x08:
-        val = s->fl_base_addr;
-        break;
-    default:
-        val = 0xffffffff;
-        break;
-    }
-    trace_usb_uhci_mmio_readl(addr, val);
-    return val;
-}
-
 /* signal resume if controller suspended */
 static void uhci_resume (void *opaque)
 {
@@ -1236,18 +1194,14 @@ static void uhci_frame_timer(void *opaque)
     qemu_mod_timer(s->frame_timer, t_now + frame_t);
 }
 
-static const MemoryRegionPortio uhci_portio[] = {
-    { 0, 32, 2, .write = uhci_ioport_writew, },
-    { 0, 32, 2, .read = uhci_ioport_readw, },
-    { 0, 32, 4, .write = uhci_ioport_writel, },
-    { 0, 32, 4, .read = uhci_ioport_readl, },
-    { 0, 32, 1, .write = uhci_ioport_writeb, },
-    { 0, 32, 1, .read = uhci_ioport_readb, },
-    PORTIO_END_OF_LIST()
-};
-
 static const MemoryRegionOps uhci_ioport_ops = {
-    .old_portio = uhci_portio,
+    .read  = uhci_port_read,
+    .write = uhci_port_write,
+    .valid.min_access_size = 1,
+    .valid.max_access_size = 4,
+    .impl.min_access_size = 2,
+    .impl.max_access_size = 2,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static USBPortOps uhci_port_ops = {
diff --git a/trace-events b/trace-events
index 4b061c6..f68e03b 100644
--- a/trace-events
+++ b/trace-events
@@ -307,8 +307,6 @@ usb_uhci_frame_loop_stop_idle(void) ""
 usb_uhci_frame_loop_continue(void) ""
 usb_uhci_mmio_readw(uint32_t addr, uint32_t val) "addr 0x%04x, ret 0x%04x"
 usb_uhci_mmio_writew(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%04x"
-usb_uhci_mmio_readl(uint32_t addr, uint32_t val) "addr 0x%04x, ret 0x%08x"
-usb_uhci_mmio_writel(uint32_t addr, uint32_t val) "addr 0x%04x, val 0x%08x"
 usb_uhci_queue_add(uint32_t token) "token 0x%x"
 usb_uhci_queue_del(uint32_t token, const char *reason) "token 0x%x: %s"
 usb_uhci_packet_add(uint32_t token, uint32_t addr) "token 0x%x, td 0x%x"
-- 
1.7.1