2015-04-08 18:42:26

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH] x86: Allow built-in command line to work in early kernel init

The kernel supports having a command line built into it. Unfortunately this
doesn't work in all cases - the built-in command line is only appended
after we've jumped to the kernel proper, but various parts of the early
boot process also pay attention to the command line.

This patch moves the command line override code from the kernel itself to
the early init code. Unfortunately the kernel can be executed by jumping
to the 16-bit entry point, the UEFI entry point or directly to the 32-bit
entry point, and there is no guarantee that any of these will have access
to data held in the others. As a result, three copies of the command line
will be embedded in the kernel.

This patch also adds a new flag to the xloadflags field of the setup header
in order to allow the 16-bit and UEFI entry points to inform the generic
setup code that the command line has already been appended and so shouldn't
be added once more.

Signed-off-by: Matthew Garrett <[email protected]>
---
Documentation/x86/boot.txt | 4 +++
arch/x86/boot/boot.h | 10 ++++++
arch/x86/boot/cmdline.c | 37 +++++++++++++++++++
arch/x86/boot/compressed/cmdline.c | 18 +++++++++-
arch/x86/boot/compressed/eboot.c | 4 ++-
arch/x86/boot/compressed/misc.c | 2 ++
arch/x86/boot/compressed/misc.h | 3 +-
arch/x86/boot/main.c | 3 ++
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/kernel/asm-offsets.c | 1 +
arch/x86/kernel/setup.c | 16 ---------
drivers/firmware/efi/libstub/efi-stub-helper.c | 49 ++++++++++++++++++++++++--
12 files changed, 127 insertions(+), 21 deletions(-)

diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index ed98366..44fedd3 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -611,6 +611,10 @@ Protocol: 2.12+
Bit 4 (read): XLF_EFI_KEXEC
- If 1, the kernel supports kexec EFI boot with EFI runtime support.

+ Bit 5 (reserved): XLF_CMDLINE_APPENDED
+ - If 1, CONFIG_CMDLINE has already been appended to the cmdline.
+ For internal use only, ignored by bootloaders.
+
Field name: cmdline_size
Type: read
Offset/size: 0x238/4
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index bd49ec6..45fc708 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -273,6 +273,7 @@ void intcall(u8 int_no, const struct biosregs *ireg, struct biosregs *oreg);
/* cmdline.c */
int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *buffer, int bufsize);
int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option);
+int __cmdline_init(unsigned long cmdline_ptr, struct setup_header *hdr);
static inline int cmdline_find_option(const char *option, char *buffer, int bufsize)
{
unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
@@ -293,6 +294,15 @@ static inline int cmdline_find_option_bool(const char *option)
return __cmdline_find_option_bool(cmd_line_ptr, option);
}

+static inline int cmdline_init(void)
+{
+ unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
+
+ if (cmd_line_ptr >= 0x100000)
+ return -1; /* inaccessible */
+
+ return __cmdline_init(cmd_line_ptr, &boot_params.hdr);
+}
/* cpu.c, cpucheck.c */
int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr);
int validate_cpu(void);
diff --git a/arch/x86/boot/cmdline.c b/arch/x86/boot/cmdline.c
index 625d21b..aadc192 100644
--- a/arch/x86/boot/cmdline.c
+++ b/arch/x86/boot/cmdline.c
@@ -14,6 +14,10 @@

#include "boot.h"

+#ifdef CONFIG_CMDLINE_BOOL
+static char builtin_cmdline[] = CONFIG_CMDLINE;
+#endif
+
static inline int myisspace(u8 c)
{
return c <= ' '; /* Close enough approximation */
@@ -156,3 +160,36 @@ int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option)

return 0; /* Buffer overrun */
}
+
+int __cmdline_init(unsigned long cmdline_ptr, struct setup_header *hdr)
+{
+#ifdef CONFIG_CMDLINE_BOOL
+ addr_t cptr;
+ int i = 0;
+
+ if (!cmdline_ptr)
+ return -1; /* No command line */
+
+ set_fs(cmdline_ptr >> 4);
+ cptr = cmdline_ptr & 0xf;
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+ while (cptr < 0x10000) {
+ char c = rdfs8(cptr);
+ if (!c) {
+ wrfs8(' ', cptr++);
+ break;
+ }
+ cptr++;
+ }
+#endif /* !CONFIG_CMDLINE_OVERRIDE */
+ while (builtin_cmdline[i] && cptr < 0xffff)
+ wrfs8(builtin_cmdline[i++], cptr++);
+
+ wrfs8('\0', cptr);
+
+ hdr->xloadflags |= XLF_CMDLINE_APPENDED;
+#endif /* CONFIG_CMDLINE_BOOL */
+
+ return 0;
+}
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index b68e303..e1b4b60 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,6 +1,6 @@
#include "misc.h"

-#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE
+#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_CMDLINE_BOOL

static unsigned long fs;
static inline void set_fs(unsigned long seg)
@@ -12,6 +12,10 @@ static inline char rdfs8(addr_t addr)
{
return *((char *)(fs + addr));
}
+static inline void wrfs8(u8 v, addr_t addr)
+{
+ *((char *)(fs + addr)) = v;
+}
#include "../cmdline.c"
static unsigned long get_cmd_line_ptr(void)
{
@@ -30,4 +34,16 @@ int cmdline_find_option_bool(const char *option)
return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
}

