Stefan Reinauer wrote:
Jordan Crouse wrote:
Stefan Reinauer wrote:
Jordan Crouse wrote:
Jordan Crouse wrote:
Having a copious amount of time on my hands right now, I had occasion
to review the coreboot v2 multiboot patch from some time back.

http://www.coreboot.org/pipermail/coreboot/2008-September/039364.html
The patch mentioned in this URL unconditionally adds a multiboot table
and the code to handle that. Please don't do that - not everyone wants
to use multiboot.
Why not?   It doesn't add any appreciable size, and we have plenty of
room in that segment for all the tables we need.
Exactly. For those we need.

Why would we add tables we don't need, given the case we explicitly
disabled the functionality in the first place?

C'mon, all I'm asking for is to put multiboot code into #if
CONFIG_MULTIBOOT guards. If the plan is, however, to force everyone into
multiboot, I suggest to drop the CONFIG_MULTIBOOT option completely.

Changes made.  Can I get a quick sanity check before committing?

Jordan
Signed-off-by: Robert Millan <[EMAIL PROTECTED]>

Index: src/include/cpu/x86/multiboot.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/include/cpu/x86/multiboot.h	2008-11-07 13:45:38.000000000 -0700
@@ -0,0 +1,175 @@
+/* multiboot.h - multiboot header file. */
+/*
+ *  Copyright (C) 2003  Free Software Foundation, Inc.
+ *  Copyright (C) 2008  Robert Millan
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef MULTIBOOT_H
+#define MULTIBOOT_H
+
+#include <stdint.h>
+
+/* How many bytes from the start of the file we search for the header.  */
+#define MB_SEARCH                 8192
+
+/* The magic field should contain this.  */
+#define MB_MAGIC                  0x1BADB002
+
+/* This should be in %eax.  */
+#define MB_MAGIC2                 0x2BADB002
+
+/* The bits in the required part of flags field we don't support.  */
+#define MB_UNSUPPORTED            0x0000fffc
+
+/* Alignment of multiboot modules.  */
+#define MB_MOD_ALIGN              0x00001000
+
+/*
+ * Flags set in the 'flags' member of the multiboot header.
+ */
+
+/* Align all boot modules on i386 page (4KB) boundaries.  */
+#define MB_PAGE_ALIGN		0x00000001
+
+/* Must pass memory information to OS.  */
+#define MB_MEMORY_INFO		0x00000002
+
+/* Must pass video information to OS.  */
+#define MB_VIDEO_MODE		0x00000004
+
+/* This flag indicates the use of the address fields in the header.  */
+#define MB_AOUT_KLUDGE		0x00010000
+
+/*
+ *  Flags to be set in the 'flags' member of the multiboot info structure.
+ */
+
+/* is there basic lower/upper memory information? */
+#define MB_INFO_MEMORY		0x00000001
+/* is there a boot device set? */
+#define MB_INFO_BOOTDEV		0x00000002
+/* is the command-line defined? */
+#define MB_INFO_CMDLINE		0x00000004
+/* are there modules to do something with? */
+#define MB_INFO_MODS		0x00000008
+
+/* These next two are mutually exclusive */
+
+/* is there a symbol table loaded? */
+#define MB_INFO_AOUT_SYMS		0x00000010
+/* is there an ELF section header table? */
+#define MB_INFO_ELF_SHDR		0x00000020
+
+/* is there a full memory map? */
+#define MB_INFO_MEM_MAP		0x00000040
+
+/* Is there drive info?  */
+#define MB_INFO_DRIVE_INFO		0x00000080
+
+/* Is there a config table?  */
+#define MB_INFO_CONFIG_TABLE	0x00000100
+
+/* Is there a boot loader name?  */
+#define MB_INFO_BOOT_LOADER_NAME	0x00000200
+
+/* Is there a APM table?  */
+#define MB_INFO_APM_TABLE		0x00000400
+
+/* Is there video information?  */
+#define MB_INFO_VIDEO_INFO		0x00000800
+
+struct multiboot_header {
+	/* Must be MB_MAGIC - see above.  */
+	uint32_t magic;
+
+	/* Feature flags.  */
+	uint32_t flags;
+
+	/* The above fields plus this one must equal 0 mod 2^32. */
+	uint32_t checksum;
+
+	/* These are only valid if MB_AOUT_KLUDGE is set.  */
+	uint32_t header_addr;
+	uint32_t load_addr;
+	uint32_t load_end_addr;
+	uint32_t bss_end_addr;
+	uint32_t entry_addr;
+
+	/* These are only valid if MB_VIDEO_MODE is set.  */
+	uint32_t mode_type;
+	uint32_t width;
+	uint32_t height;
+	uint32_t depth;
+};
+
+struct multiboot_info {
+	/* Multiboot info version number */
+	uint32_t flags;
+
+	/* Available memory from BIOS */
+	uint32_t mem_lower;
+	uint32_t mem_upper;
+
+	/* "root" partition */
+	uint32_t boot_device;
+
+	/* Kernel command line */
+	uint32_t cmdline;
+
+	/* Boot-Module list */
+	uint32_t mods_count;
+	uint32_t mods_addr;
+
+	uint32_t syms[4];
+
+	/* Memory Mapping buffer */
+	uint32_t mmap_length;
+	uint32_t mmap_addr;
+
+	/* Drive Info buffer */
+	uint32_t drives_length;
+	uint32_t drives_addr;
+
+	/* ROM configuration table */
+	uint32_t config_table;
+
+	/* Boot Loader Name */
+	uint32_t boot_loader_name;
+
+	/* APM table */
+	uint32_t apm_table;
+
+	/* Video */
+	uint32_t vbe_control_info;
+	uint32_t vbe_mode_info;
+	uint16_t vbe_mode;
+	uint16_t vbe_interface_seg;
+	uint16_t vbe_interface_off;
+	uint16_t vbe_interface_len;
+};
+
+struct multiboot_mmap_entry {
+	uint32_t size;
+	uint64_t addr;
+	uint64_t len;
+	uint32_t type;
+} __attribute__ ((packed));
+
+extern struct multiboot_info *mbi;
+
+unsigned long  write_multiboot_info(unsigned long, unsigned long, unsigned long, unsigned long);
+
+#endif
Index: src/config/Options.lb
===================================================================
--- src/config/Options.lb.orig	2008-11-06 11:47:38.000000000 -0700
+++ src/config/Options.lb	2008-11-07 13:45:38.000000000 -0700
@@ -608,6 +608,11 @@
 # Boot options
 ###############################################
 
