On 16.03.21 15:55, Ard Biesheuvel wrote: > 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. The installer should simply open a message box asking the user to set up a boot option for shimaa64.efi (or grubaa64.efi). This is nothing that can be fixed in the Linux kernel. Best regards Heinrich > >> 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