On Mon, Nov 18, 2019 at 09:18:01PM +0800, gengdongjiu wrote:
> On 2019/11/18 20:49, gengdongjiu wrote:
> >>> + */
> >>> + build_append_int_noprefix(table_data, source_id, 2);
> >>> + /* Related Source Id */
> >>> + build_append_int_noprefix(table_data, 0xffff, 2);
> >>> + /* Flags */
> >>> + build_append_int_noprefix(table_data, 0, 1);
> >>> + /* Enabled */
> >>> + build_append_int_noprefix(table_data, 1, 1);
> >>> +
> >>> + /* Number of Records To Pre-allocate */
> >>> + build_append_int_noprefix(table_data, 1, 4);
> >>> + /* Max Sections Per Record */
> >>> + build_append_int_noprefix(table_data, 1, 4);
> >>> + /* Max Raw Data Length */
> >>> + build_append_int_noprefix(table_data, ACPI_GHES_MAX_RAW_DATA_LENGTH,
> >>> 4);
> >>> +
> >>> + /* Error Status Address */
> >>> + 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,
> >>> + ACPI_GHES_ERROR_STATUS_ADDRESS_OFFSET(hest_start, source_id),
> >> it's fine only if GHESv2 is the only entries in HEST, but once
> >> other types are added this macro will silently fall apart and
> >> cause table corruption.
> why silently fall?
> I think the acpi_ghes.c only support GHESv2 type, not support other type.
>
> >>
> >> Instead of offset from hest_start, I suggest to use offset relative
> >> to GAS structure, here is an idea>>
> >> #define GAS_ADDR_OFFSET 4
> >>
> >> off = table->len
> >> build_append_gas()
> >> bios_linker_loader_add_pointer(...,
> >> off + GAS_ADDR_OFFSET, ...
>
> If use offset relative to GAS structure, the code does not easily extend to
> support more Generic Hardware Error Source.
> if use offset relative to hest_start, just use a loop, the code can support
> more error source, for example:
> for (source_id = 0; i<n; source_id++)
> {
> ......
> bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE,
> ACPI_GHES_ERROR_STATUS_ADDRESS_OFFSET(hest_start, source_id),
> sizeof(uint64_t), ACPI_GHES_ERRORS_FW_CFG_FILE,
> source_id * sizeof(uint64_t));
> .......
> }
>
> My previous series patch support 2 error sources, but now only enable 'SEA'
> type Error Source
I'd try to merge this, worry about extending things later.
This is at v21 and the simpler you can keep things,
the faster it'll go in.
