[coreboot] Patch set updated for coreboot: 7ac1720 snow: cleanup ramstage and call hardwaremain()

Ronald G. Minnich (rminnich@gmail.com) gerrit at coreboot.org
Fri Feb 8 20:16:35 CET 2013


Ronald G. Minnich (rminnich at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/2302

-gerrit

commit 7ac172046b06b6be462b679bc7ba6eaaa12cdb62
Author: David Hendricks <dhendrix at chromium.org>
Date:   Wed Feb 6 16:29:42 2013 -0800

    snow: cleanup ramstage and call hardwaremain()
    
    ** do not submit (work-in-progress) **
    
    It *no longer* dies in write_coreboot_tables().
    The trick is that lb_table_fini is more than informational, it writes
    a record and hence can not be called before any other record has been
    written, or it writes garbage. Oops. That's a problem with the original
    too: it assumes at least one record written, which is safe, because
    there's always one record written. Hence one can not call
    lb_table_fini just to ask a question. It can only be called once.
    
    Note 1: For ARM we can get away with only using one table, the
    high table and low table distinction is an x86-ism.
    
    Note 2: We'll eventually need to fill in the device structure so
    that search_global_resources() can mark regions appropriately.
    Fortunately we can get by with the assumption that 0x4000_0000-
    0xffff_ffff is plain memory without any IO holes (We'll probably
    want to specify which regions are IO later on).
    
    Change-Id: I30200432535244de971a7cea9f7902f5e9b46403
    Signed-off-by: David Hendricks <dhendrix at chromium.org>
---
 src/arch/armv7/boot/coreboot_table.c          | 90 +++++++++++++++------------
 src/arch/armv7/boot/tables.c                  | 59 +++++++-----------
 src/arch/armv7/include/arch/coreboot_tables.h |  3 +-
 src/lib/compute_ip_checksum.c                 |  2 +
 src/lib/hardwaremain.c                        |  3 +-
 src/mainboard/google/snow/ramstage.c          | 17 +++--
 6 files changed, 89 insertions(+), 85 deletions(-)

diff --git a/src/arch/armv7/boot/coreboot_table.c b/src/arch/armv7/boot/coreboot_table.c
index b2d88aa..3acfe16 100644
--- a/src/arch/armv7/boot/coreboot_table.c
+++ b/src/arch/armv7/boot/coreboot_table.c
@@ -559,11 +559,13 @@ struct lb_memory *get_lb_mem(void)
 	return mem_ranges;
 }
 
+#if 0
 static void build_lb_mem_range(void *gp, struct device *dev, struct resource *res)
 {
 	struct lb_memory *mem = gp;
 	new_lb_memory_range(mem, LB_MEM_RAM, res->base, res->size);
 }
+#endif
 
 static struct lb_memory *build_lb_mem(struct lb_header *head)
 {
@@ -573,11 +575,15 @@ static struct lb_memory *build_lb_mem(struct lb_header *head)
 	mem = lb_memory(head);
 	mem_ranges = mem;
 
+	/* FIXME: implement this */
+#if 0
 	/* Build the raw table of memory */
 	search_global_resources(
 		IORESOURCE_MEM | IORESOURCE_CACHEABLE, IORESOURCE_MEM | IORESOURCE_CACHEABLE,
 		build_lb_mem_range, mem);
-	lb_cleanup_memory_ranges(mem);
+#endif
+	/* FIXME: things die in cleanup_memory_ranges(), skip for now */
+//	lb_cleanup_memory_ranges(mem);
 	return mem;
 }
 
@@ -596,47 +602,30 @@ static void add_lb_reserved(struct lb_memory *mem)
 }
 
 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)
