[M] Change in coreboot[master]: arch/x86/tables.c: Drop placing ACPI and SMBIOS in lower memory

[Arthur Heymans (Code Review)]

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

Change subject: arch/x86/tables.c: Drop placing ACPI and SMBIOS in lower memory
......................................................................

arch/x86/tables.c: Drop placing ACPI and SMBIOS in lower memory

The codepaths to place ACPI and SMBIOS fully in lower memory are pretty
much never reached unless CBMEM overflows, which is very uncommon and
likely to have other problems.

The unused codepath also lacks any overflow test to check if it remains
below 1M. The EBDA setup also writes to 0xf6000 without any check if
that overlaps with any of those tables.

So it's more sane to never attempt placing the full tables in lower
memory.

Signed-off-by: Arthur Heymans 
Change-Id: I216e7c713d112ad1012b0beaf26196967988b951
---
M src/arch/x86/tables.c
1 file changed, 64 insertions(+), 67 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/75712/1

diff --git a/src/arch/x86/tables.c b/src/arch/x86/tables.c
index b4b97b4..9a9b2a6 100644
--- a/src/arch/x86/tables.c
+++ b/src/arch/x86/tables.c
@@ -94,47 +94,48 @@
     */
    high_table_pointer = (unsigned long)cbmem_add(CBMEM_ID_ACPI,
    	max_acpi_size);
-	if (high_table_pointer) {
-		unsigned long acpi_start = high_table_pointer;
-		unsigned long new_high_table_pointer;

-		rom_table_end = ALIGN_UP(rom_table_end, 16);
-		new_high_table_pointer = write_acpi_tables(high_table_pointer);
-		if (new_high_table_pointer > (high_table_pointer
-			+ max_acpi_size))
-			printk(BIOS_ERR, "Increase ACPI size\n");
-		printk(BIOS_DEBUG, "ACPI tables: %ld bytes.\n",
-				new_high_table_pointer - high_table_pointer);
-
-		/* Now we need to create a low table copy of the RSDP. */
-
-		/* First we look for the high table RSDP */
-		while (acpi_start < new_high_table_pointer) {
-			if (memcmp(((acpi_rsdp_t *)acpi_start)->signature,
-				RSDP_SIG, 8) == 0)
-				break;
-			acpi_start++;
-		}
-
-		/* Now, if we found the RSDP, we take the RSDT and XSDT pointer
-		 * from it in order to write the low RSDP
-		 */
-		if (acpi_start < new_high_table_pointer) {
-			acpi_rsdp_t *low_rsdp = (acpi_rsdp_t *)rom_table_end,
-				    *high_rsdp = (acpi_rsdp_t *)acpi_start;
-
-			/* Technically rsdp length varies but coreboot always
-			   writes longest size available.  */
-			memcpy(low_rsdp, high_rsdp, sizeof(acpi_rsdp_t));
-		} else {
-			printk(BIOS_ERR, "Didn't find RSDP in high table.\n");
-		}
-		rom_table_end = ALIGN_UP(rom_table_end + sizeof(acpi_rsdp_t), 16);
-	} else {
-		rom_table_end = write_acpi_tables(rom_table_end);
-		rom_table_end = ALIGN_UP(rom_table_end, 1024);
+	if (high_table_pointer == 0ul) {
+		printk(BIOS_ERR, "Failing to reserve CBMEM space for ACPI\n");
+		return rom_table_end;
    }

+	unsigned long acpi_start = high_table_pointer;
+	unsigned long new_high_table_pointer;
+
+	rom_table_end = ALIGN_UP(rom_table_end, 16);
+	new_high_table_pointer = write_acpi_tables(high_table_pointer);
+	if (new_high_table_pointer > (high_table_pointer
+				      + max_acpi_size))
+		printk(BIOS_ERR, "Increase ACPI size\n");
+	printk(BIOS_DEBUG, "ACPI tables: %ld bytes.\n",
+	       new_high_table_pointer - high_table_pointer);
+
+	/* Now we need to create a low table copy of the RSDP. */
+
+	/* First we look for the high table RSDP */
+	while (acpi_start < new_high_table_pointer) {
+		if (memcmp(((acpi_rsdp_t *)acpi_start)->signature,
+			   RSDP_SIG, 8) == 0)
+			break;
+		acpi_start++;
+	}
+
+	/* Now, if we found the RSDP, we take the RSDT and XSDT pointer
+	 * from it in order to write the low RSDP
+	 */
+	if (acpi_start < new_high_table_pointer) {
+		acpi_rsdp_t *low_rsdp = (acpi_rsdp_t *)rom_table_end,
+			*high_rsdp = (acpi_rsdp_t *)acpi_start;
+
+		/* Technically rsdp length varies but coreboot always
+		   writes longest size available.  */
+		memcpy(low_rsdp, high_rsdp, sizeof(acpi_rsdp_t));
+	} else {
+		printk(BIOS_ERR, "Didn't find RSDP in high table.\n");
+	}
+	rom_table_end = ALIGN_UP(rom_table_end + sizeof(acpi_rsdp_t), 16);
+
    return rom_table_end;
 }

@@ -146,37 +147,33 @@

high_table_pointer = (unsigned long)cbmem_add(CBMEM_ID_SMBIOS,
    	MAX_SMBIOS_SIZE);
-	if (high_table_pointer) {
-		unsigned long new_high_table_pointer;
-
-		/*
-		 * Clear the entire region to ensure the unused space doesn't
-		 * contain garbage from a previous boot, like stale table
-		 * signatures that could be found by the OS.
-		 */
-		memset((void *)high_table_pointer, 0, MAX_SMBIOS_SIZE);
-
-		new_high_table_pointer =
-			smbios_write_tables(high_table_pointer);
-		rom_table_end = ALIGN_UP(rom_table_end, 16);
-		memcpy((void *)rom_table_end, (void *)high_table_pointer,
-			sizeof(struct smbios_entry));
-		rom_table_end += sizeof(struct smbios_entry);
-
-		if (new_high_table_pointer > (high_table_pointer
-			+ MAX_SMBIOS_SIZE))
-			printk(BIOS_ERR, "Increase SMBIOS size\n");
-		printk(BIOS_DEBUG, "SMBIOS tables: %ld bytes.\n",
-				new_high_table_pointer - high_table_pointer);
-	} else {
-		unsigned long new_rom_table_end;
-
-		new_rom_table_end = smbios_write_tables(rom_table_end);
-		printk(BIOS_DEBUG, "SMBIOS size %ld bytes\n", new_rom_table_end
-			- rom_table_end);
-		rom_table_end = ALIGN_UP(new_rom_table_end, 16);
+	if (high_table_pointer == 0ul) {
+		printk(BIOS_ERR, "Failing to reserve CBMEM space for SMBIOS\n");
+		return rom_table_end;
    }

+	unsigned long new_high_table_pointer;
+
+	/*
+	 * Clear the entire region to ensure the unused space doesn't
+	 * contain garbage from a previous boot, like stale table
+	 * signatures that could be found by the OS.
+	 */
+	memset((void *)high_table_pointer, 0, MAX_SMBIOS_SIZE);
+
+	new_high_table_pointer =
+		smbios_write_tables(high_table_pointer);
+	rom_table_end = ALIGN_UP(rom_table_end, 16);
+	memcpy((void *)rom_table_end, (void *)high_table_pointer,
+	       sizeof(struct smbios_entry));
+	rom_table_end += sizeof(struct smbios_entry);
+
+	if (new_high_table_pointer > (high_table_pointer
+				      + MAX_SMBIOS_SIZE))
+		printk(BIOS_ERR, "Increase SMBIOS size\n");
+	printk(BIOS_DEBUG, "SMBIOS tables: %ld bytes.\n",
+	       new_high_table_pointer - high_table_pointer);
+
    return rom_table_end;
 }