Change in coreboot[master]: acpi: Add support for reporting CrashLog in BERT table

[Marshall Dawson (Code Review)]

Attention is currently required from: Francois Toguo Fotso.
Marshall Dawson 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:

(5 comments)

Patchset:

PS10: 
As I was reading through this, one thing I began wondering is whether you interpreted raw_data_length as being an overall length.  I don't think this would be correct.  Raw data (and its length) should be in addition to generic records.

Also since bert_append_fw_err() returns a acpi_hest_generic_data_v300_t *, i.e. _generic_ it doesn't seem it should be touching raw_data_length at all.

File src/arch/x86/acpi_bert_storage.c:

PS10:
> The raw_data_length cannot simply be removed or ignored. It is part of the ACPI 6.4 specs: […]

It was also part of the 6.2 spec that was active when the code was written.  Sorry, I'm not seeing a requirement that raw data be reported.  Our implementations were simply reporting using generic entries.

https://review.coreboot.org/c/coreboot/+/49799/comment/420ca5d7_52ca5382 
PS10, Line 109: status->raw_data_length += size;
This doesn't seem right.  It wouldn't make sense to add to both the generic length and the raw data length.  They're different regions so I don't see how one size applies to both.

Since any raw data is more platform-specific, it seems it would be better to adjust that size at the soc or whoever calls bert_new_event().  Alternatively add another argument to these two functions for raw_size.

https://review.coreboot.org/c/coreboot/+/49799/comment/5c874236_a8f2eed1 
PS10, Line 178: status->raw_data_length += sizeof(*entry);
This is a generic error entry.  Raw data length doesn't come into play here.

https://review.coreboot.org/c/coreboot/+/49799/comment/66428a01_111fabd6 
PS10, Line 531: status->raw_data_length = sizeof(*status);
I don't understand this.  sizeof(*status) is simply the size of the Generic Error Status Block sans the Generic Error Data Entries.  Since there's no raw data yet, why arbitrarily start off the raw_data_length with 20 bytes?