+int cmdline_init(void)
+{
+ if (!(real_mode->hdr.xloadflags & XLF_CMDLINE_APPENDED))
+ return __cmdline_init(get_cmd_line_ptr(), &real_mode->hdr);
+ return 0;
+}
+#else
+int cmdline_init(void)
+{
+#error "BAD"
+ return 0;
+}
#endif
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index ef17683..cf373c2 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -1109,7 +1109,9 @@ struct boot_params *make_boot_params(struct efi_config *c)
if (!cmdline_ptr)
goto fail;
hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
-
+#ifdef CONFIG_CMDLINE_BOOL
+ hdr->xloadflags |= XLF_CMDLINE_APPENDED;
+#endif
hdr->ramdisk_image = 0;
hdr->ramdisk_size = 0;

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index a950864..dc8a287 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -390,6 +390,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
lines = real_mode->screen_info.orig_video_lines;
cols = real_mode->screen_info.orig_video_cols;

+ cmdline_init();
+
console_init();
debug_putstr("early console in decompress_kernel\n");

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 04477d6..9cbec86 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -48,10 +48,11 @@ static inline void debug_putstr(const char *s)

#endif

-#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE
+#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_CMDLINE_BOOL
/* cmdline.c */
int cmdline_find_option(const char *option, char *buffer, int bufsize);
int cmdline_find_option_bool(const char *option);
+int cmdline_init(void);
#endif


diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index fd6c9f2..7c24862 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -137,6 +137,9 @@ void main(void)
/* First, copy the boot header into the "zeropage" */
copy_boot_params();

+ /* Handle built-in command line */
+ cmdline_init();
+
/* Initialize the early-boot console */
console_init();
if (cmdline_find_option_bool("debug"))
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 89a033a..524498a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -26,6 +26,7 @@
#define XLF_EFI_HANDOVER_32 (1<<2)
#define XLF_EFI_HANDOVER_64 (1<<3)
#define XLF_EFI_KEXEC (1<<4)
+#define XLF_CMDLINE_APPENDED (1<<5)

#ifndef __ASSEMBLY__

diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 9f6b934..c10ab92 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -68,6 +68,7 @@ void common(void) {
OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
OFFSET(BP_pref_address, boot_params, hdr.pref_address);
OFFSET(BP_code32_start, boot_params, hdr.code32_start);
+ OFFSET(BP_xloadflags, boot_params, hdr.xloadflags);

BLANK();
DEFINE(PTREGS_SIZE, sizeof(struct pt_regs));
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 0a2421c..1f68d32 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -232,9 +232,6 @@ unsigned long saved_video_mode;
#define RAMDISK_LOAD_FLAG 0x4000

static char __initdata command_line[COMMAND_LINE_SIZE];
-#ifdef CONFIG_CMDLINE_BOOL
-static char __initdata builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
-#endif

#if defined(CONFIG_EDD) || defined(CONFIG_EDD_MODULE)
struct edd edd;
@@ -968,19 +965,6 @@ void __init setup_arch(char **cmdline_p)
bss_resource.start = __pa_symbol(__bss_start);
bss_resource.end = __pa_symbol(__bss_stop)-1;

-#ifdef CONFIG_CMDLINE_BOOL
-#ifdef CONFIG_CMDLINE_OVERRIDE
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
-#else
- if (builtin_cmdline[0]) {
- /* append boot loader cmdline to builtin */
- strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
- strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
- }
-#endif
-#endif
-
strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
*cmdline_p = command_line;

diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index f07d4a6..54afe1f 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,9 +12,14 @@

#include <linux/efi.h>
#include <asm/efi.h>
+#include <asm/setup.h>

#include "efistub.h"

+#ifdef CONFIG_CMDLINE_BOOL
+static char builtin_cmdline[] = CONFIG_CMDLINE;
+#endif
+
/*
* Some firmware implementations have problems reading files in one go.
* A read chunk size of 1MB seems to work for most platforms.
@@ -649,6 +654,21 @@ static u8 *efi_utf16_to_utf8(u8 *dst, const u16 *src, int n)
return dst;
}

+static size_t efi_strlcat(char *dest, const char *src, size_t count)
+{
+ size_t dsize = strlen(dest);
+ size_t len = strlen(src);
+ size_t res = dsize + len;
+
+ dest += dsize;
+ count -= dsize;
+ if (len >= count)
+ len = count-1;
+ memcpy(dest, src, len);
+ dest[len] = 0;
+ return res;
+}
+
/*
* Convert the unicode UEFI command line to ASCII to pass to kernel.
* Size of memory allocated return in *cmd_line_len.
@@ -658,14 +678,16 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
efi_loaded_image_t *image,
int *cmd_line_len)
{
+ unsigned long cmdline_addr = 0;
+ int i;
+ efi_status_t status;
+#ifndef CONFIG_CMDLINE_OVERRIDE
const u16 *s2;
u8 *s1 = NULL;
- unsigned long cmdline_addr = 0;
int load_options_chars = image->load_options_size / 2; /* UTF-16 */
const u16 *options = image->load_options;
int options_bytes = 0; /* UTF-8 bytes */
int options_chars = 0; /* UTF-16 chars */
- efi_status_t status;
u16 zero = 0;

if (options) {
@@ -684,6 +706,12 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,

options_bytes++; /* NUL termination */

+#ifdef CONFIG_CMDLINE_BOOL
+ /* Add length of the built-in command line, plus a space */
+ options_bytes += strlen(builtin_cmdline);
+ options_bytes++;
+#endif
+
status = efi_low_alloc(sys_table_arg, options_bytes, 0, &cmdline_addr);
if (status != EFI_SUCCESS)
return NULL;
@@ -694,6 +722,23 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
s1 = efi_utf16_to_utf8(s1, s2, options_chars);
*s1 = '\0';

+#ifdef CONFIG_CMDLINE_BOOL
+ efi_strlcat((char *)cmdline_addr, " ", COMMAND_LINE_SIZE);
+ efi_strlcat((char *)cmdline_addr, builtin_cmdline, COMMAND_LINE_SIZE);
+#endif
*cmd_line_len = options_bytes;
+#else /* CONFIG_CMDLINE_OVERRIDE */
+ status = efi_low_alloc(sys_table_arg, strlen(builtin_cmdline), 0,
+ &cmdline_addr);
+ if (status != EFI_SUCCESS)
+ return NULL;
+ while (builtin_cmdline[i] && i < COMMAND_LINE_SIZE) {
+ ((char *)cmdline_addr)[i] = builtin_cmdline[i];
+ i++;
+ }
+ ((char *)cmdline_addr)[i] = '\0';
+ *cmd_line_len = strlen(builtin_cmdline);
+#endif /* CONFIG_CMDLINE_OVERRIDE */
+
return (char *)cmdline_addr;
}
--
2.3.4


2015-04-09 20:25:38

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH V2] x86: Allow built-in command line to work in early kernel init

