* [PATCH 0/8] generic command line v5
@ 2022-09-29 2:32 Daniel Walker
2022-09-29 2:32 ` [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline Daniel Walker
2023-01-24 16:30 ` [PATCH 0/8] generic command line v5 Sean Anderson
0 siblings, 2 replies; 9+ messages in thread
From: Daniel Walker @ 2022-09-29 2:32 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, Sean Anderson, H. Peter Anvin, Nick Desaulniers,
linux-arm-kernel, linux-mips, linux-kernel, devicetree,
linux-kbuild
Cc: xe-linux-external, linux-efi
v5 release changes. Generally a rebase from v4.
* Modified OF changes to move the ugly code into the cmdline.h
* Minor compliation update in arm64. Added ifndef __ASSEMBLY__ in
a few places.
* Worked around arm64 kaslr_early.c.
This code needs some additional review and consideration.
It appears this code is missing the opposite option to nokaslr
which is kaslr.disabled=1/0 which would allow kaslr to be turn
back on later in the command line. For example,
console=ttyS0 nokaslr root=/dev/ram0 nosmp kaslr.disabled=0 loglevel=7
On arm64 in arch/arm64/kernel/idreg-override.c this is parsed correctly
to turn on kaslr, but the kaslr_early.c is missing this logic.
Doing this results in kaslr getting disabled with the following message,
KASLR disabled due to lack of seed
Even when there is a seed in the device tree.
So change to the generic command line would leave built in command
lines with nokaslr with no option to re-enable kaslr in in the bootloader
arguments.
Daniel Walker (8):
CMDLINE: add generic builtin command line
scripts: insert-sys-cert: add command line insert capability
scripts: insert-sys-cert: change name to insert-symbol
CMDLINE: mips: convert to generic builtin command line
drivers: firmware: efi: libstub: enable generic commandline
CMDLINE: x86: convert to generic builtin command line
of: replace command line handling
CMDLINE: arm64: convert to generic builtin command line
arch/arm64/Kconfig | 33 +--
arch/arm64/include/asm/setup.h | 4 +
arch/arm64/include/uapi/asm/setup.h | 2 +
arch/arm64/kernel/idreg-override.c | 9 +-
arch/arm64/kernel/pi/kaslr_early.c | 14 +-
arch/mips/Kconfig | 4 +-
arch/mips/Kconfig.debug | 44 ----
arch/mips/configs/ar7_defconfig | 9 +-
arch/mips/configs/bcm47xx_defconfig | 8 +-
arch/mips/configs/bcm63xx_defconfig | 15 +-
arch/mips/configs/bmips_be_defconfig | 11 +-
arch/mips/configs/bmips_stb_defconfig | 6 +-
arch/mips/configs/ci20_defconfig | 9 +-
arch/mips/configs/cu1000-neo_defconfig | 10 +-
arch/mips/configs/cu1830-neo_defconfig | 10 +-
arch/mips/configs/generic_defconfig | 6 +-
arch/mips/configs/gpr_defconfig | 18 +-
arch/mips/configs/loongson3_defconfig | 12 +-
arch/mips/include/asm/setup.h | 2 +
arch/mips/kernel/relocate.c | 17 +-
arch/mips/kernel/setup.c | 36 +--
arch/mips/pic32/pic32mzda/early_console.c | 2 +-
arch/mips/pic32/pic32mzda/init.c | 3 +-
arch/x86/Kconfig | 44 +---
arch/x86/kernel/setup.c | 18 +-
.../firmware/efi/libstub/efi-stub-helper.c | 29 +++
drivers/firmware/efi/libstub/efi-stub.c | 9 +
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 13 +-
drivers/of/fdt.c | 22 +-
include/linux/cmdline.h | 137 ++++++++++
init/Kconfig | 78 ++++++
lib/Kconfig | 4 +
lib/Makefile | 3 +
lib/generic_cmdline.S | 53 ++++
lib/test_cmdline1.c | 139 ++++++++++
scripts/Makefile | 2 +-
.../{insert-sys-cert.c => insert-symbol.c} | 243 ++++++++++++------
38 files changed, 724 insertions(+), 355 deletions(-)
create mode 100644 include/linux/cmdline.h
create mode 100644 lib/generic_cmdline.S
create mode 100644 lib/test_cmdline1.c
rename scripts/{insert-sys-cert.c => insert-symbol.c} (72%)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
2022-09-29 2:32 [PATCH 0/8] generic command line v5 Daniel Walker
@ 2022-09-29 2:32 ` Daniel Walker
2023-01-24 16:30 ` [PATCH 0/8] generic command line v5 Sean Anderson
1 sibling, 0 replies; 9+ messages in thread
From: Daniel Walker @ 2022-09-29 2:32 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, Sean Anderson
Cc: xe-linux-external, Ard Biesheuvel, linux-efi, linux-kernel
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>
---
.../firmware/efi/libstub/efi-stub-helper.c | 29 +++++++++++++++++++
drivers/firmware/efi/libstub/efi-stub.c | 9 ++++++
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 13 +++++++--
4 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index 3d972061c1b0..372f06ec8c49 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -13,6 +13,7 @@
#include <linux/efi.h>
#include <linux/kernel.h>
#include <linux/printk.h> /* For CONSOLE_LOGLEVEL_* */
+#include <linux/cmdline.h>
#include <asm/efi.h>
#include <asm/setup.h>
@@ -172,6 +173,34 @@ int efi_printk(const char *fmt, ...)
return printed;
}
+/**
+ * efi_handle_cmdline() - handle adding in building parts of the command line
+ * @cmdline: kernel command line
+ *
+ * Add in the generic parts of the commandline and start the parsing of the
+ * command line.
+ *
+ * Return: status code
+ */
+efi_status_t efi_handle_cmdline(char const *cmdline)
+{
+ efi_status_t status = EFI_SUCCESS;
+
+ if (sizeof(CMDLINE_STATIC_PREPEND) > 1)
+ status |= efi_parse_options(CMDLINE_STATIC_PREPEND);
+
+ if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
+ status |= efi_parse_options(cmdline);
+
+ if (sizeof(CMDLINE_STATIC_APPEND) > 1)
+ status |= efi_parse_options(CMDLINE_STATIC_APPEND);
+
+ if (status != EFI_SUCCESS)
+ efi_err("Failed to parse options\n");
+
+ return status;
+}
+
/**
* efi_parse_options() - Parse EFI command line options
* @cmdline: kernel command line
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index f515394cce6e..ea52ebd99ea6 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -173,6 +173,14 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
goto fail;
}
+#ifdef CONFIG_GENERIC_CMDLINE
+ status = efi_handle_cmdline(cmdline_ptr);
+ if (status != EFI_SUCCESS) {
+ goto fail_free_cmdline;
+ }
+#endif
+
+#ifdef CONFIG_CMDLINE
if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
cmdline_size == 0) {
@@ -190,6 +198,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
goto fail_free_cmdline;
}
}
+#endif
efi_info("Booting Linux Kernel...\n");
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index b0ae0a454404..44c2e1194378 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -907,6 +907,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
unsigned long alignment,
unsigned long min_addr);
+efi_status_t efi_handle_cmdline(char const *cmdline);
efi_status_t efi_parse_options(char const *cmdline);
void efi_parse_option_graphics(char *option);
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 7a7abc8959d2..87196bb9c3bb 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -783,6 +783,8 @@ unsigned long efi_main(efi_handle_t handle,
unsigned long buffer_start, buffer_end;
struct setup_header *hdr = &boot_params->hdr;
unsigned long addr, size;
+ unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
+ ((u64)boot_params->ext_cmd_line_ptr << 32));
efi_status_t status;
efi_system_table = sys_table_arg;
@@ -851,6 +853,14 @@ unsigned long efi_main(efi_handle_t handle,
image_offset = 0;
}
+#ifdef CONFIG_GENERIC_CMDLINE
+ status = efi_handle_cmdline((char *)cmdline_paddr);
+ if (status != EFI_SUCCESS) {
+ efi_err("Failed to parse options\n");
+ goto fail;
+ }
+#else /* CONFIG_GENERIC_CMDLINE */
+
#ifdef CONFIG_CMDLINE_BOOL
status = efi_parse_options(CONFIG_CMDLINE);
if (status != EFI_SUCCESS) {
@@ -859,8 +869,6 @@ unsigned long efi_main(efi_handle_t handle,
}
#endif
if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
- unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
- ((u64)boot_params->ext_cmd_line_ptr << 32));
status = efi_parse_options((char *)cmdline_paddr);
if (status != EFI_SUCCESS) {
efi_err("Failed to parse options\n");
@@ -868,6 +876,7 @@ unsigned long efi_main(efi_handle_t handle,
}
}
+#endif
/*
* At this point, an initrd may already have been loaded by the
* bootloader and passed via bootparams. We permit an initrd loaded
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/8] generic command line v5
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
@ 2023-01-24 16:30 ` Sean Anderson
1 sibling, 0 replies; 9+ messages in thread
From: Sean Anderson @ 2023-01-24 16:30 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, Christophe Leroy, Rob Herring,
Daniel Gimpelevich, Andrew Morton, H. Peter Anvin,
Nick Desaulniers, linux-arm-kernel, linux-mips, linux-kernel,
devicetree, linux-kbuild
Cc: xe-linux-external, linux-efi
On 9/28/22 22:32, Daniel Walker wrote:
> v5 release changes. Generally a rebase from v4.
>
> * Modified OF changes to move the ugly code into the cmdline.h
>
> * Minor compliation update in arm64. Added ifndef __ASSEMBLY__ in
> a few places.
>
> * Worked around arm64 kaslr_early.c.
>
> This code needs some additional review and consideration.
> It appears this code is missing the opposite option to nokaslr
> which is kaslr.disabled=1/0 which would allow kaslr to be turn
> back on later in the command line. For example,
>
> console=ttyS0 nokaslr root=/dev/ram0 nosmp kaslr.disabled=0 loglevel=7
>
> On arm64 in arch/arm64/kernel/idreg-override.c this is parsed correctly
> to turn on kaslr, but the kaslr_early.c is missing this logic.
> Doing this results in kaslr getting disabled with the following message,
>
> KASLR disabled due to lack of seed
>
> Even when there is a seed in the device tree.
>
> So change to the generic command line would leave built in command
> lines with nokaslr with no option to re-enable kaslr in in the bootloader
> arguments.
>
>
> Daniel Walker (8):
> CMDLINE: add generic builtin command line
> scripts: insert-sys-cert: add command line insert capability
> scripts: insert-sys-cert: change name to insert-symbol
> CMDLINE: mips: convert to generic builtin command line
> drivers: firmware: efi: libstub: enable generic commandline
> CMDLINE: x86: convert to generic builtin command line
> of: replace command line handling
> CMDLINE: arm64: convert to generic builtin command line
>
> arch/arm64/Kconfig | 33 +--
> arch/arm64/include/asm/setup.h | 4 +
> arch/arm64/include/uapi/asm/setup.h | 2 +
> arch/arm64/kernel/idreg-override.c | 9 +-
> arch/arm64/kernel/pi/kaslr_early.c | 14 +-
> arch/mips/Kconfig | 4 +-
> arch/mips/Kconfig.debug | 44 ----
> arch/mips/configs/ar7_defconfig | 9 +-
> arch/mips/configs/bcm47xx_defconfig | 8 +-
> arch/mips/configs/bcm63xx_defconfig | 15 +-
> arch/mips/configs/bmips_be_defconfig | 11 +-
> arch/mips/configs/bmips_stb_defconfig | 6 +-
> arch/mips/configs/ci20_defconfig | 9 +-
> arch/mips/configs/cu1000-neo_defconfig | 10 +-
> arch/mips/configs/cu1830-neo_defconfig | 10 +-
> arch/mips/configs/generic_defconfig | 6 +-
> arch/mips/configs/gpr_defconfig | 18 +-
> arch/mips/configs/loongson3_defconfig | 12 +-
> arch/mips/include/asm/setup.h | 2 +
> arch/mips/kernel/relocate.c | 17 +-
> arch/mips/kernel/setup.c | 36 +--
> arch/mips/pic32/pic32mzda/early_console.c | 2 +-
> arch/mips/pic32/pic32mzda/init.c | 3 +-
> arch/x86/Kconfig | 44 +---
> arch/x86/kernel/setup.c | 18 +-
> .../firmware/efi/libstub/efi-stub-helper.c | 29 +++
> drivers/firmware/efi/libstub/efi-stub.c | 9 +
> drivers/firmware/efi/libstub/efistub.h | 1 +
> drivers/firmware/efi/libstub/x86-stub.c | 13 +-
> drivers/of/fdt.c | 22 +-
> include/linux/cmdline.h | 137 ++++++++++
> init/Kconfig | 78 ++++++
> lib/Kconfig | 4 +
> lib/Makefile | 3 +
> lib/generic_cmdline.S | 53 ++++
> lib/test_cmdline1.c | 139 ++++++++++
> scripts/Makefile | 2 +-
> .../{insert-sys-cert.c => insert-symbol.c} | 243 ++++++++++++------
> 38 files changed, 724 insertions(+), 355 deletions(-)
> create mode 100644 include/linux/cmdline.h
> create mode 100644 lib/generic_cmdline.S
> create mode 100644 lib/test_cmdline1.c
> rename scripts/{insert-sys-cert.c => insert-symbol.c} (72%)
>
For arm64:
Tested-by: Sean Anderson <sean.anderson@seco.com>
Thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/8] generic command line v6
@ 2023-11-10 1:38 Daniel Walker
2023-11-10 1:38 ` [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline Daniel Walker
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Walker @ 2023-11-10 1:38 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, Pratyush Brahma, Tomas Mudrunka, Sean Anderson,
x86, linux-mips, linuxppc-dev, H. Peter Anvin, Nathan Chancellor,
Nick Desaulniers, Nicolas Schier, linux-arm-kernel, linux-kernel,
devicetree, linux-kbuild
Cc: linux-efi
This release is an up-rev of the v5 patches. No additional features have
been added. Some changes were mode to function names and some changes to
Kconfig dependencies. Also updated the config conversion for mips.
There are a number of people who have expressed interest in these
patches either by asking for them to be merge or testing them. If
people are so inclined please continue to request them to be merge
or to ask the status of the next release. It's helpful to motivate me to
release them again and for the maintainers to see the interest
generated.
These patches have been used by Cisco Systems, Inc. on millions of
released products to great effect. Hopefully they can be used by the
entire Linux eco system.
My apologies on the length between releases. I will try to release more
often.
Daniel Walker (8):
CMDLINE: add generic builtin command line
scripts: insert-sys-cert: add command line insert capability
scripts: insert-sys-cert: change name to insert-symbol
CMDLINE: mips: convert to generic builtin command line
drivers: firmware: efi: libstub: enable generic commandline
CMDLINE: x86: convert to generic builtin command line
of: replace command line handling
CMDLINE: arm64: convert to generic builtin command line
arch/arm64/Kconfig | 33 +--
arch/arm64/include/asm/setup.h | 4 +
arch/arm64/include/uapi/asm/setup.h | 2 +
arch/arm64/kernel/idreg-override.c | 9 +-
arch/arm64/kernel/pi/kaslr_early.c | 14 +-
arch/mips/Kconfig | 4 +-
arch/mips/Kconfig.debug | 44 ----
arch/mips/configs/ar7_defconfig | 12 +-
arch/mips/configs/bcm47xx_defconfig | 10 +-
arch/mips/configs/bcm63xx_defconfig | 21 +-
arch/mips/configs/bmips_be_defconfig | 17 +-
arch/mips/configs/bmips_stb_defconfig | 139 ++++------
arch/mips/configs/ci20_defconfig | 8 +-
arch/mips/configs/cu1000-neo_defconfig | 19 +-
arch/mips/configs/cu1830-neo_defconfig | 19 +-
arch/mips/configs/generic_defconfig | 15 +-
arch/mips/configs/gpr_defconfig | 33 +--
arch/mips/configs/loongson3_defconfig | 29 +--
arch/mips/include/asm/setup.h | 2 +
arch/mips/kernel/relocate.c | 17 +-
arch/mips/kernel/setup.c | 36 +--
arch/mips/pic32/pic32mzda/early_console.c | 2 +-
arch/mips/pic32/pic32mzda/init.c | 3 +-
arch/x86/Kconfig | 44 +---
arch/x86/kernel/setup.c | 18 +-
.../firmware/efi/libstub/efi-stub-helper.c | 29 +++
drivers/firmware/efi/libstub/efi-stub.c | 9 +
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 14 +-
drivers/of/fdt.c | 22 +-
include/linux/cmdline.h | 137 ++++++++++
init/Kconfig | 79 ++++++
lib/Kconfig | 4 +
lib/Makefile | 3 +
lib/generic_cmdline.S | 53 ++++
lib/test_cmdline1.c | 139 ++++++++++
scripts/Makefile | 2 +-
.../{insert-sys-cert.c => insert-symbol.c} | 243 ++++++++++++------
38 files changed, 807 insertions(+), 482 deletions(-)
create mode 100644 include/linux/cmdline.h
create mode 100644 lib/generic_cmdline.S
create mode 100644 lib/test_cmdline1.c
rename scripts/{insert-sys-cert.c => insert-symbol.c} (72%)
--
2.39.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
2023-11-10 1:38 [PATCH 0/8] generic command line v6 Daniel Walker
@ 2023-11-10 1:38 ` Daniel Walker
2023-11-10 4:23 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Daniel Walker @ 2023-11-10 1:38 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, Pratyush Brahma, Tomas Mudrunka, Sean Anderson,
x86, linux-mips, linuxppc-dev
Cc: xe-linux-external, Ard Biesheuvel, linux-efi, linux-kernel
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>
---
.../firmware/efi/libstub/efi-stub-helper.c | 29 +++++++++++++++++++
drivers/firmware/efi/libstub/efi-stub.c | 9 ++++++
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 14 +++++++--
4 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index bfa30625f5d0..952fa2cdff51 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -11,6 +11,7 @@
#include <linux/efi.h>
#include <linux/kernel.h>
+#include <linux/cmdline.h>
#include <asm/efi.h>
#include <asm/setup.h>
@@ -29,6 +30,34 @@ bool __pure __efi_soft_reserve_enabled(void)
return !efi_nosoftreserve;
}
+/**
+ * efi_handle_cmdline() - handle adding in built-in parts of the command line
+ * @cmdline: kernel command line
+ *
+ * Add in the generic parts of the commandline and start the parsing of the
+ * command line.
+ *
+ * Return: status code
+ */
+efi_status_t efi_handle_builtin_cmdline(char const *cmdline)
+{
+ efi_status_t status = EFI_SUCCESS;
+
+ if (sizeof(CMDLINE_STATIC_PREPEND) > 1)
+ status |= efi_parse_options(CMDLINE_STATIC_PREPEND);
+
+ if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
+ status |= efi_parse_options(cmdline);
+
+ if (sizeof(CMDLINE_STATIC_APPEND) > 1)
+ status |= efi_parse_options(CMDLINE_STATIC_APPEND);
+
+ if (status != EFI_SUCCESS)
+ efi_err("Failed to parse options\n");
+
+ return status;
+}
+
/**
* efi_parse_options() - Parse EFI command line options
* @cmdline: kernel command line
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index f9c1e8a2bd1d..770abe95c0ee 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -127,6 +127,14 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
return EFI_OUT_OF_RESOURCES;
}
+#ifdef CONFIG_GENERIC_CMDLINE
+ status = efi_handle_builtin_cmdline(cmdline);
+ if (status != EFI_SUCCESS) {
+ goto fail_free_cmdline;
+ }
+#endif
+
+#ifdef CONFIG_CMDLINE
if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
cmdline_size == 0) {
@@ -144,6 +152,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
goto fail_free_cmdline;
}
}
+#endif
*cmdline_ptr = cmdline;
return EFI_SUCCESS;
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 212687c30d79..1ac6631905c5 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -996,6 +996,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
unsigned long alignment,
unsigned long min_addr);
+efi_status_t efi_handle_builtin_cmdline(char const *cmdline);
efi_status_t efi_parse_options(char const *cmdline);
void efi_parse_option_graphics(char *option);
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 9d5df683f882..273a8a9c8bbb 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -847,6 +847,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
struct setup_header *hdr = &boot_params->hdr;
const struct linux_efi_initrd *initrd = NULL;
unsigned long kernel_entry;
+ unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
+ ((u64)boot_params->ext_cmd_line_ptr << 32));
efi_status_t status;
boot_params_pointer = boot_params;
@@ -877,6 +879,14 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
goto fail;
}
+#ifdef CONFIG_GENERIC_CMDLINE
+ status = efi_handle_builtin_cmdline((char *)cmdline_paddr);
+ if (status != EFI_SUCCESS) {
+ efi_err("Failed to parse options\n");
+ goto fail;
+ }
+#else /* CONFIG_GENERIC_CMDLINE */
+
#ifdef CONFIG_CMDLINE_BOOL
status = efi_parse_options(CONFIG_CMDLINE);
if (status != EFI_SUCCESS) {
@@ -885,8 +895,6 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
}
#endif
if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
- unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
- ((u64)boot_params->ext_cmd_line_ptr << 32));
status = efi_parse_options((char *)cmdline_paddr);
if (status != EFI_SUCCESS) {
efi_err("Failed to parse options\n");
@@ -894,6 +902,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
}
}
+#endif
+
status = efi_decompress_kernel(&kernel_entry);
if (status != EFI_SUCCESS) {
efi_err("Failed to decompress kernel\n");
--
2.39.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
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
2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2023-11-10 4:23 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, Christophe Leroy, Rob Herring,
Daniel Gimpelevich, Andrew Morton, Pratyush Brahma,
Tomas Mudrunka, Sean Anderson, x86, linux-mips, linuxppc-dev
Cc: oe-kbuild-all, Linux Memory Management List, xe-linux-external,
Ard Biesheuvel, linux-efi, linux-kernel
Hi Daniel,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.6]
[cannot apply to arm64/for-next/core efi/next tip/x86/core robh/for-next linus/master next-20231110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Daniel-Walker/CMDLINE-add-generic-builtin-command-line/20231110-094423
base: v6.6
patch link: https://lore.kernel.org/r/20231110013817.2378507-6-danielwa%40cisco.com
patch subject: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
config: loongarch-randconfig-002-20231110 (https://download.01.org/0day-ci/archive/20231110/202311101224.evyh4zgY-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231110/202311101224.evyh4zgY-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311101224.evyh4zgY-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/firmware/efi/libstub/efi-stub-helper.c:43: warning: expecting prototype for efi_handle_cmdline(). Prototype was for efi_handle_builtin_cmdline() instead
drivers/firmware/efi/libstub/efi-stub-helper.c:592: warning: Function parameter or member 'out' not described in 'efi_load_initrd'
vim +43 drivers/firmware/efi/libstub/efi-stub-helper.c
32
33 /**
34 * efi_handle_cmdline() - handle adding in built-in parts of the command line
35 * @cmdline: kernel command line
36 *
37 * Add in the generic parts of the commandline and start the parsing of the
38 * command line.
39 *
40 * Return: status code
41 */
42 efi_status_t efi_handle_builtin_cmdline(char const *cmdline)
> 43 {
44 efi_status_t status = EFI_SUCCESS;
45
46 if (sizeof(CMDLINE_STATIC_PREPEND) > 1)
47 status |= efi_parse_options(CMDLINE_STATIC_PREPEND);
48
49 if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
50 status |= efi_parse_options(cmdline);
51
52 if (sizeof(CMDLINE_STATIC_APPEND) > 1)
53 status |= efi_parse_options(CMDLINE_STATIC_APPEND);
54
55 if (status != EFI_SUCCESS)
56 efi_err("Failed to parse options\n");
57
58 return status;
59 }
60
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
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
2 siblings, 0 replies; 9+ messages in thread
From: Christophe Leroy @ 2023-11-23 6:37 UTC (permalink / raw)
To: Daniel Walker, Will Deacon, Rob Herring, Daniel Gimpelevich,
Andrew Morton, Pratyush Brahma, Tomas Mudrunka, Sean Anderson,
x86, linux-mips, linuxppc-dev
Cc: xe-linux-external, Ard Biesheuvel, linux-efi, linux-kernel
Le 10/11/2023 à 02:38, Daniel Walker a écrit :
> 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.
So what can we do to improve that ?
>
> 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,
s/you/it
> 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.
Would be desirable or is desirable ? Your explanations are unclear.
>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
> .../firmware/efi/libstub/efi-stub-helper.c | 29 +++++++++++++++++++
> drivers/firmware/efi/libstub/efi-stub.c | 9 ++++++
> drivers/firmware/efi/libstub/efistub.h | 1 +
> drivers/firmware/efi/libstub/x86-stub.c | 14 +++++++--
> 4 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index bfa30625f5d0..952fa2cdff51 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -11,6 +11,7 @@
>
> #include <linux/efi.h>
> #include <linux/kernel.h>
> +#include <linux/cmdline.h>
> #include <asm/efi.h>
> #include <asm/setup.h>
>
> @@ -29,6 +30,34 @@ bool __pure __efi_soft_reserve_enabled(void)
> return !efi_nosoftreserve;
> }
>
> +/**
> + * efi_handle_cmdline() - handle adding in built-in parts of the command line
> + * @cmdline: kernel command line
> + *
> + * Add in the generic parts of the commandline and start the parsing of the
> + * command line.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_handle_builtin_cmdline(char const *cmdline)
> +{
> + efi_status_t status = EFI_SUCCESS;
> +
> + if (sizeof(CMDLINE_STATIC_PREPEND) > 1)
> + status |= efi_parse_options(CMDLINE_STATIC_PREPEND);
> +
> + if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
> + status |= efi_parse_options(cmdline);
> +
> + if (sizeof(CMDLINE_STATIC_APPEND) > 1)
> + status |= efi_parse_options(CMDLINE_STATIC_APPEND);
> +
> + if (status != EFI_SUCCESS)
> + efi_err("Failed to parse options\n");
> +
> + return status;
> +}
> +
> /**
> * efi_parse_options() - Parse EFI command line options
> * @cmdline: kernel command line
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index f9c1e8a2bd1d..770abe95c0ee 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -127,6 +127,14 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
> return EFI_OUT_OF_RESOURCES;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_builtin_cmdline(cmdline);
> + if (status != EFI_SUCCESS) {
> + goto fail_free_cmdline;
> + }
> +#endif
> +
> +#ifdef CONFIG_CMDLINE
> if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
> IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
> cmdline_size == 0) {
> @@ -144,6 +152,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
> goto fail_free_cmdline;
> }
> }
> +#endif
>
> *cmdline_ptr = cmdline;
> return EFI_SUCCESS;
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 212687c30d79..1ac6631905c5 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -996,6 +996,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
> unsigned long alignment,
> unsigned long min_addr);
>
> +efi_status_t efi_handle_builtin_cmdline(char const *cmdline);
> efi_status_t efi_parse_options(char const *cmdline);
>
> void efi_parse_option_graphics(char *option);
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 9d5df683f882..273a8a9c8bbb 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -847,6 +847,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> struct setup_header *hdr = &boot_params->hdr;
> const struct linux_efi_initrd *initrd = NULL;
> unsigned long kernel_entry;
> + unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> + ((u64)boot_params->ext_cmd_line_ptr << 32));
> efi_status_t status;
>
> boot_params_pointer = boot_params;
> @@ -877,6 +879,14 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> goto fail;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_builtin_cmdline((char *)cmdline_paddr);
> + if (status != EFI_SUCCESS) {
> + efi_err("Failed to parse options\n");
> + goto fail;
> + }
> +#else /* CONFIG_GENERIC_CMDLINE */
> +
> #ifdef CONFIG_CMDLINE_BOOL
> status = efi_parse_options(CONFIG_CMDLINE);
> if (status != EFI_SUCCESS) {
> @@ -885,8 +895,6 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> }
> #endif
> if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
> - unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> - ((u64)boot_params->ext_cmd_line_ptr << 32));
> status = efi_parse_options((char *)cmdline_paddr);
> if (status != EFI_SUCCESS) {
> efi_err("Failed to parse options\n");
> @@ -894,6 +902,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> }
> }
>
> +#endif
> +
> status = efi_decompress_kernel(&kernel_entry);
> if (status != EFI_SUCCESS) {
> efi_err("Failed to decompress kernel\n");
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
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)
2 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2023-12-12 9:55 UTC (permalink / raw)
To: Daniel Walker
Cc: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, Pratyush Brahma, Tomas Mudrunka, Sean Anderson,
x86, linux-mips, linuxppc-dev, xe-linux-external, linux-efi,
linux-kernel
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.
> ---
> .../firmware/efi/libstub/efi-stub-helper.c | 29 +++++++++++++++++++
> drivers/firmware/efi/libstub/efi-stub.c | 9 ++++++
> drivers/firmware/efi/libstub/efistub.h | 1 +
> drivers/firmware/efi/libstub/x86-stub.c | 14 +++++++--
> 4 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index bfa30625f5d0..952fa2cdff51 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -11,6 +11,7 @@
>
> #include <linux/efi.h>
> #include <linux/kernel.h>
> +#include <linux/cmdline.h>
> #include <asm/efi.h>
> #include <asm/setup.h>
>
> @@ -29,6 +30,34 @@ bool __pure __efi_soft_reserve_enabled(void)
> return !efi_nosoftreserve;
> }
>
> +/**
> + * efi_handle_cmdline() - handle adding in built-in parts of the command line
> + * @cmdline: kernel command line
> + *
> + * Add in the generic parts of the commandline and start the parsing of the
> + * command line.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_handle_builtin_cmdline(char const *cmdline)
> +{
> + efi_status_t status = EFI_SUCCESS;
> +
> + if (sizeof(CMDLINE_STATIC_PREPEND) > 1)
> + status |= efi_parse_options(CMDLINE_STATIC_PREPEND);
> +
> + if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
> + status |= efi_parse_options(cmdline);
> +
> + if (sizeof(CMDLINE_STATIC_APPEND) > 1)
> + status |= efi_parse_options(CMDLINE_STATIC_APPEND);
> +
> + if (status != EFI_SUCCESS)
> + efi_err("Failed to parse options\n");
> +
> + return status;
> +}
> +
> /**
> * efi_parse_options() - Parse EFI command line options
> * @cmdline: kernel command line
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index f9c1e8a2bd1d..770abe95c0ee 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -127,6 +127,14 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
> return EFI_OUT_OF_RESOURCES;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_builtin_cmdline(cmdline);
> + if (status != EFI_SUCCESS) {
> + goto fail_free_cmdline;
> + }
> +#endif
> +
> +#ifdef CONFIG_CMDLINE
> if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
> IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
> cmdline_size == 0) {
> @@ -144,6 +152,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
> goto fail_free_cmdline;
> }
> }
> +#endif
>
> *cmdline_ptr = cmdline;
> return EFI_SUCCESS;
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 212687c30d79..1ac6631905c5 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -996,6 +996,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
> unsigned long alignment,
> unsigned long min_addr);
>
> +efi_status_t efi_handle_builtin_cmdline(char const *cmdline);
> efi_status_t efi_parse_options(char const *cmdline);
>
> void efi_parse_option_graphics(char *option);
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 9d5df683f882..273a8a9c8bbb 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -847,6 +847,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> struct setup_header *hdr = &boot_params->hdr;
> const struct linux_efi_initrd *initrd = NULL;
> unsigned long kernel_entry;
> + unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> + ((u64)boot_params->ext_cmd_line_ptr << 32));
> efi_status_t status;
>
> boot_params_pointer = boot_params;
> @@ -877,6 +879,14 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> goto fail;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_builtin_cmdline((char *)cmdline_paddr);
> + if (status != EFI_SUCCESS) {
> + efi_err("Failed to parse options\n");
> + goto fail;
> + }
> +#else /* CONFIG_GENERIC_CMDLINE */
> +
> #ifdef CONFIG_CMDLINE_BOOL
> status = efi_parse_options(CONFIG_CMDLINE);
> if (status != EFI_SUCCESS) {
> @@ -885,8 +895,6 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> }
> #endif
> if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
> - unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> - ((u64)boot_params->ext_cmd_line_ptr << 32));
> status = efi_parse_options((char *)cmdline_paddr);
> if (status != EFI_SUCCESS) {
> efi_err("Failed to parse options\n");
> @@ -894,6 +902,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> }
> }
>
> +#endif
> +
> status = efi_decompress_kernel(&kernel_entry);
> if (status != EFI_SUCCESS) {
> efi_err("Failed to decompress kernel\n");
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
2023-12-12 9:55 ` Ard Biesheuvel
@ 2023-12-12 17:25 ` Daniel Walker (danielwa)
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Walker (danielwa) @ 2023-12-12 17:25 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, Pratyush Brahma, Tomas Mudrunka, Sean Anderson,
x86, linux-mips, linuxppc-dev, xe-linux-external(mailer list),
linux-efi, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/8] generic command line v4
@ 2021-04-16 4:09 Daniel Walker
2021-04-16 4:09 ` [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline Daniel Walker
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Walker @ 2021-04-16 4:09 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev, H. Peter Anvin,
linux-arm-kernel, linux-kernel, devicetree, linux-kbuild
Cc: linux-efi
v4 release changes
* Updated insert-sys-cert tool to change command line symbols after
compilation.
This tool is used to release binary kernels internally to companies
and then later insert certificates for each product by consumers of
the binary kernel. Cisco uses this tool for this purpose.
Cisco has a similar need for the command line to be modified on a
binary released kernels similar to how certificates are setup.
* Added global symbols to hold append and prepend values.
These changes follow the system certificate code to allow the
insert-sys-cert tool to be used.
* Added a test case to confirm functionality.
Seemed sensible to add this to make sure everything is working.
* Dropped powerpc changes
Christophe Leroy has reservations about the features for powerpc. I
don't think his reservations are founded, and these changes should
fully work on powerpc. However, I dropped these changes so Christophe
can have more time to get comfortable with the changes.
Enjoy!
Daniel Walker (8):
CMDLINE: add generic builtin command line
scripts: insert-sys-cert: add command line insert capability
scripts: insert-sys-cert: change name to insert-symbol
CMDLINE: mips: convert to generic builtin command line
drivers: firmware: efi: libstub: enable generic commandline
CMDLINE: x86: convert to generic builtin command line
of: allow sending a NULL value to early_init_dt_scan_chosen
CMDLINE: arm64: convert to generic builtin command line
arch/arm64/Kconfig | 33 +--
arch/arm64/include/asm/setup.h | 2 +
arch/arm64/kernel/idreg-override.c | 9 +-
arch/mips/Kconfig | 4 +-
arch/mips/Kconfig.debug | 44 ----
arch/mips/configs/ar7_defconfig | 9 +-
arch/mips/configs/bcm47xx_defconfig | 8 +-
arch/mips/configs/bcm63xx_defconfig | 15 +-
arch/mips/configs/bmips_be_defconfig | 11 +-
arch/mips/configs/bmips_stb_defconfig | 11 +-
arch/mips/configs/capcella_defconfig | 11 +-
arch/mips/configs/ci20_defconfig | 10 +-
arch/mips/configs/cu1000-neo_defconfig | 10 +-
arch/mips/configs/cu1830-neo_defconfig | 10 +-
arch/mips/configs/e55_defconfig | 4 +-
arch/mips/configs/generic_defconfig | 6 +-
arch/mips/configs/gpr_defconfig | 18 +-
arch/mips/configs/loongson3_defconfig | 13 +-
arch/mips/configs/mpc30x_defconfig | 7 +-
arch/mips/configs/tb0219_defconfig | 7 +-
arch/mips/configs/tb0226_defconfig | 7 +-
arch/mips/configs/tb0287_defconfig | 7 +-
arch/mips/configs/workpad_defconfig | 11 +-
arch/mips/include/asm/setup.h | 2 +
arch/mips/kernel/relocate.c | 17 +-
arch/mips/kernel/setup.c | 36 +--
arch/mips/pic32/pic32mzda/early_console.c | 2 +-
arch/mips/pic32/pic32mzda/init.c | 3 +-
arch/x86/Kconfig | 44 +---
arch/x86/kernel/setup.c | 18 +-
.../firmware/efi/libstub/efi-stub-helper.c | 29 +++
drivers/firmware/efi/libstub/efi-stub.c | 9 +
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 13 +-
drivers/of/fdt.c | 44 ++--
include/linux/cmdline.h | 103 ++++++++
init/Kconfig | 78 ++++++
lib/Kconfig | 4 +
lib/Makefile | 3 +
lib/generic_cmdline.S | 53 ++++
lib/test_cmdline1.c | 139 ++++++++++
scripts/Makefile | 2 +-
.../{insert-sys-cert.c => insert-symbol.c} | 243 ++++++++++++------
43 files changed, 716 insertions(+), 394 deletions(-)
create mode 100644 include/linux/cmdline.h
create mode 100644 lib/generic_cmdline.S
create mode 100644 lib/test_cmdline1.c
rename scripts/{insert-sys-cert.c => insert-symbol.c} (72%)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
2021-04-16 4:09 [PATCH 0/8] generic command line v4 Daniel Walker
@ 2021-04-16 4:09 ` Daniel Walker
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Walker @ 2021-04-16 4:09 UTC (permalink / raw)
To: Will Deacon, Christophe Leroy, Rob Herring, Daniel Gimpelevich,
Andrew Morton, x86, linux-mips, linuxppc-dev
Cc: xe-linux-external, Ard Biesheuvel, linux-efi, linux-kernel
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>
---
.../firmware/efi/libstub/efi-stub-helper.c | 29 +++++++++++++++++++
drivers/firmware/efi/libstub/efi-stub.c | 9 ++++++
drivers/firmware/efi/libstub/efistub.h | 1 +
drivers/firmware/efi/libstub/x86-stub.c | 13 +++++++--
4 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index aa8da0a49829..16318f55f187 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -13,6 +13,7 @@
#include <linux/efi.h>
#include <linux/kernel.h>
#include <linux/printk.h> /* For CONSOLE_LOGLEVEL_* */
+#include <linux/cmdline.h>
#include <asm/efi.h>
#include <asm/setup.h>
@@ -172,6 +173,34 @@ int efi_printk(const char *fmt, ...)
return printed;
}
+/**
+ * efi_handle_cmdline() - handle adding in building parts of the command line
+ * @cmdline: kernel command line
+ *
+ * Add in the generic parts of the commandline and start the parsing of the
+ * command line.
+ *
+ * Return: status code
+ */
+efi_status_t efi_handle_cmdline(char const *cmdline)
+{
+ efi_status_t status = EFI_SUCCESS;
+
+ if (sizeof(CMDLINE_STATIC_PREPEND) > 1)
+ status |= efi_parse_options(CMDLINE_STATIC_PREPEND);
+
+ if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
+ status |= efi_parse_options(cmdline);
+
+ if (sizeof(CMDLINE_STATIC_APPEND) > 1)
+ status |= efi_parse_options(CMDLINE_STATIC_APPEND);
+
+ if (status != EFI_SUCCESS)
+ efi_err("Failed to parse options\n");
+
+ return status;
+}
+
/**
* efi_parse_options() - Parse EFI command line options
* @cmdline: kernel command line
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 26e69788f27a..baa69b24cfdd 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -172,6 +172,14 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
goto fail;
}
+#ifdef CONFIG_GENERIC_CMDLINE
+ status = efi_handle_cmdline(cmdline_ptr);
+ if (status != EFI_SUCCESS) {
+ goto fail_free_cmdline;
+ }
+#endif
+
+#ifdef CONFIG_CMDLINE
if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
cmdline_size == 0) {
@@ -189,6 +197,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
goto fail_free_cmdline;
}
}
+#endif
efi_info("Booting Linux Kernel...\n");
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index cde0a2ef507d..07c7f9fdfffc 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -800,6 +800,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
unsigned long alignment,
unsigned long min_addr);
+efi_status_t efi_handle_cmdline(char const *cmdline);
efi_status_t efi_parse_options(char const *cmdline);
void efi_parse_option_graphics(char *option);
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index f14c4ff5839f..30ad8fb7122d 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -673,6 +673,8 @@ unsigned long efi_main(efi_handle_t handle,
unsigned long bzimage_addr = (unsigned long)startup_32;
unsigned long buffer_start, buffer_end;
struct setup_header *hdr = &boot_params->hdr;
+ unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
+ ((u64)boot_params->ext_cmd_line_ptr << 32));
efi_status_t status;
efi_system_table = sys_table_arg;
@@ -735,6 +737,14 @@ unsigned long efi_main(efi_handle_t handle,
image_offset = 0;
}
+#ifdef CONFIG_GENERIC_CMDLINE
+ status = efi_handle_cmdline((char *)cmdline_paddr);
+ if (status != EFI_SUCCESS) {
+ efi_err("Failed to parse options\n");
+ goto fail;
+ }
+#else /* CONFIG_GENERIC_CMDLINE */
+
#ifdef CONFIG_CMDLINE_BOOL
status = efi_parse_options(CONFIG_CMDLINE);
if (status != EFI_SUCCESS) {
@@ -743,8 +753,6 @@ unsigned long efi_main(efi_handle_t handle,
}
#endif
if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
- unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
- ((u64)boot_params->ext_cmd_line_ptr << 32));
status = efi_parse_options((char *)cmdline_paddr);
if (status != EFI_SUCCESS) {
efi_err("Failed to parse options\n");
@@ -752,6 +760,7 @@ unsigned long efi_main(efi_handle_t handle,
}
}
+#endif
/*
* At this point, an initrd may already have been loaded by the
* bootloader and passed via bootparams. We permit an initrd loaded
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-12-12 17:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2023-01-24 16:30 ` [PATCH 0/8] generic command line v5 Sean Anderson
-- strict thread matches above, loose matches on Subject: below --
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)
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
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).