Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932567AbbELLTY (ORCPT ); Tue, 12 May 2015 07:19:24 -0400 Received: from mga11.intel.com ([192.55.52.93]:33883 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932356AbbELLTV (ORCPT ); Tue, 12 May 2015 07:19:21 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,414,1427785200"; d="scan'208";a="492541168" Message-ID: <1431429553.28073.63.camel@linux.intel.com> Subject: Re: [PATCH v6 3/3] x86/earlyprintk: setup earlyprintk as early as possible From: Andy Shevchenko To: Alexander Kuleshov Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , LKML , Greg Kroah-Hartman , Borislav Petkov , Mark Rustad , Yinghai Lu Date: Tue, 12 May 2015 14:19:13 +0300 In-Reply-To: <1431418208-2274-1-git-send-email-kuleshovmail@gmail.com> References: <1431418137-32320-1-git-send-email-kuleshovmail@gmail.com> <1431418208-2274-1-git-send-email-kuleshovmail@gmail.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4777 Lines: 151 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 > --- > 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 Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/