Messages in this thread | | | Subject | Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible | From | Andy Shevchenko <> | Date | Tue, 12 May 2015 14:19:13 +0300 |
| |
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
| |