Re: Grub EFI fallback - patches for review
On Tue, Dec 02, 2014 at 08:36:31AM +0000, Ian Campbell wrote:
>On Mon, 2014-12-01 at 13:57 +0000, Steve McIntyre wrote:
>
>Starting with grub-install-fallback.patch:
>
>> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001
>> debian/patches/grub-install-extra-removable.patch | 115 ++++++++++++++++++++++
>
>Could you send this to grub-devel@gnu.org? Or at least provide a commit
>log for the upstream bit inline in the patch for whoever does end up
>forwarding it.
Sure, no problem. I've added a header for now. As my stuff is
intermingled with other changes in the Debian tree, I'm not sure how
well that will work upstream...
>> +@@ -829,6 +838,27 @@ fill_core_services (const char *core_ser
>> + free (sysv_plist);
>> + }
>> +
>> ++static void
>> ++also_install_removable(const char *src, const char *base_efidir, const char *efi_suffix_upper)
>> ++{
>> ++ char *efi_file = NULL;
>> ++ char *dst = NULL;
>> ++ char *dir = NULL;
>> ++
>> ++ if (!efi_suffix_upper)
>> ++ grub_util_error ("%s", _("You've found a bug"));
>
>There are one or two of these in the upstream code base already, but it
>is a bit unfriendly to the bug-reported/triagger.
>
>I see an existing instance of
> _("you found a bug ... (%s:%d)\n"), file, line)
>which is a bit nicer at least. Plain old assert() sees some usage too.
OK. Again, I was just following the surrounding (grotty) style. :-)
I'll tweak.
>The Debian-specific bits all look sensible to me, FWIW. There will be a
>minor conflict with the patches in #770412 but nothing insurmountable.
Cool, ta!
>> [...]
>> + also depend on this path. If so, uou will need to ensure that GRUB is
>
>Typo: "uou".
ACK, already fixed after KiBi's feedback.
Rebased patch V2 against current git master attached...
--
Steve McIntyre, Cambridge, UK. steve@einval.com
"The problem with defending the purity of the English language is that
English is about as pure as a cribhouse whore. We don't just borrow words; on
occasion, English has pursued other languages down alleyways to beat them
unconscious and rifle their pockets for new vocabulary." -- James D. Nicoll
>From a863157a9020edfb6d1fa8379703026cd86c45c9 Mon Sep 17 00:00:00 2001
From: Steve McIntyre <steve@einval.com>
Date: Wed, 3 Dec 2014 01:51:57 +0000
Subject: [PATCH] Add support for forcing EFI installation to the removable
media path
Add an extra option to grub-install "--force-extra-removable". On EFI
platforms, this will cause an extra copy of the grub-efi image to be
written to the appropriate removable media patch
/boot/efi/EFI/BOOT/BOOT$ARCH.EFI as well. This will help with broken
UEFI implementations where the firmware does not work when configured
with new boot paths.
Also added new debconf logic to add this extra option to grub-install
calls when grub2/force_efi_extra_removable is set true. This allows
other programs like d-i / grub-installer to configure this for general
use.
Provides part of the fix for #767037
---
debian/changelog | 8 ++
debian/config.in | 1 +
debian/patches/grub-install-extra-removable.patch | 133 ++++++++++++++++++++++
debian/patches/series | 1 +
debian/postinst.in | 6 +-
debian/templates.in | 11 ++
6 files changed, 159 insertions(+), 1 deletion(-)
create mode 100644 debian/patches/grub-install-extra-removable.patch
diff --git a/debian/changelog b/debian/changelog
index f0eac36..70dd9aa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+grub2 (2.02~beta2-18) UNRELEASED; urgency=medium
+
+ [ Steve McIntyre ]
+ * Add support for forcing an extra copy of grub-efi to the removable
+ media path /boot/efi/EFI/BOOT/BOOT$ARCH.EFI (#767037)
+
+ -- Steve McIntyre <93sam@debian.org> Wed, 03 Dec 2014 01:23:18 +0000
+
grub2 (2.02~beta2-17) unstable; urgency=medium
[ Colin Watson ]
diff --git a/debian/config.in b/debian/config.in
index fcc0dd4..d2afbcb 100644
--- a/debian/config.in
+++ b/debian/config.in
@@ -73,4 +73,5 @@ esac
db_input ${priority} grub2/linux_cmdline || true
db_input medium grub2/linux_cmdline_default || true
+db_input low grub2/force_efi_extra_removable || true
db_go
diff --git a/debian/patches/grub-install-extra-removable.patch b/debian/patches/grub-install-extra-removable.patch
new file mode 100644
index 0000000..693e885
--- /dev/null
+++ b/debian/patches/grub-install-extra-removable.patch
@@ -0,0 +1,133 @@
+From: Steve McIntyre <93sam@debian.org>
+Date: Wed, 3 Dec 2014 01:25:12 +0000
+Subject: Add support for forcing EFI installation to the removable media path
+
+Add an extra option to grub-install "--force-extra-removable". On EFI
+platforms, this will cause an extra copy of the grub-efi image to be
+written to the appropriate removable media patch
+/boot/efi/EFI/BOOT/BOOT$ARCH.EFI as well. This will help with broken
+UEFI implementations where the firmware does not work when configured
+with new boot paths.
+
+Signed-off-by: Steve McIntyre <93sam@debian.org>
+
+Bug-Debian: https://bugs.debian.org/767037
+Forwarded: Not yet
+Last-Update: 2014-12-03
+
+Patch-Name: grub-install-pvxen-paths.patch
+Index: grub2-2.02~beta2/util/grub-install.c
+===================================================================
+--- grub2-2.02~beta2.orig/util/grub-install.c
++++ grub2-2.02~beta2/util/grub-install.c
+@@ -56,6 +56,7 @@
+
+ static char *target;
+ static int removable = 0;
++static int force_extra_removable = 0;
+ static int recheck = 0;
+ static int update_nvram = 1;
+ static char *install_device = NULL;
+@@ -114,7 +115,8 @@ enum
+ OPTION_LABEL_BGCOLOR,
+ OPTION_PRODUCT_VERSION,
+ OPTION_UEFI_SECURE_BOOT,
+- OPTION_NO_UEFI_SECURE_BOOT
++ OPTION_NO_UEFI_SECURE_BOOT,
++ OPTION_FORCE_EXTRA_REMOVABLE
+ };
+
+ static int fs_probe = 1;
+@@ -217,6 +219,10 @@ argp_parser (int key, char *arg, struct
+ removable = 1;
+ return 0;
+
++ case OPTION_FORCE_EXTRA_REMOVABLE:
++ force_extra_removable = 1;
++ return 0;
++
+ case OPTION_ALLOW_FLOPPY:
+ allow_floppy = 1;
+ return 0;
+@@ -323,6 +329,9 @@ static struct argp_option options[] = {
+ N_("do not install an image usable with UEFI Secure Boot, even if the "
+ "system was currently started using it. "
+ "This option is only available on EFI."), 2},
++ {"force-extra-removable", OPTION_FORCE_EXTRA_REMOVABLE, 0, 0,
++ N_("force installation to the removable media path also. "
++ "This option is only available on EFI."), 2},
+ {0, 0, 0, 0, 0, 0}
+ };
+
+@@ -829,6 +838,27 @@ fill_core_services (const char *core_ser
+ free (sysv_plist);
+ }
+
++static void
++also_install_removable(const char *src, const char *base_efidir, const char *efi_suffix_upper)
++{
++ char *efi_file = NULL;
++ char *dst = NULL;
++ char *dir = NULL;
++
++ if (!efi_suffix_upper)
++ grub_util_error ("%s", _("efi_suffix_upper not set"));
++ efi_file = xasprintf ("BOOT%s.EFI", efi_suffix_upper);
++
++ dir = grub_util_path_concat (3, base_efidir, "EFI", "BOOT");
++ grub_install_mkdir_p (dir);
++
++ dst = grub_util_path_concat (2, dir, efi_file);
++ grub_install_copy_file (src, dst, 1);
++ free (dst);
++ free (efi_file);
++ free (dir);
++}
++
+ int
+ main (int argc, char *argv[])
+ {
+@@ -846,6 +876,7 @@ main (int argc, char *argv[])
+ char *relative_grubdir;
+ char **efidir_device_names = NULL;
+ grub_device_t efidir_grub_dev = NULL;
++ char *base_efidir = NULL;
+ char *efidir_grub_devname;
+ int efidir_is_mac = 0;
+ int is_prep = 0;
+@@ -878,6 +909,9 @@ main (int argc, char *argv[])
+ bootloader_id = xstrdup ("grub");
+ }
+
++ if (removable && force_extra_removable)
++ grub_util_error (_("Invalid to use both --removable and --force_extra_removable"));
++
+ if (!grub_install_source_directory)
+ {
+ if (!target)
+@@ -1087,6 +1121,8 @@ main (int argc, char *argv[])
+ if (!efidir_is_mac && grub_strcmp (fs->name, "fat") != 0)
+ grub_util_error (_("%s doesn't look like an EFI partition.\n"), efidir);
+
++ base_efidir = xstrdup(efidir);
++
+ /* The EFI specification requires that an EFI System Partition must
+ contain an "EFI" subdirectory, and that OS loaders are stored in
+ subdirectories below EFI. Vendors are expected to pick names that do
+@@ -1949,9 +1985,15 @@ main (int argc, char *argv[])
+ fprintf (config_dst_f, "configfile $prefix/grub.cfg\n");
+ fclose (config_dst_f);
+ free (config_dst);
++ if (force_extra_removable)
++ also_install_removable(efi_signed, base_efidir, efi_suffix_upper);
+ }
+ else
+- grub_install_copy_file (imgfile, dst, 1);
++ {
++ grub_install_copy_file (imgfile, dst, 1);
++ if (force_extra_removable)
++ also_install_removable(imgfile, base_efidir, efi_suffix_upper);
++ }
+ free (dst);
+ }
+ if (!removable && update_nvram)
diff --git a/debian/patches/series b/debian/patches/series
index f48439b..7613f36 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -62,3 +62,4 @@ ppc64el-disable-vsx.patch
grub-install-pvxen-paths.patch
gettext-print-typo.patch
insmod-xzio-and-lzopio-on-xen.patch
+grub-install-extra-removable.patch
diff --git a/debian/postinst.in b/debian/postinst.in
index 056e27d..c03dadf 100644
--- a/debian/postinst.in
+++ b/debian/postinst.in
@@ -704,7 +704,11 @@ case "$1" in
grub-efi-arm) target=arm-efi ;;
grub-efi-arm64) target=arm64-efi ;;
esac
- run_grub_install --target="$target"
+ db_get grub2/force_efi_extra_removable
+ if [ "$RET" = true ]; then
+ FORCE_EXTRA_REMOVABLE="--force-extra-removable"
+ fi
+ run_grub_install --target="$target" "$FORCE_EXTRA_REMOVABLE"
fi
# /boot/grub/ has more chances of being accessible by GRUB
diff --git a/debian/templates.in b/debian/templates.in
index c8326f6..e04e877 100644
--- a/debian/templates.in
+++ b/debian/templates.in
@@ -12,6 +12,17 @@ _Description: Linux default command line:
The following string will be used as Linux parameters for the default menu
entry but not for the recovery mode.
+Template: grub2/force_efi_extra_removable
+Type: boolean
+Default: false
+_Description: Force extra installation to the EFI removable path?
+ Some EFI-based systems are buggy and do not handle new bootloaders correctly.
+ If you force extra installation of GRUB to the EFI removable path, it should
+ make sure that this system will boot Debian correctly despite such a problem.
+ However, this may remove the ability to boot any other operating systems that
+ also depend on this path. If so, you will need to ensure that GRUB is
+ configured successfully to be able boot any other OS installations correctly.
+
# still unused
Template: grub2/kfreebsd_cmdline
Type: string
--
2.1.3
Reply to: