[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 3/4] templates: Check for EFI at runtime instead of config
From: |
Glenn Washburn |
Subject: |
Re: [PATCH v2 3/4] templates: Check for EFI at runtime instead of config generation time |
Date: |
Wed, 17 Aug 2022 17:45:33 -0500 |
On Wed, 17 Aug 2022 15:50:33 -0400
Robbie Harwood <rharwood@redhat.com> wrote:
> From: Javier Martinez Canillas <javierm@redhat.com>
>
> 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>
> Signed-off-by: Robbie Harwood <rharwood@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 d344d3883d..b6041b55e2 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
I think it would be even better if this was not displayed if
efifwsetup_is_supported () returns false. This could be done with
another condition on the output of "fwsetup --is-supported" and in the
following patch or an additional patch add support for the
"--is-supported" argument which should just return success if
efifwsetup_is_supported () is true and false otherwise. This addition
would preserve the current behavior which this patch otherwise does not
completely preserve.
Glenn
> + menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
> + fwsetup
> + }
> fi
> +EOF