From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>,
Shawn Guo <shawn.guo@linaro.org>,
Atish Patra <atish.patra@wdc.com>,
Steve McIntyre <steve@einval.com>,
Rob Clark <robdclark@gmail.com>,
linux-efi <linux-efi@vger.kernel.org>,
Jeffrey Hugo <jhugo@codeaurora.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Leif Lindholm <leif@nuviainc.com>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>
Subject: Re: [PATCH] efi: stub: override RT_PROP table supported mask based on EFI variable
Date: Tue, 16 Mar 2021 10:27:04 +0200 [thread overview]
Message-ID: <YFBr2C36ogM25Jjb@enceladus> (raw)
In-Reply-To: <CAMj1kXEz++tPZO5Bzw9f+_PzzRoN7m6fXUi3kKTXvMANpzaKpg@mail.gmail.com>
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@linaro.org> 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@gmx.de> 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
next prev parent reply other threads:[~2021-03-16 8:27 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-06 11:35 [PATCH] efi: stub: override RT_PROP table supported mask based on EFI variable Ard Biesheuvel
2021-03-07 11:02 ` Shawn Guo
2021-03-08 13:34 ` Ard Biesheuvel
2021-03-09 3:22 ` Shawn Guo
2021-03-09 8:51 ` Ard Biesheuvel
2021-03-09 18:13 ` Rob Clark
2021-03-09 18:47 ` Ard Biesheuvel
2021-03-09 21:19 ` Rob Clark
2021-03-15 3:11 ` Shawn Guo
2021-03-15 13:07 ` Ard Biesheuvel
2021-03-16 7:42 ` Heinrich Schuchardt
2021-03-16 7:52 ` Ard Biesheuvel
2021-03-16 8:04 ` Ilias Apalodimas
2021-03-16 8:14 ` Ard Biesheuvel
2021-03-16 8:27 ` Ilias Apalodimas [this message]
2021-03-16 7:52 ` Shawn Guo
2021-03-16 7:57 ` Ard Biesheuvel
2021-03-16 9:06 ` Shawn Guo
2021-03-16 9:33 ` Ard Biesheuvel
2021-03-17 6:36 ` Shawn Guo
2021-03-17 6:58 ` Ard Biesheuvel
2021-03-16 9:33 ` Ilias Apalodimas
2021-03-16 13:25 ` Heinrich Schuchardt
2021-03-16 14:06 ` Ard Biesheuvel
2021-03-16 14:45 ` Heinrich Schuchardt
2021-03-16 14:55 ` Ard Biesheuvel
2021-03-16 16:06 ` Heinrich Schuchardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YFBr2C36ogM25Jjb@enceladus \
--to=ilias.apalodimas@linaro.org \
--cc=ardb@kernel.org \
--cc=atish.patra@wdc.com \
--cc=bjorn.andersson@linaro.org \
--cc=jhugo@codeaurora.org \
--cc=leif@nuviainc.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=robdclark@gmail.com \
--cc=shawn.guo@linaro.org \
--cc=steve@einval.com \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).