[PATCH RESEND v5 0/2] x86/earlyprintk: setup earlyprintk as early as possible

[PATCH RESEND v5 0/2] x86/earlyprintk: setup earlyprintk as early as possible

[Alexander Kuleshov]

The early_printk function is usable after setup_early_printk will be executed. We
pass 'earlyprintk' through the kernel command line. It means that earlyprintk
will be usable only after the 'parse_early_param' will be executed. So we have
usable earlyprintk only during early boot, kernel decompression and after call
of the 'parse_early_param'. This patchset makes earlyprintk usable before the
call of the 'parse_early_param'.

These patchset provides two patches:

1. Move handling of the builtin command line to the separate function
from the setup_arch. Now we can call it from the arch/x86/kernel/head{32,64}.c,
and find 'earlyprintk' kernel command line paramter there.

2. Provide setup_serial_console function to setup serial earlyprintk in the
arch/x86/kernel/head{32,64}.c

Changes v5:

* Call setup_builtin_cmdline instead of setup_cmdline

Changes v4:

* Move setup_early_serial_console from the include/linux/printk.h
to the arch/x86/include/asm/serial.h, because this function is only
for x86 now.

Changes v3:

* Call setup_cmdline before setup_early_printk;
* setup_early_printk call wrapped with the setup_early_serial_console which
checks that 'serial' given to the earlyprintk command line option. This
prevents call of the setup_early_printk with the given pciserial/dbgp/efi,
because they are using early_ioremap.

Changes v2:

* Comment added before the setup_early_printk call;
* Added information about testing to the commit message.

Alexander Kuleshov (2):
  x86/setup: update boot_command_line with builtin_cmdline in separate
    function
  x86/earlyprintk: setup earlyprintk as early as possible

 arch/x86/include/asm/serial.h  |  2 ++
 arch/x86/include/asm/setup.h   |  3 ++-
 arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
 arch/x86/kernel/head32.c       |  6 ++++++
 arch/x86/kernel/head64.c       |  7 +++++++
 arch/x86/kernel/setup.c        | 28 +++++++++++++++-------------
 6 files changed, 57 insertions(+), 14 deletions(-)

-- 
2.4.0

[PATCH v2 1/2] x86/setup: introduce setup_bultin_cmdline

[Alexander Kuleshov]

This patch introduces setup_builtin_cmdline function which appends/overrides
boot_command_line with the builtin_cmdline if CONFIG_CMDLINE_BOOL is set.
Previously this functional was in the setup_arch, but we need to move
it for getting actual command line as early as possible in the
arch/x86/kernel/head{32,64}.c for the earlyprintk setup.

Changes:

v2: function renamed from setup_cmdline to setup_builtin_cmdline to
avoid conflict with the setup_cmdline from the arch/x86/kernel/kexec-bzimage64.c

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/x86/include/asm/setup.h |  3 ++-
 arch/x86/kernel/head32.c     |  1 +
 arch/x86/kernel/head64.c     |  1 +
 arch/x86/kernel/setup.c      | 28 +++++++++++++++-------------
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index f69e06b..dde474a 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -24,6 +24,7 @@
 #define OLD_CL_ADDRESS		0x020	/* Relative to real mode data */
 #define NEW_CL_POINTER		0x228	/* Relative to real mode data */
 
+
 #ifndef __ASSEMBLY__
 #include <asm/bootparam.h>
 #include <asm/x86_init.h>
@@ -117,8 +118,8 @@ asmlinkage void __init i386_start_kernel(void);
 #else
 asmlinkage void __init x86_64_start_kernel(char *real_mode);
 asmlinkage void __init x86_64_start_reservations(char *real_mode_data);
-
 #endif /* __i386__ */
+void __init setup_builtin_cmdline(void);
 #endif /* _SETUP */
 #else
 #define RESERVE_BRK(name,sz)				\
diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 2911ef3..95eed21 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -31,6 +31,7 @@ static void __init i386_default_early_setup(void)
 
 asmlinkage __visible void __init i386_start_kernel(void)
 {
+        setup_builtin_cmdline();
 	cr4_init_shadow();
 	sanitize_boot_params(&boot_params);
 
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 2b55ee6..df290d1 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -171,6 +171,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 	load_idt((const struct desc_ptr *)&idt_descr);
 
 	copy_bootdata(__va(real_mode_data));
+        setup_builtin_cmdline();
 
 	/*
 	 * Load microcode early on BSP.
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d74ac33..c9d1896 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -845,6 +845,21 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
 	return 0;
 }
 
+void __init setup_builtin_cmdline(void) {
+#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
+}
+
 /*
  * Determine if we were loaded by an EFI loader.  If so, then we have also been
  * passed the efi memmap, systab, etc., so we should use these data structures
@@ -973,19 +988,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;
 
-- 
2.4.0

Re: [PATCH v2 1/2] x86/setup: introduce setup_bultin_cmdline

[Borislav Petkov]

On Mon, May 11, 2015 at 04:09:07PM +0600, Alexander Kuleshov wrote:
> This patch introduces setup_builtin_cmdline function which appends/overrides
> boot_command_line with the builtin_cmdline if CONFIG_CMDLINE_BOOL is set.
> Previously this functional was in the setup_arch, but we need to move
> it for getting actual command line as early as possible in the
> arch/x86/kernel/head{32,64}.c for the earlyprintk setup.
> 
> Changes:
> 
> v2: function renamed from setup_cmdline to setup_builtin_cmdline to
> avoid conflict with the setup_cmdline from the arch/x86/kernel/kexec-bzimage64.c
> 
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  arch/x86/include/asm/setup.h |  3 ++-
>  arch/x86/kernel/head32.c     |  1 +
>  arch/x86/kernel/head64.c     |  1 +
>  arch/x86/kernel/setup.c      | 28 +++++++++++++++-------------
>  4 files changed, 19 insertions(+), 14 deletions(-)

Use checkpatch for your patches, some of the issues it points out are
valid:

ERROR: code indent should use tabs where possible
#63: FILE: arch/x86/kernel/head32.c:34:
+        setup_builtin_cmdline();$

WARNING: please, no spaces at the start of a line
#63: FILE: arch/x86/kernel/head32.c:34:
+        setup_builtin_cmdline();$

ERROR: code indent should use tabs where possible
#75: FILE: arch/x86/kernel/head64.c:174:
+        setup_builtin_cmdline();$

WARNING: please, no spaces at the start of a line
#75: FILE: arch/x86/kernel/head64.c:174:
+        setup_builtin_cmdline();$

ERROR: open brace '{' following function declarations go on the next line
#87: FILE: arch/x86/kernel/setup.c:848:
+void __init setup_builtin_cmdline(void) {

total: 3 errors, 3 warnings, 70 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

Your patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v2 1/2] x86/setup: introduce setup_bultin_cmdline

[Andy Shevchenko]

On Mon, 2015-05-11 at 16:09 +0600, Alexander Kuleshov wrote:
> This patch introduces setup_builtin_cmdline function which appends/overrides
> boot_command_line with the builtin_cmdline if CONFIG_CMDLINE_BOOL is set.
> Previously this functional was in the setup_arch, but we need to move
> it for getting actual command line as early as possible in the
> arch/x86/kernel/head{32,64}.c for the earlyprintk setup.
> 

Thanks for update. See my comments below. Don't forget to address what
Borislav told already twice.

> Changes:
> 
> v2: function renamed from setup_cmdline to setup_builtin_cmdline to
> avoid conflict with the setup_cmdline from the arch/x86/kernel/kexec-bzimage64.c

You mare remove those lines since it should be a part of cover letter
now. Or, if you still wish to have them, move after --- line.

> 
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  arch/x86/include/asm/setup.h |  3 ++-
>  arch/x86/kernel/head32.c     |  1 +
>  arch/x86/kernel/head64.c     |  1 +
>  arch/x86/kernel/setup.c      | 28 +++++++++++++++-------------
>  4 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index f69e06b..dde474a 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -24,6 +24,7 @@
>  #define OLD_CL_ADDRESS		0x020	/* Relative to real mode data */
>  #define NEW_CL_POINTER		0x228	/* Relative to real mode data */
>  
> +
>  #ifndef __ASSEMBLY__
>  #include <asm/bootparam.h>
>  #include <asm/x86_init.h>

This chunk is not needed.

> @@ -117,8 +118,8 @@ asmlinkage void __init i386_start_kernel(void);
>  #else
>  asmlinkage void __init x86_64_start_kernel(char *real_mode);
>  asmlinkage void __init x86_64_start_reservations(char *real_mode_data);
> -

Why? It shouldn't be here since it's redundant change.

>  #endif /* __i386__ */
> +void __init setup_builtin_cmdline(void);
>  #endif /* _SETUP */
>  #else
>  #define RESERVE_BRK(name,sz)				\
> diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
> index 2911ef3..95eed21 100644
> --- a/arch/x86/kernel/head32.c
> +++ b/arch/x86/kernel/head32.c
> @@ -31,6 +31,7 @@ static void __init i386_default_early_setup(void)
>  
>  asmlinkage __visible void __init i386_start_kernel(void)
>  {
> +        setup_builtin_cmdline();
>  	cr4_init_shadow();
>  	sanitize_boot_params(&boot_params);
>  
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 2b55ee6..df290d1 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -171,6 +171,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
>  	load_idt((const struct desc_ptr *)&idt_descr);
>  
>  	copy_bootdata(__va(real_mode_data));
> +        setup_builtin_cmdline();
>  
>  	/*
>  	 * Load microcode early on BSP.
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index d74ac33..c9d1896 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -845,6 +845,21 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
>  	return 0;
>  }
>  
> +void __init setup_builtin_cmdline(void) {
> +#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
> +}
> +

It might be a nice idea to split this to two patches:
1) split helper function,
2) update usage of it.

>  /*
>   * Determine if we were loaded by an EFI loader.  If so, then we have also been
>   * passed the efi memmap, systab, etc., so we should use these data structures
> @@ -973,19 +988,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;
>  


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

[PATCH v5 2/2] x86/earlyprintk: setup earlyprintk as early as possible

[Alexander Kuleshov]

The early_printk function is usable after setup_early_printk will be executed. We
pass 'earlyprintk' through the kernel command line. It means that earlyprintk
will be usable only after the 'parse_early_param' will be executed. So we have
usable earlyprintk only during early boot, kernel decompression and after call
of the 'parse_early_param'.

This patch makes setup_early_printk visible for head{32,64}.c So 'early_printk'
function will be usabable after decompression of the kernel and before
parse_early_param will be called. It also safe if CONFIG_CMDLINE_BOOL
and CONFIG_CMDLINE_OVERRIDE are set, because setup_cmdline function will be
called before the setup_early_printk.

Tested it with qemu, so early_printk() is usable and prints to serial console
right after setup_early_printk function called from arch/x86/kerne/head64.c

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/x86/include/asm/serial.h  |  2 ++
 arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
 arch/x86/kernel/head32.c       |  5 +++++
 arch/x86/kernel/head64.c       |  8 +++++++-
 4 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/serial.h b/arch/x86/include/asm/serial.h
index 8378b8c..be3b73a 100644
--- a/arch/x86/include/asm/serial.h
+++ b/arch/x86/include/asm/serial.h
@@ -26,4 +26,6 @@
 	{ .uart = 0,	BASE_BAUD,	0x3E8,	4,	STD_COMX_FLAGS	}, /* ttyS2 */	\
 	{ .uart = 0,	BASE_BAUD,	0x2E8,	3,	STD_COM4_FLAGS	}, /* ttyS3 */
 
+int __init setup_early_serial_console(void);
+
 #endif /* _ASM_X86_SERIAL_H */
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 89427d8..d7cc0b1 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -385,4 +385,29 @@ static int __init setup_early_printk(char *buf)
 	return 0;
 }
 
+int __init setup_early_serial_console(void)
+{
+#ifdef CONFIG_EARLY_PRINTK
+	char *arg;
+
+	/*
+	 * make sure that we have:
+	 *	"serial,0x3f8,115200"
+	 *	"serial,ttyS0,115200"
+	 *	"ttyS0,115200"
+	 */
+	arg = strstr(boot_command_line, "earlyprintk=serial");
+	if (!arg)
+		arg = strstr(boot_command_line, "earlyprintk=ttyS");
+	if (!arg)
+		return -1;
+
+	arg = strstr(boot_command_line, "earlyprintk=");
+	/* += strlen("earlyprintk"); */
+	arg += 12;
+
+	return setup_early_printk(arg);
+#endif
+}
+
 early_param("earlyprintk", setup_early_printk);
diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 95eed21..e863ecc 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -32,6 +32,11 @@ static void __init i386_default_early_setup(void)
 asmlinkage __visible void __init i386_start_kernel(void)
 {
         setup_builtin_cmdline();
+        setup_early_serial_console();
+
+	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
+                early_printk("Kernel alive\n");
+
 	cr4_init_shadow();
 	sanitize_boot_params(&boot_params);
 
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index df290d1..905e602 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -28,6 +28,7 @@
 #include <asm/bootparam_utils.h>
 #include <asm/microcode.h>
 #include <asm/kasan.h>
+#include <asm/serial.h>
 
 /*
  * Manage page tables very early on.
@@ -171,7 +172,12 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 	load_idt((const struct desc_ptr *)&idt_descr);
 
 	copy_bootdata(__va(real_mode_data));
-        setup_builtin_cmdline();
+
+	setup_builtin_cmdline();
+	setup_early_serial_console();
+
+	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
+		early_printk("Kernel alive\n");
 
 	/*
 	 * Load microcode early on BSP.
-- 
2.4.0

Re: [PATCH v5 2/2] x86/earlyprintk: setup earlyprintk as early as possible

[Borislav Petkov]

On Mon, May 11, 2015 at 04:09:18PM +0600, Alexander Kuleshov wrote:
> The early_printk function is usable after setup_early_printk will be executed. We
> pass 'earlyprintk' through the kernel command line. It means that earlyprintk
> will be usable only after the 'parse_early_param' will be executed. So we have
> usable earlyprintk only during early boot, kernel decompression and after call
> of the 'parse_early_param'.
> 
> This patch makes setup_early_printk visible for head{32,64}.c So 'early_printk'
> function will be usabable after decompression of the kernel and before
> parse_early_param will be called. It also safe if CONFIG_CMDLINE_BOOL
> and CONFIG_CMDLINE_OVERRIDE are set, because setup_cmdline function will be
> called before the setup_early_printk.
> 
> Tested it with qemu, so early_printk() is usable and prints to serial console
> right after setup_early_printk function called from arch/x86/kerne/head64.c
> 
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  arch/x86/include/asm/serial.h  |  2 ++
>  arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
>  arch/x86/kernel/head32.c       |  5 +++++
>  arch/x86/kernel/head64.c       |  8 +++++++-
>  4 files changed, 39 insertions(+), 1 deletion(-)

Also, when you send the patches, send them to yourself first and try
applying them. This way you can check whether your sending path doesn't
mangle them for some reason.

And ditto:

ERROR: code indent should use tabs where possible
#91: FILE: arch/x86/kernel/head32.c:35:
+        setup_early_serial_console();$

WARNING: please, no spaces at the start of a line
#91: FILE: arch/x86/kernel/head32.c:35:
+        setup_early_serial_console();$

ERROR: code indent should use tabs where possible
#94: FILE: arch/x86/kernel/head32.c:38:
+                early_printk("Kernel alive\n");$

WARNING: please, no spaces at the start of a line
#94: FILE: arch/x86/kernel/head32.c:38:
+                early_printk("Kernel alive\n");$

total: 2 errors, 3 warnings, 66 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

Your patch has style problems, please review.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH RESEND v5 0/2] x86/earlyprintk: setup earlyprintk as early as possible

[Andy Shevchenko]

On Mon, 2015-05-11 at 16:08 +0600, Alexander Kuleshov wrote:
> The early_printk function is usable after setup_early_printk will be executed. We
> pass 'earlyprintk' through the kernel command line. It means that earlyprintk
> will be usable only after the 'parse_early_param' will be executed. So we have
> usable earlyprintk only during early boot, kernel decompression and after call
> of the 'parse_early_param'. This patchset makes earlyprintk usable before the
> call of the 'parse_early_param'.

Better, though it would be good if native speaker fixes the wording.

P.S. Don't resend version too fast (wait for maybe couple of days for
others to give their comments). Actually this one must be v6, but you
may use that number in next iteration.

> 
> These patchset provides two patches:
> 
> 1. Move handling of the builtin command line to the separate function
> from the setup_arch. Now we can call it from the arch/x86/kernel/head{32,64}.c,
> and find 'earlyprintk' kernel command line paramter there.
> 
> 2. Provide setup_serial_console function to setup serial earlyprintk in the
> arch/x86/kernel/head{32,64}.c
> 
> Changes v5:
> 
> * Call setup_builtin_cmdline instead of setup_cmdline
> 
> Changes v4:
> 
> * Move setup_early_serial_console from the include/linux/printk.h
> to the arch/x86/include/asm/serial.h, because this function is only
> for x86 now.
> 
> Changes v3:
> 
> * Call setup_cmdline before setup_early_printk;
> * setup_early_printk call wrapped with the setup_early_serial_console which
> checks that 'serial' given to the earlyprintk command line option. This
> prevents call of the setup_early_printk with the given pciserial/dbgp/efi,
> because they are using early_ioremap.
> 
> Changes v2:
> 
> * Comment added before the setup_early_printk call;
> * Added information about testing to the commit message.
> 
> Alexander Kuleshov (2):
>   x86/setup: update boot_command_line with builtin_cmdline in separate
>     function
>   x86/earlyprintk: setup earlyprintk as early as possible
> 
>  arch/x86/include/asm/serial.h  |  2 ++
>  arch/x86/include/asm/setup.h   |  3 ++-
>  arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
>  arch/x86/kernel/head32.c       |  6 ++++++
>  arch/x86/kernel/head64.c       |  7 +++++++
>  arch/x86/kernel/setup.c        | 28 +++++++++++++++-------------
>  6 files changed, 57 insertions(+), 14 deletions(-)
> 


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

[PATCH v6 0/3]

[Alexander Kuleshov]

The early_printk function is usable after setup_early_printk will be executed. We
pass 'earlyprintk' through the kernel command line. It means that earlyprintk
will be usable only after the 'parse_early_param' will be executed. So we have
usable earlyprintk only during early boot, kernel decompression and after call
of the 'parse_early_param'. This patchset makes earlyprintk usable before the
call of the 'parse_early_param'.

These patchset provides following changes:

1. Move handling of the builtin command line to the separate function
from the setup_arch. Now we can call it from the arch/x86/kernel/head{32,64}.c,
and find 'earlyprintk' kernel command line paramter there.

2. Provide setup_serial_console function to setup serial earlyprintk in the
arch/x86/kernel/head{32,64}.c

v6:

* Style fixes.
* Call of the suetp_builtin_cmdline moved to the separate patch.

v5:

* Call setup_builtin_cmdline instead of setup_cmdline

v4:

* Move setup_early_serial_console from the include/linux/printk.h
to the arch/x86/include/asm/serial.h, because this function is only
for x86 now.

v3:

* Call setup_cmdline before setup_early_printk;
* setup_early_printk call wrapped with the setup_early_serial_console which
checks that 'serial' given to the earlyprintk command line option. This
prevents call of the setup_early_printk with the given pciserial/dbgp/efi,
because they are using early_ioremap.

v2:

* Comment added before the setup_early_printk call;
* Added information about testing to the commit message.

Alexander Kuleshov (3):
  x86/setup: introduce setup_bultin_cmdline
  x86/setup: setup builtin command line as early as possible
  x86/earlyprintk: setup earlyprintk as early as possible

 arch/x86/include/asm/serial.h  |  3 +++
 arch/x86/include/asm/setup.h   |  1 +
 arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
 arch/x86/kernel/head32.c       |  6 ++++++
 arch/x86/kernel/head64.c       |  6 ++++++
 arch/x86/kernel/setup.c        | 29 ++++++++++++++++-------------
 6 files changed, 57 insertions(+), 13 deletions(-)

--
2.4.0

[PATCH v3 1/3] x86/setup: introduce setup_bultin_cmdline

[Alexander Kuleshov]

This patch introduces the setup_builtin_cmdline function which appends or
overrides boot_command_line with the builtin_cmdline if CONFIG_CMDLINE_BOOL
is set.

Previously this functional was in the setup_arch, but we need to move
it for getting actual command line as early as possible in the
arch/x86/kernel/head{32,64}.c for the earlyprintk setup.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/x86/include/asm/setup.h |  1 +
 arch/x86/kernel/setup.c      | 29 ++++++++++++++++-------------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index f69e06b..59efd0d 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -119,6 +119,7 @@ asmlinkage void __init x86_64_start_kernel(char *real_mode);
 asmlinkage void __init x86_64_start_reservations(char *real_mode_data);

 #endif /* __i386__ */
+void __init setup_builtin_cmdline(void);
 #endif /* _SETUP */
 #else
 #define RESERVE_BRK(name,sz)				\
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d74ac33..0aeee0a 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -845,6 +845,22 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
 	return 0;
 }

+void __init setup_builtin_cmdline(void)
+{
+#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
+}
+
 /*
  * Determine if we were loaded by an EFI loader.  If so, then we have also been
  * passed the efi memmap, systab, etc., so we should use these data structures
@@ -973,19 +989,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;

--
2.4.0

Re: [PATCH v3 1/3] x86/setup: introduce setup_bultin_cmdline

[Andy Shevchenko]

On Tue, 2015-05-12 at 14:09 +0600, Alexander Kuleshov wrote:
> This patch introduces the setup_builtin_cmdline function which appends or
> overrides boot_command_line with the builtin_cmdline if CONFIG_CMDLINE_BOOL
> is set.
> 

Couple of comments.

First of all,6 is a property of each patch in the series, since series
means the logically linked pieces of changes. So, Why v3 is here?

> Previously this functional was in the setup_arch, but we need to move
> it for getting actual command line as early as possible in the
> arch/x86/kernel/head{32,64}.c for the earlyprintk setup.
> 
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  arch/x86/include/asm/setup.h |  1 +
>  arch/x86/kernel/setup.c      | 29 ++++++++++++++++-------------
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index f69e06b..59efd0d 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -119,6 +119,7 @@ asmlinkage void __init x86_64_start_kernel(char *real_mode);
>  asmlinkage void __init x86_64_start_reservations(char *real_mode_data);
> 
>  #endif /* __i386__ */
> +void __init setup_builtin_cmdline(void);
>  #endif /* _SETUP */
>  #else
>  #define RESERVE_BRK(name,sz)				\
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index d74ac33..0aeee0a 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -845,6 +845,22 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p)
>  	return 0;
>  }
> 
> +void __init setup_builtin_cmdline(void)
> +{
> +#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
> +}
> +
>  /*
>   * Determine if we were loaded by an EFI loader.  If so, then we have also been
>   * passed the efi memmap, systab, etc., so we should use these data structures
> @@ -973,19 +989,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
> -

This one breaks kernel to work. You have to call setup_builtin_cmdline()
here.

>  	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
>  	*cmdline_p = command_line;
> 
> --
> 2.4.0


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

Re: [PATCH v3 1/3] x86/setup: introduce setup_bultin_cmdline

[Alexander Kuleshov]

Hi Andy,


2015-05-12 16:50 GMT+06:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>
> This one breaks kernel to work. You have to call setup_builtin_cmdline()
> here.
>
>>       strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
>>       *cmdline_p = command_line;
>>

Why? The setup_builtin_cmdline will be called before setup_arch in the
head{32, 64}.c,
or I've missed something?

Re: [PATCH v3 1/3] x86/setup: introduce setup_bultin_cmdline

[Andy Shevchenko]

On Tue, 2015-05-12 at 22:14 +0600, Alexander Kuleshov wrote:
> Hi Andy,
> 
> 
> 2015-05-12 16:50 GMT+06:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> >
> > This one breaks kernel to work. You have to call setup_builtin_cmdline()
> > here.
> >
> >>       strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> >>       *cmdline_p = command_line;
> >>
> 
> Why? The setup_builtin_cmdline will be called before setup_arch in the
> head{32, 64}.c,
> or I've missed something?

Yes, you missed the call here in *this* patch.

So, here is a trade off between smaller understandable changes and
bisectability. On one hand you may go like you did previously (one patch
for two changes), on the other — make it work after each patch applied
one-by-one.

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

[PATCH v1 2/3] x86/setup: handle builtin command line as early as possible

[Alexander Kuleshov]

This patch adds the call of the setup_builtin_cmdline to
handle builtin command line before we will setup earlyprintk.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/x86/kernel/head32.c | 1 +
 arch/x86/kernel/head64.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 2911ef3..92e8b5f 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -31,6 +31,7 @@ static void __init i386_default_early_setup(void)

 asmlinkage __visible void __init i386_start_kernel(void)
 {
+	setup_builtin_cmdline();
 	cr4_init_shadow();
 	sanitize_boot_params(&boot_params);

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 2b55ee6..38da21c 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -171,6 +171,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 	load_idt((const struct desc_ptr *)&idt_descr);

 	copy_bootdata(__va(real_mode_data));
+	setup_builtin_cmdline();

 	/*
 	 * Load microcode early on BSP.
--
2.4.0

[PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible

[Alexander Kuleshov]

The early_printk function is usable only after the setup_early_printk will
be executed. We pass 'earlyprintk' through the kernel command line, so it
will be usable only after the 'parse_early_param' will be executed. This means
that we have usable earlyprintk only during early boot, kernel decompression
and after call of the 'parse_early_param'. This patchset makes earlyprintk
usable before the call of the 'parse_early_param'.

This patch makes setup_early_printk visible for head{32,64}.c. So the
'early_printk' function will be usabable after decompression of the
kernel and before parse_early_param will be called. It also must be
safe if CONFIG_CMDLINE_BOOL and CONFIG_CMDLINE_OVERRIDE are set, because
setup_cmdline function will be called before setup_early_printk.

Tested it with qemu, so early_printk() is usable and prints to serial
console right after setup_early_printk function called.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/x86/include/asm/serial.h  |  3 +++
 arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
 arch/x86/kernel/head32.c       |  5 +++++
 arch/x86/kernel/head64.c       |  5 +++++
 4 files changed, 38 insertions(+)

diff --git a/arch/x86/include/asm/serial.h b/arch/x86/include/asm/serial.h
index 8378b8c..198f041 100644
--- a/arch/x86/include/asm/serial.h
+++ b/arch/x86/include/asm/serial.h
@@ -26,4 +26,7 @@
 	{ .uart = 0,	BASE_BAUD,	0x3E8,	4,	STD_COMX_FLAGS	}, /* ttyS2 */	\
 	{ .uart = 0,	BASE_BAUD,	0x2E8,	3,	STD_COM4_FLAGS	}, /* ttyS3 */

