linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: Setup early console as early as possible
@ 2010-07-11 21:44 Yinghai Lu
  2010-07-12  8:58 ` Pekka Enberg
  0 siblings, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2010-07-11 21:44 UTC (permalink / raw)
  To: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andrew Morton
  Cc: Pekka Enberg, linux-kernel@vger.kernel.org


Analyze "console=uart8250,io,0x3f8,115200n8" in i386_start_kernel/x86_64_start_kernel,
and call setup_early_serial8250_console() to init early serial console.

only can handle io port kind of 8250. because mmio need ioremap.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/include/asm/setup.h |    1 +
 arch/x86/kernel/head.c       |   32 ++++++++++++++++++++++++++++++++
 arch/x86/kernel/head32.c     |    2 ++
 arch/x86/kernel/head64.c     |   11 +++++++++++
 kernel/printk.c              |    4 ++++
 5 files changed, 50 insertions(+)

Index: linux-2.6/arch/x86/include/asm/setup.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/setup.h
+++ linux-2.6/arch/x86/include/asm/setup.h
@@ -42,6 +42,7 @@ static inline void visws_early_detect(vo
 #endif
 
 extern unsigned long saved_video_mode;
+void setup_early_console(void);
 
 extern void reserve_standard_io_resources(void);
 extern void i386_reserve_resources(void);
Index: linux-2.6/arch/x86/kernel/head.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/head.c
+++ linux-2.6/arch/x86/kernel/head.c
@@ -53,3 +53,35 @@ void __init reserve_ebda_region(void)
 	/* reserve all memory between lowmem and the 1MB mark */
 	reserve_early_overlap_ok(lowmem, 0x100000, "BIOS reserved");
 }
+
+#ifdef CONFIG_SERIAL_8250_CONSOLE
+int setup_early_serial8250_console(char *cmdline);
+
+void __init setup_early_console(void)
+{
+	char constr[64], *p, *q;
+
+	/* Can not handle mmio type 8250 uart yet, too early */
+	p = strstr(boot_command_line, "console=uart8250,io,");
+	if (!p)
+		p = strstr(boot_command_line, "console=uart,io,");
+	if (!p)
+		return;
+
+	p += 8;	/* sizeof "console=" */
+	q = strchr(p, ' ');
+	if ((q - p) >= sizeof(constr))
+		return;
+
+	memset(constr, 0, sizeof(constr));
+	memcpy(constr, p, q - p);
+
+	lockdep_init();
+
+	setup_early_serial8250_console(constr);
+}
+#else
+void __init setup_early_console(void)
+{
+}
+#endif
Index: linux-2.6/arch/x86/kernel/head32.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/head32.c
+++ linux-2.6/arch/x86/kernel/head32.c
@@ -30,6 +30,8 @@ static void __init i386_default_early_se
 
 void __init i386_start_kernel(void)
 {
+	setup_early_console();
+
 #ifdef CONFIG_X86_TRAMPOLINE
 	/*
 	 * But first pinch a few for the stack/trampoline stuff
Index: linux-2.6/arch/x86/kernel/head64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/head64.c
+++ linux-2.6/arch/x86/kernel/head64.c
@@ -41,15 +41,21 @@ static void __init clear_bss(void)
 	       (unsigned long) __bss_stop - (unsigned long) __bss_start);
 }
 
+static int __initdata bootdata_copied;
 static void __init copy_bootdata(char *real_mode_data)
 {
 	char * command_line;
 
+	if (bootdata_copied)
+		return;
+
 	memcpy(&boot_params, real_mode_data, sizeof boot_params);
 	if (boot_params.hdr.cmd_line_ptr) {
 		command_line = __va(boot_params.hdr.cmd_line_ptr);
 		memcpy(boot_command_line, command_line, COMMAND_LINE_SIZE);
 	}
+
+	bootdata_copied = 1;
 }
 
 void __init x86_64_start_kernel(char * real_mode_data)
@@ -73,6 +79,10 @@ void __init x86_64_start_kernel(char * r
 	/* clear bss before set_intr_gate with early_idt_handler */
 	clear_bss();
 
+	/* boot_params is in bss */
+	copy_bootdata(__va(real_mode_data));
+	setup_early_console();
+
 	/* Make NULL pointers segfault */
 	zap_identity_mappings();
 
@@ -97,6 +107,7 @@ void __init x86_64_start_kernel(char * r
 void __init x86_64_start_reservations(char *real_mode_data)
 {
 	copy_bootdata(__va(real_mode_data));
+	setup_early_console();
 
 	reserve_early(__pa_symbol(&_text), __pa_symbol(&__bss_stop), "TEXT DATA BSS");
 
Index: linux-2.6/kernel/printk.c
===================================================================
--- linux-2.6.orig/kernel/printk.c
+++ linux-2.6/kernel/printk.c
@@ -1203,6 +1203,10 @@ void register_console(struct console *ne
 	if (console_drivers && newcon->flags & CON_BOOT) {
 		/* find the last or real console */
 		for_each_console(bcon) {
+			/* not again */
+			if (bcon == newcon)
+				return;
+
 			if (!(bcon->flags & CON_BOOT)) {
 				printk(KERN_INFO "Too late to register bootconsole %s%d\n",
 					newcon->name, newcon->index);

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-11 21:44 [PATCH] x86: Setup early console as early as possible Yinghai Lu
@ 2010-07-12  8:58 ` Pekka Enberg
  2010-07-12 15:47   ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Pekka Enberg @ 2010-07-12  8:58 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

Hi Yinghai,

Yinghai Lu wrote:
> Analyze "console=uart8250,io,0x3f8,115200n8" in i386_start_kernel/x86_64_start_kernel,
> and call setup_early_serial8250_console() to init early serial console.
> 
> only can handle io port kind of 8250. because mmio need ioremap.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

What's the purpose of this patch? Does it make my early boot I/O patch 
obsolete?

			Pekka

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12  8:58 ` Pekka Enberg
@ 2010-07-12 15:47   ` H. Peter Anvin
  2010-07-12 16:21     ` Yinghai Lu
  2010-07-12 17:44     ` [PATCH] x86: Setup early console as early as possible Cyrill Gorcunov
  0 siblings, 2 replies; 20+ messages in thread
From: H. Peter Anvin @ 2010-07-12 15:47 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Yinghai Lu, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On 07/12/2010 01:58 AM, Pekka Enberg wrote:
> Hi Yinghai,
> 
> Yinghai Lu wrote:
>> Analyze "console=uart8250,io,0x3f8,115200n8" in
>> i386_start_kernel/x86_64_start_kernel,
>> and call setup_early_serial8250_console() to init early serial console.
>>
>> only can handle io port kind of 8250. because mmio need ioremap.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> What's the purpose of this patch? Does it make my early boot I/O patch
> obsolete?
> 
>             Pekka

No, they're complementary.  Your patch serial-port enables the RM
kernel, whereas Yinghai pushes the initialization earlier in the PM kernel.

Incidentally, Yinghai: it would be possible to push even an MMIO
reference earlier by reserving a fixmap slot for the early console.  I'm
not sure if it's worth it, though.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 15:47   ` H. Peter Anvin
@ 2010-07-12 16:21     ` Yinghai Lu
  2010-07-12 17:30       ` H. Peter Anvin
  2010-07-13 17:43       ` Pekka Enberg
  2010-07-12 17:44     ` [PATCH] x86: Setup early console as early as possible Cyrill Gorcunov
  1 sibling, 2 replies; 20+ messages in thread
From: Yinghai Lu @ 2010-07-12 16:21 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Pekka Enberg, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On 07/12/2010 08:47 AM, H. Peter Anvin wrote:
> On 07/12/2010 01:58 AM, Pekka Enberg wrote:
>> Hi Yinghai,
>>
>> Yinghai Lu wrote:
>>> Analyze "console=uart8250,io,0x3f8,115200n8" in
>>> i386_start_kernel/x86_64_start_kernel,
>>> and call setup_early_serial8250_console() to init early serial console.
>>>
>>> only can handle io port kind of 8250. because mmio need ioremap.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> What's the purpose of this patch? Does it make my early boot I/O patch
>> obsolete?
>>
>>             Pekka
> 
> No, they're complementary.  Your patch serial-port enables the RM
> kernel, whereas Yinghai pushes the initialization earlier in the PM kernel.

yes. cover more range.

Can you consider to ask Pekka to anaylze "console=uart8250,io, 0x3f8,115200n8" instead?

it looks like we can remove "earlyprintk=ttyS0,115200", or "earlyprintk=serial" etc.

earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
because it is shared between different archs already.

> 
> Incidentally, Yinghai: it would be possible to push even an MMIO
> reference earlier by reserving a fixmap slot for the early console.  I'm
> not sure if it's worth it, though.

in setup_arch() for x86, now we have

        /* VMI may relocate the fixmap; do this before touching ioremap area */
        vmi_init();

        /* OFW also may relocate the fixmap */
        olpc_ofw_detect();

        early_trap_init();
        early_cpu_init();
        early_ioremap_init()

so may need to move these 
	vmi_init()
	olpc_ofw_detect()
	early_ioremap_init()

to i386_start_kernel(), x86_start_kernel()

may be not worth it at this time, could do that later if needed.

Thanks

Yinghai

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 16:21     ` Yinghai Lu
@ 2010-07-12 17:30       ` H. Peter Anvin
  2010-07-13 17:43       ` Pekka Enberg
  1 sibling, 0 replies; 20+ messages in thread
From: H. Peter Anvin @ 2010-07-12 17:30 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Pekka Enberg, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On 07/12/2010 09:21 AM, Yinghai Lu wrote:
> 
> Can you consider to ask Pekka to anaylze "console=uart8250,io, 0x3f8,115200n8" instead?
> 
> it looks like we can remove "earlyprintk=ttyS0,115200", or "earlyprintk=serial" etc.
> 
> earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
> because it is shared between different archs already.
> 

Did you manage to miss my comment about that?

console= is crap, because it makes the user keep track of items they
should not have to deal with directly (unless they want to override the
defaults), i.e. the specific I/O ports used by the serial ports.

It's insanely user-unfriendly, and as far as I can see the movement has
been *toward* earlyprintk= rather than away from it.  It makes sense:
it's a higher-order interface that is closer to the user.

So, no, I will not ask Pekka to change from a good user interface to a
bad one.

>>
>> Incidentally, Yinghai: it would be possible to push even an MMIO
>> reference earlier by reserving a fixmap slot for the early console.  I'm
>> not sure if it's worth it, though.
> 
> in setup_arch() for x86, now we have
> 
>         /* VMI may relocate the fixmap; do this before touching ioremap area */
>         vmi_init();
> 
>         /* OFW also may relocate the fixmap */
>         olpc_ofw_detect();
> 
>         early_trap_init();
>         early_cpu_init();
>         early_ioremap_init()
> 
> so may need to move these 
> 	vmi_init()
> 	olpc_ofw_detect()
> 	early_ioremap_init()
> 
> to i386_start_kernel(), x86_start_kernel()
> 
> may be not worth it at this time, could do that later if needed.

Don't think so at this time, since mmio 8250 serial ports are basically
never used in the x86 world, but it could be interesting for non-8250
consoles, e.g. USB, in the future.

	-hpa

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 15:47   ` H. Peter Anvin
  2010-07-12 16:21     ` Yinghai Lu
@ 2010-07-12 17:44     ` Cyrill Gorcunov
  2010-07-12 18:09       ` H. Peter Anvin
  1 sibling, 1 reply; 20+ messages in thread
From: Cyrill Gorcunov @ 2010-07-12 17:44 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Pekka Enberg, Yinghai Lu, Ingo Molnar, Thomas Gleixner,
	Andrew Morton, linux-kernel@vger.kernel.org

On Mon, Jul 12, 2010 at 08:47:03AM -0700, H. Peter Anvin wrote:
> On 07/12/2010 01:58 AM, Pekka Enberg wrote:
> > Hi Yinghai,
> > 
> > Yinghai Lu wrote:
> >> Analyze "console=uart8250,io,0x3f8,115200n8" in
> >> i386_start_kernel/x86_64_start_kernel,
> >> and call setup_early_serial8250_console() to init early serial console.
> >>
> >> only can handle io port kind of 8250. because mmio need ioremap.
> >>
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> > 
> > What's the purpose of this patch? Does it make my early boot I/O patch
> > obsolete?
> > 
> >             Pekka
> 
> No, they're complementary.  Your patch serial-port enables the RM
> kernel, whereas Yinghai pushes the initialization earlier in the PM kernel.
> 
> Incidentally, Yinghai: it would be possible to push even an MMIO
> reference earlier by reserving a fixmap slot for the early console.  I'm
> not sure if it's worth it, though.
> 
> 	-hpa
>

Peter, while reviewing this patch I found another nit in
context of early_param usage, so the patch is below. It's
completely trivial. Actually I thought I've already fixed
all early_param cases long ago but this one somehow sneaked ;)

Anyway, Yinghai, Peter,

I'm not sure but can't we use some boot_param "pad" field for
"being copied" flag instead of new variable? There is a case
when boot_param is used as __initdata and I'm not sure we clear
this section explicitly.

	-- Cyrill
---
From: Cyrill Gorcunov <gorcunov@openvz.org>
Subject: [PATCH] early console: Prevent early_param null dereference

In case if user passes "earlycon" without args there will
be null dereference.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 drivers/serial/8250_early.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6.git/drivers/serial/8250_early.c
=====================================================================
--- linux-2.6.git.orig/drivers/serial/8250_early.c
+++ linux-2.6.git/drivers/serial/8250_early.c
@@ -217,6 +217,9 @@ int __init setup_early_serial8250_consol
 	char *options;
 	int err;
 
+	if (!cmdline)
+		return 0;
+
 	options = strstr(cmdline, "uart8250,");
 	if (!options) {
 		options = strstr(cmdline, "uart,");

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 17:44     ` [PATCH] x86: Setup early console as early as possible Cyrill Gorcunov
@ 2010-07-12 18:09       ` H. Peter Anvin
  2010-07-12 18:11         ` Yinghai Lu
  0 siblings, 1 reply; 20+ messages in thread