+	unsigned long table_start, unsigned long table_end)
 {
 	struct lb_header *head;
 	struct lb_memory *mem;
+	unsigned long fini;
 
-#if CONFIG_WRITE_HIGH_TABLES
-	printk(BIOS_DEBUG, "Writing high 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, 0);
-	printk(BIOS_DEBUG, "New low_table_end: 0x%08lx\n", low_table_end);
-	printk(BIOS_DEBUG, "Now going to write high coreboot table at 0x%08lx\n",
-			rom_table_end);
-
-	head = lb_table_init(rom_table_end);
-	rom_table_end = (unsigned long)head;
-	printk(BIOS_DEBUG, "rom_table_end = 0x%08lx\n", rom_table_end);
-#else
-	if(low_table_end > (0x1000 - sizeof(struct lb_header))) { /* after 4K */
-		/* We need to put lbtable on  to [0xf0000,0x100000) */
-		head = lb_table_init(rom_table_end);
-		rom_table_end = (unsigned long)head;
-	} else {
-		head = lb_table_init(low_table_end);
-		low_table_end = (unsigned long)head;
-	}
-#endif
+	printk(BIOS_DEBUG, "table_start: 0x%lx, table_end: 0x%lx, ",
+			   table_start, table_end);
+	head = lb_table_init(table_start);
+
+	printk(BIOS_DEBUG, "writing head (0x%p)... ", head);
+	printk(BIOS_DEBUG, "Writing table forward entry at 0x%08lx\n",
+			table_end);
+	lb_forward(head, (struct lb_header*)table_end);
+	printk(BIOS_DEBUG, "done\n");
 
-	printk(BIOS_DEBUG, "Adjust low_table_end from 0x%08lx to ", low_table_end);
-	low_table_end += 0xfff; // 4K aligned
-	low_table_end &= ~0xfff;
-	printk(BIOS_DEBUG, "0x%08lx \n", low_table_end);
+	table_end = (unsigned long) head + head->table_bytes;
+	printk(BIOS_DEBUG, "New table_end: 0x%08lx\n", table_end);
 
-	/* The Linux kernel assumes this region is reserved */
-	printk(BIOS_DEBUG, "Adjust rom_table_end from 0x%08lx to ", rom_table_end);
-	rom_table_end += 0xffff; // 64K align
-	rom_table_end &= ~0xffff;
-	printk(BIOS_DEBUG, "0x%08lx \n", rom_table_end);
+	/* FIXME(dhendrix): do we need this? */
+	printk(BIOS_DEBUG, "Adjust table_end from 0x%08lx to ", table_end);
+	table_end += 0xfff; // 4K aligned
+	table_end &= ~0xfff;
+	printk(BIOS_DEBUG, "0x%08lx \n", table_end);
 
 #if CONFIG_USE_OPTION_TABLE
 	{
@@ -655,35 +644,50 @@ unsigned long write_coreboot_table(
 	}
 #endif
 	/* Record where RAM is located */
+	/* FIXME(dhendrix): add global resources */
+	printk(BIOS_DEBUG, "%s: head: 0x%p\n", __func__, head);
 	mem = build_lb_mem(head);
+	/* FIXME: we seem to get a bogus return value */
+	printk(BIOS_DEBUG, "%s: mem: 0x%p\n", __func__, mem);
+	if ((unsigned long)mem < CONFIG_RAMBASE) {
+		printk(BIOS_DEBUG, "%s: ZOMG PANIC!!!111n", __func__);
+		while (1);
+	}
 
 	/* Record the mptable and the the lb_table (This will be adjusted later) */
 	lb_add_memory_range(mem, LB_MEM_TABLE,
-		low_table_start, low_table_end - low_table_start);
+		table_start, table_end - table_start);
 
+	printk(BIOS_DEBUG, "%s: checkpoint 1\n", __func__);
 	/* Record the pirq table, acpi tables, and maybe the mptable */
 	lb_add_memory_range(mem, LB_MEM_TABLE,
-		rom_table_start, rom_table_end-rom_table_start);
+		table_start, table_end - table_start);
 
+	printk(BIOS_DEBUG, "%s: checkpoint 2\n", __func__);
 #if CONFIG_WRITE_HIGH_TABLES
 	printk(BIOS_DEBUG, "Adding high table area\n");
 	// should this be LB_MEM_ACPI?
 	lb_add_memory_range(mem, LB_MEM_TABLE,
-		high_tables_base, high_tables_size);
+			table_start, table_end - table_start);
 #endif
 
