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

[Jan Kiszka <jan.kiszka@web.de>]

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