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

[PATCH v7 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

v7:

* Move setup_early_serial_console to the the arch/x86/include/setup.h
* Add ifdefs to prevent setup_serial_console if CONFIG_EARLY_PRINTK
is not set.

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: 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 v7 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      | 30 +++++++++++++++++-------------
 2 files changed, 18 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..edd4857 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,7 @@ 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
-
+	setup_builtin_cmdline();
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;
 
--
2.4.0

[PATCH v7 2/3] x86/setup: handle builtin command line early

[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 | 2 ++
 arch/x86/kernel/setup.c  | 1 -
 3 files changed, 3 insertions(+), 1 deletion(-)

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..346a042 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -172,6 +172,8 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
 
 	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 28dea1c..12124df 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -989,8 +989,6 @@ void __init setup_arch(char **cmdline_p)
 	bss_resource.start = __pa_symbol(__bss_start);
 	bss_resource.end = __pa_symbol(__bss_stop)-1;
 
-	setup_builtin_cmdline();
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;
 
--
2.4.0

[PATCH v7 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

[PATCH v8 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'.

Patchset was tested for both x86 and x86_64 architectures with the qemu.

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

v8:

* Fixed warning with the definition of the setup_early_serial_console
in the arch/x86/include/asm/setup.h

v7:

* Move setup_early_serial_console to the the arch/x86/include/setup.h
* Add ifdefs to prevent setup_serial_console if CONFIG_EARLY_PRINTK
is not set.

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: 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.350.g9532ead

[PATCH v8 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'.

Patchset was tested for both x86 and x86_64 architectures with the qemu.

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

v8:

* Fixed warning with the definition of the setup_early_serial_console
in the arch/x86/include/asm/setup.h

v7:

* Move setup_early_serial_console to the the arch/x86/include/setup.h
* Add ifdefs to prevent setup_serial_console if CONFIG_EARLY_PRINTK
is not set.

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: 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.350.g9532ead

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Borislav Petkov]

On Tue, May 26, 2015 at 07:06:47PM +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
> 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'.

... and?

Why do we need it usable before parse_early_param()? Use case?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Alexander Kuleshov]

2015-05-26 19:17 GMT+06:00 Borislav Petkov <bp@suse.de>:
> On Tue, May 26, 2015 at 07:06:47PM +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
>> 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'.
>
> ... and?
>
> Why do we need it usable before parse_early_param()? Use case?
>

To be able to use early_printk and serial console between kernel
decompression and
parse_eary_param.

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Borislav Petkov]

On Tue, May 26, 2015 at 07:20:02PM +0600, Alexander Kuleshov wrote:
> To be able to use early_printk and serial console between kernel
> decompression and parse_eary_param.

So no real use case - only a "just because we can" case.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Alexander Kuleshov]

2015-05-26 19:57 GMT+06:00 Borislav Petkov <bp@suse.de>:
> On Tue, May 26, 2015 at 07:20:02PM +0600, Alexander Kuleshov wrote:
>> To be able to use early_printk and serial console between kernel
>> decompression and parse_eary_param.
>
> So no real use case - only a "just because we can" case.
>

It is not "just because we can". There is some code after kernel decompression
and before parse_early_param and earlyprintk can be useful for
printing debugging information
in this case.

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Andy Shevchenko]

On Tue, 2015-05-26 at 20:11 +0600, Alexander Kuleshov wrote:
> 2015-05-26 19:57 GMT+06:00 Borislav Petkov <bp@suse.de>:
> > On Tue, May 26, 2015 at 07:20:02PM +0600, Alexander Kuleshov wrote:
> >> To be able to use early_printk and serial console between kernel
> >> decompression and parse_eary_param.
> >
> > So no real use case - only a "just because we can" case.
> >
> 
> It is not "just because we can". There is some code after kernel decompression
> and before parse_early_param and earlyprintk can be useful for
> printing debugging information
> in this case.

So, what is *yours* use case? From here you describe a *potential* use
case, not a real one.

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

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Borislav Petkov]

On Tue, May 26, 2015 at 08:11:07PM +0600, Alexander Kuleshov wrote:
> It is not "just because we can". There is some code after kernel
> decompression and before parse_early_param and earlyprintk can be
> useful for printing debugging information in this case.

Which code? Please show me.

Because *that* would be a use case.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Alexander Kuleshov]

Hello Borislav,

2015-05-27 3:34 GMT+06:00 Borislav Petkov <bp@suse.de>:
> On Tue, May 26, 2015 at 08:11:07PM +0600, Alexander Kuleshov wrote:
>> It is not "just because we can". There is some code after kernel
>> decompression and before parse_early_param and earlyprintk can be
>> useful for printing debugging information in this case.
>
> Which code? Please show me.
>
> Because *that* would be a use case.

We can't use earlyprintk from the arch/x86/kernel/head{32,64}.c before
parse_early_param from the
arch/x86/kernel/setup.c. But I think you know it better than me. So I
can't understand what's wrong
here, maybe you will give me explanation. I'm very appricate it.

