Patchwork =?iso-8859-1?q?=A0=5BPATCH=5D_Generate_multiboot_table?= =?iso-8859-1?q?s_after=09and_from_coreboot_tables?=

login
register
about
Submitter Juhana Helovuo
Date 2010-08-30 18:54:49
Message ID <1283194490.3637.84.camel@bart>
Download mbox | patch
Permalink /patch/1818/
State Superseded
Headers show

Comments

Juhana Helovuo - 2010-08-30 18:54:49
Hello,

The attached patch adds all the memory memory ranges in the coreboot
tables also to the multiboot tables. It is useful e.g. when booting
Linux with Grub2 payload, since then Linux gets the Coreboot memory map
via multiboot tables.

This patch causes multiboot table to be located in memory before
coreboot tables, but it seemed to cause no harm for me.

Signed-off-by: Juhana Helovuo <juhe@iki.fi>

Best regards,
Juhana Helovuo
Patrick Georgi - 2010-08-30 19:46:52
Am 30.08.2010 20:54, schrieb Juhana Helovuo:
> Hello,
> 
> The attached patch adds all the memory memory ranges in the coreboot
> tables also to the multiboot tables. It is useful e.g. when booting
> Linux with Grub2 payload, since then Linux gets the Coreboot memory map
> via multiboot tables.
> 
> This patch causes multiboot table to be located in memory before
> coreboot tables, but it seemed to cause no harm for me.
> 
> Signed-off-by: Juhana Helovuo <juhe@iki.fi>
I like the change, except for the #if 0 section. Could that code be
removed entirely, or is there a reason for keeping it?
Also, what's up with reserved_mem_count? Does that serve any purpose
other than dumping the multiboot tables?


Regards,
Patrick
Juhana Helovuo - 2010-08-31 06:05:08
Patrick Georgi kirjoitti:
> Am 30.08.2010 20:54, schrieb Juhana Helovuo:
>> Hello,
>>
>> The attached patch adds all the memory memory ranges in the coreboot
>> tables also to the multiboot tables. It is useful e.g. when booting
>> Linux with Grub2 payload, since then Linux gets the Coreboot memory map
>> via multiboot tables.
>>
>> This patch causes multiboot table to be located in memory before
>> coreboot tables, but it seemed to cause no harm for me.
>>
>> Signed-off-by: Juhana Helovuo <juhe@iki.fi>
> I like the change, except for the #if 0 section. Could that code be
> removed entirely, or is there a reason for keeping it?

That section is AFAIK cleaning up in case some memory ranges overlap. 
Currently it is not used, because coreboot tables already do similar 
cleaning. I did not dare to remove it altogether, because some future 
extension may need to insert other memory ranges than those from 
coreboot tables.

If that is not a concern, I think there are two reasonable choices: (1) 
insert all entries to multiboot table via that cleanup routine, 
including ones copied from coreboot table, or (2) just copy everything 
from coreboot table and remove cleanup routines. Also remove the two 
built-in entries from multiboot table code, since those seem to be 
duplicated in coreboot table.

Option (2) results in much simpler code, but then multiboot table is 
entirely dependent on coreboot table. Is that ok as a design?


> Also, what's up with reserved_mem_count? Does that serve any purpose
> other than dumping the multiboot tables?

Not at the moment, since cleanup code is commented out. It is also 
needed in the cleanup routine.


Best regards,
Juhana Helovuo

Patch

diff -u -r -N -x .svn coreboot.orig.r5756/src/arch/i386/boot/multiboot.c coreboot.r5756/src/arch/i386/boot/multiboot.c
--- coreboot.orig.r5756/src/arch/i386/boot/multiboot.c	2010-08-31 05:11:34.000000000 +0300
+++ coreboot.r5756/src/arch/i386/boot/multiboot.c	2010-08-31 05:15:11.000000000 +0300
@@ -22,6 +22,9 @@ 
 #include <string.h>
 #include <device/resource.h>
 #include <console/console.h>
+#include <boot/coreboot_tables.h>
+#include <arch/coreboot_tables.h>
+
 
 static struct multiboot_mmap_entry *mb_mem;
 struct multiboot_info *mbi = NULL;
@@ -30,11 +33,14 @@ 
 	u64 addr;
 	u64 len;
 } reserved_mem[2];
+static int reserved_mem_count = 2; // must be the same as array size above
 
