From: "Daniel Walker (danielwa)" <danielwa@cisco.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Will Deacon <will@kernel.org>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
Rob Herring <robh@kernel.org>,
Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>,
Andrew Morton <akpm@linux-foundation.org>,
Pratyush Brahma <quic_pbrahma@quicinc.com>,
Tomas Mudrunka <tomas.mudrunka@gmail.com>,
Sean Anderson <sean.anderson@seco.com>,
"x86@kernel.org" <x86@kernel.org>,
"linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"xe-linux-external(mailer list)" <xe-linux-external@cisco.com>,
"linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
Date: Tue, 12 Dec 2023 17:25:50 +0000 [thread overview]
Message-ID: <ZXiXncjsKXzW5gBE@goliath> (raw)
In-Reply-To: <CAMj1kXFK-4mbNd_4La7Jhr_+ygBVyHdJm72vh7S41TGf2FYDdQ@mail.gmail.com>
On Tue, Dec 12, 2023 at 10:55:33AM +0100, Ard Biesheuvel wrote:
> On Fri, 10 Nov 2023 at 02:39, Daniel Walker <danielwa@cisco.com> wrote:
> >
> > This adds code to handle the generic command line changes.
> > The efi code appears that it doesn't benefit as much from this design
> > as it could.
> >
> > For example, if you had a prepend command line with "nokaslr" then
> > you might be helpful to re-enable it in the boot loader or dts,
> > but there appears to be no way to re-enable kaslr or some of the
> > other options.
> >
> > The efi command line handling is incorrect. x86 and arm have an append
> > system however the efi code prepends the command line.
> >
> > For example, you could have a non-upgradable bios which sends
> >
> > efi=disable_early_pci_dma
> >
> > This hypothetically could have been set because early pci dma caused
> > issues on early versions of the product.
> >
> > Then later the early pci dma was made to work and the company desired
> > to start using it. To override the bios you could set the CONFIG_CMDLINE
> > to,
> >
> > efi=no_disable_early_pci_dma
> >
> > then parsing would normally start with the bios command line, then move
> > to the CONFIG_CMDLINE and you would end up with early pci dma turned on.
> >
> > however, current efi code keeps early pci dma off because the bios
> > arguments always override the built in.
> >
> > Per my reading this is different from the main body of x86, arm, and
> > arm64.
> >
> > The generic command line provides both append and prepend, so it
> > alleviates this issue if it's used. However not all architectures use
> > it.
> >
> > It would be desirable to allow the efi stub to have it's builtin command
> > line to be modified after compile, but I don't see a feasible way to do
> > that currently.
> >
> > Cc: xe-linux-external@cisco.com
> > Signed-off-by: Daniel Walker <danielwa@cisco.com>
>
> There are quite some 'might be's and 'hypothetical's in this commit log.
>
> Is there an actual use case that you are addressing here? Without
> that, this looks like unnecessary churn to me, tbh.
>
> Note that this code executes in the context of platform firmware,
> which may be old and buggy so we should be cautious about making
> unnecessary changes here.
It's been a while since I wrote this patch description, but there is an example I
provided in the description. If you intend to enable early pci dma the command
line parsing may still disable it because of how the parsing works inside the
efi stub. There is an implementation difference between the efi stub and the
architectures which results in this problem. This was not an issue I observed in
the wild, it was the results of code review of the efi stub.
Maybe the greater issue is that the efi stub seems to have connected itself to
the kernels command line system. If the kernel changes then EFI stub must also
change.
Daniel
next prev parent reply other threads:[~2023-12-12 17:26 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-10 1:38 [PATCH 0/8] generic command line v6 Daniel Walker
2023-11-10 1:38 ` [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline Daniel Walker
2023-11-10 4:23 ` kernel test robot
2023-11-23 6:37 ` Christophe Leroy
2023-12-12 9:55 ` Ard Biesheuvel
2023-12-12 17:25 ` Daniel Walker (danielwa) [this message]
2023-11-10 1:51 ` [PATCH 0/8] generic command line v6 Andrew Morton
2023-11-10 2:22 ` Daniel Walker (danielwa)
2023-11-10 2:40 ` Andrew Morton
2023-11-23 6:23 ` Christophe Leroy
-- strict thread matches above, loose matches on Subject: below --
2022-09-29 2:32 [PATCH 0/8] generic command line v5 Daniel Walker
2022-09-29 2:32 ` [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline Daniel Walker
2021-04-16 4:09 [PATCH 0/8] generic command line v4 Daniel Walker
2021-04-16 4:09 ` [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline Daniel Walker
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=ZXiXncjsKXzW5gBE@goliath \
--to=danielwa@cisco.com \
--cc=akpm@linux-foundation.org \
--cc=ardb@kernel.org \
--cc=christophe.leroy@csgroup.eu \
--cc=daniel@gimpelevich.san-francisco.ca.us \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=quic_pbrahma@quicinc.com \
--cc=robh@kernel.org \
--cc=sean.anderson@seco.com \
--cc=tomas.mudrunka@gmail.com \
--cc=will@kernel.org \
--cc=x86@kernel.org \
--cc=xe-linux-external@cisco.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).