[L] Change in coreboot[master]: acpi/acpi.c: Reduce boilerplate

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/76127?usp=email )

Change subject: acpi/acpi.c: Reduce boilerplate
......................................................................

acpi/acpi.c: Reduce boilerplate

Adding tables to R/XSDT, aligning current pointer, computing checksum is
a lot of boilerplate that needs to be done for each table.

TESTED on foxconn/g41.

Signed-off-by: Arthur Heymans 
Change-Id: If4915b8cdfcfdbb34284ea75fa8a0fd23554152d
---
M src/acpi/acpi.c
1 file changed, 126 insertions(+), 192 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/76127/1

diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c
index cfc388b..5a571d7 100644
--- a/src/acpi/acpi.c
+++ b/src/acpi/acpi.c
@@ -27,6 +27,7 @@
 #include <device/mmio.h>
 #include <device/pci.h>
 #include <pc80/mc146818rtc.h>
+#include <stdint.h>
 #include <string.h>
 #include <types.h>
 #include <version.h>
@@ -338,13 +339,11 @@
    return current;
 }

-static void acpi_create_madt(acpi_madt_t *madt)
+static void acpi_create_madt(acpi_header_t *header, void *unused)
 {
-	acpi_header_t *header = &(madt->header);
+	acpi_madt_t *madt = (acpi_madt_t *)header;
    unsigned long current = (unsigned long)madt + sizeof(acpi_madt_t);

-	memset((void *)madt, 0, sizeof(acpi_madt_t));
-
    if (!header)
    	return;

@@ -371,10 +370,8 @@
    if (CONFIG(ACPI_CUSTOM_MADT))
    	current = acpi_fill_madt(current);

-	/* (Re)calculate length and checksum. */
+	/* (Re)calculate length . */
    header->length = current - (unsigned long)madt;
-
-	header->checksum = acpi_checksum((void *)madt, header->length);
 }

static unsigned long acpi_fill_mcfg(unsigned long current)
@@ -386,9 +383,9 @@
 }

/* MCFG is defined in the PCI Firmware Specification 3.0. */
-static void acpi_create_mcfg(acpi_mcfg_t *mcfg)
+static void acpi_create_mcfg(acpi_header_t *header, void *unused)
 {
-	acpi_header_t *header = &(mcfg->header);
+	acpi_mcfg_t *mcfg = (acpi_mcfg_t *)header;
    unsigned long current = (unsigned long)mcfg + sizeof(acpi_mcfg_t);

memset((void *)mcfg, 0, sizeof(acpi_mcfg_t));
@@ -409,9 +406,8 @@
    if (CONFIG(ECAM_MMCONF_SUPPORT))
    	current = acpi_fill_mcfg(current);

-	/* (Re)calculate length and checksum. */
+	/* (Re)calculate length */
    header->length = current - (unsigned long)mcfg;
-	header->checksum = acpi_checksum((void *)mcfg, header->length);
 }

static void *get_tcpa_log(u32 *size)
@@ -439,14 +435,15 @@
    return lasa;
 }

-static void acpi_create_tcpa(acpi_tcpa_t *tcpa)
+static void acpi_create_tcpa(acpi_header_t *header, void *unused)
 {
-	acpi_header_t *header = &(tcpa->header);
+	if (!CONFIG(TPM1))
+		return;
+
+	acpi_tcpa_t *tcpa = (acpi_tcpa_t *)header;
    u32 tcpa_log_len;
    void *lasa;

-	memset((void *)tcpa, 0, sizeof(acpi_tcpa_t));
-
    lasa = get_tcpa_log(&tcpa_log_len);
    if (!lasa)
    	return;
@@ -467,9 +464,6 @@
    tcpa->platform_class = 0;
    tcpa->laml = tcpa_log_len;
    tcpa->lasa = (uintptr_t)lasa;
-
-	/* Calculate checksum. */
-	header->checksum = acpi_checksum((void *)tcpa, header->length);
 }

static void *get_tpm2_log(u32 *size)
@@ -497,14 +491,15 @@
    return lasa;
 }

