[RFC] Tagged mbi

[RFC] Tagged mbi

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1.1: Type: text/plain, Size: 694 bytes --]

Hello. Currently mbi contains a lot of pointers and substructures. It's
has various problems like:
-Unused fields occupy space if any subsequent fields is used.
-Difficult to relocate since it may be spilled all over the memory
-It's unstraightforward to e.g. specify 2 framebuffers
I propose to use tagged MBI. I attach proposed patch for multiboot and
grub2. Patch for grub2 is based on my efiboot branch
The patched branches are available under people/phcoder/mbtag-spec and
people/phcoder/taggedmbi.
The parts for which I haven't defined tagged equivalent yet:
-Symbols
-ROM information table
-Drives table
-APM table

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: mbtaggedimpl.diff --]
[-- Type: text/x-diff; name="mbtaggedimpl.diff", Size: 18956 bytes --]

=== modified file 'include/grub/i386/multiboot.h'
--- include/grub/i386/multiboot.h	2009-12-17 23:59:02 +0000
+++ include/grub/i386/multiboot.h	2010-01-02 02:04:52 +0000
@@ -39,4 +39,7 @@
 void grub_multiboot_set_bootdev (void);
 void grub_multiboot_set_accepts_video (int val);
 
+void grub_multiboot_set_tagged (int val);
+int grub_multiboot_get_tagged (void);
+
 #endif /* ! GRUB_MULTIBOOT_CPU_HEADER */

=== modified file 'include/multiboot.h'
--- include/multiboot.h	2010-01-02 17:50:06 +0000
+++ include/multiboot.h	2010-01-02 21:06:42 +0000
@@ -31,8 +31,11 @@
 /* This should be in %eax.  */
 #define MULTIBOOT_BOOTLOADER_MAGIC		0x2BADB002
 
+/* This should be in %eax.  */
+#define MULTIBOOT_BOOTLOADER_MAGIC_TAGGED	0x3BADB002
+
 /* The bits in the required part of flags field we don't support.  */
-#define MULTIBOOT_UNSUPPORTED                   0x0000fff8
+#define MULTIBOOT_UNSUPPORTED                   0x0000fff0
 
 /* Alignment of multiboot modules.  */
 #define MULTIBOOT_MOD_ALIGN			0x00001000
@@ -51,6 +54,9 @@
 /* Must pass video information to OS.  */
 #define MULTIBOOT_VIDEO_MODE			0x00000004
 
+/* Must pass tagged mbi to OS.  */
+#define MULTIBOOT_TAGGED_MBI			0x00000008
+
 /* This flag indicates the use of the address fields in the header.  */
 #define MULTIBOOT_AOUT_KLUDGE			0x00010000
 
@@ -254,6 +260,114 @@
 };
 typedef struct multiboot_mod_list multiboot_module_t;
 
+struct multiboot_tag
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+};
+
+struct multiboot_tag_string
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  char string[0];
+};
+
+struct multiboot_tag_module
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  multiboot_uint32_t mod_start;
+  multiboot_uint32_t mod_end;
+  char cmdline[0];
+};
+
+struct multiboot_tag_basic_meminfo
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  multiboot_uint32_t mem_lower;
+  multiboot_uint32_t mem_upper;
+};
+
+struct multiboot_tag_bootdev
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  multiboot_uint32_t biosdev;
+  multiboot_uint32_t slice;
+  multiboot_uint32_t part;
+};
+
+struct multiboot_tag_mmap
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  struct multiboot_mmap_entry entries[0];  
+};
+
+#include <grub/i386/pc/vbe.h>
+
+struct multiboot_tag_vbe
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+
+  multiboot_uint16_t vbe_mode;
+  multiboot_uint16_t vbe_interface_seg;
+  multiboot_uint16_t vbe_interface_off;
+  multiboot_uint16_t vbe_interface_len;
+
+  struct grub_vbe_info_block vbe_control_info;
+  struct grub_vbe_mode_info_block vbe_mode_info;
+};
+
+struct multiboot_tag_framebuffer_common
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+
+  multiboot_uint64_t framebuffer_addr;
+  multiboot_uint32_t framebuffer_pitch;
+  multiboot_uint32_t framebuffer_width;
+  multiboot_uint32_t framebuffer_height;
+  multiboot_uint8_t framebuffer_bpp;
+  multiboot_uint8_t framebuffer_type;
+};
+
+struct multiboot_tag_framebuffer
+{
+  struct multiboot_tag_framebuffer_common common;
+
+  union
+  {
+    struct
+    {
+      multiboot_uint16_t framebuffer_palette_num_colors;
+      struct multiboot_color framebuffer_palette[0];
+    };
+    struct
+    {
+      multiboot_uint8_t framebuffer_red_field_position;
+      multiboot_uint8_t framebuffer_red_mask_size;
+      multiboot_uint8_t framebuffer_green_field_position;
+      multiboot_uint8_t framebuffer_green_mask_size;
+      multiboot_uint8_t framebuffer_blue_field_position;
+      multiboot_uint8_t framebuffer_blue_mask_size;
+    };
+  };
+};
+
+#define MULTIBOOT_TAG_TYPE_END               0
+#define MULTIBOOT_TAG_TYPE_CMDLINE           1
+#define MULTIBOOT_TAG_TYPE_BOOT_LOADER_NAME  2
+#define MULTIBOOT_TAG_TYPE_MODULE            3
+#define MULTIBOOT_TAG_TYPE_BASIC_MEMINFO     4
+#define MULTIBOOT_TAG_TYPE_BOOTDEV           5
+#define MULTIBOOT_TAG_TYPE_MMAP              6
+#define MULTIBOOT_TAG_TYPE_VBE               7
+#define MULTIBOOT_TAG_TYPE_FRAMEBUFFER       8
+
 #endif /* ! ASM_FILE */
 
 #endif /* ! MULTIBOOT_HEADER */

=== modified file 'loader/i386/multiboot.c'
--- loader/i386/multiboot.c	2010-01-02 17:50:06 +0000
+++ loader/i386/multiboot.c	2010-01-02 21:57:30 +0000
@@ -62,7 +62,6 @@
   grub_err_t err;
   struct grub_relocator32_state state =
     {
-      .eax = MULTIBOOT_BOOTLOADER_MAGIC,
       .ecx = 0,
       .edx = 0,
       .eip = grub_multiboot_payload_eip,
@@ -71,6 +70,11 @@
       .esp = 0x7ff00
     };
 
+  if (grub_multiboot_get_tagged ())
+    state.eax = MULTIBOOT_BOOTLOADER_MAGIC_TAGGED;
+  else
+    state.eax = MULTIBOOT_BOOTLOADER_MAGIC;
+
   mbi_size = grub_multiboot_get_mbi_size ();
   if (alloc_mbi < mbi_size)
     {
@@ -197,6 +201,9 @@
   /* Skip filename.  */
   grub_multiboot_init_mbi (argc - 1, argv + 1);
 
+  if (header->flags & MULTIBOOT_TAGGED_MBI)
+    grub_multiboot_set_tagged (1);
+
   if (header->flags & MULTIBOOT_AOUT_KLUDGE)
     {
       int offset = ((char *) header - buffer -

=== modified file 'loader/i386/multiboot_mbi.c'
--- loader/i386/multiboot_mbi.c	2010-01-02 20:54:52 +0000
+++ loader/i386/multiboot_mbi.c	2010-01-02 21:01:58 +0000
@@ -55,9 +55,10 @@
 static grub_size_t total_modcmd;
 static unsigned modcnt;
 static char *cmdline = NULL;
-static grub_uint32_t bootdev;
 static int bootdev_set;
 static int accepts_video;
+static int tagged;
+static grub_uint32_t biosdev, slice, part;
 
 void
 grub_multiboot_set_accepts_video (int val)
@@ -65,6 +66,18 @@
   accepts_video = val;
 }
 
+void
+grub_multiboot_set_tagged (int val)
+{
+  tagged = val;
+}
+
+int
+grub_multiboot_get_tagged (void)
+{
+  return tagged;
+}
+
 /* Return the length of the Multiboot mmap that will be needed to allocate
    our platform's map.  */
 static grub_uint32_t
@@ -89,12 +102,23 @@
 grub_size_t
 grub_multiboot_get_mbi_size (void)
 {
-  return sizeof (struct multiboot_info) + ALIGN_UP (cmdline_size, 4)
-    + modcnt * sizeof (struct multiboot_mod_list) + total_modcmd
-    + ALIGN_UP (sizeof(PACKAGE_STRING), 4) + grub_get_multiboot_mmap_len ()
-    + sizeof (struct grub_vbe_info_block)
-    + sizeof (struct grub_vbe_mode_info_block)
-    + 256 * sizeof (struct multiboot_color);
+  if (tagged)
+    return sizeof (struct multiboot_tag)
+      + (sizeof (struct multiboot_tag_string) + ALIGN_UP (cmdline_size, 4))
+      + (sizeof (struct multiboot_tag_string)
+	 + ALIGN_UP (sizeof (PACKAGE_STRING), 4))
+      + (modcnt * sizeof (struct multiboot_tag_module) + total_modcmd)
+      + sizeof (struct multiboot_tag_basic_meminfo)
+      + sizeof (struct multiboot_tag_bootdev)
+      + (sizeof (struct multiboot_tag_mmap) + grub_get_multiboot_mmap_len ())
+      + sizeof (struct multiboot_tag_vbe);
+  else
+    return sizeof (struct multiboot_info) + ALIGN_UP (cmdline_size, 4)
+      + modcnt * sizeof (struct multiboot_mod_list) + total_modcmd
+      + ALIGN_UP (sizeof(PACKAGE_STRING), 4) + grub_get_multiboot_mmap_len ()
+      + sizeof (struct grub_vbe_info_block)
+      + sizeof (struct grub_vbe_mode_info_block)
+      + 256 * sizeof (struct multiboot_color);
 }
 
 /* Fill previously allocated Multiboot mmap.  */
@@ -158,40 +182,67 @@
 
 #if HAS_VBE
 static grub_err_t
-fill_vbe_info (struct multiboot_info *mbi, grub_uint8_t *ptrorig,
-	       grub_uint32_t ptrdest)
+fill_vbe_info_real (struct grub_vbe_info_block *vbe_control_info,
+		    struct grub_vbe_mode_info_block *vbe_mode_info,
+		    multiboot_uint16_t *vbe_mode,
+		    multiboot_uint16_t *vbe_interface_seg,
+		    multiboot_uint16_t *vbe_interface_off,
+		    multiboot_uint16_t *vbe_interface_len)
 {
   grub_vbe_status_t status;
-  grub_uint32_t vbe_mode;
   void *scratch = (void *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
     
   status = grub_vbe_bios_get_controller_info (scratch);
   if (status != GRUB_VBE_STATUS_OK)
     return grub_error (GRUB_ERR_IO, "Can't get controller info.");
-  
-  mbi->vbe_control_info = ptrdest;
-  grub_memcpy (ptrorig, scratch, sizeof (struct grub_vbe_info_block));
-  ptrorig += sizeof (struct grub_vbe_info_block);
-  ptrdest += sizeof (struct grub_vbe_info_block);
-  
+
+  grub_memcpy (vbe_control_info, scratch, sizeof (struct grub_vbe_info_block));
+
   status = grub_vbe_bios_get_mode (scratch);
-  vbe_mode = *(grub_uint32_t *) scratch;
+  *vbe_mode = *(grub_uint32_t *) scratch;
   if (status != GRUB_VBE_STATUS_OK)
     return grub_error (GRUB_ERR_IO, "Can't get VBE mode.");
-  mbi->vbe_mode = vbe_mode;
 
-  status = grub_vbe_bios_get_mode_info (vbe_mode, scratch);
+  status = grub_vbe_bios_get_mode_info (*vbe_mode, scratch);
   if (status != GRUB_VBE_STATUS_OK)
     return grub_error (GRUB_ERR_IO, "Can't get mode info.");
+  grub_memcpy (vbe_mode_info, scratch,
+	       sizeof (struct grub_vbe_mode_info_block));
+
+  /* FIXME: retrieve those.  */
+  *vbe_interface_seg = 0;
+  *vbe_interface_off = 0;
+  *vbe_interface_len = 0;
+
+  return GRUB_ERR_NONE;
+}
+
+static grub_err_t
+fill_vbe_info (struct multiboot_info *mbi, grub_uint8_t *ptrorig,
+	       grub_uint32_t ptrdest)
+{
+  struct grub_vbe_info_block *vbe_control_info;
+  struct grub_vbe_mode_info_block *vbe_mode_info;
+  grub_err_t err;
+
+  vbe_control_info = (struct grub_vbe_info_block *) ptrorig;
+  mbi->vbe_control_info = ptrdest;
+  ptrorig += sizeof (struct grub_vbe_info_block);
+  ptrdest += sizeof (struct grub_vbe_info_block);
+
+  vbe_mode_info = (struct grub_vbe_mode_info_block *) ptrorig;  
   mbi->vbe_mode_info = ptrdest;
-  grub_memcpy (ptrorig, scratch, sizeof (struct grub_vbe_mode_info_block));
   ptrorig += sizeof (struct grub_vbe_mode_info_block);
   ptrdest += sizeof (struct grub_vbe_mode_info_block);
-      
-  /* FIXME: retrieve those.  */
-  mbi->vbe_interface_seg = 0;
-  mbi->vbe_interface_off = 0;
-  mbi->vbe_interface_len = 0;
+
+  err = fill_vbe_info_real (vbe_control_info,
+			    vbe_mode_info,
+			    &(mbi->vbe_mode),
+			    &(mbi->vbe_interface_seg),
+			    &(mbi->vbe_interface_off),
+			    &(mbi->vbe_interface_len));
+  if (err)
+    return err;
   
   mbi->flags |= MULTIBOOT_INFO_VBE_INFO;
 
@@ -276,16 +327,221 @@
   mbi->flags |= MULTIBOOT_INFO_FRAMEBUFFER_INFO;
 
 #if HAS_VBE
+  fill_vbe_info (mbi, ptrorig, ptrdest);
+#endif
+
+  return GRUB_ERR_NONE;
+}
+
+static grub_err_t
+set_video_info_tagged (struct multiboot_tag_framebuffer *tag,
+#if HAS_VBE
+		       int *vbe_active
+#endif
+		       )
+{
+  struct grub_video_palette_data palette[256];
+  grub_err_t err;
+  struct grub_video_mode_info mode_info;
+  void *framebuffer;
+
+  tag->common.type = MULTIBOOT_TAG_TYPE_FRAMEBUFFER;
+  tag->common.size = 0;
+    
+  err = set_video_mode ();
+  if (err)
+    return err;
+
+  grub_video_get_palette (0, ARRAY_SIZE (palette), palette);
+    
+#if HAS_VBE
+  {
+    const char *adapter_name;
+    adapter_name = grub_video_get_active_adapter_name ();
+    
+    *vbe_active = ((adapter_name
+		    && grub_strcmp (adapter_name,
+				    "VESA BIOS Extension Video Driver") == 0)
+		   || (HAS_VGA_TEXT && !adapter_name));
+  }
+#endif
+
+  err = grub_video_get_info_and_fini (&mode_info, &framebuffer);
+  if (err)
+    return err;
+  
+  tag->common.framebuffer_addr = (grub_addr_t) framebuffer;
+  tag->common.framebuffer_pitch = mode_info.pitch;
+
+  tag->common.framebuffer_width = mode_info.width;
+  tag->common.framebuffer_height = mode_info.height;
+  
+  tag->common.framebuffer_bpp = mode_info.bpp;
+      
+  if (mode_info.mode_type & GRUB_VIDEO_MODE_TYPE_INDEX_COLOR)
+    {
+      unsigned i;
+      tag->common.framebuffer_type = MULTIBOOT_FRAMEBUFFER_TYPE_INDEXED;
+      tag->framebuffer_palette_num_colors = mode_info.number_of_colors;
+      if (tag->framebuffer_palette_num_colors > ARRAY_SIZE (palette))
+	tag->framebuffer_palette_num_colors = ARRAY_SIZE (palette);
+      tag->common.size = sizeof (struct multiboot_tag_framebuffer_common)
+	+ sizeof (multiboot_uint16_t) + tag->framebuffer_palette_num_colors
+	* sizeof (struct multiboot_color);
+      for (i = 0; i < tag->framebuffer_palette_num_colors; i++)
+	{
+	  tag->framebuffer_palette[i].red = palette[i].r;
+	  tag->framebuffer_palette[i].green = palette[i].g;
+	  tag->framebuffer_palette[i].blue = palette[i].b;
+	}
+    }
+  else
+    {
+      tag->common.framebuffer_type = MULTIBOOT_FRAMEBUFFER_TYPE_RGB;
+      tag->framebuffer_red_field_position = mode_info.green_field_pos;
+      tag->framebuffer_red_mask_size = mode_info.green_mask_size;
+      tag->framebuffer_green_field_position = mode_info.green_field_pos;
+      tag->framebuffer_green_mask_size = mode_info.green_mask_size;
+      tag->framebuffer_blue_field_position = mode_info.blue_field_pos;
+      tag->framebuffer_blue_mask_size = mode_info.blue_mask_size;
+
+      tag->common.size = sizeof (struct multiboot_tag_framebuffer_common) + 6;
+    }
+
+  return GRUB_ERR_NONE;
+}
+
+static grub_err_t
+make_mbi_tagged (void *orig, grub_uint32_t dest, grub_off_t buf_off)
+{
+  grub_uint8_t *ptrorig = (grub_uint8_t *) orig + buf_off;
+#if HAS_VBE
+  int vbe_active = 0;
+#endif
+  grub_err_t err;
+
+  {
+    struct multiboot_tag_string *tag = (struct multiboot_tag_string *) ptrorig;
+    tag->type = MULTIBOOT_TAG_TYPE_CMDLINE;
+    tag->size = sizeof (struct multiboot_tag_string)
+      + ALIGN_UP (cmdline_size, 4); 
+    grub_memcpy (tag->string, cmdline, cmdline_size);
+    ptrorig += tag->size;
+  }
+
+  {
+    struct multiboot_tag_string *tag = (struct multiboot_tag_string *) ptrorig;
+    tag->type = MULTIBOOT_TAG_TYPE_BOOT_LOADER_NAME;
+    tag->size = sizeof (struct multiboot_tag_string)
+      + ALIGN_UP (sizeof (PACKAGE_STRING), 4); 
+    grub_memcpy (tag->string, PACKAGE_STRING, sizeof (PACKAGE_STRING));
+    ptrorig += tag->size;
+  }
+
+  {
+    unsigned i;
+    struct module *cur;
+
+    for (i = 0, cur = modules; i < modcnt; i++, cur = cur->next)
+      {
+	struct multiboot_tag_module *tag
+	  = (struct multiboot_tag_module *) ptrorig;
+	tag->type = MULTIBOOT_TAG_TYPE_MODULE;
+	tag->size = sizeof (struct multiboot_tag_module)
+	  + ALIGN_UP (sizeof (cur->cmdline_size), 4); 
+
+	tag->mod_start = dest + cur->start;
+	tag->mod_end = tag->mod_start + cur->size;
+	grub_memcpy (tag->cmdline, cur->cmdline, cur->cmdline_size);
+	ptrorig += tag->size;
+      }
+  }
+
+  {
+    struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *) ptrorig;
+    tag->type = MULTIBOOT_TAG_TYPE_MMAP;
+    tag->size = sizeof (struct multiboot_tag_mmap) 
+      + grub_get_multiboot_mmap_len (); 
+    grub_fill_multiboot_mmap (tag->entries);
+    ptrorig += tag->size;
+  }
+
+  {
+    struct multiboot_tag_basic_meminfo *tag
+      = (struct multiboot_tag_basic_meminfo *) ptrorig;
+    tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO;
+    tag->size = sizeof (struct multiboot_tag_basic_meminfo); 
+
+    /* Convert from bytes to kilobytes.  */
+    tag->mem_lower = grub_mmap_get_lower () / 1024;
+    tag->mem_upper = grub_mmap_get_upper () / 1024;
+    ptrorig += tag->size;
+  }
+
+  if (bootdev_set)
+    {
+      struct multiboot_tag_bootdev *tag
+	= (struct multiboot_tag_bootdev *) ptrorig;
+      tag->type = MULTIBOOT_TAG_TYPE_BOOTDEV;
+      tag->size = sizeof (struct multiboot_tag_bootdev); 
+
+      tag->biosdev = biosdev;
+      tag->slice = slice;
+      tag->part = part;
+      ptrorig += tag->size;
+    }
+
+  {
+    struct multiboot_tag_framebuffer *tag
+      = (struct multiboot_tag_framebuffer *) ptrorig;
+    err = set_video_info_tagged (tag,
+#if HAS_VBE
+				 &vbe_active
+#endif
+				 );
+    if (err)
+      {
+	grub_print_error ();
+	grub_errno = GRUB_ERR_NONE;
+      }
+    else
+      ptrorig += tag->common.size;
+  }
+
+#if HAS_VBE
   if (vbe_active)
-    fill_vbe_info (mbi, ptrorig, ptrdest);
+    {
+      struct multiboot_tag_vbe *tag = (struct multiboot_tag_vbe *) ptrorig;
+      tag->type = MULTIBOOT_TAG_TYPE_VBE;
+      tag->size = sizeof (struct multiboot_tag_vbe);
+      err = fill_vbe_info_real (&(tag->vbe_control_info),
+				&(tag->vbe_mode_info),
+				&(tag->vbe_mode),
+				&(tag->vbe_interface_seg),
+				&(tag->vbe_interface_off),
+				&(tag->vbe_interface_len));
+      if (err)
+	{
+	  grub_print_error ();
+	  grub_errno = GRUB_ERR_NONE;
+	}
+      else
+	ptrorig += tag->size;
+    }
 #endif
+  
+  {
+    struct multiboot_tag *tag = (struct multiboot_tag *) ptrorig;
+    tag->type = MULTIBOOT_TAG_TYPE_END;
+    tag->size = sizeof (struct multiboot_tag);
+    ptrorig += tag->size;
+  }
 
   return GRUB_ERR_NONE;
 }
 
-grub_err_t
-grub_multiboot_make_mbi (void *orig, grub_uint32_t dest, grub_off_t buf_off,
-			 grub_size_t bufsize)
+static grub_err_t
+make_mbi (void *orig, grub_uint32_t dest, grub_off_t buf_off)
 {
   grub_uint8_t *ptrorig = (grub_uint8_t *) orig + buf_off;
   grub_uint32_t ptrdest = dest + buf_off;
@@ -296,9 +552,6 @@
   grub_size_t mmap_size;
   grub_err_t err;
 
-  if (bufsize < grub_multiboot_get_mbi_size ())
-    return grub_error (GRUB_ERR_OUT_OF_MEMORY, "mbi buffer is too small");
-
   mbi = (struct multiboot_info *) ptrorig;
   ptrorig += sizeof (*mbi);
   ptrdest += sizeof (*mbi);
@@ -356,7 +609,8 @@
 
   if (bootdev_set)
     {
-      mbi->boot_device = bootdev;
+      mbi->boot_device = (((biosdev & 0xff) << 24) | ((slice & 0xff) << 16) 
+			  | ((part & 0xff) << 8) | 0xff);
       mbi->flags |= MULTIBOOT_INFO_BOOTDEV;
     }
 
@@ -374,6 +628,19 @@
   return GRUB_ERR_NONE;
 }
 
+grub_err_t
+grub_multiboot_make_mbi (void *orig, grub_uint32_t dest, grub_off_t buf_off,
+			 grub_size_t bufsize)
+{
+  if (bufsize < grub_multiboot_get_mbi_size ())
+    return grub_error (GRUB_ERR_OUT_OF_MEMORY, "mbi buffer is too small");
+
+  if (tagged)
+    return make_mbi_tagged (orig, dest, buf_off);
+  else
+    return make_mbi (orig, dest, buf_off);
+}
+
 void
 grub_multiboot_free_mbi (void)
 {
@@ -403,6 +670,7 @@
   char *p;
   int i;
 
+  tagged = 0;
   grub_multiboot_free_mbi ();
 
   for (i = 0; i < argc; i++)
@@ -486,9 +754,11 @@
 grub_multiboot_set_bootdev (void)
 {
   char *p;
-  grub_uint32_t biosdev, slice = ~0, part = ~0;
   grub_device_t dev;
 
+  slice = ~0;
+  part = ~0;
+
 #ifdef GRUB_MACHINE_PCBIOS
   biosdev = grub_get_root_biosnumber ();
 #else
@@ -517,7 +787,5 @@
   if (dev)
     grub_device_close (dev);
 
-  bootdev = ((biosdev & 0xff) << 24) | ((slice & 0xff) << 16) 
-    | ((part & 0xff) << 8) | 0xff;
   bootdev_set = 1;
 }


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: mbtagged.diff --]
[-- Type: text/x-diff; name="mbtagged.diff", Size: 20898 bytes --]

=== modified file 'doc/boot.S'
--- doc/boot.S	2010-01-02 17:52:11 +0000
+++ doc/boot.S	2010-01-04 19:44:53 +0000
@@ -30,9 +30,9 @@
 
 /* The flags for the Multiboot header.  */
 #ifdef __ELF__
-# define MULTIBOOT_HEADER_FLAGS		0x00000007
+# define MULTIBOOT_HEADER_FLAGS		0x0000000f
 #else
-# define MULTIBOOT_HEADER_FLAGS		0x00010007
+# define MULTIBOOT_HEADER_FLAGS		0x0001000f
 #endif
 	
 	.text

=== modified file 'doc/kernel.c'
--- doc/kernel.c	2010-01-02 17:52:11 +0000
+++ doc/kernel.c	2010-01-04 19:43:33 +0000
@@ -47,6 +47,153 @@
 static void putchar (int c);
 void printf (const char *format, ...);
 
+static void
+tagged (unsigned long addr)
+{
+  struct multiboot_tag *tag;
+
+  for (tag = (struct multiboot_tag *) addr; tag->type != MULTIBOOT_TAG_TYPE_END;
+       tag = (struct multiboot_tag *) ((multiboot_uint8_t *) tag + tag->size))
+    {
+      printf ("Tag 0x%x, Size 0x%x\n", tag->type, tag->size);
+      switch (tag->type)
+	{
+	case MULTIBOOT_TAG_TYPE_CMDLINE:
+	  printf ("Command line = %s\n",
+		  ((struct multiboot_tag_string *) tag)->string);
+	  break;
+	case MULTIBOOT_TAG_TYPE_BOOT_LOADER_NAME:
+	  printf ("Boot loader name = %s\n",
+		  ((struct multiboot_tag_string *) tag)->string);
+	  break;
+	case MULTIBOOT_TAG_TYPE_MODULE:
+	  printf ("Module at 0x%x-0x%x. Command line %s\n",
+		  ((struct multiboot_tag_module *) tag)->mod_start,
+		  ((struct multiboot_tag_module *) tag)->mod_end,
+		  ((struct multiboot_tag_module *) tag)->cmdline);
+	  break;
+	case MULTIBOOT_TAG_TYPE_BASIC_MEMINFO:
+	  printf ("mem_lower = %uKB, mem_upper = %uKB\n",
+		  ((struct multiboot_tag_basic_meminfo *) tag)->mem_lower,
+		  ((struct multiboot_tag_basic_meminfo *) tag)->mem_upper);
+	  break;
+	case MULTIBOOT_TAG_TYPE_BOOTDEV:
+	  printf ("Boot device 0x%x,%u,%u\n",
+		  ((struct multiboot_tag_bootdev *) tag)->biosdev,
+		  ((struct multiboot_tag_bootdev *) tag)->slice,
+		  ((struct multiboot_tag_bootdev *) tag)->part);
+	  break;
+	case MULTIBOOT_TAG_TYPE_MMAP:
+	  {
+	    multiboot_memory_map_t *mmap;
+
+	    printf ("mmap\n");
+      
+	    for (mmap = ((struct multiboot_tag_mmap *) tag)->entries;
+		 (multiboot_uint8_t *) mmap 
+		   < (multiboot_uint8_t *) tag + tag->size;
+		 mmap = (multiboot_memory_map_t *) ((unsigned long) mmap
+						    + mmap->size
+						    + sizeof (mmap->size)))
+	      printf (" size = 0x%x, base_addr = 0x%x%x,"
+		      " length = 0x%x%x, type = 0x%x\n",
+		      (unsigned) mmap->size,
+		      (unsigned) (mmap->addr >> 32),
+		      (unsigned) (mmap->addr & 0xffffffff),
+		      (unsigned) (mmap->len >> 32),
+		      (unsigned) (mmap->len & 0xffffffff),
+		      (unsigned) mmap->type);
+	  }
+	  break;
+	case MULTIBOOT_TAG_TYPE_FRAMEBUFFER:
+	  {
+	    multiboot_uint32_t color;
+	    unsigned i;
+	    struct multiboot_tag_framebuffer *tagfb
+	      = (struct multiboot_tag_framebuffer *) tag;
+	    void *fb = (void *) (unsigned long) tagfb->common.framebuffer_addr;
+
+	    switch (tagfb->common.framebuffer_type)
+	      {
+	      case MULTIBOOT_FRAMEBUFFER_TYPE_INDEXED:
+		{
+		  unsigned best_distance, distance;
+		  struct multiboot_color *palette;
+	    
+		  palette = tagfb->framebuffer_palette;
+
+		  color = 0;
+		  best_distance = 4*256*256;
+	    
+		  for (i = 0; i < tagfb->framebuffer_palette_num_colors; i++)
+		    {
+		      distance = (0xff - palette[i].blue) 
+			* (0xff - palette[i].blue)
+			+ palette[i].red * palette[i].red
+			+ palette[i].green * palette[i].green;
+		      if (distance < best_distance)
+			{
+			  color = i;
+			  best_distance = distance;
+			}
+		    }
+		}
+		break;
+
+	      case MULTIBOOT_FRAMEBUFFER_TYPE_RGB:
+		color = ((1 << tagfb->framebuffer_blue_mask_size) - 1) 
+		  << tagfb->framebuffer_blue_field_position;
+		break;
+
+	      default:
+		color = 0xffffffff;
+		break;
+	      }
+	    
+	    for (i = 0; i < tagfb->common.framebuffer_width
+		   && i < tagfb->common.framebuffer_height; i++)
+	      {
+		switch (tagfb->common.framebuffer_bpp)
+		  {
+		  case 8:
+		    {
+		      multiboot_uint8_t *pixel = fb
+			+ tagfb->common.framebuffer_pitch * i + i;
+		      *pixel = color;
+		    }
+		    break;
+		  case 15:
+		  case 16:
+		    {
+		      multiboot_uint16_t *pixel
+			= fb + tagfb->common.framebuffer_pitch * i + 2 * i;
+		      *pixel = color;
+		    }
+		    break;
+		  case 24:
+		    {
+		      multiboot_uint32_t *pixel
+			= fb + tagfb->common.framebuffer_pitch * i + 3 * i;
+		      *pixel = (color & 0xffffff) | (*pixel & 0xff000000);
+		    }
+		    break;
+
+		  case 32:
+		    {
+		      multiboot_uint32_t *pixel
+			= fb + tagfb->common.framebuffer_pitch * i + 4 * i;
+		      *pixel = color;
+		    }
+		    break;
+		  }
+	      }
+	    break;
+	  }
+	}
+    }
+}
+
+
 /* Check if MAGIC is valid and print the Multiboot information structure
    pointed by ADDR.  */
 void
@@ -57,6 +204,12 @@
   /* Clear the screen.  */
   cls ();
 
+  if (magic == MULTIBOOT_BOOTLOADER_MAGIC_TAGGED)
+    {
+      tagged (addr);
+      return;
+    }
+
   /* Am I booted by a Multiboot-compliant boot loader?  */
   if (magic != MULTIBOOT_BOOTLOADER_MAGIC)
     {

=== modified file 'doc/multiboot.h'
--- doc/multiboot.h	2010-01-02 17:52:11 +0000
+++ doc/multiboot.h	2010-01-04 20:35:55 +0000
@@ -31,8 +31,11 @@
 /* This should be in %eax.  */
 #define MULTIBOOT_BOOTLOADER_MAGIC		0x2BADB002
 
+/* This should be in %eax.  */
+#define MULTIBOOT_BOOTLOADER_MAGIC_TAGGED	0x3BADB002
+
 /* The bits in the required part of flags field we don't support.  */
-#define MULTIBOOT_UNSUPPORTED                   0x0000fff8
+#define MULTIBOOT_UNSUPPORTED                   0x0000fff0
 
 /* Alignment of multiboot modules.  */
 #define MULTIBOOT_MOD_ALIGN			0x00001000
@@ -51,6 +54,9 @@
 /* Must pass video information to OS.  */
 #define MULTIBOOT_VIDEO_MODE			0x00000004
 
+/* Must pass tagged mbi to OS.  */
+#define MULTIBOOT_TAGGED_MBI			0x00000008
+
 /* This flag indicates the use of the address fields in the header.  */
 #define MULTIBOOT_AOUT_KLUDGE			0x00010000
 
@@ -254,6 +260,122 @@
 };
 typedef struct multiboot_mod_list multiboot_module_t;
 
+struct multiboot_tag
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+};
+
+struct multiboot_tag_string
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  char string[0];
+};
+
+struct multiboot_tag_module
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  multiboot_uint32_t mod_start;
+  multiboot_uint32_t mod_end;
+  char cmdline[0];
+};
+
+struct multiboot_tag_basic_meminfo
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  multiboot_uint32_t mem_lower;
+  multiboot_uint32_t mem_upper;
+};
+
+struct multiboot_tag_bootdev
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  multiboot_uint32_t biosdev;
+  multiboot_uint32_t slice;
+  multiboot_uint32_t part;
+};
+
+struct multiboot_tag_mmap
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+  struct multiboot_mmap_entry entries[0];  
+};
+
+struct multiboot_vbe_info_block
+{
+  multiboot_uint8_t external_specification[512];
+};
+
+struct multiboot_vbe_mode_info_block
+{
+  multiboot_uint8_t external_specification[256];
+};
+
+struct multiboot_tag_vbe
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+
+  multiboot_uint16_t vbe_mode;
+  multiboot_uint16_t vbe_interface_seg;
+  multiboot_uint16_t vbe_interface_off;
+  multiboot_uint16_t vbe_interface_len;
+
+  struct multiboot_vbe_info_block vbe_control_info;
+  struct multiboot_vbe_mode_info_block vbe_mode_info;
+};
+
+struct multiboot_tag_framebuffer_common
+{
+  multiboot_uint32_t type;
+  multiboot_uint32_t size;
+
+  multiboot_uint64_t framebuffer_addr;
+  multiboot_uint32_t framebuffer_pitch;
+  multiboot_uint32_t framebuffer_width;
+  multiboot_uint32_t framebuffer_height;
+  multiboot_uint8_t framebuffer_bpp;
+  multiboot_uint8_t framebuffer_type;
+};
+
+struct multiboot_tag_framebuffer
+{
+  struct multiboot_tag_framebuffer_common common;
+
+  union
+  {
+    struct
+    {
+      multiboot_uint16_t framebuffer_palette_num_colors;
+      struct multiboot_color framebuffer_palette[0];
+    };
+    struct
+    {
+      multiboot_uint8_t framebuffer_red_field_position;
+      multiboot_uint8_t framebuffer_red_mask_size;
+      multiboot_uint8_t framebuffer_green_field_position;
+      multiboot_uint8_t framebuffer_green_mask_size;
+      multiboot_uint8_t framebuffer_blue_field_position;
+      multiboot_uint8_t framebuffer_blue_mask_size;
+    };
+  };
+};
+
+#define MULTIBOOT_TAG_TYPE_END               0
+#define MULTIBOOT_TAG_TYPE_CMDLINE           1
+#define MULTIBOOT_TAG_TYPE_BOOT_LOADER_NAME  2
+#define MULTIBOOT_TAG_TYPE_MODULE            3
+#define MULTIBOOT_TAG_TYPE_BASIC_MEMINFO     4
+#define MULTIBOOT_TAG_TYPE_BOOTDEV           5
+#define MULTIBOOT_TAG_TYPE_MMAP              6
+#define MULTIBOOT_TAG_TYPE_VBE               7
+#define MULTIBOOT_TAG_TYPE_FRAMEBUFFER       8
+
 #endif /* ! ASM_FILE */
 
 #endif /* ! MULTIBOOT_HEADER */

=== modified file 'doc/multiboot.texi'
--- doc/multiboot.texi	2010-01-04 19:44:39 +0000
+++ doc/multiboot.texi	2010-01-04 21:05:10 +0000
@@ -414,6 +414,9 @@
 mode table (@pxref{Boot information format}) must be available to the
 kernel.
 
+If bit 3 in the @samp{flags} word is set, the multiboot information 
+must be in tagged format.
+
 If bit 16 in the @samp{flags} word is set, then the fields at offsets
 12-28 in the Multiboot header are valid, and the boot loader should use
 them instead of the fields in the actual executable header to calculate
@@ -515,7 +518,7 @@
 
 @table @samp
 @item EAX
-Must contain the magic value @samp{0x2BADB002}; the presence of this
+Must contain the magic value @samp{0x2BADB002} if Boot info is non-tagged and @samp{0x3BADB002} otherwise; the presence of this
 value indicates to the operating system that it was loaded by a
 Multiboot-compliant boot loader (e.g. as opposed to another type of
 boot loader that the operating system can also be loaded from).
@@ -578,9 +581,8 @@
 
 
 @node Boot information format
-@section Boot information format
-
-FIXME: Split this chapter like the chapter ``OS image format''.
+@section Boot information
+@subsection Boot information format
 
 Upon entry to the operating system, the @code{EBX} register contains the
 physical address of a @dfn{Multiboot information} data structure,
