[Qemu-devel] [PATCH V2 0/7] memory: unify ioport registration

[Qemu-devel] [PATCH V2 0/7] memory: unify ioport registration

[Julien Grall]

This is the second version of patch serie (the previous patch was not split)
about ioport registration.

For the moment some part of QEMU use register_ioport* functions to register
ioport. These functions don't permit the logging of range with Memory Listener.

Modifications between V1 and V2 :
  - Remove the use of get_system_io. Instead of use isa and pci IO address
  space.
  - Avoid allocation of PortioList. Use the different device structure.
  - Still remove register_ioport* (hw/dma.c, hw/apm.c, hw/acpi_piix4.c).
  - Use MemoryRegion when we have only a range of ioport.
  - For some functions, add IO address space as argument.
  - Add isa_address_space_io function

I may not to be aware about some new modifications (hw/dma.c and hw/apm.c),
any advice on its will be welcome.

Julien Grall (7):
  isa: add isa_address_space_io
  hw/acpi_piix4.c: replace register_ioport*
  hw/cirrus_vga.c: replace register_ioport*
  hw/serial.c: replace register_ioport*
  hw/pc.c: replace register_ioport*
  hw/dma.c: replace register_ioport*
  hw/apm.c: replace register_ioport*

 hw/acpi_piix4.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++--------
 hw/apm.c        |   24 +++++++++++++---
 hw/apm.h        |    5 +++-
 hw/cirrus_vga.c |   38 +++++++++++++++-----------
 hw/dma.c        |   55 ++++++++++++++++++++++++++------------
 hw/isa-bus.c    |    5 +++
 hw/isa.h        |    1 +
 hw/pc.c         |   37 ++++++++++++++++----------
 hw/pc.h         |    2 +-
 hw/serial.c     |    8 +++--
 hw/vt82c686.c   |    2 +-
 11 files changed, 184 insertions(+), 71 deletions(-)

-- 
Julien Grall

[Qemu-devel] [PATCH V2 1/7] isa: add isa_address_space_io

[Julien Grall]

This function permits to retrieve ISA IO address space.
It will be usefull when we need to pass IO address space as argument.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 hw/isa-bus.c |    5 +++++
 hw/isa.h     |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 5a43f03..043e1f7 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -235,4 +235,9 @@ MemoryRegion *isa_address_space(ISADevice *dev)
     return get_system_memory();
 }
 
+MemoryRegion *isa_address_space_io(ISADevice *dev)
+{
+    return get_system_io();
+}
+
 type_init(isabus_register_types)
diff --git a/hw/isa.h b/hw/isa.h
index 40373fb..a9bf65a 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -42,6 +42,7 @@ void isa_bus_irqs(ISABus *bus, qemu_irq *irqs);
 qemu_irq isa_get_irq(ISADevice *dev, int isairq);
 void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq);
 MemoryRegion *isa_address_space(ISADevice *dev);
+MemoryRegion *isa_address_space_io(ISADevice *dev);
 ISADevice *isa_create(ISABus *bus, const char *name);
 ISADevice *isa_try_create(ISABus *bus, const char *name);
 ISADevice *isa_create_simple(ISABus *bus, const char *name);
-- 
Julien Grall

[Qemu-devel] [PATCH V2 2/7] hw/acpi_piix4.c: replace register_ioport*

[Julien Grall]

This patch replaces all register_ioport* with portio_*. It permits to
use the new Memory stuff like listener.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 hw/acpi_piix4.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 797ed24..b9b195b 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -61,6 +61,13 @@ typedef struct PIIX4PMState {
     PMSMBus smb;
     uint32_t smb_io_base;
 
+    PortioList port_list;
+    PortioList acpi_port_list;
+    PortioList acpi_hot_port_list;
+    PortioList pci_hot_port_list;
+    PortioList pciej_hot_port_list;
+    PortioList pcirmv_hot_port_list;
+
     qemu_irq irq;
     qemu_irq smi_irq;
     int kvm_enabled;
@@ -325,6 +332,17 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque)
 
 }
 
