qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration
@ 2012-09-03 10:03 Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 1/8] isa: add isa_address_space_io Julien Grall
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

This is the seventh version of patch series about ioport registration.

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 V5:
   - Rebase on qemu upstream.
   - Forget some ioport_register_* in acpi_piix4.c.
   - Register 0x3b0 - 0x3df range for cirrus instead of ioport by ioport.

Modifications between V5 and V6:
   - Add read function on cirrus ioport (forget on the previous patch).
   - Rework PM memory range handling.
   - Fix PCI_BASE in acpi_piix4.c (wrong conversion during port).
   - Rewrite isa_address_space_io to use ISA bus address space.
   - Fix compilation in vt82c686.c

Modifications between V6 and V7:
   - acpi_piix4: use memory_region_set_enabled instead of a boolean. I'm not
     sure about this modification (adviced by Avi).
   - Fix device endianess in acpi_piix4 (reported by Avi).
   - Avoid dependencies between patches and reorder it (reported by Jan).
     Some code moved from acpi_piix4 patch to smb/apm patches.

Julien Grall (8):
  isa: add isa_address_space_io
  hw/apm.c: replace register_ioport*
  smb: replace_register_ioport*
  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/acpi_piix4.c   |  171 ++++++++++++++++++++++++++++++++++++++++++-----------
 hw/apm.c          |   24 ++++++--
 hw/apm.h          |    5 +-
 hw/cirrus_vga.c   |   50 +++++++++------
 hw/dma.c          |  108 ++++++++++++++++++++++-----------
 hw/isa-bus.c      |    9 +++
 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, 347 insertions(+), 125 deletions(-)

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 1/8] isa: add isa_address_space_io
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 2/8] hw/apm.c: replace register_ioport* Julien Grall
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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 |    9 +++++++++
 hw/isa.h     |    1 +
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index f9b2373..c1d8309 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -244,4 +244,13 @@ MemoryRegion *isa_address_space(ISADevice *dev)
     return get_system_memory();
 }
 
+MemoryRegion *isa_address_space_io(ISADevice *dev)
+{
+    if (dev) {
+        return isa_bus_from_device(dev)->address_space_io;
+    }
+
+    return isabus->address_space_io;
+}
+
 type_init(isabus_register_types)
diff --git a/hw/isa.h b/hw/isa.h
index dc97052..3891c1f 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -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);
-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 2/8] hw/apm.c: replace register_ioport*
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 1/8] isa: add isa_address_space_io Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 3/8] smb: replace_register_ioport* Julien Grall
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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 c56220b..fd2fc33 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -395,7 +395,7 @@ static int piix4_pm_initfn(PCIDevice *dev)
     pci_conf[0x3d] = 0x01; // interrupt pin 1
 
     /* 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);
 
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 5d7c00c..7f11dbe 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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 3/8] smb: replace_register_ioport*
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 1/8] isa: add isa_address_space_io Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 2/8] hw/apm.c: replace register_ioport* Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport* Julien Grall
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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 <julien.grall@citrix.com>
---
 hw/acpi_piix4.c |   18 ++++++++++++++++--
 hw/pm_smbus.c   |    7 ++++---
 hw/pm_smbus.h   |    6 ++++--
 hw/vt82c686.c   |   18 ++++++++++++++++--
 4 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index fd2fc33..1ad45ce 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -63,6 +63,8 @@ typedef struct PIIX4PMState {
     PMSMBus smb;
     uint32_t smb_io_base;
 
+    MemoryRegion smb_io;
+
     qemu_irq irq;
     qemu_irq smi_irq;
     int kvm_enabled;
@@ -383,6 +385,16 @@ 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 int piix4_pm_initfn(PCIDevice *dev)
 {
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
@@ -410,8 +422,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);
diff --git a/hw/pm_smbus.c b/hw/pm_smbus.c
index 5d6046d..fe59ca6 100644
--- a/hw/pm_smbus.c
+++ b/hw/pm_smbus.c
@@ -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) {
diff --git a/hw/pm_smbus.h b/hw/pm_smbus.h
index 4750a40..45b4330 100644
--- a/hw/pm_smbus.h
+++ b/hw/pm_smbus.h
@@ -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 */
diff --git a/hw/vt82c686.c b/hw/vt82c686.c
index 7f11dbe..3ad75ea 100644
--- a/hw/vt82c686.c
+++ b/hw/vt82c686.c
@@ -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,
+    .endianness = 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);
 