From: Matthew Garrett <[email protected]>

The kernel supports having a command line built into it. Unfortunately this
doesn't work in all cases - the built-in command line is only appended
after we've jumped to the kernel proper, but various parts of the early
boot process also pay attention to the command line.

This patch moves the command line override code from the kernel itself to
the early init code. Unfortunately the kernel can be executed by jumping
to the 16-bit entry point, the UEFI entry point, directly to the 32-bit
entry point or even to the entry point of the uncompressed image, and
there is no guarantee that any of these will have access to data held in
the others. As a result, four copies of the command line will be embedded
in the kernel.

This patch also defines a new field in boot_params in order to allow the
earlier entry points to inform the generic setup code that the command line
has already been appended and so shouldn't be added once more.

Signed-off-by: Matthew Garrett <[email protected]>
---

V2: Use some empty data in bootparams rather than in the setup header since
there's no need for the bootloader to touch this. Add back the code to
kernel/setup.c because some scenarios may involve skipping the setup code
entirely.

Documentation/x86/zero-page.txt | 1 +
arch/x86/boot/boot.h | 10 ++++++
arch/x86/boot/cmdline.c | 37 +++++++++++++++++++
arch/x86/boot/compressed/cmdline.c | 18 +++++++++-
arch/x86/boot/compressed/eboot.c | 4 ++-
arch/x86/boot/compressed/misc.c | 2 ++
arch/x86/boot/compressed/misc.h | 3 +-
arch/x86/boot/main.c | 3 ++
arch/x86/include/uapi/asm/bootparam.h | 5 ++-
arch/x86/kernel/setup.c | 16 +++++----
drivers/firmware/efi/libstub/efi-stub-helper.c | 49 ++++++++++++++++++++++++--
11 files changed, 135 insertions(+), 13 deletions(-)

diff --git a/Documentation/x86/zero-page.txt b/Documentation/x86/zero-page.txt
index 82fbdbc..22eaecf 100644
--- a/Documentation/x86/zero-page.txt
+++ b/Documentation/x86/zero-page.txt
@@ -12,6 +12,7 @@ Offset Proto Name Meaning
000/040 ALL screen_info Text mode or frame buffer information
(struct screen_info)
040/014 ALL apm_bios_info APM BIOS information (struct apm_bios_info)
+054/004 ALL setup_flags Flags passed from early kernel setup
058/008 ALL tboot_addr Physical address of tboot shared page
060/010 ALL ist_info Intel SpeedStep (IST) BIOS support information
(struct ist_info)
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index bd49ec6..3718244 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -273,6 +273,7 @@ void intcall(u8 int_no, const struct biosregs *ireg, struct biosregs *oreg);
/* cmdline.c */
int __cmdline_find_option(unsigned long cmdline_ptr, const char *option, char *buffer, int bufsize);
int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option);
+int __cmdline_init(unsigned long cmdline_ptr, struct boot_params *params);
static inline int cmdline_find_option(const char *option, char *buffer, int bufsize)
{
unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
@@ -293,6 +294,15 @@ static inline int cmdline_find_option_bool(const char *option)
return __cmdline_find_option_bool(cmd_line_ptr, option);
}

+static inline int cmdline_init(void)
+{
+ unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
+
+ if (cmd_line_ptr >= 0x100000)
+ return -1; /* inaccessible */
+
+ return __cmdline_init(cmd_line_ptr, &boot_params);
+}
/* cpu.c, cpucheck.c */
int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr);
int validate_cpu(void);
diff --git a/arch/x86/boot/cmdline.c b/arch/x86/boot/cmdline.c
index 625d21b..2273d27 100644
--- a/arch/x86/boot/cmdline.c
+++ b/arch/x86/boot/cmdline.c
@@ -14,6 +14,10 @@

#include "boot.h"

+#ifdef CONFIG_CMDLINE_BOOL
+static char builtin_cmdline[] = CONFIG_CMDLINE;
+#endif
+
static inline int myisspace(u8 c)
{
return c <= ' '; /* Close enough approximation */
@@ -156,3 +160,36 @@ int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option)

return 0; /* Buffer overrun */
}
+
+int __cmdline_init(unsigned long cmdline_ptr, struct boot_params *params)
+{
+#ifdef CONFIG_CMDLINE_BOOL
+ addr_t cptr;
+ int i = 0;
+
+ if (!cmdline_ptr)
+ return -1; /* No command line */
+
+ set_fs(cmdline_ptr >> 4);
+ cptr = cmdline_ptr & 0xf;
+
+#ifndef CONFIG_CMDLINE_OVERRIDE
+ while (cptr < 0x10000) {
+ char c = rdfs8(cptr);
+ if (!c) {
+ wrfs8(' ', cptr++);
+ break;
+ }
+ cptr++;
+ }
+#endif /* !CONFIG_CMDLINE_OVERRIDE */
+ while (builtin_cmdline[i] && cptr < 0xffff)
+ wrfs8(builtin_cmdline[i++], cptr++);
+
+ wrfs8('\0', cptr);
+
+ params->setup_flags |= SETUP_CMDLINE_APPENDED;
+#endif /* CONFIG_CMDLINE_BOOL */
+
+ return 0;
+}
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index b68e303..bf8ea07 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,6 +1,6 @@
#include "misc.h"

