grub_prefix

PATCH In review Robert Millan 2 participants 4 messages Jun 12, 2007 – Jun 15, 2007

lore

Share & tools

lore

Robert Millan

author

2007-06-12 13:53 UTC #0 lore raw

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?

Signature · 4 lines

Robert Millan

My spam trap is [email protected].  Note: this address is only intended
for spam harvesters.  Writing to it will get you added to my black list.

Bean

2007-06-12 17:37 UTC #1 lore raw

Quoted 5 lines · Robert Millan

Is there any reason why grub_prefix is in boot.img rather than core.img ?
This brings problems when the following conditions are met:

Quoted 8 lines

- 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.

Signature · 1 line

Bean <[email protected]>

Robert Millan

author

2007-06-13 17:01 UTC #2 lore raw

PATCH 1/1 move prefix initialisation to grub-mkimage (Re: grub_prefix)

Quoted 12 lines · Bean

Quoted 11 lines · Robert Millan
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.

Quoted 8 lines

Quoted 2 lines
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?

Signature · 4 lines

Robert Millan

My spam trap is [email protected].  Note: this address is only intended
for spam harvesters.  Writing to it will get you added to my black list.

kern/i386/efi/startup.S +1 -1

diff -ur grub2-1.95+20070611.old/kern/i386/efi/startup.S grub2-1.95+20070611/kern/i386/efi/startup.S
@@ -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.

kern/i386/pc/startup.S +1 -1

diff -ur grub2-1.95+20070611.old/kern/i386/pc/startup.S grub2-1.95+20070611/kern/i386/pc/startup.S
@@ -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.

util/i386/efi/grub-mkimage.c +19 -6

diff -ur grub2-1.95+20070611.old/util/i386/efi/grub-mkimage.c grub2-1.95+20070611/util/i386/efi/grub-mkimage.c
@@ -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);

util/i386/pc/grub-mkimage.c +20 -6

diff -ur grub2-1.95+20070611.old/util/i386/pc/grub-mkimage.c grub2-1.95+20070611/util/i386/pc/grub-mkimage.c
@@ -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);

Robert Millan

author

2007-06-15 18:06 UTC #3 lore raw

Re: [PATCH] move prefix initialisation to grub-mkimage (Re: grub_prefix)

I made some changes based on Marco's suggestions on IRC. Got rid of that FIXME ;-)

It removes some redundancy, although the offsets are still duplicated in startup.S, but this isn't easy to fix (gas doesn't like C declarations, or even the preprocessor macros).

If noone objects, I'll commit this in a few days.

Signature · 4 lines

Robert Millan

My spam trap is [email protected].  Note: this address is only intended
for spam harvesters.  Writing to it will get you added to my black list.

2007-06-15 Robert Millan <[email protected]>

* include/grub/i386/pc/kernel.h: Define GRUB_KERNEL_MACHINE_DATA_END to indicate end of data section in kernel image.
* include/grub/i386/efi/kernel.h: Define GRUB_KERNEL_MACHINE_PREFIX and GRUB_KERNEL_MACHINE_DATA_END.

* kern/i386/pc/startup.S: Do not initialize grub_prefix, only reserve space for it.
* kern/i386/efi/startup.S: Likewise.

* util/i386/pc/grub-mkimage.c: Initialize grub_prefix to /boot/grub during image generation. Implement --prefix option to override this patch.
* util/i386/efi/grub-mkimage.c: Likewise.

include/grub/i386/efi/kernel.h +6 -0

diff -ur grub2-1.95+20070614.old/include/grub/i386/efi/kernel.h grub2-1.95+20070614/include/grub/i386/efi/kernel.h
@@ -24,5 +24,11 @@
    configuration file are located.  */
 extern char grub_prefix[];
 
+/* The offset of GRUB_PREFIX.  */
+#define GRUB_KERNEL_MACHINE_PREFIX		0x8
+
+/* End of the data section. */
+#define GRUB_KERNEL_MACHINE_DATA_END		0x50
+
 #endif /* ! GRUB_MACHINE_KERNEL_HEADER */

include/grub/i386/pc/kernel.h +3 -0

diff -ur grub2-1.95+20070614.old/include/grub/i386/pc/kernel.h grub2-1.95+20070614/include/grub/i386/pc/kernel.h
@@ -38,6 +38,9 @@
 /* The offset of GRUB_PREFIX.  */
 #define GRUB_KERNEL_MACHINE_PREFIX		0x1c
 
+/* End of the data section. */
+#define GRUB_KERNEL_MACHINE_DATA_END		0x50
+
 /* The size of the first region which won't be compressed.  */
 #define GRUB_KERNEL_MACHINE_RAW_SIZE		0x4A0

kern/i386/efi/startup.S +1 -1

diff -ur grub2-1.95+20070614.old/kern/i386/efi/startup.S grub2-1.95+20070614/kern/i386/efi/startup.S
@@ -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.

kern/i386/pc/startup.S +1 -1

diff -ur grub2-1.95+20070614.old/kern/i386/pc/startup.S grub2-1.95+20070614/kern/i386/pc/startup.S
@@ -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.

util/i386/efi/grub-mkimage.c +19 -6

diff -ur grub2-1.95+20070614.old/util/i386/efi/grub-mkimage.c grub2-1.95+20070614/util/i386/efi/grub-mkimage.c
@@ -30,6 +30,7 @@
 #include <grub/util/resolve.h>
 #include <grub/kernel.h>
 #include <grub/efi/pe32.h>
+#include <grub/machine/kernel.h>
 
 static const grub_uint8_t stub[] = GRUB_PE32_MSDOS_STUB;

@@ -48,7 +49,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 +59,10 @@
   kernel_image = grub_util_read_image (kernel_path);
   free (kernel_path);
 
+  if (GRUB_KERNEL_MACHINE_PREFIX + strlen (prefix) + 1 > GRUB_KERNEL_MACHINE_DATA_END)
+    grub_util_error ("prefix too long");
+  strcpy (kernel_image + 0x34 + GRUB_KERNEL_MACHINE_PREFIX, 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);

util/i386/pc/grub-mkimage.c +20 -6

diff -ur grub2-1.95+20070614.old/util/i386/pc/grub-mkimage.c grub2-1.95+20070614/util/i386/pc/grub-mkimage.c
@@ -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;

@@ -102,6 +102,10 @@
   kernel_img = xmalloc (kernel_size + total_module_size);
   grub_util_load_image (kernel_path, kernel_img);
 
+  if (GRUB_KERNEL_MACHINE_PREFIX + strlen (prefix) + 1 > GRUB_KERNEL_MACHINE_DATA_END)
+    grub_util_error ("prefix too long");
+  strcpy (kernel_img + GRUB_KERNEL_MACHINE_PREFIX, prefix);
+
   /* Fill in the grub_module_info structure.  */
   modinfo = (struct grub_module_info *) (kernel_img + kernel_size);
   modinfo->magic = GRUB_MODULE_MAGIC;

@@ -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);