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

[Avi Kivity <avi@redhat.com>]

On 08/29/2012 06:01 AM, Julien Grall wrote:
> This patch replaces all register_ioport* with the new memory API. It permits
> to use the new Memory stuff like listener.
> 
>  
> @@ -55,7 +55,8 @@ struct pci_status {
>  
>  typedef struct PIIX4PMState {
>      PCIDevice dev;
> -    IORange ioport;
> +    MemoryRegion pm_io;
> +    bool pm_io_enabled;
>      ACPIREGS ar;
>  
>  
> -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)
>  {

Generally we don't change argument names needlessly.

> -    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);

So the printf above doesn't break.

>  }
>  
> -static const IORangeOps pm_iorange_ops = {
> +static const MemoryRegionOps pm_io_ops = {
>      .read = pm_ioport_read,
>      .write = pm_ioport_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,

DEVICE_LITTLE_ENDIAN

> +    .impl = {
> +        .min_access_size = 2,
> +        .max_access_size = 2,
> +    },
>  };
>  
> @@ -192,14 +207,22 @@ static void pm_io_space_update(PIIX4PMState *s)
>  {
>      uint32_t pm_io_base;
>  
> +    if (s->pm_io_enabled) {
> +        memory_region_del_subregion(pci_address_space_io(&s->dev), &s->pm_io);
> +        s->pm_io_enabled = false;
> +    }
> +

You can drop this code (and s->pm_io_enabled) and replace it with
memory_region_set_enabled().

>  
> @@ -380,6 +403,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,

DEVICE_LITTLE_ENDIAN

> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
> +static const MemoryRegionOps acpi_io_ops = {
> +    .write = acpi_dbg_writel,
> +    .endianness = DEVICE_NATIVE_ENDIAN,

DEVICE_LITTLE_ENDIAN

> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
> +

> @@ -560,20 +613,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,

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(),
> +};
> +
> +static const MemoryRegionOps pciej_hot_io_ops = {
> +    .read = pci_features_read,
> +    .write = pciej_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,

DEVICE_LITTLE_ENDIAN

> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +};
>  


-- 
error compiling committee.c: too many arguments to function