+/* used by arch/x86/kernel/head{32,64}.c */
+int __init setup_early_serial_console(void);
+
 #endif /* _ASM_X86_SERIAL_H */
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 89427d8..96425c3 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -385,4 +385,29 @@ static int __init setup_early_printk(char *buf)
 	return 0;
 }

+int __init setup_early_serial_console(void)
+{
+#ifdef CONFIG_EARLY_PRINTK
+	char *arg;
+
+	/*
+	* make sure that we have:
+	*      "serial,0x3f8,115200"
+	*      "serial,ttyS0,115200"
+	*      "ttyS0,115200"
+	*/
+	arg = strstr(boot_command_line, "earlyprintk=serial");
+	if (!arg)
+		arg = strstr(boot_command_line, "earlyprintk=ttyS");
+	if (!arg)
+		return -1;
+
+	arg = strstr(boot_command_line, "earlyprintk=");
+	/* += strlen("earlyprintk"); */
+	arg += 12;
+
+	return setup_early_printk(arg);
+#endif
+}
+
 early_param("earlyprintk", setup_early_printk);
diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 92e8b5f..d325d0c 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -32,6 +32,11 @@ static void __init i386_default_early_setup(void)
 asmlinkage __visible void __init i386_start_kernel(void)
 {
 	setup_builtin_cmdline();
+	setup_early_serial_console();
+
+	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
+		early_printk("Kernel alive\n");
+
 	cr4_init_shadow();
 	sanitize_boot_params(&boot_params);

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 38da21c..06fcc1b 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -173,6 +173,11 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 	copy_bootdata(__va(real_mode_data));
 	setup_builtin_cmdline();

+	setup_early_serial_console();
+
+	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
+		early_printk("Kernel alive\n");
+
 	/*
 	 * Load microcode early on BSP.
 	 */
--
2.4.0

Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible

[Andy Shevchenko]

On Tue, 2015-05-12 at 14:10 +0600, Alexander Kuleshov wrote:
> The early_printk function is usable only after the setup_early_printk will
> be executed. We pass 'earlyprintk' through the kernel command line, so it
> will be usable only after the 'parse_early_param' will be executed. This means
> that we have usable earlyprintk only during early boot, kernel decompression
> and after call of the 'parse_early_param'. This patchset makes earlyprintk
> usable before the call of the 'parse_early_param'.
> 
> This patch makes setup_early_printk visible for head{32,64}.c. So the
> 'early_printk' function will be usabable after decompression of the
> kernel and before parse_early_param will be called. It also must be
> safe if CONFIG_CMDLINE_BOOL and CONFIG_CMDLINE_OVERRIDE are set, because
> setup_cmdline function will be called before setup_early_printk.
> 

Few comments below.

> Tested it with qemu, so early_printk() is usable and prints to serial
> console right after setup_early_printk function called.
> 
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  arch/x86/include/asm/serial.h  |  3 +++
>  arch/x86/kernel/early_printk.c | 25 +++++++++++++++++++++++++
>  arch/x86/kernel/head32.c       |  5 +++++
>  arch/x86/kernel/head64.c       |  5 +++++
>  4 files changed, 38 insertions(+)
> 
> diff --git a/arch/x86/include/asm/serial.h b/arch/x86/include/asm/serial.h
> index 8378b8c..198f041 100644
> --- a/arch/x86/include/asm/serial.h
> +++ b/arch/x86/include/asm/serial.h
> @@ -26,4 +26,7 @@
>  	{ .uart = 0,	BASE_BAUD,	0x3E8,	4,	STD_COMX_FLAGS	}, /* ttyS2 */	\
>  	{ .uart = 0,	BASE_BAUD,	0x2E8,	3,	STD_COM4_FLAGS	}, /* ttyS3 */
> 
> +/* used by arch/x86/kernel/head{32,64}.c */
> +int __init setup_early_serial_console(void);
> +

Actually, have you investigated how this works on the other supported
architectures? It might be better to align this with them.

>  #endif /* _ASM_X86_SERIAL_H */
> diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
> index 89427d8..96425c3 100644
> --- a/arch/x86/kernel/early_printk.c
> +++ b/arch/x86/kernel/early_printk.c
> @@ -385,4 +385,29 @@ static int __init setup_early_printk(char *buf)
>  	return 0;
>  }
> 
> +int __init setup_early_serial_console(void)
> +{
> +#ifdef CONFIG_EARLY_PRINTK
> +	char *arg;
> +
> +	/*
> +	* make sure that we have:
> +	*      "serial,0x3f8,115200"
> +	*      "serial,ttyS0,115200"
> +	*      "ttyS0,115200"
> +	*/

Indentation and text styling problems.

> +	arg = strstr(boot_command_line, "earlyprintk=serial");
> +	if (!arg)
> +		arg = strstr(boot_command_line, "earlyprintk=ttyS");
> +	if (!arg)
> +		return -1;

What about other cases like that described in setup_early_printk()?

> +
> +	arg = strstr(boot_command_line, "earlyprintk=");
> +	/* += strlen("earlyprintk"); */
> +	arg += 12;
> +
> +	return setup_early_printk(arg);
> +#endif

#ifdef seems redundant here, you already build this module iff
EARLY_PRINTK is set.

> +}

So, the logic of this function seems broken. I don't get why you have to
check the contents of earlyprintk parameter.

> +
>  early_param("earlyprintk", setup_early_printk);
> diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
> index 92e8b5f..d325d0c 100644
> --- a/arch/x86/kernel/head32.c
> +++ b/arch/x86/kernel/head32.c
> @@ -32,6 +32,11 @@ static void __init i386_default_early_setup(void)
>  asmlinkage __visible void __init i386_start_kernel(void)
>  {
>  	setup_builtin_cmdline();
> +	setup_early_serial_console();
> +
> +	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> +		early_printk("Kernel alive\n");

This should be unified (like printed "Early printk is initialized" in
setup_early_printk() ), moreover what about other architectures?


> +
>  	cr4_init_shadow();
>  	sanitize_boot_params(&boot_params);
> 
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 38da21c..06fcc1b 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -173,6 +173,11 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
>  	copy_bootdata(__va(real_mode_data));


>  	setup_builtin_cmdline();
> 
> +	setup_early_serial_console();

Those two can be grouped in the same way like in previous change (see
above).

> +
> +	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
> +		early_printk("Kernel alive\n");

Same comment as in above chunk.

> +
>  	/*
>  	 * Load microcode early on BSP.
>  	 */
> --
> 2.4.0


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible

[Alexander Kuleshov]

2015-05-12 17:19 GMT+06:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>> +/* used by arch/x86/kernel/head{32,64}.c */
>> +int __init setup_early_serial_console(void);
>> +
>
> Actually, have you investigated how this works on the other supported
> architectures? It might be better to align this with them.

Yes. In other architetures, earlyprintk setup occurs through 'early_param',
as it in the x86 now.

>
> What about other cases like that described in setup_early_printk()?
>
>> +
>> +     arg = strstr(boot_command_line, "earlyprintk=");
>> +     /* += strlen("earlyprintk"); */
>> +     arg += 12;
>> +
>> +     return setup_early_printk(arg);
>> +#endif
>
> So, the logic of this function seems broken. I don't get why you have to
> check the contents of earlyprintk parameter.
>

Because for now we can setup only serial console, for other we need ioremap
which is not enabled for this moment. Here we just try to find serial console
and setup it, if another argument passed to the 'earlyprintk', it will
be parsed in the
'setup_arch'.

>>
>> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
>> index 38da21c..06fcc1b 100644
>> --- a/arch/x86/kernel/head64.c
>> +++ b/arch/x86/kernel/head64.c
>> @@ -173,6 +173,11 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
>>       copy_bootdata(__va(real_mode_data));
>
>
>>       setup_builtin_cmdline();
>>
>> +     setup_early_serial_console();
>
> Those two can be grouped in the same way like in previous change (see
> above).
>

I'm not sure that I understand this. Can you please, explain what did
you mean here.

Thank you.

Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible

[Andy Shevchenko]

On Tue, 2015-05-12 at 22:26 +0600, Alexander Kuleshov wrote:
> 2015-05-12 17:19 GMT+06:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> >> +/* used by arch/x86/kernel/head{32,64}.c */
> >> +int __init setup_early_serial_console(void);
> >> +
> >
> > Actually, have you investigated how this works on the other supported
> > architectures? It might be better to align this with them.
> 
> Yes. In other architetures, earlyprintk setup occurs through 'early_param',
> as it in the x86 now.

So, which means that instead of this proposal (in a hackish way, since
it half-working solution) maybe better to reconsider how you may handle
early_param?

> 
> >
> > What about other cases like that described in setup_early_printk()?
> >
> >> +
> >> +     arg = strstr(boot_command_line, "earlyprintk=");
> >> +     /* += strlen("earlyprintk"); */
> >> +     arg += 12;
> >> +
> >> +     return setup_early_printk(arg);
> >> +#endif
> >
> > So, the logic of this function seems broken. I don't get why you have to
> > check the contents of earlyprintk parameter.
> >
> 
> Because for now we can setup only serial console, for other we need ioremap
> which is not enabled for this moment. Here we just try to find serial console
> and setup it, if another argument passed to the 'earlyprintk', it will
> be parsed in the
> 'setup_arch'.

Even for EFI case?

So, you might redesign that to somehow test if the setup_early_printk()
is called in early_param() context or even earlier and depending on that
do initializations regarding to possibilities, though I have no idea how
to this in clean way.

Currently you have two places where you check the content of the
parameter, which is not okay from my p.o.v.

> 
> >>
> >> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> >> index 38da21c..06fcc1b 100644
> >> --- a/arch/x86/kernel/head64.c
> >> +++ b/arch/x86/kernel/head64.c
> >> @@ -173,6 +173,11 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
> >>       copy_bootdata(__va(real_mode_data));
> >
> >
> >>       setup_builtin_cmdline();
> >>
> >> +     setup_early_serial_console();
> >
> > Those two can be grouped in the same way like in previous change (see
> > above).
> >
> 
> I'm not sure that I understand this. Can you please, explain what did
> you mean here.

It's about style. Just make empty line before setup_builtin_cmdline()
instead of doing this in between two setup_ functions.

> 
> Thank you.


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy

Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible

[Alexander Kuleshov]

2015-05-12 23:58 GMT+06:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>> >
>>
>> Because for now we can setup only serial console, for other we need ioremap
>> which is not enabled for this moment. Here we just try to find serial console
>> and setup it, if another argument passed to the 'earlyprintk', it will
>> be parsed in the
>> 'setup_arch'.
>
> Even for EFI case?
>

If I understand correctly, yes. As we can read from early_efi_map_fb function
(which setup with early_initcall) comment: efi earlyprintk need use
early_ioremap
to map the framebuffer.

[RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Alexander Kuleshov]

The early_printk function is usable only after the setup_early_printk will
be executed. We pass 'earlyprintk' through the kernel command line. So, it
means that earlyprintk will be usable only after the 'parse_early_param'
will be executed. We have usable earlyprintk only during early boot, kernel
decompression and after call of the 'parse_early_param'. This patchset makes
serial earlyprintk usable before the call of the 'parse_early_param'.

These patchset provides following changes:

1. Move handling of the builtin command line to the separate function
from the setup_arch. Now we can call it from the arch/x86/kernel/head{32,64}.c,
and find 'earlyprintk' kernel command line paramter there.

2. Provide setup_serial_console function to setup serial earlyprintk in the
arch/x86/kernel/head{32,64}.c

v6:

* Style fixes.
* Call of the suetp_builtin_cmdline moved to the separate patch.
* Move setup_early_serial_console to the arch/x86/include/setup.h
* Add ifdefs to prevent setup_serial_console if CONFIG_EARLY_PRINTK
is not set.

v5:

* Call setup_builtin_cmdline instead of setup_cmdline

v4:

* Move setup_early_serial_console from the include/linux/printk.h
to the arch/x86/include/asm/serial.h, because this function is only
for x86 now.

v3:

* Call setup_cmdline before setup_early_printk;
* setup_early_printk call wrapped with the setup_early_serial_console which
checks that 'serial' given to the earlyprintk command line option. This
prevents call of the setup_early_printk with the given pciserial/dbgp/efi,
because they are using early_ioremap.

v2:

* Comment added before the setup_early_printk call;
* Added information about testing to the commit message.

Alexander Kuleshov (3):
  x86/setup: introduce setup_bultin_cmdline
  x86/setup: handle builtin command line as early as possible
  x86/earlyprintk: setup earlyprintk as early as possible

 arch/x86/include/asm/setup.h   |  8 ++++++++
 arch/x86/kernel/early_printk.c | 24 ++++++++++++++++++++++++
 arch/x86/kernel/head32.c       |  7 +++++++
 arch/x86/kernel/head64.c       |  7 +++++++
 arch/x86/kernel/setup.c        | 33 ++++++++++++++++++---------------
 5 files changed, 64 insertions(+), 15 deletions(-)

--
2.4.0

[PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible

[Alexander Kuleshov]

The early_printk function is usable only after the setup_early_printk will
be executed. We pass 'earlyprintk' through the kernel command line, so it
will be usable only after the 'parse_early_param' will be executed. This means
that we have usable earlyprintk only during early boot, kernel decompression
and after call of the 'parse_early_param'. This patchset makes earlyprintk
usable before the call of the 'parse_early_param'.

This patch makes the setup_early_printk visible for head{32,64}.c. So the
'early_printk' function will be usabable after decompression of the
kernel and before parse_early_param will be called. It also must be
safe if CONFIG_CMDLINE_BOOL and CONFIG_CMDLINE_OVERRIDE are set, because
setup_cmdline function will be called before setup_early_printk.

It provides earlyprintk only for serial console, because other needs in
ioremap which is not initialized yet.

Tested it with qemu, so early_printk() is usable and prints to serial
console right after setup_early_printk function called.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/x86/include/asm/setup.h   |  7 +++++++
 arch/x86/kernel/early_printk.c | 24 ++++++++++++++++++++++++
 arch/x86/kernel/head32.c       |  6 ++++++
 arch/x86/kernel/head64.c       |  5 +++++
 4 files changed, 42 insertions(+)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 59efd0d..4234ada 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -120,6 +120,13 @@ asmlinkage void __init x86_64_start_reservations(char *real_mode_data);
 
 #endif /* __i386__ */
 void __init setup_builtin_cmdline(void);
+
+#ifdef CONFIG_EARLY_PRINTK
+/* used by arch/x86/kernel/head{32,64}.c */
+int __init setup_early_serial_console(void);
+#else
+int __init setup_early_serial_console(void) { }
+#endif /* CONFIG_EARLY_PRINTK */
 #endif /* _SETUP */
 #else
 #define RESERVE_BRK(name,sz)				\
diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
index 89427d8..0b2a626 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -385,4 +385,28 @@ static int __init setup_early_printk(char *buf)
 	return 0;
 }
 
+int __init setup_early_serial_console(void)
+{
+	char *arg;
+
+	/*
+	 * make sure that we have:
+	 *      "serial,0x3f8,115200"
+	 *      "serial,ttyS0,115200"
+	 *      "ttyS0,115200"
+	 */
+	arg = strstr(boot_command_line, "earlyprintk=serial");
+	if (!arg)
+		arg = strstr(boot_command_line, "earlyprintk=ttyS");
+	if (!arg)
+		return -1;
+
+	arg = strstr(boot_command_line, "earlyprintk=");
+
+	/* += strlen("earlyprintk"); */
+	arg += 12;
+
+	return setup_early_printk(arg);
+}
+
 early_param("earlyprintk", setup_early_printk);
diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 92e8b5f..759d115 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -32,6 +32,12 @@ static void __init i386_default_early_setup(void)
 asmlinkage __visible void __init i386_start_kernel(void)
 {
 	setup_builtin_cmdline();
+
+	setup_early_serial_console();
+
+	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
+		early_printk("Early printk is initialized\n");
+
 	cr4_init_shadow();
 	sanitize_boot_params(&boot_params);
 
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 346a042..6b7529a 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -174,7 +174,11 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 
 	setup_builtin_cmdline();
 
+	setup_early_serial_console();
+
+	if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
+		early_printk("Early printk is initialized\n");
+
 	/*
 	 * Load microcode early on BSP.
 	 */
--
2.4.0