+static const MemoryRegionPortio piix4_portio_list[] = {
+    { 0x00, 64, 1, .read = smb_ioport_readb, }, /* s->smb_io_base */
+    { 0x00, 64, 1, .write = smb_ioport_writeb, }, /* s->smb_io_base */
+    PORTIO_END_OF_LIST(),
+};
+
+static const MemoryRegionPortio acpi_portio_list[] = {
+    { 0x00, 4, 4, .write = acpi_dbg_writel, }, /* ACPI_DBG_IO_ADDR */
+    PORTIO_END_OF_LIST(),
+};
+
 static int piix4_pm_initfn(PCIDevice *dev)
 {
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
@@ -341,7 +359,9 @@ static int piix4_pm_initfn(PCIDevice *dev)
     /* APM */
     apm_init(&s->apm, apm_ctrl_changed, s);
 
-    register_ioport_write(ACPI_DBG_IO_ADDR, 4, 4, acpi_dbg_writel, s);
+    portio_list_init(&s->acpi_port_list, acpi_portio_list, s, "piix4-acpi");
+    portio_list_add(&s->acpi_port_list, pci_address_space_io(dev),
+                    ACPI_DBG_IO_ADDR);
 
     if (s->kvm_enabled) {
         /* Mark SMM as already inited to prevent SMM from running.  KVM does not
@@ -354,8 +374,10 @@ static int piix4_pm_initfn(PCIDevice *dev)
     pci_conf[0x90] = s->smb_io_base | 1;
     pci_conf[0x91] = s->smb_io_base >> 8;
     pci_conf[0xd2] = 0x09;
-    register_ioport_write(s->smb_io_base, 64, 1, smb_ioport_writeb, &s->smb);
-    register_ioport_read(s->smb_io_base, 64, 1, smb_ioport_readb, &s->smb);
+
+    portio_list_init(&s->port_list, piix4_portio_list, s, "piix4-acpi");
+    portio_list_add(&s->port_list, pci_address_space_io(dev),
+                    s->smb_io_base);
 
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer);
     acpi_gpe_init(&s->ar, GPE_LEN);
@@ -521,22 +543,54 @@ static void pcirmv_write(void *opaque, uint32_t addr, uint32_t val)
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
                                 PCIHotplugState state);
 
+static const MemoryRegionPortio acpi_hot_portio_list[] = {
+    { 0x00, GPE_LEN, 1, .write = gpe_writeb,
+        .read = gpe_readb, }, /* GPE_BASE */
+    PORTIO_END_OF_LIST(),
+};
+
+/* IOport from PCI_BASE */
+static const MemoryRegionPortio pci_hot_portio_list[] = {
+    { 0x00, 8, 4, .write = pcihotplug_write, .read = pcihotplug_read, },
+    PORTIO_END_OF_LIST(),
+};
+
+/* IOport from PCI_EJ_BASE */
+static const MemoryRegionPortio pciej_hot_portio_list[] = {
+    { 0x00, 4, 4, .write = pciej_write, .read = pciej_read, },
+    PORTIO_END_OF_LIST(),
+};
+
+/* IOport form PCI_RMV_BASE */
+static const MemoryRegionPortio pcirmv_hot_portio_list[] = {
+    { 0x00, 4, 4, .write = pcirmv_write, .read = pcirmv_read, },
+    PORTIO_END_OF_LIST(),
+};
+
 static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
 {
     struct pci_status *pci0_status = &s->pci0_status;
 
-    register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s);
-    register_ioport_read(GPE_BASE, GPE_LEN, 1,  gpe_readb, s);
+    portio_list_init(&s->acpi_hot_port_list, acpi_hot_portio_list,
+                     s, "piix4-acpi-hot");
+    portio_list_add(&s->acpi_hot_port_list, pci_address_space_io(&s->dev),
+                    GPE_BASE);
     acpi_gpe_blk(&s->ar, GPE_BASE);
 
-    register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, pci0_status);
-    register_ioport_read(PCI_BASE, 8, 4,  pcihotplug_read, pci0_status);
+    portio_list_init(&s->pci_hot_port_list, pci_hot_portio_list,
+                     pci0_status, "piix4-pci-hot");
+    portio_list_add(&s->pci_hot_port_list, pci_address_space_io(&s->dev),
+                    PCI_BASE);
 