+	printk(BIOS_DEBUG, "%s: checkpoint 3\n", __func__);
 	/* Add reserved regions */
 	add_lb_reserved(mem);
 
+	printk(BIOS_DEBUG, "%s: checkpoint 4\n", __func__);
 	lb_dump_memory_ranges(mem);
+	printk(BIOS_DEBUG, "%s: checkpoint 5\n", __func__);
 
 	/* Note:
 	 * I assume that there is always memory at immediately after
-	 * the low_table_end.  This means that after I setup the coreboot table.
+	 * the table_end.  This means that after I setup the coreboot table.
 	 * I can trivially fixup the reserved memory ranges to hold the correct
 	 * size of the coreboot table.
 	 */
 
+	printk(BIOS_DEBUG, "%s: checkpoint 5\n", __func__);
+	/* FIXME(dhendrix): Most of these do nothing at the moment */
 	/* Record our motherboard */
 	lb_mainboard(head);
 	/* Record the serial port, if present */
@@ -695,6 +699,7 @@ unsigned long write_coreboot_table(
 	/* Record our framebuffer */
 	lb_framebuffer(head);
 
+#if 0
 #if CONFIG_CHROMEOS
 	/* Record our GPIO settings (ChromeOS specific) */
 	lb_gpios(head);
@@ -705,9 +710,12 @@ unsigned long write_coreboot_table(
 	/* pass along VBNV offsets in CMOS */
 	lb_vbnv(head);
 #endif
+#endif
 	add_cbmem_pointers(head);
 
 	/* Remember where my valid memory ranges are */
-	return lb_table_fini(head, 1);
+	fini =  lb_table_fini(head, 1);
+	printk(BIOS_DEBUG, "%s: DONE: fini is 0x%lx\n", __func__, fini);
+	return fini;
 
 }
diff --git a/src/arch/armv7/boot/tables.c b/src/arch/armv7/boot/tables.c
index 07c5635..a60ebca 100644
--- a/src/arch/armv7/boot/tables.c
+++ b/src/arch/armv7/boot/tables.c
@@ -28,6 +28,10 @@
 #include <cbmem.h>
 #include <lib.h>
 
+/*
+ * TODO: "High" tables are a convention used on x86. Maybe we can
+ * clean up that naming at some point.
+ */
 uint64_t high_tables_base = 0;
 uint64_t high_tables_size;
 
@@ -37,59 +41,40 @@ void cbmem_arch_init(void)
 
 struct lb_memory *write_tables(void)
 {
-	unsigned long low_table_start, low_table_end;
-	unsigned long rom_table_start, rom_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;
+	unsigned long table_pointer;
 
 	if (!high_tables_base) {
-		printk(BIOS_ERR, "ERROR: High Tables Base is not set.\n");
+		printk(BIOS_ERR, "ERROR: coreboot_tables_base is not set.\n");
 		// Are there any boards without?
 		// Stepan thinks we should die() here!
 	}
 
-	printk(BIOS_DEBUG, "High Tables Base is %llx.\n", high_tables_base);
-
-	rom_table_start = 0xf0000;
-	rom_table_end =   0xf0000;
-
-	/* Start low addr at 0x500, so we don't run into conflicts with the BDA
-	 * in case our data structures grow beyound 0x400. Only multiboot, GDT
-	 * and the coreboot table use low_tables.
-	 */
-	low_table_start = 0;
-	low_table_end = 0x500;
+	printk(BIOS_DEBUG, "high_tables_base: %llx.\n", high_tables_base);
 
 #define MAX_COREBOOT_TABLE_SIZE (8 * 1024)
 	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;
-
-		/* Also put a forwarder entry into 0-4K */
-		new_high_table_pointer = write_coreboot_table(low_table_start, low_table_end,
-				high_tables_base, high_table_pointer);
-
-		if (new_high_table_pointer > (high_table_pointer +
-					MAX_COREBOOT_TABLE_SIZE))
+	table_pointer = (unsigned long)cbmem_add(CBMEM_ID_CBTABLE,
+						MAX_COREBOOT_TABLE_SIZE);
+	if (table_pointer) {
+		unsigned long new_table_pointer;
+		new_table_pointer = write_coreboot_table(table_pointer,
+							high_tables_size);
+		if (table_pointer > (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 {
+				   __func__, new_table_pointer - table_pointer);
+		}
+		printk(BIOS_DEBUG, "coreboot table: %ld bytes.\n",
+			new_table_pointer - table_pointer);
+	}
+#if 0
+	else {
 		/* The coreboot table must be in 0-4K or 960K-1M */
 		rom_table_end = write_coreboot_table(
 				     low_table_start, low_table_end,
 				     rom_table_start, rom_table_end);
 	}
+#endif
 
 	post_code(0x9e);
 
diff --git a/src/arch/armv7/include/arch/coreboot_tables.h b/src/arch/armv7/include/arch/coreboot_tables.h
index 3c9bf98..e8bb7be 100644
--- a/src/arch/armv7/include/arch/coreboot_tables.h
+++ b/src/arch/armv7/include/arch/coreboot_tables.h
@@ -5,8 +5,7 @@
 
 /* This file holds 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);
+	unsigned long table_start, unsigned long table_end);
 
 void lb_memory_range(struct lb_memory *mem,
 	uint32_t type, uint64_t start, uint64_t size);
diff --git a/src/lib/compute_ip_checksum.c b/src/lib/compute_ip_checksum.c
index 48f93d4..8526f6d 100644
--- a/src/lib/compute_ip_checksum.c
+++ b/src/lib/compute_ip_checksum.c
@@ -1,6 +1,8 @@
 #include <stdint.h>
 #include <ip_checksum.h>
 
+#include <console/console.h>
+
 unsigned long compute_ip_checksum(void *addr, unsigned long length)
 {
 	uint8_t *ptr;
diff --git a/src/lib/hardwaremain.c b/src/lib/hardwaremain.c
index 85c05f5..43a11ad 100644
--- a/src/lib/hardwaremain.c
+++ b/src/lib/hardwaremain.c
@@ -114,7 +114,8 @@ void hardwaremain(int boot_complete)
 	timestamp_stash(TS_DEVICE_DONE);
 
 #if CONFIG_WRITE_HIGH_TABLES
-	cbmem_initialize();
+	int rc = cbmem_initialize();
+	printk(BIOS_DEBUG, "cbmem_initialize returned %d\n", rc);
 #if CONFIG_CONSOLE_CBMEM
 	cbmemc_reinit();
 #endif
diff --git a/src/mainboard/google/snow/ramstage.c b/src/mainboard/google/snow/ramstage.c
index f9d9c96..d280dbe 100644
--- a/src/mainboard/google/snow/ramstage.c
+++ b/src/mainboard/google/snow/ramstage.c
@@ -19,12 +19,21 @@
 
 #include <console/console.h>
 
+#if CONFIG_WRITE_HIGH_TABLES
+#include <cbmem.h>
+#endif
+
+void hardwaremain(int boot_complete);
 void main(void)
 {
-//	volatile unsigned long *pshold = (unsigned long *)0x1004330c;
-//	*pshold &= ~0x100;	/* shut down */
-
 	console_init();
 	printk(BIOS_INFO, "hello from ramstage\n");
-	while (1);
+
+#if CONFIG_WRITE_HIGH_TABLES
+	/* Leave some space for ACPI tables */
+	high_tables_base = CONFIG_RAMBASE;
+	high_tables_size = CONFIG_RAMBASE + 0x100000;
+#endif
+
+	hardwaremain(0);
 }



More information about the coreboot mailing list