[coreboot-gerrit] Change in coreboot[master]: acpi: rework BERT SSDT generation logic

[Felix Held (Code Review)]

Attention is currently required from: Lance Zhao, Tim Wawrzynczak, Patrick Rudolph.
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55054 )

Change subject: acpi: rework BERT SSDT generation logic
......................................................................

acpi: rework BERT SSDT generation logic

Check if the ACPI_BERT Kconfig option is selected and only then try to
generate the SSDT. Also remove the acpi_is_boot_error_src_present weak
function from the ACPI global function and use the return value of
acpi_soc_get_bert_region to determine if there is a valid BERT region
with logged errors.

Signed-off-by: Felix Held 
Change-Id: I2a281f5f636010ba3b2e7e097e9cf53683022aea
---
M src/acpi/acpi.c
M src/include/acpi/acpi.h
M src/soc/intel/common/block/acpi/acpi_bert.c
3 files changed, 33 insertions(+), 35 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/55054/1

diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c
index 2326169..d7d7d3d 100644
--- a/src/acpi/acpi.c
+++ b/src/acpi/acpi.c
@@ -1560,11 +1560,6 @@
 }

/* BERT helpers */
-bool __weak acpi_is_boot_error_src_present(void)
-{
-	return false;
-}
-
 __weak enum cb_err acpi_soc_get_bert_region(void **region, size_t *length)
 {
    return CB_ERR;
@@ -1813,18 +1808,19 @@

current = acpi_align_current(current);

-	if (acpi_is_boot_error_src_present()) {
+	if (CONFIG(ACPI_BERT)) {
    	void *region;
    	size_t size;
-		printk(BIOS_DEBUG, "ACPI:    * BERT\n");
    	bert = (acpi_bert_t *) current;
-		acpi_soc_get_bert_region(&region, &size);
-		acpi_write_bert(bert, (uintptr_t)region, size);
-		if (bert->header.length >= sizeof(acpi_bert_t)) {
-			current += bert->header.length;
-			acpi_add_table(rsdp, bert);
+		if (acpi_soc_get_bert_region(&region, &size) == CB_SUCCESS) {
+			printk(BIOS_DEBUG, "ACPI:    * BERT\n");
+			acpi_write_bert(bert, (uintptr_t)region, size);
+			if (bert->header.length >= sizeof(acpi_bert_t)) {
+				current += bert->header.length;
+				acpi_add_table(rsdp, bert);
+			}
+			current = acpi_align_current(current);
    	}
-		current = acpi_align_current(current);
    }

printk(BIOS_DEBUG, "current = %lx\n", current);
diff --git a/src/include/acpi/acpi.h b/src/include/acpi/acpi.h
index 7b0a775..b1eb664 100644
--- a/src/include/acpi/acpi.h
+++ b/src/include/acpi/acpi.h
@@ -1341,7 +1341,6 @@
 unsigned long acpi_create_lpi_desc_ncst(acpi_lpi_desc_ncst_t *lpi_desc, uint16_t uid);

/* For crashlog. */
-bool acpi_is_boot_error_src_present(void);
 enum cb_err acpi_soc_get_bert_region(void **region, size_t *length);

/* For ACPI S3 support. */
diff --git a/src/soc/intel/common/block/acpi/acpi_bert.c b/src/soc/intel/common/block/acpi/acpi_bert.c
index 168f329..f411bb2 100644
--- a/src/soc/intel/common/block/acpi/acpi_bert.c
+++ b/src/soc/intel/common/block/acpi/acpi_bert.c
@@ -7,6 +7,26 @@
 #include <intelblocks/acpi.h>
 #include <intelblocks/crashlog.h>

+static bool boot_error_src_present(void)
+{
+
+	if (!CONFIG(SOC_INTEL_CRASHLOG)) {
+		printk(BIOS_DEBUG, "Crashlog disabled.\n");
+		return false;
+	}
+
+	if (!discover_crashlog()) {
+		printk(BIOS_SPEW, "Crashlog discovery result: crashlog not found\n");
+		return false;
+	}
+
+	collect_pmc_and_cpu_crashlog_from_srams();
+
+	/* Discovery tables sizes can be larger than the actual valid collected data */
+	u32 crashlog_size = cl_get_total_data_size();
+
+	return (crashlog_size > 0);
+}

enum cb_err acpi_soc_get_bert_region(void **region, size_t *length)
 {
@@ -14,6 +34,10 @@
    size_t cpu_record_size, pmc_record_size;
    void *cl_data = NULL;

+	if (!boot_error_src_present()) {
+		return CB_ERR;
+	}
+
    if (!cl_get_total_data_size()) {
    	printk(BIOS_ERR, "Error: No crashlog record present\n");
    	return CB_ERR;
@@ -71,24 +95,3 @@

return CB_SUCCESS;
 }
-
-bool acpi_is_boot_error_src_present(void)
-{
-
-	if (!CONFIG(SOC_INTEL_CRASHLOG)) {
-		printk(BIOS_DEBUG, "Crashlog disabled.\n");
-		return false;
-	}
-
-	if (!discover_crashlog()) {
-		printk(BIOS_SPEW, "Crashlog discovery result: crashlog not found\n");
-		return false;
-	}
-
-	collect_pmc_and_cpu_crashlog_from_srams();
-
-	/* Discovery tables sizes can be larger than the actual valid collected data */
-	u32 crashlog_size = cl_get_total_data_size();
-
-	return (crashlog_size > 0);
-}