On Tue, Mar 16, 2021 at 09:14:22AM +0100, Ard Biesheuvel wrote: > On Tue, 16 Mar 2021 at 09:04, Ilias Apalodimas > <ilias.apalodimas@xxxxxxxxxx> wrote: > > > > Hi Ard, > > > > On Tue, Mar 16, 2021 at 08:52:52AM +0100, Ard Biesheuvel wrote: > > > On Tue, 16 Mar 2021 at 08:42, Heinrich Schuchardt <xypron.glpk@xxxxxx> wrote: > > > > > ... > > > > looking at this thread it is hard to understand why this patch should be > > > > needed. > > > > > > > > If an UEFI application does not want to consume a service, it can do so > > > > without having to manipulate the RT properties table. > > > > > > > > Which UEFI applications are broken? Why can't they be fixed instead of > > > > patching the kernel? > > > > > > > > Can we have complete descriptions of the deficiencies of the involved > > > > applications. I saw GRUB and the Debian installer mentioned in the > > > > thread. Are there others? > > > > > > > > > > The problem is that the proprietary EDK2 / UEFI firmware on Qualcomm > > > Snapdragon based laptops that were built to run Windows does not > > > implement get/setvariable after ExitBootServices. Instead, every call > > > to any of the variable services returns with an EFI_UNSUPPORTED error. > > > > > > The correct way to address this is a RT_PROP table that encodes this > > > behavior, and this is what we added in the special DtbLoader driver > > > that is used to boot Linux in DT mode (as the firmware only implements > > > ACPI support). So for systems that can/will run DtbLoader, the problem > > > is solved. > > > > > > What remains is ACPI boot, or boot modes where DtbLoader does not > > > work. In those cases, it would be useful to have another way to convey > > > this information to the OS in a way that does not rely on the kernel > > > command line. > > > > > > But thinking about this, perhaps we should be fixing this in > > > efibootmgr instead. EFI_UNSUPPORTED is a valid and documented return > > > code that conveys that the operation did not fail with an error, but > > > that efibootmgr is not supported to begin with on the platform in > > > question. > > > > It all depends on how smart we want to make the efi stub. In essence > > it's an OS loader, that we have complete control over and we can play tricks > > on broken/incompatible firmwares, but is that what we want ? And if yes, were > > do we draw the line of what we fix or not? > > > > I think the current problem doesn't make a strong case to add such > > functionality. U-Boot doesn't expose SetVariable at all, but even if it did > > and returned EFI_UNSUPPORTED, I'd expect the consuming applications to handle > > the error gracefully. I mean why should we treat EFI_UNSUPPORTED differently > > than EFI_DEVICE_ERROR or EFI_INVALID_PARAMETER (or all the allowed return > > codes)? > > > > EFI_DEVICE_ERROR or EFI_INVALID_PARAMETER means that the particular > call resulted in an error, which may be related to the values of the > arguments, the state of the the flash, etc etc > > EFI_UNSUPPORTED means that the platform in question does not support > the routine at all at runtime, and the arguments or the context is > irrelevant. By differently I implied 'not handle the error correctly'. So my point was that an application must handle all errors that are allowed from the spec. Not select the ones it prefers in a meaningfull way. Which brings us to your next point. > > Given that GRUB already tolerates the second condition, but only if it > is communicated explicitly (via --no-nvram) or implicitly when > efivarfs is absent altogether, I am saying that we should classify a > EFI_UNSUPPORTED return value in the same way, and tolerate it rather > than abort the install. +1 Thanks /Ilias
