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 -- Best regards, Javier Martinez Canillas Core Platforms Red Hat _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