Thank you.

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Borislav Petkov]

On Wed, May 27, 2015 at 01:33:23PM +0600, Alexander Kuleshov wrote:
> We can't use earlyprintk from the arch/x86/kernel/head{32,64}.c before
> parse_early_param from the arch/x86/kernel/setup.c.

Yes, and I'm simply asking why do we even want to use it there at all.
What do you think would need to be dumped with early_printk() calls
there and would be useful?

IOW, do you have a sensible use case for why we need it there or not?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Alexander Kuleshov]

2015-05-27 18:01 GMT+06:00 Borislav Petkov <bp@suse.de>:
> Yes, and I'm simply asking why do we even want to use it there at all.
> What do you think would need to be dumped with early_printk() calls
> there and would be useful?
>
> IOW, do you have a sensible use case for why we need it there or not?

Is early printk debugging sensible use case? We have memblock usage on the
early stage which uses printk for debugging which does not work on the
early stage,
early ioremap intitialization, early cpu initialization, many others
stuff and I'm not sure
that this code between head{32,64}.c and parse_early_param will remain
unchanged forever.
Somebody will update/add/remove something from it and ealyprintk
allows us to see what's
going on.

Is it bad or we no need in it? I want to understand not only how
kernel works, but also
why it written so and I can't understand why we have 'earlyprintk' (or
another feature) in
one place and do not have in another. Maybe there is explanation and
I very want to know about it.

Thank you Borislav.

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Borislav Petkov]

On Wed, May 27, 2015 at 06:12:59PM +0600, Alexander Kuleshov wrote:
> Is it bad or we no need in it? I want to understand not only how
> kernel works, but also why it written so and I can't understand why we
> have 'earlyprintk' (or another feature) in one place and do not have
> in another.

So how hard was it to say that the first time?

That you want to be able to dump interesting parts of the early boot
process and you would like the early console working as early as
possible?

This is what I wanted to know - what is your use case. Not what is there
and what isn't there in the code. That I can figure out by browsing the
code myself. Most of the time, at least. :-)

Now, have you tested this on real hardware? Because your 0/n message
said "qemu" but I don't think that's sufficient. If we're going to
enable early_printk this early, we'd need it working on real hardware
too.

For that I'd suggest adding a 4th, test patch which issues a couple of
numbered early_printk() lines which we could run on a bunch of machines
to see whether it actually works.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Alexander Kuleshov]

2015-05-27 18:51 GMT+06:00 Borislav Petkov <bp@suse.de>:
> Now, have you tested this on real hardware? Because your 0/n message
> said "qemu" but I don't think that's sufficient. If we're going to
> enable early_printk this early, we'd need it working on real hardware
> too.
>
> For that I'd suggest adding a 4th, test patch which issues a couple of
> numbered early_printk() lines which we could run on a bunch of machines
> to see whether it actually works.

Thank you for suggestions. But how to do it? The patch already contains:

if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
      early_printk("Early printk is initialized\n");

But I've just noticesed that it does not print anything, because
console_loglevel can be CONSOLE_LOGLEVEL_DEBUG
only after parse_early_params too :) How to better be with it? To remove
if clause and just leave early_printk or something else?

Or you meant something else about patch for testing?

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Borislav Petkov]

On Thu, May 28, 2015 at 02:01:12PM +0600, Alexander Kuleshov wrote:
> 2015-05-27 18:51 GMT+06:00 Borislav Petkov <bp@suse.de>:
> > Now, have you tested this on real hardware? Because your 0/n message
> > said "qemu" but I don't think that's sufficient. If we're going to
> > enable early_printk this early, we'd need it working on real hardware
> > too.
> >
> > For that I'd suggest adding a 4th, test patch which issues a couple of
> > numbered early_printk() lines which we could run on a bunch of machines
> > to see whether it actually works.
> 
> Thank you for suggestions. But how to do it? The patch already contains:
> 
> if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG)
>       early_printk("Early printk is initialized\n");

You can boot with "ignore_loglevel" to see that message. Provided
ignore_loglevel is even parsed that early.

> But I've just noticesed that it does not print anything, because
> console_loglevel can be CONSOLE_LOGLEVEL_DEBUG
> only after parse_early_params too :) How to better be with it? To remove
> if clause and just leave early_printk or something else?
> 
> Or you meant something else about patch for testing?

Yes, I meant a testing patch which is used only for testing and which
issues a bunch of early_printk() calls.

But, before you do that, your patchset has more problems. Booting in
qemu+kvm gives this:

early console in setup code
early console in decompress_kernel

Decompressing Linux... Parsing ELF... done.
Booting the kernel.
[    0.000000] bootconsole [earlyser0] enabled
[    0.000000] ERROR: earlyprintk= earlyser already used
	       ^^^^^^

[    0.000000] Linux version 4.1.0-rc5+ (boris@pd) (gcc version 4.9.2 (Debian 4.9.2-10) ) #3 SMP PREEMPT Fri May 29 18:41:59 CEST 2015
[    0.000000] Command line: root=/dev/sda1 debug ignore_loglevel log_buf_len=16M earlyprintk=ttyS0,115200 console=ttyS0,115200 console=tty0

