[coreboot-gerrit] Change in coreboot[master]: soc/intel/tigerlake: Add CrashLog implementation for intel TGL

[Tim Wawrzynczak (Code Review)]

Attention is currently required from: Francois Toguo Fotso, Martin Roth, Paul Menzel, Duncan Laurie, Tim Wawrzynczak, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49943 )

Change subject: soc/intel/tigerlake: Add CrashLog implementation for intel TGL
......................................................................

Patch Set 10:

(4 comments)

Patchset:

PS7:
> Tim, […]

Gotcha, let's try and share as much as possible; if it makes sense to share most of it between TGL & ADL, then we can move it to src/soc/common/intel/block/crashlog for example

File src/soc/intel/tigerlake/Kconfig:

https://review.coreboot.org/c/coreboot/+/49943/comment/14ae18d7_7599d8f8 
PS6, Line 266: config ACPI_BERT
             : 	bool "Enables ACPI BERT"
             : 	default y
             : 	help
             : 	  Enables ACPI Boot Error Record Table generation.
             : 
             : config SOC_INTEL_CRASHLOG
             : 	def_bool n
             : 	help
             : 	  Enables CrashLog.
             :
> Your rational is correct, but for practical purpose I think it is better to keep them separate for t […]

You are correct; I have a suggestion then, how about moving the `acpi_soc_fill_bert` and `acpi_is_boot_error_src_present` function to a new file, e.g. soc/intel/tigerlake/acpi_bert.c, and then the Makefile can reflect this too:

```
+ramstage-$(CONFIG_SOC_INTEL_CRASHLOG) += acpi_bert.c

```

and then you can `select` ACPI_BERT in the Kconfig:
```
 config SOC_INTEL_CRASHLOG
        def_bool n
        select ACPI_BERT
        help
          Enables CrashLog.
```

and Volteer only needs to select SOC_INTEL_CRASHLOG, and then ACPI_BERT comes for free

File src/soc/intel/tigerlake/Makefile.inc:

https://review.coreboot.org/c/coreboot/+/49943/comment/143e59ac_51a355f0 
PS10, Line 49: y
$(CONFIG_SOC_INTEL_CRASHLOG)

File src/soc/intel/tigerlake/acpi.c:

https://review.coreboot.org/c/coreboot/+/49943/comment/9510dac2_69dcdcd9 
PS6, Line 367: 	if (crashlog_get_total_data_size() > bert_storage_remaining()) {
             : 		printk(BIOS_ERR, "Error: Crashlog entry would exceed "
             : 				"available region\n");
             : 		return;
             : 	}
> It is not expected, but I think the check is warranted. […]

Understood.