diff mbox

[V5,2/8] hw/acpi_piix4.c: replace register_ioport*

Message ID 1fa099f2f1c404e8b562a2ad32a39a784c68be1c.1345549695.git.julien.grall@citrix.com
State New
Headers show

Commit Message

Julien Grall Aug. 22, 2012, 12:27 p.m. UTC
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 |  160 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 124 insertions(+), 36 deletions(-)

Comments

Jan Kiszka Aug. 23, 2012, 6:01 p.m. UTC | #1
On 2012-08-22 14:27, 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 |  160 ++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 124 insertions(+), 36 deletions(-)
> 

...

> @@ -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
Jan Kiszka Aug. 26, 2012, 9:10 a.m. UTC | #2
On 2012-08-22 14:27, 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 |  160 ++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 124 insertions(+), 36 deletions(-)
> 
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 0aace60..26d5559 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -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);
>  }
> 

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

Jan
Jan Kiszka Aug. 26, 2012, 9:36 a.m. UTC | #3
On 2012-08-26 11:10, Jan Kiszka wrote:
> On 2012-08-22 14:27, 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 |  160 ++++++++++++++++++++++++++++++++++++++++++------------
>>  1 files changed, 124 insertions(+), 36 deletions(-)
>>
>> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
>> index 0aace60..26d5559 100644
>> --- a/hw/acpi_piix4.c
>> +++ b/hw/acpi_piix4.c
>> @@ -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);
>>  }
>>
> 
> 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
diff mbox

Patch

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0aace60..26d5559 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -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);
 }