Re: [PATCH v17 04/10] acpi: add build_append_ghes_generic_data() helper for Generic Error Data Entry

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 14 May 2019 04:18:17 -0700
Dongjiu Geng <gengdongjiu@huawei.com> wrote:

> It will help to add Generic Error Data Entry to ACPI tables
> without using packed C structures and avoid endianness
> issues as API doesn't need explicit conversion.
> 
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
>  hw/acpi/aml-build.c         | 32 ++++++++++++++++++++++++++++++++
>  include/hw/acpi/aml-build.h |  6 ++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index fb53f21..102a288 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -296,6 +296,38 @@ void build_append_ghes_notify(GArray *table, const uint8_t type,
>          build_append_int_noprefix(table, error_threshold_window, 4);
>  }
>  
> +/* Generic Error Data Entry
> + * ACPI 4.0: 17.3.2.6.1 Generic Error Data
> + */
> +void build_append_ghes_generic_data(GArray *table, const char *section_type,
s/build_append_ghes_generic_data/build_append_ghes_generic_error_data/

> +                                    uint32_t error_severity, uint16_t revision,
> +                                    uint8_t validation_bits, uint8_t flags,
> +                                    uint32_t error_data_length, uint8_t *fru_id,
> +                                    uint8_t *fru_text, uint64_t time_stamp)
checkpatch probably will complain due to too long lines
you can use:
void build_append_ghe...
         uint32_t error_severity, uint16_t revision,
         ...

> +{
> +    int i;
> +
> +    for (i = 0; i < 16; i++) {
> +        build_append_int_noprefix(table, section_type[i], 1);
                                            ^^^
use QemuUUID instead, see vmgenid_build_acpi

> +    }
> +
> +    build_append_int_noprefix(table, error_severity, 4);
> +    build_append_int_noprefix(table, revision, 2);
> +    build_append_int_noprefix(table, validation_bits, 1);
> +    build_append_int_noprefix(table, flags, 1);
> +    build_append_int_noprefix(table, error_data_length, 4);
> +
> +    for (i = 0; i < 16; i++) {
> +        build_append_int_noprefix(table, fru_id[i], 1);
same as section_type

> +    }
> +
> +    for (i = 0; i < 20; i++) {
> +        build_append_int_noprefix(table, fru_text[i], 1);
> +    }
instead of loop use g_array_insert_vals()

> +
> +    build_append_int_noprefix(table, time_stamp, 8);
that's not part of 'Table 17-13'
where does it come from?

> +}
> +
>  /*
>   * 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 90c8ef8..a71db2f 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -419,6 +419,12 @@ void build_append_ghes_notify(GArray *table, const uint8_t type,
>                                uint32_t error_threshold_value,
>                                uint32_t error_threshold_window);
>  
> +void build_append_ghes_generic_data(GArray *table, const char *section_type,
> +                                    uint32_t error_severity, uint16_t revision,
> +                                    uint8_t validation_bits, uint8_t flags,
> +                                    uint32_t error_data_length, uint8_t *fru_id,
> +                                    uint8_t *fru_text, uint64_t time_stamp);
> +
>  void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>                         uint64_t len, int node, MemoryAffinityFlags flags);
>