grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Smarter EFI prefix handling


From: Colin Watson
Subject: [PATCH] Smarter EFI prefix handling
Date: Wed, 8 Sep 2010 13:23:36 +0100
User-agent: Mutt/1.5.18 (2008-05-17)

The install branch right now suffers from an interesting problem on EFI
systems.  The EFI System Partition doesn't necessarily contain GRUB
modules; the way it's set up right now is that the ESP is mounted on
/efi and GRUB modules are in their usual place in /boot/grub.  This
means that if prefix is empty then our initialisation code will default
to setting it to the ESP, which is not going to work properly.

It seems to me that the obvious way to deal with this is to try to unify
it with how PC BIOS systems behave: use a UUID to find /boot/grub,
unless the drive where core.img is installed is the same drive as that
containing /boot/grub in which case we can optimise away the costly UUID
search by simply hardcoding the partition number.

In the BIOS case, grub-setup is responsible for filling in the partition
number.  There's no grub-setup on EFI, though, and it seems overkill to
create one, so it seems best for grub-install to just set an appropriate
prefix.  This means that we need a prefix notation for "partition number
N on the same drive as core.img".  As it happens, I created such a
notation a while back to address problems with nested partitions:

2010-06-17  Colin Watson  <address@hidden>

        Fix i386-pc prefix handling with nested partitions (Debian bug
        #585068).  Note that the case where the core image is booted using
        multiboot and relocated from its original location still requires
        more work.

        * kern/i386/pc/init.c (make_install_device): If the prefix starts
        with "(,", fill the boot drive in between those two characters, but
        expect that a full partition specification including partition map
        names will follow.
        * util/i386/pc/grub-setup.c (setup): Unless an explicit prefix was
        specified, write a prefix without the drive name but including a
        full partition specification.

So, in the cause of platform unification, I suggest that we support the
same scheme on EFI.  Patch follows:

2010-09-08  Colin Watson  <address@hidden>

        * grub-core/kern/efi/init.c (grub_efi_set_prefix): If the prefix
        starts with "(,", fill the drive containing the loaded image in
        between those two characters, but expect that a full partition
        specification including partition map names will follow.

=== modified file 'grub-core/kern/efi/init.c'
--- grub-core/kern/efi/init.c   2010-07-02 12:42:18 +0000
+++ grub-core/kern/efi/init.c   2010-09-08 12:03:04 +0000
@@ -66,10 +66,33 @@ grub_efi_set_prefix (void)
       path = grub_strdup (pptr);
   }
 
-  if (!device || !path)
+  if ((!device || device[0] == ',' || !device[0]) || !path)
     image = grub_efi_get_loaded_image (grub_efi_image_handle);
-  if (image && !device)
-    device = grub_efidisk_get_device_name (image->device_handle);
+  if (image)
+    {
+      if (!device)
+       device = grub_efidisk_get_device_name (image->device_handle);
+      else if (device[0] == ',' || !device[0])
+       {
+         /* We have a partition, but still need to fill in the drive.  */
+         char *image_device, *comma, *new_device;
+
+         image_device = grub_efidisk_get_device_name (image->device_handle);
+         comma = grub_strchr (image_device, ',');
+         if (comma)
+           {
+             char *drive = grub_strndup (image_device, comma - image_device);
+             new_device = grub_xasprintf ("%s%s", drive, device);
+             grub_free (drive);
+           }
+         else
+           new_device = grub_xasprintf ("%s%s", image_device, device);
+
+         grub_free (image_device);
+         grub_free (device);
+         device = new_device;
+       }
+    }
 
   if (image && !path)
     {

We could then make the following change to the unified grub-install in
the install branch:

=== modified file 'util/grub-install.in'
--- util/grub-install.in        2010-09-08 12:07:21 +0000
+++ util/grub-install.in        2010-09-08 12:20:44 +0000
@@ -498,6 +498,7 @@ if [ "x${devabstraction_module}" = "x" ]
     grub_drive="`$grub_probe --target=drive --device ${grub_device}`" || exit 1
 
     # Strip partition number
+    grub_partition="`echo ${grub_drive} | sed -e 's/^[^,]*,//; s/)$//'`"
     grub_drive="`echo ${grub_drive} | sed -e s/,[a-z0-9,]*//g`"
     if [ "$disk_module" = ata ] ; then
         # generic method (used on coreboot and ata mod)
@@ -520,6 +521,10 @@ if [ "x${devabstraction_module}" = "x" ]
        echo 'set prefix=($root)'"${relative_grubdir}" >> ${grubdir}/load.cfg
        config_opt="-c ${grubdir}/load.cfg "
         modules="$modules search_fs_uuid"
+    elif [ "x$platform" = xefi ]; then
+        # No grub-setup, so we need to hardcode the partition number in the
+        # core image's prefix.
+        prefix_drive="(,$grub_partition)"
     fi
 else
     prefix_drive=`$grub_probe --target=drive --device ${grub_device}` || exit 1

What do you think?

Thanks,

-- 
Colin Watson                                       address@hidden



reply via email to

[Prev in Thread] Current Thread [Next in Thread]