Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49799 )
Change subject: acpi: Add support for reporting CrashLog in BERT table ......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
The Intel SOC flow to gather and check the data is quite different from the AMD's one it seems. […]
yes, the flow is a bit different right now. the flow used on amd and xeon sp (iirc that patch isn't merged and currently not publicly visible) populate the bert region during ramstage before the acpi table generation and the acpi table generation just calls into the common bert code to get the bert memory region and the info if it's empty or not and doesn't need any weak functions. the two new functions introduced in the code duplicate basically what the arch/x86/acpi/bert code has already implemented. i'm not sure if that common bert code should be in arch/x86, since there's also bert support on arm, but i'm not sure if i'd really like to open that can of worms before we need bert support on arm. what i also like about the approach to fill the bert region from the soc code and then have the acpi table generation code only get the location of the bert region is that entries for the bert table can be generated in the code for the components where the error gets detected, so for example when it comes from a pci(e) device, that could just be done from the pcie driver which i like from the design point
-
Felix Held (Code Review)