memory: unifiy ioport registration

PATCH Superseded Julien Grall 4 participants 21 messages Aug 22, 2012 – Aug 28, 2012

Share & tools

Julien Grall

author

2012-08-22 18:50 UTC #0 lore raw

v5 cover memory: unifiy ioport registration

This is the fifth version of patch series about ioport registration. The fourth version was sent few months ago
(https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01396.html).

Some part of QEMU still use register_ioport* functions to register ioport. These functions doesn't allow to use Memory Listener on it.

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

Modifications between V2 and V3:
- Remove some register_ioport_* on hw/vt82c686.c.
- Split smb ioport part in new patch.
- Still replace MemoryRegion when we have only a range of ioport.
- Fix read/write ioports prototype to be compliant with memory callback.

Modifications between V3 and V4:
- Fix compilation in hw/dma.c
- Fix address conversion (hw/dma.c, hw/acpi_piix4.c) with MemorySection. Indeed the new version use offset from MemorySection start instead of 0.

Modifications between V4 and V4:
- Rebase on qemu upstream.
- Forget some ioport_register_* in acpi_piix4.c.
- Register 0x3b0 - 0x3df range for cirrus instead of ioport by ioport.

Julien Grall (8):
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*
smb: replace_register_ioport*

 hw/acpi_piix4.c   |  160 +++++++++++++++++++++++++++++++++++++++++------------
 hw/apm.c          |   24 ++++++--
 hw/apm.h          |    5 +-
 hw/cirrus_vga.c   |   42 ++++++++------
 hw/dma.c          |  108 ++++++++++++++++++++++++------------
 hw/isa-bus.c      |    5 ++
 hw/isa.h          |    1 +
 hw/mips_mipssim.c |    3 +-
 hw/pc.c           |   58 ++++++++++++++-----
 hw/pc.h           |    2 +-
 hw/pm_smbus.c     |    7 +-
 hw/pm_smbus.h     |    6 +-
 hw/serial.c       |    8 ++-
 hw/vt82c686.c     |   20 ++++++-
 14 files changed, 325 insertions(+), 124 deletions(-)

Signature · 1 line

Julien Grall

Julien Grall

author

2012-08-22 18:50 UTC #1 lore raw

PATCH v5 1/8 isa: add isa_address_space_io

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

---
 hw/isa-bus.c |    5 +++++
 hw/isa.h     |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

hw/isa-bus.c +5 -0

index f9b2373..662c86b 100644
@@ -244,4 +244,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)

hw/isa.h +1 -0

index dc97052..3891c1f 100644
@@ -43,6 +43,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);

Signature · 1 line

Julien Grall

Andreas Färber

2012-08-24 16:10 UTC #2 lore raw

Re: [Qemu-devel] [PATCH V5 1/8] isa: add isa_address_space_io

Am 22.08.2012 14:27, schrieb Julien Grall:

Quoted 14 lines

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 <[email protected]>
---
 hw/isa-bus.c |    5 +++++
 hw/isa.h     |    1 +
 2 files changed, 6 insertions(+), 0 deletions(-)
hw/isa-bus.c +0 -0
index f9b2373..662c86b 100644

hw/isa-bus.c MemoryRegion *isa_address_space(ISADevice *dev) · line 250

Quoted 7 lines

hw/isa-bus.c +4 -0

@@ -244,4 +244,9 @@ MemoryRegion *isa_address_space(ISADevice *dev)
     return get_system_memory();
 }
 
+MemoryRegion *isa_address_space_io(ISADevice *dev)
+{
+    return get_system_io();
+}

Unlike the address_space above, there's an address_space_io field in ISABus, so I guess the implementation of this function should rather obtain the device's BusState via isa_bus_from_device(dev) and return its field rather than hardcoding get_system_io() here.

For x86 it shouldn't make a difference but I think on PReP there's two different runtime-switchable I/O space configurations or so...

Regards,
Andreas

Quoted 14 lines

hw/isa-bus.c +1 -0

+
 type_init(isabus_register_types)

hw/isa.h +1 -0

index dc97052..3891c1f 100644
@@ -43,6 +43,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);

Signature · 2 lines

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

Julien Grall

author

2012-08-28 15:41 UTC #3 lore raw

Re: [Qemu-devel] [PATCH V5 1/8] isa: add isa_address_space_io

Quoted 19 lines · Andreas Färber

Am 22.08.2012 14:27, schrieb Julien Grall:
Quoted 17 lines
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<[email protected]>
---
  hw/isa-bus.c |    5 +++++
  hw/isa.h     |    1 +
  2 files changed, 6 insertions(+), 0 deletions(-)
hw/isa-bus.c +0 -0
index f9b2373..662c86b 100644
@@ -244,4 +244,9 @@ MemoryRegion *isa_address_space(ISADevice *dev)
      return get_system_memory();
  }
 

Quoted 10 lines

hw/isa-bus.c MemoryRegion *isa_address_space(ISADevice *dev)
Quoted 5 lines
hw/isa-bus.c +4 -0
+MemoryRegion *isa_address_space_io(ISADevice *dev)
+{
+    return get_system_io();
+}
 
Unlike the address_space above, there's an address_space_io field in ISABus, so I guess the implementation of this function should rather obtain the device's BusState via isa_bus_from_device(dev) and return its field rather than hardcoding get_system_io() here.

I use this function in hw/dma.c. For the moment, the code doesn't use ISA device, so I pass NULL to isa_address_space_io (See patch 6).

Instead of using isa_bus_from_device, can I just return "isabus->address_space_io" as in isa_register_portio_list ?

Signature · 1 line

Julien Grall

Jan Kiszka

2012-08-28 17:09 UTC #4 lore raw

Re: [Qemu-devel] [PATCH V5 1/8] isa: add isa_address_space_io

Quoted 30 lines · Julien Grall

Quoted 19 lines · Andreas Färber
Am 22.08.2012 14:27, schrieb Julien Grall:
Quoted 17 lines
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<[email protected]>
---
  hw/isa-bus.c |    5 +++++
  hw/isa.h     |    1 +
  2 files changed, 6 insertions(+), 0 deletions(-)
hw/isa-bus.c +0 -0
index f9b2373..662c86b 100644
@@ -244,4 +244,9 @@ MemoryRegion *isa_address_space(ISADevice *dev)
      return get_system_memory();
  }
 
Quoted 10 lines
hw/isa-bus.c MemoryRegion *isa_address_space(ISADevice *dev)
Quoted 5 lines
hw/isa-bus.c +4 -0
+MemoryRegion *isa_address_space_io(ISADevice *dev)
+{
+    return get_system_io();
+}
 
Unlike the address_space above, there's an address_space_io field in ISABus, so I guess the implementation of this function should rather obtain the device's BusState via isa_bus_from_device(dev) and return its field rather than hardcoding get_system_io() here.

Quoted 3 lines

I use this function in hw/dma.c. For the moment, the code doesn't use ISA device, so I pass NULL to isa_address_space_io (See patch 6).

Yes, there are some old drivers in QEMU that doesn't follow QOM or even qdev patterns. They may not work on systems Andreas is thinking of anyway.

But this is a generic service, so it should try harder: If a device is given, use the io space of its bus. If not, fall back to isabus.

Jan

Signature · 2 lines

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

Julien Grall

author

2012-08-22 18:50 UTC #5 lore raw

PATCH v5 2/8 hw/acpi_piix4.c: replace register_ioport*

In reply to Julien Grall

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

Signed-off-by Julien Grall <[email protected]>

---
 hw/acpi_piix4.c |  160 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 124 insertions(+), 36 deletions(-)

hw/acpi_piix4.c +124 ~~-36~~

index 0aace60..26d5559 100644
@@ -28,6 +28,7 @@
 #include "range.h"
 #include "ioport.h"
 #include "fw_cfg.h"
+#include "exec-memory.h"
 
 //#define DEBUG

@@ -41,8 +42,7 @@
 
 #define GPE_BASE 0xafe0
 #define GPE_LEN 4
-#define PCI_UP_BASE 0xae00
-#define PCI_DOWN_BASE 0xae04
+#define PCI_BASE 0xaa00
 #define PCI_EJ_BASE 0xae08
 #define PCI_RMV_BASE 0xae0c