-    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus);
-    register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, bus);
+    portio_list_init(&s->pciej_hot_port_list, pciej_hot_portio_list,
+                     bus, "piix4-pciej-hot");
+    portio_list_add(&s->pciej_hot_port_list, pci_address_space_io(&s->dev),
+                    PCI_EJ_BASE);
 
-    register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s);
-    register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
+    portio_list_init(&s->pcirmv_hot_port_list, pcirmv_hot_portio_list,
+                     s, "piix4-pcirmv-hot");
+    portio_list_add(&s->pcirmv_hot_port_list, pci_address_space_io(&s->dev),
+                    PCI_RMV_BASE);
 
     pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
 }
-- 
Julien Grall

[Qemu-devel] [PATCH V2 3/7] hw/cirrus_vga.c: replace register_ioport*

[Julien Grall]

This patch replaces all register_ioport* with portio_*. It permits to
use the new Memory stuff like listener.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 hw/cirrus_vga.c |   38 ++++++++++++++++++++++----------------
 1 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index afedaa4..43731ac 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -198,6 +198,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
 typedef struct CirrusVGAState {
     VGACommonState vga;
 
+    PortioList cirrus_port_list;
     MemoryRegion cirrus_linear_io;
     MemoryRegion cirrus_linear_bitblt_io;
     MemoryRegion cirrus_mmio_io;
@@ -2781,8 +2782,23 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
     },
 };
 
+static const MemoryRegionPortio cirrus_portio_list[] = {
+    { 0x04, 2, 1, .write = cirrus_vga_ioport_write,
+    .read = cirrus_vga_ioport_read, }, /* 0x3b4 */
+    { 0x0a, 1, 1, .write = cirrus_vga_ioport_write,
+    .read = cirrus_vga_ioport_read, }, /* 0x3ba */
+    { 0x10, 16, 1, .write = cirrus_vga_ioport_write,
+    .read = cirrus_vga_ioport_read, }, /* 0x3c0 */
+    { 0x24, 2, 1, .write = cirrus_vga_ioport_write,
+    .read = cirrus_vga_ioport_read, }, /* 0x3d4 */
+    { 0x2a, 1, 1, .write = cirrus_vga_ioport_write,
+    .read = cirrus_vga_ioport_read, }, /* 0x3da */
+    PORTIO_END_OF_LIST(),
+};
+
 static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci,
-                               MemoryRegion *system_memory)
+                               MemoryRegion *system_memory,
+                               MemoryRegion *system_io)
 {
     int i;
     static int inited;
@@ -2814,19 +2830,8 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci,
             s->bustype = CIRRUS_BUSTYPE_ISA;
     }
 
