Re: [PATCH v7 05/10] acpi/ghes: rework the logic to handle HEST source ID

[Mauro Carvalho Chehab]

Em Wed, 14 Aug 2024 01:23:27 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:

This hunk is wrong:

> @@ -350,9 +380,10 @@ static void build_ghes_v2(GArray *table_data, int 
> source_id, BIOSLinker *linker)
>      build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0,
>                       4 /* QWord access */, 0);
>      bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> -        address_offset + GAS_ADDR_OFFSET,
> -        sizeof(uint64_t), ACPI_GHES_ERRORS_FW_CFG_FILE,
> -        (ACPI_GHES_ERROR_SOURCE_COUNT + source_id) * sizeof(uint64_t));
> +                                   address_offset + GAS_ADDR_OFFSET,
> +                                   sizeof(uint64_t),
> +                                   ACPI_BUILD_TABLE_FILE,
> +                                   address_offset + GAS_ADDR_OFFSET);
>  
>      /*
>       * Read Ack Preserve field

It should be, instead:

    /*
     * Read Ack Register
     * ACPI 6.1: 18.3.2.8 Generic Hardware Error Source
     * version 2 (GHESv2 - Type 10)
     */
    address_offset = table_data->len;
    build_append_gas(table_data, AML_AS_SYSTEM_MEMORY, 0x40, 0,
                     4 /* QWord access */, 0);
    bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
                                   address_offset + GAS_ADDR_OFFSET,
                                   sizeof(uint64_t),
                                   ACPI_HW_ERROR_FW_CFG_FILE,
                                   (ACPI_HEST_SRC_ID_COUNT + source_id) *
                                   sizeof(uint64_t));

Funny enough, even with this problem, error injection was working.

I'll be preparing a v8 with such fix applied.

I'll also add an optional patch at the end to double-check if the
links are properly generated, using an abort() in case something
ever goes wrong.

Regards,
Mauro