Re: [PATCH 6/8] drivers: firmware: efi: libstub: enable generic commandline

On Fri, Apr 02, 2021 at 07:36:53PM +0200, Christophe Leroy wrote:

Le 30/03/2021 à 19:57, Daniel Walker a écrit :

quoted

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.

Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <redacted>
---
  .../firmware/efi/libstub/efi-stub-helper.c    | 35 +++++++++++++++++++
  drivers/firmware/efi/libstub/efi-stub.c       |  7 ++++
  drivers/firmware/efi/libstub/efistub.h        |  1 +
  drivers/firmware/efi/libstub/x86-stub.c       | 13 +++++--
  4 files changed, 54 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..c155837cedc9 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,40 @@ 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;
+
+	status = efi_parse_options(CMDLINE_PREPEND);
+	if (status != EFI_SUCCESS) {
+		efi_err("Failed to parse options\n");
+		return status;
+	}
+
+	status = efi_parse_options(IS_ENABLED(CONFIG_CMDLINE_OVERRIDE) ? "" : cmdline);
+	if (status != EFI_SUCCESS) {
+		efi_err("Failed to parse options\n");
+		return status;
+	}
+
+	status = efi_parse_options(CMDLINE_APPEND);
+	if (status != EFI_SUCCESS) {
+		efi_err("Failed to parse options\n");
+		return status;
+	}
+
+	return EFI_SUCCESS;
+}

I think we can refactor to first build the final command line, then call
efi_parse_options() only once after that.

 
I tried this, like what you did in your v4 .. The issues are similar to the
prom_init.c problems. The environment is delicate and requires careful
programming to get it done correctly.

The big advantage of GENERIC_CMDLINE should be to not address anymore
CONFIG_CMDLINE_XXX options at all outside of linux/cmdline.h

 
I agree , but not I've found that it's not likely to get this all changed in a
single series.

Daniel