[Qemu-devel] [PATCH 00/16] isa_register_portio_list, v2

[Qemu-devel] [PATCH 00/16] isa_register_portio_list, v2

[Richard Henderson]

The problem that malc saw with sb16 was a major think-o on my part
with the whole interface.  We can't re-use the const sub-arrays of
the original MemoryRegionPortio array because they have the wrong
offset for the MemoryRegion to which it is attached -- the lookup
in find_portio fails.  We must adjust the offsets of the old_portio
array to be based against the MemoryRegion.

Which means we can easily eliminate the major complaint that came
with the previous round of comments -- the double PORTIO_END_OF_LIST
and the explicit marking of the ranges.  All we require of users
is that the array be sorted by offset.

The entire patch set is at

  git://repo.or.cz/qemu/rth.git mem-api-isa

and is of course based on

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

Avi, the first two patches are fixes for compile errors in your
tree.  They probably ought to be squashed when next you rebase.

Malc, I tested sb16 vs the xtc-play you provided.  However I failed
to test the gus changes; the xtc-play program could not recognize
the device with "--device gus" before or after the patch set.
Should I be using another set of command-line options for that?


r~


Richard Henderson (16):
  fixup: merge with last sm501 patch
  fixup: merge with last arm_sysctl patch
  isa: Tidy support code for isabus_get_fw_dev_path.
  isa: Add isa_register_portio_list().
  fdc: Convert to isa_register_portio_list.
  gus: Convert to isa_register_portio_list.
  m48t59: Convert to isa_register_ioport.
  rtc: Convert to isa_register_ioport.
  ne2000: Convert to isa_register_ioport.
  parallel: Convert to isa_register_portio_list.
  sb16: Convert to isa_register_portio_list.
  vga: Convert to isa_register_portio_list.
  pc: Convert port92 to isa_register_ioport.
  vmport: Convert to isa_register_ioport.
  ide: Convert to isa_register_portio_list.
  isa: Remove isa_init_ioport_range and isa_init_ioport.

 hw/arm_sysctl.c   |    1 -
 hw/devices.h      |    4 +-
 hw/fdc.c          |   34 ++----------------
 hw/gus.c          |   39 ++++++++++-----------
 hw/ide/core.c     |   30 ++++++++++------
 hw/ide/internal.h |    3 +-
 hw/ide/isa.c      |    4 +--
 hw/ide/piix.c     |    7 ++--
 hw/ide/via.c      |    7 ++--
 hw/isa-bus.c      |   99 +++++++++++++++++++++++++++++++++++++++++------------
 hw/isa.h          |   38 ++++++++++++++++----
 hw/m48t59.c       |   15 ++++++--
 hw/mc146818rtc.c  |   15 ++++++--
 hw/ne2000-isa.c   |    5 +--
 hw/parallel.c     |   47 +++++++++++++++----------
 hw/pc.c           |   16 +++++++--
 hw/sb16.c         |   32 +++++++----------
 hw/vga-isa.c      |   10 -----
 hw/vga.c          |   55 +++++++++++++++--------------
 hw/vmport.c       |   16 +++++++--
 memory.c          |    8 ++--
 21 files changed, 287 insertions(+), 198 deletions(-)

-- 
1.7.4.4

[Qemu-devel] [PATCH 01/16] fixup: merge with last sm501 patch

[Richard Henderson]

---
 hw/devices.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/devices.h b/hw/devices.h
index 36c21e7..8ac384f 100644
--- a/hw/devices.h
+++ b/hw/devices.h
@@ -2,7 +2,7 @@
 #define QEMU_DEVICES_H
 
 /* ??? Not all users of this file can include cpu-common.h.  */
-typedef struct MemoryRegion MemoryRegion;
+struct MemoryRegion;
 
 /* Devices that have nowhere better to go.  */
 
@@ -60,7 +60,7 @@ qemu_irq *tc6393xb_gpio_in_get(TC6393xbState *s);
 qemu_irq tc6393xb_l3v_get(TC6393xbState *s);
 
 /* sm501.c */
-void sm501_init(MemoryRegion *address_space_mem, uint32_t base,
+void sm501_init(struct MemoryRegion *address_space_mem, uint32_t base,
                 uint32_t local_mem_bytes, qemu_irq irq,
                 CharDriverState *chr);
 
-- 
1.7.4.4

[Qemu-devel] [PATCH 02/16] fixup: merge with last arm_sysctl patch

[Richard Henderson]

---
 hw/arm_sysctl.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
index 5b845a2..17cf6f7 100644
--- a/hw/arm_sysctl.c
+++ b/hw/arm_sysctl.c
@@ -19,7 +19,6 @@ typedef struct {
     SysBusDevice busdev;
     MemoryRegion iomem;
     qemu_irq pl110_mux_ctrl;
-    MemoryRegion iomem;
 
     uint32_t sys_id;
     uint32_t leds;
-- 
1.7.4.4

[Qemu-devel] [PATCH 03/16] isa: Tidy support code for isabus_get_fw_dev_path.

[Richard Henderson]

The only user of ISADevice.ioports is isabus_get_fw_dev_path, and it
only looks at the first entry of the array.  Which suggests that this
entire array+sort operation can be replaced by a simple minimum.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/isa-bus.c |   25 +++++--------------------
 hw/isa.h     |    5 +----
 2 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 6c15a31..e9c1712 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -83,24 +83,11 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
     dev->nirqs++;
 }
 
-static void isa_init_ioport_one(ISADevice *dev, uint16_t ioport)
-{
-    assert(dev->nioports < ARRAY_SIZE(dev->ioports));
-    dev->ioports[dev->nioports++] = ioport;
-}
-
-static int isa_cmp_ports(const void *p1, const void *p2)
-{
-    return *(uint16_t*)p1 - *(uint16_t*)p2;
-}
-
 void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
 {
-    int i;
-    for (i = start; i < start + length; i++) {
-        isa_init_ioport_one(dev, i);
+    if (dev->ioport_id == 0 || start < dev->ioport_id) {
+        dev->ioport_id = start;
     }
-    qsort(dev->ioports, dev->nioports, sizeof(dev->ioports[0]), isa_cmp_ports);
 }
 
 void isa_init_ioport(ISADevice *dev, uint16_t ioport)
@@ -112,9 +99,7 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)
 {
     memory_region_add_subregion(isabus->address_space_io, start, io);
     if (dev != NULL) {
-        assert(dev->nio < ARRAY_SIZE(dev->io));
-        dev->io[dev->nio++] = io;
-        isa_init_ioport_range(dev, start, memory_region_size(io));
+        isa_init_ioport(dev, start);
     }
 }
 
@@ -208,8 +193,8 @@ static char *isabus_get_fw_dev_path(DeviceState *dev)
     int off;
 
     off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
-    if (d->nioports) {
-        snprintf(path + off, sizeof(path) - off, "@%04x", d->ioports[0]);
+    if (d->ioport_id) {
+        snprintf(path + off, sizeof(path) - off, "@%04x", d->ioport_id);
     }
 
     return strdup(path);
diff --git a/hw/isa.h b/hw/isa.h
index 432d17a..c5c2618 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -13,12 +13,9 @@ typedef struct ISADeviceInfo ISADeviceInfo;
 
 struct ISADevice {
     DeviceState qdev;
-    MemoryRegion *io[32];
     uint32_t isairq[2];
-    uint16_t ioports[32];
     int nirqs;
-    int nioports;
-    int nio;
+    int ioport_id;
 };
 
 typedef int (*isa_qdev_initfn)(ISADevice *dev);
-- 
1.7.4.4

[Qemu-devel] [PATCH 04/16] isa: Add isa_register_portio_list().

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/isa-bus.c |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/isa.h     |   31 ++++++++++++++++++++++-
 memory.c     |    8 +++---
 3 files changed, 113 insertions(+), 5 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index e9c1712..648b421 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -103,6 +103,85 @@ void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)
     }
 }
 