@@ -55,7 +55,7 @@ struct pci_status {
 
 typedef struct PIIX4PMState {
     PCIDevice dev;
-    IORange ioport;
+    MemoryRegion pm_io;
     ACPIREGS ar;
 
     APMState apm;

@@ -63,6 +63,13 @@ typedef struct PIIX4PMState {
     PMSMBus smb;
     uint32_t smb_io_base;
 
+    MemoryRegion smb_io;
+    MemoryRegion acpi_io;
+    MemoryRegion acpi_hot_io;
+    PortioList pci_hot_port_list;
+    MemoryRegion pciej_hot_io;
+    MemoryRegion pcirmv_hot_io;
+
     qemu_irq irq;
     qemu_irq smi_irq;
     int kvm_enabled;

@@ -108,12 +115,12 @@ static void pm_tmr_timer(ACPIREGS *ar)
     pm_update_sci(s);
 }
 
-static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
-                            uint64_t val)
+static void pm_ioport_write(void *opaque, target_phys_addr_t addr,
+                            uint64_t val, unsigned size)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
+    PIIX4PMState *s = opaque;
 
-    if (width != 2) {
+    if (size != 2) {
         PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
                       (unsigned)addr, width, (unsigned)val);
     }

@@ -137,11 +144,11 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
                   (unsigned int)val);
 }
 
-static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
-                            uint64_t *data)
+static uint64_t pm_ioport_read(void *opaque, target_phys_addr_t addr,
+                               unsigned size)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
-    uint32_t val;
+    PIIX4PMState *s = opaque;
+    uint64_t val;
 
     switch(addr) {
     case 0x00:

@@ -161,12 +168,18 @@ static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
         break;
     }
     PIIX4_DPRINTF("PM readw port=0x%04x val=0x%04x\n", (unsigned int)addr, val);
-    *data = val;
+
+    return val;
 }
 
-static const IORangeOps pm_iorange_ops = {
+static const MemoryRegionOps pm_io_ops = {
     .read = pm_ioport_read,
     .write = pm_ioport_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 2,
+        .max_access_size = 2,
+    },
 };
 
 static void apm_ctrl_changed(uint32_t val, void *arg)

@@ -183,7 +196,8 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
     }
 }
 
-static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val)
+static void acpi_dbg_writel(void *opaque, target_phys_addr_t addr,
+                            uint64_t val, unsigned size)
 {
     PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
 }

@@ -198,8 +212,10 @@ static void pm_io_space_update(PIIX4PMState *s)
 
         /* XXX: need to improve memory and ioport allocation */
         PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
-        iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
-        ioport_register(&s->ioport);
+
+        memory_region_init_io(&s->pm_io, &pm_io_ops, s, "piix4-pm", 64);
+        memory_region_add_subregion(pci_address_space_io(&s->dev),
+                                    pm_io_base, &s->pm_io);
     }
 }

@@ -381,6 +397,25 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque)
 
 }
 
+static const MemoryRegionOps smb_io_ops = {
+    .read = smb_ioport_readb,
+    .write = smb_ioport_writeb,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static const MemoryRegionOps acpi_io_ops = {
+    .write = acpi_dbg_writel,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
 static int piix4_pm_initfn(PCIDevice *dev)
 {
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);

@@ -395,9 +430,11 @@ 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);
 
-    register_ioport_write(ACPI_DBG_IO_ADDR, 4, 4, acpi_dbg_writel, s);
+    memory_region_init_io(&s->acpi_io, &acpi_io_ops, s, "piix4-acpi", 4);
+    memory_region_add_subregion(pci_address_space_io(dev), ACPI_DBG_IO_ADDR,
+                                &s->acpi_io);
 
     if (s->kvm_enabled) {
         /* Mark SMM as already inited to prevent SMM from running.  KVM does not

@@ -410,8 +447,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);
+
+    memory_region_init_io(&s->smb_io, &smb_io_ops, &s->smb, "piix4-smb", 64);
+    memory_region_add_subregion(pci_address_space_io(dev), s->smb_io_base,
+                                &s->smb_io);
 
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer);
     acpi_gpe_init(&s->ar, GPE_LEN);

@@ -496,16 +535,17 @@ static void piix4_pm_register_types(void)
 
 type_init(piix4_pm_register_types)
 
-static uint32_t gpe_readb(void *opaque, uint32_t addr)
+static uint64_t gpe_readb(void *opaque, target_phys_addr_t addr, unsigned size)
 {
     PIIX4PMState *s = opaque;
-    uint32_t val = acpi_gpe_ioport_readb(&s->ar, addr);
+    uint64_t val = acpi_gpe_ioport_readb(&s->ar, addr);
 
     PIIX4_DPRINTF("gpe read %x == %x\n", addr, val);
     return val;
 }
 
-static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void gpe_writeb(void *opaque, target_phys_addr_t addr, uint64_t val,
+                       unsigned size)
 {
     PIIX4PMState *s = opaque;

@@ -537,21 +577,24 @@ static uint32_t pci_down_read(void *opaque, uint32_t addr)
     return val;
 }
 
-static uint32_t pci_features_read(void *opaque, uint32_t addr)
+static uint64_t pci_features_read(void *opaque, target_phys_addr_t addr,
+                                  unsigned size)
 {
     /* 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)
+static void pciej_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+                        unsigned size)
 {
     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 uint64_t pcirmv_read(void *opaque, target_phys_addr_t addr,
+                            unsigned size)
 {
     PIIX4PMState *s = opaque;

@@ -561,20 +604,65 @@ static uint32_t pcirmv_read(void *opaque, uint32_t addr)
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
                                 PCIHotplugState state);
 
-static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
-{
+static const MemoryRegionOps acpi_hot_io_ops = {
+    .read = gpe_readb,
+    .write = gpe_writeb,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+/* PCI hot plug registers */
+static const MemoryRegionPortio pci_hot_portio_list[] = {
+    { 0x00, 4, 4, .read = pci_up_read, }, /* 0xae00 */
+    { 0x04, 4, 4, .read = pci_down_read, }, /* 0xae04 */
+    PORTIO_END_OF_LIST(),
+};
 
-    register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s);
-    register_ioport_read(GPE_BASE, GPE_LEN, 1,  gpe_readb, s);
-    acpi_gpe_blk(&s->ar, GPE_BASE);
+static const MemoryRegionOps pciej_hot_io_ops = {
+    .read = pci_features_read,
+    .write = pciej_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s);
-    register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s);
+static const MemoryRegionOps pcirmv_hot_io_ops = {
+    .read = pcirmv_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
-    register_ioport_read(PCI_EJ_BASE, 4, 4,  pci_features_read, s);
+static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
+{
 
-    register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
+    memory_region_init_io(&s->acpi_hot_io, &acpi_hot_io_ops, s,
+                          "piix4-acpi-hot", GPE_LEN);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), GPE_BASE,
+                                &s->acpi_hot_io);
+    acpi_gpe_blk(&s->ar, 0);
+
+    portio_list_init(&s->pci_hot_port_list, pci_hot_portio_list, s,
+                     "piix4-pci-hot");
+    portio_list_add(&s->pci_hot_port_list, pci_address_space_io(&s->dev),
+                    PCI_BASE);
+
+    memory_region_init_io(&s->pciej_hot_io, &pciej_hot_io_ops, s,
+                          "piix4-pciej-hot", 4);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), PCI_EJ_BASE,
+                                &s->pciej_hot_io);
+
+    memory_region_init_io(&s->pcirmv_hot_io, &pcirmv_hot_io_ops, s,
+                          "piix4-pcirmv-hot", 4);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), PCI_RMV_BASE,
+                                &s->pcirmv_hot_io);
 
     pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
 }

Signature · 1 line

Julien Grall

Jan Kiszka

2012-08-23 18:01 UTC #6 lore raw

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

Quoted 8 lines · Julien Grall

This patch replaces all register_ioport* with the new memory API. It permits to use the new Memory stuff like listener.
Signed-off-by Julien Grall <[email protected]>
---
 hw/acpi_piix4.c |  160 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 124 insertions(+), 36 deletions(-)

...

hw/acpi_piix4.c static void pm_io_space_update(PIIX4PMState *s) · line 218

Quoted 10 lines

hw/acpi_piix4.c +4 -2

@@ -198,8 +212,10 @@ static void pm_io_space_update(PIIX4PMState *s)
 
         /* XXX: need to improve memory and ioport allocation */
         PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
-        iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
-        ioport_register(&s->ioport);
+
+        memory_region_init_io(&s->pm_io, &pm_io_ops, s, "piix4-pm", 64);
+        memory_region_add_subregion(pci_address_space_io(&s->dev),
+                                    pm_io_base, &s->pm_io);

This was broken before, but now I'm worried a guest can even crash qemu by updating an existing mapping. So you will have to
- initialize pm_io only once
- track the active address
- remove a registered region before registering a new one

Didn't look at the spec, but I bet that (config[0x80] & 1) == 0 means disable this mapping. Should be fixed as well if that is true.

Jan

Signature · 2 lines

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

Jan Kiszka

2012-08-26 09:10 UTC #7 lore raw

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

In reply to Julien Grall

Quoted 299 lines · Julien Grall

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

Signed-off-by Julien Grall <[email protected]>

---
 hw/acpi_piix4.c |  160 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 124 insertions(+), 36 deletions(-)

hw/acpi_piix4.c +117 ~~-36~~

index 0aace60..26d5559 100644
@@ -28,6 +28,7 @@
 #include "range.h"
 #include "ioport.h"
 #include "fw_cfg.h"
+#include "exec-memory.h"
 
 //#define DEBUG

@@ -41,8 +42,7 @@
 
 #define GPE_BASE 0xafe0
 #define GPE_LEN 4
-#define PCI_UP_BASE 0xae00
-#define PCI_DOWN_BASE 0xae04
+#define PCI_BASE 0xaa00
 #define PCI_EJ_BASE 0xae08
 #define PCI_RMV_BASE 0xae0c

@@ -55,7 +55,7 @@ struct pci_status {
 
 typedef struct PIIX4PMState {
     PCIDevice dev;
-    IORange ioport;
+    MemoryRegion pm_io;
     ACPIREGS ar;
 
     APMState apm;

@@ -63,6 +63,13 @@ typedef struct PIIX4PMState {
     PMSMBus smb;
     uint32_t smb_io_base;
 
+    MemoryRegion smb_io;
+    MemoryRegion acpi_io;
+    MemoryRegion acpi_hot_io;
+    PortioList pci_hot_port_list;
+    MemoryRegion pciej_hot_io;
+    MemoryRegion pcirmv_hot_io;
+
     qemu_irq irq;
     qemu_irq smi_irq;
     int kvm_enabled;

@@ -108,12 +115,12 @@ static void pm_tmr_timer(ACPIREGS *ar)
     pm_update_sci(s);
 }
 
-static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
-                            uint64_t val)
+static void pm_ioport_write(void *opaque, target_phys_addr_t addr,
+                            uint64_t val, unsigned size)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
+    PIIX4PMState *s = opaque;
 
-    if (width != 2) {
+    if (size != 2) {
         PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
                       (unsigned)addr, width, (unsigned)val);
     }

@@ -137,11 +144,11 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
                   (unsigned int)val);
 }
 
-static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
-                            uint64_t *data)
+static uint64_t pm_ioport_read(void *opaque, target_phys_addr_t addr,
+                               unsigned size)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
-    uint32_t val;
+    PIIX4PMState *s = opaque;
+    uint64_t val;
 
     switch(addr) {
     case 0x00:

@@ -161,12 +168,18 @@ static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
         break;
     }
     PIIX4_DPRINTF("PM readw port=0x%04x val=0x%04x\n", (unsigned int)addr, val);
-    *data = val;
+
+    return val;
 }
 
-static const IORangeOps pm_iorange_ops = {
+static const MemoryRegionOps pm_io_ops = {
     .read = pm_ioport_read,
     .write = pm_ioport_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 2,
+        .max_access_size = 2,
+    },
 };
 
 static void apm_ctrl_changed(uint32_t val, void *arg)

@@ -183,7 +196,8 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
     }
 }
 
-static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val)
+static void acpi_dbg_writel(void *opaque, target_phys_addr_t addr,
+                            uint64_t val, unsigned size)
 {
     PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
 }

@@ -198,8 +212,10 @@ static void pm_io_space_update(PIIX4PMState *s)
 
         /* XXX: need to improve memory and ioport allocation */
         PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
-        iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
-        ioport_register(&s->ioport);
+
+        memory_region_init_io(&s->pm_io, &pm_io_ops, s, "piix4-pm", 64);
+        memory_region_add_subregion(pci_address_space_io(&s->dev),
+                                    pm_io_base, &s->pm_io);
     }
 }

@@ -381,6 +397,25 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque)
 
 }
 
+static const MemoryRegionOps smb_io_ops = {
+    .read = smb_ioport_readb,
+    .write = smb_ioport_writeb,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static const MemoryRegionOps acpi_io_ops = {
+    .write = acpi_dbg_writel,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
 static int piix4_pm_initfn(PCIDevice *dev)
 {
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);

@@ -395,9 +430,11 @@ 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);
 
-    register_ioport_write(ACPI_DBG_IO_ADDR, 4, 4, acpi_dbg_writel, s);
+    memory_region_init_io(&s->acpi_io, &acpi_io_ops, s, "piix4-acpi", 4);
+    memory_region_add_subregion(pci_address_space_io(dev), ACPI_DBG_IO_ADDR,
+                                &s->acpi_io);
 
     if (s->kvm_enabled) {
         /* Mark SMM as already inited to prevent SMM from running.  KVM does not

@@ -410,8 +447,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);
+
+    memory_region_init_io(&s->smb_io, &smb_io_ops, &s->smb, "piix4-smb", 64);
+    memory_region_add_subregion(pci_address_space_io(dev), s->smb_io_base,
+                                &s->smb_io);
 
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer);
     acpi_gpe_init(&s->ar, GPE_LEN);

@@ -496,16 +535,17 @@ static void piix4_pm_register_types(void)
 
 type_init(piix4_pm_register_types)
 
-static uint32_t gpe_readb(void *opaque, uint32_t addr)
+static uint64_t gpe_readb(void *opaque, target_phys_addr_t addr, unsigned size)
 {
     PIIX4PMState *s = opaque;
-    uint32_t val = acpi_gpe_ioport_readb(&s->ar, addr);
+    uint64_t val = acpi_gpe_ioport_readb(&s->ar, addr);
 
     PIIX4_DPRINTF("gpe read %x == %x\n", addr, val);
     return val;
 }
 
-static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void gpe_writeb(void *opaque, target_phys_addr_t addr, uint64_t val,
+                       unsigned size)
 {
     PIIX4PMState *s = opaque;

@@ -537,21 +577,24 @@ static uint32_t pci_down_read(void *opaque, uint32_t addr)
     return val;
 }
 
-static uint32_t pci_features_read(void *opaque, uint32_t addr)
+static uint64_t pci_features_read(void *opaque, target_phys_addr_t addr,
+                                  unsigned size)
 {
     /* 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)
+static void pciej_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+                        unsigned size)
 {
     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 uint64_t pcirmv_read(void *opaque, target_phys_addr_t addr,
+                            unsigned size)
 {
     PIIX4PMState *s = opaque;

@@ -561,20 +604,65 @@ static uint32_t pcirmv_read(void *opaque, uint32_t addr)
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
                                 PCIHotplugState state);
 
-static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
-{
+static const MemoryRegionOps acpi_hot_io_ops = {
+    .read = gpe_readb,
+    .write = gpe_writeb,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+/* PCI hot plug registers */
+static const MemoryRegionPortio pci_hot_portio_list[] = {
+    { 0x00, 4, 4, .read = pci_up_read, }, /* 0xae00 */
+    { 0x04, 4, 4, .read = pci_down_read, }, /* 0xae04 */
+    PORTIO_END_OF_LIST(),
+};
 
-    register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s);
-    register_ioport_read(GPE_BASE, GPE_LEN, 1,  gpe_readb, s);
-    acpi_gpe_blk(&s->ar, GPE_BASE);
+static const MemoryRegionOps pciej_hot_io_ops = {
+    .read = pci_features_read,
+    .write = pciej_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s);
-    register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s);
+static const MemoryRegionOps pcirmv_hot_io_ops = {
+    .read = pcirmv_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
-    register_ioport_read(PCI_EJ_BASE, 4, 4,  pci_features_read, s);
+static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
+{
 
-    register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
+    memory_region_init_io(&s->acpi_hot_io, &acpi_hot_io_ops, s,
+                          "piix4-acpi-hot", GPE_LEN);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), GPE_BASE,
+                                &s->acpi_hot_io);
+    acpi_gpe_blk(&s->ar, 0);
+
+    portio_list_init(&s->pci_hot_port_list, pci_hot_portio_list, s,
+                     "piix4-pci-hot");
+    portio_list_add(&s->pci_hot_port_list, pci_address_space_io(&s->dev),
+                    PCI_BASE);
+
+    memory_region_init_io(&s->pciej_hot_io, &pciej_hot_io_ops, s,
+                          "piix4-pciej-hot", 4);

hw/acpi_piix4.c static uint32_t pcirmv_read(void *opaque, uint32_t addr) · line 668

Quoted 10 lines

hw/acpi_piix4.c +7 -0

+    memory_region_add_subregion(pci_address_space_io(&s->dev), PCI_EJ_BASE,
+                                &s->pciej_hot_io);
+
+    memory_region_init_io(&s->pcirmv_hot_io, &pcirmv_hot_io_ops, s,
+                          "piix4-pcirmv-hot", 4);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), PCI_RMV_BASE,
+                                &s->pcirmv_hot_io);
 
     pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
 }

