Re: [PATCH 3/3] efi/libstub: consider CONFIG_CMDLINE for initrd= and dtb= options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/12/24 9:36 AM, Ard Biesheuvel wrote:
On Sat, 12 Oct 2024 at 14:04, Jonathan Marek <jonathan@xxxxxxxx> wrote:



On 10/12/24 3:54 AM, Ard Biesheuvel wrote:
On Sat, 12 Oct 2024 at 00:52, Jonathan Marek <jonathan@xxxxxxxx> wrote:

Replace cmdline with CONFIG_CMDLINE when it should be used instead of
load_options.

In the EXTEND case, it may be necessary to combine both CONFIG_CMDLINE and
load_options. In that case, keep the old behavior and print a warning about
the incorrect behavior.


The core kernel has its own handling for EXTEND/FORCE, so while we
should parse it in the EFI stub to look for options that affect the
stub's own behavior, we should not copy it into the command line that
the stub provides to the core kernel.

E.g., drivers/of/fdt.c takes the bootargs from the DT and combines
them with CONFIG_CMDLINE.



I'm aware of that - the replacement the commit message is referring to
is specifically for handle_cmdline_files() which this commit is modifying.


Ah ok - I missed that.

This is the kind of context that I'd expect in a cover letter, i.e.,
that the command line handling is inconsistent, and that we obtain the
command line from the loaded image twice.

Also, the fact the initrd= handling and dtb= are special, because
a) multiple initrd=  arguments are processed in order, and the files
concatenated,
b) the filenames are consumed as UTF-16 as they are plugged into the
file I/O protocols


(not relevant to this commit, but I need to say that concatenating dtb files makes no sense, only the first one will be used by the kernel)

Currently efistub completely ignores initrd= and dtb= options provided
through CONFIG_CMDLINE (handle_cmdline_files() only parses the EFI options)


Indeed. You haven't explained why this is a problem. initrd= and dtb=
contain references to files in the file system, and this does not seem
like something CONFIG_EXTEND was intended for.


Its not the expected/documented behavior, that should be enough to make it a problem. Nowhere is it documented that these options would be ignored if provided through CONFIG_CMDLINE.

For the EXTEND case, I didn't implement the full solution because its
more complex and EXTEND is not available on arm64 anyway, so I went with
just printing a warning instead.

This code is shared between all architectures, so what arm64 does or
does not support is irrelevant.

Can you explain your use case please?


I boot linux as the "EFI/Boot/bootaa64.efi" on my EFI partition. The firmware does not provide any load options. This system needs a dtb, so I add the dtb to my EFI partition and configure it using the dtb= option (using CONFIG_CMDLINE).




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux