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 or in another words earlyprintk is usable only during early
boot, kernel decompression and after call of the parse_early_param. But
there are many stuff after the kernel decompression and before the
parse_early_param will be called as memblock usage, early cpu initialization,
early ioremap initialization and etc... So earlyprintk allows us to see
what's going on there.
Patchset was tested for both x86 and x86_64 architectures with the qemu
and real hadware.
It provides call of the earlyprintk function right after the early
initialization of the serial console in the arch/x86/kernel/head{32,64.c},
so it must prints the debug string.
These patchset provides following changes:
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.
Provide setup_serial_console function to setup serial earlyprintk in the
arch/x86/kernel/head{32,64}.c
Changelog:
v11:
* setup_log_buf moved to the arch/x86/kernel/head{32,64.c} from
the arch/x86/kernel/setup.c to setup early log_buf for the earlyprintl
* Update log_buf in the early_printk function
* Added additional patch for testing earlyprintk
v10:
* Removed style issues which are not related to the patchset.
v9:
* Add call of the lockdep_init to the arch/x86/kernel/head{32,64}.c
before the serial console initialization to prevent:
[ 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] [] save_stack_trace+0x2f/0x50
[ 0.000000] [] __lock_acquire+0xa2c/0xf00
[ 0.000000] [] lock_acquire+0xdb/0x2b0
[ 0.000000] [] _raw_spin_lock_irqsave+0x53/0x90
[ 0.000000] [] down+0x16/0x50
[ 0.000000] [] console_lock+0x19/0x60
[ 0.000000] [] register_console+0x116/0x350
[ 0.000000] [] setup_early_printk+0x165/0x467
[ 0.000000] [] setup_early_serial_console+0x56/0x58
[ 0.000000] [] x86_64_start_kernel+0xce/0x110
[ 0.000000] [] 0xffffffffffffffff
[ 0.000000] ------------------------
during early serial console initialization.
* Add additional check to the earlyprintk initialization to protect double initialization
of the early_serial_console.
* Fixed comment.
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 (5):
x86/setup: introduce setup_bultin_cmdline
x86/setup: handle builtin command line as early as possible
x86/earlyprintk: Allocate log_buf as early as possible
x86/earlyprintk: setup earlyprintk as early as possible
x86/earlyprintk: Patch for testing earlyprintk
arch/x86/include/asm/setup.h | 7 +++++++
arch/x86/kernel/early_printk.c | 33 ++++++++++++++++++++++++++++++---
arch/x86/kernel/head32.c | 10 ++++++++++
arch/x86/kernel/head64.c | 11 +++++++++++
arch/x86/kernel/setup.c | 33 +++++++++++++++++----------------
init/main.c | 2 +-
kernel/printk/printk.c | 5 +++++
7 files changed, 81 insertions(+), 20 deletions(-)
--
2.4.0.GIT
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 <[email protected]>
---
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.GIT
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 <[email protected]>
---
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 5a46681..1e5f064 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 edd4857..0aeee0a 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -989,0 +989,0 @@ 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.GIT
This patch moves call of the early setup_log_buf from the
arch/x86/kernel/setup.c to the arch/x86/kernel/head{32,64}.c
and updates log_buf with the earlyprintk messages.
We need to do it because without it we will not see earlyprintk
messages in the kernel log.
Signed-off-by: Alexander Kuleshov <[email protected]>
---
arch/x86/kernel/head32.c | 6 ++++++
arch/x86/kernel/head64.c | 5 +++++
arch/x86/kernel/setup.c | 3 ---
kernel/printk/printk.c | 5 +++++
4 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
index 92e8b5f..f28d10f 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -32,2 +32,2 @@ static void __init i386_default_early_setup(void)
asmlinkage __visible void __init i386_start_kernel(void)
{
setup_builtin_cmdline();
+
+ lockdep_init();
+
+ /* Allocate early log buffer */
+ setup_log_buf(1);
+
cr4_init_shadow();
sanitize_boot_params(&boot_params);
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 1e5f064..53662d2 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -174,0 +174,0 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
setup_builtin_cmdline();
+ lockdep_init();
+
+ /* Allocate early log buffer */
+ setup_log_buf(1);
+
/*
* Load microcode early on BSP.
*/
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 0aeee0a..edfdb6a 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1146,9 +1146,6 @@ void __init setup_arch(char **cmdline_p)
if (init_ohci1394_dma_early)
init_ohci1394_dma_on_all_controllers();
#endif
- /* Allocate bigger log buffer */
- setup_log_buf(1);
-
reserve_initrd();
#if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index c099b08..d76a032 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1913,6 +1913,7 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
va_list ap;
char buf[512];
int n;
+ printk_func_t vprintk_func;
if (!early_console)
return;
@@ -1922,7 +1923,10 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
va_end(ap);
early_console->write(early_console, buf, n);
+
+ /* Store earlyprintk messages in the log_buf */
+ vprintk_func = this_cpu_read(printk_func);
+ vprintk_func(fmt, ap);
}
#endif
--
2.4.0.GIT
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 and real hardware, so early_printk() is usable and
prints to serial console right after setup_early_printk function called.
Signed-off-by: Alexander Kuleshov <[email protected]>
---
arch/x86/include/asm/setup.h | 6 ++++++
arch/x86/kernel/early_printk.c | 33 ++++++++++++++++++++++++++++++---
arch/x86/kernel/head32.c | 3 +++
arch/x86/kernel/head64.c | 5 ++++-
4 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 59efd0d..180bda4 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -120,0 +120,0 @@ 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 */
+extern int __init setup_early_serial_console(void);
+#else
+static inline int __init setup_early_serial_console(void) { return 0; }
+#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..6442cd2 100644
--- a/arch/x86/kernel/early_printk.c
+++ b/arch/x86/kernel/early_printk.c
@@ -342,19 +342,22 @@ static int __init setup_early_printk(char *buf)
keep = (strstr(buf, "keep") != NULL);
while (*buf != '\0') {
- if (!strncmp(buf, "serial", 6)) {
+ if (!strncmp(buf, "serial", 6) &&
+ early_serial_console.index == -1) {
buf += 6;
early_serial_init(buf);
early_console_register(&early_serial_console, keep);
if (!strncmp(buf, ",ttyS", 5))
buf += 5;
}
- if (!strncmp(buf, "ttyS", 4)) {
+ if (!strncmp(buf, "ttyS", 4) &&
+ early_serial_console.index == -1) {
early_serial_init(buf + 4);
early_console_register(&early_serial_console, keep);
}
#ifdef CONFIG_PCI
- if (!strncmp(buf, "pciserial", 9)) {
+ if (!strncmp(buf, "pciserial", 9) &&
+ early_serial_console.index == -1) {
early_pci_serial_init(buf + 9);
early_console_register(&early_serial_console, keep);
buf += 9; /* Keep from match the above "serial" */
@@ -385,4 +388,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 f28d10f..908ee47 100644
--- a/arch/x86/kernel/head32.c
+++ b/arch/x86/kernel/head32.c
@@ -38,0 +38,0 @@ asmlinkage __visible void __init i386_start_kernel(void)
/* Allocate early log buffer */
setup_log_buf(1);
+ setup_early_serial_console();
+ 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 53662d2..041be2c 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -176,0 +176,0 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
/* Allocate early log buffer */
setup_log_buf(1);
+ setup_early_serial_console();
+ early_printk("Early printk is initialized\n");
+
/*
* Load microcode early on BSP.
*/
--
2.4.0.GIT
Signed-off-by: Alexander Kuleshov <[email protected]>
---
arch/x86/kernel/head64.c | 1 +
arch/x86/kernel/setup.c | 1 +
init/main.c | 2 +-
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 041be2c..1d1d167 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -204,5 +204,6 @@ void __init x86_64_start_reservations(char *real_mode_data)
reserve_ebda_region();
+ early_printk("Go on the start_kernel\n");
start_kernel();
}
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index edfdb6a..22e8f39 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -876,6 +876,7 @@ void __init setup_builtin_cmdline(void)
void __init setup_arch(char **cmdline_p)
{
+ early_printk("Now setup arch is working\n");
memblock_reserve(__pa_symbol(_text),
(unsigned long)__bss_stop - (unsigned long)_text);
diff --git a/init/main.c b/init/main.c
index 2115055..79087b8 100644
--- a/init/main.c
+++ b/init/main.c
@@ -493,7 +493,7 @@ asmlinkage __visible void __init start_kernel(void)
{
char *command_line;
char *after_dashes;
-
+ early_printk("start_kernel function started to work\n");
/*
* Need to run as early as possible, to initialize the
* lockdep hash:
--
2.4.0.GIT
On Tue, 2015-06-09 at 17:11 +0600, Alexander Kuleshov wrote:
> Signed-off-by: Alexander Kuleshov <[email protected]>
Please, mark those patches as something like "[NOT TO APPLY]".
Or you may (if others have no objections) create a configuration option
like CONFIG_TEST_EARLY_PRINTK and make this patch upstreamable.
See below.
> ---
> arch/x86/kernel/head64.c | 1 +
> arch/x86/kernel/setup.c | 1 +
> init/main.c | 2 +-
> 3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 041be2c..1d1d167 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -204,5 +204,6 @@ void __init x86_64_start_reservations(char *real_mode_data)
>
> reserve_ebda_region();
>
> + early_printk("Go on the start_kernel\n");
#ifdef CONFIG_TEST_EARLY_PRINTK
early_printk("%s: Starting kernel...\n", __func__);
#endif
> start_kernel();
> }
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index edfdb6a..22e8f39 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -876,6 +876,7 @@ void __init setup_builtin_cmdline(void)
>
> void __init setup_arch(char **cmdline_p)
> {
> + early_printk("Now setup arch is working\n");
#ifdef CONFIG_TEST_EARLY_PRINTK
early_printk("%s: Enter\n", __func__);
#endif
> memblock_reserve(__pa_symbol(_text),
> (unsigned long)__bss_stop - (unsigned long)_text);
>
> diff --git a/init/main.c b/init/main.c
> index 2115055..79087b8 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -493,7 +493,7 @@ asmlinkage __visible void __init start_kernel(void)
> {
> char *command_line;
> char *after_dashes;
> -
This removal is wrong. Leave empty string after definition block.
> + early_printk("start_kernel function started to work\n");
#ifdef CONFIG_TEST_EARLY_PRINTK
early_printk("%s: Enter\n", __func__);
#endif
> /*
> * Need to run as early as possible, to initialize the
> * lockdep hash:
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On Tue, 2015-06-09 at 17:11 +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.
> 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.
This paragraph not related to the changes in the patch. So, remove it or
replace by something like "The patch is a preparation for further
improvements".
>
> Signed-off-by: Alexander Kuleshov <[email protected]>
> ---
> 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;
>
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On Tue, 2015-06-09 at 17:11 +0600, Alexander Kuleshov wrote:
> This patch moves call of the early setup_log_buf from the
> arch/x86/kernel/setup.c to the arch/x86/kernel/head{32,64}.c
> and updates log_buf with the earlyprintk messages.
>
Didn't mention lockdep_init() change. Regarding to what Borislav told
you how do you handle double call of lockdep_init()?
> We need to do it because without it we will not see earlyprintk
> messages in the kernel log.
>
> Signed-off-by: Alexander Kuleshov <[email protected]>
> ---
> arch/x86/kernel/head32.c | 6 ++++++
> arch/x86/kernel/head64.c | 5 +++++
> arch/x86/kernel/setup.c | 3 ---
> kernel/printk/printk.c | 5 +++++
> 4 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
> index 92e8b5f..f28d10f 100644
> --- a/arch/x86/kernel/head32.c
> +++ b/arch/x86/kernel/head32.c
> @@ -32,2 +32,2 @@ static void __init i386_default_early_setup(void)
> asmlinkage __visible void __init i386_start_kernel(void)
> {
> setup_builtin_cmdline();
> +
> + lockdep_init();
> +
> + /* Allocate early log buffer */
> + setup_log_buf(1);
> +
> cr4_init_shadow();
> sanitize_boot_params(&boot_params);
>
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 1e5f064..53662d2 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -174,0 +174,0 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
>
> setup_builtin_cmdline();
>
> + lockdep_init();
> +
> + /* Allocate early log buffer */
> + setup_log_buf(1);
> +
> /*
> * Load microcode early on BSP.
> */
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 0aeee0a..edfdb6a 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1146,9 +1146,6 @@ void __init setup_arch(char **cmdline_p)
> if (init_ohci1394_dma_early)
> init_ohci1394_dma_on_all_controllers();
> #endif
> - /* Allocate bigger log buffer */
> - setup_log_buf(1);
> -
> reserve_initrd();
>
> #if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD)
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index c099b08..d76a032 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1913,6 +1913,7 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> va_list ap;
> char buf[512];
> int n;
> + printk_func_t vprintk_func;
Rearrange this to be upper a bit, for example before char buf[].
>
> if (!early_console)
> return;
> @@ -1922,7 +1923,10 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> va_end(ap);
>
> early_console->write(early_console, buf, n);
> +
> + /* Store earlyprintk messages in the log_buf */
> + vprintk_func = this_cpu_read(printk_func);
> + vprintk_func(fmt, ap);
Shouldn't be
va_start(ap, fmt);
…
vprintk_func(…);
va_end(ap);
?
Also, when you print the same message to the serial and to kernel
buffer, do you avoid duplication? Your early_printk messages needs flag
LOG_NOCONS if I understand correctly.
> }
> #endif
>
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On Tue, Jun 09, 2015 at 06:58:52PM +0300, Andy Shevchenko wrote:
> On Tue, 2015-06-09 at 17:11 +0600, Alexander Kuleshov wrote:
> > This patch moves call of the early setup_log_buf from the
> > arch/x86/kernel/setup.c to the arch/x86/kernel/head{32,64}.c
> > and updates log_buf with the earlyprintk messages.
> >
>
> Didn't mention lockdep_init() change. Regarding to what Borislav told
> you how do you handle double call of lockdep_init()?
>
>
> > We need to do it because without it we will not see earlyprintk
> > messages in the kernel log.
> >
> > Signed-off-by: Alexander Kuleshov <[email protected]>
> > ---
> > arch/x86/kernel/head32.c | 6 ++++++
> > arch/x86/kernel/head64.c | 5 +++++
> > arch/x86/kernel/setup.c | 3 ---
> > kernel/printk/printk.c | 5 +++++
> > 4 files changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
> > index 92e8b5f..f28d10f 100644
> > --- a/arch/x86/kernel/head32.c
> > +++ b/arch/x86/kernel/head32.c
> > @@ -32,2 +32,2 @@ static void __init i386_default_early_setup(void)
> > asmlinkage __visible void __init i386_start_kernel(void)
> > {
> > setup_builtin_cmdline();
> > +
> > + lockdep_init();
> > +
> > + /* Allocate early log buffer */
> > + setup_log_buf(1);
> > +
> > cr4_init_shadow();
> > sanitize_boot_params(&boot_params);
> >
> > diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> > index 1e5f064..53662d2 100644
> > --- a/arch/x86/kernel/head64.c
> > +++ b/arch/x86/kernel/head64.c
> > @@ -174,0 +174,0 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
> >
> > setup_builtin_cmdline();
> >
> > + lockdep_init();
> > +
> > + /* Allocate early log buffer */
> > + setup_log_buf(1);
> > +
> > /*
> > * Load microcode early on BSP.
> > */
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 0aeee0a..edfdb6a 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -1146,9 +1146,6 @@ void __init setup_arch(char **cmdline_p)
> > if (init_ohci1394_dma_early)
> > init_ohci1394_dma_on_all_controllers();
> > #endif
> > - /* Allocate bigger log buffer */
> > - setup_log_buf(1);
> > -
> > reserve_initrd();
> >
> > #if defined(CONFIG_ACPI) && defined(CONFIG_BLK_DEV_INITRD)
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index c099b08..d76a032 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -1913,6 +1913,7 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> > va_list ap;
> > char buf[512];
> > int n;
> > + printk_func_t vprintk_func;
>
> Rearrange this to be upper a bit, for example before char buf[].
>
> >
> > if (!early_console)
> > return;
> > @@ -1922,7 +1923,10 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> > va_end(ap);
> >
> > early_console->write(early_console, buf, n);
> > +
> > + /* Store earlyprintk messages in the log_buf */
> > + vprintk_func = this_cpu_read(printk_func);
> > + vprintk_func(fmt, ap);
>
> Shouldn't be
>
> va_start(ap, fmt);
> …
> vprintk_func(…);
> va_end(ap);
>
> ?
>
> Also, when you print the same message to the serial and to kernel
> buffer, do you avoid duplication? Your early_printk messages needs flag
> LOG_NOCONS if I understand correctly.
Calling printk_func that early - which basically is vprintk_default
- I'm not sure, TBH. He pulls setup_log_buf() up but I'm not sure
everything would be kosher that early in the boot. And I don't know the
whole "fun" of printk() to make an informed decision here.
Let me add akpm. Andrew, see above (not snipping it). Do you see any
problems with snatching that printk_func per-cpu pointer that early and
and using it to print to log_buf so that early_printk() calls very early
in the boot are visible in dmesg?
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
On Tue, Jun 09, 2015 at 06:30:18PM +0300, Andy Shevchenko wrote:
> On Tue, 2015-06-09 at 17:11 +0600, Alexander Kuleshov wrote:
> > Signed-off-by: Alexander Kuleshov <[email protected]>
>
> Please, mark those patches as something like "[NOT TO APPLY]".
>
> Or you may (if others have no objections) create a configuration option
> like CONFIG_TEST_EARLY_PRINTK and make this patch upstreamable.
I think this is too much. No need for it - it's not like you can
automatically test early printk - you need to go stare at dmesg. And
if you want to do that, you can sprinkle some early_printk() calls
yourself.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
On Tue, 2015-06-09 at 17:11 +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 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 and real hardware, so early_printk() is usable and
> prints to serial console right after setup_early_printk function called.
>
One topic to discuss below.
> Signed-off-by: Alexander Kuleshov <[email protected]>
> ---
> arch/x86/include/asm/setup.h | 6 ++++++
> arch/x86/kernel/early_printk.c | 33 ++++++++++++++++++++++++++++++---
> arch/x86/kernel/head32.c | 3 +++
> arch/x86/kernel/head64.c | 5 ++++-
> 4 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index 59efd0d..180bda4 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -120,0 +120,0 @@ 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 */
> +extern int __init setup_early_serial_console(void);
> +#else
> +static inline int __init setup_early_serial_console(void) { return 0; }
> +#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..6442cd2 100644
> --- a/arch/x86/kernel/early_printk.c
> +++ b/arch/x86/kernel/early_printk.c
> @@ -342,19 +342,22 @@ static int __init setup_early_printk(char *buf)
> keep = (strstr(buf, "keep") != NULL);
>
> while (*buf != '\0') {
> - if (!strncmp(buf, "serial", 6)) {
> + if (!strncmp(buf, "serial", 6) &&
> + early_serial_console.index == -1) {
> buf += 6;
> early_serial_init(buf);
> early_console_register(&early_serial_console, keep);
> if (!strncmp(buf, ",ttyS", 5))
> buf += 5;
> }
> - if (!strncmp(buf, "ttyS", 4)) {
> + if (!strncmp(buf, "ttyS", 4) &&
> + early_serial_console.index == -1) {
> early_serial_init(buf + 4);
> early_console_register(&early_serial_console, keep);
> }
> #ifdef CONFIG_PCI
> - if (!strncmp(buf, "pciserial", 9)) {
> + if (!strncmp(buf, "pciserial", 9) &&
> + early_serial_console.index == -1) {
> early_pci_serial_init(buf + 9);
> early_console_register(&early_serial_console, keep);
> buf += 9; /* Keep from match the above "serial" */
> @@ -385,4 +388,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);
> +}
I'm still not convincing by this code to be in that form and here. What
about to refactor setup_early_printk() to helper which will do parse
parameters to a let say structure where one of the flag will be
struct early_printk_param {
…
const char *arg;
bool serial;
}
Your function will be something like this
struct early_printk_param epp;
parse_early_printk_param(&epp);
if (!epp->serial)
return /* whatever error code */;
return setup_early_printk(epp.arg);
> +
> early_param("earlyprintk", setup_early_printk);
> diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c
> index f28d10f..908ee47 100644
> --- a/arch/x86/kernel/head32.c
> +++ b/arch/x86/kernel/head32.c
> @@ -38,0 +38,0 @@ asmlinkage __visible void __init i386_start_kernel(void)
> /* Allocate early log buffer */
> setup_log_buf(1);
>
> + setup_early_serial_console();
> + 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 53662d2..041be2c 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -176,0 +176,0 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
>
> /* Allocate early log buffer */
> setup_log_buf(1);
>
> + setup_early_serial_console();
> + early_printk("Early printk is initialized\n");
> +
> /*
> * Load microcode early on BSP.
> */
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
On Tue, Jun 09, 2015 at 06:58:52PM +0300, Andy Shevchenko wrote:
>> On Tue, 2015-06-09 at 17:11 +0600, Alexander Kuleshov wrote:
>> > This patch moves call of the early setup_log_buf from the
>> > arch/x86/kernel/setup.c to the arch/x86/kernel/head{32,64}.c
>> > and updates log_buf with the earlyprintk messages.
>> >
>>
>> Didn't mention lockdep_init() change. Regarding to what Borislav told
>> you how do you handle double call of lockdep_init()?
>>
Double call of the lockdep_init is safe, since it has
if (lockdep_initialized)
return;
at the beginning. Will update commit message with it.
>> Also, when you print the same message to the serial and to kernel
>> buffer, do you avoid duplication? Your early_printk messages needs flag
>> LOG_NOCONS if I understand correctly.
Yes, just checked. They will be duplicated. Thanks for advice about LOG_NOCONS.
Borislav, I'm really not sure too, that using of printk to update
log_buf with the earlyprintk
is a right correct here. So, we can update log_buf with earlyprintk
messages with the call
of the
log_store(0, LOGLEVEL_DEFAULT, LOG_NOCONS|LOG_CONT, 0, NULL, 0,
buf, strlen(buf));
in the early_printk function. What do you think about it?
Do we need to see earlyprintk messages in the dmesg output? Maybe
there need to create yet another option, something like:
CONFIG_EARLY_PRINTK_KERNEL_LOG
or something like this?
2015-06-09 23:00 GMT+06:00 Andy Shevchenko <[email protected]>:
>
> I'm still not convincing by this code to be in that form and here. What
> about to refactor setup_early_printk() to helper which will do parse
> parameters to a let say structure where one of the flag will be
> struct early_printk_param {
> …
> const char *arg;
> bool serial;
> }
>
> Your function will be something like this
>
> struct early_printk_param epp;
>
> parse_early_printk_param(&epp);
>
> if (!epp->serial)
> return /* whatever error code */;
>
> return setup_early_printk(epp.arg);
>
Hello Andy,
But what is difference between parsing to string and
passing it and parsing to structure and pass its field?
Thank you.
On Tue, Jun 09, 2015 at 11:37:27PM +0600, Alexander Kuleshov wrote:
> Borislav, I'm really not sure too, that using of printk to update
> log_buf with the earlyprintk is a right correct here.
Yes, so this whole approach and what you're trying to achieve
seems kinda confusing and wrong. First of all, the early_printk()
machinery prints to a special console driver, i.e., I'm looking at the
registration fun in setup_early_printk().
So using early_printk() to print to dmesg is the wrong tool for the
job. Actually, if you want to do that, you can just as well use plain
printk() and try to make it work much earlier. Which is basically what
you did by using the printk_func per_cpu ptr, but that was hacky and
ugly.
In order to do that right, you need to slow down first, think hard
and look hard and long at printk(), log_buf, the statically allocated
smaller __log_buf and the whole machinery behind it. Whether it can be
used that early or not. And to explain why it can or why it cannot in
your commit messages. Then test your stuff a *lot* on the hw you have
access to because printk() is not a joke. It needs to be very reliable
and to work.
If you don't do your homework and expect other people to do it, you will
have a lot less success with your patches.
I sincerely hope that helps.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
2015-06-10 15:04 GMT+06:00 Borislav Petkov <[email protected]>:
> Yes, so this whole approach and what you're trying to achieve
> seems kinda confusing and wrong. First of all, the early_printk()
> machinery prints to a special console driver, i.e., I'm looking at the
> registration fun in setup_early_printk().
>
> So using early_printk() to print to dmesg is the wrong tool for the
> job. Actually, if you want to do that, you can just as well use plain
> printk() and try to make it work much earlier. Which is basically what
> you did by using the printk_func per_cpu ptr, but that was hacky and
> ugly.
>
> In order to do that right, you need to slow down first, think hard
> and look hard and long at printk(), log_buf, the statically allocated
> smaller __log_buf and the whole machinery behind it. Whether it can be
> used that early or not. And to explain why it can or why it cannot in
> your commit messages. Then test your stuff a *lot* on the hw you have
> access to because printk() is not a joke. It needs to be very reliable
> and to work.
>
That's right, I thought this too. But when you answered on 10th
(https://lkml.org/lkml/2015/6/8/157) revision that you do not see
earlyprintk messages in the dmesg output and I thought that he
should be there, but didint research printk/log_buf and related
stuff properly.
I will try to learn all earlyprintk related things in more clear way and
resend the patch.
Thank you.
On Wed, 2015-06-10 at 00:00 +0600, Alexander Kuleshov wrote:
> 2015-06-09 23:00 GMT+06:00 Andy Shevchenko <[email protected]>:
> >
> > I'm still not convincing by this code to be in that form and here. What
> > about to refactor setup_early_printk() to helper which will do parse
> > parameters to a let say structure where one of the flag will be
> > struct early_printk_param {
> > …
> > const char *arg;
> > bool serial;
> > }
> >
> > Your function will be something like this
> >
> > struct early_printk_param epp;
> >
> > parse_early_printk_param(&epp);
> >
> > if (!epp->serial)
> > return /* whatever error code */;
> >
> > return setup_early_printk(epp.arg);
> >
>
> Hello Andy,
>
> But what is difference between parsing to string and
> passing it and parsing to structure and pass its field?
You do parsing twice (still original code and your piece here), and
honestly I don't like your approach in this form.
>
> Thank you.
--
Andy Shevchenko <[email protected]>
Intel Finland Oy
2015-06-10 15:44 GMT+06:00 Andy Shevchenko <[email protected]>:
> You do parsing twice (still original code and your piece here), and
> honestly I don't like your approach in this form.
I just researched earlyprintk and we can use not only serial, but
vga and pciserial. What if I'll rename setup_early_serial_console
to the setup_earlyprintk_console, will add variable, something like
this
static unsigned int early = 1;
to the arch/x86/kernel/earlyprintk.c and refactor the
setup_early_serial_console as:
int __init setup_earlyprintk_console(void)
{
char *arg;
arg = strstr(boot_command_line, "earlyprintk=");
if (!arg)
return -1;
early = 0;
return setup_early_printk(arg);
}
After this we can check in the setup_earlyprintk
is it early to set other consoles or not,
while (*buf != '\0') {
....
#ifdef CONFIG_EARLY_PRINTK_EFI
if (!strncmp(buf, "efi", 3) && early)
early_console_register(&early_efi_console, keep);
#endif
....
}
early = 1;
So, when we will call setup_earlyprintk_console from the
arch/x86/kernel/head{32,64}.c, early variable will be 0 and
it allows us to setup only 'real early' consoles and when
setup_earlyprintk will be called by the do_early_param
early variable will be one and we will be able to setup
efi console and etc....
What do you think about it?
2015-06-10 16:36 GMT+06:00 Alexander Kuleshov <[email protected]>:
> 2015-06-10 15:44 GMT+06:00 Andy Shevchenko <[email protected]>:
>> You do parsing twice (still original code and your piece here), and
>> honestly I don't like your approach in this form.
>
> I just researched earlyprintk and we can use not only serial, but
> vga and pciserial. What if I'll rename setup_early_serial_console
> to the setup_earlyprintk_console, will add variable, something like
> this
>
We can't use pciserial, accidently hit from cut&paste,
sorry for the noise.