[coreboot-gerrit] Patch set updated for coreboot: lib: add common write_tables() implementation

Aaron Durbin (adurbin@chromium.org) gerrit at coreboot.org
Thu Apr 21 20:48:47 CEST 2016


Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/14436

-gerrit

commit cb6d384ae6be510928c9869b1b34346477197f37
Author: Aaron Durbin <adurbin at chromium.org>
Date:   Tue Apr 19 21:38:18 2016 -0500

    lib: add common write_tables() implementation
    
    In order to de-duplicate common patterns implement one write_tables()
    function. The new write_tables() replaces all the architecture-specific
    ones that were largely copied. The callbacks are put in place to
    handle any per-architecture requirements.
    
    Change-Id: Id3d7abdce5b30f5557ccfe1dacff3c58c59f5e2b
    Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
 src/arch/arm/tables.c              | 34 --------------------
 src/arch/arm64/tables.c            | 34 --------------------
 src/arch/mips/tables.c             | 34 --------------------
 src/arch/power8/tables.c           | 35 --------------------
 src/arch/riscv/tables.c            | 34 --------------------
 src/arch/x86/tables.c              | 65 ++++++++++----------------------------
 src/include/boot/coreboot_tables.h |  4 ---
 src/lib/coreboot_table.c           | 63 +++++++++++++++++++-----------------
 8 files changed, 50 insertions(+), 253 deletions(-)

diff --git a/src/arch/arm/tables.c b/src/arch/arm/tables.c
index 92213c5..eef6bf2 100644
--- a/src/arch/arm/tables.c
+++ b/src/arch/arm/tables.c
@@ -15,14 +15,9 @@
  * GNU General Public License for more details.
  */
 
-#include <console/console.h>
-#include <cpu/cpu.h>
 #include <bootmem.h>
 #include <boot/tables.h>
 #include <boot/coreboot_tables.h>