+define CONFIG_MULTIBOOT
+	default 1
+	export always
+	comment "Use Multiboot (rather than ELF boot notes) to boot the payload"
+end
 define CONFIG_IDE_PAYLOAD
 	default 0
 	export always
Index: src/arch/i386/boot/multiboot.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/arch/i386/boot/multiboot.c	2008-11-11 09:12:01.000000000 -0700
@@ -0,0 +1,122 @@
+/*
+ * support for Multiboot payloads
+ *
+ * Copyright (C) 2008 Robert Millan
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#if CONFIG_MULTIBOOT
+
+#include <cpu/x86/multiboot.h>
+#include <string.h>
+#include <device/resource.h>
+#include <console/console.h>
+
+typedef unsigned long long u64;
+
+static struct multiboot_mmap_entry *mb_mem;
+struct multiboot_info *mbi;
+
+static struct {
+	u64 addr;
+	u64 len;
+} reserved_mem[2];
+
+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++) {
+		/* 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;
+		/* reserved region splits free region */
+		if (addr < reserved_mem[i].addr && addr + len > reserved_mem[i].addr + reserved_mem[i].len) {
+			build_mb_mem_range_nooverlap(addr, reserved_mem[i].addr - addr);
+			build_mb_mem_range_nooverlap(reserved_mem[i].addr + reserved_mem[i].len, (addr + len) - (reserved_mem[i].addr + reserved_mem[i].len));
+			return;
+		}
+		/* left overlap */
+		if (addr < reserved_mem[i].addr + reserved_mem[i].len && addr + len > reserved_mem[i].addr + reserved_mem[i].len) {
+			len += addr;
+			addr = reserved_mem[i].addr + reserved_mem[i].len;
+			len -= addr;
+			/* len += addr - old_addr */
+			continue;
+		}
+		/* right overlap */
+		if (addr < reserved_mem[i].addr && addr + len > reserved_mem[i].addr) {
+			len = reserved_mem[i].addr - addr;
+			continue;
+		}
+		/* none of the above, just add it */
+	}
+
+	mb_mem->addr = addr;
+	mb_mem->len = len;
+	mb_mem->type = 1;
+	mb_mem->size = sizeof(*mb_mem) - sizeof(mb_mem->size);
+	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);
+}
+
+#define ROUND(_r,_a) ((_r) + (((_a) - 1)) & ~((_a) - 1))
+
+unsigned long write_multiboot_info(
+	unsigned long low_table_start, unsigned long low_table_end,
+	unsigned long rom_table_start, unsigned long rom_table_end)
+{
+	struct multiboot_info *mbi;
+	int i;
+
+	mbi = rom_table_end;
+	memset(mbi, 0, sizeof(*mbi));
+	rom_table_end += sizeof(*mbi);
+
+	mbi->mmap_addr = (u32) rom_table_end;
+	mb_mem = rom_table_end;
+
+	/* reserved regions */
+	reserved_mem[0].addr = low_table_start;
+	reserved_mem[0].len = ROUND(low_table_end - low_table_start, 4096);
+	reserved_mem[1].addr = rom_table_start;
+	reserved_mem[1].len = ROUND(rom_table_end - rom_table_start, 4096);
+
+	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->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);
+
+	mbi->mmap_length = ((u32) mb_mem) - mbi->mmap_addr;
+	mbi->flags |= MB_INFO_MEM_MAP;
+
+	printk_info("Multiboot Information structure has been written.\n");
+
+	return mb_mem;
+}
+
+#endif
+
Index: src/arch/i386/boot/Config.lb
===================================================================
--- src/arch/i386/boot/Config.lb.orig	2008-11-06 11:47:48.000000000 -0700
+++ src/arch/i386/boot/Config.lb	2008-11-07 13:45:38.000000000 -0700
@@ -3,6 +3,7 @@
 
 object boot.o
 object coreboot_table.o
