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
