Re: [PATCH v3 6/7] acpi/ghes: update comments to point to newer ACPI specs

From: Igor Mammedov
Date: Tue Jul 30 2024 - 07:24:43 EST

On Mon, 22 Jul 2024 08:45:58 +0200
Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:

> There is one reference to ACPI 4.0 and several references
> to ACPI 6.x versions.
>
> Update them to point to ACPI 6.5 whenever possible.

when it comes to APCI doc comments, they should point to
the 1st (earliest) revision that provides given feature/value/field/table.


> There's one reference that was kept pointing to ACPI 6.4,
> though, with HEST revision 1.
>
> ACPI 6.5 now defines HEST revision 2, and defined a new
> way to handle source types starting from 12. According
> with ACPI 6.5 revision history:
>
> 2312 Update to the HEST table and adding new error
> source descriptor - Table 18.2.
>
> Yet, the spec doesn't define yet any new source
> descriptors. It just defines a different behavior when
> source type is above 11.
>
> I also double-checked GHES implementation on an open
> source project (Linux Kernel). Currently upstream
> doesn't currently handle HEST revision, ignoring such
> field.
>
> In any case, revision 2 seems to be backward-compatible
> with revison 1 when type <= 11 and just one error is
> contained on a HEST record.
>
> So, while it is probably safe to update it, there's no
> real need. So, let's keep the implementation using
> an ACPI 6.4 compatible table, e. g. HEST revision 1.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
> ---
> hw/acpi/ghes.c | 48 ++++++++++++++++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/hw/acpi/ghes.c b/hw/acpi/ghes.c
> index 6075ef5893ce..ebf1b812aaaa 100644
> --- a/hw/acpi/ghes.c
> +++ b/hw/acpi/ghes.c
> @@ -45,9 +45,9 @@
> #define GAS_ADDR_OFFSET 4
>
> /*
> - * The total size of Generic Error Data Entry
> - * ACPI 6.1/6.2: 18.3.2.7.1 Generic Error Data,
> - * Table 18-343 Generic Error Data Entry
> + * The total size of Generic Error Data Entry before data field
> + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> + * Table 18.12 Generic Error Data Entry
> */
> #define ACPI_GHES_DATA_LENGTH 72
>
> @@ -65,8 +65,8 @@
>
> /*
> * Total size for Generic Error Status Block except Generic Error Data Entries
> - * ACPI 6.2: 18.3.2.7.1 Generic Error Data,
> - * Table 18-380 Generic Error Status Block
> + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> + * Table 18.11 Generic Error Status Block
> */
> #define ACPI_GHES_GESB_SIZE 20
>
> @@ -82,7 +82,8 @@ enum AcpiGenericErrorSeverity {
>
> /*
> * Hardware Error Notification
> - * ACPI 4.0: 17.3.2.7 Hardware Error Notification
> + * ACPI 6.5: 18.3.2.9 Hardware Error Notification,
> + * Table 18.14 - Hardware Error Notification Structure
> * Composes dummy Hardware Error Notification descriptor of specified type
> */
> static void build_ghes_hw_error_notification(GArray *table, const uint8_t type)
> @@ -112,7 +113,8 @@ static void build_ghes_hw_error_notification(GArray *table, const uint8_t type)
>
> /*
> * Generic Error Data Entry
> - * ACPI 6.1: 18.3.2.7.1 Generic Error Data
> + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> + * Table 18.12 - Generic Error Data Entry
> */
> static void acpi_ghes_generic_error_data(GArray *table,
> const uint8_t *section_type, uint32_t error_severity,
> @@ -148,7 +150,8 @@ static void acpi_ghes_generic_error_data(GArray *table,
>
> /*
> * Generic Error Status Block
> - * ACPI 6.1: 18.3.2.7.1 Generic Error Data
> + * ACPI 6.5: 18.3.2.7.1 Generic Error Data,
> + * Table 18.11 - Generic Hardware Error Source Structure
> */
> static void acpi_ghes_generic_error_status(GArray *table, uint32_t block_status,
> uint32_t raw_data_offset, uint32_t raw_data_length,
> @@ -429,15 +432,18 @@ void build_ghes_error_table(GArray *hardware_errors, BIOSLinker *linker)
> 0, sizeof(uint64_t), ACPI_GHES_ERRORS_FW_CFG_FILE, 0);
> }
>
> -/* Build Generic Hardware Error Source version 2 (GHESv2) */
> +/*
> + * Build Generic Hardware Error Source version 2 (GHESv2)
> + * ACPI 6.5: 18.3.2.8 Generic Hardware Error Source version 2 (GHESv2 - Type 10),
> + * Table 18.13: Generic Hardware Error Source version 2 (GHESv2)
> + */
> static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker)
> {
> uint64_t address_offset;
> - /*
> - * Type:
> - * Generic Hardware Error Source version 2(GHESv2 - Type 10)
> - */
> + /* Type: (GHESv2 - Type 10) */
> build_append_int_noprefix(table_data, ACPI_GHES_SOURCE_GENERIC_ERROR_V2, 2);
> +
> + /* ACPI 6.5: Table 18.10 - Generic Hardware Error Source Structure */
> /* Source Id */
> build_append_int_noprefix(table_data, source_id, 2);
> /* Related Source Id */
> @@ -481,11 +487,8 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker)
> /* Error Status Block Length */
> build_append_int_noprefix(table_data, ACPI_GHES_MAX_RAW_DATA_LENGTH, 4);
>
> - /*
> - * Read Ack Register
> - * ACPI 6.1: 18.3.2.8 Generic Hardware Error Source
> - * version 2 (GHESv2 - Type 10)
> - */
> + /* ACPI 6.5: fields defined at GHESv2 table */
> + /* Read Ack Register */
> address_offset = table_data->len;
> build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0,
> 4 /* QWord access */, 0);
> @@ -504,11 +507,16 @@ static void build_ghes_v2(GArray *table_data, int source_id, BIOSLinker *linker)
> build_append_int_noprefix(table_data, 0x1, 8);
> }
>
> -/* Build Hardware Error Source Table */
> +/*
> + * Build Hardware Error Source Table
> + * ACPI 6.4: 18.3.2 ACPI Error Source
> + * Table 18.2: Hardware Error Source Table (HEST)
> + */
> void acpi_build_hest(GArray *table_data, BIOSLinker *linker,
> const char *oem_id, const char *oem_table_id)
> {
> - AcpiTable table = { .sig = "HEST", .rev = 1,
> + AcpiTable table = { .sig = "HEST",
> + .rev = 1, /* ACPI 4.0 to 6.4 */
> .oem_id = oem_id, .oem_table_id = oem_table_id };
>
> acpi_table_begin(&table, table_data);