-static void acpi_create_tpm2(acpi_tpm2_t *tpm2)
+static void acpi_create_tpm2(acpi_header_t *header, void *unused)
 {
-	acpi_header_t *header = &(tpm2->header);
+	if (!CONFIG(TPM2))
+		return;
+
+	acpi_tpm2_t *tpm2 = (acpi_tpm2_t *)header;
    u32 tpm2_log_len;
    void *lasa;

-	memset((void *)tpm2, 0, sizeof(acpi_tpm2_t));
-
    /*
     * Some payloads like SeaBIOS depend on log area to use TPM2.
     * Get the memory size and address of TPM2 log area or initialize it.
@@ -542,9 +537,6 @@
    /* Fill the log area size and start address fields. */
    tpm2->laml = tpm2_log_len;
    tpm2->lasa = (uintptr_t)lasa;
-
-	/* Calculate checksum. */
-	header->checksum = acpi_checksum((void *)tpm2, header->length);
 }

static void acpi_ssdt_write_cbtable(void)
@@ -570,7 +562,7 @@
    acpigen_pop_len();
 }

-static void acpi_create_ssdt_generator(acpi_header_t *ssdt, const char *oem_table_id)
+static void acpi_create_ssdt_generator(acpi_header_t *ssdt, void *unused)
 {
    unsigned long current = (unsigned long)ssdt + sizeof(acpi_header_t);

@@ -579,7 +571,7 @@
    memcpy(&ssdt->signature, "SSDT", 4);
    ssdt->revision = get_acpi_table_revision(SSDT);
    memcpy(&ssdt->oem_id, OEM_ID, 6);
-	memcpy(&ssdt->oem_table_id, oem_table_id, 8);
+	memcpy(&ssdt->oem_table_id, ACPI_TABLE_CREATOR, 8);
    ssdt->oem_revision = 42;
    memcpy(&ssdt->asl_compiler_id, ASLC, 4);
    ssdt->asl_compiler_revision = asl_revision;
@@ -1504,9 +1496,9 @@
    return current;
 }

-static void acpi_create_facs(acpi_facs_t *facs)
+static void acpi_create_facs(void *header)
 {
-	memset((void *)facs, 0, sizeof(acpi_facs_t));
+	acpi_facs_t *facs = header;

memcpy(facs->signature, "FACS", 4);
    facs->length = sizeof(acpi_facs_t);
@@ -1681,15 +1673,21 @@
 }

/* ACPI 3.0b */
-static void acpi_write_bert(acpi_bert_t *bert, uintptr_t region, size_t length)
+static void acpi_create_bert(acpi_header_t *header, void *unused)
 {
-	acpi_header_t *header = &(bert->header);
+	if (!CONFIG(ACPI_BERT))
+		return;

-	memset(bert, 0, sizeof(acpi_bert_t));
+	acpi_bert_t *bert = (acpi_bert_t *)header;

if (!header)
    	return;

+	void *region;
+	size_t size;
+	if (acpi_soc_get_bert_region(&region, &size) != CB_SUCCESS)
+		return;
+
    memcpy(header->signature, "BERT", 4);
    memcpy(header->oem_id, OEM_ID, 6);
    memcpy(header->oem_table_id, ACPI_TABLE_CREATOR, 8);
@@ -1698,22 +1696,19 @@
    header->length += sizeof(acpi_bert_t);
    header->revision = get_acpi_table_revision(BERT);

-	bert->error_region = region;
-	bert->region_length = length;
-
-	/* Calculate checksums. */
-	header->checksum = acpi_checksum((void *)bert, header->length);
+	bert->error_region = (uintptr_t)region;
+	bert->region_length = (size_t)size;
 }

__weak void arch_fill_fadt(acpi_fadt_t *fadt) { }
 __weak void soc_fill_fadt(acpi_fadt_t *fadt) { }
 __weak void mainboard_fill_fadt(acpi_fadt_t *fadt) { }

-static void acpi_create_fadt(acpi_fadt_t *fadt, acpi_facs_t *facs, void *dsdt)
+static acpi_header_t *dsdt;
+static void acpi_create_fadt(acpi_header_t *header, void *arg1)
 {
-	acpi_header_t *header = &(fadt->header);
-
-	memset((void *)fadt, 0, sizeof(acpi_fadt_t));
+	acpi_fadt_t *fadt = (acpi_fadt_t *)header;
+	acpi_facs_t *facs = (acpi_facs_t *)(*(acpi_facs_t **)arg1);

if (!header)
    	return;
@@ -1756,17 +1751,15 @@

soc_fill_fadt(fadt);
    mainboard_fill_fadt(fadt);
-
-	header->checksum =
-	    acpi_checksum((void *)fadt, header->length);
 }

