Re: [PATCH] efi: stub: override RT_PROP table supported mask based on EFI variable

[Ard Biesheuvel <ardb@kernel.org>]

On Tue, 16 Mar 2021 at 08:52, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> On Mon, Mar 15, 2021 at 02:07:01PM +0100, Ard Biesheuvel wrote:
> > On Mon, 15 Mar 2021 at 04:11, Shawn Guo <shawn.guo@linaro.org> wrote:
> > >
> > > On Tue, Mar 09, 2021 at 07:47:25PM +0100, Ard Biesheuvel wrote:
> > > > On Tue, 9 Mar 2021 at 19:10, Rob Clark <robdclark@gmail.com> wrote:
> > > > >
> > ...
> > > > > fwiw, the valid use-case for ACPI boot on these things is for distro
> > > > > installer.. it might not be the shiny accelerated experience, but you
> > > > > want to be able to get thru the installer and then install updates to
> > > > > get latest kernel/dtb/etc
> > > > >
> > > > > it is a small use-case, but kinda an important step ;-)
> > > > >
> > > >
> > > > That is a fair point. However, as I understand it, we need this to work around
> > > > - the need to pass efi=novamap
> > > > - broken poweroff on Flex5g
> > >
> > > One more: broken EFI variable runtime services on all Snapdragon laptops
> > >
> > > It's been another pain of running debian-installer (d-i) on these
> > > laptops, where EFI NV variables are just stored on UFS disk.  So after
> > > Linux takes over the control of UFS, EFI NV variable runtime services
> > > then become out of service.  Currently, we have to apply a hack [1] on
> > > d-i grub-installer to get around the issue,  and it's been the only
> > > remaining out-of-tree patch we have to carry for d-i.  With this nice
> > > `OverrideSupported` support, we will be able to drop that hack
> > > completely.
> > >
> > > >
> > > > So an installer either needs to set the EFI variable, or pass
> > > > efi=novamap on the first boot. Note that there are no arm64 EFI
> > > > systems known where efi=novamap causes problems. In fact, I would
> > > > prefer to stop using SetVirtualAddressMap() altogether, as it does not
> > > > provide any benefit whatsoever. So perhaps we should make efi=novamap
> > > > the default and be done with it.
> > > >
> > > > Broken poweroff is hardly a showstopper for an installer, given that
> > > > we cannot even install GRUB correctly.
> > > >
> > > > In summary, I am more than happy to collaborate constructively on this
> > > > (which is why I wrote the patch), but I don't think we're at a point
> > > > yet where this is the only thing standing in our way when it comes to
> > > > a smooth out-of-the-box Linux installation experience.
> > >
> > > There might be more to be done for getting a smooth Linux installation
> > > experience.  But IMHO, this `OverrideSupported` thing is definitely
> > > a big step to that.
> > >
> >
> > So the problem here seems to be that grub-install (or efibootmgr)
> > tolerates efivarfs being absent entirely, but bails out if it exists
> > but gives an error when trying to access it, right?
>
> Yes, with EFI variables runtime service marked as unsupported,
> efibootmgr will just exit on efi_variables_supported() check [1] in
> a way that its parent process, i.e. grub-install, doesn't take as an
> error.  But otherwise, efibootmgr will go much further and exit with
> a real error when trying to access efivars.
>

OK, so I suggest we fix efibootmgr, by extending the
efi_variables_supported() check to perform a GetVariable() call on an
arbitrary variable (e.g., BootOrder), and treat EFI_UNSUPPORTED as a
special case that means that carrying on is pointless.

(but someone please confirm that the snapdragon efi firmware does
return EFI_UNSUPPORTED and not some other return value when calling
GetVariable() from under the OS)