@@ -595,6 +597,25 @@
 operating system's responsibility to avoid overwriting this memory until
 it is done using it.
 
+@subsection Basic tags structure
+Boot information can be passed in one of two formats: pointer-based
+or tag-based. Tag-based format must be used if and only it was requested
+in multiboot header. Every tag begins with following fields:
+
+@example
+@group
+        +-------------------+
+0       | type              |
+4-7     | size              |
+        +-------------------+
+@end group
+@end example
+
+@samp{type} contains an identifier of contents of the rest of the tag.
+@samp{size} contains the size of tag including header fields.
+Tags follow one another without any gaps. Tags are terminated by a tag of type @samp{0} and size @samp{8}.
+
+@subsection Basic non-tagged structure
 The format of the Multiboot information structure (as defined so far)
 follows:
 
@@ -655,6 +676,7 @@
 information structure to be expanded in the future without breaking
 anything.
 
+@subsection Basic memory information
 If bit 0 in the @samp{flags} word is set, then the @samp{mem_*} fields
 are valid. @samp{mem_lower} and @samp{mem_upper} indicate the amount of
 lower and upper memory, respectively, in kilobytes. Lower memory starts
@@ -663,6 +685,20 @@
 upper memory is maximally the address of the first upper memory hole
 minus 1 megabyte. It is not guaranteed to be this value.
 
