Re: [Qemu-devel] [PATCH v5 5/7] acpi: extend aml_field() to support UpdateRule

[Igor Mammedov]

On Fri, 27 Mar 2015 17:20:38 +0800
Zhu Guihua <address@hidden> wrote:

> The flags field is declared with default update rule 'Preserve',
> this patch is to extend aml_field() to support UpdateRule so that
s/is to extend/extends/
> we can specify different value to UpdateRule for different field.
s/value to UpdateRule for different/values per/

Also you forgot to mention that you are fixing
"Memory device control fields" register with reason why.
I'd suggest to only extend aml_field() in this patch and
put fix in a separate patch.

> 
> Signed-off-by: Zhu Guihua <address@hidden>
> ---
>  hw/acpi/aml-build.c         |  4 +++-
>  hw/i386/acpi-build.c        | 13 ++++++++-----
>  include/hw/acpi/aml-build.h | 10 ++++++++--
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index d7945f6..f926c9a 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -635,9 +635,11 @@ Aml *aml_reserved_field(unsigned length)
>  }
>  
>  /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefField */
> -Aml *aml_field(const char *name, AmlFieldFlags flags)
> +Aml *aml_field(const char *name, AmlAccessType type, AmlUpdateRule rule)
>  {
>      Aml *var = aml_bundle(0x81 /* FieldOp */, AML_EXT_PACKAGE);
> +    uint8_t flags = rule << 5 | type;
> +
>      build_append_namestring(var->buf, "%s", name);
>      build_append_byte(var->buf, flags);
>      return var;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d0a5c85..b767cbe 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -798,7 +798,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>  
>          aml_append(dev, aml_operation_region("PEOR", aml_system_io,
>                                                misc->pvpanic_port, 1));
> -        field = aml_field("PEOR", aml_byte_acc);
> +        field = aml_field("PEOR", aml_byte_acc, aml_preserve);
>          aml_append(field, aml_named_field("PEPT", 8));
>          aml_append(dev, field);
>  
> @@ -835,7 +835,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>          /* declare CPU hotplug MMIO region and PRS field to access it */
>          aml_append(sb_scope, aml_operation_region(
>              "PRST", aml_system_io, pm->cpu_hp_io_base, pm->cpu_hp_io_len));
> -        field = aml_field("PRST", aml_byte_acc);
> +        field = aml_field("PRST", aml_byte_acc, aml_preserve);
>          aml_append(field, aml_named_field("PRS", 256));
>          aml_append(sb_scope, field);
>  
> @@ -909,7 +909,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>              pm->mem_hp_io_base, pm->mem_hp_io_len)
>          );
>  
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), 
> aml_dword_acc);
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
> +                                    aml_preserve);
>          aml_append(field, /* read only */
>              aml_named_field(stringify(MEMORY_SLOT_ADDR_LOW), 32));
>          aml_append(field, /* read only */
> @@ -922,7 +923,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>              aml_named_field(stringify(MEMORY_SLOT_PROXIMITY), 32));
>          aml_append(scope, field);
>  
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc);
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_byte_acc,
> +                          aml_write_as_zeros);
>          aml_append(field, aml_reserved_field(160 /* bits, Offset(20) */));
>          aml_append(field, /* 1 if enabled, read only */
>              aml_named_field(stringify(MEMORY_SLOT_ENABLED), 1));
> @@ -931,7 +933,8 @@ build_ssdt(GArray *table_data, GArray *linker,
>              aml_named_field(stringify(MEMORY_SLOT_INSERT_EVENT), 1));
>          aml_append(scope, field);
>  
> -        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), 
> aml_dword_acc);
> +        field = aml_field(stringify(MEMORY_HOTPLUG_IO_REGION), aml_dword_acc,
> +                          aml_preserve);
>          aml_append(field, /* DIMM selector, write only */
>              aml_named_field(stringify(MEMORY_SLOT_SLECTOR), 32));
>          aml_append(field, /* _OST event code, write only */
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 17d3beb..5aa5e7a 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -35,7 +35,13 @@ typedef enum {
>      aml_dword_acc = 3,
>      aml_qword_acc = 4,
>      aml_buffer_acc = 5,
> -} AmlFieldFlags;
> +} AmlAccessType;
> +
> +typedef enum {
> +    aml_preserve = 0,
> +    aml_write_as_ones = 1,
> +    aml_write_as_zeros = 2,
> +} AmlUpdateRule;
>  
>  typedef enum {
>      aml_system_memory = 0x00,
> @@ -185,7 +191,7 @@ Aml *aml_if(Aml *predicate);
>  Aml *aml_package(uint8_t num_elements);
>  Aml *aml_buffer(void);
>  Aml *aml_resource_template(void);
> -Aml *aml_field(const char *name, AmlFieldFlags flags);
> +Aml *aml_field(const char *name, AmlAccessType type, AmlUpdateRule rule);
>  Aml *aml_varpackage(uint32_t num_elements);
>  
>  #endif