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
