x86: Setup early console as early as possible

PATCH In review Yinghai Lu 6 participants 18 messages Jul 11, 2010 – Jul 14, 2010 x86

Share & tools

Yinghai Lu

author

2010-07-11 21:48 UTC #0 lore raw

PATCH 1/1 x86: Setup early console as early as possible

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 <[email protected]>

---
 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(+)

arch/x86/include/asm/setup.h +1 -0

Index: 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);

arch/x86/kernel/head.c +32 -0

Index: 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

arch/x86/kernel/head32.c +2 -0

Index: 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

arch/x86/kernel/head64.c +11 -0

Index: 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");

kernel/printk.c +4 -0

Index: 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);

Pekka Enberg

2010-07-12 08:59 UTC #1 lore raw

Hi Yinghai,

Quoted 6 lines · Yinghai Lu

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 <[email protected]>

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

Pekka

H. Peter Anvin

reviewer

2010-07-12 15:48 UTC #2 lore raw

Quoted 7 lines · Pekka Enberg

Hi Yinghai,
Quoted 4 lines · Yinghai Lu
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.

Quoted 8 lines

Quoted 3 lines
only can handle io port kind of 8250. because mmio need ioremap.
Signed-off-by Yinghai Lu <[email protected]>
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

Signature · 2 lines

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

Yinghai Lu

2010-07-12 16:24 UTC #3 lore raw

Quoted 12 lines · H. Peter Anvin

Quoted 11 lines · Pekka Enberg
Hi Yinghai,
Quoted 7 lines · Yinghai Lu
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 <[email protected]>

Quoted 7 lines

Quoted 4 lines
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.

Quoted 4 lines

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

H. Peter Anvin

reviewer

2010-07-12 17:31 UTC #4 lore raw

Quoted 8 lines · Yinghai Lu

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.

Quoted 17 lines

Quoted 4 lines
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()

Quoted 8 lines

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

Pekka Enberg

2010-07-13 17:43 UTC #5 lore raw

In reply to Yinghai Lu

Hi Yingahai,

Quoted 19 lines · Yinghai Lu

Quoted 10 lines · H. Peter Anvin
Quoted 9 lines · Pekka Enberg
Hi Yinghai,
Quoted 6 lines · Yinghai Lu
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.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Yinghai Lu <[email protected]>
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.

Quoted 8 lines

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

Yinghai Lu

2010-07-13 20:41 UTC #6 lore raw

Quoted 32 lines · Pekka Enberg

Hi Yingahai,
Quoted 29 lines · Yinghai Lu
Quoted 11 lines · H. Peter Anvin
Quoted 10 lines · Pekka Enberg
Hi Yinghai,
Quoted 7 lines · Yinghai Lu
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.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Signed-off-by Yinghai Lu <[email protected]>
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.

Quoted 6 lines

Quoted 2 lines
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 <[email protected]>

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

arch/x86/boot/tty.c +58 -1

Index: 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);
 }

arch/x86/boot/string.c +22 -0

Index: 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;

Pekka Enberg

Acked-by

2010-07-13 20:46 UTC #7 lore raw

Quoted 8 lines · 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 <[email protected]>

Cool! Thanks Yinghai!

Acked-by Pekka Enberg <[email protected]>

Yinghai Lu

2010-07-14 02:12 UTC #8 lore raw

PATCH 1/1 x86: 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.

Signed-off-by Yinghai Lu <[email protected]>

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

arch/x86/boot/tty.c +33 ~~-31~~

Index: 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();
 }

Pekka Enberg

Acked-by on v1 1/1

2010-07-14 08:41 UTC #9 lore raw

Re: [PATCH] x86: only set early_serial_base after port is initialized

Quoted 9 lines · Yinghai Lu

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 <[email protected]>

Acked-by Pekka Enberg <[email protected]>

Yinghai Lu

2010-07-14 18:31 UTC #10 lore raw

PATCH 1/1 x86: only set early_serial_base after port is initialized in setup code

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 <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

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

arch/x86/boot/tty.c +33 ~~-30~~

Index: 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();
 }

tip-bot for Yinghai Lu

2010-07-14 19:13 UTC #11 lore raw

[tip:x86/setup] x86, setup: Only set early_serial_base after port is initialized

Commit-ID:  70b0d22d581a5deef7b2876b0c3774635b8d846c
Gitweb:     http://git.kernel.org/tip/70b0d22d581a5deef7b2876b0c3774635b8d846c
Author:     Yinghai Lu <[email protected]>
AuthorDate: Wed, 14 Jul 2010 11:26:57 -0700
Committer:  H. Peter Anvin <[email protected]>
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 <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by H. Peter Anvin <[email protected]>

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

arch/x86/boot/tty.c +33 ~~-30~~

index f6d52e6..ff4b27a 100644
@@ -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();
 }

tip-bot for Yinghai Lu

2010-07-13 21:13 UTC #12 lore raw

[tip:x86/setup] x86, setup: Make the setup code also accept console=uart8250

In reply to Yinghai Lu

Commit-ID:  ce0aa5dd20e44372f9617dd67c984f41fcdbed88
Gitweb:     http://git.kernel.org/tip/ce0aa5dd20e44372f9617dd67c984f41fcdbed88
Author:     Yinghai Lu <[email protected]>
AuthorDate: Tue, 13 Jul 2010 13:35:17 -0700
Committer:  H. Peter Anvin <[email protected]>
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 <[email protected]>

LKML-Reference: <[email protected]>

Acked-by Pekka Enberg <[email protected]>

Signed-off-by H. Peter Anvin <[email protected]>

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

arch/x86/boot/string.c +22 -0

index aba29df..3cbc405 100644
@@ -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;

arch/x86/boot/tty.c +58 -1

71 lines not on this page · raw message

Cyrill Gorcunov

2010-07-12 17:44 UTC #13 lore raw

In reply to H. Peter Anvin

Quoted 17 lines · H. Peter Anvin

Quoted 7 lines · Pekka Enberg
Hi Yinghai,
Quoted 4 lines · Yinghai Lu
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.
Quoted 8 lines
Quoted 3 lines
only can handle io port kind of 8250. because mmio need ioremap.
Signed-off-by Yinghai Lu <[email protected]>
What's the purpose of this patch? Does it make my early boot I/O patch obsolete?
Pekka

Quoted 9 lines

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 <[email protected]>
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 <[email protected]>

---
 drivers/serial/8250_early.c |    3 +++
 1 file changed, 3 insertions(+)

drivers/serial/8250_early.c +3 -0

Index: 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,");

H. Peter Anvin

reviewer

2010-07-12 18:11 UTC #14 lore raw

Quoted 6 lines · Cyrill Gorcunov

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

Quoted 7 lines

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

Yinghai Lu

author

2010-07-12 18:15 UTC #15 lore raw

Quoted 9 lines · H. Peter Anvin

Quoted 8 lines · Cyrill Gorcunov
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,

Quoted 8 lines

Quoted 5 lines
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

Jeremy Fitzhardinge

2010-07-12 22:57 UTC #16 lore raw

Quoted 17 lines · Yinghai Lu

Quoted 15 lines · H. Peter Anvin
Quoted 13 lines · Cyrill Gorcunov
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.

Quoted 7 lines

Quoted 3 lines
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?

Yinghai Lu

author

2010-07-12 23:40 UTC #17 lore raw

Quoted 35 lines · Jeremy Fitzhardinge

Quoted 17 lines · Yinghai Lu
Quoted 15 lines · H. Peter Anvin
Quoted 13 lines · Cyrill Gorcunov
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.
Quoted 7 lines
Quoted 3 lines
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

Quoted 5 lines

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