-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport*
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
                   ` (2 preceding siblings ...)
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 3/8] smb: replace_register_ioport* Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  2012-09-04  7:24   ` Jan Kiszka
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 5/8] hw/cirrus_vga.c: " Julien Grall
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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 <julien.grall@citrix.com>
---
 hw/acpi_piix4.c |  151 +++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 119 insertions(+), 32 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 1ad45ce..527dfc1 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -41,8 +41,7 @@
 
 #define GPE_BASE 0xafe0
 #define GPE_LEN 4
-#define PCI_UP_BASE 0xae00
-#define PCI_DOWN_BASE 0xae04
+#define PCI_BASE 0xae00
 #define PCI_EJ_BASE 0xae08
 #define PCI_RMV_BASE 0xae0c
 
@@ -55,7 +54,8 @@ struct pci_status {
 
 typedef struct PIIX4PMState {
     PCIDevice dev;
-    IORange ioport;
+    MemoryRegion pm_io;
+    uint32_t pm_io_base;
     ACPIREGS ar;
 
     APMState apm;
@@ -64,6 +64,11 @@ typedef struct PIIX4PMState {
     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;
@@ -110,10 +115,10 @@ 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 width)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
+    PIIX4PMState *s = opaque;
 
     if (width != 2) {
         PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
@@ -139,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 width)
 {
-    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
-    uint32_t val;
+    PIIX4PMState *s = opaque;
+    uint64_t val;
 
     switch(addr) {
     case 0x00:
@@ -163,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_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 2,
+        .max_access_size = 2,
+    },
 };
 
 static void apm_ctrl_changed(uint32_t val, void *arg)
@@ -185,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);
 }
@@ -200,8 +212,18 @@ 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);
+
+        if (!s->pm_io_base) {
+            memory_region_add_subregion(pci_address_space_io(&s->dev),
+                                        pm_io_base, &s->pm_io);
+        } else {
+            memory_region_set_address(&s->pm_io, pm_io_base);
+        }
+
+        s->pm_io_base = pm_io_base;
+        memory_region_set_enabled(&s->pm_io, true);
+    } else {
+        memory_region_set_enabled(&s->pm_io, false);
     }
 }
 
@@ -395,6 +417,15 @@ static const MemoryRegionOps smb_io_ops = {
     },
 };
 
+static const MemoryRegionOps acpi_io_ops = {
+    .write = acpi_dbg_writel,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
 static int piix4_pm_initfn(PCIDevice *dev)
 {
     PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev);
@@ -409,7 +440,9 @@ static int piix4_pm_initfn(PCIDevice *dev)
     /* APM */
     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
@@ -427,6 +460,11 @@ static int piix4_pm_initfn(PCIDevice *dev)
     memory_region_add_subregion(pci_address_space_io(dev), s->smb_io_base,
                                 &s->smb_io);
 
+    /* PM  */
+    memory_region_init_io(&s->pm_io, &pm_io_ops, s, "piix4-pm", 64);
+    memory_region_set_enabled(&s->pm_io, false);
+    s->pm_io_base = 0;
+
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer);
     acpi_gpe_init(&s->ar, GPE_LEN);
 
@@ -510,16 +548,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;
 
@@ -551,21 +590,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;
 
@@ -575,20 +617,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_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
 
-    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);
+/* 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_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 pciej_hot_io_ops = {
+    .read = pci_features_read,
+    .write = pciej_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+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);
 }
-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 5/8] hw/cirrus_vga.c: replace register_ioport*
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
                   ` (3 preceding siblings ...)
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport* Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 6/8] hw/serial.c: " Julien Grall
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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 |   50 ++++++++++++++++++++++++++++++--------------------
 1 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index e8dcc6b..aa81f0b 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -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;
@@ -2435,12 +2436,15 @@ static void cirrus_update_memory_access(CirrusVGAState *s)
 
 /* I/O ports */
 