+static void isa_register_portio_1(ISADevice *dev,
+                                  const MemoryRegionPortio *pio_init,
+                                  unsigned count, unsigned start,
+                                  unsigned off_low, unsigned off_high,
+                                  void *opaque, const char *name)
+{
+    MemoryRegionPortio *pio;
+    MemoryRegionOps *ops;
+    MemoryRegion *region;
+    unsigned i;
+
+    if (off_low == 0 && pio_init[count].size == 0) {
+        /* Special case simple adjustments.  */
+        pio = (MemoryRegionPortio *) pio_init;
+    } else {
+        /* 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);
+    memory_region_init_io(region, ops, opaque, name, off_high - off_low);
+    memory_region_set_offset(region, start + off_low);
+    memory_region_add_subregion(isabus->address_space_io,
+                                start + off_low, region);
+}
+
+void isa_register_portio_list(ISADevice *dev, uint16_t start,
+                              const MemoryRegionPortio *pio_start,
+                              void *opaque, const char *name)
+{
+    const MemoryRegionPortio *pio;
+    unsigned int off_low, off_high, off_last, count;
+
+    /* 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.  */
+    if (dev) {
+        isa_init_ioport(dev, start);
+    }
+
+    /* 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) {
+            isa_register_portio_1(dev, pio_start, count, start, off_low,
+                                  off_high, opaque, name);
+            /* ... 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.  */
+    isa_register_portio_1(dev, pio_start, count, start, off_low,
+                          off_high, opaque, name);
+}
+
 static int isa_qdev_init(DeviceState *qdev, DeviceInfo *base)
 {
     ISADevice *dev = DO_UPCAST(ISADevice, qdev, qdev);
diff --git a/hw/isa.h b/hw/isa.h
index c5c2618..177ef95 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -28,7 +28,6 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_get_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
-void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start);
 void isa_init_ioport(ISADevice *dev, uint16_t ioport);
 void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
 void isa_qdev_register(ISADeviceInfo *info);
@@ -37,6 +36,36 @@ ISADevice *isa_create(const char *name);
 ISADevice *isa_try_create(const char *name);
 ISADevice *isa_create_simple(const char *name);
 
+/**
+ * isa_register_ioport: Install an I/O port region on the ISA bus.
+ *
+ * Register an I/O port region via memory_region_add_subregion
+ * inside the ISA I/O address space.
+ *
+ * @dev: the ISADevice against which these are registered; may be NULL.
+ * @io: the #MemoryRegion being registered.
+ * @start: the base I/O port.
+ */
+void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start);
+
+/**
+ * isa_register_portio_list: Initialize a set of ISA io ports
+ *
+ * Several ISA devices have many dis-joint I/O ports.  Worse, these I/O
+ * ports can be interleaved with I/O ports from other devices.  This
+ * function makes it easy to create multiple MemoryRegions for a single
+ * device and use the legacy portio routines.
+ *
+ * @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.
+ * @name: passed into memory_region_init_io.
+ */
+void isa_register_portio_list(ISADevice *dev, uint16_t start,
+                              const MemoryRegionPortio *portio,
+                              void *opaque, const char *name);
+
 extern target_phys_addr_t isa_mem_base;
 
 void isa_mmio_setup(MemoryRegion *mr, target_phys_addr_t size);
diff --git a/memory.c b/memory.c
index 8e9ac46..6fc53b8 100644
--- a/memory.c
+++ b/memory.c
@@ -396,12 +396,12 @@ static void memory_region_iorange_read(IORange *iorange,
 
         *data = ((uint64_t)1 << (width * 8)) - 1;
         if (mrp) {
-            *data = mrp->read(mr->opaque, offset);
+            *data = mrp->read(mr->opaque, offset + mr->offset);
         }
         return;
     }
     *data = 0;