-#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE
+#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_CMDLINE_BOOL

static unsigned long fs;
static inline void set_fs(unsigned long seg)
@@ -12,6 +12,10 @@ static inline char rdfs8(addr_t addr)
{
return *((char *)(fs + addr));
}
+static inline void wrfs8(u8 v, addr_t addr)
+{
+ *((char *)(fs + addr)) = v;
+}
#include "../cmdline.c"
static unsigned long get_cmd_line_ptr(void)
{
@@ -30,4 +34,16 @@ int cmdline_find_option_bool(const char *option)
return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
}

+int cmdline_init(void)
+{
+ if (!(real_mode->setup_flags & SETUP_CMDLINE_APPENDED))
+ return __cmdline_init(get_cmd_line_ptr(), real_mode);
+ return 0;
+}
+#else
+int cmdline_init(void)
+{
+#error "BAD"
+ return 0;
+}
#endif
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index ef17683..15a932c 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -1109,7 +1109,9 @@ struct boot_params *make_boot_params(struct efi_config *c)
if (!cmdline_ptr)
goto fail;
hdr->cmd_line_ptr = (unsigned long)cmdline_ptr;
-
+#ifdef CONFIG_CMDLINE_BOOL
+ boot_params->setup_flags |= SETUP_CMDLINE_APPENDED;
+#endif
hdr->ramdisk_image = 0;
hdr->ramdisk_size = 0;

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index a950864..dc8a287 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -390,6 +390,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
lines = real_mode->screen_info.orig_video_lines;
cols = real_mode->screen_info.orig_video_cols;

+ cmdline_init();
+
console_init();
debug_putstr("early console in decompress_kernel\n");

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 04477d6..9cbec86 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -48,10 +48,11 @@ static inline void debug_putstr(const char *s)

#endif

-#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE
+#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_CMDLINE_BOOL
/* cmdline.c */
int cmdline_find_option(const char *option, char *buffer, int bufsize);
int cmdline_find_option_bool(const char *option);
+int cmdline_init(void);
#endif


diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index fd6c9f2..7c24862 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -137,6 +137,9 @@ void main(void)
/* First, copy the boot header into the "zeropage" */
copy_boot_params();

+ /* Handle built-in command line */
+ cmdline_init();
+
/* Initialize the early-boot console */
console_init();
if (cmdline_find_option_bool("debug"))
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 89a033a..2dfb945 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -27,6 +27,9 @@
#define XLF_EFI_HANDOVER_64 (1<<3)
#define XLF_EFI_KEXEC (1<<4)

+/* setup_flags */
+#define SETUP_CMDLINE_APPENDED (1<<0)
+
#ifndef __ASSEMBLY__

#include <linux/types.h>
@@ -114,7 +117,7 @@ struct efi_info {
struct boot_params {
struct screen_info screen_info; /* 0x000 */
struct apm_bios_info apm_bios_info; /* 0x040 */
- __u8 _pad2[4]; /* 0x054 */
+ __u32 setup_flags; /* 0x054 */
__u64 tboot_addr; /* 0x058 */
struct ist_info ist_info; /* 0x060 */
__u8 _pad3[16]; /* 0x070 */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 0a2421c..b0d50b1 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -969,16 +969,18 @@ void __init setup_arch(char **cmdline_p)
bss_resource.end = __pa_symbol(__bss_stop)-1;

#ifdef CONFIG_CMDLINE_BOOL
+ if (!(boot_params.setup_flags & SETUP_CMDLINE_APPENDED)) {
#ifdef CONFIG_CMDLINE_OVERRIDE
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
-#else
- if (builtin_cmdline[0]) {
- /* append boot loader cmdline to builtin */
- strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
- strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
- }
+#else
+ if (builtin_cmdline[0]) {
+ /* append boot loader cmdline to builtin */
+ strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
+ strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
+ strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+ }
#endif
+ }
#endif

strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index f07d4a6..54afe1f 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,9 +12,14 @@

#include <linux/efi.h>
#include <asm/efi.h>
+#include <asm/setup.h>

#include "efistub.h"

+#ifdef CONFIG_CMDLINE_BOOL
+static char builtin_cmdline[] = CONFIG_CMDLINE;
+#endif
+
/*
* Some firmware implementations have problems reading files in one go.
* A read chunk size of 1MB seems to work for most platforms.
@@ -649,6 +654,21 @@ static u8 *efi_utf16_to_utf8(u8 *dst, const u16 *src, int n)
return dst;
}

+static size_t efi_strlcat(char *dest, const char *src, size_t count)
+{
+ size_t dsize = strlen(dest);
+ size_t len = strlen(src);
+ size_t res = dsize + len;
+
+ dest += dsize;
+ count -= dsize;
+ if (len >= count)
+ len = count-1;
+ memcpy(dest, src, len);
+ dest[len] = 0;
+ return res;
+}
+
/*
* Convert the unicode UEFI command line to ASCII to pass to kernel.
* Size of memory allocated return in *cmd_line_len.
@@ -658,14 +678,16 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
efi_loaded_image_t *image,
int *cmd_line_len)
{
+ unsigned long cmdline_addr = 0;
+ int i;
+ efi_status_t status;
+#ifndef CONFIG_CMDLINE_OVERRIDE
const u16 *s2;
u8 *s1 = NULL;
- unsigned long cmdline_addr = 0;
int load_options_chars = image->load_options_size / 2; /* UTF-16 */
const u16 *options = image->load_options;
int options_bytes = 0; /* UTF-8 bytes */
int options_chars = 0; /* UTF-16 chars */
- efi_status_t status;
u16 zero = 0;

if (options) {
@@ -684,6 +706,12 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,

options_bytes++; /* NUL termination */

+#ifdef CONFIG_CMDLINE_BOOL
+ /* Add length of the built-in command line, plus a space */
+ options_bytes += strlen(builtin_cmdline);
+ options_bytes++;
+#endif
+
status = efi_low_alloc(sys_table_arg, options_bytes, 0, &cmdline_addr);
if (status != EFI_SUCCESS)
return NULL;
@@ -694,6 +722,23 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg,
s1 = efi_utf16_to_utf8(s1, s2, options_chars);
*s1 = '\0';

+#ifdef CONFIG_CMDLINE_BOOL
+ efi_strlcat((char *)cmdline_addr, " ", COMMAND_LINE_SIZE);
+ efi_strlcat((char *)cmdline_addr, builtin_cmdline, COMMAND_LINE_SIZE);
+#endif
*cmd_line_len = options_bytes;
+#else /* CONFIG_CMDLINE_OVERRIDE */
+ status = efi_low_alloc(sys_table_arg, strlen(builtin_cmdline), 0,
+ &cmdline_addr);
+ if (status != EFI_SUCCESS)
+ return NULL;
+ while (builtin_cmdline[i] && i < COMMAND_LINE_SIZE) {
+ ((char *)cmdline_addr)[i] = builtin_cmdline[i];
+ i++;
+ }
+ ((char *)cmdline_addr)[i] = '\0';
+ *cmd_line_len = strlen(builtin_cmdline);
+#endif /* CONFIG_CMDLINE_OVERRIDE */
+
return (char *)cmdline_addr;
}
--
2.3.4

2015-05-12 09:28:35

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH V2] x86: Allow built-in command line to work in early kernel init


* Matthew Garrett <[email protected]> wrote:

> Any feedback on this?

So I worry about:

- the fragmented nature of it: lots of non-obvious pieces of code
will now be scattered all around arch/x86/.

- the crazy #ifdefs scattered around

- the various pieces of data scattered around

... so while I don't mind the feature in principle, but it should be
centralized much better IMHO, made Kconfig invariant at least in its
fragmented callbacks, with only the very strictly boot method specific
details spelled out explicitly.

I have not thought much about how to achieve that :-/

Thanks,

Ingo

2015-05-12 17:39:36

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH V2] x86: Allow built-in command line to work in early kernel init