-    register_ioport_write(0x3c0, 16, 1, cirrus_vga_ioport_write, s);
-
-    register_ioport_write(0x3b4, 2, 1, cirrus_vga_ioport_write, s);
-    register_ioport_write(0x3d4, 2, 1, cirrus_vga_ioport_write, s);
-    register_ioport_write(0x3ba, 1, 1, cirrus_vga_ioport_write, s);
-    register_ioport_write(0x3da, 1, 1, cirrus_vga_ioport_write, s);
-
-    register_ioport_read(0x3c0, 16, 1, cirrus_vga_ioport_read, s);
-
-    register_ioport_read(0x3b4, 2, 1, cirrus_vga_ioport_read, s);
-    register_ioport_read(0x3d4, 2, 1, cirrus_vga_ioport_read, s);
-    register_ioport_read(0x3ba, 1, 1, cirrus_vga_ioport_read, s);
-    register_ioport_read(0x3da, 1, 1, cirrus_vga_ioport_read, s);
+    portio_list_init(&s->cirrus_port_list, cirrus_portio_list, s, "cirrus-io");
+    portio_list_add(&s->cirrus_port_list, system_io, 0x3b0);
 
     memory_region_init(&s->low_mem_container,
                        "cirrus-lowmem-container",
@@ -2893,7 +2898,7 @@ static int vga_initfn(ISADevice *dev)
 
     vga_common_init(s, VGA_RAM_SIZE);
     cirrus_init_common(&d->cirrus_vga, CIRRUS_ID_CLGD5430, 0,
-                       isa_address_space(dev));
+                       isa_address_space(dev), isa_address_space_io(dev));
     s->ds = graphic_console_init(s->update, s->invalidate,
                                  s->screen_dump, s->text_update,
                                  s);
@@ -2934,7 +2939,8 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
 
      /* setup VGA */
      vga_common_init(&s->vga, VGA_RAM_SIZE);
-     cirrus_init_common(s, device_id, 1, pci_address_space(dev));
+     cirrus_init_common(s, device_id, 1, pci_address_space(dev),
+                        pci_address_space_io(dev));
      s->vga.ds = graphic_console_init(s->vga.update, s->vga.invalidate,
                                       s->vga.screen_dump, s->vga.text_update,
                                       &s->vga);
-- 
Julien Grall

[Qemu-devel] [PATCH V2 4/7] hw/serial.c: replace register_ioport*

[Julien Grall]

This patch replaces all register_ioport* with a MemoryRegion. It permits to
use the new Memory stuff like listener.

For more flexibility, the IO address space is passed as an argument.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 hw/pc.h     |    2 +-
 hw/serial.c |    8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index 74d3369..e6135a5 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -15,7 +15,7 @@
 /* serial.c */
 
 SerialState *serial_init(int base, qemu_irq irq, int baudbase,
-                         CharDriverState *chr);
+                         CharDriverState *chr, MemoryRegion *system_io);
 SerialState *serial_mm_init(MemoryRegion *address_space,
                             target_phys_addr_t base, int it_shift,
                             qemu_irq irq, int baudbase,
diff --git a/hw/serial.c b/hw/serial.c
index c0ee55d..9e755e8 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -28,6 +28,7 @@
 #include "pc.h"
 #include "qemu-timer.h"
 #include "sysemu.h"
+#include "exec-memory.h"
 
 //#define DEBUG_SERIAL
 
@@ -807,7 +808,7 @@ static const VMStateDescription vmstate_isa_serial = {
 };
 
 SerialState *serial_init(int base, qemu_irq irq, int baudbase,
-                         CharDriverState *chr)
+                         CharDriverState *chr, MemoryRegion *system_io)
 {
     SerialState *s;
 
@@ -820,8 +821,9 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
 
     vmstate_register(NULL, base, &vmstate_serial, s);
 
-    register_ioport_write(base, 8, 1, serial_ioport_write, s);
-    register_ioport_read(base, 8, 1, serial_ioport_read, s);
+    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
+    memory_region_add_subregion(system_io, base, &s->io);
+
     return s;
 }
 
-- 
Julien Grall

[Qemu-devel] [PATCH V2 5/7] hw/pc.c: replace register_ioport*

[Julien Grall]

This patch replaces all register_ioport* with portio_* or
isa_register_portio_list. It permits to use the new Memory
stuff like listener.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 hw/pc.c |   37 +++++++++++++++++++++++--------------
 1 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 83a1b5b..6036393 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -592,6 +592,17 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type)
     return index;
 }
 
