Re: [PATCH 1/7] hw/acpi/{ich9,piix4}: Reuse existing attributes for QOM properties

[Igor Mammedov <imammedo@redhat.com>]

On Sun, 22 Jan 2023 18:07:18 +0100
Bernhard Beschow <shentey@gmail.com> wrote:

> The QOM properties are accessed after the device models have been
> realized. This means that the constants are redundant. Remove them.
not sure it above means.

Perhaps:

subj: use existing fields type::foo instead of static memory.

all object_property_add_*_ptr() needs is a pointer to memory
storing so RO defaults where provided as pointers to static
constants. Instead of keeping static constants around, drop
them and initialize use existing type::foo field, which were
set set later on to this constant later at ...

(also see below: maybe squash ICH9_PMIO_GPE0_LEN cleanup here as well)

or something along this lines. 

> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  hw/acpi/ich9.c  |  5 ++---
>  hw/acpi/piix4.c | 10 ++++------
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index a93c470e9d..2050af67b9 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -433,7 +433,6 @@ static void ich9_pm_set_keep_pci_slot_hpc(Object *obj, bool value, Error **errp)
>  
>  void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
>  {
> -    static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;

you are loosing default value here, (true it's set somewhere later)
suggest:
      pm->acpi_regs.gpe.len = ICH9_PMIO_GPE0_LEN;

and in patch on top maybe cleanup other places that use
ICH9_PMIO_GPE0_LEN with pm->acpi_regs.gpe.len

>      pm->acpi_memory_hotplug.is_enabled = true;
>      pm->cpu_hotplug_legacy = true;
>      pm->disable_s3 = 0;
> @@ -448,8 +447,8 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
>      object_property_add(obj, ACPI_PM_PROP_GPE0_BLK, "uint32",
>                          ich9_pm_get_gpe0_blk,
>                          NULL, NULL, pm);
> -    object_property_add_uint32_ptr(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
> -                                   &gpe0_len, OBJ_PROP_FLAG_READ);
> +    object_property_add_uint8_ptr(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
> +                                  &pm->acpi_regs.gpe.len, OBJ_PROP_FLAG_READ);
>      object_property_add_bool(obj, "memory-hotplug-support",
>                               ich9_pm_get_memory_hotplug_support,
>                               ich9_pm_set_memory_hotplug_support);
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 0a81f1ad93..370b34eacf 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -421,18 +421,16 @@ static void piix4_pm_add_properties(PIIX4PMState *s)
>  {
>      static const uint8_t acpi_enable_cmd = ACPI_ENABLE;
>      static const uint8_t acpi_disable_cmd = ACPI_DISABLE;
> -    static const uint32_t gpe0_blk = GPE_BASE;
> -    static const uint32_t gpe0_blk_len = GPE_LEN;

ditto

also maybe split on 2 patches 1 for ich9 another for piix4

>      static const uint16_t sci_int = 9;
>  
>      object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_ENABLE_CMD,
>                                    &acpi_enable_cmd, OBJ_PROP_FLAG_READ);
>      object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_ACPI_DISABLE_CMD,
>                                    &acpi_disable_cmd, OBJ_PROP_FLAG_READ);
> -    object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK,
> -                                  &gpe0_blk, OBJ_PROP_FLAG_READ);
> -    object_property_add_uint32_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN,
> -                                  &gpe0_blk_len, OBJ_PROP_FLAG_READ);
> +    object_property_add_uint64_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK,
> +                                   &s->io_gpe.addr, OBJ_PROP_FLAG_READ);
> +    object_property_add_uint8_ptr(OBJECT(s), ACPI_PM_PROP_GPE0_BLK_LEN,
> +                                  &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,