On Tue, May 12, 2015 at 2:28 AM, Ingo Molnar <[email protected]> wrote:
>
> * Matthew Garrett <[email protected]> wrote:
>
>> Any feedback on this?
>
> So I worry about:
>
> - the fragmented nature of it: lots of non-obvious pieces of code
> will now be scattered all around arch/x86/.

We've got four different entry points and they all work under very
different constraints - I can't see any real way to share this code.
We could move all the command line handling code into a single file
and #include it with different ifdefs, but I'm not sure that that's
less confusing?

> - the crazy #ifdefs scattered around

The #ifdefs could be abstracted behind a couple of simple helper functions?

> - the various pieces of data scattered around

We're trying to use the same data in what are pretty much four
logically distinct codebases. I'm not sure that there's any way around
that other than linker magic, and that seems like it'd make things
even more non-obvious.

> ... so while I don't mind the feature in principle, but it should be
> centralized much better IMHO, made Kconfig invariant at least in its
> fragmented callbacks, with only the very strictly boot method specific
> details spelled out explicitly.
>
> I have not thought much about how to achieve that :-/

I'm happy to reimplement this, but outside losing the #ifdefs I'm
struggling to see any real way of improving it.

2015-05-12 18:43:05

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH V2] x86: Allow built-in command line to work in early kernel init

Ok, maybe something more like this? It even gets rid of some of the
#ifdefs in setup.c.

diff --git a/Documentation/x86/zero-page.txt b/Documentation/x86/zero-page.txt
index 82fbdbc..22eaecf 100644
--- a/Documentation/x86/zero-page.txt
+++ b/Documentation/x86/zero-page.txt
@@ -12,6 +12,7 @@ Offset Proto Name Meaning
000/040 ALL screen_info Text mode or frame buffer information
(struct screen_info)
040/014 ALL apm_bios_info APM BIOS information (struct apm_bios_info)
+054/004 ALL setup_flags Flags passed from early kernel setup
058/008 ALL tboot_addr Physical address of tboot shared page
060/010 ALL ist_info Intel SpeedStep (IST) BIOS support information
(struct ist_info)
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index bd49ec6..3718244 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -273,6 +273,7 @@ void intcall(u8 int_no, const struct biosregs
*ireg, struct biosregs *oreg);
/* cmdline.c */
int __cmdline_find_option(unsigned long cmdline_ptr, const char
*option, char *buffer, int bufsize);
int __cmdline_find_option_bool(unsigned long cmdline_ptr, const char *option);
+int __cmdline_init(unsigned long cmdline_ptr, struct boot_params *params);
static inline int cmdline_find_option(const char *option, char
*buffer, int bufsize)
{
unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
@@ -293,6 +294,15 @@ static inline int cmdline_find_option_bool(const
char *option)
return __cmdline_find_option_bool(cmd_line_ptr, option);
}

