Re: [PATCH 1/2] templates: Check for EFI at runtime instead of config generation time

[Daniel Kiper]

On Tue, Jul 06, 2021 at 11:02:14AM +0200, Javier Martinez Canillas wrote:
> The 30_uefi-firmware template checks if an OsIndicationsSupported UEFI var
> exists and EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set, to decide whether
> a "fwsetup" menu entry would be added or not to the GRUB menu.
>
> But this has the problem that it will only work if the configuration file
> was created on an UEFI machine that supports booting to a firmware UI.
>
> This for example doesn't support creating GRUB config files when executing
> on systems that support both UEFI and legacy BIOS booting. Since creating
> the config file from legacy BIOS wouldn't allow to access the firmware UI.
>
> To prevent this, make the template to unconditionally create the grub.cfg
> snippet but check at runtime if was booted through UEFI to decide if this
> entry should be added. That way it won't be added when booting with BIOS.
>
> There's no need to check if EFI_OS_INDICATIONS_BOOT_TO_FW_UI bit is set,
> since that's already done by the "fwsetup" command when is executed.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>
>  util/grub.d/30_uefi-firmware.in | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
> index d344d3883d7..b6041b55e2a 100644
> --- a/util/grub.d/30_uefi-firmware.in
> +++ b/util/grub.d/30_uefi-firmware.in
> @@ -26,19 +26,14 @@ export TEXTDOMAINDIR="@localedir@"
>
>  . "$pkgdatadir/grub-mkconfig_lib"
>
> -EFI_VARS_DIR=/sys/firmware/efi/efivars
> -EFI_GLOBAL_VARIABLE=8be4df61-93ca-11d2-aa0d-00e098032b8c
> -OS_INDICATIONS="$EFI_VARS_DIR/OsIndicationsSupported-$EFI_GLOBAL_VARIABLE"
> +LABEL="UEFI Firmware Settings"
>
> -if [ -e "$OS_INDICATIONS" ] && \
> -   [ "$(( $(printf 0x%x \'"$(cat $OS_INDICATIONS | cut -b5)"\') & 1 ))" = 1 
> ]; then
> -  LABEL="UEFI Firmware Settings"
> +gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
>
> -  gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" 
> >&2
> -
> -  cat << EOF
> -menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
> -     fwsetup
> -}
> -EOF
> +cat << EOF
> +if [ "\$grub_platform" = "efi" ]; then
> +     menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
> +             fwsetup
> +     }

Should not we go further and enable this menu entry only if (UEFI)
firmware settings are available? Of course this requires exporting
relevant information to the GRUB shell.

Daniel