-static uint32_t cirrus_vga_ioport_read(void *opaque, uint32_t addr)
+static uint64_t cirrus_vga_ioport_read(void *opaque, target_phys_addr_t addr,
+                                       unsigned size)
 {
     CirrusVGAState *c = opaque;
     VGACommonState *s = &c->vga;
     int val, index;
 
+    addr += 0x3b0;
+
     if (vga_ioport_invalid(s, addr)) {
 	val = 0xff;
     } else {
@@ -2528,12 +2532,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;
@@ -2645,7 +2652,7 @@ static uint64_t cirrus_mmio_read(void *opaque, target_phys_addr_t addr,
     if (addr >= 0x100) {
         return cirrus_mmio_blt_read(s, addr - 0x100);
     } else {
-        return cirrus_vga_ioport_read(s, addr + 0x3c0);
+        return cirrus_vga_ioport_read(s, addr + 0x10, size);
     }
 }
 
@@ -2657,7 +2664,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 +2790,19 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
     },
 };
 
+static const MemoryRegionOps cirrus_vga_io_ops = {
+    .read = cirrus_vga_ioport_read,
+    .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 +2834,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 +2905,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 +2947,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);
-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 6/8] hw/serial.c: replace register_ioport*
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
                   ` (4 preceding siblings ...)
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 5/8] hw/cirrus_vga.c: " Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 7/8] hw/pc.c: " Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 8/8] hw/dma.c: " Julien Grall
  7 siblings, 0 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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/mips_mipssim.c |    3 ++-
 hw/pc.h           |    2 +-
 hw/serial.c       |    8 +++++---
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
index 830f635..a204ab1 100644
--- a/hw/mips_mipssim.c
+++ b/hw/mips_mipssim.c
@@ -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. */
diff --git a/hw/pc.h b/hw/pc.h
index e4db071..f2b7af5 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 a421d1e..4ed20c0 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
 
@@ -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;
 }
 
-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 7/8] hw/pc.c: replace register_ioport*
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
                   ` (5 preceding siblings ...)
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 6/8] hw/serial.c: " Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 8/8] hw/dma.c: " Julien Grall
  7 siblings, 0 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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 |   58 +++++++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 112739a..4abd4e2 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -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_bus->address_space_io, 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_bus->address_space_io, 0xf0, ioportF0_io);
 
     /*
      * Check if an HPET shall be created.
-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH V7 8/8] hw/dma.c: replace register_ioport*
  2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
                   ` (6 preceding siblings ...)
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 7/8] hw/pc.c: " Julien Grall
@ 2012-09-03 10:03 ` Julien Grall
  7 siblings, 0 replies; 13+ messages in thread
From: Julien Grall @ 2012-09-03 10:03 UTC (permalink / raw)
  To: avi, jan.kiszka
  Cc: Julien Grall, kraxel, qemu-devel, afaerber, Stefano.Stabellini

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 <julien.grall@citrix.com>
---
 hw/dma.c |  108 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/hw/dma.c b/hw/dma.c
index 0a9322d..59c0dac 100644
--- a/hw/dma.c
+++ b/hw/dma.c
@@ -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) {
-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport*
  2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport* Julien Grall
@ 2012-09-04  7:24   ` Jan Kiszka
  2012-09-04 11:27     ` Julien Grall
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2012-09-04  7:24 UTC (permalink / raw)
  To: Julien Grall; +Cc: qemu-devel, Stefano Stabellini, avi, afaerber, kraxel

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

On 2012-09-03 12:03, Julien Grall wrote:
> 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 <julien.grall@citrix.com>
> ---
>  hw/acpi_piix4.c |  151 +++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 119 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 1ad45ce..527dfc1 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -41,8 +41,7 @@
>  
>  #define GPE_BASE 0xafe0
>  #define GPE_LEN 4
> -#define PCI_UP_BASE 0xae00
> -#define PCI_DOWN_BASE 0xae04
> +#define PCI_BASE 0xae00
>  #define PCI_EJ_BASE 0xae08
>  #define PCI_RMV_BASE 0xae0c
>  
> @@ -55,7 +54,8 @@ struct pci_status {
>  
>  typedef struct PIIX4PMState {
>      PCIDevice dev;
> -    IORange ioport;
> +    MemoryRegion pm_io;
> +    uint32_t pm_io_base;
>      ACPIREGS ar;
>  
>      APMState apm;
> @@ -64,6 +64,11 @@ typedef struct PIIX4PMState {
>      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;
> @@ -110,10 +115,10 @@ 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 width)
>  {
> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
> +    PIIX4PMState *s = opaque;
>  
>      if (width != 2) {
>          PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
> @@ -139,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 width)
>  {
> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
> -    uint32_t val;
> +    PIIX4PMState *s = opaque;
> +    uint64_t val;
>  
>      switch(addr) {
>      case 0x00:
> @@ -163,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_LITTLE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 2,
> +        .max_access_size = 2,
> +    },
>  };
>  
>  static void apm_ctrl_changed(uint32_t val, void *arg)
> @@ -185,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);
>  }
> @@ -200,8 +212,18 @@ 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);
> +
> +        if (!s->pm_io_base) {
> +            memory_region_add_subregion(pci_address_space_io(&s->dev),
> +                                        pm_io_base, &s->pm_io);

This doesn't work reliably. Add the region in a disabled state (ie. the
default) during init, only toggle enable and update the base here.

Jan

PS: Series now builds cleanly here.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport*
  2012-09-04  7:24   ` Jan Kiszka
@ 2012-09-04 11:27     ` Julien Grall
  2012-09-04 11:37       ` Jan Kiszka
  0 siblings, 1 reply; 13+ messages in thread
From: Julien Grall @ 2012-09-04 11:27 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: qemu-devel@nongnu.org, Stefano Stabellini, avi@redhat.com,
	afaerber@suse.de, kraxel@redhat.com

On 09/04/2012 08:24 AM, Jan Kiszka wrote:
> On 2012-09-03 12:03, Julien Grall wrote:
>    
>> 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<julien.grall@citrix.com>
>> ---
>>   hw/acpi_piix4.c |  151 +++++++++++++++++++++++++++++++++++++++++++------------
>>   1 files changed, 119 insertions(+), 32 deletions(-)
>>
>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>> index 1ad45ce..527dfc1 100644
>> --- a/hw/acpi_piix4.c
>> +++ b/hw/acpi_piix4.c
>> @@ -41,8 +41,7 @@
>>
>>   #define GPE_BASE 0xafe0
>>   #define GPE_LEN 4
>> -#define PCI_UP_BASE 0xae00
>> -#define PCI_DOWN_BASE 0xae04
>> +#define PCI_BASE 0xae00
>>   #define PCI_EJ_BASE 0xae08
>>   #define PCI_RMV_BASE 0xae0c
>>
>> @@ -55,7 +54,8 @@ struct pci_status {
>>
>>   typedef struct PIIX4PMState {
>>       PCIDevice dev;
>> -    IORange ioport;
>> +    MemoryRegion pm_io;
>> +    uint32_t pm_io_base;
>>       ACPIREGS ar;
>>
>>       APMState apm;
>> @@ -64,6 +64,11 @@ typedef struct PIIX4PMState {
>>       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;
>> @@ -110,10 +115,10 @@ 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 width)
>>   {
>> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>> +    PIIX4PMState *s = opaque;
>>
>>       if (width != 2) {
>>           PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
>> @@ -139,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 width)
>>   {
>> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>> -    uint32_t val;
>> +    PIIX4PMState *s = opaque;
>> +    uint64_t val;
>>
>>       switch(addr) {
>>       case 0x00:
>> @@ -163,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_LITTLE_ENDIAN,
>> +    .impl = {
>> +        .min_access_size = 2,
>> +        .max_access_size = 2,
>> +    },
>>   };
>>
>>   static void apm_ctrl_changed(uint32_t val, void *arg)
>> @@ -185,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);
>>   }
>> @@ -200,8 +212,18 @@ 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);
>> +
>> +        if (!s->pm_io_base) {
>> +            memory_region_add_subregion(pci_address_space_io(&s->dev),
>> +                                        pm_io_base,&s->pm_io);
>>      
> This doesn't work reliably. Add the region in a disabled state (ie. the
> default) during init, only toggle enable and update the base here.
>    
What is the default base address ? The datasheet
doesn't give default value. But the base address seems to
always be 0xb000. Is there any drawbacks to used it ?

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport*
  2012-09-04 11:27     ` Julien Grall
@ 2012-09-04 11:37       ` Jan Kiszka
  2012-09-04 11:41         ` Jan Kiszka
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2012-09-04 11:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: qemu-devel@nongnu.org, Stefano Stabellini, avi@redhat.com,
	afaerber@suse.de, kraxel@redhat.com

On 2012-09-04 13:27, Julien Grall wrote:
> On 09/04/2012 08:24 AM, Jan Kiszka wrote:
>> On 2012-09-03 12:03, Julien Grall wrote:
>>   
>>> 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<julien.grall@citrix.com>
>>> ---
>>>   hw/acpi_piix4.c |  151
>>> +++++++++++++++++++++++++++++++++++++++++++------------
>>>   1 files changed, 119 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>>> index 1ad45ce..527dfc1 100644
>>> --- a/hw/acpi_piix4.c
>>> +++ b/hw/acpi_piix4.c
>>> @@ -41,8 +41,7 @@
>>>
>>>   #define GPE_BASE 0xafe0
>>>   #define GPE_LEN 4
>>> -#define PCI_UP_BASE 0xae00
>>> -#define PCI_DOWN_BASE 0xae04
>>> +#define PCI_BASE 0xae00
>>>   #define PCI_EJ_BASE 0xae08
>>>   #define PCI_RMV_BASE 0xae0c
>>>
>>> @@ -55,7 +54,8 @@ struct pci_status {
>>>
>>>   typedef struct PIIX4PMState {
>>>       PCIDevice dev;
>>> -    IORange ioport;
>>> +    MemoryRegion pm_io;
>>> +    uint32_t pm_io_base;
>>>       ACPIREGS ar;
>>>
>>>       APMState apm;
>>> @@ -64,6 +64,11 @@ typedef struct PIIX4PMState {
>>>       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;
>>> @@ -110,10 +115,10 @@ 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 width)
>>>   {
>>> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>>> +    PIIX4PMState *s = opaque;
>>>
>>>       if (width != 2) {
>>>           PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
>>> @@ -139,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 width)
>>>   {
>>> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>>> -    uint32_t val;
>>> +    PIIX4PMState *s = opaque;
>>> +    uint64_t val;
>>>
>>>       switch(addr) {
>>>       case 0x00:
>>> @@ -163,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_LITTLE_ENDIAN,
>>> +    .impl = {
>>> +        .min_access_size = 2,
>>> +        .max_access_size = 2,
>>> +    },
>>>   };
>>>
>>>   static void apm_ctrl_changed(uint32_t val, void *arg)
>>> @@ -185,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);
>>>   }
>>> @@ -200,8 +212,18 @@ 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);
>>> +
>>> +        if (!s->pm_io_base) {
>>> +            memory_region_add_subregion(pci_address_space_io(&s->dev),
>>> +                                        pm_io_base,&s->pm_io);
>>>      
>> This doesn't work reliably. Add the region in a disabled state (ie. the
>> default) during init, only toggle enable and update the base here.
>>    
> What is the default base address ? The datasheet
> doesn't give default value. But the base address seems to
> always be 0xb000. Is there any drawbacks to used it ?

According to the spec
(http://www.intel.com/assets/pdf/datasheet/290562.pdf), the reset value
must be 0.

Jan

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport*
  2012-09-04 11:37       ` Jan Kiszka
@ 2012-09-04 11:41         ` Jan Kiszka
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2012-09-04 11:41 UTC (permalink / raw)
  To: Julien Grall
  Cc: qemu-devel@nongnu.org, Stefano Stabellini, avi@redhat.com,
	afaerber@suse.de, kraxel@redhat.com

On 2012-09-04 13:37, Jan Kiszka wrote:
> On 2012-09-04 13:27, Julien Grall wrote:
>> On 09/04/2012 08:24 AM, Jan Kiszka wrote:
>>> On 2012-09-03 12:03, Julien Grall wrote:
>>>   
>>>> 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<julien.grall@citrix.com>
>>>> ---
>>>>   hw/acpi_piix4.c |  151
>>>> +++++++++++++++++++++++++++++++++++++++++++------------
>>>>   1 files changed, 119 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>>>> index 1ad45ce..527dfc1 100644
>>>> --- a/hw/acpi_piix4.c
>>>> +++ b/hw/acpi_piix4.c
>>>> @@ -41,8 +41,7 @@
>>>>
>>>>   #define GPE_BASE 0xafe0
>>>>   #define GPE_LEN 4
>>>> -#define PCI_UP_BASE 0xae00
>>>> -#define PCI_DOWN_BASE 0xae04
>>>> +#define PCI_BASE 0xae00
>>>>   #define PCI_EJ_BASE 0xae08
>>>>   #define PCI_RMV_BASE 0xae0c
>>>>
>>>> @@ -55,7 +54,8 @@ struct pci_status {
>>>>
>>>>   typedef struct PIIX4PMState {
>>>>       PCIDevice dev;
>>>> -    IORange ioport;
>>>> +    MemoryRegion pm_io;
>>>> +    uint32_t pm_io_base;
>>>>       ACPIREGS ar;
>>>>
>>>>       APMState apm;
>>>> @@ -64,6 +64,11 @@ typedef struct PIIX4PMState {
>>>>       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;
>>>> @@ -110,10 +115,10 @@ 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 width)
>>>>   {
>>>> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>>>> +    PIIX4PMState *s = opaque;
>>>>
>>>>       if (width != 2) {
>>>>           PIIX4_DPRINTF("PM write port=0x%04x width=%d val=0x%08x\n",
>>>> @@ -139,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 width)
>>>>   {
>>>> -    PIIX4PMState *s = container_of(ioport, PIIX4PMState, ioport);
>>>> -    uint32_t val;
>>>> +    PIIX4PMState *s = opaque;
>>>> +    uint64_t val;
>>>>
>>>>       switch(addr) {
>>>>       case 0x00:
>>>> @@ -163,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_LITTLE_ENDIAN,
>>>> +    .impl = {
>>>> +        .min_access_size = 2,
>>>> +        .max_access_size = 2,
>>>> +    },
>>>>   };
>>>>
>>>>   static void apm_ctrl_changed(uint32_t val, void *arg)
>>>> @@ -185,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);
>>>>   }
>>>> @@ -200,8 +212,18 @@ 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);
>>>> +
>>>> +        if (!s->pm_io_base) {
>>>> +            memory_region_add_subregion(pci_address_space_io(&s->dev),
>>>> +                                        pm_io_base,&s->pm_io);
>>>>      
>>> This doesn't work reliably. Add the region in a disabled state (ie. the
>>> default) during init, only toggle enable and update the base here.
>>>    
>> What is the default base address ? The datasheet
>> doesn't give default value. But the base address seems to
>> always be 0xb000. Is there any drawbacks to used it ?
> 
> According to the spec
> (http://www.intel.com/assets/pdf/datasheet/290562.pdf), the reset value
> must be 0.

More precisely, the base is 0, the corresponding register is 1 after
reset (page 120, Power Management Base Address)).

Jan

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-09-04 11:41 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-03 10:03 [Qemu-devel] [PATCH V7 0/8] memory: unify ioport registration Julien Grall
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 1/8] isa: add isa_address_space_io Julien Grall
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 2/8] hw/apm.c: replace register_ioport* Julien Grall
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 3/8] smb: replace_register_ioport* Julien Grall
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 4/8] hw/acpi_piix4.c: replace register_ioport* Julien Grall
2012-09-04  7:24   ` Jan Kiszka
2012-09-04 11:27     ` Julien Grall
2012-09-04 11:37       ` Jan Kiszka
2012-09-04 11:41         ` Jan Kiszka
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 5/8] hw/cirrus_vga.c: " Julien Grall
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 6/8] hw/serial.c: " Julien Grall
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 7/8] hw/pc.c: " Julien Grall
2012-09-03 10:03 ` [Qemu-devel] [PATCH V7 8/8] hw/dma.c: " Julien Grall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).