-static void acpi_create_lpit(acpi_lpit_t *lpit)
+static void acpi_create_lpit(acpi_header_t *header, void *unused)
 {
-	acpi_header_t *header = &(lpit->header);
-	unsigned long current = (unsigned long)lpit + sizeof(acpi_lpit_t);
+	if (!CONFIG(ACPI_LPIT))
+		return;

-	memset((void *)lpit, 0, sizeof(acpi_lpit_t));
+	acpi_lpit_t *lpit = (acpi_lpit_t *)header;
+	unsigned long current = (unsigned long)lpit + sizeof(acpi_lpit_t);

if (!header)
    	return;
@@ -1784,9 +1777,8 @@

current = acpi_fill_lpit(current);

-	/* (Re)calculate length and checksum. */
+	/* (Re)calculate length. */
    header->length = current - (unsigned long)lpit;
-	header->checksum = acpi_checksum((void *)lpit, header->length);
 }

unsigned long acpi_create_lpi_desc_ncst(acpi_lpi_desc_ncst_t *lpi_desc, uint16_t uid)
@@ -1815,6 +1807,57 @@
    cbfs_preload(file);
 }

+static void acpi_create_dsdt(acpi_header_t *header, void *dsdt_file_arg)
+{
+	dsdt = header;
+	acpi_header_t *dsdt_file = (acpi_header_t *)(*(acpi_header_t **)(dsdt_file_arg));
+	unsigned long current = (unsigned long)header;
+
+	dsdt = (acpi_header_t *)current;
+	memcpy(dsdt, dsdt_file, sizeof(acpi_header_t));
+	if (dsdt->length >= sizeof(acpi_header_t)) {
+		current += sizeof(acpi_header_t);
+
+		acpigen_set_current((char *)current);
+
+		if (CONFIG(ACPI_SOC_NVS))
+			acpi_fill_gnvs();
+		if (CONFIG(CHROMEOS_NVS))
+			acpi_fill_cnvs();
+
+		for (const struct device *dev = all_devices; dev; dev = dev->next)
+			if (dev->ops && dev->ops->acpi_inject_dsdt)
+				dev->ops->acpi_inject_dsdt(dev);
+		current = (unsigned long)acpigen_get_current();
+		memcpy((char *)current,
+		       (char *)dsdt_file + sizeof(acpi_header_t),
+		       dsdt->length - sizeof(acpi_header_t));
+		current += dsdt->length - sizeof(acpi_header_t);
+
+		/* (Re)calculate length. */
+		dsdt->length = current - (unsigned long)dsdt;
+	}
+
+	/*
+	 * cbfs_unmap() uses mem_pool_free() which works correctly only
+	 * if freeing is done in reverse order than memory allocation.
+	 * This is why unmapping of dsdt_file must be done after
+	 * unmapping slic file.
+	 */
+	cbfs_unmap(dsdt_file);
+}
+
+static void acpi_create_slic(acpi_header_t *header, void *slic_file_arg)
+{
+	acpi_header_t *slic_file = (acpi_header_t *)(*(acpi_header_t **)slic_file_arg);
+	acpi_header_t *slic = header;
+	if (slic_file) {
+		memcpy(slic, slic_file, slic_file->length);
+		cbfs_unmap(slic_file);
+	}
+
+}
+
 static uintptr_t coreboot_rsdp;

uintptr_t get_coreboot_rsdp(void)
@@ -1860,22 +1903,32 @@
    acpi_rsdp_t *rsdp;
    acpi_rsdt_t *rsdt = NULL;
    acpi_xsdt_t *xsdt = NULL;
-	acpi_fadt_t *fadt = NULL;
    acpi_facs_t *facs = NULL;
-	acpi_header_t *slic_file, *slic = NULL;
+	acpi_header_t *slic_file;
    acpi_header_t *ssdt = NULL;
-	acpi_header_t *dsdt_file, *dsdt = NULL;
-	acpi_mcfg_t *mcfg = NULL;
-	acpi_tcpa_t *tcpa = NULL;
-	acpi_tpm2_t *tpm2 = NULL;
-	acpi_madt_t *madt = NULL;
-	acpi_lpit_t *lpit = NULL;
-	acpi_bert_t *bert = NULL;
+	acpi_header_t *dsdt_file;
    struct device *dev;
    unsigned long fw;
    size_t slic_size, dsdt_size;
    char oem_id[6], oem_table_id[8];

+	const struct acpi_table_generator {
+		void (*create_table)(acpi_header_t *table, void *arg);
+		void *args;
+		size_t min_size;
+	} tables[] = {
+		{ acpi_create_dsdt, &dsdt_file, sizeof(acpi_header_t) },
+		{ acpi_create_fadt, &facs, sizeof(acpi_fadt_t) },
+		{ acpi_create_slic, &slic_file, sizeof(acpi_header_t) },
+		{ acpi_create_ssdt_generator, NULL, sizeof(acpi_header_t) },
+		{ acpi_create_mcfg, NULL, sizeof(acpi_mcfg_t) },
+		{ acpi_create_tcpa, NULL, sizeof(acpi_tcpa_t) },
+		{ acpi_create_tpm2, NULL, sizeof(acpi_tpm2_t) },
+		{ acpi_create_lpit, NULL, sizeof(acpi_lpit_t) },
+		{ acpi_create_madt, NULL, sizeof(acpi_header_t) },
+		{ acpi_create_bert, NULL, sizeof(acpi_bert_t) },
+	};
+
    current = start;

/* Align ACPI tables to 16byte */
@@ -1921,7 +1974,7 @@
    	ssdt->length = current - (unsigned long)ssdt;
    	ssdt->checksum = acpi_checksum((void *)ssdt, ssdt->length);

-		acpi_create_ssdt_generator(ssdt, ACPI_TABLE_CREATOR);
+		acpi_create_ssdt_generator(ssdt, NULL);

acpi_add_table(rsdp, ssdt);

@@ -1981,145 +2034,26 @@
    acpi_write_rsdt(rsdt, oem_id, oem_table_id);
    acpi_write_xsdt(xsdt, oem_id, oem_table_id);

-	printk(BIOS_DEBUG, "ACPI:    * FACS\n");
    current = ALIGN_UP(current, 64);
+
+	printk(BIOS_DEBUG, "ACPI:    * FACS\n");
    facs = (acpi_facs_t *)current;
    current += sizeof(acpi_facs_t);
    current = acpi_align_current(current);
    acpi_create_facs(facs);

-	printk(BIOS_DEBUG, "ACPI:    * DSDT\n");
-	dsdt = (acpi_header_t *)current;
-	memcpy(dsdt, dsdt_file, sizeof(acpi_header_t));
-	if (dsdt->length >= sizeof(acpi_header_t)) {
-		current += sizeof(acpi_header_t);
-
-		acpigen_set_current((char *)current);
-
-		if (CONFIG(ACPI_SOC_NVS))
-			acpi_fill_gnvs();
-		if (CONFIG(CHROMEOS_NVS))
-			acpi_fill_cnvs();
-
-		for (dev = all_devices; dev; dev = dev->next)
-			if (dev->ops && dev->ops->acpi_inject_dsdt)
-				dev->ops->acpi_inject_dsdt(dev);
-		current = (unsigned long)acpigen_get_current();
-		memcpy((char *)current,
-		       (char *)dsdt_file + sizeof(acpi_header_t),
-		       dsdt->length - sizeof(acpi_header_t));
-		current += dsdt->length - sizeof(acpi_header_t);
-
-		/* (Re)calculate length and checksum. */
-		dsdt->length = current - (unsigned long)dsdt;
-		dsdt->checksum = 0;
-		dsdt->checksum = acpi_checksum((void *)dsdt, dsdt->length);
-	}
-
-	current = acpi_align_current(current);
-
-	printk(BIOS_DEBUG, "ACPI:    * FADT\n");
-	fadt = (acpi_fadt_t *)current;
-	current += sizeof(acpi_fadt_t);
-	current = acpi_align_current(current);
-
-	acpi_create_fadt(fadt, facs, dsdt);
-	acpi_add_table(rsdp, fadt);
-
-	if (slic_file) {
-		printk(BIOS_DEBUG, "ACPI:     * SLIC\n");
-		slic = (acpi_header_t *)current;
-		memcpy(slic, slic_file, slic_file->length);
-		current += slic_file->length;
+	for (size_t i = 0; i < ARRAY_SIZE(tables); i++) {
+		acpi_header_t *header = (acpi_header_t *)current;
+		memset(header, 0, tables[i].min_size);
+		tables[i].create_table(header, tables[i].args);
+		if (header->length < tables[i].min_size)
+			continue;
+		header->checksum = 0;
+		header->checksum = acpi_checksum((void *)header, header->length);
+		current += header->length;
    	current = acpi_align_current(current);
-		acpi_add_table(rsdp, slic);
-		cbfs_unmap(slic_file);
-	}
-
-	/*
-	 * cbfs_unmap() uses mem_pool_free() which works correctly only
-	 * if freeing is done in reverse order than memory allocation.
-	 * This is why unmapping of dsdt_file must be done after
-	 * unmapping slic file.
-	 */
-	cbfs_unmap(dsdt_file);
-
-	printk(BIOS_DEBUG, "ACPI:     * SSDT\n");
-	ssdt = (acpi_header_t *)current;
-	acpi_create_ssdt_generator(ssdt, ACPI_TABLE_CREATOR);
-	if (ssdt->length > sizeof(acpi_header_t)) {
-		current += ssdt->length;
-		acpi_add_table(rsdp, ssdt);
-		current = acpi_align_current(current);
-	}
-
-	printk(BIOS_DEBUG, "ACPI:    * MCFG\n");
-	mcfg = (acpi_mcfg_t *)current;
-	acpi_create_mcfg(mcfg);
-	if (mcfg->header.length > sizeof(acpi_mcfg_t)) {
-		current += mcfg->header.length;
-		current = acpi_align_current(current);
-		acpi_add_table(rsdp, mcfg);
-	}
-
-	if (CONFIG(TPM1)) {
-		printk(BIOS_DEBUG, "ACPI:    * TCPA\n");
-		tcpa = (acpi_tcpa_t *)current;
-		acpi_create_tcpa(tcpa);
-		if (tcpa->header.length >= sizeof(acpi_tcpa_t)) {
-			current += tcpa->header.length;
-			current = acpi_align_current(current);
-			acpi_add_table(rsdp, tcpa);
-		}
-	}
-
-	if (CONFIG(TPM2)) {
-		printk(BIOS_DEBUG, "ACPI:    * TPM2\n");
-		tpm2 = (acpi_tpm2_t *)current;
-		acpi_create_tpm2(tpm2);
-		if (tpm2->header.length >= sizeof(acpi_tpm2_t)) {
-			current += tpm2->header.length;
-			current = acpi_align_current(current);
-			acpi_add_table(rsdp, tpm2);
-		}
-	}
-
-	if (CONFIG(ACPI_LPIT)) {
-		printk(BIOS_DEBUG, "ACPI:     * LPIT\n");
-
-		lpit = (acpi_lpit_t *)current;
-		acpi_create_lpit(lpit);
-		if (lpit->header.length >= sizeof(acpi_lpit_t)) {
-			current += lpit->header.length;
-			current = acpi_align_current(current);
-			acpi_add_table(rsdp, lpit);
-		}
-	}
-
-	printk(BIOS_DEBUG, "ACPI:    * MADT\n");
-
-	madt = (acpi_madt_t *)current;
-	acpi_create_madt(madt);
-	if (madt->header.length > sizeof(acpi_madt_t)) {
-		current += madt->header.length;
-		acpi_add_table(rsdp, madt);
-	}
-
-	current = acpi_align_current(current);
-
-	if (CONFIG(ACPI_BERT)) {
-		void *region;
-		size_t size;
-		bert = (acpi_bert_t *)current;
-		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);
-		}
+		printk(BIOS_DEBUG, "ACPI:    * %.4s\n", header->signature);
+		acpi_add_table(rsdp, header);
    }

printk(BIOS_DEBUG, "current = %lx\n", current);