linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible
@ 2015-05-18  8:46 Alexander Kuleshov
  2015-05-18  8:47 ` [PATCH v6 1/3] x86/setup: introduce setup_bultin_cmdline Alexander Kuleshov
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-18  8:46 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin
  Cc: Alexander Kuleshov, Andy Shevchenko, LKML, Greg Kroah-Hartman,
	Borislav Petkov, Mark Rustad, Yinghai Lu

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6 1/3] x86/setup: introduce setup_bultin_cmdline
  2015-05-18  8:46 [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible Alexander Kuleshov
@ 2015-05-18  8:47 ` Alexander Kuleshov
  2015-05-18  8:47 ` [PATCH v6 2/3] x86/setup: handle builtin command line early Alexander Kuleshov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-18  8:47 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin
  Cc: Alexander Kuleshov, Andy Shevchenko, LKML, Greg Kroah-Hartman,
	Borislav Petkov, Mark Rustad, Yinghai Lu

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 2/3] x86/setup: handle builtin command line early
  2015-05-18  8:46 [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible Alexander Kuleshov
  2015-05-18  8:47 ` [PATCH v6 1/3] x86/setup: introduce setup_bultin_cmdline Alexander Kuleshov
@ 2015-05-18  8:47 ` Alexander Kuleshov
  2015-05-18  8:47 ` [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible Alexander Kuleshov
  2015-05-18  8:57 ` [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial " Ingo Molnar
  3 siblings, 0 replies; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-18  8:47 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin
  Cc: Alexander Kuleshov, Andy Shevchenko, LKML, Greg Kroah-Hartman,
	Borislav Petkov, Mark Rustad, Yinghai Lu

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible
  2015-05-18  8:46 [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible Alexander Kuleshov
  2015-05-18  8:47 ` [PATCH v6 1/3] x86/setup: introduce setup_bultin_cmdline Alexander Kuleshov
  2015-05-18  8:47 ` [PATCH v6 2/3] x86/setup: handle builtin command line early Alexander Kuleshov
@ 2015-05-18  8:47 ` Alexander Kuleshov
  2015-05-18  8:57 ` [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial " Ingo Molnar
  3 siblings, 0 replies; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-18  8:47 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin
  Cc: Alexander Kuleshov, Andy Shevchenko, LKML, Greg Kroah-Hartman,
	Borislav Petkov, Mark Rustad, Yinghai Lu

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible
  2015-05-18  8:46 [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible Alexander Kuleshov
                   ` (2 preceding siblings ...)
  2015-05-18  8:47 ` [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible Alexander Kuleshov
@ 2015-05-18  8:57 ` Ingo Molnar
  2015-05-18  9:00   ` Alexander Kuleshov
  3 siblings, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2015-05-18  8:57 UTC (permalink / raw)
  To: Alexander Kuleshov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andy Shevchenko,
	LKML, Greg Kroah-Hartman, Borislav Petkov, Mark Rustad,
	Yinghai Lu


So why is this marked a 'RESEND'? Isn't this a new iteration?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible
  2015-05-18  8:57 ` [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial " Ingo Molnar
@ 2015-05-18  9:00   ` Alexander Kuleshov
  2015-05-18  9:08     ` Ingo Molnar
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-18  9:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andy Shevchenko,
	LKML, Greg Kroah-Hartman, Borislav Petkov, Mark Rustad,
	Yinghai Lu

Hello Ingo,

Yes, it is new iteration, but it contains mostly cosmetical fixes and
I was not sure how to mark this patchset correctly.

How to be with it? Need to update changelog with this fixes and resend
it with v7?


2015-05-18 14:57 GMT+06:00 Ingo Molnar <mingo@kernel.org>:
>
> So why is this marked a 'RESEND'? Isn't this a new iteration?
>
> Thanks,
>
>         Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible
  2015-05-18  9:00   ` Alexander Kuleshov
@ 2015-05-18  9:08     ` Ingo Molnar
  0 siblings, 0 replies; 12+ messages in thread
From: Ingo Molnar @ 2015-05-18  9:08 UTC (permalink / raw)
  To: Alexander Kuleshov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andy Shevchenko,
	LKML, Greg Kroah-Hartman, Borislav Petkov, Mark Rustad,
	Yinghai Lu


* Alexander Kuleshov <kuleshovmail@gmail.com> wrote:

> Hello Ingo,
> 
> Yes, it is new iteration, but it contains mostly cosmetical fixes 
> and I was not sure how to mark this patchset correctly.

By marking it as v7 and by not adding a pushy 'RESEND', which only 
makes reviewers skip it?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible
  2015-05-12 17:58       ` Andy Shevchenko
@ 2015-05-12 18:08         ` Alexander Kuleshov
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-12 18:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, LKML,
	Greg Kroah-Hartman, Borislav Petkov, Mark Rustad, Yinghai Lu

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible
  2015-05-12 16:26     ` Alexander Kuleshov
@ 2015-05-12 17:58       ` Andy Shevchenko
  2015-05-12 18:08         ` Alexander Kuleshov
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2015-05-12 17:58 UTC (permalink / raw)
  To: Alexander Kuleshov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, LKML,
	Greg Kroah-Hartman, Borislav Petkov, Mark Rustad, Yinghai Lu

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible
  2015-05-12 11:19   ` Andy Shevchenko
@ 2015-05-12 16:26     ` Alexander Kuleshov
  2015-05-12 17:58       ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-12 16:26 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, LKML,
	Greg Kroah-Hartman, Borislav Petkov, Mark Rustad, Yinghai Lu

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible
  2015-05-12  8:10 ` [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible Alexander Kuleshov
@ 2015-05-12 11:19   ` Andy Shevchenko
  2015-05-12 16:26     ` Alexander Kuleshov
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2015-05-12 11:19 UTC (permalink / raw)
  To: Alexander Kuleshov
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, LKML,
	Greg Kroah-Hartman, Borislav Petkov, Mark Rustad, Yinghai Lu

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible
  2015-05-12  8:08 [PATCH v6 0/3] Alexander Kuleshov
@ 2015-05-12  8:10 ` Alexander Kuleshov
  2015-05-12 11:19   ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Kuleshov @ 2015-05-12  8:10 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, H. Peter Anvin
  Cc: Alexander Kuleshov, Andy Shevchenko, LKML, Greg Kroah-Hartman,
	Borislav Petkov, Mark Rustad, Yinghai Lu

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-05-18  9:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-18  8:46 [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial earlyprintk as early as possible Alexander Kuleshov
2015-05-18  8:47 ` [PATCH v6 1/3] x86/setup: introduce setup_bultin_cmdline Alexander Kuleshov
2015-05-18  8:47 ` [PATCH v6 2/3] x86/setup: handle builtin command line early Alexander Kuleshov
2015-05-18  8:47 ` [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible Alexander Kuleshov
2015-05-18  8:57 ` [RESEND PATCH v6 0/3] x86/earlyprintk: setup serial " Ingo Molnar
2015-05-18  9:00   ` Alexander Kuleshov
2015-05-18  9:08     ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2015-05-12  8:08 [PATCH v6 0/3] Alexander Kuleshov
2015-05-12  8:10 ` [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible Alexander Kuleshov
2015-05-12 11:19   ` Andy Shevchenko
2015-05-12 16:26     ` Alexander Kuleshov
2015-05-12 17:58       ` Andy Shevchenko
2015-05-12 18:08         ` Alexander Kuleshov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).