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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 16 Mar 2021 at 15:45, Heinrich Schuchardt <xypron.glpk@xxxxxx> wrote:
>
> On 16.03.21 15:06, Ard Biesheuvel wrote:
> > On Tue, 16 Mar 2021 at 14:25, Heinrich Schuchardt <xypron.glpk@xxxxxx> wrote:
> >>
> >> On 16.03.21 10:33, Ilias Apalodimas wrote:
> >>> Hi Shawn,
> >>>
> >>>>>>>>> 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),
> >>>>
> >>>> Hmm, I'm not sure we should ask more from user space, as it's already
> >>>> been doing the right thing, and efi_variables_supported() is proved to
> >>>> work properly with any sane low-level software (kernel + firmware),
> >>>> either EFI variables service is supported or not.  That said, IMHO,
> >>
> >> No, there is not one but there are three EFI variable services.
> >>
> >> GetVariable() available after ExitBootServices() and SetVariable() not
> >> available() is completely legal according to the current UEFI specification.
> >>
> >
> > This is a valid point. efibootmgr may be able to read the Boot####
> > variables, but may be unable to change them.
> >
> >>>> right now the low-level software on Snapdragon laptops is insane, i.e.
> >>>> the unsupported/broken EFI runtime services are not communicated to
> >>>> user space properly in established way.
> >>
> >> Please, describe:
> >>
> >> * Which UEFI version is reported by your Snapdragon laptop?
> >> * What is the observed behavior?
> >>
> >
> > These laptops have the EFI varstore in a eMMC partition. This is
> > basically the same problem that many uboot based platforms have as
> > well, i.e., that it is not possible for the OS and the firmware to
> > share the MMC because the ownership of the MMC controller cannot be
> > shared.
> >
> >>>
> >>> But the EFI_UNSUPPORTED is an error that's allowed from the spec.
> >>> Yes the sane thing to do is not expose it at all, but it's not violating
> >>> any spec by doing so.
> >>> So why shouldn't a userspace application be able to handle all return codes
> >>> explicitly and instead treat them as a single error? And when that happens why
> >>> shouldut  the kernel mask that error out for it?
> >>
> >> The runtime services must always be callable even they can only report
> >> EFI_UNSUPPORTED.
> >>
> >> Only since UEFI specification 2.8 errata B do we have the
> >> EFI_RT_PROPERTIES_TABLE which tells us which runtime services are
> >> implemented.
> >>
> >> UEFI 2.8 B makes it clear that any runtime service may be reported as
> >> unsupported. EFI applications are expected to cope with a service being
> >> unavailable.
> >>
> >
> > Indeed. The firmware on these machines predates the UEFI 2.8B
> > specification, but given the fact that EFI_UNSUPPORTED is a valid
> > return code now for these services, we should deal with them
> > gracefully anyway. And apparently, doing so would also remove the need
> > for special hacks to support installing GRUB on these platforms.
> >
>
> Hello Ard,
>
> it is still unclear to why you would need the patch. Why should a user
> provide a UEFI variable telling which service is not working correctly?
>

Why would it be the user setting this variable?

> Is the firmware correctly returning EFI_UNSUPPORTED for unsupported
> services?

Yes

> For which services?
>

All runtime services except SetVirtualAddressMap() and ResetSystem()

> In which software is the bug that cannot gracefully deal with
> unsupported services?
>

The debian installer gives up if it cannot set the boot path for GRUB.

> GRUB never gave me a problem on boards with U-Boot which only provides
> GetVariable() and not SetVariable(). GRUB spits out warnings but works
> as expected.
>
> Best regards
>
> Heinrich



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux