Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753931AbbDHLc6 (ORCPT ); Wed, 8 Apr 2015 07:32:58 -0400 Received: from mail-qk0-f182.google.com ([209.85.220.182]:33285 "EHLO mail-qk0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751385AbbDHLc4 (ORCPT ); Wed, 8 Apr 2015 07:32:56 -0400 Message-ID: <552511E5.9060005@hurleysoftware.com> Date: Wed, 08 Apr 2015 07:32:53 -0400 From: Peter Hurley User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Alexander Kuleshov , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Greg Kroah-Hartman , Yinghai Lu CC: linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] x86/earlyprintk: setup earlyprintk as early as possible References: <1428485466-24326-1-git-send-email-kuleshovmail@gmail.com> <1428485519-25587-1-git-send-email-kuleshovmail@gmail.com> In-Reply-To: <1428485519-25587-1-git-send-email-kuleshovmail@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7056 Lines: 203 Hi Alexander, On 04/08/2015 05:31 AM, Alexander Kuleshov wrote: > As setup_early_printk passed to the early_param, it will be usable only after > 'parse_early_param' function will be called from the 'setup_arch'. So we have > earlyprintk during early boot and decompression. Next point after decompression > of the kernel where we can use early_printk is after call of the > 'parse_early_param'. > > This patch makes setup_early_printk visible for head{32,64}.c So '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 from arch/x86/kerne/head64.c > > Changes v1->v2: > > * Comment added before the setup_early_printk call; > * Added information about testing to the commit message. > > Changes v2->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. > > Signed-off-by: Alexander Kuleshov > --- > arch/x86/kernel/early_printk.c | 2 +- > arch/x86/kernel/head.c | 14 ++++++++++++++ > arch/x86/kernel/head32.c | 2 ++ > arch/x86/kernel/head64.c | 7 ++++++- > include/linux/printk.h | 4 ++++ > kernel/printk/printk.c | 11 +++++++---- > 6 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c > index a62536a..19a94e0 100644 > --- a/arch/x86/kernel/early_printk.c > +++ b/arch/x86/kernel/early_printk.c > @@ -329,7 +329,7 @@ static inline void early_console_register(struct console *con, int keep_early) > register_console(early_console); > } > > -static int __init setup_early_printk(char *buf) > +int __init setup_early_printk(char *buf) > { > int keep; > > diff --git a/arch/x86/kernel/head.c b/arch/x86/kernel/head.c > index 992f442..b6e72aa 100644 > --- a/arch/x86/kernel/head.c > +++ b/arch/x86/kernel/head.c > @@ -69,3 +69,17 @@ void __init reserve_ebda_region(void) > /* reserve all memory between lowmem and the 1MB mark */ > memblock_reserve(lowmem, 0x100000 - lowmem); > } > + > +void setup_early_serial_console() { If you put this function in kernel/early_printk.c, then setup_early_printk() can remain file scope. > +#ifdef CONFIG_EARLY_PRINTK > + char *arg; > + > + arg = strstr(boot_command_line, "earlyprintk=serial"); > + if (!arg) > + arg = strstr(boot_command_line, "earlyprintk=ttyS"); > + if (!arg) > + return; > + > + setup_early_printk(boot_command_line); Like Ingo already pointed out, you need to emit a printk() after the call to setup_early_printk() which validates that earlier-than-early is working. Because this isn't. setup_early_printk() expects the argument to point to the start of the earlyprintk= parameter; ie., earlyprintk=ttyS0,115200 ^ buf > +#endif > +} > diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c > index 7ad0ad0..64c7a80 100644 > --- a/arch/x86/kernel/head32.c > +++ b/arch/x86/kernel/head32.c > @@ -32,6 +32,8 @@ static void __init i386_default_early_setup(void) > asmlinkage __visible void __init i386_start_kernel(void) > { > setup_cmdline(); > + /* setup serial console as early as possible */ > + setup_early_serial_console(); > cr4_init_shadow(); > sanitize_boot_params(&boot_params); > > diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c > index 6eea2de..f0c03bd 100644 > --- a/arch/x86/kernel/head64.c > +++ b/arch/x86/kernel/head64.c > @@ -172,12 +172,15 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data) > > copy_bootdata(__va(real_mode_data)); > setup_cmdline(); > + /* setup serial console as early as possible */ > + setup_early_serial_console(); > > /* > * Load microcode early on BSP. > */ > load_ucode_bsp(); > > + > if (console_loglevel >= CONSOLE_LOGLEVEL_DEBUG) > early_printk("Kernel alive\n"); > > @@ -193,8 +196,10 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data) > void __init x86_64_start_reservations(char *real_mode_data) > { > /* version is always not zero if it is copied */ > - if (!boot_params.hdr.version) > + if (!boot_params.hdr.version) { > copy_bootdata(__va(real_mode_data)); > + setup_early_serial_console(); > + } > > reserve_ebda_region(); > > diff --git a/include/linux/printk.h b/include/linux/printk.h > index baa3f97..17358dd 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -115,9 +115,13 @@ int no_printk(const char *fmt, ...) > #ifdef CONFIG_EARLY_PRINTK > extern asmlinkage __printf(1, 2) > void early_printk(const char *fmt, ...); > +int setup_early_printk(char *buf); > +void setup_early_serial_console(void); > #else > static inline __printf(1, 2) __cold > void early_printk(const char *s, ...) { } > +int setup_early_printk(char *buf) { } > +static void setup_early_serial_console(void) { } > #endif These can't go here, because this will break the build of every non-x86 arch, since these symbols will not be defined in those arches. These need to be in x86 includes only. > > typedef int(*printk_func_t)(const char *fmt, va_list args); > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index bb0635b..e56285c 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2409,11 +2409,14 @@ void register_console(struct console *newcon) > struct console_cmdline *c; > > if (console_drivers) > - for_each_console(bcon) > - if (WARN(bcon == newcon, > - "console '%s%d' already registered\n", > - bcon->name, bcon->index)) > + for_each_console(bcon) { > + /* not again */ > + if (bcon == newcon) { > + printk(KERN_INFO "console '%s%d' already registered\n", > + bcon->name, bcon->index); > return; > + } > + } The WARN is still required here because the register_console() caller is not always obvious. A better way to achieve what you want is to not re-register the console to begin with. Looking at early_console_register() and setup_early_printk(), it seems like re-registration would already be prevented (since early_console != 0 after the first call to early_console_register())? Have you tried this? Regards, Peter Hurley > > /* > * before we register a new CON_BOOT console, make sure we don't > -- 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/