+Corresponding tag is:
+
+@example
+@group
+        +-------------------+
+0       | type = 4          |
+4       | size = 16         |
+8       | mem_lower         |
+12-15   | mem_upper         |
+        +-------------------+
+@end group
+@end example
+
+@subsection BIOS Boot device
 If bit 1 in the @samp{flags} word is set, then the @samp{boot_device}
 field is valid, and indicates which @sc{bios} disk device the boot
 loader loaded the OS image from. If the OS image was not loaded from a
@@ -705,11 +741,39 @@
 @samp{part1} will be 5, and @samp{part2} and @samp{part3} will both be
 0xFF.
 
+Corresponding tag is:
+@example
+@group
+        +-------------------+
+0       | type = 5          |
+4       | size = 20         |
+8       | biosdev           |
+12      | partition         |
+16-19   | sub-parition      |
+        +-------------------+
+@end group
+@end example
+
+@subsection Boot command line
 If bit 2 of the @samp{flags} longword is set, the @samp{cmdline} field
 is valid, and contains the physical address of the command line to
 be passed to the kernel. The command line is a normal C-style
-zero-terminated string.
-
+UTF-8 zero-terminated string.
+
+Corresponding tag is:
+@example
+@group
+        +-------------------+
+0       | type = 1          |
+4       | size              |
+8-xx    | string            |
+        +-------------------+
+@end group
+@end example
+
+@samp{string} contains zero-terminated UTF-8 string padded to have length divisible by 4.
+
+@subsection Modules
 If bit 3 of the @samp{flags} is set, then the @samp{mods} fields
 indicate to the kernel what boot modules were loaded along with the
 kernel image, and where they can be found. @samp{mods_count} contains