+static const MemoryRegionPortio bochs_bios_portio_list[] = {
+    { 0x400, 2, 2, .write = bochs_bios_write, }, /* 0x400 */
+    { 0x402, 2, 1, .write = bochs_bios_write, }, /* 0x402 */
+    { 0x500, 1, 1, .write = bochs_bios_write, }, /* 0x500 */
+    { 0x501, 1, 1, .write = bochs_bios_write, }, /* 0x501 */
+    { 0x501, 2, 2, .write = bochs_bios_write, }, /* 0x501 */
+    { 0x503, 1, 1, .write = bochs_bios_write, }, /* 0x503  */
+    { 0x8900, 1, 1, .write = bochs_bios_write, }, /* 0x8900 */
+    PORTIO_END_OF_LIST(),
+};
+
 static void *bochs_bios_init(void)
 {
     void *fw_cfg;
@@ -599,18 +610,11 @@ static void *bochs_bios_init(void)
     size_t smbios_len;
     uint64_t *numa_fw_cfg;
     int i, j;
+    PortioList *bochs_bios_port_list = g_new(PortioList, 1);
 
-    register_ioport_write(0x400, 1, 2, bochs_bios_write, NULL);
-    register_ioport_write(0x401, 1, 2, bochs_bios_write, NULL);
-    register_ioport_write(0x402, 1, 1, bochs_bios_write, NULL);
-    register_ioport_write(0x403, 1, 1, bochs_bios_write, NULL);
-    register_ioport_write(0x8900, 1, 1, bochs_bios_write, NULL);
-
-    register_ioport_write(0x501, 1, 1, bochs_bios_write, NULL);
-    register_ioport_write(0x501, 1, 2, bochs_bios_write, NULL);
-    register_ioport_write(0x502, 1, 2, bochs_bios_write, NULL);
-    register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
-    register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
+    portio_list_init(bochs_bios_port_list, bochs_bios_portio_list,
+                     NULL, "bosch-bios");
+    portio_list_add(bochs_bios_port_list, get_system_io(), 0x0);
 
     fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
 
@@ -1077,6 +1081,12 @@ static void cpu_request_exit(void *opaque, int irq, int level)
     }
 }
 
+static const MemoryRegionPortio pc_basic_portio_list[] = {
+    { 0x00, 1, 1, .write = ioport80_write, }, /* 0x80 */
+    { 0x70, 1, 1, .write = ioportF0_write, }, /* 0xf0 */
+    PORTIO_END_OF_LIST(),
+};
+
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice **rtc_state,
                           ISADevice **floppy,
@@ -1092,9 +1102,8 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     ISADevice *i8042, *port92, *vmmouse, *pit;
     qemu_irq *cpu_exit_irq;
 
