[Qemu-devel] [PATCH 00/10] memory: remove old_portio usage

[Qemu-devel] [PATCH 00/10] 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.

Changes since RFC:
- fixed ppc/oldworld, ppc/newworld and sun4u machines
- removed bochs port changes, as they have been committed

Gerd Hoffmann (1):
  uhci: stop using portio lists

Hervé Poussineau (9):
  ppc/newworld: add ISA bus, required by VGA card
  ppc/oldworld: add ISA bus, required by VGA card
  sun4u: create VGA card after ISA bus
  xen_platform: 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/ppc_newworld.c     |    1 +
 hw/ppc_oldworld.c     |    1 +
 hw/qxl.c              |    4 +-
 hw/sun4u.c            |    3 +-
 hw/usb/hcd-uhci.c     |  106 ++++++++++++-----------------------------
 hw/vga.c              |    8 +---
 hw/xen_platform.c     |   21 ++++----
 include/exec/ioport.h |   19 --------
 include/exec/memory.h |    4 --
 ioport.c              |  121 ----------------------------------------------
 memory.c              |   44 -----------------
 trace-events          |    2 -
 15 files changed, 212 insertions(+), 342 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH 01/10] ppc/newworld: add ISA bus, required by VGA card

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/ppc_newworld.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index fabcc08..cd30f42 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -274,6 +274,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
     }
 
     /* Register 8 MB of ISA IO space */
+    isa_bus_new(NULL, get_system_io());
     isa_mmio_init(0xf2000000, 0x00800000);
 
     /* UniN init */
-- 
1.7.10.4

[Qemu-devel] [PATCH 02/10] ppc/oldworld: add ISA bus, required by VGA card

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/ppc_oldworld.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index fff5129..99eb22e 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -223,6 +223,7 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
     }
 
     /* Register 2 MB of ISA IO space */
+    isa_bus_new(NULL, get_system_io());
     isa_mmio_init(0xfe000000, 0x00200000);
 
     /* XXX: we register only 1 output pin for heathrow PIC */
-- 
1.7.10.4

[Qemu-devel] [PATCH 03/10] uhci: stop using portio lists

[Hervé Poussineau]

From: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Tested-by: Hervé Poussineau <hpoussin@reactos.org>
---
 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 2af754b..528d23f 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -455,40 +455,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) {
@@ -537,6 +508,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;
@@ -568,12 +550,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;
@@ -587,6 +568,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;
@@ -609,38 +599,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)
 {
@@ -1213,18 +1171,14 @@ 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 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 4023a4c..2183183 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.10.4

[Qemu-devel] [PATCH 04/10] sun4u: create VGA card after ISA bus

[Hervé Poussineau]

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/sun4u.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sun4u.c b/hw/sun4u.c
index cbfd217..c9150ad 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     ivec_irqs = qemu_allocate_irqs(cpu_set_ivec_irq, cpu, IVEC_MAX);
     pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_bus2,
                            &pci_bus3, &pbm_irqs);
-    pci_vga_init(pci_bus);
 
     // XXX Should be pci_bus3
     isa_bus = pci_ebus_init(pci_bus, -1, pbm_irqs);
 
+    pci_vga_init(pci_bus);
+
     i = 0;
     if (hwdef->console_serial_base) {
         serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
-- 
1.7.10.4

[Qemu-devel] [PATCH 05/10] 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 e7611bb..02e0146 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -279,7 +279,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);
@@ -288,30 +289,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] [PATCH 06/10] 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 06a8aca..63b41db 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -531,68 +531,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] [PATCH 07/10] 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 d08b9bd..8279a95 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 e2ba7f2..ffa9db5 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] [PATCH 08/10] 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 86b0bbd..bcf7cd4 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 62e89d3..c3b01ea 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] [PATCH 09/10] ioport: remove now useless portio_list_* functions

[Hervé Poussineau]

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

diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index fc28350..b02ddf7 100644
--- a/include/exec/ioport.h
+++ b/include/exec/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 */
diff --git a/ioport.c b/ioport.c
index a0ac2a0..a942a19 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;
-    }
-}
-- 
1.7.10.4

[Qemu-devel] [PATCH 10/10] memory: remove old_portio-style callbacks support

[Hervé Poussineau]

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

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..4a757f0 100644
--- a/include/exec/memory.h
+++ b/include/exec/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
      */
diff --git a/memory.c b/memory.c
index 410c5f8..e56ca3c 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,
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 03/10] uhci: stop using portio lists

[Andreas Färber]