This patch doesn't build without patch 8 and then still generates warnings.

Jan

Jan Kiszka

2012-08-26 09:36 UTC #8 lore raw

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

Quoted 307 lines · Jan Kiszka

Quoted 306 lines · Julien Grall

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

Signed-off-by Julien Grall <[email protected]>

---
 hw/acpi_piix4.c |  160 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 124 insertions(+), 36 deletions(-)

hw/acpi_piix4.c +124 ~~-36~~

index 0aace60..26d5559 100644
@@ -28,6 +28,7 @@
 #include "range.h"
 #include "ioport.h"
 #include "fw_cfg.h"
+#include "exec-memory.h"
 
 //#define DEBUG

@@ -41,8 +42,7 @@
 
 #define GPE_BASE 0xafe0
 #define GPE_LEN 4
-#define PCI_UP_BASE 0xae00
-#define PCI_DOWN_BASE 0xae04
+#define PCI_BASE 0xaa00
 #define PCI_EJ_BASE 0xae08
 #define PCI_RMV_BASE 0xae0c

@@ -55,7 +55,7 @@ struct pci_status {
 
 typedef struct PIIX4PMState {
     PCIDevice dev;
-    IORange ioport;
+    MemoryRegion pm_io;
     ACPIREGS ar;
 
     APMState apm;

@@ -63,6 +63,13 @@ typedef struct PIIX4PMState {
     PMSMBus smb;
     uint32_t smb_io_base;
 
+    MemoryRegion smb_io;
+    MemoryRegion acpi_io;
+    MemoryRegion acpi_hot_io;
+    PortioList pci_hot_port_list;
+    MemoryRegion pciej_hot_io;
+    MemoryRegion pcirmv_hot_io;
+
     qemu_irq irq;
     qemu_irq smi_irq;
     int kvm_enabled;

@@ -108,12 +115,12 @@ static void pm_tmr_timer(ACPIREGS *ar)
     pm_update_sci(s);
 }
 
-static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
-                            uint64_t val)
+static void pm_ioport_write(void *opaque, target_phys_addr_t addr,
+                            uint64_t val, unsigned size)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
+    PIIX4PMState *s = opaque;
 
-    if (width != 2) {
+    if (size != 2) {
         PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
                       (unsigned)addr, width, (unsigned)val);
     }

@@ -137,11 +144,11 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, unsigned width,
                   (unsigned int)val);
 }
 
-static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
-                            uint64_t *data)
+static uint64_t pm_ioport_read(void *opaque, target_phys_addr_t addr,
+                               unsigned size)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
-    uint32_t val;
+    PIIX4PMState *s = opaque;
+    uint64_t val;
 
     switch(addr) {
     case 0x00:

@@ -161,12 +168,18 @@ static void pm_ioport_read(IORange *ioport, uint64_t addr, unsigned width,
         break;
     }
     PIIX4_DPRINTF("PM readw port=0x%04x val=0x%04x\n", (unsigned int)addr, val);
-    *data = val;
+
+    return val;
 }
 
-static const IORangeOps pm_iorange_ops = {
+static const MemoryRegionOps pm_io_ops = {
     .read = pm_ioport_read,
     .write = pm_ioport_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 2,
+        .max_access_size = 2,
+    },
 };
 
 static void apm_ctrl_changed(uint32_t val, void *arg)

@@ -183,7 +196,8 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
     }
 }
 
-static void acpi_dbg_writel(void *opaque, uint32_t addr, uint32_t val)
+static void acpi_dbg_writel(void *opaque, target_phys_addr_t addr,
+                            uint64_t val, unsigned size)
 {
     PIIX4_DPRINTF("ACPI: DBG: 0x%08x\n", val);
 }

@@ -198,8 +212,10 @@ static void pm_io_space_update(PIIX4PMState *s)
 
         /* XXX: need to improve memory and ioport allocation */
         PIIX4_DPRINTF("PM: mapping to 0x%x\n", pm_io_base);
-        iorange_init(&s->ioport, &pm_iorange_ops, pm_io_base, 64);
-        ioport_register(&s->ioport);
+
+        memory_region_init_io(&s->pm_io, &pm_io_ops, s, "piix4-pm", 64);
+        memory_region_add_subregion(pci_address_space_io(&s->dev),
+                                    pm_io_base, &s->pm_io);
     }
 }

@@ -381,6 +397,25 @@ static void piix4_pm_machine_ready(Notifier *n, void *opaque)
 
 }
 
+static const MemoryRegionOps smb_io_ops = {
+    .read = smb_ioport_readb,
+    .write = smb_ioport_writeb,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static const MemoryRegionOps acpi_io_ops = {
+    .write = acpi_dbg_writel,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
 static int piix4_pm_initfn(PCIDevice *dev)
 {
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);

@@ -395,9 +430,11 @@ 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);
 
-    register_ioport_write(ACPI_DBG_IO_ADDR, 4, 4, acpi_dbg_writel, s);
+    memory_region_init_io(&s->acpi_io, &acpi_io_ops, s, "piix4-acpi", 4);
+    memory_region_add_subregion(pci_address_space_io(dev), ACPI_DBG_IO_ADDR,
+                                &s->acpi_io);
 
     if (s->kvm_enabled) {
         /* Mark SMM as already inited to prevent SMM from running.  KVM does not

@@ -410,8 +447,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);
+
+    memory_region_init_io(&s->smb_io, &smb_io_ops, &s->smb, "piix4-smb", 64);
+    memory_region_add_subregion(pci_address_space_io(dev), s->smb_io_base,
+                                &s->smb_io);
 
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer);
     acpi_gpe_init(&s->ar, GPE_LEN);

@@ -496,16 +535,17 @@ static void piix4_pm_register_types(void)
 
 type_init(piix4_pm_register_types)
 
-static uint32_t gpe_readb(void *opaque, uint32_t addr)
+static uint64_t gpe_readb(void *opaque, target_phys_addr_t addr, unsigned size)
 {
     PIIX4PMState *s = opaque;
-    uint32_t val = acpi_gpe_ioport_readb(&s->ar, addr);
+    uint64_t val = acpi_gpe_ioport_readb(&s->ar, addr);
 
     PIIX4_DPRINTF("gpe read %x == %x\n", addr, val);
     return val;
 }
 
-static void gpe_writeb(void *opaque, uint32_t addr, uint32_t val)
+static void gpe_writeb(void *opaque, target_phys_addr_t addr, uint64_t val,
+                       unsigned size)
 {
     PIIX4PMState *s = opaque;

@@ -537,21 +577,24 @@ static uint32_t pci_down_read(void *opaque, uint32_t addr)
     return val;
 }
 
-static uint32_t pci_features_read(void *opaque, uint32_t addr)
+static uint64_t pci_features_read(void *opaque, target_phys_addr_t addr,
+                                  unsigned size)
 {
     /* 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)
+static void pciej_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+                        unsigned size)
 {
     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 uint64_t pcirmv_read(void *opaque, target_phys_addr_t addr,
+                            unsigned size)
 {
     PIIX4PMState *s = opaque;

@@ -561,20 +604,65 @@ static uint32_t pcirmv_read(void *opaque, uint32_t addr)
 static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
                                 PCIHotplugState state);
 
-static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
-{
+static const MemoryRegionOps acpi_hot_io_ops = {
+    .read = gpe_readb,
+    .write = gpe_writeb,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+/* PCI hot plug registers */
+static const MemoryRegionPortio pci_hot_portio_list[] = {
+    { 0x00, 4, 4, .read = pci_up_read, }, /* 0xae00 */
+    { 0x04, 4, 4, .read = pci_down_read, }, /* 0xae04 */
+    PORTIO_END_OF_LIST(),
+};
 
-    register_ioport_write(GPE_BASE, GPE_LEN, 1, gpe_writeb, s);
-    register_ioport_read(GPE_BASE, GPE_LEN, 1,  gpe_readb, s);
-    acpi_gpe_blk(&s->ar, GPE_BASE);
+static const MemoryRegionOps pciej_hot_io_ops = {
+    .read = pci_features_read,
+    .write = pciej_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s);
-    register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s);
+static const MemoryRegionOps pcirmv_hot_io_ops = {
+    .read = pcirmv_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
 
-    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
-    register_ioport_read(PCI_EJ_BASE, 4, 4,  pci_features_read, s);
+static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
+{
 
-    register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
+    memory_region_init_io(&s->acpi_hot_io, &acpi_hot_io_ops, s,
+                          "piix4-acpi-hot", GPE_LEN);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), GPE_BASE,
+                                &s->acpi_hot_io);
+    acpi_gpe_blk(&s->ar, 0);
+
+    portio_list_init(&s->pci_hot_port_list, pci_hot_portio_list, s,
+                     "piix4-pci-hot");
+    portio_list_add(&s->pci_hot_port_list, pci_address_space_io(&s->dev),
+                    PCI_BASE);
+
+    memory_region_init_io(&s->pciej_hot_io, &pciej_hot_io_ops, s,
+                          "piix4-pciej-hot", 4);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), PCI_EJ_BASE,
+                                &s->pciej_hot_io);
+
+    memory_region_init_io(&s->pcirmv_hot_io, &pcirmv_hot_io_ops, s,
+                          "piix4-pcirmv-hot", 4);
+    memory_region_add_subregion(pci_address_space_io(&s->dev), PCI_RMV_BASE,
+                                &s->pcirmv_hot_io);

Quoted 5 lines

hw/acpi_piix4.c static uint32_t pcirmv_read(void *opaque, uint32_t addr) · line 668
Quoted 3 lines
hw/acpi_piix4.c +0 -0
     pci_bus_hotplug(bus, piix4_device_hotplug, &s->dev.qdev);
 }
This patch doesn't build without patch 8 and then still generates warnings.

Sorry, it depends on patches 7 & 8. Then the warnings are gone as well. Anyway, every patch has to build and work on its own.

Jan

Julien Grall

author

2012-08-22 18:51 UTC #9 lore raw

PATCH v5 3/8 hw/cirrus_vga.c: replace register_ioport*

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

---
 hw/cirrus_vga.c |   42 ++++++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 18 deletions(-)

hw/cirrus_vga.c +24 ~~-18~~

index e8dcc6b..adfc855 100644
@@ -200,6 +200,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
 typedef struct CirrusVGAState {
     VGACommonState vga;
 
+    MemoryRegion cirrus_vga_io;
     MemoryRegion cirrus_linear_io;
     MemoryRegion cirrus_linear_bitblt_io;
     MemoryRegion cirrus_mmio_io;

@@ -2528,12 +2529,15 @@ static uint32_t cirrus_vga_ioport_read(void *opaque, uint32_t addr)
     return val;
 }
 
-static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void cirrus_vga_ioport_write(void *opaque, target_phys_addr_t addr,
+                                    uint64_t val, unsigned size)
 {
     CirrusVGAState *c = opaque;
     VGACommonState *s = &c->vga;
     int index;
 
+    addr += 0x3b0;
+
     /* check port range access depending on color/monochrome mode */
     if (vga_ioport_invalid(s, addr)) {
 	return;

@@ -2657,7 +2661,7 @@ static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
     if (addr >= 0x100) {
 	cirrus_mmio_blt_write(s, addr - 0x100, val);
     } else {
-        cirrus_vga_ioport_write(s, addr + 0x3c0, val);
+        cirrus_vga_ioport_write(s, addr + 0x10, val, size);
     }
 }

@@ -2783,8 +2787,18 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
     },
 };
 
+static const MemoryRegionOps cirrus_vga_io_ops = {
+    .write = cirrus_vga_ioport_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 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;

@@ -2816,19 +2830,10 @@ 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);
+    /* Register ioport 0x3b0 - 0x3df */
+    memory_region_init_io(&s->cirrus_vga_io, &cirrus_vga_io_ops, s,
+                          "cirrus-io", 0x30);
+    memory_region_add_subregion(system_io, 0x3b0, &s->cirrus_vga_io);
 
     memory_region_init(&s->low_mem_container,
                        "cirrus-lowmem-container",

@@ -2896,7 +2901,7 @@ static int vga_initfn(ISADevice *dev)
     s->vram_size_mb = VGA_RAM_SIZE >> 20;
     vga_common_init(s);
     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);

@@ -2938,7 +2943,8 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
      /* setup VGA */
      s->vga.vram_size_mb = VGA_RAM_SIZE >> 20;
      vga_common_init(&s->vga);
-     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);

Signature · 1 line

Julien Grall

Jan Kiszka

2012-08-24 13:44 UTC #10 lore raw

Re: [Qemu-devel] [PATCH V5 3/8] hw/cirrus_vga.c: replace register_ioport*

Quoted 77 lines · 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 <[email protected]>

---
 hw/cirrus_vga.c |   42 ++++++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 18 deletions(-)

hw/cirrus_vga.c +17 ~~-10~~

index e8dcc6b..adfc855 100644
@@ -200,6 +200,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
 typedef struct CirrusVGAState {
     VGACommonState vga;
 
+    MemoryRegion cirrus_vga_io;
     MemoryRegion cirrus_linear_io;
     MemoryRegion cirrus_linear_bitblt_io;
     MemoryRegion cirrus_mmio_io;

@@ -2528,12 +2529,15 @@ static uint32_t cirrus_vga_ioport_read(void *opaque, uint32_t addr)
     return val;
 }
 
-static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void cirrus_vga_ioport_write(void *opaque, target_phys_addr_t addr,
+                                    uint64_t val, unsigned size)
 {
     CirrusVGAState *c = opaque;
     VGACommonState *s = &c->vga;
     int index;
 
+    addr += 0x3b0;
+
     /* check port range access depending on color/monochrome mode */
     if (vga_ioport_invalid(s, addr)) {
 	return;

@@ -2657,7 +2661,7 @@ static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
     if (addr >= 0x100) {
 	cirrus_mmio_blt_write(s, addr - 0x100, val);
     } else {
-        cirrus_vga_ioport_write(s, addr + 0x3c0, val);
+        cirrus_vga_ioport_write(s, addr + 0x10, val, size);
     }
 }

@@ -2783,8 +2787,18 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
     },
 };
 
+static const MemoryRegionOps cirrus_vga_io_ops = {
+    .write = cirrus_vga_ioport_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 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;

@@ -2816,19 +2830,10 @@ 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);
-

hw/cirrus_vga.c static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci, · line 2836

Quoted 10 lines

hw/cirrus_vga.c +4 -6

-    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);
+    /* Register ioport 0x3b0 - 0x3df */
+    memory_region_init_io(&s->cirrus_vga_io, &cirrus_vga_io_ops, s,
+                          "cirrus-io", 0x30);
+    memory_region_add_subregion(system_io, 0x3b0, &s->cirrus_vga_io);

Can't imagine that this reflects the original ranges and constraints correctly. Or were they all wrong?

Jan

Signature · 2 lines

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

Julien Grall

author

2012-08-24 14:48 UTC #11 lore raw

Re: [Qemu-devel] [PATCH V5 3/8] hw/cirrus_vga.c: replace register_ioport*

Quoted 84 lines · Jan Kiszka

Quoted 82 lines · 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<[email protected]>

---
  hw/cirrus_vga.c |   42 ++++++++++++++++++++++++------------------
  1 files changed, 24 insertions(+), 18 deletions(-)

hw/cirrus_vga.c +17 ~~-15~~

index e8dcc6b..adfc855 100644
@@ -200,6 +200,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
  typedef struct CirrusVGAState {
      VGACommonState vga;
 
+    MemoryRegion cirrus_vga_io;
      MemoryRegion cirrus_linear_io;
      MemoryRegion cirrus_linear_bitblt_io;
      MemoryRegion cirrus_mmio_io;

@@ -2528,12 +2529,15 @@ static uint32_t cirrus_vga_ioport_read(void *opaque, uint32_t addr)
      return val;
  }
 
-static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void cirrus_vga_ioport_write(void *opaque, target_phys_addr_t addr,
+                                    uint64_t val, unsigned size)
  {
      CirrusVGAState *c = opaque;
      VGACommonState *s =&c->vga;
      int index;
 
+    addr += 0x3b0;
+
      /* check port range access depending on color/monochrome mode */
      if (vga_ioport_invalid(s, addr)) {
  	return;

@@ -2657,7 +2661,7 @@ static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
      if (addr>= 0x100) {
  	cirrus_mmio_blt_write(s, addr - 0x100, val);
      } else {
-        cirrus_vga_ioport_write(s, addr + 0x3c0, val);
+        cirrus_vga_ioport_write(s, addr + 0x10, val, size);
      }
  }

@@ -2783,8 +2787,18 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
      },
  };
 
+static const MemoryRegionOps cirrus_vga_io_ops = {
+    .write = cirrus_vga_ioport_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
  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;

@@ -2816,19 +2830,10 @@ 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);

Quoted 10 lines

hw/cirrus_vga.c static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci, · line 2837
Quoted 6 lines
hw/cirrus_vga.c +4 -1
-    register_ioport_read(0x3da, 1, 1, cirrus_vga_ioport_read, s);
+    /* Register ioport 0x3b0 - 0x3df */
+    memory_region_init_io(&s->cirrus_vga_io,&cirrus_vga_io_ops, s,
+                          "cirrus-io", 0x30);
+    memory_region_add_subregion(system_io, 0x3b0,&s->cirrus_vga_io);
 