From: H. Peter Anvin @ 2010-07-12 18:09 UTC (permalink / raw)
  To: Cyrill Gorcunov
  Cc: Pekka Enberg, Yinghai Lu, Ingo Molnar, Thomas Gleixner,
	Andrew Morton, linux-kernel@vger.kernel.org

On 07/12/2010 10:44 AM, Cyrill Gorcunov wrote:
> 
> Peter, while reviewing this patch I found another nit in
> context of early_param usage, so the patch is below. It's
> completely trivial. Actually I thought I've already fixed
> all early_param cases long ago but this one somehow sneaked ;)
> 
> Anyway, Yinghai, Peter,
> 
> I'm not sure but can't we use some boot_param "pad" field for
> "being copied" flag instead of new variable? There is a case
> when boot_param is used as __initdata and I'm not sure we clear
> this section explicitly.
> 

Actually, even better would be to simply use boot_params.hdr.version,
which will never be zero.

	-hpa

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 18:09       ` H. Peter Anvin
@ 2010-07-12 18:11         ` Yinghai Lu
  2010-07-12 22:57           ` Jeremy Fitzhardinge
  0 siblings, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2010-07-12 18:11 UTC (permalink / raw)
  To: H. Peter Anvin, Jeremy Fitzhardinge
  Cc: Cyrill Gorcunov, Pekka Enberg, Ingo Molnar, Thomas Gleixner,
	Andrew Morton, linux-kernel@vger.kernel.org

On 07/12/2010 11:09 AM, H. Peter Anvin wrote:
> On 07/12/2010 10:44 AM, Cyrill Gorcunov wrote:
>>
>> Peter, while reviewing this patch I found another nit in
>> context of early_param usage, so the patch is below. It's
>> completely trivial. Actually I thought I've already fixed
>> all early_param cases long ago but this one somehow sneaked ;)
>>
>> Anyway, Yinghai, Peter,
>>
>> I'm not sure but can't we use some boot_param "pad" field for
>> "being copied" flag instead of new variable? There is a case
>> when boot_param is used as __initdata and I'm not sure we clear
>> this section explicitly.
>>
> 
> Actually, even better would be to simply use boot_params.hdr.version,
> which will never be zero.

Jeremy,

any reason that xen cat not use x86_64_start_kernel directly?

Thanks

Yinghai

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

* Re: [PATCH] x86: Setup early console as early as possible
       [not found]       ` <f5v90-5Wo-7@gated-at.bofh.it>
@ 2010-07-12 20:57         ` Bodo Eggert
  2010-07-12 21:52           ` H. Peter Anvin
  0 siblings, 1 reply; 20+ messages in thread
From: Bodo Eggert @ 2010-07-12 20:57 UTC (permalink / raw)
  To: H.  Peter Anvin, Yinghai Lu, Pekka Enberg, Ingo Molnar,
	Thomas Gleixner, Andrew Morton, linux-kernel@vger.kernel.org

H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/12/2010 09:21 AM, Yinghai Lu wrote:

>> earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
>> because it is shared between different archs already.

> console= is crap, because it makes the user keep track of items they
> should not have to deal with directly (unless they want to override the
> defaults), i.e. the specific I/O ports used by the serial ports.

Why does console= require these parameters, if earlyprintk does not?

IMO you should fix console= to not need these parameters unless they cannot
be guessed (in which case earlyprintk cannot guess them, either).

¢¢

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 20:57         ` Bodo Eggert
@ 2010-07-12 21:52           ` H. Peter Anvin
  0 siblings, 0 replies; 20+ messages in thread
From: H. Peter Anvin @ 2010-07-12 21:52 UTC (permalink / raw)
  To: 7eggert
  Cc: Bodo Eggert, Yinghai Lu, Pekka Enberg, Ingo Molnar,
	Thomas Gleixner, Andrew Morton, linux-kernel@vger.kernel.org

On 07/12/2010 01:57 PM, Bodo Eggert wrote:
> H. Peter Anvin <hpa@zytor.com> wrote:
>> On 07/12/2010 09:21 AM, Yinghai Lu wrote:
> 
>>> earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
>>> because it is shared between different archs already.
> 
>> console= is crap, because it makes the user keep track of items they
>> should not have to deal with directly (unless they want to override the
>> defaults), i.e. the specific I/O ports used by the serial ports.
> 
> Why does console= require these parameters, if earlyprintk does not?
> 
> IMO you should fix console= to not need these parameters unless they cannot
> be guessed (in which case earlyprintk cannot guess them, either).
> 

earlycon is basically a low-level interface where you can steer
everything.  This is what you have to do on most embedded architectures,
where the firmware doesn't tell you where the console is located.

On x86, either the firmware can tell you or you can rely on 20 years of
legacy, with very few divergences.

	-hpa


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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 18:11         ` Yinghai Lu
@ 2010-07-12 22:57           ` Jeremy Fitzhardinge
  2010-07-12 23:37             ` Yinghai Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-12 22:57 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: H. Peter Anvin, Jeremy Fitzhardinge, Cyrill Gorcunov,
	Pekka Enberg, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On 07/12/2010 11:11 AM, Yinghai Lu wrote:
> On 07/12/2010 11:09 AM, H. Peter Anvin wrote:
>   
>> On 07/12/2010 10:44 AM, Cyrill Gorcunov wrote:
>>     
>>> Peter, while reviewing this patch I found another nit in
>>> context of early_param usage, so the patch is below. It's
>>> completely trivial. Actually I thought I've already fixed
>>> all early_param cases long ago but this one somehow sneaked ;)
>>>
>>> Anyway, Yinghai, Peter,
>>>
>>> I'm not sure but can't we use some boot_param "pad" field for
>>> "being copied" flag instead of new variable? There is a case
>>> when boot_param is used as __initdata and I'm not sure we clear
>>> this section explicitly.
>>>
>>>       
>> Actually, even better would be to simply use boot_params.hdr.version,
>> which will never be zero.
>>     
> Jeremy,
>
> any reason that xen cat not use x86_64_start_kernel directly?
>   

As I remember it, I split x86_64_start_kernel into two pieces, one
containing the bits that were awkward with Xen.  I don't remember which
were the problematic parts, but it all looks pretty tricky.  Specifically:

    * Xen will pre-clear the bss, so that's not necessary
    * we don't go via head, so cleanup_highmap isn't either
    * PV domains don't have an IDT available to them, or any of their
      associated structures

So the whole thing looks at best reundant, and at worst has the
potential for causing subtle damage.

Why do you ask?  Does it relate to the early console stuff, or are you
just asking because you're looking at it?

    J

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 22:57           ` Jeremy Fitzhardinge
@ 2010-07-12 23:37             ` Yinghai Lu
  0 siblings, 0 replies; 20+ messages in thread
From: Yinghai Lu @ 2010-07-12 23:37 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: H. Peter Anvin, Jeremy Fitzhardinge, Cyrill Gorcunov,
	Pekka Enberg, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On 07/12/2010 03:57 PM, Jeremy Fitzhardinge wrote:
> On 07/12/2010 11:11 AM, Yinghai Lu wrote:
>> On 07/12/2010 11:09 AM, H. Peter Anvin wrote:
>>   
>>> On 07/12/2010 10:44 AM, Cyrill Gorcunov wrote:
>>>     
>>>> Peter, while reviewing this patch I found another nit in
>>>> context of early_param usage, so the patch is below. It's
>>>> completely trivial. Actually I thought I've already fixed
>>>> all early_param cases long ago but this one somehow sneaked ;)
>>>>
>>>> Anyway, Yinghai, Peter,
>>>>
>>>> I'm not sure but can't we use some boot_param "pad" field for
>>>> "being copied" flag instead of new variable? There is a case
>>>> when boot_param is used as __initdata and I'm not sure we clear
>>>> this section explicitly.
>>>>
>>>>       
>>> Actually, even better would be to simply use boot_params.hdr.version,
>>> which will never be zero.
>>>     
>> Jeremy,
>>
>> any reason that xen cat not use x86_64_start_kernel directly?
>>   
> 
> As I remember it, I split x86_64_start_kernel into two pieces, one
> containing the bits that were awkward with Xen.  I don't remember which
> were the problematic parts, but it all looks pretty tricky.  Specifically:
> 
>     * Xen will pre-clear the bss, so that's not necessary
>     * we don't go via head, so cleanup_highmap isn't either
>     * PV domains don't have an IDT available to them, or any of their
>       associated structures
> 
> So the whole thing looks at best reundant, and at worst has the
> potential for causing subtle damage.
> 
> Why do you ask?  Does it relate to the early console stuff, or are you
> just asking because you're looking at it?

yes, make setup_early_console()/copy_bootdata two times....

and that looks strange.

 
+static int __initdata bootdata_copied;
 static void __init copy_bootdata(char *real_mode_data)
 {
        char * command_line;
 
+       if (bootdata_copied)
+               return;
+
        memcpy(&boot_params, real_mode_data, sizeof boot_params);
        if (boot_params.hdr.cmd_line_ptr) {
                command_line = __va(boot_params.hdr.cmd_line_ptr);
                memcpy(boot_command_line, command_line, COMMAND_LINE_SIZE);
        }
+
+       bootdata_copied = 1;
 }
 
 void __init x86_64_start_kernel(char * real_mode_data)
@@ -73,6 +80,10 @@ void __init x86_64_start_kernel(char * real_mode_data)
        /* clear bss before set_intr_gate with early_idt_handler */
        clear_bss();
 
+       /* boot_params is in bss */
+       copy_bootdata(__va(real_mode_data));
+       setup_early_console();
+
        /* Make NULL pointers segfault */
        zap_identity_mappings();
 
@@ -97,8 +108,11 @@ void __init x86_64_start_kernel(char * real_mode_data)
 void __init x86_64_start_reservations(char *real_mode_data)
 {
        copy_bootdata(__va(real_mode_data));
+       setup_early_console();
+

Thanks

Yinghai Lu

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-12 16:21     ` Yinghai Lu
  2010-07-12 17:30       ` H. Peter Anvin
@ 2010-07-13 17:43       ` Pekka Enberg
  2010-07-13 20:35         ` Yinghai Lu
  1 sibling, 1 reply; 20+ messages in thread
From: Pekka Enberg @ 2010-07-13 17:43 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

Hi Yingahai,

Yinghai Lu wrote:
> On 07/12/2010 08:47 AM, H. Peter Anvin wrote:
>> On 07/12/2010 01:58 AM, Pekka Enberg wrote:
>>> Hi Yinghai,
>>>
>>> Yinghai Lu wrote:
>>>> Analyze "console=uart8250,io,0x3f8,115200n8" in
>>>> i386_start_kernel/x86_64_start_kernel,
>>>> and call setup_early_serial8250_console() to init early serial console.
>>>>
>>>> only can handle io port kind of 8250. because mmio need ioremap.
>>>>
>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> What's the purpose of this patch? Does it make my early boot I/O patch
>>> obsolete?
>>>
>>>             Pekka
>> No, they're complementary.  Your patch serial-port enables the RM
>> kernel, whereas Yinghai pushes the initialization earlier in the PM kernel.
> 
> yes. cover more range.
> 
> Can you consider to ask Pekka to anaylze "console=uart8250,io, 0x3f8,115200n8" instead?
> 
> it looks like we can remove "earlyprintk=ttyS0,115200", or "earlyprintk=serial" etc.
> 
> earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
> because it is shared between different archs already.

So just to clarify: I wasn't ignoring your comment here. I simply 
followed hpa's recommendation on which I also happen to agree with 
completely. ;-)

			Pekka

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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-13 17:43       ` Pekka Enberg
@ 2010-07-13 20:35         ` Yinghai Lu
  2010-07-13 20:46           ` Pekka Enberg
  2010-07-13 21:12           ` [tip:x86/setup] x86, setup: Make the setup code also accept console=uart8250 tip-bot for Yinghai Lu
  0 siblings, 2 replies; 20+ messages in thread
From: Yinghai Lu @ 2010-07-13 20:35 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On 07/13/2010 10:43 AM, Pekka Enberg wrote:
> Hi Yingahai,
> 
> Yinghai Lu wrote:
>> On 07/12/2010 08:47 AM, H. Peter Anvin wrote:
>>> On 07/12/2010 01:58 AM, Pekka Enberg wrote:
>>>> Hi Yinghai,
>>>>
>>>> Yinghai Lu wrote:
>>>>> Analyze "console=uart8250,io,0x3f8,115200n8" in
>>>>> i386_start_kernel/x86_64_start_kernel,
>>>>> and call setup_early_serial8250_console() to init early serial
>>>>> console.
>>>>>
>>>>> only can handle io port kind of 8250. because mmio need ioremap.
>>>>>
>>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>> What's the purpose of this patch? Does it make my early boot I/O patch
>>>> obsolete?
>>>>
>>>>             Pekka
>>> No, they're complementary.  Your patch serial-port enables the RM
>>> kernel, whereas Yinghai pushes the initialization earlier in the PM
>>> kernel.
>>
>> yes. cover more range.
>>
>> Can you consider to ask Pekka to anaylze "console=uart8250,io,
>> 0x3f8,115200n8" instead?
>>
>> it looks like we can remove "earlyprintk=ttyS0,115200", or
>> "earlyprintk=serial" etc.
>>
>> earlycon=uart8250 or console=uart8250 should be better than earlyprintk.
>> because it is shared between different archs already.
> 
> So just to clarify: I wasn't ignoring your comment here. I simply
> followed hpa's recommendation on which I also happen to agree with
> completely. ;-)

never mind.

following patch add that checking.

also you missed simple_guess_base(), your patch may have problem with baud rate reading.

                baud = simple_strtoull(arg + pos, &e, 0);
                if (baud == 0 || arg + pos == e)
                        baud = DEFAULT_BAUD;

	and your copied simple_strtoull does not calling simple_guess_base(), so base will 0.
	so you are always using DEFAULT_BAUD.

Thanks

Yinghai Lu


[PATCH] x86: make boot code to analyze console=uart8250 too

So we use console=uart8250,io,0x2f8,115200n all the way

Also add back simple_guess_base(), otherwise those simple_strtoull(,,0) are not
going to work.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/boot/string.c |   22 ++++++++++++++++++
 arch/x86/boot/tty.c    |   59 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/boot/tty.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/tty.c
+++ linux-2.6/arch/x86/boot/tty.c
@@ -170,7 +170,7 @@ static void early_serial_init(int baud)
 	outb(c & ~DLAB, early_serial_base + LCR);
 }
 
-void console_init(void)
+static int parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
 	char arg[32];
@@ -208,6 +208,63 @@ void console_init(void)
 			baud = DEFAULT_BAUD;
 	}
 
+	return baud;
+}
+
+#define BASE_BAUD (1843200/16)
+static unsigned int probe_baud(int port)
+{
+	unsigned char lcr, dll, dlh;
+	unsigned int quot;
+
+	lcr = inb(port + LCR);
+	outb(lcr | DLAB, port + LCR);
+	dll = inb(port + DLL);
+	dlh = inb(port + DLH);
+	outb(lcr, port + LCR);
+	quot = (dlh << 8) | dll;
+
+	return BASE_BAUD / quot;
+}
+
+static int parse_console_uart8250(void)
+{
+	char optstr[64], *options;
+	int baud = DEFAULT_BAUD;
+
+	/*
+	 * console=uart8250,io,0x3f8,115200n8
+	 * need to make sure it is last one console !
+	 */
+	if (cmdline_find_option("console", optstr, sizeof optstr) <= 0)
+		return baud;
+
+	options = optstr;
+
+	if (!strncmp(options, "uart8250,io,", 12))
+		early_serial_base = simple_strtoull(options + 12, &options, 0);
+	else if (!strncmp(options, "uart,io,", 8))
+		early_serial_base = simple_strtoull(options + 8, &options, 0);
+	else
+		return baud;
+
+	if (options && (options[0] == ','))
+		baud = simple_strtoull(options + 1, &options, 0);
+	else
+		baud = probe_baud(early_serial_base);
+
+	return baud;
+}
+
+void console_init(void)
+{
+	int baud;
+
+	baud = parse_earlyprintk();
+
+	if (!early_serial_base)
+		baud = parse_console_uart8250();
+
 	if (early_serial_base != 0)
 		early_serial_init(baud);
 }
Index: linux-2.6/arch/x86/boot/string.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/string.c
+++ linux-2.6/arch/x86/boot/string.c
@@ -68,10 +68,32 @@ unsigned int atou(const char *s)
 /* Works only for digits and letters, but small and fast */
 #define TOLOWER(x) ((x) | 0x20)
 
+static unsigned int simple_guess_base(const char *cp)
+{
+	if (cp[0] == '0') {
+		if (TOLOWER(cp[1]) == 'x' && isxdigit(cp[2]))
+			return 16;
+		else
+			return 8;
+	} else {
+		return 10;
+	}
+}
+
+/**
+ * simple_strtoull - convert a string to an unsigned long long
+ * @cp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ */
+
 unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
 {
 	unsigned long long result = 0;
 
+	if (!base)
+		base = simple_guess_base(cp);
+
 	if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
 		cp += 2;
 




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

* Re: [PATCH] x86: Setup early console as early as possible
  2010-07-13 20:35         ` Yinghai Lu