Am 04.01.2013 22:29, schrieb Hervé Poussineau:
> From: Gerd Hoffmann <kraxel@redhat.com>
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Tested-by: Hervé Poussineau <hpoussin@reactos.org>

This is missing your Sob, but I believe it was merged through Gerd's USB
queue already.

Andreas

-- 
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] [PATCH 04/10] sun4u: create VGA card after ISA bus

[Andreas Färber]

Am 04.01.2013 22:29, schrieb Hervé Poussineau:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/sun4u.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10,
which switches to isa_register_portio_list().

Andreas

> 
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index cbfd217..c9150ad 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      ivec_irqs = qemu_allocate_irqs(cpu_set_ivec_irq, cpu, IVEC_MAX);
>      pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_bus2,
>                             &pci_bus3, &pbm_irqs);
> -    pci_vga_init(pci_bus);
>  
>      // XXX Should be pci_bus3
>      isa_bus = pci_ebus_init(pci_bus, -1, pbm_irqs);
>  
> +    pci_vga_init(pci_bus);
> +
>      i = 0;
>      if (hwdef->console_serial_base) {
>          serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
> 


-- 
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] [PATCH 10/10] memory: remove old_portio-style callbacks support

[Andreas Färber]

Am 04.01.2013 22:29, schrieb Hervé Poussineau:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  include/exec/memory.h |    4 ----
>  memory.c              |   44 --------------------------------------------
>  2 files changed, 48 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 2322732..4a757f0 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/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
>       */

Can't struct MemoryRegionPortio be removed as part of the patch?

Andreas

-- 
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] [PATCH 04/10] sun4u: create VGA card after ISA bus

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 04.01.2013 22:29, schrieb Hervé Poussineau:
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  hw/sun4u.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10,
> which switches to isa_register_portio_list().

Indeed.
ISA bus is created in pci_ebus_init(). VGA card registers some ISA I/O 
ports, so an ISA bus must be present.

Indeed.

> 
> Andreas
> 
>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>> index cbfd217..c9150ad 100644
>> --- a/hw/sun4u.c
>> +++ b/hw/sun4u.c
>> @@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>      ivec_irqs = qemu_allocate_irqs(cpu_set_ivec_irq, cpu, IVEC_MAX);
>>      pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs, &pci_bus2,
>>                             &pci_bus3, &pbm_irqs);
>> -    pci_vga_init(pci_bus);
>>  
>>      // XXX Should be pci_bus3
>>      isa_bus = pci_ebus_init(pci_bus, -1, pbm_irqs);
>>  
>> +    pci_vga_init(pci_bus);
>> +
>>      i = 0;
>>      if (hwdef->console_serial_base) {
>>          serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
>>
> 
> 

Re: [Qemu-devel] [PATCH 10/10] memory: remove old_portio-style callbacks support

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 04.01.2013 22:29, schrieb Hervé Poussineau:
>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>> ---
>>  include/exec/memory.h |    4 ----
>>  memory.c              |   44 --------------------------------------------
>>  2 files changed, 48 deletions(-)
>>
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 2322732..4a757f0 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/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
>>       */
> 
> Can't struct MemoryRegionPortio be removed as part of the patch?

No, it is also used as parameter for the isa_register_portio_list() 
function. isa_register_portio_list() parses this list and creates 
corresponding MemoryRegions.

Hervé

Re: [Qemu-devel] [PATCH 03/10] uhci: stop using portio lists

[Gerd Hoffmann]

On 01/10/13 18:18, Andreas Färber wrote:
> Am 04.01.2013 22:29, schrieb Hervé Poussineau:
>> From: Gerd Hoffmann <kraxel@redhat.com>
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> Tested-by: Hervé Poussineau <hpoussin@reactos.org>
> 
> This is missing your Sob, but I believe it was merged through Gerd's USB
> queue already.

Yes, it's in already via usb queue, git will drop it the next time you
rebase the patchset.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 04/10] sun4u: create VGA card after ISA bus

[Blue Swirl]

On Thu, Jan 10, 2013 at 7:30 PM, Hervé Poussineau <hpoussin@reactos.org> wrote:
> Andreas Färber a écrit :
>
>> Am 04.01.2013 22:29, schrieb Hervé Poussineau:
>>>
>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>> ---
>>>  hw/sun4u.c |    3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>>
>> Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10,
>> which switches to isa_register_portio_list().
>
>
> Indeed.
> ISA bus is created in pci_ebus_init(). VGA card registers some ISA I/O
> ports, so an ISA bus must be present.

OK by me.

>
> Indeed.
>
>
>>
>> Andreas
>>
>>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>>> index cbfd217..c9150ad 100644
>>> --- a/hw/sun4u.c
>>> +++ b/hw/sun4u.c
>>> @@ -825,11 +825,12 @@ static void sun4uv_init(MemoryRegion
>>> *address_space_mem,
>>>      ivec_irqs = qemu_allocate_irqs(cpu_set_ivec_irq, cpu, IVEC_MAX);
>>>      pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, ivec_irqs,
>>> &pci_bus2,
>>>                             &pci_bus3, &pbm_irqs);
>>> -    pci_vga_init(pci_bus);
>>>       // XXX Should be pci_bus3
>>>      isa_bus = pci_ebus_init(pci_bus, -1, pbm_irqs);
>>>  +    pci_vga_init(pci_bus);
>>> +
>>>      i = 0;
>>>      if (hwdef->console_serial_base) {
>>>          serial_mm_init(address_space_mem, hwdef->console_serial_base, 0,
>>>
>>
>>
>

Re: [Qemu-devel] [PATCH 04/10] sun4u: create VGA card after ISA bus

[Andreas Färber]

Am 12.01.2013 12:42, schrieb Blue Swirl:
> On Thu, Jan 10, 2013 at 7:30 PM, Hervé Poussineau <hpoussin@reactos.org> wrote:
>> Andreas Färber a écrit :
>>
>>> Am 04.01.2013 22:29, schrieb Hervé Poussineau:
>>>>
>>>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
>>>> ---
>>>>  hw/sun4u.c |    3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>>
>>> Doesn't look wrong to me, cc'ing Blue. I figure this is for 07/10,
>>> which switches to isa_register_portio_list().
>>
>>
>> Indeed.
>> ISA bus is created in pci_ebus_init(). VGA card registers some ISA I/O
>> ports, so an ISA bus must be present.
> 
> OK by me.

Thanks, applied to memory-ioport branch (with an additional explanatory
sentence in the commit message):
https://github.com/afaerber/qemu-cpu/commits/memory-ioport

Andreas

-- 
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] [PATCH 01/10] ppc/newworld: add ISA bus, required by VGA card

[Andreas Färber]

Am 04.01.2013 22:29, schrieb Hervé Poussineau:
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/ppc_newworld.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
> index fabcc08..cd30f42 100644
> --- a/hw/ppc_newworld.c
> +++ b/hw/ppc_newworld.c
> @@ -274,6 +274,7 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
>      }
>  
>      /* Register 8 MB of ISA IO space */
> +    isa_bus_new(NULL, get_system_io());
>      isa_mmio_init(0xf2000000, 0x00800000);
>  
>      /* UniN init */

I understand that a bus will be necessary for the ISA I/O ports, but I
do not like the bus added to the machine like this when at the same time
we are trying to QOM'ify the chipsets for IDE.

Where does this device logically sit? On UniNorth chipset?

Andreas

-- 
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] [PATCH 05/10] xen_platform: do not use old_portio-style callbacks

[Andreas Färber]

Am 04.01.2013 22:29, schrieb 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 e7611bb..02e0146 100644
> --- a/hw/xen_platform.c
> +++ b/hw/xen_platform.c
> @@ -279,7 +279,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);
> @@ -288,30 +289,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)

Thanks, applied to memory-ioport queue:
https://github.com/afaerber/qemu-cpu/commits/memory-ioport

Is there some kind of guidance in which cases to use .impl vs. .valid
for the access sizes in these conversions?

Andreas

-- 
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] [PATCH 06/10] acpi-piix4: do not use old_portio-style callbacks

[Andreas Färber]

Am 04.01.2013 22:29, schrieb 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 06a8aca..63b41db 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -531,68 +531,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;

Here you are still using a local uint32_t variable...

> +
> +    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);

...but here you are printing hwaddr using %x. Fixing it up as follows:

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 61aea8c..ee81c05 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -567,7 +567,8 @@ static void pci_write(void *opaque, hwaddr addr,
uint64_t data,
     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);
+        PIIX4_DPRINTF("pciej write %" HWADDR_PRIx " <== % " PRIu64 "\n",
+                      addr, data);
         break;
     default:
         break;

Thanks, applied to memory-ioport queue:
https://github.com/afaerber/qemu-cpu/commits/memory-ioport

Andreas

> +        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,

-- 
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] [PATCH 06/10] acpi-piix4: do not use old_portio-style callbacks