-    register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
-
-    register_ioport_write(0xf0, 1, 1, ioportF0_write, NULL);
+    isa_register_portio_list(NULL, 0x80, pc_basic_portio_list, NULL,
+                             "pc-basic");
 
     /*
      * Check if an HPET shall be created.
-- 
Julien Grall

[Qemu-devel] [PATCH V2 6/7] hw/dma.c: replace register_ioport*

[Julien Grall]

This patch replaces all register_ioport* with isa_register_portio_list.
It permits to use the new Memory stuff like listener.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 hw/dma.c |   55 +++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/hw/dma.c b/hw/dma.c
index 0a9322d..985bfe6 100644
--- a/hw/dma.c
+++ b/hw/dma.c
@@ -473,37 +473,56 @@ static int dma_phony_handler (void *opaque, int nchan, int dma_pos, int dma_len)
     return dma_pos;
 }
 
+
+/* IOport for channel */
+static const MemoryRegionPortio channel_portio_list[] = {
+    { 0x00, 1, 1, .write = write_chan, .read = read_chan, },
+    PORTIO_END_OF_LIST(),
+};
+
+/* 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(),
+};
+
+/* IOport for cont */
+static const MemoryRegionPortio cont_portio_list[] = {
+    { 0x00, 1, 1, .write = write_cont, .read = read_cont, },
+    PORTIO_END_OF_LIST(),
+};
+
 /* dshift = 0: 8 bit DMA, 1 = 16 bit DMA */
 static void dma_init2(struct dma_cont *d, int base, int dshift,
                       int page_base, int pageh_base,
                       qemu_irq *cpu_request_exit)
 {
-    static const int page_port_list[] = { 0x1, 0x2, 0x3, 0x7 };
     int i;
 
     d->dshift = dshift;
     d->cpu_request_exit = cpu_request_exit;
     for (i = 0; i < 8; i++) {
-        register_ioport_write (base + (i << dshift), 1, 1, write_chan, d);
-        register_ioport_read (base + (i << dshift), 1, 1, read_chan, d);
+        isa_register_portio_list(NULL, base + (i << dshift),
+                                 channel_portio_list, d, "dma-chan");
     }
-    for (i = 0; i < ARRAY_SIZE (page_port_list); i++) {
-        register_ioport_write (page_base + page_port_list[i], 1, 1,
-                               write_page, d);
-        register_ioport_read (page_base + page_port_list[i], 1, 1,
-                              read_page, d);
-        if (pageh_base >= 0) {
-            register_ioport_write (pageh_base + page_port_list[i], 1, 1,
-                                   write_pageh, d);
-            register_ioport_read (pageh_base + page_port_list[i], 1, 1,
-                                  read_pageh, d);
-        }
+
+    isa_register_portio_list(NULL, page_base, page_portio_list, d,
+                             "dma-page");
+    if (pageh_base >= 0) {
+        isa_register_portio_list(NULL, pageh_base, pageh_portio_list, d,
+                                 "dma-pageh");
     }
     for (i = 0; i < 8; i++) {
-        register_ioport_write (base + ((i + 8) << dshift), 1, 1,
-                               write_cont, d);
-        register_ioport_read (base + ((i + 8) << dshift), 1, 1,
-                              read_cont, d);
+        isa_register_portio_list(NULL, base + ((i + 8) << dshift),
+                                 cont_portio_list, d, "dma-cont");
     }
     qemu_register_reset(dma_reset, d);
     dma_reset(d);
-- 
Julien Grall

[Qemu-devel] [PATCH V2 7/7] hw/apm.c: replace register_ioport*

[Julien Grall]

This patch replaces all register_ioport* by a MemorySection.
It permits to use the new Memory stuff like listener.

Moreover, the PCI is added as an argument for apm_init, so we
can register IO inside the pci IO address space.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
 hw/acpi_piix4.c |    2 +-
 hw/apm.c        |   24 +++++++++++++++++++-----
 hw/apm.h        |    5 ++++-
 hw/vt82c686.c   |    2 +-
 4 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index b9b195b..b0e7749 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -357,7 +357,7 @@ static int piix4_pm_initfn(PCIDevice *dev)
     pci_conf[0x40] = 0x01; /* PM io base read only bit */
 
     /* APM */
-    apm_init(&s->apm, apm_ctrl_changed, s);
+    apm_init(dev, &s->apm, apm_ctrl_changed, s);
 
     portio_list_init(&s->acpi_port_list, acpi_portio_list, s, "piix4-acpi");
     portio_list_add(&s->acpi_port_list, pci_address_space_io(dev),
diff --git a/hw/apm.c b/hw/apm.c
index 2aead52..fe7bc21 100644
--- a/hw/apm.c
+++ b/hw/apm.c
@@ -22,6 +22,7 @@
 
 #include "apm.h"
 #include "hw.h"
+#include "pci.h"
 
 //#define DEBUG
 
@@ -35,7 +36,8 @@
 #define APM_CNT_IOPORT  0xb2
 #define APM_STS_IOPORT  0xb3
 
-static void apm_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void apm_ioport_writeb(void *opaque, target_phys_addr_t addr,
+                              uint64_t val, unsigned size)
 {
     APMState *apm = opaque;
     addr &= 1;
@@ -51,7 +53,8 @@ static void apm_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-static uint32_t apm_ioport_readb(void *opaque, uint32_t addr)
+static uint64_t apm_ioport_readb(void *opaque, target_phys_addr_t addr,
+                                 unsigned size)
 {
     APMState *apm = opaque;
     uint32_t val;
@@ -78,12 +81,23 @@ const VMStateDescription vmstate_apm = {
     }
 };
 
-void apm_init(APMState *apm, apm_ctrl_changed_t callback, void *arg)
+static const MemoryRegionOps apm_ops = {
+    .read = apm_ioport_readb,
+    .write = apm_ioport_writeb,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+void apm_init(PCIDevice *dev, APMState *apm, apm_ctrl_changed_t callback,
+              void *arg)
 {
     apm->callback = callback;
     apm->arg = arg;
 
     /* ioport 0xb2, 0xb3 */
-    register_ioport_write(APM_CNT_IOPORT, 2, 1, apm_ioport_writeb, apm);
-    register_ioport_read(APM_CNT_IOPORT, 2, 1, apm_ioport_readb, apm);
+    memory_region_init_io(&apm->io, &apm_ops, apm, "apm-io", 2);
+    memory_region_add_subregion(pci_address_space_io(dev), APM_CNT_IOPORT,
+                                &apm->io);
 }
diff --git a/hw/apm.h b/hw/apm.h
index f7c741e..5431b6d 100644
--- a/hw/apm.h
+++ b/hw/apm.h
@@ -4,6 +4,7 @@
 #include <stdint.h>
 #include "qemu-common.h"
 #include "hw.h"
+#include "memory.h"
 
 typedef void (*apm_ctrl_changed_t)(uint32_t val, void *arg);
 
@@ -13,9 +14,11 @@ typedef struct APMState {
 
     apm_ctrl_changed_t callback;
     void *arg;
+    MemoryRegion io;
 } APMState;
 
-void apm_init(APMState *s, apm_ctrl_changed_t callback, void *arg);
+void apm_init(PCIDevice *dev, APMState *s, apm_ctrl_changed_t callback,
+              void *arg);
 
 extern const VMStateDescription vmstate_apm;
 
diff --git a/hw/vt82c686.c b/hw/vt82c686.c
index 6fb7950..1994bc5 100644
--- a/hw/vt82c686.c
+++ b/hw/vt82c686.c
@@ -427,7 +427,7 @@ static int vt82c686b_pm_initfn(PCIDevice *dev)
     register_ioport_write(s->smb_io_base, 0xf, 1, smb_ioport_writeb, &s->smb);
     register_ioport_read(s->smb_io_base, 0xf, 1, smb_ioport_readb, &s->smb);
 
-    apm_init(&s->apm, NULL, s);
+    apm_init(dev, &s->apm, NULL, s);
 
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer);
     acpi_pm1_cnt_init(&s->ar);
-- 
Julien Grall

Re: [Qemu-devel] [PATCH V2 2/7] hw/acpi_piix4.c: replace register_ioport*

[Avi Kivity]

On 04/04/2012 09:21 PM, Julien Grall wrote:
> This patch replaces all register_ioport* with portio_*. It permits to
> use the new Memory stuff like listener.
>
>
>  
> +static const MemoryRegionPortio piix4_portio_list[] = {
> +    { 0x00, 64, 1, .read = smb_ioport_readb, }, /* s->smb_io_base */
> +    { 0x00, 64, 1, .write = smb_ioport_writeb, }, /* s->smb_io_base */
> +    PORTIO_END_OF_LIST(),
> +};
> +
> +static const MemoryRegionPortio acpi_portio_list[] = {
> +    { 0x00, 4, 4, .write = acpi_dbg_writel, }, /* ACPI_DBG_IO_ADDR */
> +    PORTIO_END_OF_LIST(),
> +};

Please replace single-port lists with MemoryRegionOps (and MemoryRegions
in the state structure).  I think I mentioned this in the first review,
but perhaps only about a single instance, sorry.

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