+static inline int cmdline_init(void)
+{
+ unsigned long cmd_line_ptr = boot_params.hdr.cmd_line_ptr;
+
+ if (cmd_line_ptr >= 0x100000)
+ return -1; /* inaccessible */
+
+ return __cmdline_init(cmd_line_ptr, &boot_params);
+}
/* cpu.c, cpucheck.c */
int check_cpu(int *cpu_level_ptr, int *req_level_ptr, u32 **err_flags_ptr);
int validate_cpu(void);
diff --git a/arch/x86/boot/cmdline.c b/arch/x86/boot/cmdline.c
index 625d21b..9ddffd0 100644
--- a/arch/x86/boot/cmdline.c
+++ b/arch/x86/boot/cmdline.c
@@ -12,8 +12,15 @@
* Simple command-line parser for early boot.
*/

+#include <asm/cmdline_append.h>
#include "boot.h"

+#ifdef CONFIG_CMDLINE_BOOL
+static char builtin_cmdline[] = CONFIG_CMDLINE;
+#else
+static char *builtin_cmdline;
+#endif
+
static inline int myisspace(u8 c)
{
return c <= ' '; /* Close enough approximation */
@@ -156,3 +163,38 @@ int __cmdline_find_option_bool(unsigned long
cmdline_ptr, const char *option)

return 0; /* Buffer overrun */
}
+
+int __cmdline_init(unsigned long cmdline_ptr, struct boot_params *params)
+{
+ addr_t cptr;
+ int i = 0;
+
+ if (!builtin_cmdline)
+ return 0;
+
+ if (!cmdline_ptr)
+ return -1; /* No command line */
+
+ set_fs(cmdline_ptr >> 4);
+ cptr = cmdline_ptr & 0xf;
+
+ if (append_cmdline) {
+ while (cptr < 0x10000) {
+ char c = rdfs8(cptr);
+ if (!c) {
+ wrfs8(' ', cptr++);
+ break;
+ }
+ cptr++;
+ }
+ }
+
+ while (builtin_cmdline[i] && cptr < 0xffff)
+ wrfs8(builtin_cmdline[i++], cptr++);
+
+ wrfs8('\0', cptr);
+
+ params->setup_flags |= SETUP_CMDLINE_APPENDED;
+
+ return 0;
+}
diff --git a/arch/x86/boot/compressed/cmdline.c
b/arch/x86/boot/compressed/cmdline.c
index b68e303..4348828 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,6 +1,6 @@
#include "misc.h"

-#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE
+#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_CMDLINE_BOOL

static unsigned long fs;
static inline void set_fs(unsigned long seg)
@@ -12,6 +12,10 @@ static inline char rdfs8(addr_t addr)
{
return *((char *)(fs + addr));
}
+static inline void wrfs8(u8 v, addr_t addr)
+{
+ *((char *)(fs + addr)) = v;
+}
#include "../cmdline.c"
static unsigned long get_cmd_line_ptr(void)
{
@@ -30,4 +34,15 @@ int cmdline_find_option_bool(const char *option)
return __cmdline_find_option_bool(get_cmd_line_ptr(), option);
}

+int cmdline_init(void)
+{
+ if (!(real_mode->setup_flags & SETUP_CMDLINE_APPENDED))
+ return __cmdline_init(get_cmd_line_ptr(), real_mode);
+ return 0;
+}
+#else
+int cmdline_init(void)
+{
+ return 0;
+}
#endif
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 48304b8..e44146d 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -9,6 +9,7 @@

#include <linux/efi.h>
#include <linux/pci.h>
+#include <asm/cmdline_append.h>
#include <asm/efi.h>
#include <asm/setup.h>
#include <asm/desc.h>
@@ -1112,6 +1113,9 @@ struct boot_params *make_boot_params(struct efi_config *c)
/* Fill in upper bits of command line address, NOP on 32 bit */
boot_params->ext_cmd_line_ptr = (u64)(unsigned long)cmdline_ptr >> 32;

