Date: Wed, 13 Jun 2007 19:01:59 +0200
From: Robert Millan <rmh@aybabtu.com>
To: The development of GRUB 2 <grub-devel@gnu.org>
Message-ID: <20070613170159.GA29173@aragorn>
References: <20070612135412.GA1683@aragorn> <20070612173714.GA621@ws3.vdp.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="a8Wt8u1KmwUX3Y2C"
Content-Disposition: inline
In-Reply-To: <20070612173714.GA621@ws3.vdp.com>
Organization: free as in freedom
User-Agent: Mutt/1.5.13 (2006-08-11)
Subject: [PATCH] move prefix initialisation to grub-mkimage (Re: grub_prefix)
Precedence: list
Reply-To: The development of GRUB 2 <grub-devel@gnu.org>


--a8Wt8u1KmwUX3Y2C
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Jun 13, 2007 at 01:37:15AM +0800, Bean wrote:
> On Tue, Jun 12, 2007 at 03:54:12PM +0200, Robert Millan wrote:
> > 
> > Is there any reason why grub_prefix is in boot.img rather than core.img ?
> > 
> > This brings problems when the following conditions are met:
> > 
> >   - core.img is loaded directly without boot.img (via GRUB Legacy)
> >   - grub is not in /boot/grub (typicaly, when /boot is a separate partition)
> > 
> > If grub_prefix is moved to core.img, then grub-mkimage could be modified to
> > override prefix, supporting this setup.
> > 
> > Is this a good way to fix it?
> > 
> 
> I believe grub_prefix IS in core.img. It begins at 0x21C from the beginning
> of core.img. You may also want to modify the variable grub_install_dos_part
> at 0x214, for example, if booting from the first partition, set it to 0.
> 
> The above variables are defined in kern/i386/pc/startup.S.

Ah, somehow I got the impression that startup.S was part of boot.img :-)

Here's a patch to add a --prefix option for the pc and efi version of
grub-mkimage, that moves initialisation of grub_prefix from kernel image
to grub-mkimage, possibly overriding it.

I tested it and is known to work, but does this look right?  Can someone
comment?

-- 
Robert Millan

My spam trap is honeypot@aybabtu.com.  Note: this address is only intended
for spam harvesters.  Writing to it will get you added to my black list.

--a8Wt8u1KmwUX3Y2C
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="move_prefix_initialisation_to_mkimage.diff"

diff -ur grub2-1.95+20070611.old/kern/i386/efi/startup.S grub2-1.95+20070611/kern/i386/efi/startup.S
--- grub2-1.95+20070611.old/kern/i386/efi/startup.S	2006-04-18 08:18:15.000000000 +0200
+++ grub2-1.95+20070611/kern/i386/efi/startup.S	2007-06-13 15:51:18.000000000 +0200
@@ -45,7 +45,7 @@
         . = EXT_C(start) + 0x8
 
 VARIABLE(grub_prefix)
-        .string "/boot/grub"
+	/* to be filled by grub-mkimage */
 
         /*
          *  Leave some breathing room for the prefix.
diff -ur grub2-1.95+20070611.old/kern/i386/pc/startup.S grub2-1.95+20070611/kern/i386/pc/startup.S
--- grub2-1.95+20070611.old/kern/i386/pc/startup.S	2006-05-14 23:16:19.000000000 +0200
+++ grub2-1.95+20070611/kern/i386/pc/startup.S	2007-06-13 15:23:06.000000000 +0200
@@ -96,7 +96,7 @@
 VARIABLE(grub_install_bsd_part)
 	.long	0xFFFFFFFF
 VARIABLE(grub_prefix)
-	.string "/boot/grub"
+	/* to be filled by grub-mkimage */
 
 	/*
 	 *  Leave some breathing room for the prefix.
diff -ur grub2-1.95+20070611.old/util/i386/efi/grub-mkimage.c grub2-1.95+20070611/util/i386/efi/grub-mkimage.c
--- grub2-1.95+20070611.old/util/i386/efi/grub-mkimage.c	2006-05-08 21:29:10.000000000 +0200
+++ grub2-1.95+20070611/util/i386/efi/grub-mkimage.c	2007-06-13 16:07:40.000000000 +0200
@@ -48,7 +48,7 @@
 /* Read the whole kernel image. Return the pointer to a read image,
    and store the size in bytes in *SIZE.  */
 static char *
-read_kernel_module (const char *dir, size_t *size)
+read_kernel_module (const char *dir, char *prefix, size_t *size)
 {
   char *kernel_image;
   char *kernel_path;
@@ -58,6 +58,11 @@
   kernel_image = grub_util_read_image (kernel_path);
   free (kernel_path);
 
+  // FIXME: can we avoid hardcoding offset and size of grub_prefix here?
+  if (strlen (prefix) > 0x47)
+    grub_util_error ("prefix too long");
+  strcpy (kernel_image + 0x3c, prefix);
+
   return kernel_image;
 }
 
@@ -838,7 +843,7 @@
 
 /* Convert an ELF relocatable object into an EFI Application (PE32).  */
 void
-convert_elf (const char *dir, FILE *out, char *mods[])
+convert_elf (const char *dir, char *prefix, FILE *out, char *mods[])
 {
   char *kernel_image;
   size_t kernel_size;
@@ -855,7 +860,7 @@
   Elf32_Addr end_address;
 
   /* Get the kernel image and check the format.  */
-  kernel_image = read_kernel_module (dir, &kernel_size);
+  kernel_image = read_kernel_module (dir, prefix, &kernel_size);
   e = (Elf32_Ehdr *) kernel_image;
   if (! check_elf_header (e, kernel_size))
     grub_util_error ("invalid ELF header");
@@ -912,6 +917,7 @@
 static struct option options[] =
   {
     {"directory", required_argument, 0, 'd'},
+    {"prefix", required_argument, 0, 'p'},
     {"output", required_argument, 0, 'o'},
     {"help", no_argument, 0, 'h'},
     {"version", no_argument, 0, 'V'},
@@ -931,13 +937,14 @@
 Make a bootable image of GRUB.\n\
 \n\
 -d, --directory=DIR     use images and modules under DIR [default=%s]\n\
+-p, --prefix=DIR        set grub_prefix directory [default=%s]\n\
 -o, --output=FILE       output a generated image to FILE\n\
 -h, --help              display this message and exit\n\
 -V, --version           print version information and exit\n\
 -v, --verbose           print verbose messages\n\
 \n\
 Report bugs to <%s>.\n\
-", GRUB_LIBDIR, PACKAGE_BUGREPORT);
+", GRUB_LIBDIR, DEFAULT_DIRECTORY, PACKAGE_BUGREPORT);
 
   exit (status);
 }
@@ -948,12 +955,13 @@
   FILE *fp;
   char *output = NULL;
   char *dir = NULL;
+  char *prefix = NULL;
 
   progname = "grub-mkimage";
 
   while (1)
     {
-      int c = getopt_long (argc, argv, "d:o:hVv", options, 0);
+      int c = getopt_long (argc, argv, "d:p:o:hVv", options, 0);
       if (c == -1)
 	break;
 
@@ -972,6 +980,11 @@
 	      free (output);
 	    output = xstrdup (optarg);
 	    break;
+	  case 'p':
+	    if (prefix)
+	      free (prefix);
+	    prefix = xstrdup (optarg);
+	    break;
 	  case 'V':
 	    printf ("grub-mkimage (%s) %s\n", PACKAGE_NAME, PACKAGE_VERSION);
 	    return 0;
@@ -991,7 +1004,7 @@
   if (! fp)
     grub_util_error ("cannot open %s", output);
 
-  convert_elf (dir ? : GRUB_LIBDIR, fp, argv + optind);
+  convert_elf (dir ? : GRUB_LIBDIR, prefix ? : DEFAULT_DIRECTORY, fp, argv + optind);
 
   fclose (fp);
 
diff -ur grub2-1.95+20070611.old/util/i386/pc/grub-mkimage.c grub2-1.95+20070611/util/i386/pc/grub-mkimage.c
--- grub2-1.95+20070611.old/util/i386/pc/grub-mkimage.c	2006-11-25 04:21:29.000000000 +0100
+++ grub2-1.95+20070611/util/i386/pc/grub-mkimage.c	2007-06-13 16:07:05.000000000 +0200
@@ -76,7 +76,7 @@
 }
 
 static void
-generate_image (const char *dir, FILE *out, char *mods[])
+generate_image (const char *dir, char *prefix, FILE *out, char *mods[])
 {
   grub_addr_t module_addr = 0;
   char *kernel_img, *boot_img, *core_img;
@@ -101,6 +101,10 @@
 
   kernel_img = xmalloc (kernel_size + total_module_size);
   grub_util_load_image (kernel_path, kernel_img);
+  // FIXME: can we avoid hardcoding offset and size of grub_prefix here?
+  if (strlen (prefix) > 0x33)
+    grub_util_error ("prefix too long");
+  strcpy (kernel_img + 0x1c, prefix);
 
   /* Fill in the grub_module_info structure.  */
   modinfo = (struct grub_module_info *) (kernel_img + kernel_size);
@@ -182,6 +186,7 @@
 static struct option options[] =
   {
     {"directory", required_argument, 0, 'd'},
+    {"prefix", required_argument, 0, 'p'},
     {"output", required_argument, 0, 'o'},
     {"help", no_argument, 0, 'h'},
     {"version", no_argument, 0, 'V'},
@@ -201,13 +206,14 @@
 Make a bootable image of GRUB.\n\
 \n\
   -d, --directory=DIR     use images and modules under DIR [default=%s]\n\
+  -p, --prefix=DIR        set grub_prefix directory [default=%s]\n\
   -o, --output=FILE       output a generated image to FILE [default=stdout]\n\
   -h, --help              display this message and exit\n\
   -V, --version           print version information and exit\n\
   -v, --verbose           print verbose messages\n\
 \n\
 Report bugs to <%s>.\n\
-", GRUB_LIBDIR, PACKAGE_BUGREPORT);
+", GRUB_LIBDIR, DEFAULT_DIRECTORY, PACKAGE_BUGREPORT);
 
   exit (status);
 }
@@ -215,15 +221,16 @@
 int
 main (int argc, char *argv[])
 {
-  char *output = 0;
-  char *dir = 0;
+  char *output = NULL;
+  char *dir = NULL;
+  char *prefix = NULL;
   FILE *fp = stdout;
 
   progname = "grub-mkimage";
   
   while (1)
     {
-      int c = getopt_long (argc, argv, "d:o:hVv", options, 0);
+      int c = getopt_long (argc, argv, "d:p:o:hVv", options, 0);
 
       if (c == -1)
 	break;
@@ -248,6 +255,13 @@
 	    usage (0);
 	    break;
 
+	  case 'p':
+	    if (prefix)
+	      free (prefix);
+
+	    prefix = xstrdup (optarg);
+	    break;
+
 	  case 'V':
 	    printf ("grub-mkimage (%s) %s\n", PACKAGE_NAME, PACKAGE_VERSION);
 	    return 0;
@@ -269,7 +283,7 @@
 	grub_util_error ("cannot open %s", output);
     }
 
-  generate_image (dir ? : GRUB_LIBDIR, fp, argv + optind);
+  generate_image (dir ? : GRUB_LIBDIR, prefix ? : DEFAULT_DIRECTORY, fp, argv + optind);
 
   fclose (fp);
 

--a8Wt8u1KmwUX3Y2C--