Re: [Qemu-devel] [PATCH v17 03/10] acpi: add build_append_ghes_notify() helper for Hardware Error Notification

[Igor Mammedov]

On Tue, 14 May 2019 04:18:16 -0700
Dongjiu Geng <address@hidden> wrote:

> It will help to add Hardware Error Notification to ACPI tables
> without using packed C structures and avoid endianness
> issues as API doesn't need explicit conversion.
> 
> Signed-off-by: Dongjiu Geng <address@hidden>
> ---
>  hw/acpi/aml-build.c         | 22 ++++++++++++++++++++++
>  include/hw/acpi/aml-build.h |  8 ++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 555c24f..fb53f21 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -274,6 +274,28 @@ void build_append_gas(GArray *table, AmlAddressSpace as,
>      build_append_int_noprefix(table, address, 8);
>  }
>  
> +/* Hardware Error Notification
> + * ACPI 4.0: 17.3.2.7 Hardware Error Notification
> + */
> +void build_append_ghes_notify(GArray *table, const uint8_t type,
> +                              uint8_t length, uint16_t config_write_enable,
> +                              uint32_t poll_interval, uint32_t vector,
> +                              uint32_t polling_threshold_value,
> +                              uint32_t polling_threshold_window,
> +                              uint32_t error_threshold_value,
> +                              uint32_t error_threshold_window)
> +{
> +        build_append_int_noprefix(table, type, 1); /* type */
comment should be verbatim copy from spec, in this case /* Type */
also do the same for other fields below

other than that patch looks good to me

> +        build_append_int_noprefix(table, length, 1);
> +        build_append_int_noprefix(table, config_write_enable, 2);
> +        build_append_int_noprefix(table, poll_interval, 4);
> +        build_append_int_noprefix(table, vector, 4);
> +        build_append_int_noprefix(table, polling_threshold_value, 4);
> +        build_append_int_noprefix(table, polling_threshold_window, 4);
> +        build_append_int_noprefix(table, error_threshold_value, 4);
> +        build_append_int_noprefix(table, error_threshold_window, 4);
> +}
> +
>  /*
>   * Build NAME(XXXX, 0x00000000) where 0x00000000 is encoded as a dword,
>   * and return the offset to 0x00000000 for runtime patching.
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 1a563ad..90c8ef8 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -411,6 +411,14 @@ build_append_gas_from_struct(GArray *table, const struct 
> AcpiGenericAddress *s)
>                       s->access_width, s->address);
>  }
>  
> +void build_append_ghes_notify(GArray *table, const uint8_t type,
> +                              uint8_t length, uint16_t config_write_enable,
> +                              uint32_t poll_interval, uint32_t vector,
> +                              uint32_t polling_threshold_value,
> +                              uint32_t polling_threshold_window,
> +                              uint32_t error_threshold_value,
> +                              uint32_t error_threshold_window);
> +
>  void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>                         uint64_t len, int node, MemoryAffinityFlags flags);
>