-#include <string.h>
-#include <cbmem.h>
-#include <lib.h>
 
 void arch_write_tables(uintptr_t coreboot_table)
 {
@@ -32,35 +27,6 @@ void bootmem_arch_add_ranges(void)
 {
 }
 
-#define MAX_COREBOOT_TABLE_SIZE CONFIG_COREBOOT_TABLE_SIZE
-
-void write_tables(void)
-{
-	unsigned long table_pointer, new_table_pointer;
-
-	post_code(0x9d);
-
-	table_pointer = (unsigned long)cbmem_add(CBMEM_ID_CBTABLE,
-						MAX_COREBOOT_TABLE_SIZE);
-	if (!table_pointer) {
-		printk(BIOS_ERR, "Could not add CBMEM for coreboot table.\n");
-		return;
-	}
-
-	new_table_pointer = write_coreboot_table(0UL, 0UL,
-				table_pointer, table_pointer);
-
-	if (new_table_pointer > (table_pointer + MAX_COREBOOT_TABLE_SIZE)) {
-		printk(BIOS_ERR, "coreboot table didn't fit (%lx/%x bytes)\n",
-			   new_table_pointer - table_pointer, MAX_COREBOOT_TABLE_SIZE);
-	}
-
-	printk(BIOS_DEBUG, "coreboot table: %ld bytes.\n",
-			new_table_pointer - table_pointer);
-
-	post_code(0x9e);
-}
-
 void lb_arch_add_records(struct lb_header *header)
 {
 }
diff --git a/src/arch/arm64/tables.c b/src/arch/arm64/tables.c
index 92213c5..eef6bf2 100644
--- a/src/arch/arm64/tables.c
+++ b/src/arch/arm64/tables.c
@@ -15,14 +15,9 @@
  * GNU General Public License for more details.
  */
 
-#include <console/console.h>
-#include <cpu/cpu.h>
 #include <bootmem.h>
 #include <boot/tables.h>
 #include <boot/coreboot_tables.h>
-#include <string.h>
-#include <cbmem.h>
-#include <lib.h>
 
 void arch_write_tables(uintptr_t coreboot_table)
 {
@@ -32,35 +27,6 @@ void bootmem_arch_add_ranges(void)
 {
 }
 
-#define MAX_COREBOOT_TABLE_SIZE CONFIG_COREBOOT_TABLE_SIZE
-
-void write_tables(void)
-{
-	unsigned long table_pointer, new_table_pointer;
-
-	post_code(0x9d);
-
-	table_pointer = (unsigned long)cbmem_add(CBMEM_ID_CBTABLE,
-						MAX_COREBOOT_TABLE_SIZE);
-	if (!table_pointer) {
-		printk(BIOS_ERR, "Could not add CBMEM for coreboot table.\n");
-		return;
-	}
-
-	new_table_pointer = write_coreboot_table(0UL, 0UL,
-				table_pointer, table_pointer);
-
-	if (new_table_pointer > (table_pointer + MAX_COREBOOT_TABLE_SIZE)) {
-		printk(BIOS_ERR, "coreboot table didn't fit (%lx/%x bytes)\n",
-			   new_table_pointer - table_pointer, MAX_COREBOOT_TABLE_SIZE);
-	}
-
-	printk(BIOS_DEBUG, "coreboot table: %ld bytes.\n",
-			new_table_pointer - table_pointer);
-
-	post_code(0x9e);
-}
-
 void lb_arch_add_records(struct lb_header *header)
 {
 }
diff --git a/src/arch/mips/tables.c b/src/arch/mips/tables.c
index 32e73bc..50d2a55 100644
--- a/src/arch/mips/tables.c
+++ b/src/arch/mips/tables.c
@@ -16,14 +16,9 @@
  * GNU General Public License for more details.
  */
 
-#include <console/console.h>
-#include <cpu/cpu.h>
 #include <bootmem.h>
 #include <boot/tables.h>
 #include <boot/coreboot_tables.h>
-#include <string.h>
-#include <cbmem.h>
-#include <lib.h>
 
 void arch_write_tables(uintptr_t coreboot_table)
 {
@@ -33,35 +28,6 @@ void bootmem_arch_add_ranges(void)
 {
 }
 
-#define MAX_COREBOOT_TABLE_SIZE CONFIG_COREBOOT_TABLE_SIZE
-
-void write_tables(void)
-{
-	unsigned long table_pointer, new_table_pointer;
-
-	post_code(0x9d);
-
-	table_pointer = (unsigned long)cbmem_add(CBMEM_ID_CBTABLE,
-						 MAX_COREBOOT_TABLE_SIZE);
-	if (!table_pointer) {
-		printk(BIOS_ERR, "Could not add CBMEM for coreboot table.\n");
-		return;
-	}
-
-	new_table_pointer = write_coreboot_table(0UL, 0UL, table_pointer,
-						 table_pointer);
-
-	if (new_table_pointer > (table_pointer + MAX_COREBOOT_TABLE_SIZE))
-		printk(BIOS_ERR, "coreboot table didn't fit (%lx/%x bytes)\n",
-		       new_table_pointer - table_pointer,
-		       MAX_COREBOOT_TABLE_SIZE);
-
-	printk(BIOS_DEBUG, "coreboot table: %ld bytes.\n",
-	       new_table_pointer - table_pointer);
-
-	post_code(0x9e);
-}
-
 void lb_arch_add_records(struct lb_header *header)
 {
 }
diff --git a/src/arch/power8/tables.c b/src/arch/power8/tables.c
index ff07c31..eef6bf2 100644
--- a/src/arch/power8/tables.c
+++ b/src/arch/power8/tables.c
@@ -15,14 +15,9 @@
  * GNU General Public License for more details.
  */
 
-#include <console/console.h>
-#include <cpu/cpu.h>
 #include <bootmem.h>
 #include <boot/tables.h>
 #include <boot/coreboot_tables.h>
-#include <string.h>
-#include <cbmem.h>
-#include <lib.h>
 
 void arch_write_tables(uintptr_t coreboot_table)
 {
@@ -32,36 +27,6 @@ void bootmem_arch_add_ranges(void)
 {
 }
 
-#define MAX_COREBOOT_TABLE_SIZE CONFIG_COREBOOT_TABLE_SIZE
-
-void write_tables(void)
-{
-	unsigned long table_pointer, new_table_pointer;
-
-	post_code(0x9d);
-
-	table_pointer = (unsigned long)cbmem_add(CBMEM_ID_CBTABLE,
-						MAX_COREBOOT_TABLE_SIZE);
-	if (!table_pointer) {
-		printk(BIOS_ERR, "Could not add CBMEM for coreboot table.\n");
-		return;
-	}
-
-	new_table_pointer = write_coreboot_table(0UL, 0UL,
-				table_pointer, table_pointer);
-
-	if (new_table_pointer > (table_pointer + MAX_COREBOOT_TABLE_SIZE)) {
-		printk(BIOS_ERR, "coreboot table didn't fit (%lx/%x bytes)\n",
-			   new_table_pointer - table_pointer,
-				MAX_COREBOOT_TABLE_SIZE);
-	}
-
-	printk(BIOS_DEBUG, "coreboot table: %ld bytes.\n",
-			new_table_pointer - table_pointer);
-
-	post_code(0x9e);
-}
-
 void lb_arch_add_records(struct lb_header *header)
 {
 }
diff --git a/src/arch/riscv/tables.c b/src/arch/riscv/tables.c
index 92213c5..eef6bf2 100644
--- a/src/arch/riscv/tables.c
+++ b/src/arch/riscv/tables.c
@@ -15,14 +15,9 @@
  * GNU General Public License for more details.
  */
 
-#include <console/console.h>
-#include <cpu/cpu.h>
 #include <bootmem.h>
 #include <boot/tables.h>
 #include <boot/coreboot_tables.h>
-#include <string.h>
-#include <cbmem.h>
-#include <lib.h>
 
 void arch_write_tables(uintptr_t coreboot_table)
 {
@@ -32,35 +27,6 @@ void bootmem_arch_add_ranges(void)
 {
 }
 
-#define MAX_COREBOOT_TABLE_SIZE CONFIG_COREBOOT_TABLE_SIZE
-
-void write_tables(void)
-{
-	unsigned long table_pointer, new_table_pointer;
-
-	post_code(0x9d);
-
-	table_pointer = (unsigned long)cbmem_add(CBMEM_ID_CBTABLE,
-						MAX_COREBOOT_TABLE_SIZE);
-	if (!table_pointer) {
-		printk(BIOS_ERR, "Could not add CBMEM for coreboot table.\n");
-		return;
-	}
-
-	new_table_pointer = write_coreboot_table(0UL, 0UL,
-				table_pointer, table_pointer);
-
-	if (new_table_pointer > (table_pointer + MAX_COREBOOT_TABLE_SIZE)) {
-		printk(BIOS_ERR, "coreboot table didn't fit (%lx/%x bytes)\n",
-			   new_table_pointer - table_pointer, MAX_COREBOOT_TABLE_SIZE);
-	}
-
-	printk(BIOS_DEBUG, "coreboot table: %ld bytes.\n",
-			new_table_pointer - table_pointer);
-
-	post_code(0x9e);
-}
-
 void lb_arch_add_records(struct lb_header *header)
 {
 }
diff --git a/src/arch/x86/tables.c b/src/arch/x86/tables.c
index 94fe691..edcb717 100644
--- a/src/arch/x86/tables.c
+++ b/src/arch/x86/tables.c
@@ -27,8 +27,6 @@
 #include <cbmem.h>
 #include <smbios.h>
 
-#define MAX_COREBOOT_TABLE_SIZE CONFIG_COREBOOT_TABLE_SIZE
-
 static unsigned long write_pirq_table(unsigned long rom_table_end)
 {
 	unsigned long high_table_pointer;
@@ -181,8 +179,15 @@ static unsigned long write_smbios_table(unsigned long rom_table_end)
 	return rom_table_end;
 }
 
+/* Start forwarding table at 0x500, so we don't run into conflicts with the BDA
+ * in case our data structures grow beyond 0x400. Only GDT
+ * and the coreboot table use low_tables.
+ */
+static uintptr_t forwarding_table = 0x500;
+
 void arch_write_tables(uintptr_t coreboot_table)
 {
+	size_t sz;
 	unsigned long rom_table_end = 0xf0000;
 
 	/* This table must be between 0x0f0000 and 0x100000 */
@@ -198,56 +203,18 @@ void arch_write_tables(uintptr_t coreboot_table)
 
 	if (IS_ENABLED(CONFIG_GENERATE_SMBIOS_TABLES))
 		rom_table_end = write_smbios_table(rom_table_end);
-}
 
-void bootmem_arch_add_ranges(void)
-{
+	sz = write_coreboot_forwarding_table(forwarding_table, coreboot_table);
+
+	forwarding_table += sz;
+	/* Align up to page boundary for historical consistency. */
+	forwarding_table = ALIGN_UP(forwarding_table, 4*KiB);
 }
 
-void write_tables(void)
+void bootmem_arch_add_ranges(void)
 {
-	unsigned long low_table_start, low_table_end;
-
-	/* Even if high tables are configured, some tables are copied both to
-	 * the low and the high area, so payloads and OSes don't need to know
-	 * about the high tables.
-	 */
-	unsigned long high_table_pointer;
-
-	/* Start low addr at 0x500, so we don't run into conflicts with the BDA
-	 * in case our data structures grow beyond 0x400. Only GDT
-	 * and the coreboot table use low_tables.
-	 */
-	low_table_start = 0;
-	low_table_end = 0x500;
+	/* Memory from 0 through the forwarding_table is reserved. */
+	const uintptr_t base = 0;
 
-	post_code(0x9e);
-
-	post_code(0x9d);
-
-	high_table_pointer = (unsigned long)cbmem_add(CBMEM_ID_CBTABLE, MAX_COREBOOT_TABLE_SIZE);
-
-	if (high_table_pointer) {
-		unsigned long new_high_table_pointer;
-
-		/* FIXME: The high_table_base parameter is not reference when tables are high,
-		 * or high_table_pointer >1 MB.
-		 */
-		u64 fixme_high_tables_base = 0;
-
-		/* Also put a forwarder entry into 0-4K */
-		new_high_table_pointer = write_coreboot_table(low_table_start, low_table_end,
-				fixme_high_tables_base, high_table_pointer);
-
-		if (new_high_table_pointer > (high_table_pointer +
-					MAX_COREBOOT_TABLE_SIZE))
-			printk(BIOS_ERR, "%s: coreboot table didn't fit (%lx)\n",
-				   __func__, new_high_table_pointer -
-				   high_table_pointer);
-
-			printk(BIOS_DEBUG, "coreboot table: %ld bytes.\n",
-				new_high_table_pointer - high_table_pointer);
-	} else {
-		printk(BIOS_ERR, "Could not add CBMEM for coreboot table.\n");
-	}
+	bootmem_add_range(base, forwarding_table - base, LB_MEM_TABLE);
 }
diff --git a/src/include/boot/coreboot_tables.h b/src/include/boot/coreboot_tables.h
index dda1e1a..5647629 100644
--- a/src/include/boot/coreboot_tables.h
+++ b/src/include/boot/coreboot_tables.h
@@ -5,10 +5,6 @@
 #include <stddef.h>
 /* function prototypes for building the coreboot table */
 
-unsigned long write_coreboot_table(
-	unsigned long low_table_start, unsigned long low_table_end,
-	unsigned long rom_table_start, unsigned long rom_table_end);
-
 /*
  * Write forwarding table of target address at entry address returning size
  * of table written.
diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c
index 583f609..58c6f48 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -459,29 +459,12 @@ size_t write_coreboot_forwarding_table(uintptr_t entry, uintptr_t target)
 	return (uintptr_t)lb_table_fini(head) - entry;
 }
 
-unsigned long write_coreboot_table(
-	unsigned long low_table_start, unsigned long low_table_end,
-	unsigned long rom_table_start __unused, unsigned long rom_table_end)
+static uintptr_t write_coreboot_table(uintptr_t rom_table_end)
 {
 	struct lb_header *head;
 
-	arch_write_tables(rom_table_end);
-
-	if (low_table_start || low_table_end) {
-		printk(BIOS_DEBUG, "Writing table forward entry at 0x%08lx\n",
-				low_table_end);
-		head = lb_table_init(low_table_end);
-		lb_forward(head, (struct lb_header*)rom_table_end);
-
-		low_table_end = (unsigned long) lb_table_fini(head);
-		printk(BIOS_DEBUG, "Table forward entry ends at 0x%08lx.\n",
-			low_table_end);
-		low_table_end = ALIGN(low_table_end, 4096);
-		printk(BIOS_DEBUG, "... aligned to 0x%08lx\n", low_table_end);
-	}
-
 	printk(BIOS_DEBUG, "Writing coreboot table at 0x%08lx\n",
-		rom_table_end);
+		(long)rom_table_end);
 
 	head = lb_table_init(rom_table_end);
 
@@ -505,13 +488,6 @@ unsigned long write_coreboot_table(
 	/* Initialize the memory map at boot time. */
 	bootmem_init();
 
-	if (low_table_start || low_table_end) {
-		uint64_t size = low_table_end - low_table_start;
-		/* Record the mptable and the the lb_table.
-		 * (This will be adjusted later)  */
-		bootmem_add_range(low_table_start, size, LB_MEM_TABLE);
-	}
-
 	/* No other memory areas can be added after the memory table has been
 	 * committed as the entries won't show up in the serialize mem table. */
 	bootmem_write_memory_table(lb_memory(head));
@@ -575,9 +551,38 @@ unsigned long write_coreboot_table(
 	/* Add all cbmem entries into the coreboot tables. */
 	cbmem_add_records_to_cbtable(head);
 
-	/* Print CBMEM sections */
-	cbmem_list();
-
 	/* Remember where my valid memory ranges are */
 	return lb_table_fini(head);
 }
+
+void write_tables(void)
+{
+	uintptr_t cbtable_start;
+	uintptr_t cbtable_end;
+	size_t cbtable_size;
+	const size_t max_table_size = CONFIG_COREBOOT_TABLE_SIZE;
+
+	cbtable_start = (uintptr_t)cbmem_add(CBMEM_ID_CBTABLE, max_table_size);
+
+	if (!cbtable_start) {
+		printk(BIOS_ERR, "Could not add CBMEM for coreboot table.\n");
+		return;
+	}
+
+	/* Add architecture specific tables. */
+	arch_write_tables(cbtable_start);
+
+	/* Write the coreboot table. */
+	cbtable_end = write_coreboot_table(cbtable_start);
+	cbtable_size = cbtable_end - cbtable_start;
+
+	if (cbtable_size > max_table_size) {
+		printk(BIOS_ERR, "%s: coreboot table didn't fit (%zx/%zx)\n",
+			__func__, cbtable_size, max_table_size);
+	}
+
+	printk(BIOS_DEBUG, "coreboot table: %zd bytes.\n", cbtable_size);
+
+	/* Print CBMEM sections */
+	cbmem_list();
+}



More information about the coreboot-gerrit mailing list