@ 2010-07-13 20:46           ` Pekka Enberg
  2010-07-14  2:07             ` [PATCH] x86: only set early_serial_base after port is initialized Yinghai Lu
  2010-07-13 21:12           ` [tip:x86/setup] x86, setup: Make the setup code also accept console=uart8250 tip-bot for Yinghai Lu
  1 sibling, 1 reply; 20+ messages in thread
From: Pekka Enberg @ 2010-07-13 20:46 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On Tue, Jul 13, 2010 at 11:35 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> [PATCH] x86: make boot code to analyze console=uart8250 too
>
> So we use console=uart8250,io,0x2f8,115200n all the way
>
> Also add back simple_guess_base(), otherwise those simple_strtoull(,,0) are not
> going to work.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Cool! Thanks Yinghai!

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

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

* [tip:x86/setup] x86, setup: Make the setup code also accept console=uart8250
  2010-07-13 20:35         ` Yinghai Lu
  2010-07-13 20:46           ` Pekka Enberg
@ 2010-07-13 21:12           ` tip-bot for Yinghai Lu
  1 sibling, 0 replies; 20+ messages in thread
From: tip-bot for Yinghai Lu @ 2010-07-13 21:12 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, yinghai, penberg, tglx, hpa

Commit-ID:  ce0aa5dd20e44372f9617dd67c984f41fcdbed88
Gitweb:     http://git.kernel.org/tip/ce0aa5dd20e44372f9617dd67c984f41fcdbed88
Author:     Yinghai Lu <yinghai@kernel.org>
AuthorDate: Tue, 13 Jul 2010 13:35:17 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Tue, 13 Jul 2010 14:08:12 -0700

x86, setup: Make the setup code also accept console=uart8250

Make the boot code also accept the console=uart8250,io,0x2f8,115200n
form of early console.

Also add back simple_guess_base(), otherwise those simple_strtoull(,,0)
are not going to work.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
LKML-Reference: <4C3CCE05.4090505@kernel.org>
Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/boot/string.c |   22 ++++++++++++++++++
 arch/x86/boot/tty.c    |   59 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 1 deletions(-)

diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index aba29df..3cbc405 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -68,10 +68,32 @@ unsigned int atou(const char *s)
 /* Works only for digits and letters, but small and fast */
 #define TOLOWER(x) ((x) | 0x20)
 
+static unsigned int simple_guess_base(const char *cp)
+{
+	if (cp[0] == '0') {
+		if (TOLOWER(cp[1]) == 'x' && isxdigit(cp[2]))
+			return 16;
+		else
+			return 8;
+	} else {
+		return 10;
+	}
+}
+
+/**
+ * simple_strtoull - convert a string to an unsigned long long
+ * @cp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ */
+
 unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
 {
 	unsigned long long result = 0;
 
+	if (!base)
+		base = simple_guess_base(cp);
+
 	if (base == 16 && cp[0] == '0' && TOLOWER(cp[1]) == 'x')
 		cp += 2;
 
diff --git a/arch/x86/boot/tty.c b/arch/x86/boot/tty.c
index f3ceee2..f6d52e6 100644
--- a/arch/x86/boot/tty.c
+++ b/arch/x86/boot/tty.c
@@ -170,7 +170,7 @@ static void early_serial_init(int baud)
 	outb(c & ~DLAB, early_serial_base + LCR);
 }
 
-void console_init(void)
+static int parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
 	char arg[32];
