Javier Martinez Canillas <javi...@redhat.com> writes: > On 8/31/22 00:07, Philip Müller wrote: >> On 30.08.22 23:34, Javier Martinez Canillas wrote: >>>> You could add a feature flag, which causes grub-core to set an >>>> environment variable when a new feature is supported. See the features >>>> array in grub-core/normal/main.c. >>>> >>>> You would then check for this feature flag in the grub.d snippet >>>> before calling fwsetup --is-supported. >>>> >>> Please don't. As mentioned, I think we should aim to simplify the grub.cfg >>> instead of making it more complicated. >> >> Well I think it would be the best approach to add backward compatibility >> as most users don't even know on how to install grub via grub-install. >> That is done via the graphical installer Calamares on most Arch-based >> Distributions. Updating the grub menu is common if you install multiple >> kernels or use snapshots via BTRFS. >> >> Simply calling 'fwsetup' is a big NO-NO to me and others. The old >> version runs into the EFI firware or simply turn off the PC during boot, >> which creates boot loops for some or unbootable systems. > > I'm OK to not call fwsetup at all, that's what we originally had in the > series posted by Robbie, for example in v2: > > https://lists.gnu.org/archive/html/grub-devel/2022-08/msg00169.html > > Then they added the fwsetup --is-supported option as mentioned because > other developer asked for that. That patch was included in v3 onward: > > https://lists.gnu.org/archive/html/grub-devel/2022-08/msg00196.html > >> I checked on my end with an older grub in /boot and the updated menu.cfg >> script. Only when removing the snippet of 30_uefi-firmware the system is >> bootable again. >> > > That's fair. Then probably what we should do is to partially revert > commit 26031d3b1016 ("efi: Don't display a uefi-firmware entry if > it's not supported"). Something like the following patch perhaps ? > > From c3edc64687686d9a5a54f769ec03101b2c4cdef1 Mon Sep 17 00:00:00 2001 > From: Javier Martinez Canillas <javi...@redhat.com> > Date: Wed, 31 Aug 2022 00:30:31 +0200 > Subject: [PATCH] templates: Don't execute fwsetup unconditionally > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Commit 26031d3b1016 ("efi: Don't display a uefi-firmware entry if it's not > supported") added a new `fwsetup --is-supported` option that could be used > to check if the firmware allows to enter into a firmware user interface. > > That option was then used by /etc/grub.d/30_uefi-firmware script to figure > out whether a `fwsetup` entry should be included or not in the boot menu. > > But unfortunately, old GRUB images will fail to boot if are booted with an > updated GRUB config file. Since the `fwsetup --is-supported` call would be > taken as a plan `fwsetup` with no options. > > This could either lead to a crash (i.e: on legacy BIOS systems where that > is not supported) or to the machine entering into the firmware settings. > > To prevent that, let's partially revert the mentioned commit and keep the > old logic that didn't execute the `fwsetup` command and just included an > entry for it if GRUB is executed in an EFI platform. > > Fixes: 26031d3b1016 ("efi: Don't display a uefi-firmware entry if it's not > supported") > Reported-by: Philip Müller <ph...@manjaro.org> > Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> > --- > util/grub.d/30_uefi-firmware.in | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in > index c1731e5bbbb3..b6041b55e2a0 100644 > --- a/util/grub.d/30_uefi-firmware.in > +++ b/util/grub.d/30_uefi-firmware.in > @@ -31,8 +31,7 @@ LABEL="UEFI Firmware Settings" > gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2 > > cat << EOF > -fwsetup --is-supported > -if [ "\$grub_platform" = "efi" -a "\$?" = 0 ]; then > +if [ "\$grub_platform" = "efi" ]; then > menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' { > fwsetup > } > -- > 2.37.1
While we could revert the entire --is-supported logic as well, since this is upstream pre-release code, it's probably easier for the downstreams that pulled this change if we don't, so: Reviewed-by: Robbie Harwood <rharw...@redhat.com>. Be well, --Robbie
signature.asc
Description: PGP signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