[Andreas Färber]

Am 04.01.2013 22:29, schrieb Hervé Poussineau:
>  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,

Ugh, question: Why is this changing Little Endian to native?

Andreas

>  };
>  
>  static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,

-- 
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] [PATCH 06/10] acpi-piix4: do not use old_portio-style callbacks

[Hervé Poussineau]

Andreas Färber a écrit :
> Am 04.01.2013 22:29, schrieb Hervé Poussineau:
>>  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,
> 
> Ugh, question: Why is this changing Little Endian to native?

An error.
However, I was unable to find in the spec if this field is native on 
little endian. Not that it changes anything for now, as x86 is little 
endian...
I will respin patches, except those already in your memory-ioport branch.

Hervé

Re: [Qemu-devel] [PATCH 08/10] isa: use memory regions instead of portio_list_* functions

[Andreas Färber]

Am 04.01.2013 22:29, schrieb 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 86b0bbd..bcf7cd4 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);

I note that patch 10/10 shows that memory.c does it similarly, assuming
Little Endian for size == 2 and ignoring assembled size == 4.

> +    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);

Ditto.

> +}
> +
> +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 = {

static const?

> +    .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 62e89d3..c3b01ea 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,

Otherwise looks okay to me, but I'd appreciate another pair of eyes.

AFAIU this moves the current implementation from ioport.c (09/10) and
memory.c (10/10) into isa-bus.c, to remove MemoryRegionOps::old_portio
in 10/10. But ISA keeps using MemoryRegionPortio unlike memory.c itself.

I wonder if it were not a cleaner course of action to remove
isa_register_portio_list() completely by converting callers to use
isa_register_ioport() with a MemoryRegion owned by the caller?
Remaining MemoryRegionPortio lists after this series below.

Regards,
Andreas


dma.c:
/* IOport from page_base */
static const MemoryRegionPortio page_portio_list[] = {
    { 0x01, 3, 1, .write = write_page, .read = read_page, },
    { 0x07, 1, 1, .write = write_page, .read = read_page, },
    PORTIO_END_OF_LIST(),
};

/* IOport from pageh_base */
static const MemoryRegionPortio pageh_portio_list[] = {
    { 0x01, 3, 1, .write = write_pageh, .read = read_pageh, },
    { 0x07, 3, 1, .write = write_pageh, .read = read_pageh, },
    PORTIO_END_OF_LIST(),
};

fdc.c:
static const MemoryRegionPortio fdc_portio_list[] = {
    { 1, 5, 1, .read = fdctrl_read, .write = fdctrl_write },
    { 7, 1, 1, .read = fdctrl_read, .write = fdctrl_write },
    PORTIO_END_OF_LIST(),
};

gus.c:
static const MemoryRegionPortio gus_portio_list1[] = {
    {0x000,  1, 1, .write = gus_writeb },
    {0x000,  1, 2, .write = gus_writew },
    {0x006, 10, 1, .read = gus_readb, .write = gus_writeb },
    {0x006, 10, 2, .read = gus_readw, .write = gus_writew },
    {0x100,  8, 1, .read = gus_readb, .write = gus_writeb },
    {0x100,  8, 2, .read = gus_readw, .write = gus_writew },
    PORTIO_END_OF_LIST (),
};

static const MemoryRegionPortio gus_portio_list2[] = {
    {0, 1, 1, .read = gus_readb },
    {0, 1, 2, .read = gus_readw },
    PORTIO_END_OF_LIST (),
};

ide/core.c:
static const MemoryRegionPortio ide_portio_list[] = {
    { 0, 8, 1, .read = ide_ioport_read, .write = ide_ioport_write },
    { 0, 2, 2, .read = ide_data_readw, .write = ide_data_writew },
    { 0, 4, 4, .read = ide_data_readl, .write = ide_data_writel },
    PORTIO_END_OF_LIST(),
};

static const MemoryRegionPortio ide_portio2_list[] = {
    { 0, 1, 1, .read = ide_status_read, .write = ide_cmd_write },
    PORTIO_END_OF_LIST(),
};

parallel.c:
static const MemoryRegionPortio isa_parallel_portio_hw_list[] = {
    { 0, 8, 1,
      .read = parallel_ioport_read_hw,
      .write = parallel_ioport_write_hw },
    { 4, 1, 2,
      .read = parallel_ioport_eppdata_read_hw2,
      .write = parallel_ioport_eppdata_write_hw2 },
    { 4, 1, 4,
      .read = parallel_ioport_eppdata_read_hw4,
      .write = parallel_ioport_eppdata_write_hw4 },
    { 0x400, 8, 1,
      .read = parallel_ioport_ecp_read,
      .write = parallel_ioport_ecp_write },
    PORTIO_END_OF_LIST(),
};

static const MemoryRegionPortio isa_parallel_portio_sw_list[] = {
    { 0, 8, 1,
      .read = parallel_ioport_read_sw,
      .write = parallel_ioport_write_sw },
    PORTIO_END_OF_LIST(),
};

qxl.c:
static const MemoryRegionPortio qxl_vga_portio_list[] = {
    { 0x04,  2, 1, .read  = vga_ioport_read,
                   .write = qxl_vga_ioport_write }, /* 3b4 */
    { 0x0a,  1, 1, .read  = vga_ioport_read,
                   .write = qxl_vga_ioport_write }, /* 3ba */
    { 0x10, 16, 1, .read  = vga_ioport_read,
                   .write = qxl_vga_ioport_write }, /* 3c0 */
    { 0x24,  2, 1, .read  = vga_ioport_read,
                   .write = qxl_vga_ioport_write }, /* 3d4 */
    { 0x2a,  1, 1, .read  = vga_ioport_read,
                   .write = qxl_vga_ioport_write }, /* 3da */
    PORTIO_END_OF_LIST(),
};

sb16.c:
static const MemoryRegionPortio sb16_ioport_list[] = {
    {  4, 1, 1, .write = mixer_write_indexb },
    {  4, 1, 2, .write = mixer_write_indexw },
    {  5, 1, 1, .read = mixer_read, .write = mixer_write_datab },
    {  6, 1, 1, .read = dsp_read, .write = dsp_write },
    { 10, 1, 1, .read = dsp_read },
    { 12, 1, 1, .write = dsp_write },
    { 12, 4, 1, .read = dsp_read },
    PORTIO_END_OF_LIST (),
};

vga.c:
static const MemoryRegionPortio vga_portio_list[] = {
    { 0x04,  2, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3b4 */
    { 0x0a,  1, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3ba */
    { 0x10, 16, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3c0 */
    { 0x24,  2, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3d4 */
    { 0x2a,  1, 1, .read = vga_ioport_read, .write = vga_ioport_write },
/* 3da */
    PORTIO_END_OF_LIST(),
};

static const MemoryRegionPortio vbe_portio_list[] = {
    { 0, 1, 2, .read = vbe_ioport_read_index, .write =
vbe_ioport_write_index },
# ifdef TARGET_I386
    { 1, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
# endif
    { 2, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
    PORTIO_END_OF_LIST(),
};

-- 
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] [PATCH 05/10] xen_platform: do not use old_portio-style callbacks

[Andreas Färber]

Stefano,

Am 12.01.2013 17:06, schrieb Andreas Färber:
> Am 04.01.2013 22:29, schrieb 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 e7611bb..02e0146 100644
>> --- a/hw/xen_platform.c
>> +++ b/hw/xen_platform.c
>> @@ -279,7 +279,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);
>> @@ -288,30 +289,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)
> 
> Thanks, applied to memory-ioport queue:
> https://github.com/afaerber/qemu-cpu/commits/memory-ioport

If you want to ack or object, now would be the time. :)
https://github.com/afaerber/qemu-cpu/commit/016861a22283da3931c4b343e6d2df0dc123b6ef

Cheers,
Andreas

> 
> Is there some kind of guidance in which cases to use .impl vs. .valid
> for the access sizes in these conversions?
> 
> Andreas

-- 
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] [PATCH 05/10] xen_platform: do not use old_portio-style callbacks

[Stefano Stabellini]

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

On Tue, 15 Jan 2013, Andreas Färber wrote:
> Stefano,
> 
> Am 12.01.2013 17:06, schrieb Andreas Färber:
> > Am 04.01.2013 22:29, schrieb 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 e7611bb..02e0146 100644
> >> --- a/hw/xen_platform.c
> >> +++ b/hw/xen_platform.c
> >> @@ -279,7 +279,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);
> >> @@ -288,30 +289,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)
> > 
> > Thanks, applied to memory-ioport queue:
> > https://github.com/afaerber/qemu-cpu/commits/memory-ioport
> 
> If you want to ack or object, now would be the time. :)
> https://github.com/afaerber/qemu-cpu/commit/016861a22283da3931c4b343e6d2df0dc123b6ef

That patch is also fine by me. Thanks for the heads up!