-    access_with_adjusted_size(offset, data, width,
+    access_with_adjusted_size(offset + mr->offset, data, width,
                               mr->ops->impl.min_access_size,
                               mr->ops->impl.max_access_size,
                               memory_region_read_accessor, mr);
@@ -418,11 +418,11 @@ static void memory_region_iorange_write(IORange *iorange,
         const MemoryRegionPortio *mrp = find_portio(mr, offset, width, true);
 
         if (mrp) {
-            mrp->write(mr->opaque, offset, data);
+            mrp->write(mr->opaque, offset + mr->offset, data);
         }
         return;
     }
-    access_with_adjusted_size(offset, &data, width,
+    access_with_adjusted_size(offset + mr->offset, &data, width,
                               mr->ops->impl.min_access_size,
                               mr->ops->impl.max_access_size,
                               memory_region_write_accessor, mr);
-- 
1.7.4.4

[Qemu-devel] [PATCH 05/16] fdc: Convert to isa_register_portio_list.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/fdc.c |   34 ++++------------------------------
 1 files changed, 4 insertions(+), 30 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index c3ae956..b83c045 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -425,7 +425,6 @@ typedef struct FDCtrlSysBus {
 
 typedef struct FDCtrlISABus {
     ISADevice busdev;
-    MemoryRegion io_0, io_7;
     struct FDCtrl state;
     int32_t bootindexA;
     int32_t bootindexB;
@@ -1882,32 +1881,10 @@ static int fdctrl_init_common(FDCtrl *fdctrl)
     return fdctrl_connect_drives(fdctrl);
 }
 
-static uint32_t fdctrl_read_port_7(void *opaque, uint32_t reg)
-{
-    return fdctrl_read(opaque, reg + 7);
-}
-
-static void fdctrl_write_port_7(void *opaque, uint32_t reg, uint32_t value)
-{
-    fdctrl_write(opaque, reg + 7, value);
-}
-
-static const MemoryRegionPortio fdc_portio_0[] = {
+static const MemoryRegionPortio fdc_portio_list[] = {
     { 1, 5, 1, .read = fdctrl_read, .write = fdctrl_write },
-    PORTIO_END_OF_LIST()
-};
-
-static const MemoryRegionPortio fdc_portio_7[] = {
-    { 0, 1, 1, .read = fdctrl_read_port_7, .write = fdctrl_write_port_7 },
-    PORTIO_END_OF_LIST()
-};
-
-static const MemoryRegionOps fdc_ioport_0_ops = {
-    .old_portio = fdc_portio_0
-};
-
-static const MemoryRegionOps fdc_ioport_7_ops = {
-    .old_portio = fdc_portio_7
+    { 7, 1, 1, .read = fdctrl_read, .write = fdctrl_write },
+    PORTIO_END_OF_LIST(),
 };
 
 static int isabus_fdc_init1(ISADevice *dev)
@@ -1919,10 +1896,7 @@ static int isabus_fdc_init1(ISADevice *dev)
     int dma_chann = 2;
     int ret;
 
-    memory_region_init_io(&isa->io_0, &fdc_ioport_0_ops, fdctrl, "fdc", 6);
-    memory_region_init_io(&isa->io_7, &fdc_ioport_7_ops, fdctrl, "fdc", 1);
-    isa_register_ioport(dev, &isa->io_0, iobase);
-    isa_register_ioport(dev, &isa->io_7, iobase + 7);
+    isa_register_portio_list(dev, iobase, fdc_portio_list, fdctrl, "fdc");
 
     isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
     fdctrl->dma_chann = dma_chann;
-- 
1.7.4.4

[Qemu-devel] [PATCH 06/16] gus: Convert to isa_register_portio_list.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/gus.c |   39 +++++++++++++++++++--------------------
 1 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/hw/gus.c b/hw/gus.c
index 37e543a..1532686 100644
--- a/hw/gus.c
+++ b/hw/gus.c
@@ -232,6 +232,22 @@ static const VMStateDescription vmstate_gus = {
     }
 };
 
+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(),
+};
+
 static int gus_initfn (ISADevice *dev)
 {
     GUSState *s = DO_UPCAST(GUSState, dev, dev);
@@ -262,26 +278,9 @@ static int gus_initfn (ISADevice *dev)
     s->samples = AUD_get_buffer_size_out (s->voice) >> s->shift;
     s->mixbuf = g_malloc0 (s->samples << s->shift);
 
-    register_ioport_write (s->port, 1, 1, gus_writeb, s);
-    register_ioport_write (s->port, 1, 2, gus_writew, s);
-    isa_init_ioport_range(dev, s->port, 2);
-
-    register_ioport_read ((s->port + 0x100) & 0xf00, 1, 1, gus_readb, s);
-    register_ioport_read ((s->port + 0x100) & 0xf00, 1, 2, gus_readw, s);
-    isa_init_ioport_range(dev, (s->port + 0x100) & 0xf00, 2);
-
-    register_ioport_write (s->port + 6, 10, 1, gus_writeb, s);
-    register_ioport_write (s->port + 6, 10, 2, gus_writew, s);
-    register_ioport_read (s->port + 6, 10, 1, gus_readb, s);
-    register_ioport_read (s->port + 6, 10, 2, gus_readw, s);
-    isa_init_ioport_range(dev, s->port + 6, 10);
-
-
-    register_ioport_write (s->port + 0x100, 8, 1, gus_writeb, s);
-    register_ioport_write (s->port + 0x100, 8, 2, gus_writew, s);
-    register_ioport_read (s->port + 0x100, 8, 1, gus_readb, s);
-    register_ioport_read (s->port + 0x100, 8, 2, gus_readw, s);
-    isa_init_ioport_range(dev, s->port + 0x100, 8);
+    isa_register_portio_list(dev, s->port, gus_portio_list1, s, "gus");
+    isa_register_portio_list(dev, (s->port + 0x100) & 0xf00,
+                             gus_portio_list2, s, "gus");
 
     DMA_register_channel (s->emu.gusdma, GUS_read_DMA, s);
     s->emu.himemaddr = s->himem;
-- 
1.7.4.4

[Qemu-devel] [PATCH 07/16] m48t59: Convert to isa_register_ioport.

[Richard Henderson]

The sysbus interface is as yet unconverted.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/m48t59.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/m48t59.c b/hw/m48t59.c
index 0cc361e..f318e67 100644
--- a/hw/m48t59.c
+++ b/hw/m48t59.c
@@ -73,6 +73,7 @@ struct M48t59State {
 typedef struct M48t59ISAState {
     ISADevice busdev;
     M48t59State state;
+    MemoryRegion io;
 } M48t59ISAState;
 
 typedef struct M48t59SysBusState {
@@ -626,6 +627,15 @@ static void m48t59_reset_sysbus(DeviceState *d)
     m48t59_reset_common(NVRAM);
 }
 
+static const MemoryRegionPortio m48t59_portio[] = {
+    {0, 4, 1, .read = NVRAM_readb, .write = NVRAM_writeb },
+    PORTIO_END_OF_LIST(),
+};
+
+static const MemoryRegionOps m48t59_io_ops = {
+    .old_portio = m48t59_portio,
+};
+
 /* Initialisation routine */
 M48t59State *m48t59_init(qemu_irq IRQ, target_phys_addr_t mem_base,
                          uint32_t io_base, uint16_t size, int type)
@@ -669,10 +679,9 @@ M48t59State *m48t59_init_isa(uint32_t io_base, uint16_t size, int type)
     d = DO_UPCAST(M48t59ISAState, busdev, dev);
     s = &d->state;
 
+    memory_region_init_io(&d->io, &m48t59_io_ops, s, "m48t59", 4);
     if (io_base != 0) {
-        register_ioport_read(io_base, 0x04, 1, NVRAM_readb, s);
-        register_ioport_write(io_base, 0x04, 1, NVRAM_writeb, s);
-        isa_init_ioport_range(dev, io_base, 4);
+        isa_register_ioport(dev, &d->io, io_base);
     }
 
     return s;
-- 
1.7.4.4

[Qemu-devel] [PATCH 08/16] rtc: Convert to isa_register_ioport.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/mc146818rtc.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index feb3b25..2aaca2f 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -81,6 +81,7 @@
 
 typedef struct RTCState {
     ISADevice dev;
+    MemoryRegion io;
     uint8_t cmos_data[128];
     uint8_t cmos_index;
     struct tm current_tm;
@@ -604,6 +605,15 @@ static void rtc_reset(void *opaque)
 #endif
 }
 
+static const MemoryRegionPortio cmos_portio[] = {
+    {0, 2, 1, .read = cmos_ioport_read, .write = cmos_ioport_write },
+    PORTIO_END_OF_LIST(),
+};
+
+static const MemoryRegionOps cmos_ops = {
+    .old_portio = cmos_portio
+};
+
 static int rtc_initfn(ISADevice *dev)
 {
     RTCState *s = DO_UPCAST(RTCState, dev, dev);
@@ -632,9 +642,8 @@ static int rtc_initfn(ISADevice *dev)
         qemu_get_clock_ns(rtc_clock) + (get_ticks_per_sec() * 99) / 100;
     qemu_mod_timer(s->second_timer2, s->next_second_time);
 
-    register_ioport_write(base, 2, 1, cmos_ioport_write, s);
-    register_ioport_read(base, 2, 1, cmos_ioport_read, s);
-    isa_init_ioport_range(dev, base, 2);
+    memory_region_init_io(&s->io, &cmos_ops, s, "rtc", 2);
+    isa_register_ioport(dev, &s->io, base);
 
     qdev_set_legacy_instance_id(&dev->qdev, base, 2);
     qemu_register_reset(rtc_reset, s);
-- 
1.7.4.4

[Qemu-devel] [PATCH 09/16] ne2000: Convert to isa_register_ioport.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/ne2000-isa.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/hw/ne2000-isa.c b/hw/ne2000-isa.c
index 756ed5c..11ffee7 100644
--- a/hw/ne2000-isa.c
+++ b/hw/ne2000-isa.c
@@ -68,10 +68,7 @@ static int isa_ne2000_initfn(ISADevice *dev)
     NE2000State *s = &isa->ne2000;
 
     ne2000_setup_io(s, 0x20);
-    isa_init_ioport_range(dev, isa->iobase, 16);
-    isa_init_ioport_range(dev, isa->iobase + 0x10, 2);
-    isa_init_ioport(dev, isa->iobase + 0x1f);
-    memory_region_add_subregion(get_system_io(), isa->iobase, &s->io);
+    isa_register_ioport(dev, &s->io, isa->iobase);
 
     isa_init_irq(dev, &s->irq, isa->isairq);
 
-- 
1.7.4.4

[Qemu-devel] [PATCH 10/16] parallel: Convert to isa_register_portio_list.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/parallel.c |   47 ++++++++++++++++++++++++++++-------------------
 1 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/hw/parallel.c b/hw/parallel.c
index ecbc8c3..8494d94 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -448,6 +448,29 @@ static void parallel_reset(void *opaque)
 
 static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 
+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(),
+};
+
 static int parallel_isa_initfn(ISADevice *dev)
 {
     static int index;
@@ -478,25 +501,11 @@ static int parallel_isa_initfn(ISADevice *dev)
         s->status = dummy;
     }
 
-    if (s->hw_driver) {
-        register_ioport_write(base, 8, 1, parallel_ioport_write_hw, s);
-        register_ioport_read(base, 8, 1, parallel_ioport_read_hw, s);
-        isa_init_ioport_range(dev, base, 8);
-
-        register_ioport_write(base+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
-        register_ioport_read(base+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
-        register_ioport_write(base+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
-        register_ioport_read(base+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
-        isa_init_ioport(dev, base+4);
-        register_ioport_write(base+0x400, 8, 1, parallel_ioport_ecp_write, s);
-        register_ioport_read(base+0x400, 8, 1, parallel_ioport_ecp_read, s);
-        isa_init_ioport_range(dev, base+0x400, 8);
-    }
-    else {
-        register_ioport_write(base, 8, 1, parallel_ioport_write_sw, s);
-        register_ioport_read(base, 8, 1, parallel_ioport_read_sw, s);
-        isa_init_ioport_range(dev, base, 8);
-    }
+    isa_register_portio_list(dev, base,
+                             (s->hw_driver
+                              ? &isa_parallel_portio_hw_list[0]
+                              : &isa_parallel_portio_sw_list[0]),
+                             s, "parallel");
     return 0;
 }
 
-- 
1.7.4.4

[Qemu-devel] [PATCH 11/16] sb16: Convert to isa_register_portio_list.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/sb16.c |   32 +++++++++++++-------------------
 1 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/hw/sb16.c b/hw/sb16.c
index a76df1b..fe927e2 100644
--- a/hw/sb16.c
+++ b/hw/sb16.c
@@ -1341,12 +1341,21 @@ static const VMStateDescription vmstate_sb16 = {
     }
 };
 
+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(),
+};
+
+
 static int sb16_initfn (ISADevice *dev)
 {
-    static const uint8_t dsp_write_ports[] = {0x6, 0xc};
-    static const uint8_t dsp_read_ports[] = {0x6, 0xa, 0xc, 0xd, 0xe, 0xf};
     SB16State *s;
-    int i;
 
     s = DO_UPCAST (SB16State, dev, dev);
 
@@ -1366,22 +1375,7 @@ static int sb16_initfn (ISADevice *dev)
         dolog ("warning: Could not create auxiliary timer\n");
     }
 
-    for (i = 0; i < ARRAY_SIZE (dsp_write_ports); i++) {
-        register_ioport_write (s->port + dsp_write_ports[i], 1, 1, dsp_write, s);
-        isa_init_ioport(dev, s->port + dsp_write_ports[i]);
-    }
-
-    for (i = 0; i < ARRAY_SIZE (dsp_read_ports); i++) {
-        register_ioport_read (s->port + dsp_read_ports[i], 1, 1, dsp_read, s);
-        isa_init_ioport(dev, s->port + dsp_read_ports[i]);
-    }
-
-    register_ioport_write (s->port + 0x4, 1, 1, mixer_write_indexb, s);
-    register_ioport_write (s->port + 0x4, 1, 2, mixer_write_indexw, s);
-    isa_init_ioport(dev, s->port + 0x4);
-    register_ioport_read (s->port + 0x5, 1, 1, mixer_read, s);
-    register_ioport_write (s->port + 0x5, 1, 1, mixer_write_datab, s);
-    isa_init_ioport(dev, s->port + 0x5);
+    isa_register_portio_list(dev, s->port, sb16_ioport_list, s, "sb16");
 
     DMA_register_channel (s->hdma, SB_read_DMA, s);
     DMA_register_channel (s->dma, SB_read_DMA, s);
-- 
1.7.4.4

[Qemu-devel] [PATCH 12/16] vga: Convert to isa_register_portio_list.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/vga-isa.c |   10 ----------
 hw/vga.c     |   55 ++++++++++++++++++++++++++++---------------------------
 2 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/hw/vga-isa.c b/hw/vga-isa.c
index 0d19901..510ace8 100644
--- a/hw/vga-isa.c
+++ b/hw/vga-isa.c
@@ -54,16 +54,6 @@ static int vga_initfn(ISADevice *dev)
                                         isa_mem_base + 0x000a0000,
                                         vga_io_memory, 1);
     memory_region_set_coalescing(vga_io_memory);
-    isa_init_ioport(dev, 0x3c0);
-    isa_init_ioport(dev, 0x3b4);
-    isa_init_ioport(dev, 0x3ba);
-    isa_init_ioport(dev, 0x3da);
-    isa_init_ioport(dev, 0x3c0);
-#ifdef CONFIG_BOCHS_VBE
-    isa_init_ioport(dev, 0x1ce);
-    isa_init_ioport(dev, 0x1cf);
-    isa_init_ioport(dev, 0x1d0);
-#endif /* CONFIG_BOCHS_VBE */
     s->ds = graphic_console_init(s->update, s->invalidate,
                                  s->screen_dump, s->text_update, s);
 
diff --git a/hw/vga.c b/hw/vga.c
index b0371d5..7a0dedd 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2198,40 +2198,41 @@ void vga_common_init(VGACommonState *s, int vga_ram_size)
     vga_dirty_log_start(s);
 }
 
-/* used by both ISA and PCI */
+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 */
+    { 0x14,  2, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3d4 */
+    { 0x1a,  1, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3da */
+    PORTIO_END_OF_LIST(),
+};
+
+#ifdef CONFIG_BOCHS_VBE
+static const MemoryRegionPortio vbe_portio_list[] = {
+# ifdef TARGET_I386
+    { 0, 1, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
+    { 1, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
+# else
+    { 0, 2, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
+    { 2, 2, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
+# endif
+    PORTIO_END_OF_LIST(),
+};
+#endif /* CONFIG_BOCHS_VBE */
+
+/* Used by both ISA and PCI */
 MemoryRegion *vga_init_io(VGACommonState *s)
 {
     MemoryRegion *vga_mem;
 
-    register_ioport_write(0x3c0, 16, 1, vga_ioport_write, s);
-
-    register_ioport_write(0x3b4, 2, 1, vga_ioport_write, s);
-    register_ioport_write(0x3d4, 2, 1, vga_ioport_write, s);
-    register_ioport_write(0x3ba, 1, 1, vga_ioport_write, s);
-    register_ioport_write(0x3da, 1, 1, vga_ioport_write, s);
-
-    register_ioport_read(0x3c0, 16, 1, vga_ioport_read, s);
-
-    register_ioport_read(0x3b4, 2, 1, vga_ioport_read, s);
-    register_ioport_read(0x3d4, 2, 1, vga_ioport_read, s);
-    register_ioport_read(0x3ba, 1, 1, vga_ioport_read, s);
-    register_ioport_read(0x3da, 1, 1, vga_ioport_read, s);
+    /* The PCI-ISA bridge should have been configured properly such that
+       this works for PCI devices as well.  This only supports one bridge,
+       but "secondary" VGA cards are generally accessed by MMIO only anyway.  */
+    isa_register_portio_list(NULL, 0x3b0, vga_portio_list, s, "vga");
 
 #ifdef CONFIG_BOCHS_VBE
-#if defined (TARGET_I386)
-    register_ioport_read(0x1ce, 1, 2, vbe_ioport_read_index, s);
-    register_ioport_read(0x1cf, 1, 2, vbe_ioport_read_data, s);
-
-    register_ioport_write(0x1ce, 1, 2, vbe_ioport_write_index, s);
-    register_ioport_write(0x1cf, 1, 2, vbe_ioport_write_data, s);
-#else
-    register_ioport_read(0x1ce, 1, 2, vbe_ioport_read_index, s);
-    register_ioport_read(0x1d0, 1, 2, vbe_ioport_read_data, s);
-
-    register_ioport_write(0x1ce, 1, 2, vbe_ioport_write_index, s);
-    register_ioport_write(0x1d0, 1, 2, vbe_ioport_write_data, s);
+    isa_register_portio_list(NULL, 0x1ce, vbe_portio_list, s, "vbe");
 #endif
-#endif /* CONFIG_BOCHS_VBE */
 
     vga_mem = g_malloc(sizeof(*vga_mem));
     memory_region_init_io(vga_mem, &vga_mem_ops, s,
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH 12/16] vga: Convert to isa_register_portio_list.

[Avi Kivity]

On 08/24/2011 03:13 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson<rth@twiddle.net>

Breaks qemu-system-ppc -M mac99

> +/* Used by both ISA and PCI */
>   MemoryRegion *vga_init_io(VGACommonState *s)
>   {
>       MemoryRegion *vga_mem;
>
> -    register_ioport_write(0x3c0, 16, 1, vga_ioport_write, s);
> -
> -    register_ioport_write(0x3b4, 2, 1, vga_ioport_write, s);
> -    register_ioport_write(0x3d4, 2, 1, vga_ioport_write, s);
> -    register_ioport_write(0x3ba, 1, 1, vga_ioport_write, s);
> -    register_ioport_write(0x3da, 1, 1, vga_ioport_write, s);
> -
> -    register_ioport_read(0x3c0, 16, 1, vga_ioport_read, s);
> -
> -    register_ioport_read(0x3b4, 2, 1, vga_ioport_read, s);
> -    register_ioport_read(0x3d4, 2, 1, vga_ioport_read, s);
> -    register_ioport_read(0x3ba, 1, 1, vga_ioport_read, s);
> -    register_ioport_read(0x3da, 1, 1, vga_ioport_read, s);
> +    /* The PCI-ISA bridge should have been configured properly such that
> +       this works for PCI devices as well.  This only supports one bridge,
> +       but "secondary" VGA cards are generally accessed by MMIO only anyway.  */
> +    isa_register_portio_list(NULL, 0x3b0, vga_portio_list, s, "vga");
>
>       memory_region_init_io(vga_mem,&vga_mem_ops, s,

This is called even for pci machines which have no ISA bus (and even if 
they did, the code should work wit the pci bus, not ISA).  The code 
should return the portio list of the caller to register, or perhaps 
accept a callback to do the registration.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 12/16] vga: Convert to isa_register_portio_list.

[Richard Henderson]

On 09/18/2011 06:45 AM, Avi Kivity wrote:
>> +    /* The PCI-ISA bridge should have been configured properly such that
>> +       this works for PCI devices as well.  This only supports one bridge,
>> +       but "secondary" VGA cards are generally accessed by MMIO only anyway.  */
>> +    isa_register_portio_list(NULL, 0x3b0, vga_portio_list, s, "vga");
>>
>>       memory_region_init_io(vga_mem,&vga_mem_ops, s,
> 
> This is called even for pci machines which have no ISA bus (and even
> if they did, the code should work wit the pci bus, not ISA). The code
> should return the portio list of the caller to register, or perhaps
> accept a callback to do the registration.

You're over-thinking this.  It's all legacy ISA crap full stop.
If the machine doesn't have a PCI-ISA bridge, then the machine will
also be prepared to access the VGA registers via its BARs.

In such a case we just should skip this entire section.  Probably
isa_register_portio_list should simply notice no ISA bus has been
registered and do nothing.


r~

Re: [Qemu-devel] [PATCH 12/16] vga: Convert to isa_register_portio_list.

[Avi Kivity]

On 09/18/2011 05:16 PM, Richard Henderson wrote:
> On 09/18/2011 06:45 AM, Avi Kivity wrote:
> >>  +    /* The PCI-ISA bridge should have been configured properly such that
> >>  +       this works for PCI devices as well.  This only supports one bridge,
> >>  +       but "secondary" VGA cards are generally accessed by MMIO only anyway.  */
> >>  +    isa_register_portio_list(NULL, 0x3b0, vga_portio_list, s, "vga");
> >>
> >>        memory_region_init_io(vga_mem,&vga_mem_ops, s,
> >
> >  This is called even for pci machines which have no ISA bus (and even
> >  if they did, the code should work wit the pci bus, not ISA). The code
> >  should return the portio list of the caller to register, or perhaps
> >  accept a callback to do the registration.
>
> You're over-thinking this.  It's all legacy ISA crap full stop.
> If the machine doesn't have a PCI-ISA bridge, then the machine will
> also be prepared to access the VGA registers via its BARs.
>
> In such a case we just should skip this entire section.  Probably
> isa_register_portio_list should simply notice no ISA bus has been
> registered and do nothing.

Depends, if it doesn't need those ports, then vga_init_io() can be 
passed a parameter not to register them, or perhaps it can be split into 
two.

But is this the case? Alex?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 12/16] vga: Convert to isa_register_portio_list.

[Avi Kivity]

On 09/18/2011 05:27 PM, Avi Kivity wrote:
> On 09/18/2011 05:16 PM, Richard Henderson wrote:
>> On 09/18/2011 06:45 AM, Avi Kivity wrote:
>> >>  +    /* The PCI-ISA bridge should have been configured properly 
>> such that
>> >>  +       this works for PCI devices as well.  This only supports 
>> one bridge,
>> >>  +       but "secondary" VGA cards are generally accessed by MMIO 
>> only anyway.  */
>> >>  +    isa_register_portio_list(NULL, 0x3b0, vga_portio_list, s, 
>> "vga");
>> >>
>> >>        memory_region_init_io(vga_mem,&vga_mem_ops, s,
>> >
>> >  This is called even for pci machines which have no ISA bus (and even
>> >  if they did, the code should work wit the pci bus, not ISA). The code
>> >  should return the portio list of the caller to register, or perhaps
>> >  accept a callback to do the registration.
>>
>> You're over-thinking this.  It's all legacy ISA crap full stop.
>> If the machine doesn't have a PCI-ISA bridge, then the machine will
>> also be prepared to access the VGA registers via its BARs.
>>
>> In such a case we just should skip this entire section.  Probably
>> isa_register_portio_list should simply notice no ISA bus has been
>> registered and do nothing.
>
> Depends, if it doesn't need those ports, then vga_init_io() can be 
> passed a parameter not to register them, or perhaps it can be split 
> into two.
>

It's also wrong for cirrus.  Even though it is a legacy address, it's 
not an ISA address, it's on the PCI bus (though not mapped by a BAR).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 12/16] vga: Convert to isa_register_portio_list.

[Richard Henderson]

On 09/18/2011 07:56 AM, Avi Kivity wrote:
> It's also wrong for cirrus. Even though it is a legacy address, it's
> not an ISA address, it's on the PCI bus (though not mapped by a BAR).

Huh?  How do define that as not an ISA address?  Especially
since all that's called from isa_cirrus_vga_init?


r~

Re: [Qemu-devel] [PATCH 12/16] vga: Convert to isa_register_portio_list.

[Avi Kivity]

On 09/18/2011 06:15 PM, Richard Henderson wrote:
> On 09/18/2011 07:56 AM, Avi Kivity wrote:
> >  It's also wrong for cirrus. Even though it is a legacy address, it's
> >  not an ISA address, it's on the PCI bus (though not mapped by a BAR).
>
> Huh?  How do define that as not an ISA address?  Especially
> since all that's called from isa_cirrus_vga_init?
>

Ah, sorry, cirrus/pci indeed has its own ioports registration which 
doesn't go through isa.

So it's actually the opposite problem - generic port registration 
instead of bus-specific registration.  As soon as we convert it, we'll 
have the same problem again.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] [PATCH 13/16] pc: Convert port92 to isa_register_ioport.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/pc.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 263fb1a..4c460fd 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -428,6 +428,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
 /* port 92 stuff: could be split off */
 typedef struct Port92State {
     ISADevice dev;
+    MemoryRegion io;
     uint8_t outport;
     qemu_irq *a20_out;
 } Port92State;
@@ -479,13 +480,22 @@ static void port92_reset(DeviceState *d)
     s->outport &= ~1;
 }
 
+static const MemoryRegionPortio port92_portio[] = {
+    { 0, 1, 1, .read = port92_read, .write = port92_write },
+    PORTIO_END_OF_LIST(),
+};
+
+static const MemoryRegionOps port92_ops = {
+    .old_portio = port92_portio
+};
+
 static int port92_initfn(ISADevice *dev)
 {
     Port92State *s = DO_UPCAST(Port92State, dev, dev);
 
-    register_ioport_read(0x92, 1, 1, port92_read, s);
-    register_ioport_write(0x92, 1, 1, port92_write, s);
-    isa_init_ioport(dev, 0x92);
+    memory_region_init_io(&s->io, &port92_ops, s, "port92", 1);
+    isa_register_ioport(dev, &s->io, 0x92);
+
     s->outport = 0;
     return 0;
 }
-- 
1.7.4.4

[Qemu-devel] [PATCH 14/16] vmport: Convert to isa_register_ioport.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/vmport.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/vmport.c b/hw/vmport.c
index c8aefaa..b5c6fa1 100644
--- a/hw/vmport.c
+++ b/hw/vmport.c
@@ -38,6 +38,7 @@
 typedef struct _VMPortState
 {
     ISADevice dev;
+    MemoryRegion io;
     IOPortReadFunc *func[VMPORT_ENTRIES];
     void *opaque[VMPORT_ENTRIES];
 } VMPortState;
@@ -120,13 +121,22 @@ void vmmouse_set_data(const uint32_t *data)
     env->regs[R_ESI] = data[4]; env->regs[R_EDI] = data[5];
 }
 
+static const MemoryRegionPortio vmport_portio[] = {
+    {0, 1, 4, .read = vmport_ioport_read, .write = vmport_ioport_write },
+    PORTIO_END_OF_LIST(),
+};
+
+static const MemoryRegionOps vmport_ops = {
+    .old_portio = vmport_portio
+};
+
 static int vmport_initfn(ISADevice *dev)
 {
     VMPortState *s = DO_UPCAST(VMPortState, dev, dev);
 
-    register_ioport_read(0x5658, 1, 4, vmport_ioport_read, s);
-    register_ioport_write(0x5658, 1, 4, vmport_ioport_write, s);
-    isa_init_ioport(dev, 0x5658);
+    memory_region_init_io(&s->io, &vmport_ops, s, "vmport", 1);
+    isa_register_ioport(dev, &s->io, 0x5658);
+
     port_state = s;
     /* Register some generic port commands */
     vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, NULL);
-- 
1.7.4.4

[Qemu-devel] [PATCH 15/16] ide: Convert to isa_register_portio_list.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/ide/core.c     |   30 +++++++++++++++++++-----------
 hw/ide/internal.h |    3 ++-
 hw/ide/isa.c      |    4 +---
 hw/ide/piix.c     |    7 ++++---
 hw/ide/via.c      |    7 ++++---
 5 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d145b19..45da9df 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -25,6 +25,7 @@
 #include <hw/hw.h>
 #include <hw/pc.h>
 #include <hw/pci.h>
+#include <hw/isa.h>
 #include "qemu-error.h"
 #include "qemu-timer.h"
 #include "sysemu.h"
@@ -1873,20 +1874,27 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
     bus->dma = &ide_dma_nop;
 }
 
-void ide_init_ioport(IDEBus *bus, int iobase, int iobase2)
+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(),
+};
+
+void ide_init_ioport(IDEBus *bus, ISADevice *dev, int iobase, int iobase2)
 {
-    register_ioport_write(iobase, 8, 1, ide_ioport_write, bus);
-    register_ioport_read(iobase, 8, 1, ide_ioport_read, bus);
+    /* ??? Assume only ISA and PCI configurations, and that the PCI-ISA
+       bridge has been setup properly to always register with ISA.  */
+    isa_register_portio_list(dev, iobase, ide_portio_list, bus, "ide");
+
     if (iobase2) {
-        register_ioport_read(iobase2, 1, 1, ide_status_read, bus);
-        register_ioport_write(iobase2, 1, 1, ide_cmd_write, bus);
+        isa_register_portio_list(dev, iobase2, ide_portio2_list, bus, "ide");
     }
-
-    /* data ports */
-    register_ioport_write(iobase, 2, 2, ide_data_writew, bus);
-    register_ioport_read(iobase, 2, 2, ide_data_readw, bus);
-    register_ioport_write(iobase, 4, 4, ide_data_writel, bus);
-    register_ioport_read(iobase, 4, 4, ide_data_readl, bus);
 }
 
 static bool is_identify_set(void *opaque, int version_id)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 02e805f..978d848 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -7,6 +7,7 @@
  * non-internal declarations are in hw/ide.h
  */
 #include <hw/ide.h>
+#include <hw/isa.h>
 #include "block_int.h"
 #include "iorange.h"
 #include "dma.h"
@@ -588,7 +589,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                                     DriveInfo *hd1, qemu_irq irq);
-void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
+void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
 
 void ide_exec_cmd(IDEBus *bus, uint32_t val);
 void ide_dma_cb(void *opaque, int ret);
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 4ac7453..f4a86f2 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -67,10 +67,8 @@ static int isa_ide_initfn(ISADevice *dev)
     ISAIDEState *s = DO_UPCAST(ISAIDEState, dev, dev);
 
     ide_bus_new(&s->bus, &s->dev.qdev, 0);
-    ide_init_ioport(&s->bus, s->iobase, s->iobase2);
+    ide_init_ioport(&s->bus, dev, s->iobase, s->iobase2);
     isa_init_irq(dev, &s->irq, s->isairq);
-    isa_init_ioport_range(dev, s->iobase, 8);
-    isa_init_ioport(dev, s->iobase2);
     ide_init2(&s->bus, s->irq);
     vmstate_register(&dev->qdev, 0, &vmstate_ide_isa, s);
     return 0;
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 8525336..4df3c0d 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -123,8 +123,7 @@ static void piix3_reset(void *opaque)
 }
 
 static void pci_piix_init_ports(PCIIDEState *d) {
-    int i;
-    struct {
+    static const struct {
         int iobase;
         int iobase2;
         int isairq;
@@ -132,10 +131,12 @@ static void pci_piix_init_ports(PCIIDEState *d) {
         {0x1f0, 0x3f6, 14},
         {0x170, 0x376, 15},
     };
+    int i;
 
     for (i = 0; i < 2; i++) {
         ide_bus_new(&d->bus[i], &d->dev.qdev, i);
-        ide_init_ioport(&d->bus[i], port_info[i].iobase, port_info[i].iobase2);
+        ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
+                        port_info[i].iobase2);
         ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq));
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
diff --git a/hw/ide/via.c b/hw/ide/via.c
index c0b9d43..a2fb995 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -147,8 +147,7 @@ static void via_reset(void *opaque)
 }
 
 static void vt82c686b_init_ports(PCIIDEState *d) {
-    int i;
-    struct {
+    static const struct {
         int iobase;
         int iobase2;
         int isairq;
@@ -156,10 +155,12 @@ static void vt82c686b_init_ports(PCIIDEState *d) {
         {0x1f0, 0x3f6, 14},
         {0x170, 0x376, 15},
     };
+    int i;
 
     for (i = 0; i < 2; i++) {
         ide_bus_new(&d->bus[i], &d->dev.qdev, i);
-        ide_init_ioport(&d->bus[i], port_info[i].iobase, port_info[i].iobase2);
+        ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
+                        port_info[i].iobase2);
         ide_init2(&d->bus[i], isa_get_irq(port_info[i].isairq));
 
         bmdma_init(&d->bus[i], &d->bmdma[i], d);
-- 
1.7.4.4

[Qemu-devel] [PATCH 16/16] isa: Remove isa_init_ioport_range and isa_init_ioport.

[Richard Henderson]

All users have been converted to either isa_register_ioport
or isa_register_old_portio_list.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/isa-bus.c |   19 +++++--------------
 hw/isa.h     |    2 --
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 648b421..27c76b4 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -83,24 +83,17 @@ void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq)
     dev->nirqs++;
 }
 
-void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length)
+static inline void isa_init_ioport(ISADevice *dev, uint16_t ioport)
 {
-    if (dev->ioport_id == 0 || start < dev->ioport_id) {
-        dev->ioport_id = start;
+    if (dev && (dev->ioport_id == 0 || ioport < dev->ioport_id)) {
+        dev->ioport_id = ioport;
     }
 }
 
-void isa_init_ioport(ISADevice *dev, uint16_t ioport)
-{
-    isa_init_ioport_range(dev, ioport, 1);
-}
-
 void isa_register_ioport(ISADevice *dev, MemoryRegion *io, uint16_t start)
 {
     memory_region_add_subregion(isabus->address_space_io, start, io);
-    if (dev != NULL) {
-        isa_init_ioport(dev, start);
-    }
+    isa_init_ioport(dev, start);
 }
 
 static void isa_register_portio_1(ISADevice *dev,
@@ -149,9 +142,7 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start,
     /* 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.  */
-    if (dev) {
-        isa_init_ioport(dev, start);
-    }
+    isa_init_ioport(dev, start);
 
     /* Handle the first entry specially.  */
     off_last = off_low = pio_start->offset;
diff --git a/hw/isa.h b/hw/isa.h
index 177ef95..d3cae35 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -28,8 +28,6 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io);
 void isa_bus_irqs(qemu_irq *irqs);
 qemu_irq isa_get_irq(int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
-void isa_init_ioport(ISADevice *dev, uint16_t ioport);
-void isa_init_ioport_range(ISADevice *dev, uint16_t start, uint16_t length);
 void isa_qdev_register(ISADeviceInfo *info);
 MemoryRegion *isa_address_space(ISADevice *dev);
 ISADevice *isa_create(const char *name);
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH 00/16] isa_register_portio_list, v2

[Avi Kivity]

On 08/24/2011 03:13 AM, Richard Henderson wrote:
> The problem that malc saw with sb16 was a major think-o on my part
> with the whole interface.  We can't re-use the const sub-arrays of
> the original MemoryRegionPortio array because they have the wrong
> offset for the MemoryRegion to which it is attached -- the lookup
> in find_portio fails.  We must adjust the offsets of the old_portio
> array to be based against the MemoryRegion.
>
> Which means we can easily eliminate the major complaint that came
> with the previous round of comments -- the double PORTIO_END_OF_LIST
> and the explicit marking of the ranges.  All we require of users
> is that the array be sorted by offset.
>
> The entire patch set is at
>
>    git://repo.or.cz/qemu/rth.git mem-api-isa
>
> and is of course based on
>
>    git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/master
>
> Avi, the first two patches are fixes for compile errors in your
> tree.  They probably ought to be squashed when next you rebase.

Sloppy of me...

The patchset looks good, I'll leave it on the list for a few days before 
pulling to allow further review.

Once we're done with the conversion, we should look at extending the 
idea to mmio registers - decode the registers in the core instead of a 
per-region switch statement.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 00/16] isa_register_portio_list, v2

[malc]

On Tue, 23 Aug 2011, Richard Henderson wrote:

> The problem that malc saw with sb16 was a major think-o on my part
> with the whole interface.  We can't re-use the const sub-arrays of
> the original MemoryRegionPortio array because they have the wrong
> offset for the MemoryRegion to which it is attached -- the lookup
> in find_portio fails.  We must adjust the offsets of the old_portio
> array to be based against the MemoryRegion.
> 
> Which means we can easily eliminate the major complaint that came
> with the previous round of comments -- the double PORTIO_END_OF_LIST
> and the explicit marking of the ranges.  All we require of users
> is that the array be sorted by offset.
> 
> The entire patch set is at
> 
>   git://repo.or.cz/qemu/rth.git mem-api-isa
> 
> and is of course based on
> 
>   git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/master
> 
> Avi, the first two patches are fixes for compile errors in your
> tree.  They probably ought to be squashed when next you rebase.
> 
> Malc, I tested sb16 vs the xtc-play you provided.  However I failed
> to test the gus changes; the xtc-play program could not recognize
> the device with "--device gus" before or after the patch set.
> Should I be using another set of command-line options for that?
> 

I guess xtc can't live without environment variable which tells it
where gus is located `set ULTRASND=240,3,3,7,7'. Anyhow i've tested
things and both sb16 and gus work, thanks.

-- 
mailto:av1474@comtv.ru