+object multiboot.o
 object tables.o
 if HAVE_PIRQ_TABLE
 object pirq_routing.o 
Index: src/arch/i386/boot/boot.c
===================================================================
--- src/arch/i386/boot/boot.c.orig	2008-11-06 11:47:48.000000000 -0700
+++ src/arch/i386/boot/boot.c	2008-11-11 08:59:22.000000000 -0700
@@ -3,6 +3,7 @@
 #include <boot/elf.h>
 #include <boot/elf_boot.h>
 #include <string.h>
+#include <cpu/x86/multiboot.h>
 
 
 #ifndef CMD_LINE
@@ -139,7 +140,7 @@
 		"	rep	movsl\n\t"
 
 		/* Now jump to the loaded image */
-		"	movl	$0x0E1FB007, %%eax\n\t"
+		"	movl	%5, %%eax\n\t"
 		"	movl	 0(%%esp), %%ebx\n\t"
 		"	call	*4(%%esp)\n\t"
 
@@ -175,7 +176,12 @@
 
 		:: 
 		"g" (lb_start), "g" (buffer), "g" (lb_size),
-		"g" (entry), "g"(adjusted_boot_notes)
+		"g" (entry),
+#if CONFIG_MULTIBOOT
+		"g"(mbi), "g" (MB_MAGIC2)
+#else
+		"g"(adjusted_boot_notes), "g" (0x0E1FB007)
+#endif
 		);
 }
 
Index: src/arch/i386/boot/tables.c
===================================================================
--- src/arch/i386/boot/tables.c.orig	2008-11-06 11:47:48.000000000 -0700
+++ src/arch/i386/boot/tables.c	2008-11-11 08:59:00.000000000 -0700
@@ -9,6 +9,7 @@
 #include <arch/smp/mpspec.h>
 #include <arch/acpi.h>
 #include <string.h>
+#include <cpu/x86/multiboot.h>
 #include "coreboot_table.h"
 
 // Global Descriptor Table, defined in c_start.S
@@ -106,6 +107,14 @@
 	move_gdt(low_table_end);
 	low_table_end += &gdt_end - &gdt;
 
+#if CONFIG_MULTIBOOT
+	/* The Multiboot information structure */
+	mbi = rom_table_end;
+	rom_table_end = write_multiboot_info(
+				low_table_start, low_table_end,
+				rom_table_start, rom_table_end);
+#endif
+
 	/* The coreboot table must be in 0-4K or 960K-1M */
 	write_coreboot_table(low_table_start, low_table_end,
 			      rom_table_start, rom_table_end);
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to