Re: [PATCH 3/7] hw/acpi/{ich9,piix4}: Resolve redundant io_base address attributes

[Philippe Mathieu-Daudé]

Hi Bernhard,

On 22/1/23 18:07, Bernhard Beschow wrote:
> A MemoryRegion has an addr attribute which gets set to the same values
> as the redundant io_addr attributes.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/acpi/ich9.h  |  1 -
>   include/hw/acpi/piix4.h |  2 --
>   hw/acpi/ich9.c          | 17 ++++++++---------
>   hw/acpi/piix4.c         | 11 ++++++-----
>   4 files changed, 14 insertions(+), 17 deletions(-)

> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 370b34eacf..2e9bc63fca 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -91,13 +91,14 @@ static void apm_ctrl_changed(uint32_t val, void *arg)
>   static void pm_io_space_update(PIIX4PMState *s)
>   {
>       PCIDevice *d = PCI_DEVICE(s);
> +    uint32_t io_base;
>   
> -    s->io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> -    s->io_base &= 0xffc0;
> +    io_base = le32_to_cpu(*(uint32_t *)(d->config + 0x40));
> +    io_base &= 0xffc0;
>   
>       memory_region_transaction_begin();
>       memory_region_set_enabled(&s->io, d->config[0x80] & 1);
> -    memory_region_set_address(&s->io, s->io_base);
> +    memory_region_set_address(&s->io, io_base);

OK for this part.

>       memory_region_transaction_commit();
>   }
>   
> @@ -433,8 +434,8 @@ static void piix4_pm_add_properties(PIIX4PMState *s)
>                                     &s->ar.gpe.len, OBJ_PROP_FLAG_READ);
>       object_property_add_uint16_ptr(OBJECT(s), ACPI_PM_PROP_SCI_INT,
>                                     &sci_int, OBJ_PROP_FLAG_READ);
> -    object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
> -                                  &s->io_base, OBJ_PROP_FLAG_READ);
> +    object_property_add_uint64_ptr(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
> +                                   &s->io.addr, OBJ_PROP_FLAG_READ);

+Eduardo/Mark

We shouldn't do that IMO, because we access an internal field from
another QOM object.

We can however alias the MR property:

  object_property_add_alias(OBJECT(s), ACPI_PM_PROP_PM_IO_BASE,
                            OBJECT(&s->io), "addr");

>   }