...

[    0.000000] Console: colour VGA+ 80x25
[    0.000000] console [tty0] enabled
[    0.000000] bootconsole [earlyser0] disabled
[    0.000000] bootconsole [earlyser0] enabled
[    0.000000] ERROR: earlyprintk= earlyser already used

...

[    0.000000]  per task-struct memory footprint: 2688 bytes
[    0.000000] WARNING: lockdep init error! lock-(console_sem).lock was acquiredbefore lockdep_init
[    0.000000] Call stack leading to lockdep invocation was:
[    0.000000]  [<ffffffff81013f0f>] save_stack_trace+0x2f/0x50
[    0.000000]  [<ffffffff810a364c>] __lock_acquire+0xa2c/0xf00
[    0.000000]  [<ffffffff810a440b>] lock_acquire+0xdb/0x2b0
[    0.000000]  [<ffffffff81676b33>] _raw_spin_lock_irqsave+0x53/0x90
[    0.000000]  [<ffffffff8109b146>] down+0x16/0x50
[    0.000000]  [<ffffffff810b4d49>] console_lock+0x19/0x60
[    0.000000]  [<ffffffff810b78e6>] register_console+0x116/0x350
[    0.000000]  [<ffffffff81c36162>] setup_early_printk+0x165/0x467
[    0.000000]  [<ffffffff81c364ba>] setup_early_serial_console+0x56/0x58
[    0.000000]  [<ffffffff81c24565>] x86_64_start_kernel+0xce/0x110
[    0.000000]  [<ffffffffffffffff>] 0xffffffffffffffff
[    0.000000] ------------------------

This last one shows that you'd need to look into lockdep first. It also
shows that early boot order is pretty fragile so one has to be very
careful when changing stuff there.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v8 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible

[Alexander Kuleshov]

2015-05-29 22:55 GMT+06:00 Borislav Petkov <bp@suse.de>:
>
> You can boot with "ignore_loglevel" to see that message. Provided
> ignore_loglevel is even parsed that early.
>

But ignore_loglevel is early_param too...

>
> Yes, I meant a testing patch which is used only for testing and which
> issues a bunch of early_printk() calls.
>

Maybe I just remove 'if' clause and just leave:

early_printk("Early printk is initialized\n");

In the arch/x86/kernel/head{32,64}.c? Call of the early_printk
is safe here, even earlyprintk is not enabled and it must
be good for testing this patchset.

> But, before you do that, your patchset has more problems. Booting in
> qemu+kvm gives this:
>
> early console in setup code
> early console in decompress_kernel
>
> Decompressing Linux... Parsing ELF... done.
> Booting the kernel.
> [    0.000000] bootconsole [earlyser0] enabled
> [    0.000000] ERROR: earlyprintk= earlyser already used
>                ^^^^^^

Yes, thanks. This is because we go though a buffer in a loop
and after the earlyprintk=ttyS0, it finds console=ttyS0. So,
I think we can add additional check, something like this:

if (!strncmp(buf, "serial", 6) &&
    early_serial_console.index == -1) {
    ...
    ...
    ...
}

and for the "ttyS...". Just tested it and it works without
"ERROR: earlyprintk= earlyser already used".

>
> [    0.000000]  per task-struct memory footprint: 2688 bytes
> [    0.000000] WARNING: lockdep init error! lock-(console_sem).lock was acquiredbefore lockdep_init
> [    0.000000] Call stack leading to lockdep invocation was:
> [    0.000000]  [<ffffffff81013f0f>] save_stack_trace+0x2f/0x50
> [    0.000000]  [<ffffffff810a364c>] __lock_acquire+0xa2c/0xf00
> [    0.000000]  [<ffffffff810a440b>] lock_acquire+0xdb/0x2b0
> [    0.000000]  [<ffffffff81676b33>] _raw_spin_lock_irqsave+0x53/0x90
> [    0.000000]  [<ffffffff8109b146>] down+0x16/0x50
> [    0.000000]  [<ffffffff810b4d49>] console_lock+0x19/0x60
> [    0.000000]  [<ffffffff810b78e6>] register_console+0x116/0x350
> [    0.000000]  [<ffffffff81c36162>] setup_early_printk+0x165/0x467
> [    0.000000]  [<ffffffff81c364ba>] setup_early_serial_console+0x56/0x58
> [    0.000000]  [<ffffffff81c24565>] x86_64_start_kernel+0xce/0x110
> [    0.000000]  [<ffffffffffffffff>] 0xffffffffffffffff
> [    0.000000] ------------------------
>
> This last one shows that you'd need to look into lockdep first. It also
> shows that early boot order is pretty fragile so one has to be very
> careful when changing stuff there.
>

What if I add call of the lockdep_init before the setup_early_serial_console?
It must be safe because there is check in the lockdep_init:

if (lockdep_initialized)
    return;

and some architectures (powerpc, microblaze, s390) already do it in
the same way.

What do you think about it?

Thank you.