Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported

[Dimitri John Ledkov]

On Tue, 30 Aug 2022 at 21:22, Robbie Harwood <rharwood@redhat.com> wrote:
>
> Philip Müller <philm@manjaro.org> writes:
>
> >> Hello Robbie, hello Daniel,
> >>
> >> with the commit 26031d3b101648352e4e427f04bf69d320088e77
> >> 30_uefi-firmware will always call `fwsetup --is-supported' to check
> >> if the system supports EFI or not. However most installed grub
> >> versions on MBR don't support the '--is-supported' flag and crash the
> >> menu creation routine.
> >>
> >> Arch Linux is tracking the issue here: 
> >> https://bugs.archlinux.org/task/75701
> >>
> >> What would be the best approach with older installations of grub via
> >> grub-install not having to reinstall grub to MBR as some users don't
> >> even know on how to install grub properly as that job a graphical
> >> installer does.
> >
> > Hi all,
>
> Adding grub-devel to CC.
>
> > I looked into the '30_uefi-firmware' changes a little more and also
> > commented my findings at the Arch-Bugtracker:
> > https://bugs.archlinux.org/task/75701#comment210684
> >
> > Before Menu changes we simply had this:
> >
> > ### BEGIN /etc/grub.d/30_uefi-firmware ###
> > menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
> >    fwsetup
> > }
> > ### END /etc/grub.d/30_uefi-firmware ###
> >
> > After Menu changes we went to that:
> >
> > ### BEGIN /etc/grub.d/30_uefi-firmware ###
> > fwsetup --is-supported
> > if [ "$grub_platform" = "efi" -a "$?" = 0 ]; then
> >    menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
> >      fwsetup
> >    }
> > fi

above code looks buggy to me. As far as I understand fwsetup only
works on EFI grub_platform, and thus originally the menu entry and
fwsetup call was scoped under efi platform. thus there should be two
if conditions, not one:

if [ "$grub_platform" = "efi" ]; then
fwsetup --is-supported
if [ "$?" = 0 ]; then
menuetry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
fwsetup
}
fi
fi

This way the code continues to work correctly on bios platforms
(skipped completely), and on EFI like platforms fwsetup menu entry is
only shown if fwsetup --is-supported executes successfully.

the optimisation of having two conditionals evaluated together, whilst
efficient, is wrong given cross grub platform compatibilities desires.

This fwsetup has never before been executed on bios platform, and we
should not be introducing this.

> > ### END /etc/grub.d/30_uefi-firmware ###
> >
> > So 0eb684e8bfb0a9d2d42017a354740be25947babe I get and
> > 26031d3b101648352e4e427f04bf69d320088e77 tries to improve things,
> > however doesn't count in what will happen if 'fwsetup' is not supporting
> > the flag. It may either crash due to
> > 1e79bbfbda24a08cb856ff30f8b1bec460779b91 or start UEFI firmware instead.
>
> Why doesn't grub on the MBR get updated when you install a new grub
> package?  That seems like the real issue here - what am I missing?
>
> Regardless, if we can't count on fwsetup being updated, then I think we
> need to go back to the original version of my patch which doesn't have
> --is-supported.
>
> > Simply don't break user-space when older grub got installed to MBR.
> > Somehow this idea needs some more thinking and a solution for that
> > case too.
>
> Sure, what do you suggest?
>
> Be well,
> --Robbie
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
okurrr,

Dimitri