All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Walker <danielwa@cisco.com>
To: 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, linux-mips@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
Cc: xe-linux-external@cisco.com, Ard Biesheuvel <ardb@kernel.org>,
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
Date: Thu,  9 Nov 2023 17:38:09 -0800	[thread overview]
Message-ID: <20231110013817.2378507-6-danielwa@cisco.com> (raw)
In-Reply-To: <20231110013817.2378507-1-danielwa@cisco.com>

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


WARNING: multiple messages have this Message-ID (diff)
From: Daniel Walker <danielwa@cisco.com>
To: 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, linux-mips@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
	Ard Biesheuvel <ardb@kernel.org>,
	xe-linux-external@cisco.com
Subject: [PATCH 5/8] drivers: firmware: efi: libstub: enable generic commandline
Date: Thu,  9 Nov 2023 17:38:09 -0800	[thread overview]
Message-ID: <20231110013817.2378507-6-danielwa@cisco.com> (raw)
In-Reply-To: <20231110013817.2378507-1-danielwa@cisco.com>

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


  parent reply	other threads:[~2023-11-10  1:39 UTC|newest]

Thread overview: 64+ 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 ` Daniel Walker
2023-11-10  1:38 ` [PATCH 1/8] CMDLINE: add generic builtin command line Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-10 16:12   ` kernel test robot
2023-11-10 16:12     ` kernel test robot
2023-11-23  6:32   ` Christophe Leroy
2023-11-23  6:32     ` Christophe Leroy
2023-12-04 11:11   ` Jaskaran Singh
2023-12-04 11:11     ` Jaskaran Singh
2023-11-10  1:38 ` [PATCH 2/8] scripts: insert-sys-cert: add command line insert capability Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-23  6:33   ` Christophe Leroy
2023-11-23  6:33     ` Christophe Leroy
2023-11-10  1:38 ` [PATCH 3/8] scripts: insert-sys-cert: change name to insert-symbol Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-23  6:34   ` Christophe Leroy
2023-11-23  6:34     ` Christophe Leroy
2023-11-10  1:38 ` [PATCH 4/8] CMDLINE: mips: convert to generic builtin command line Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-23  6:36   ` Christophe Leroy
2023-11-23  6:36     ` Christophe Leroy
2023-11-10  1:38 ` Daniel Walker [this message]
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-10  4:23     ` kernel test robot
2023-11-23  6:37   ` Christophe Leroy
2023-11-23  6:37     ` Christophe Leroy
2023-12-12  9:55   ` Ard Biesheuvel
2023-12-12  9:55     ` Ard Biesheuvel
2023-12-12 17:25     ` Daniel Walker (danielwa)
2023-12-12 17:25       ` Daniel Walker (danielwa)
2023-11-10  1:38 ` [PATCH 6/8] CMDLINE: x86: convert to generic builtin command line Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-10  7:17   ` kernel test robot
2023-11-10  7:17     ` kernel test robot
2023-11-10  1:38 ` [PATCH 7/8] of: replace command line handling Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-16 16:09   ` Rob Herring
2023-11-16 16:09     ` Rob Herring
2023-11-16 16:33     ` Daniel Walker (danielwa)
2023-11-16 16:33       ` Daniel Walker (danielwa)
2023-11-23  6:39   ` Christophe Leroy
2023-11-23  6:39     ` Christophe Leroy
2023-11-10  1:38 ` [PATCH 8/8] CMDLINE: arm64: convert to generic builtin command line Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-10  1:38   ` Daniel Walker
2023-11-23  6:39   ` Christophe Leroy
2023-11-23  6:39     ` Christophe Leroy
2023-11-23  6:39     ` Christophe Leroy
2023-11-10  1:51 ` [PATCH 0/8] generic command line v6 Andrew Morton
2023-11-10  1:51   ` Andrew Morton
2023-11-10  1:51   ` Andrew Morton
2023-11-10  2:22   ` Daniel Walker (danielwa)
2023-11-10  2:22     ` Daniel Walker (danielwa)
2023-11-10  2:22     ` Daniel Walker (danielwa)
2023-11-10  2:40     ` Andrew Morton
2023-11-10  2:40       ` Andrew Morton
2023-11-10  2:40       ` Andrew Morton
2023-11-23  6:23 ` Christophe Leroy
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
2021-04-16  4:09   ` 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=20231110013817.2378507-6-danielwa@cisco.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.