+ if (append_cmdline)
+ boot_params->setup_flags |= SETUP_CMDLINE_APPENDED;
+
hdr->ramdisk_image = 0;
hdr->ramdisk_size = 0;

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index a107b93..832c9a8 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -393,6 +393,8 @@ asmlinkage __visible void *decompress_kernel(void
*rmode, memptr heap,
lines = real_mode->screen_info.orig_video_lines;
cols = real_mode->screen_info.orig_video_cols;

+ cmdline_init();
+
console_init();
debug_putstr("early console in decompress_kernel\n");

diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 89dd0d7..1584397 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -48,10 +48,11 @@ static inline void debug_putstr(const char *s)

#endif

-#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE
+#if CONFIG_EARLY_PRINTK || CONFIG_RANDOMIZE_BASE || CONFIG_CMDLINE_BOOL
/* cmdline.c */
int cmdline_find_option(const char *option, char *buffer, int bufsize);
int cmdline_find_option_bool(const char *option);
+int cmdline_init(void);
#endif


diff --git a/arch/x86/boot/main.c b/arch/x86/boot/main.c
index fd6c9f2..7c24862 100644
--- a/arch/x86/boot/main.c
+++ b/arch/x86/boot/main.c
@@ -137,6 +137,9 @@ void main(void)
/* First, copy the boot header into the "zeropage" */
copy_boot_params();

+ /* Handle built-in command line */
+ cmdline_init();
+
/* Initialize the early-boot console */
console_init();
if (cmdline_find_option_bool("debug"))
diff --git a/arch/x86/include/asm/cmdline_append.h
b/arch/x86/include/asm/cmdline_append.h
new file mode 100644
index 0000000..a57bf7e
--- /dev/null
+++ b/arch/x86/include/asm/cmdline_append.h
@@ -0,0 +1,10 @@
+#ifndef _ASM_X86_CMDLINE_APPEND_H
+#define _ASM_X86_CMDLINE_APPEND_H
+
+#ifdef CONFIG_CMDLINE_OVERRIDE
+static char append_cmdline;
+#else
+static char append_cmdline = 1;
+#endif
+
+#endif /* _ASM_X86_CMDLINE_APPEND_H */
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index ab456dc..3fda079 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -27,6 +27,9 @@
#define XLF_EFI_HANDOVER_64 (1<<3)
#define XLF_EFI_KEXEC (1<<4)

+/* setup_flags */
+#define SETUP_CMDLINE_APPENDED (1<<0)
+
#ifndef __ASSEMBLY__

#include <linux/types.h>
@@ -114,7 +117,7 @@ struct efi_info {
struct boot_params {
struct screen_info screen_info; /* 0x000 */
struct apm_bios_info apm_bios_info; /* 0x040 */
- __u8 _pad2[4]; /* 0x054 */
+ __u32 setup_flags; /* 0x054 */
__u64 tboot_addr; /* 0x058 */
struct ist_info ist_info; /* 0x060 */
__u8 _pad3[16]; /* 0x070 */
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d74ac33..4c6456d 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -112,6 +112,7 @@
#include <asm/alternative.h>
#include <asm/prom.h>

+#include <asm/cmdline_append.h>
/*
* max_low_pfn_mapped: highest direct mapped pfn under 4GB
* max_pfn_mapped: highest direct mapped pfn over 4GB
@@ -234,6 +235,8 @@ unsigned long saved_video_mode;
static char __initdata command_line[COMMAND_LINE_SIZE];
#ifdef CONFIG_CMDLINE_BOOL
static char __initdata builtin_cmdline[COMMAND_LINE_SIZE] = CONFIG_CMDLINE;
+#else
+static char *builtin_cmdline;
#endif

#if defined(CONFIG_EDD) || defined(CONFIG_EDD_MODULE)
@@ -973,18 +976,21 @@ void __init setup_arch(char **cmdline_p)
bss_resource.start = __pa_symbol(__bss_start);
bss_resource.end = __pa_symbol(__bss_stop)-1;

-#ifdef CONFIG_CMDLINE_BOOL
-#ifdef CONFIG_CMDLINE_OVERRIDE
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
-#else
- if (builtin_cmdline[0]) {
- /* append boot loader cmdline to builtin */
- strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
- strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
- strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+ if (builtin_cmdline) {
+ /* Fix up the command line if an earlier stage hasn't */
+ if (!(boot_params.setup_flags & SETUP_CMDLINE_APPENDED)) {
+ if (append_cmdline) {
+ /* append boot loader cmdline to builtin */
+ strlcat(builtin_cmdline, " ",
+ COMMAND_LINE_SIZE);
+ strlcat(builtin_cmdline, boot_command_line,
+ COMMAND_LINE_SIZE);
+ }
+
+ strlcpy(boot_command_line, builtin_cmdline,
+ COMMAND_LINE_SIZE);
+ }
}
-#endif
-#endif

strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
*cmdline_p = command_line;
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
b/drivers/firmware/efi/libstub/efi-stub-helper.c
index f07d4a6..743e4a5 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,9 +12,22 @@

#include <linux/efi.h>
#include <asm/efi.h>
+#include <asm/setup.h>

#include "efistub.h"

+#ifdef CONFIG_CMDLINE_BOOL
+static char builtin_cmdline[] = CONFIG_CMDLINE;
+#else
+static char* builtin_cmdline;
+#endif
+
+#ifdef CONFIG_CMDLINE_OVERRIDE
+static bool append_cmdline;
+#else
+static bool append_cmdline = true;
+#endif
+
/*
* Some firmware implementations have problems reading files in one go.
* A read chunk size of 1MB seems to work for most platforms.
@@ -649,6 +662,21 @@ static u8 *efi_utf16_to_utf8(u8 *dst, const u16
*src, int n)
return dst;
}

+static size_t efi_strlcat(char *dest, const char *src, size_t count)
+{
+ size_t dsize = strlen(dest);
+ size_t len = strlen(src);
+ size_t res = dsize + len;
+
+ dest += dsize;
+ count -= dsize;
+ if (len >= count)
+ len = count-1;
+ memcpy(dest, src, len);
+ dest[len] = 0;
+ return res;
+}
+
/*
* Convert the unicode UEFI command line to ASCII to pass to kernel.
* Size of memory allocated return in *cmd_line_len.
@@ -658,42 +686,72 @@ char *efi_convert_cmdline(efi_system_table_t
*sys_table_arg,
efi_loaded_image_t *image,
int *cmd_line_len)
{
+ unsigned long cmdline_addr = 0;
+ int i;
+ efi_status_t status;
const u16 *s2;
u8 *s1 = NULL;
- unsigned long cmdline_addr = 0;
int load_options_chars = image->load_options_size / 2; /* UTF-16 */
const u16 *options = image->load_options;
int options_bytes = 0; /* UTF-8 bytes */
int options_chars = 0; /* UTF-16 chars */
- efi_status_t status;
u16 zero = 0;

- if (options) {
- s2 = options;
- while (*s2 && *s2 != '\n'
- && options_chars < load_options_chars) {
- options_bytes += efi_utf8_bytes(*s2++);
- options_chars++;
+ if (!builtin_cmdline || append_cmdline) {
+ if (options) {
+ s2 = options;
+ while (*s2 && *s2 != '\n'
+ && options_chars < load_options_chars) {
+ options_bytes += efi_utf8_bytes(*s2++);
+ options_chars++;
+ }
+ }
+
+ if (!options_chars) {
+ /* No command line options, so return empty string*/
+ options = &zero;
}
- }

- if (!options_chars) {
- /* No command line options, so return empty string*/
- options = &zero;
- }
+ options_bytes++; /* NUL termination */

- options_bytes++; /* NUL termination */
+ if (builtin_cmdline) {
+ /* Add length of the built-in command line + space */
+ options_bytes += strlen(builtin_cmdline);
+ options_bytes++;
+ }

- status = efi_low_alloc(sys_table_arg, options_bytes, 0, &cmdline_addr);
- if (status != EFI_SUCCESS)
- return NULL;
+ status = efi_low_alloc(sys_table_arg, options_bytes, 0,
+ &cmdline_addr);

- s1 = (u8 *)cmdline_addr;
- s2 = (const u16 *)options;
+ if (status != EFI_SUCCESS)
+ return NULL;

- s1 = efi_utf16_to_utf8(s1, s2, options_chars);
- *s1 = '\0';
+ s1 = (u8 *)cmdline_addr;
+ s2 = (const u16 *)options;
+
+ s1 = efi_utf16_to_utf8(s1, s2, options_chars);
+ *s1 = '\0';
+
+ if (builtin_cmdline) {
+ efi_strlcat((char *)cmdline_addr, " ",
+ COMMAND_LINE_SIZE);
+ efi_strlcat((char *)cmdline_addr, builtin_cmdline,
+ COMMAND_LINE_SIZE);
+ }
+ *cmd_line_len = options_bytes;
+ } else {
+ /* Ignore the provided command line, use the built-in one */
+ status = efi_low_alloc(sys_table_arg, strlen(builtin_cmdline),
+ 0, &cmdline_addr);
+ if (status != EFI_SUCCESS)
+ return NULL;
+ while (builtin_cmdline[i] && i < COMMAND_LINE_SIZE) {
+ ((char *)cmdline_addr)[i] = builtin_cmdline[i];
+ i++;
+ }
+ ((char *)cmdline_addr)[i] = '\0';
+ *cmd_line_len = strlen(builtin_cmdline);
+ }

- *cmd_line_len = options_bytes;
return (char *)cmdline_addr;
}

--
Matthew Garrett | [email protected]

2015-05-12 23:41:26

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH V2] x86: Allow built-in command line to work in early kernel init

On Tue, May 12, 2015 at 11:42 AM, Matthew Garrett
<[email protected]> wrote:
> Ok, maybe something more like this? It even gets rid of some of the
> #ifdefs in setup.c.
>
> diff --git a/arch/x86/boot/cmdline.c b/arch/x86/boot/cmdline.c
> index 625d21b..9ddffd0 100644
> --- a/arch/x86/boot/cmdline.c
> +++ b/arch/x86/boot/cmdline.c
> @@ -12,8 +12,15 @@
> * Simple command-line parser for early boot.
> */
>
> +#include <asm/cmdline_append.h>
> #include "boot.h"
>
> +#ifdef CONFIG_CMDLINE_BOOL
> +static char builtin_cmdline[] = CONFIG_CMDLINE;
> +#else
> +static char *builtin_cmdline;
> +#endif
> +
> static inline int myisspace(u8 c)
> {
> return c <= ' '; /* Close enough approximation */
> @@ -156,3 +163,38 @@ int __cmdline_find_option_bool(unsigned long
> cmdline_ptr, const char *option)
>
> return 0; /* Buffer overrun */
> }
> +
> +int __cmdline_init(unsigned long cmdline_ptr, struct boot_params *params)
> +{
> + addr_t cptr;
> + int i = 0;
> +
> + if (!builtin_cmdline)
> + return 0;
> +
> + if (!cmdline_ptr)
> + return -1; /* No command line */
> +
> + set_fs(cmdline_ptr >> 4);
> + cptr = cmdline_ptr & 0xf;
> +
> + if (append_cmdline) {
> + while (cptr < 0x10000) {
> + char c = rdfs8(cptr);
> + if (!c) {
> + wrfs8(' ', cptr++);
> + break;
> + }
> + cptr++;
> + }
> + }
> +
> + while (builtin_cmdline[i] && cptr < 0xffff)
> + wrfs8(builtin_cmdline[i++], cptr++);
> +
> + wrfs8('\0', cptr);
> +
> + params->setup_flags |= SETUP_CMDLINE_APPENDED;
> +
> + return 0;
> +}

You can not assume that you can use buffer after cmd_line from bootloader
blindly.

Thanks

Yinghai

2015-05-12 23:53:47

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH V2] x86: Allow built-in command line to work in early kernel init

On Tue, May 12, 2015 at 04:41:22PM -0700, Yinghai Lu wrote:

> You can not assume that you can use buffer after cmd_line from bootloader
> blindly.

Are there any bootloaders that don't allocate a buffer of the maximum
command line size?

--
Matthew Garrett | [email protected]

2015-05-13 02:46:42

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH V2] x86: Allow built-in command line to work in early kernel init

On 05/12/2015 04:53 PM, Matthew Garrett wrote:
> On Tue, May 12, 2015 at 04:41:22PM -0700, Yinghai Lu wrote:
>
>> You can not assume that you can use buffer after cmd_line from bootloader
>> blindly.
>
> Are there any bootloaders that don't allocate a buffer of the maximum
> command line size?
>

YES! In fact, it is quite common for the command line to be dynamically
allocated.

-hpa

2015-05-13 03:06:54

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH V2] x86: Allow built-in command line to work in early kernel init

On Tue, May 12, 2015 at 07:46:11PM -0700, H. Peter Anvin wrote:
> On 05/12/2015 04:53 PM, Matthew Garrett wrote:
> > On Tue, May 12, 2015 at 04:41:22PM -0700, Yinghai Lu wrote:
> >
> >> You can not assume that you can use buffer after cmd_line from bootloader
> >> blindly.
> >
> > Are there any bootloaders that don't allocate a buffer of the maximum
> > command line size?
> >
>
> YES! In fact, it is quite common for the command line to be dynamically
> allocated.

Hmm. Well, this is a little awkward. Let me think about it.

--
Matthew Garrett | [email protected]