Re: [PATCH] decouple mmap parsing and implement Multiboot mmap in the loader

[Robert Millan <rmh@aybabtu.com>]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]


And here we go *AGAIN*, this time not forgetting to include all files in the
patch.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: mmap.diff --]
[-- Type: text/x-diff, Size: 17138 bytes --]

2008-08-12  Robert Millan  <rmh@aybabtu.com>

	* conf/i386-pc.rmk (kernel_img_SOURCES): Add `kern/i386/pc/mmap.c'.

	* include/grub/i386/pc/init.h: Include `<grub/multiboot.h>'.
	(GRUB_MACHINE_MEMORY_AVAILABLE, GRUB_MACHINE_MEMORY_RESERVED): New
	macros.
	(grub_mmap_iterate): New function declaration.
	(struct grub_machine_mmap_entry): Move from here ...
	* include/grub/multiboot.h (struct grub_mmap_entry): ... to here.
	Update all users.
	(GRUB_MMAP_MEMORY_AVAILABLE, GRUB_MMAP_MEMORY_RESERVED): New
	macros.

	* kern/i386/pc/init.c (grub_machine_init): Replace hardcoded region
	type check value with `GRUB_MACHINE_MEMORY_AVAILABLE'.
	Move e820 parsing from here ...
	* kern/i386/pc/mmap.c: New file.
	(grub_mmap_iterate): ... to here.

	* include/grub/i386/coreboot/memory.h: Remove `<grub/err.h>'.
	(GRUB_LINUXBIOS_MEMORY_AVAILABLE): Rename (for consistency) to ...
	(GRUB_MACHINE_MEMORY_AVAILABLE): ... this.  Update all users.
	(grub_available_iterate): Redeclare to return `void', and redeclare
	its hook to use grub_uint64_t as addr and size parameters, and rename
	to ...
	(grub_mmap_iterate): ... this.  Update all users.

	* kern/i386/coreboot/mmap.c (grub_mmap_iterate): Simplify parser loop
	to make it more readable.

	* loader/i386/pc/multiboot.c (mmap_addr, mmap_length): New variables.
	(grub_get_multiboot_mmap_len, grub_fill_multiboot_mmap): New functions.
	(grub_multiboot): Allocate an extra region after the payload, and fill
	it with a Multiboot memory map.  Adjust a.out loader to calculate size
	with the extra space.
	(grub_multiboot_load_elf32): Adjust elf32 loader to calculate size
	with the extra space.

Index: conf/i386-pc.rmk
===================================================================
--- conf/i386-pc.rmk	(revision 1802)
+++ conf/i386-pc.rmk	(working copy)
@@ -43,7 +43,8 @@
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
 	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
 	kern/time.c \
-	kern/i386/dl.c kern/i386/pc/init.c kern/parser.c kern/partition.c \
+	kern/i386/dl.c kern/i386/pc/init.c kern/i386/pc/mmap.c \
+	kern/parser.c kern/partition.c \
 	kern/i386/tsc.c kern/i386/pit.c \
 	kern/generic/rtc_get_time_ms.c \
 	kern/generic/millisleep.c \
Index: kern/i386/pc/init.c
===================================================================
--- kern/i386/pc/init.c	(revision 1802)
+++ kern/i386/pc/init.c	(working copy)
@@ -132,9 +132,6 @@
 void
 grub_machine_init (void)
 {
-  grub_uint32_t cont;
-  struct grub_machine_mmap_entry *entry
-    = (struct grub_machine_mmap_entry *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
   int i;
   
   /* Initialize the console as early as possible.  */
@@ -156,55 +153,35 @@
     add_mem_region (GRUB_MEMORY_MACHINE_RESERVED_END,
 		    grub_lower_mem - GRUB_MEMORY_MACHINE_RESERVED_END);
   
-  /* Check if grub_get_mmap_entry works.  */
-  cont = grub_get_mmap_entry (entry, 0);
-
-  if (entry->size)
-    do
-      {
-	/* Avoid the lower memory.  */
-	if (entry->addr < 0x100000)
-	  {
-	    if (entry->len <= 0x100000 - entry->addr)
-	      goto next;
-
-	    entry->len -= 0x100000 - entry->addr;
-	    entry->addr = 0x100000;
-	  }
-	
-	/* Ignore >4GB.  */
-	if (entry->addr <= 0xFFFFFFFF && entry->type == 1)
-	  {
-	    grub_addr_t addr;
-	    grub_size_t len;
-
-	    addr = (grub_addr_t) entry->addr;
-	    len = ((addr + entry->len > 0xFFFFFFFF)
-		   ? 0xFFFFFFFF - addr
-		   : (grub_size_t) entry->len);
-	    add_mem_region (addr, len);
-	  }
-	
-      next:
-	if (! cont)
-	  break;
-	
-	cont = grub_get_mmap_entry (entry, cont);
-      }
-    while (entry->size);
-  else
+  auto int hook (grub_uint64_t, grub_uint64_t, grub_uint32_t);
+  int hook (grub_uint64_t addr, grub_uint64_t size, grub_uint32_t type)
     {
-      grub_uint32_t eisa_mmap = grub_get_eisa_mmap ();
-
-      if (eisa_mmap)
+      /* Avoid the lower memory.  */
+      if (addr < 0x100000)
 	{
-	  add_mem_region (0x100000, (eisa_mmap & 0xFFFF) << 10);
-	  add_mem_region (0x1000000, eisa_mmap & ~0xFFFF);
+	  if (size <= 0x100000 - addr)
+	    return 0;
+	  
+	  size -= 0x100000 - addr;
+	  addr = 0x100000;
 	}
-      else
-	add_mem_region (0x100000, grub_get_memsize (1) << 10);
+	
+      /* Ignore >4GB.  */
+      if (addr <= 0xFFFFFFFF && type == GRUB_MMAP_MEMORY_AVAILABLE)
+	{
+	  grub_size_t len;
+	  
+	  len = (grub_size_t) ((addr + size > 0xFFFFFFFF)
+		 ? 0xFFFFFFFF - addr
+		 : size);
+	  add_mem_region (addr, len);
+	}
+
+      return 0;
     }
 
+  grub_mmap_iterate (hook);
+  
   compact_mem_regions ();
 
   /* Add the memory regions to free memory, except for the region starting
Index: kern/i386/pc/mmap.c
===================================================================
--- kern/i386/pc/mmap.c	(revision 0)
+++ kern/i386/pc/mmap.c	(revision 0)
@@ -0,0 +1,60 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2002,2003,2004,2005,2006,2007,2008  Free Software Foundation, Inc.
+ *
+ *  GRUB 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 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB 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 GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/machine/init.h>
+#include <grub/machine/memory.h>
+#include <grub/types.h>
+
+void
+grub_mmap_iterate (int NESTED_FUNC_ATTR (*hook) (grub_uint64_t, grub_uint64_t, grub_uint32_t))
+{
+  grub_uint32_t cont;
+  struct grub_mmap_entry *entry
+    = (struct grub_mmap_entry *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+  
+  /* Check if grub_get_mmap_entry works.  */
+  cont = grub_get_mmap_entry (entry, 0);
+
+  if (entry->size)
+    do
+      {
+	if (hook (entry->addr, entry->len,
+		  /* Multiboot mmaps have been defined to match with the E820 definition.
+		     Therefore, we can just pass type through.  */
+		  entry->type))
+	  break;
+
+	if (! cont)
+	  break;
+
+	cont = grub_get_mmap_entry (entry, cont);
+      }
+    while (entry->size);
+  else
+    {
+      grub_uint32_t eisa_mmap = grub_get_eisa_mmap ();
+
+      if (eisa_mmap)
+	{
+	  if (hook (0x100000, (eisa_mmap & 0xFFFF) << 10, GRUB_MMAP_MEMORY_AVAILABLE) == 0)
+	    hook (0x1000000, eisa_mmap & ~0xFFFF, GRUB_MMAP_MEMORY_AVAILABLE);
+	}
+      else
+	hook (0x100000, grub_get_memsize (1) << 10, GRUB_MMAP_MEMORY_AVAILABLE);
+    }
+}
Index: kern/i386/coreboot/init.c
===================================================================
--- kern/i386/coreboot/init.c	(revision 1802)
+++ kern/i386/coreboot/init.c	(working copy)
@@ -82,12 +82,9 @@
   grub_lower_mem = GRUB_MEMORY_MACHINE_LOWER_USABLE;
   grub_upper_mem = 0;
 
-  auto int heap_init (mem_region_t);
-  int heap_init (mem_region_t mem_region)
+  auto int heap_init (grub_uint64_t, grub_uint64_t, grub_uint32_t);
+  int heap_init (grub_uint64_t addr, grub_uint64_t size, grub_uint32_t type)
   {
-    grub_uint64_t addr = mem_region->addr;
-    grub_uint64_t size = mem_region->size;
-
 #if GRUB_CPU_SIZEOF_VOID_P == 4
     /* Restrict ourselves to 32-bit memory space.  */
     if (addr > ULONG_MAX)
@@ -101,7 +98,7 @@
 
     grub_upper_mem = grub_max (grub_upper_mem, addr + size);
 
-    if (mem_region->type != GRUB_LINUXBIOS_MEMORY_AVAILABLE)
+    if (type != GRUB_MACHINE_MEMORY_AVAILABLE)
       return 0;
 
     /* Avoid the lower memory.  */
@@ -134,7 +131,7 @@
     return 0;
   }
 
-  grub_available_iterate (heap_init);
+  grub_mmap_iterate (heap_init);
 
   /* This variable indicates size, not offset.  */
   grub_upper_mem -= GRUB_MEMORY_MACHINE_UPPER_START;
Index: kern/i386/coreboot/mmap.c
===================================================================
--- kern/i386/coreboot/mmap.c	(revision 1802)
+++ kern/i386/coreboot/mmap.c	(working copy)
@@ -63,8 +63,8 @@
   return 0;
 }
 
-grub_err_t
-grub_available_iterate (int (*hook) (mem_region_t))
+void
+grub_mmap_iterate (int (*hook) (grub_uint64_t, grub_uint64_t, grub_uint32_t))
 {
   mem_region_t mem_region;
 
@@ -77,11 +77,17 @@
     mem_region =
       (mem_region_t) ((long) table_item +
 				 sizeof (struct grub_linuxbios_table_item));
-    for (; (long) mem_region < (long) table_item + (long) table_item->size;
-	 mem_region++)
-      if (hook (mem_region))
-	return 1;
+    while ((long) mem_region < (long) table_item + (long) table_item->size)
+      {
+	if (hook (mem_region->addr, mem_region->size,
+		  /* Multiboot mmaps match with the coreboot mmap definition.
+		     Therefore, we can just pass type through.  */
+		  mem_region->type))
+	  return 1;
 
+	mem_region++;
+      }
+
     return 0;
   }
 
Index: include/grub/i386/pc/init.h
===================================================================
--- include/grub/i386/pc/init.h	(revision 1802)
+++ include/grub/i386/pc/init.h	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2004,2005,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2004,2005,2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -21,6 +21,8 @@
 
 #include <grub/types.h>
 #include <grub/symbol.h>
+#include <grub/multiboot.h>	/* For struct grub_mmap_entry, which is also
+				   needed by Multiboot.  */
 
 /* Get the memory size in KB. If EXTENDED is zero, return conventional
    memory, otherwise return extended memory.  */
@@ -30,19 +32,18 @@
    in 1KB parts, and upper 16 bits are above 16MB in 64KB parts.  */
 grub_uint32_t grub_get_eisa_mmap (void);
 
-struct grub_machine_mmap_entry
-{
-  grub_uint32_t size;
-  grub_uint64_t addr;
-  grub_uint64_t len;
-  grub_uint32_t type;
-} __attribute__((packed));
+/* Multiboot mmaps have been defined to match with the E820 definition.  */
+#define GRUB_MACHINE_MEMORY_AVAILABLE	GRUB_MMAP_MEMORY_AVAILABLE
+#define GRUB_MACHINE_MEMORY_RESERVED	GRUB_MMAP_MEMORY_RESERVED
 
 /* Get a memory map entry. Return next continuation value. Zero means
    the end.  */
-grub_uint32_t EXPORT_FUNC(grub_get_mmap_entry) (struct grub_machine_mmap_entry *entry,
+grub_uint32_t EXPORT_FUNC(grub_get_mmap_entry) (struct grub_mmap_entry *entry,
 				   grub_uint32_t cont);
 
+void EXPORT_FUNC(grub_mmap_iterate)
+     (int (*hook) (grub_uint64_t, grub_uint64_t, grub_uint32_t));
+
 /* Turn on/off Gate A20.  */
 void grub_gate_a20 (int on);
 
Index: include/grub/i386/coreboot/memory.h
===================================================================
--- include/grub/i386/coreboot/memory.h	(revision 1802)
+++ include/grub/i386/coreboot/memory.h	(working copy)
@@ -25,7 +25,6 @@
 
 #ifndef ASM_FILE
 #include <grub/types.h>
-#include <grub/err.h>
 #endif
 
 #define GRUB_MEMORY_MACHINE_LOWER_USABLE		0x9fc00		/* 640 kiB - 1 kiB */
@@ -55,13 +54,13 @@
 {
   grub_uint64_t addr;
   grub_uint64_t size;
-#define GRUB_LINUXBIOS_MEMORY_AVAILABLE	1
+#define GRUB_MACHINE_MEMORY_AVAILABLE		1
   grub_uint32_t type;
 };
 typedef struct grub_linuxbios_mem_region *mem_region_t;
 
-grub_err_t EXPORT_FUNC(grub_available_iterate)
-     (int (*hook) (mem_region_t));
+void EXPORT_FUNC(grub_mmap_iterate)
+     (int (*hook) (grub_uint64_t, grub_uint64_t, grub_uint32_t));
 
 #endif
 
Index: include/grub/multiboot.h
===================================================================
--- include/grub/multiboot.h	(revision 1802)
+++ include/grub/multiboot.h	(working copy)
@@ -1,7 +1,7 @@
 /* multiboot.h - multiboot header file with grub definitions. */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2003,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2003,2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -101,6 +101,16 @@
   grub_uint16_t vbe_interface_len;
 };
 
+struct grub_mmap_entry
+{
+  grub_uint32_t size;
+  grub_uint64_t addr;
+  grub_uint64_t len;
+#define GRUB_MMAP_MEMORY_AVAILABLE	1
+#define GRUB_MMAP_MEMORY_RESERVED	2
+  grub_uint32_t type;
+} __attribute__((packed));
+
 struct grub_mod_list
 {
   /* the memory used goes from bytes 'mod_start' to 'mod_end-1' inclusive */
Index: loader/i386/pc/multiboot.c
===================================================================
--- loader/i386/pc/multiboot.c	(revision 1802)
+++ loader/i386/pc/multiboot.c	(working copy)
@@ -78,14 +78,60 @@
       grub_free ((void *) mbi->cmdline);
       grub_free (mbi);
     }
-
-
+  
   mbi = 0;
   grub_dl_unref (my_mod);
 
   return GRUB_ERR_NONE;
 }
 
+/* FIXME: grub_uint32_t will break for addresses above 4 GiB, but is mandated
+   by the spec.  Is there something we can do about it?  */
+static grub_uint32_t mmap_addr = 0;
+static grub_uint32_t mmap_length;
+
+/* Return the length of the Multiboot mmap that will be needed to allocate
+   our platform's map.  */
+static grub_uint32_t
+grub_get_multiboot_mmap_len ()
+{
+  grub_size_t count = 0;
+
+  auto int hook (grub_uint64_t, grub_uint64_t, grub_uint32_t);
+  int hook (grub_uint64_t addr __attribute__ ((unused)),
+	    grub_uint64_t size __attribute__ ((unused)),
+	    grub_uint32_t type __attribute__ ((unused)))
+    {
+      count++;
+      return 0;
+    }
+  
+  grub_mmap_iterate (hook);
+  
+  return count * sizeof (struct grub_mmap_entry);
+}
+
+/* Fill previously allocated Multiboot mmap.  */
+static void
+grub_fill_multiboot_mmap (struct grub_mmap_entry *first_entry)
+{
+  struct grub_mmap_entry *mmap_entry = (struct grub_mmap_entry *) first_entry;
+
+  auto int hook (grub_uint64_t, grub_uint64_t, grub_uint32_t);
+  int hook (grub_uint64_t addr, grub_uint64_t size, grub_uint32_t type)
+    {
+      mmap_entry->addr = addr;
+      mmap_entry->len = size;
+      mmap_entry->type = type;
+      mmap_entry->size = sizeof (struct grub_mmap_entry) - sizeof (mmap_entry->size);
+      mmap_entry++;
+
+      return 0;
+    }
+
+  grub_mmap_iterate (hook);
+}
+
 /* Check if BUFFER contains ELF32.  */
 static int
 grub_multiboot_is_elf32 (void *buffer)
@@ -127,7 +173,7 @@
 	if (phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
 	  highest_segment = i;
       }
-  grub_multiboot_payload_size = (phdr(highest_segment)->p_paddr + phdr(highest_segment)->p_memsz) - phdr(lowest_segment)->p_paddr;
+  grub_multiboot_payload_size += (phdr(highest_segment)->p_paddr + phdr(highest_segment)->p_memsz) - phdr(lowest_segment)->p_paddr;
   grub_multiboot_payload_dest = phdr(lowest_segment)->p_paddr;
 
   playground = grub_malloc (RELOCATOR_SIZEOF(forward) + grub_multiboot_payload_size + RELOCATOR_SIZEOF(backward));
@@ -379,6 +425,9 @@
       playground = NULL;
     }
 
+  mmap_length = grub_get_multiboot_mmap_len ();
+  grub_multiboot_payload_size = mmap_length;
+
   if (header->flags & MULTIBOOT_AOUT_KLUDGE)
     {
       int offset = ((char *) header - buffer -
@@ -387,9 +436,9 @@
 		       header->load_end_addr - header->load_addr);
 
       if (header->bss_end_addr)
-	grub_multiboot_payload_size = (header->bss_end_addr - header->load_addr);
+	grub_multiboot_payload_size += (header->bss_end_addr - header->load_addr);
       else
-	grub_multiboot_payload_size = load_size;
+	grub_multiboot_payload_size += load_size;
       grub_multiboot_payload_dest = header->load_addr;
 
       playground = grub_malloc (RELOCATOR_SIZEOF(forward) + grub_multiboot_payload_size + RELOCATOR_SIZEOF(backward));
@@ -416,6 +465,12 @@
     goto fail;
 
       
+  grub_fill_multiboot_mmap ((struct grub_mmap_entry *) (grub_multiboot_payload_orig
+							+ grub_multiboot_payload_size
+							- mmap_length));
+
+  mmap_addr = grub_multiboot_payload_dest + grub_multiboot_payload_size - mmap_length;
+
   if (grub_multiboot_payload_dest >= grub_multiboot_payload_orig)
     {
       grub_memmove (playground, &grub_multiboot_forward_relocator, RELOCATOR_SIZEOF(forward));
@@ -439,12 +494,15 @@
 
   grub_memset (mbi, 0, sizeof (struct grub_multiboot_info));
 
-  mbi->flags = MULTIBOOT_INFO_MEMORY;
-
   /* Convert from bytes to kilobytes.  */
   mbi->mem_lower = grub_lower_mem / 1024;
   mbi->mem_upper = grub_upper_mem / 1024;
+  mbi->flags |= MULTIBOOT_INFO_MEMORY;
 
+  mbi->mmap_addr = mmap_addr;
+  mbi->mmap_length = mmap_length;
+  mbi->flags |= MULTIBOOT_INFO_MEM_MAP;
+
   for (i = 0, len = 0; i < argc; i++)
     len += grub_strlen (argv[i]) + 1;
 
Index: loader/i386/bsd.c
===================================================================
--- loader/i386/bsd.c	(revision 1802)
+++ loader/i386/bsd.c	(working copy)
@@ -313,7 +313,7 @@
 grub_openbsd_boot (void)
 {
   char *buf = (char *) GRUB_BSD_TEMP_BUFFER;
-  struct grub_machine_mmap_entry mmap;
+  struct grub_mmap_entry mmap;
   struct grub_openbsd_bios_mmap *pm;
   struct grub_openbsd_bootargs *pa;
   grub_uint32_t bootdev, biosdev, unit, slice, part, cont;