Martin Roth has posted comments on this change. ( https://review.coreboot.org/28470 )
Change subject: arch/x86: Add BERT region support functions ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/28470/2/src/arch/x86/acpi_bert_storage.c File src/arch/x86/acpi_bert_storage.c:
https://review.coreboot.org/#/c/28470/2/src/arch/x86/acpi_bert_storage.c@350 PS2, Line 350: 3 is this CPER_IA32X64_CTX_64BIT_EX?
https://review.coreboot.org/#/c/28470/2/src/arch/x86/acpi_bert_storage.c@399 PS2, Line 399: cpuid_eax use "struct cpuid_result" and cpuid() instead of calling cpuid 4 times?
https://review.coreboot.org/#/c/28470/2/src/arch/x86/acpi_bert_storage.c@519 PS2, Line 519: *start = NULL; Add a printf stating that the weak function was called and that a full implementation must be added?
printk(BIOS_ERR, "Error: %s not implemented. BERT region generation disabled", __func__);
https://review.coreboot.org/#/c/28470/2/src/arch/x86/include/arch/bert_stora... File src/arch/x86/include/arch/bert_storage.h:
https://review.coreboot.org/#/c/28470/2/src/arch/x86/include/arch/bert_stora... PS2, Line 88: acpi_hest_generic_data_nth Why make this inline?
-
Martin Roth (Code Review)