Patchwork [V9,4/8] hw/acpi_piix4.c: replace register_ioport*

login
register
mail settings
Submitter Julien Grall
Date Sept. 4, 2012, 3:13 p.m.
Message ID <767c2d0e1f86cd99872eb51f9545eebc7f27411e.1346770982.git.julien.grall@citrix.com>
Download mbox | patch
Permalink /patch/181696/
State New
Headers show

Comments

Julien Grall - Sept. 4, 2012, 3:13 p.m.
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 |  145 +++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 113 insertions(+), 32 deletions(-)
Avi Kivity - Sept. 9, 2012, 2:22 p.m.
On 09/04/2012 06:13 PM, Julien Grall wrote:
> This patch replaces all register_ioport* with the new memory API. It permits
> to use the new Memory stuff like listener.
> 

> @@ -200,8 +212,11 @@ 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_set_address(&s->pm_io, pm_io_base);
> +        memory_region_set_enabled(&s->pm_io, true);
> +    } else {
> +        memory_region_set_enabled(&s->pm_io, false);
>      }
>  }
>  

The entire if () can be simplified to

       pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
       pm_io_base &= 0xffc0;
       memory_region_transaction_begin()
       memory_region_set_enabled(&s->pm_io, s->dev.config[0x80] & 1);
       memory_region_set_address(&s->pm_io, pm_io_base);
       memory_region_transaction_commit();
Julien Grall - Sept. 10, 2012, 10:37 a.m.
On 09/09/2012 03:22 PM, Avi Kivity wrote:
> On 09/04/2012 06:13 PM, Julien Grall wrote:
>    
>> This patch replaces all register_ioport* with the new memory API. It permits
>> to use the new Memory stuff like listener.
>>
>>      
>    
>> @@ -200,8 +212,11 @@ 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_set_address(&s->pm_io, pm_io_base);
>> +        memory_region_set_enabled(&s->pm_io, true);
>> +    } else {
>> +        memory_region_set_enabled(&s->pm_io, false);
>>       }
>>   }
>>
>>      
> The entire if () can be simplified to
>
>         pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
>         pm_io_base&= 0xffc0;
>         memory_region_transaction_begin()
>         memory_region_set_enabled(&s->pm_io, s->dev.config[0x80]&  1);
>         memory_region_set_address(&s->pm_io, pm_io_base);
>         memory_region_transaction_commit();
>    
I will modify, do I need to resend all the patch series or just this patch ?
Avi Kivity - Sept. 10, 2012, 10:44 a.m.
On 09/10/2012 01:37 PM, Julien Grall wrote:
> On 09/09/2012 03:22 PM, Avi Kivity wrote:
>> On 09/04/2012 06:13 PM, Julien Grall wrote:
>>   
>>> This patch replaces all register_ioport* with the new memory API. It
>>> permits
>>> to use the new Memory stuff like listener.
>>>
>>>      
>>   
>>> @@ -200,8 +212,11 @@ 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_set_address(&s->pm_io, pm_io_base);
>>> +        memory_region_set_enabled(&s->pm_io, true);
>>> +    } else {
>>> +        memory_region_set_enabled(&s->pm_io, false);
>>>       }
>>>   }
>>>
>>>      
>> The entire if () can be simplified to
>>
>>         pm_io_base = le32_to_cpu(*(uint32_t *)(s->dev.config + 0x40));
>>         pm_io_base&= 0xffc0;
>>         memory_region_transaction_begin()
>>         memory_region_set_enabled(&s->pm_io, s->dev.config[0x80]&  1);
>>         memory_region_set_address(&s->pm_io, pm_io_base);
>>         memory_region_transaction_commit();
>>    
> I will modify, do I need to resend all the patch series or just this
> patch ?
> 

Just this patch would be fine.

Patch

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0b4ad24..2400a35 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -41,10 +41,10 @@ 
 
 #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
+#define PM_BASE 0x00
 
 #define PIIX4_PCI_HOTPLUG_STATUS 2
 
@@ -55,7 +55,7 @@  struct pci_status {
 
 typedef struct PIIX4PMState {
     PCIDevice dev;
-    IORange ioport;
+    MemoryRegion pm_io;
     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 = 1,
+        .max_access_size = 4,
+    },
 };
 
 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,11 @@  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_set_address(&s->pm_io, pm_io_base);
+        memory_region_set_enabled(&s->pm_io, true);
+    } else {
+        memory_region_set_enabled(&s->pm_io, false);
     }
 }
 
@@ -395,6 +410,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 +433,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 +453,12 @@  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);
+    memory_region_add_subregion(pci_address_space_io(&s->dev),
+                                PM_BASE, &s->pm_io);
+
     acpi_pm_tmr_init(&s->ar, pm_tmr_timer);
     acpi_gpe_init(&s->ar, GPE_LEN);
 
@@ -510,16 +542,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 +584,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 +611,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,
+    },
+};
+
+/* 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_LITTLE_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);
 }