Re: [PATCH] efi: add feature_efifwsetup_check

[Robbie Harwood <rharwood@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2767 bytes --]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> In commit 26031d3b1 (efi: Don't display a uefi-firmware entry if it's
> not supported, 2022-08-18) an unconditional call to `fwsetup
> --is-supported` was added which causes the system to reboot if
> `grub-install` hasn't been run because `fwsetup` doesn't have the new
> option.
>
> This is a huge regression.
>
> To make sure `fwsetup --is-supported` works we add a new feature that
> only newer GRUB installations have: feature_efifwsetup_check.
>
> If the feature is available the current behavior is maintained (`fwsetup
> --is-supported`) but if it's not available the behavior before 26031d3b1
> is maintained (unconditionally show the menu entry on EFI platforms).
>
> Also do this only on the EFI platform as Dimitri John Ledkov suggested.
>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Robbie Harwood <rharwood@redhat.com>
> Cc: Dimitri John Ledkov <xnox@ubuntu.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  grub-core/normal/main.c         |  2 +-
>  util/grub.d/30_uefi-firmware.in | 16 +++++++++++-----
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
> index cb0e8e7fd..072e8e681 100644
> --- a/grub-core/normal/main.c
> +++ b/grub-core/normal/main.c
> @@ -506,7 +506,7 @@ static const char *features[] = {
>    "feature_chainloader_bpb", "feature_ntldr", "feature_platform_search_hint",
>    "feature_default_font_path", "feature_all_video_module",
>    "feature_menuentry_id", "feature_menuentry_options", "feature_200_final",
> -  "feature_nativedisk_cmd", "feature_timeout_style"
> +  "feature_nativedisk_cmd", "feature_timeout_style", "feature_efifwsetup_check"
>  };
>  
>  GRUB_MOD_INIT(normal)
> diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
> index c1731e5bb..c23d84741 100644
> --- a/util/grub.d/30_uefi-firmware.in
> +++ b/util/grub.d/30_uefi-firmware.in
> @@ -31,10 +31,16 @@ 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
> -	menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
> -		fwsetup
> -	}
> +if [ "\$grub_platform" = "efi" ]; then
> +  if [ "\$feature_efifwsetup_check" = "y" ] ; then
> +    fwsetup --is-supported
> +  else
> +    true
> +  fi
> +  if [ "\$?" = 0 ]; then
> +	  menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
> +		  fwsetup
> +	  }
> +  fi
>  fi
>  EOF
> -- 
> 2.37.3

Thanks for your interest, but I prefer Javier's proposed approach.

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]