@@ -208,6 +208,63 @@ void console_init(void)
 			baud = DEFAULT_BAUD;
 	}
 
+	return baud;
+}
+
+#define BASE_BAUD (1843200/16)
+static unsigned int probe_baud(int port)
+{
+	unsigned char lcr, dll, dlh;
+	unsigned int quot;
+
+	lcr = inb(port + LCR);
+	outb(lcr | DLAB, port + LCR);
+	dll = inb(port + DLL);
+	dlh = inb(port + DLH);
+	outb(lcr, port + LCR);
+	quot = (dlh << 8) | dll;
+
+	return BASE_BAUD / quot;
+}
+
+static int parse_console_uart8250(void)
+{
+	char optstr[64], *options;
+	int baud = DEFAULT_BAUD;
+
+	/*
+	 * console=uart8250,io,0x3f8,115200n8
+	 * need to make sure it is last one console !
+	 */
+	if (cmdline_find_option("console", optstr, sizeof optstr) <= 0)
+		return baud;
+
+	options = optstr;
+
+	if (!strncmp(options, "uart8250,io,", 12))
+		early_serial_base = simple_strtoull(options + 12, &options, 0);
+	else if (!strncmp(options, "uart,io,", 8))
+		early_serial_base = simple_strtoull(options + 8, &options, 0);
+	else
+		return baud;
+
+	if (options && (options[0] == ','))
+		baud = simple_strtoull(options + 1, &options, 0);
+	else
+		baud = probe_baud(early_serial_base);
+
+	return baud;
+}
+
+void console_init(void)
+{
+	int baud;
+
+	baud = parse_earlyprintk();
+
+	if (!early_serial_base)
+		baud = parse_console_uart8250();
+
 	if (early_serial_base != 0)
 		early_serial_init(baud);
 }

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

* [PATCH] x86: only set early_serial_base after port is initialized
  2010-07-13 20:46           ` Pekka Enberg
@ 2010-07-14  2:07             ` Yinghai Lu
  2010-07-14  8:41               ` Pekka Enberg
  0 siblings, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2010-07-14  2:07 UTC (permalink / raw)
  To: Pekka Enberg, H. Peter Anvin
  Cc: Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org



putchar is using early_serial_base to check if port is initialized.

So we only assign it after early_serial_init() is called.

in case we need use VGA to debug early serial console.

Also add display for port addr and baud.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/boot/tty.c |   64 ++++++++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

Index: linux-2.6/arch/x86/boot/tty.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/tty.c
+++ linux-2.6/arch/x86/boot/tty.c
@@ -152,35 +152,40 @@ int getchar_timeout(void)
 	return 0;		/* Timeout! */
 }
 
-static void early_serial_init(int baud)
+static void early_serial_init(int port, int baud)
 {
 	unsigned char c;
 	unsigned divisor;
 
-	outb(0x3, early_serial_base + LCR);	/* 8n1 */
-	outb(0, early_serial_base + IER);	/* no interrupt */
-	outb(0, early_serial_base + FCR);	/* no fifo */
-	outb(0x3, early_serial_base + MCR);	/* DTR + RTS */
+	outb(0x3, port + LCR);	/* 8n1 */
+	outb(0, port + IER);	/* no interrupt */
+	outb(0, port + FCR);	/* no fifo */
+	outb(0x3, port + MCR);	/* DTR + RTS */
 
 	divisor	= 115200 / baud;
-	c = inb(early_serial_base + LCR);
-	outb(c | DLAB, early_serial_base + LCR);
-	outb(divisor & 0xff, early_serial_base + DLL);
-	outb((divisor >> 8) & 0xff, early_serial_base + DLH);
-	outb(c & ~DLAB, early_serial_base + LCR);
+	c = inb(port + LCR);
+	outb(c | DLAB, port + LCR);
+	outb(divisor & 0xff, port + DLL);
+	outb((divisor >> 8) & 0xff, port + DLH);
+	outb(c & ~DLAB, port + LCR);
+
+	early_serial_base = port;
+
+	printf("Early serial console at I/O port 0x%x baud: %d\n", port, baud);
 }
 
-static int parse_earlyprintk(void)
+static void parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
 	char arg[32];
 	int pos = 0;
+	int port = 0;
 
 	if (cmdline_find_option("earlyprintk", arg, sizeof arg) > 0) {
 		char *e;
 
 		if (!strncmp(arg, "serial", 6)) {
-			early_serial_base = DEFAULT_SERIAL_PORT;
+			port = DEFAULT_SERIAL_PORT;
 			pos += 6;
 		}
 
@@ -189,15 +194,15 @@ static int parse_earlyprintk(void)
 
 		if (!strncmp(arg, "ttyS", 4)) {
 			static const int bases[] = { 0x3f8, 0x2f8 };
-			int port = 0;
+			int idx = 0;
 
 			if (!strncmp(arg + pos, "ttyS", 4))
 				pos += 4;
 
 			if (arg[pos++] == '1')
-				port = 1;
+				idx = 1;
 
-			early_serial_base = bases[port];
+			port = bases[idx];
 		}
 
 		if (arg[pos] == ',')
@@ -208,7 +213,8 @@ static int parse_earlyprintk(void)
 			baud = DEFAULT_BAUD;
 	}
 
-	return baud;
+	if (port)
+		early_serial_init(port, baud);
 }
 
 #define BASE_BAUD (1843200/16)
@@ -227,45 +233,41 @@ static unsigned int probe_baud(int port)
 	return BASE_BAUD / quot;
 }
 
-static int parse_console_uart8250(void)
+static void parse_console_uart8250(void)
 {
 	char optstr[64], *options;
 	int baud = DEFAULT_BAUD;
+	int port = 0;
 
 	/*
 	 * console=uart8250,io,0x3f8,115200n8
 	 * need to make sure it is last one console !
 	 */
 	if (cmdline_find_option("console", optstr, sizeof optstr) <= 0)
-		return baud;
+		return;
 
 	options = optstr;
 
 	if (!strncmp(options, "uart8250,io,", 12))
-		early_serial_base = simple_strtoull(options + 12, &options, 0);
+		port = simple_strtoull(options + 12, &options, 0);
 	else if (!strncmp(options, "uart,io,", 8))
-		early_serial_base = simple_strtoull(options + 8, &options, 0);
+		port = simple_strtoull(options + 8, &options, 0);
 	else
-		return baud;
+		return;
 
 	if (options && (options[0] == ','))
 		baud = simple_strtoull(options + 1, &options, 0);
 	else
-		baud = probe_baud(early_serial_base);
+		baud = probe_baud(port);
 
-	return baud;
+	if (port)
+		early_serial_init(port, baud);
 }
 
 void console_init(void)
 {
-	int baud;
-
-	baud = parse_earlyprintk();
+	parse_earlyprintk();
 
 	if (!early_serial_base)
-		baud = parse_console_uart8250();
-
-	if (early_serial_base != 0) {
-		early_serial_init(baud);
-	}
+		parse_console_uart8250();
 }

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

* Re: [PATCH] x86: only set early_serial_base after port is initialized
  2010-07-14  2:07             ` [PATCH] x86: only set early_serial_base after port is initialized Yinghai Lu
@ 2010-07-14  8:41               ` Pekka Enberg
  2010-07-14 18:26                 ` [PATCH -v2] x86: only set early_serial_base after port is initialized in setup code Yinghai Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Pekka Enberg @ 2010-07-14  8:41 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org

On Wed, Jul 14, 2010 at 5:07 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> putchar is using early_serial_base to check if port is initialized.
>
> So we only assign it after early_serial_init() is called.
>
> in case we need use VGA to debug early serial console.
>
> Also add display for port addr and baud.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

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

* [PATCH -v2] x86: only set early_serial_base after port is initialized in setup code
  2010-07-14  8:41               ` Pekka Enberg
@ 2010-07-14 18:26                 ` Yinghai Lu
  2010-07-14 19:12                   ` [tip:x86/setup] x86, setup: Only set early_serial_base after port is initialized tip-bot for Yinghai Lu
  0 siblings, 1 reply; 20+ messages in thread
From: Yinghai Lu @ 2010-07-14 18:26 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Pekka Enberg, Ingo Molnar, Thomas Gleixner, Andrew Morton,
	linux-kernel@vger.kernel.org


putchar is using early_serial_base to check if port is initialized.

So we only assign it after early_serial_init() is called.

in case we need use VGA to debug early serial console.

Also add display for port addr and baud.

-v2: update to current tip

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/boot/tty.c |   63 +++++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

Index: linux-2.6/arch/x86/boot/tty.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/tty.c
+++ linux-2.6/arch/x86/boot/tty.c
@@ -152,35 +152,40 @@ int getchar_timeout(void)
 	return 0;		/* Timeout! */
 }
 
-static void early_serial_init(int baud)
+static void early_serial_init(int port, int baud)
 {
 	unsigned char c;
 	unsigned divisor;
 
-	outb(0x3, early_serial_base + LCR);	/* 8n1 */
-	outb(0, early_serial_base + IER);	/* no interrupt */
-	outb(0, early_serial_base + FCR);	/* no fifo */
-	outb(0x3, early_serial_base + MCR);	/* DTR + RTS */
+	outb(0x3, port + LCR);	/* 8n1 */
+	outb(0, port + IER);	/* no interrupt */
+	outb(0, port + FCR);	/* no fifo */
+	outb(0x3, port + MCR);	/* DTR + RTS */
 
 	divisor	= 115200 / baud;
-	c = inb(early_serial_base + LCR);
-	outb(c | DLAB, early_serial_base + LCR);
-	outb(divisor & 0xff, early_serial_base + DLL);
-	outb((divisor >> 8) & 0xff, early_serial_base + DLH);
-	outb(c & ~DLAB, early_serial_base + LCR);
+	c = inb(port + LCR);
+	outb(c | DLAB, port + LCR);
+	outb(divisor & 0xff, port + DLL);
+	outb((divisor >> 8) & 0xff, port + DLH);
+	outb(c & ~DLAB, port + LCR);
+
+	early_serial_base = port;
+
+	printf("Early serial console at I/O port 0x%x baud: %d\n", port, baud);
 }
 
-static int parse_earlyprintk(void)
+static void parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
 	char arg[32];
 	int pos = 0;
+	int port = 0;
 
 	if (cmdline_find_option("earlyprintk", arg, sizeof arg) > 0) {
 		char *e;
 
 		if (!strncmp(arg, "serial", 6)) {
-			early_serial_base = DEFAULT_SERIAL_PORT;
+			port = DEFAULT_SERIAL_PORT;
 			pos += 6;
 		}
 
@@ -189,15 +194,15 @@ static int parse_earlyprintk(void)
 
 		if (!strncmp(arg, "ttyS", 4)) {
 			static const int bases[] = { 0x3f8, 0x2f8 };
-			int port = 0;
+			int idx = 0;
 
 			if (!strncmp(arg + pos, "ttyS", 4))
 				pos += 4;
 
 			if (arg[pos++] == '1')
-				port = 1;
+				idx = 1;
 
-			early_serial_base = bases[port];
+			port = bases[idx];
 		}
 
 		if (arg[pos] == ',')
@@ -208,7 +213,8 @@ static int parse_earlyprintk(void)
 			baud = DEFAULT_BAUD;
 	}
 
-	return baud;
+	if (port)
+		early_serial_init(port, baud);
 }
 
 #define BASE_BAUD (1843200/16)
@@ -227,44 +233,41 @@ static unsigned int probe_baud(int port)
 	return BASE_BAUD / quot;
 }
 
-static int parse_console_uart8250(void)
+static void parse_console_uart8250(void)
 {
 	char optstr[64], *options;
 	int baud = DEFAULT_BAUD;
+	int port = 0;
 
 	/*
 	 * console=uart8250,io,0x3f8,115200n8
 	 * need to make sure it is last one console !
 	 */
 	if (cmdline_find_option("console", optstr, sizeof optstr) <= 0)
-		return baud;
+		return;
 
 	options = optstr;
 
 	if (!strncmp(options, "uart8250,io,", 12))
-		early_serial_base = simple_strtoull(options + 12, &options, 0);
+		port = simple_strtoull(options + 12, &options, 0);
 	else if (!strncmp(options, "uart,io,", 8))
-		early_serial_base = simple_strtoull(options + 8, &options, 0);
+		port = simple_strtoull(options + 8, &options, 0);
 	else
-		return baud;
+		return;
 
 	if (options && (options[0] == ','))
 		baud = simple_strtoull(options + 1, &options, 0);
 	else
-		baud = probe_baud(early_serial_base);
+		baud = probe_baud(port);
 
-	return baud;
+	if (port)
+		early_serial_init(port, baud);
 }
 
 void console_init(void)
 {
-	int baud;
-
-	baud = parse_earlyprintk();
+	parse_earlyprintk();
 
 	if (!early_serial_base)
-		baud = parse_console_uart8250();
-
-	if (early_serial_base != 0)
-		early_serial_init(baud);
+		parse_console_uart8250();
 }

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

* [tip:x86/setup] x86, setup: Only set early_serial_base after port is initialized
  2010-07-14 18:26                 ` [PATCH -v2] x86: only set early_serial_base after port is initialized in setup code Yinghai Lu
@ 2010-07-14 19:12                   ` tip-bot for Yinghai Lu
  0 siblings, 0 replies; 20+ messages in thread
From: tip-bot for Yinghai Lu @ 2010-07-14 19:12 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, yinghai, penberg, tglx, hpa

Commit-ID:  70b0d22d581a5deef7b2876b0c3774635b8d846c
Gitweb:     http://git.kernel.org/tip/70b0d22d581a5deef7b2876b0c3774635b8d846c
Author:     Yinghai Lu <yinghai@kernel.org>
AuthorDate: Wed, 14 Jul 2010 11:26:57 -0700
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Wed, 14 Jul 2010 12:07:16 -0700

x86, setup: Only set early_serial_base after port is initialized

putchar is using early_serial_base to check if port is initialized.

So we only assign it after early_serial_init() is called,
in case we need use VGA to debug early serial console.

Also add display for port addr and baud.

-v2: update to current tip

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
LKML-Reference: <4C3E0171.6050008@kernel.org>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/boot/tty.c |   63 ++++++++++++++++++++++++++------------------------
 1 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/arch/x86/boot/tty.c b/arch/x86/boot/tty.c
index f6d52e6..ff4b27a 100644
--- a/arch/x86/boot/tty.c
+++ b/arch/x86/boot/tty.c
@@ -152,35 +152,40 @@ int getchar_timeout(void)
 	return 0;		/* Timeout! */
 }
 
-static void early_serial_init(int baud)
+static void early_serial_init(int port, int baud)
 {
 	unsigned char c;
 	unsigned divisor;
 
-	outb(0x3, early_serial_base + LCR);	/* 8n1 */
-	outb(0, early_serial_base + IER);	/* no interrupt */
-	outb(0, early_serial_base + FCR);	/* no fifo */
-	outb(0x3, early_serial_base + MCR);	/* DTR + RTS */
+	outb(0x3, port + LCR);	/* 8n1 */
+	outb(0, port + IER);	/* no interrupt */
+	outb(0, port + FCR);	/* no fifo */
+	outb(0x3, port + MCR);	/* DTR + RTS */
 
 	divisor	= 115200 / baud;
-	c = inb(early_serial_base + LCR);
-	outb(c | DLAB, early_serial_base + LCR);
-	outb(divisor & 0xff, early_serial_base + DLL);
-	outb((divisor >> 8) & 0xff, early_serial_base + DLH);
-	outb(c & ~DLAB, early_serial_base + LCR);
+	c = inb(port + LCR);
+	outb(c | DLAB, port + LCR);
+	outb(divisor & 0xff, port + DLL);
+	outb((divisor >> 8) & 0xff, port + DLH);
+	outb(c & ~DLAB, port + LCR);
+
+	early_serial_base = port;
+
+	printf("Early serial console at I/O port 0x%x baud: %d\n", port, baud);
 }
 
-static int parse_earlyprintk(void)
+static void parse_earlyprintk(void)
 {
 	int baud = DEFAULT_BAUD;
 	char arg[32];
 	int pos = 0;
+	int port = 0;
 
 	if (cmdline_find_option("earlyprintk", arg, sizeof arg) > 0) {
 		char *e;
 
 		if (!strncmp(arg, "serial", 6)) {
-			early_serial_base = DEFAULT_SERIAL_PORT;
+			port = DEFAULT_SERIAL_PORT;
 			pos += 6;
 		}
 
@@ -189,15 +194,15 @@ static int parse_earlyprintk(void)
 
 		if (!strncmp(arg, "ttyS", 4)) {
 			static const int bases[] = { 0x3f8, 0x2f8 };
-			int port = 0;
+			int idx = 0;
 
 			if (!strncmp(arg + pos, "ttyS", 4))
 				pos += 4;
 
 			if (arg[pos++] == '1')
-				port = 1;
+				idx = 1;
 
-			early_serial_base = bases[port];
+			port = bases[idx];
 		}
 
 		if (arg[pos] == ',')
@@ -208,7 +213,8 @@ static int parse_earlyprintk(void)
 			baud = DEFAULT_BAUD;
 	}
 
-	return baud;
+	if (port)
+		early_serial_init(port, baud);
 }
 
 #define BASE_BAUD (1843200/16)
@@ -227,44 +233,41 @@ static unsigned int probe_baud(int port)
 	return BASE_BAUD / quot;
 }
 
-static int parse_console_uart8250(void)
+static void parse_console_uart8250(void)
 {
 	char optstr[64], *options;
 	int baud = DEFAULT_BAUD;
+	int port = 0;
 
 	/*
 	 * console=uart8250,io,0x3f8,115200n8
 	 * need to make sure it is last one console !
 	 */
 	if (cmdline_find_option("console", optstr, sizeof optstr) <= 0)
-		return baud;
+		return;
 
 	options = optstr;
 
 	if (!strncmp(options, "uart8250,io,", 12))
-		early_serial_base = simple_strtoull(options + 12, &options, 0);
+		port = simple_strtoull(options + 12, &options, 0);
 	else if (!strncmp(options, "uart,io,", 8))
-		early_serial_base = simple_strtoull(options + 8, &options, 0);
+		port = simple_strtoull(options + 8, &options, 0);
 	else
-		return baud;
+		return;
 
 	if (options && (options[0] == ','))
 		baud = simple_strtoull(options + 1, &options, 0);
 	else
-		baud = probe_baud(early_serial_base);
+		baud = probe_baud(port);
 
-	return baud;
+	if (port)
+		early_serial_init(port, baud);
 }
 
 void console_init(void)
 {
-	int baud;
-
-	baud = parse_earlyprintk();
+	parse_earlyprintk();
 
 	if (!early_serial_base)
-		baud = parse_console_uart8250();
-
-	if (early_serial_base != 0)
-		early_serial_init(baud);
+		parse_console_uart8250();
 }

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

end of thread, other threads:[~2010-07-14 19:13 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-11 21:44 [PATCH] x86: Setup early console as early as possible Yinghai Lu
2010-07-12  8:58 ` Pekka Enberg
2010-07-12 15:47   ` H. Peter Anvin
2010-07-12 16:21     ` Yinghai Lu
2010-07-12 17:30       ` H. Peter Anvin
2010-07-13 17:43       ` Pekka Enberg
2010-07-13 20:35         ` Yinghai Lu
2010-07-13 20:46           ` Pekka Enberg
2010-07-14  2:07             ` [PATCH] x86: only set early_serial_base after port is initialized Yinghai Lu
2010-07-14  8:41               ` Pekka Enberg
2010-07-14 18:26                 ` [PATCH -v2] x86: only set early_serial_base after port is initialized in setup code Yinghai Lu
2010-07-14 19:12                   ` [tip:x86/setup] x86, setup: Only set early_serial_base after port is initialized tip-bot for Yinghai Lu
2010-07-13 21:12           ` [tip:x86/setup] x86, setup: Make the setup code also accept console=uart8250 tip-bot for Yinghai Lu
2010-07-12 17:44     ` [PATCH] x86: Setup early console as early as possible Cyrill Gorcunov
2010-07-12 18:09       ` H. Peter Anvin
2010-07-12 18:11         ` Yinghai Lu
2010-07-12 22:57           ` Jeremy Fitzhardinge
2010-07-12 23:37             ` Yinghai Lu
     [not found] <f5czp-42V-9@gated-at.bofh.it>
     [not found] ` <f5n1M-2lt-21@gated-at.bofh.it>
     [not found]   ` <f5tqz-3ll-37@gated-at.bofh.it>
     [not found]     ` <f5u3g-4mj-11@gated-at.bofh.it>
     [not found]       ` <f5v90-5Wo-7@gated-at.bofh.it>
2010-07-12 20:57         ` Bodo Eggert
2010-07-12 21:52           ` H. Peter Anvin

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).