Can't imagine that this reflects the original ranges and constraints correctly. Or were they all wrong?

I made a version (V4) with the same mapping, but Anthony has proposed to register 0x3b0 - 0x3df
(https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03329.html)

I don't see a problem, and it works on my computer.

By the way, I will resend this patch because I forget read access in MemoryRegionOps. Sorry.

Quoted 3 lines

Jan

Jan Kiszka

2012-08-24 15:01 UTC #12 lore raw

Re: [Qemu-devel] [PATCH V5 3/8] hw/cirrus_vga.c: replace register_ioport*

Quoted 96 lines · Julien Grall

Quoted 84 lines · Jan Kiszka

Quoted 82 lines · 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<[email protected]>

---
  hw/cirrus_vga.c |   42 ++++++++++++++++++++++++------------------
  1 files changed, 24 insertions(+), 18 deletions(-)

hw/cirrus_vga.c +17 ~~-15~~

index e8dcc6b..adfc855 100644
@@ -200,6 +200,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
  typedef struct CirrusVGAState {
      VGACommonState vga;
 
+    MemoryRegion cirrus_vga_io;
      MemoryRegion cirrus_linear_io;
      MemoryRegion cirrus_linear_bitblt_io;
      MemoryRegion cirrus_mmio_io;

@@ -2528,12 +2529,15 @@ static uint32_t cirrus_vga_ioport_read(void *opaque, uint32_t addr)
      return val;
  }
 
-static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void cirrus_vga_ioport_write(void *opaque, target_phys_addr_t addr,
+                                    uint64_t val, unsigned size)
  {
      CirrusVGAState *c = opaque;
      VGACommonState *s =&c->vga;
      int index;
 
+    addr += 0x3b0;
+
      /* check port range access depending on color/monochrome mode */
      if (vga_ioport_invalid(s, addr)) {
  	return;

@@ -2657,7 +2661,7 @@ static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
      if (addr>= 0x100) {
  	cirrus_mmio_blt_write(s, addr - 0x100, val);
      } else {
-        cirrus_vga_ioport_write(s, addr + 0x3c0, val);
+        cirrus_vga_ioport_write(s, addr + 0x10, val, size);
      }
  }

@@ -2783,8 +2787,18 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
      },
  };
 
+static const MemoryRegionOps cirrus_vga_io_ops = {
+    .write = cirrus_vga_ioport_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
  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;

@@ -2816,19 +2830,10 @@ 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);

Quoted 10 lines

hw/cirrus_vga.c static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci, · line 2837
Quoted 6 lines
hw/cirrus_vga.c +4 -1
-    register_ioport_read(0x3da, 1, 1, cirrus_vga_ioport_read, s);
+    /* Register ioport 0x3b0 - 0x3df */
+    memory_region_init_io(&s->cirrus_vga_io,&cirrus_vga_io_ops, s,
+                          "cirrus-io", 0x30);
+    memory_region_add_subregion(system_io, 0x3b0,&s->cirrus_vga_io);
 
Can't imagine that this reflects the original ranges and constraints correctly. Or were they all wrong?

Quoted 3 lines

I made a version (V4) with the same mapping, but Anthony has proposed to register 0x3b0 - 0x3df
(https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03329.html)

Yes, likely no problem, the handlers seem to catch invalid accesses.

Quoted 5 lines

I don't see a problem, and it works on my computer.
By the way, I will resend this patch because I forget read access in MemoryRegionOps. Sorry.

Well, the fix for patch 2 is also still required. ;)

I'm currently working on removing the remaining register_ioport users as I'd like to tweak the portio interface. I will follow up on your series.

Jan

Signature · 2 lines

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

Jan Kiszka

2012-08-26 09:21 UTC #13 lore raw

Re: [Qemu-devel] [PATCH V5 3/8] hw/cirrus_vga.c: replace register_ioport*

In reply to Julien Grall

Quoted 47 lines · 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 <[email protected]>

---
 hw/cirrus_vga.c |   42 ++++++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 18 deletions(-)

hw/cirrus_vga.c +6 -2

index e8dcc6b..adfc855 100644
@@ -200,6 +200,7 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
 typedef struct CirrusVGAState {
     VGACommonState vga;
 
+    MemoryRegion cirrus_vga_io;
     MemoryRegion cirrus_linear_io;
     MemoryRegion cirrus_linear_bitblt_io;
     MemoryRegion cirrus_mmio_io;

@@ -2528,12 +2529,15 @@ static uint32_t cirrus_vga_ioport_read(void *opaque, uint32_t addr)
     return val;
 }
 
-static void cirrus_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void cirrus_vga_ioport_write(void *opaque, target_phys_addr_t addr,
+                                    uint64_t val, unsigned size)
 {
     CirrusVGAState *c = opaque;
     VGACommonState *s = &c->vga;
     int index;
 
+    addr += 0x3b0;
+
     /* check port range access depending on color/monochrome mode */
     if (vga_ioport_invalid(s, addr)) {
 	return;

@@ -2657,7 +2661,7 @@ static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
     if (addr >= 0x100) {
 	cirrus_mmio_blt_write(s, addr - 0x100, val);
     } else {
-        cirrus_vga_ioport_write(s, addr + 0x3c0, val);
+        cirrus_vga_ioport_write(s, addr + 0x10, val, size);
     }
 }

hw/cirrus_vga.c static const MemoryRegionOps cirrus_linear_io_ops = { · line 2791

Quoted 5 lines

hw/cirrus_vga.c +2 -0

@@ -2783,8 +2787,18 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
     },
 };
 
+static const MemoryRegionOps cirrus_vga_io_ops = {
+    .write = cirrus_vga_ioport_write,

Missing .read. Crashes immediately when you test it.

Jan

Julien Grall

author

2012-08-22 18:51 UTC #14 lore raw

PATCH v5 4/8 hw/serial.c: replace register_ioport*

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

---
 hw/mips_mipssim.c |    3 ++-
 hw/pc.h           |    2 +-
 hw/serial.c       |    8 +++++---
 3 files changed, 8 insertions(+), 5 deletions(-)

hw/mips_mipssim.c +2 -1

index 830f635..a204ab1 100644
@@ -215,7 +215,8 @@ mips_mipssim_init (ram_addr_t ram_size,
     /* A single 16450 sits at offset 0x3f8. It is attached to
        MIPS CPU INT2, which is interrupt 4. */
     if (serial_hds[0])
-        serial_init(0x3f8, env->irq[4], 115200, serial_hds[0]);
+        serial_init(0x3f8, env->irq[4], 115200, serial_hds[0],
+                    get_system_io());
 
     if (nd_table[0].used)
         /* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */

hw/pc.h +1 -1

index e4db071..f2b7af5 100644
@@ -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,

hw/serial.c +5 -3

index a421d1e..4ed20c0 100644
@@ -28,6 +28,7 @@
 #include "pc.h"
 #include "qemu-timer.h"
 #include "sysemu.h"
+#include "exec-memory.h"
 
 //#define DEBUG_SERIAL

@@ -810,7 +811,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;

@@ -823,8 +824,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;
 }

Signature · 1 line

Julien Grall

Julien Grall

author

2012-08-22 18:51 UTC #15 lore raw

PATCH v5 5/8 hw/pc.c: replace register_ioport*

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

---
 hw/pc.c |   58 +++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 43 insertions(+), 15 deletions(-)

hw/pc.c +43 ~~-15~~

index 112739a..f65abd8 100644
@@ -104,7 +104,8 @@ void gsi_handler(void *opaque, int n, int level)
     qemu_set_irq(s->ioapic_irq[n], level);
 }
 
-static void ioport80_write(void *opaque, uint32_t addr, uint32_t data)
+static void ioport80_write(void *opaque, target_phys_addr_t addr,
+                           uint64_t data, unsigned size)
 {
 }

@@ -122,7 +123,8 @@ void cpu_set_ferr(CPUX86State *s)
     qemu_irq_raise(ferr_irq);
 }
 
-static void ioportF0_write(void *opaque, uint32_t addr, uint32_t data)
+static void ioportF0_write(void *opaque, target_phys_addr_t addr,
+                           uint64_t data, unsigned size)
 {
     qemu_irq_lower(ferr_irq);
 }

@@ -588,6 +590,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;

@@ -595,18 +608,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);

@@ -1059,6 +1065,24 @@ static void cpu_request_exit(void *opaque, int irq, int level)
     }
 }
 