+#if 0
 static void build_mb_mem_range_nooverlap(u64 addr, u64 len)
 {
 	int i;
-	for (i = 0; i < sizeof(reserved_mem) / sizeof(reserved_mem[0]); i++) {
+
+	for (i = 0; i < reserved_mem_count; i++) {
 		/* free region fully contained in reserved region, abort */
 		if (addr >= reserved_mem[i].addr && addr + len <= reserved_mem[i].addr + reserved_mem[i].len)
 			return;
@@ -67,10 +73,12 @@ 
 	mb_mem++;
 }
 
+
 static void build_mb_mem_range(void *gp, struct device *dev, struct resource *res)
 {
 	build_mb_mem_range_nooverlap(res->base, res->size);
 }
+#endif
 
 #define ROUND(_r,_a) (((_r) + (((_a) - 1))) & ~((_a) - 1))
 
@@ -78,7 +86,10 @@ 
 	unsigned long low_table_start, unsigned long low_table_end,
 	unsigned long rom_table_start, unsigned long rom_table_end)
 {
+        struct lb_memory* coreboot_table;
+	int entries;
 	int i;
+	struct multiboot_mmap_entry *multiboot_mmap_table;  
 
 	mbi = (struct multiboot_info *)rom_table_end;
 
@@ -87,10 +98,9 @@ 
 
 	mbi->mmap_addr = (u32) rom_table_end;
 	mb_mem = (struct multiboot_mmap_entry *)rom_table_end;
+	multiboot_mmap_table = mb_mem; // Save a copy for dumpig table later
+
 
-	/* FIXME This code is broken, it does not know about high memory
-	 * tables, nor does it reserve the coreboot table area.
-	 */
 	/* reserved regions */
 	reserved_mem[0].addr = low_table_start;
 	reserved_mem[0].len = ROUND(low_table_end - low_table_start, 4096);
@@ -100,15 +110,48 @@ 
 	for (i = 0; i < sizeof(reserved_mem) / sizeof(reserved_mem[0]); i++) {
 		mb_mem->addr = reserved_mem[i].addr;
 		mb_mem->len = reserved_mem[i].len;
-		mb_mem->type = 2;
+		mb_mem->type = MULTIBOOT_MEMORY_RESERVED;
 		mb_mem->size = sizeof(*mb_mem) - sizeof(mb_mem->size);
 		mb_mem++;
 	}
 
-	/* free regions */
-	search_global_resources( IORESOURCE_MEM | IORESOURCE_CACHEABLE,
-		IORESOURCE_MEM | IORESOURCE_CACHEABLE, build_mb_mem_range, NULL);
+        /* regions from coreboot tables */
+	coreboot_table = get_lb_mem();
+	entries = (coreboot_table->size - sizeof(*coreboot_table))/sizeof(coreboot_table->map[0]);
+
+	if (coreboot_table == NULL || entries < 1) {
+	    printk(BIOS_INFO, "%s: Cannot find coreboot table for filling multiboot table.\n",
+		   __FILE__);
+	    return (unsigned long) mb_mem;
+	}
 
+	for (i = 0; i < entries; i++) {
+	  uint64_t entry_start = unpack_lb64(coreboot_table->map[i].start);
+	  uint64_t entry_size = unpack_lb64(coreboot_table->map[i].size);
+	  mb_mem->addr = entry_start;
+	  mb_mem->len = entry_size;
+	  switch (coreboot_table->map[i].type) {
+	    case LB_MEM_RAM:
+	      mb_mem->type = MULTIBOOT_MEMORY_AVAILABLE;  
+	      break;
+	    default:
+	      mb_mem->type = MULTIBOOT_MEMORY_RESERVED; // anything else than usable RAM
+	      break;
+	  }
+	  mb_mem->size = sizeof(*mb_mem) - sizeof(mb_mem->size);
+	  mb_mem++;
+	  reserved_mem_count++;
+	}    
+
+	printk(BIOS_DEBUG, "Multiboot memory table dump, %d entries.\n",reserved_mem_count);
+	for (i = 0; i < reserved_mem_count; i++) {
+	  printk(BIOS_DEBUG, " %02d. %016llx %016llx : %s\n", i ,
+		 multiboot_mmap_table[i].addr,
+		 multiboot_mmap_table[i].len,
+		 (multiboot_mmap_table[i].type == 1 ? "RAM" : "RESERVED")
+		);
+	}
+	  
 	mbi->mmap_length = ((u32) mb_mem) - mbi->mmap_addr;
 	mbi->flags |= MB_INFO_MEM_MAP;
 
diff -u -r -N -x .svn coreboot.orig.r5756/src/arch/i386/boot/tables.c coreboot.r5756/src/arch/i386/boot/tables.c
--- coreboot.orig.r5756/src/arch/i386/boot/tables.c	2010-08-31 05:11:34.000000000 +0300
+++ coreboot.r5756/src/arch/i386/boot/tables.c	2010-08-31 05:15:11.000000000 +0300
@@ -179,14 +179,6 @@ 
 
 #endif
 
-#if CONFIG_MULTIBOOT
-	post_code(0x9d);
-
-	/* The Multiboot information structure */
-	rom_table_end = write_multiboot_info(
-				low_table_start, low_table_end,
-				rom_table_start, rom_table_end);
-#endif
 
 #define MAX_COREBOOT_TABLE_SIZE (8 * 1024)
 	post_code(0x9d);
@@ -210,7 +202,7 @@ 
 				new_high_table_pointer - high_table_pointer);
 	} else {
 		/* The coreboot table must be in 0-4K or 960K-1M */
-		write_coreboot_table(low_table_start, low_table_end,
+		rom_table_end = write_coreboot_table(low_table_start, low_table_end,
 			      rom_table_start, rom_table_end);
 	}
 
@@ -224,6 +216,15 @@ 
 	cbmem_add(CBMEM_ID_RESUME, 1024 * (1024-64));
 #endif
 
+#if CONFIG_MULTIBOOT
+	post_code(0x9d);
+
+	/* The Multiboot information structure */
+	rom_table_end = write_multiboot_info(
+				low_table_start, low_table_end,
+				rom_table_start, rom_table_end);
+#endif
+
 	// Remove before sending upstream
 	cbmem_list();
 
diff -u -r -N -x .svn coreboot.orig.r5756/src/include/cpu/x86/multiboot.h coreboot.r5756/src/include/cpu/x86/multiboot.h
--- coreboot.orig.r5756/src/include/cpu/x86/multiboot.h	2010-08-31 05:11:34.000000000 +0300
+++ coreboot.r5756/src/include/cpu/x86/multiboot.h	2010-08-31 05:15:11.000000000 +0300
@@ -167,6 +167,9 @@ 
 	uint16_t vbe_interface_len;
 };
 
+#define MULTIBOOT_MEMORY_AVAILABLE              1
+#define MULTIBOOT_MEMORY_RESERVED               2
+
 struct multiboot_mmap_entry {
 	uint32_t size;
 	uint64_t addr;
diff -u -r -N -x .svn coreboot.orig.r5756/src/mainboard/asus/Kconfig coreboot.r5756/src/mainboard/asus/Kconfig