@@ -734,7 +798,7 @@
 The first two fields contain the start and end addresses of the boot
 module itself. The @samp{string} field provides an arbitrary string to
 be associated with that particular boot module; it is a zero-terminated
-ASCII string, just like the kernel command line. The @samp{string} field
+UTF-8 string, just like the kernel command line. The @samp{string} field
 may be 0 if there is no string associated with the module. Typically the
 string might be a command line (e.g. if the operating system treats boot
 modules as executable programs), or a pathname (e.g. if the operating
@@ -742,6 +806,25 @@
 is specific to the operating system. The @samp{reserved} field must be
 set to 0 by the boot loader and ignored by the operating system.
 
+Corresponding tag is:
+
+@example
+@group
+        +-------------------+
+0       | type = 3          |
+4       | size              |
+8       | mod_start         |
+12      | mod_end           |
+16-xx   | string            |   
+        +-------------------+
+@end group
+@end example
+
+Tag is padded in the way to have size divisible by 4.
+
+One tag appears per module.
+
+@subsection Symbols
 @strong{Caution:} Bits 4 & 5 are mutually exclusive.
 
 If bit 4 in the @samp{flags} word is set, then the following fields in
@@ -795,6 +878,7 @@
 @samp{shdr_num} may be 0, indicating no symbols, even if bit 5 in the
 @samp{flags} word is set.
 
+@subsection Memory map
 If bit 6 in the @samp{flags} word is set, then the @samp{mmap_*} fields
 are valid, and indicate the address and length of a buffer containing a
 memory map of the machine provided by the @sc{bios}. @samp{mmap_addr} is
@@ -824,6 +908,18 @@
 The map provided is guaranteed to list all standard @sc{ram} that should
 be available for normal use.
 
+The corresponding tag is
+@example
+@group
+        +-------------------+
+0       | type = 6          |
+4       | size              |
+8-xx    | entries           |
+        +-------------------+
+@end group
+@end example
+
+@subsection Drives table
 If bit 7 in the @samp{flags} is set, then the @samp{drives_*} fields
 are valid, and indicate the address of the physical address of the first
 drive structure and the size of drive structures. @samp{drives_addr}
@@ -878,16 +974,32 @@
 array may contain any number of I/O ports that are not related to the
 drive actually (such as @sc{dma} controller's ports).
 
+@subsection Configuration table
 If bit 8 in the @samp{flags} is set, then the @samp{config_table} field
 is valid, and indicates the address of the @sc{rom} configuration table
 returned by the @dfn{GET CONFIGURATION} @sc{bios} call. If the @sc{bios}
 call fails, then the size of the table must be @emph{zero}.
 
+@subsection Boot loader name
 If bit 9 in the @samp{flags} is set, the @samp{boot_loader_name} field
 is valid, and contains the physical address of the name of a boot
 loader booting the kernel. The name is a normal C-style zero-terminated
 string.
 
+Corresponding tag is:
+@example
+@group
+        +-------------------+
+0       | type = 2          |
+4       | size              |
+8-xx    | string            |
+        +-------------------+
+@end group
+@end example
+
+@samp{string} contains zero-terminated UTF-8 string padded to have length divisible by 4.
+
+@subsection APM table
 If bit 10 in the @samp{flags} is set, the @samp{apm_table} field is
 valid, and contains the physical address of an @sc{apm} table defined as
 below:
@@ -920,6 +1032,7 @@
 @uref{http://www.microsoft.com/hwdev/busbios/amp_12.htm, Advanced Power
 Management (APM) BIOS Interface Specification}, for more information.
 
+@subsection VBE info
 If bit 11 in the @samp{flags} is set, the @sc{vbe} table is available.
 
 The fields @samp{vbe_control_info} and @samp{vbe_mode_info} contain
@@ -942,6 +1055,25 @@
 Multiboot boot loaders may simulate @sc{vbe} on non-@sc{vbe} modes, as
 if they were @sc{vbe} modes.
 
+Corresponding tag is:
+@example
+@group
+        +-------------------+
+0       | type = 7          |
+4       | size = 784        |
+8       | vbe_mode          |
+10      | vbe_interface_seg |
+12      | vbe_interface_off |
+14      | vbe_interface_len |
+16      | vbe_control_info  |
+528-783 | vbe_mode_info     |
+        +-------------------+
+@end group
+@end example
+
+Notice that the tag contains VBE control and mode information structures directly rather than a pointer to them
+
+@subsection Framebuffer info
 If bit 12 in the @samp{flags} is set, the @sc{Framebuffer} table is available.
 
 The field @samp{framebuffer_addr} contains framebuffer physical address. This field is 64-bit wide but bootloader @dfn{should} set it under 4GiB if possible for compatibility with payloads which aren't aware of PAE or amd64. The field @samp{framebuffer_pitch} contains pitch in bytes. The fields @samp{framebuffer_width}, @samp{framebuffer_height} contain framebuffer dimensions in pixels. The field @samp{framebuffer_bpp} contains number of bits per pixel. If @samp{framebuffer_type} is set to 0 it means indexed color. In this case color_info is defined as follows:
@@ -959,7 +1091,7 @@
         +-------------+
 0       | red_value   |
 1       | green_value |
-2       | blue_value  |
+2-2     | blue_value  |
         +-------------+
 @end group
 @end example
@@ -981,6 +1113,40 @@
 If @samp{framebuffer_type} is set to 2 it means EGA text. In this case @samp{framebuffer_width} and @samp{framebuffer_height} are expressed in characters and not in pixels. @samp{framebuffer_bpp} is equal 16 (16 bits per character) and @samp{framebuffer_pitch} is expressed in bytes per text line.
 All further values of @samp{framebuffer_type} are reserved for future expansion
 
+Corresponding tag is:
+@example
+@group
+        +--------------------+
+0       | type = 8           |
+4       | size               |
+8       | framebuffer_addr   |
+16      | framebuffer_pitch  |
+20      | framebuffer_width  |
+24      | framebuffer_height |
+28      | framebuffer_bpp    |
+29      | framebuffer_type   |
+30      | color_info         |
+        +--------------------+
+@end group
+@end example
+
+If @samp{framebuffer_type} is @samp{0} color_info has the following format:
+
+@example
+@group
+        +---------------------------------+
+0       | framebuffer_palette_num_colors  |
+4-xx    | framebuffer_palette             |
+        +---------------------------------+
+@end group
+@end example
+
+With @samp{framebuffer_palette_num_colors} and @samp{framebuffer_palette} having the same format as in non-tagged version
+
+Note: @samp{framebuffer_palette} contains the palette and not its address.
+
+If @samp{framebuffer_type} is @samp{1} or @samp{2}  color_info has the same format as non-tagged version.
+
 @node Examples
 @chapter Examples
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]

Re: [RFC] Tagged mbi

[Robert Millan]

On Mon, Jan 04, 2010 at 10:12:51PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> Hello. Currently mbi contains a lot of pointers and substructures. It's
> has various problems like:
> -Unused fields occupy space if any subsequent fields is used.
> -Difficult to relocate since it may be spilled all over the memory
> -It's unstraightforward to e.g. specify 2 framebuffers

Seems like there's a problem indeed.

> I propose to use tagged MBI.

Can you provide a short explanation on this proposed solution?  You explained
the problem, but not the solution.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi

Multiboot 2 branch (Re: [RFC] Tagged mbi)

[Robert Millan]

On Mon, Jan 04, 2010 at 10:12:51PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> --- include/multiboot.h	2010-01-02 17:50:06 +0000
> +++ include/multiboot.h	2010-01-02 21:06:42 +0000
> @@ -31,8 +31,11 @@
>  /* This should be in %eax.  */
>  #define MULTIBOOT_BOOTLOADER_MAGIC		0x2BADB002
>  
> +/* This should be in %eax.  */
> +#define MULTIBOOT_BOOTLOADER_MAGIC_TAGGED	0x3BADB002

I created a branch for backward-incompatible changes, it's in
/branches/multiboot2.

This does NOT mean that backward-incompatible changes are postponed.  The
draft in /branches/multiboot2 is already implemented in latest GRUB trunk.
Further changes to this draft can be implemented in GRUB inmediately.

So the workflow is basically:

  - Non-incompatible ammendments can go in /trunk/multiboot.

  - Incompatible ammendments can go in /branches/multiboot2.  This branch
    can (and should) be periodically resynced with /trunk/multiboot.

  - In both cases, ammendments should be accompanied by changes to the
    example kernel (when this makes sense, of course).

  - The master copies of multiboot.h and multiboot2.h are in those
    respective branches.  GRUB shouldn't need to diverge from them, as
    all the GRUB-specific declarations are split in <grub/*> namespace
    already.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi

Re: [RFC] Tagged mbi

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

Robert Millan wrote:
> On Mon, Jan 04, 2010 at 10:12:51PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>   
>> Hello. Currently mbi contains a lot of pointers and substructures. It's
>> has various problems like:
>> -Unused fields occupy space if any subsequent fields is used.
>> -Difficult to relocate since it may be spilled all over the memory
>> -It's unstraightforward to e.g. specify 2 framebuffers
>>     
>
> Seems like there's a problem indeed.
>
>   
>> I propose to use tagged MBI.
>>     
>
> Can you provide a short explanation on this proposed solution?  You explained
> the problem, but not the solution.
>
>   
The solution is to have so called tags. Tag is a structure which begins
with fields "size" and "type" and the rest of the content is determined
by these 2 fields. Tags follow each other and the end of structure is a
predefined tag. This way it's easy to skip unknown tags because of size
field, easy to relocate because mbi is in a contiguous chunk of memory.
Same type of tag may appear multiple times declaring e.g. 2
framebuffers. Details are in spec patch


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]