+static const MemoryRegionOps ioport80_io_ops = {
+    .write = ioport80_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static const MemoryRegionOps ioportF0_io_ops = {
+    .write = ioportF0_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice **rtc_state,
                           ISADevice **floppy,

@@ -1073,10 +1097,14 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     qemu_irq *a20_line;
     ISADevice *i8042, *port92, *vmmouse, *pit = NULL;
     qemu_irq *cpu_exit_irq;
+    MemoryRegion *ioport80_io = g_new(MemoryRegion, 1);
+    MemoryRegion *ioportF0_io = g_new(MemoryRegion, 1);
 
-    register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
+    memory_region_init_io(ioport80_io, &ioport80_io_ops, NULL, "ioport80", 1);
+    memory_region_add_subregion(isa_address_space_io(NULL), 0x80, ioport80_io);
 
-    register_ioport_write(0xf0, 1, 1, ioportF0_write, NULL);
+    memory_region_init_io(ioportF0_io, &ioportF0_io_ops, NULL, "ioportF0", 1);
+    memory_region_add_subregion(isa_address_space_io(NULL), 0xf0, ioportF0_io);
 
     /*
      * Check if an HPET shall be created.

Signature · 1 line

Julien Grall

Julien Grall

author

2012-08-22 18:51 UTC #16 lore raw

PATCH v5 6/8 hw/dma.c: replace register_ioport*

In reply to Julien Grall

This patch replaces all register_ioport* be the new memory API functions. It permits to use the new Memory stuff like listener.

Signed-off-by Julien Grall <[email protected]>

---
 hw/dma.c |  108 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 72 insertions(+), 36 deletions(-)

hw/dma.c +72 ~~-36~~

index 0a9322d..59c0dac 100644
@@ -58,6 +58,8 @@ static struct dma_cont {
     int dshift;
     struct dma_regs regs[4];
     qemu_irq *cpu_request_exit;
+    MemoryRegion channel_io;
+    MemoryRegion cont_io;
 } dma_controllers[2];
 
 enum {

@@ -149,7 +151,8 @@ static inline int getff (struct dma_cont *d)
     return ff;
 }
 
-static uint32_t read_chan (void *opaque, uint32_t nport)
+static uint64_t read_chan(void *opaque, target_phys_addr_t nport,
+                          unsigned size)
 {
     struct dma_cont *d = opaque;
     int ichan, nreg, iport, ff, val, dir;

@@ -171,7 +174,8 @@ static uint32_t read_chan (void *opaque, uint32_t nport)
     return (val >> (d->dshift + (ff << 3))) & 0xff;
 }
 
-static void write_chan (void *opaque, uint32_t nport, uint32_t data)
+static void write_chan(void *opaque, target_phys_addr_t nport, uint64_t data,
+                       unsigned size)
 {
     struct dma_cont *d = opaque;
     int iport, ichan, nreg;

@@ -189,22 +193,23 @@ static void write_chan (void *opaque, uint32_t nport, uint32_t data)
     }
 }
 
-static void write_cont (void *opaque, uint32_t nport, uint32_t data)
+static void write_cont(void *opaque, target_phys_addr_t nport, uint64_t data,
+                       unsigned size)
 {
     struct dma_cont *d = opaque;
     int iport, ichan = 0;
 
     iport = (nport >> d->dshift) & 0x0f;
     switch (iport) {
-    case 0x08:                  /* command */
+    case 0x01:                  /* command */
         if ((data != 0) && (data & CMD_NOT_SUPPORTED)) {
-            dolog ("command %#x not supported\n", data);
+            dolog("command %#lx not supported\n", data);
             return;
         }
         d->command = data;
         break;
 
-    case 0x09:
+    case 0x02:
         ichan = data & 3;
         if (data & 4) {
             d->status |= 1 << (ichan + 4);

@@ -216,7 +221,7 @@ static void write_cont (void *opaque, uint32_t nport, uint32_t data)
         DMA_run();
         break;
 
-    case 0x0a:                  /* single mask */
+    case 0x03:                  /* single mask */
         if (data & 4)
             d->mask |= 1 << (data & 3);
         else

@@ -224,7 +229,7 @@ static void write_cont (void *opaque, uint32_t nport, uint32_t data)
         DMA_run();
         break;
 
-    case 0x0b:                  /* mode */
+    case 0x04:                  /* mode */
         {
             ichan = data & 3;
 #ifdef DEBUG_DMA

@@ -243,23 +248,23 @@ static void write_cont (void *opaque, uint32_t nport, uint32_t data)
             break;
         }
 
-    case 0x0c:                  /* clear flip flop */
+    case 0x05:                  /* clear flip flop */
         d->flip_flop = 0;
         break;
 
-    case 0x0d:                  /* reset */
+    case 0x06:                  /* reset */
         d->flip_flop = 0;
         d->mask = ~0;
         d->status = 0;
         d->command = 0;
         break;
 
-    case 0x0e:                  /* clear mask for all channels */
+    case 0x07:                  /* clear mask for all channels */
         d->mask = 0;
         DMA_run();
         break;
 
-    case 0x0f:                  /* write mask for all channels */
+    case 0x08:                  /* write mask for all channels */
         d->mask = data;
         DMA_run();
         break;

@@ -277,7 +282,8 @@ static void write_cont (void *opaque, uint32_t nport, uint32_t data)
 #endif
 }
 
-static uint32_t read_cont (void *opaque, uint32_t nport)
+static uint64_t read_cont(void *opaque, target_phys_addr_t nport,
+                          unsigned size)
 {
     struct dma_cont *d = opaque;
     int iport, val;

@@ -463,7 +469,7 @@ void DMA_schedule(int nchan)
 static void dma_reset(void *opaque)
 {
     struct dma_cont *d = opaque;
-    write_cont (d, (0x0d << d->dshift), 0);
+    write_cont(d, (0x06 << d->dshift), 0, 1);
 }
 
 static int dma_phony_handler (void *opaque, int nchan, int dma_pos, int dma_len)

@@ -473,38 +479,68 @@ static int dma_phony_handler (void *opaque, int nchan, int dma_pos, int dma_len)
     return dma_pos;
 }
 
+
+static const MemoryRegionOps channel_io_ops = {
+    .read = read_chan,
+    .write = write_chan,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+/* 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(),
+};
+
+static const MemoryRegionOps cont_io_ops = {
+    .read = read_cont,
+    .write = write_cont,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 /* 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);
-    }
-    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);
-        }
-    }
-    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);
+
+    memory_region_init_io(&d->channel_io, &channel_io_ops, d,
+                          "dma-chan", 8 << d->dshift);
+    memory_region_add_subregion(isa_address_space_io(NULL),
+                                base, &d->channel_io);
+
+    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");
     }
+
+    memory_region_init_io(&d->cont_io, &cont_io_ops, d, "dma-cont",
+                          8 << d->dshift);
+    memory_region_add_subregion(isa_address_space_io(NULL),
+                                base + (8 << d->dshift), &d->cont_io);
+
     qemu_register_reset(dma_reset, d);
     dma_reset(d);
     for (i = 0; i < ARRAY_SIZE (d->regs); ++i) {

Signature · 1 line

Julien Grall

Julien Grall

author

2012-08-22 18:51 UTC #17 lore raw

PATCH v5 7/8 hw/apm.c: replace register_ioport*

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

---
 hw/apm.c      |   24 +++++++++++++++++++-----
 hw/apm.h      |    5 ++++-
 hw/vt82c686.c |    2 +-
 3 files changed, 24 insertions(+), 7 deletions(-)

hw/apm.c +19 -5

index 2aead52..fe7bc21 100644
@@ -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);
 }

hw/apm.h +4 -1

index f7c741e..5431b6d 100644
@@ -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;

hw/vt82c686.c +1 -1

index 5d7c00c..7f11dbe 100644
@@ -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);

Signature · 1 line

Julien Grall

Julien Grall

author

2012-08-22 18:50 UTC #18 lore raw

PATCH v5 8/8 smb: replace_register_ioport*

In reply to Julien Grall

This patch fix smb_ioport_* to be compliant with read/write memory callback. Moreover it replaces all register_ioport* which use theses functions by the new Memory API.

Signed-off-by Julien Grall <[email protected]>

---
 hw/pm_smbus.c |    7 ++++---
 hw/pm_smbus.h |    6 ++++--
 hw/vt82c686.c |   18 ++++++++++++++++--
 3 files changed, 24 insertions(+), 7 deletions(-)

hw/pm_smbus.c +4 -3

index 5d6046d..fe59ca6 100644
@@ -94,7 +94,8 @@ static void smb_transaction(PMSMBus *s)
     s->smb_stat |= 0x04;
 }
 
-void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
+void smb_ioport_writeb(void *opaque, target_phys_addr_t addr, uint64_t val,
+                       unsigned size)
 {
     PMSMBus *s = opaque;
     addr &= 0x3f;

@@ -131,10 +132,10 @@ void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-uint32_t smb_ioport_readb(void *opaque, uint32_t addr)
+uint64_t smb_ioport_readb(void *opaque, target_phys_addr_t addr, unsigned size)
 {
     PMSMBus *s = opaque;
-    uint32_t val;
+    uint64_t val;
 
     addr &= 0x3f;
     switch(addr) {

hw/pm_smbus.h +4 -2

index 4750a40..45b4330 100644
@@ -15,7 +15,9 @@ typedef struct PMSMBus {
 } PMSMBus;
 
 void pm_smbus_init(DeviceState *parent, PMSMBus *smb);
-void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val);
-uint32_t smb_ioport_readb(void *opaque, uint32_t addr);
+void smb_ioport_writeb(void *opaque, target_phys_addr_t addr, uint64_t val,
+                       unsigned size);
+uint64_t smb_ioport_readb(void *opaque, target_phys_addr_t addr,
+                          unsigned size);
 
 #endif /* !PM_SMBUS_H */

hw/vt82c686.c +16 -2

index 7f11dbe..6b35155 100644
@@ -163,6 +163,7 @@ typedef struct VT686PMState {
     APMState apm;
     PMSMBus smb;
     uint32_t smb_io_base;
+    MemoryRegion smb_io;
 } VT686PMState;
 
 typedef struct VT686AC97State {

@@ -405,6 +406,16 @@ static TypeInfo via_mc97_info = {
     .class_init    = via_mc97_class_init,
 };
 
+static const MemoryRegionOps smb_io_ops = {
+    .read = smb_ioport_readb,
+    .write = smb_ioport_writeb,
+    .endianess = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 /* vt82c686 pm init */
 static int vt82c686b_pm_initfn(PCIDevice *dev)
 {

@@ -424,8 +435,11 @@ static int vt82c686b_pm_initfn(PCIDevice *dev)
     pci_conf[0x90] = s->smb_io_base | 1;
     pci_conf[0x91] = s->smb_io_base >> 8;
     pci_conf[0xd2] = 0x90;
-    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);
+
+    memory_region_init_io(&s->smb_io, &smb_io_ops, &s->smb, "vt82c686b-smb",
+                          0xf);
+    memory_region_add_subregion(pci_address_space_io(dev), s->smb_io_base,
+                                &s->smb_io);
 
     apm_init(dev, &s->apm, NULL, s);

Signature · 1 line

Julien Grall

Jan Kiszka

2012-08-24 16:19 UTC #19 lore raw

Re: [Qemu-devel] [PATCH V5 8/8] smb: replace_register_ioport*

Quoted 67 lines · Julien Grall

This patch fix smb_ioport_* to be compliant with read/write memory callback. Moreover it replaces all register_ioport* which use theses functions by the new Memory API.

Signed-off-by Julien Grall <[email protected]>

---
 hw/pm_smbus.c |    7 ++++---
 hw/pm_smbus.h |    6 ++++--
 hw/vt82c686.c |   18 ++++++++++++++++--
 3 files changed, 24 insertions(+), 7 deletions(-)

hw/pm_smbus.c +4 -3

index 5d6046d..fe59ca6 100644
@@ -94,7 +94,8 @@ static void smb_transaction(PMSMBus *s)
     s->smb_stat |= 0x04;
 }
 
-void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
+void smb_ioport_writeb(void *opaque, target_phys_addr_t addr, uint64_t val,
+                       unsigned size)
 {
     PMSMBus *s = opaque;
     addr &= 0x3f;

@@ -131,10 +132,10 @@ void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-uint32_t smb_ioport_readb(void *opaque, uint32_t addr)
+uint64_t smb_ioport_readb(void *opaque, target_phys_addr_t addr, unsigned size)
 {
     PMSMBus *s = opaque;
-    uint32_t val;
+    uint64_t val;
 
     addr &= 0x3f;
     switch(addr) {

hw/pm_smbus.h +4 -2

index 4750a40..45b4330 100644
@@ -15,7 +15,9 @@ typedef struct PMSMBus {
 } PMSMBus;
 
 void pm_smbus_init(DeviceState *parent, PMSMBus *smb);
-void smb_ioport_writeb(void *opaque, uint32_t addr, uint32_t val);
-uint32_t smb_ioport_readb(void *opaque, uint32_t addr);
+void smb_ioport_writeb(void *opaque, target_phys_addr_t addr, uint64_t val,
+                       unsigned size);
+uint64_t smb_ioport_readb(void *opaque, target_phys_addr_t addr,
+                          unsigned size);
 
 #endif /* !PM_SMBUS_H */

hw/vt82c686.c +1 -0

index 7f11dbe..6b35155 100644
@@ -163,6 +163,7 @@ typedef struct VT686PMState {
     APMState apm;
     PMSMBus smb;
     uint32_t smb_io_base;
+    MemoryRegion smb_io;
 } VT686PMState;
 
 typedef struct VT686AC97State {

hw/vt82c686.c static TypeInfo via_mc97_info = { · line 412

Quoted 7 lines

hw/vt82c686.c +4 -0

@@ -405,6 +406,16 @@ static TypeInfo via_mc97_info = {
     .class_init    = via_mc97_class_init,
 };
 
+static const MemoryRegionOps smb_io_ops = {
+    .read = smb_ioport_readb,
+    .write = smb_ioport_writeb,
+    .endianess = DEVICE_NATIVE_ENDIAN,

        ^^^^^^^^^

Typo. Please ensure that you build-test everything (full QEMU build).

Quoted 14 lines

hw/vt82c686.c +6 -0

+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
 /* vt82c686 pm init */
 static int vt82c686b_pm_initfn(PCIDevice *dev)
 {

@@ -424,8 +435,11 @@ static int vt82c686b_pm_initfn(PCIDevice *dev)
     pci_conf[0x90] = s->smb_io_base | 1;
     pci_conf[0x91] = s->smb_io_base >> 8;
     pci_conf[0xd2] = 0x90;

hw/vt82c686.c static int vt82c686b_pm_initfn(PCIDevice *dev) · line 445

Quoted 10 lines

hw/vt82c686.c +5 -2

-    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);
+
+    memory_region_init_io(&s->smb_io, &smb_io_ops, &s->smb, "vt82c686b-smb",
+                          0xf);
+    memory_region_add_subregion(pci_address_space_io(dev), s->smb_io_base,
+                                &s->smb_io);
 
     apm_init(dev, &s->apm, NULL, s);

This file is only half-converted. Working on the other bits...

Jan

Signature · 2 lines

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

Jan Kiszka

2012-08-23 18:06 UTC #20 lore raw

In reply to Julien Grall

Quoted 45 lines · Julien Grall

This is the fifth version of patch series about ioport registration. The fourth version was sent few months ago
(https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01396.html).
Some part of QEMU still use register_ioport* functions to register ioport. These functions doesn't allow to use Memory Listener on it.
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
Modifications between V2 and V3:
- Remove some register_ioport_* on hw/vt82c686.c.
- Split smb ioport part in new patch.
- Still replace MemoryRegion when we have only a range of ioport.
- Fix read/write ioports prototype to be compliant with memory callback.
Modifications between V3 and V4:
- Fix compilation in hw/dma.c
- Fix address conversion (hw/dma.c, hw/acpi_piix4.c) with MemorySection. Indeed the new version use offset from MemorySection start instead of 0.
Modifications between V4 and V4:
- Rebase on qemu upstream.
- Forget some ioport_register_* in acpi_piix4.c.
- Register 0x3b0 - 0x3df range for cirrus instead of ioport by ioport.
Julien Grall (8):
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*
smb: replace_register_ioport*

Quoted 16 lines

 hw/acpi_piix4.c   |  160 +++++++++++++++++++++++++++++++++++++++++------------
 hw/apm.c          |   24 ++++++--
 hw/apm.h          |    5 +-
 hw/cirrus_vga.c   |   42 ++++++++------
 hw/dma.c          |  108 ++++++++++++++++++++++++------------
 hw/isa-bus.c      |    5 ++
 hw/isa.h          |    1 +
 hw/mips_mipssim.c |    3 +-
 hw/pc.c           |   58 ++++++++++++++-----
 hw/pc.h           |    2 +-
 hw/pm_smbus.c     |    7 +-
 hw/pm_smbus.h     |    6 +-
 hw/serial.c       |    8 ++-
 hw/vt82c686.c     |   20 ++++++-
 14 files changed, 325 insertions(+), 124 deletions(-)

Nice, only few users of register_ioport_read/write remaining after this. Hope we can drop those functions soon.

Jan

Signature · 2 lines

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