[PATCH] 2.6.4-rc1 fix x86 early_printk and make it early

[PATCH] 2.6.4-rc1 fix x86 early_printk and make it early

[Eric W. Biederman]

Trying to use early_printk on x86 I found two issues.
- setup_early_printk is currently called much later than necessary.
- VGABASE is using an identity mapped physical address on x86.
  o That is problematic with PAE support, because there is a moment
    during paging_init() when the physical identity mappings do not
    work. 
  o Using raw physical addresses is in bad form, and doesn't always work.
  o I can't possibly see how Andrew's Changelog that using __pa
    is more friendly to the 4G/4G split is correct.  Unless someone
    was hard coding a virtual address previously.


The first hunk of this patch moves setup_early_printk as
early as possible on x86.

The second hunk in early_printk.c redefines VGABASE as __va(0xb8000).
This is correct on both x86 and x86-64 so we don't need any more
special cases.  The only thing that might be slightly more correct
would be to use isa_readw/isa_writew but the only difference is
in where PAGE_OFFSET is added in.

Eric


diff -uNrX linux-ignore-files linux-2.6.4-rc1/arch/i386/kernel/setup.c linux-2.6.4-rc1.earlyprintk/arch/i386/kernel/setup.c
--- linux-2.6.4-rc1/arch/i386/kernel/setup.c	Sun Feb 29 14:09:16 2004
+++ linux-2.6.4-rc1.earlyprintk/arch/i386/kernel/setup.c	Sun Feb 29 19:10:40 2004
@@ -1048,6 +1048,18 @@
 {
 	unsigned long max_low_pfn;
 
+#ifdef CONFIG_EARLY_PRINTK
+	extern void setup_early_printk(char *);
+	char *str;
+
+	COMMAND_LINE[COMMAND_LINE_SIZE -1] = '\0';
+	str = strstr(COMMAND_LINE, "earlyprintk=");
+	if (str) {
+		setup_early_printk(str);
+		printk("early console enabled\n");
+	}
+#endif
+
 	memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
 	pre_setup_arch_hook();
 	early_cpu_init();
@@ -1117,19 +1129,6 @@
 	smp_alloc_memory(); /* AP processor realmode stacks in low memory*/
 #endif
 	paging_init();
-
-#ifdef CONFIG_EARLY_PRINTK
-	{
-		char *s = strstr(*cmdline_p, "earlyprintk=");
-		if (s) {
-			extern void setup_early_printk(char *);
-
-			setup_early_printk(s);
-			printk("early console enabled\n");
-		}
-	}
-#endif
-
 
 	dmi_scan_machine();
 
diff -uNrX linux-ignore-files linux-2.6.4-rc1/arch/x86_64/kernel/early_printk.c linux-2.6.4-rc1.earlyprintk/arch/x86_64/kernel/early_printk.c
--- linux-2.6.4-rc1/arch/x86_64/kernel/early_printk.c	Sun Feb 29 21:24:55 2004
+++ linux-2.6.4-rc1.earlyprintk/arch/x86_64/kernel/early_printk.c	Sun Feb 29 21:26:02 2004
@@ -7,11 +7,7 @@
 
 /* Simple VGA output */
 
-#ifdef __i386__
-#define VGABASE		__pa(__PAGE_OFFSET + 0xb8000UL)
-#else
-#define VGABASE		0xffffffff800b8000UL
-#endif
+#define VGABASE		__va(0xb8000UL)
 
 #define MAX_YPOS	25
 #define MAX_XPOS	80

Re: [PATCH] 2.6.4-rc1 fix x86 early_printk and make it early

[Andi Kleen]

On 29 Feb 2004 22:24:12 -0700
ebiederm@xmission.com (Eric W. Biederman) wrote:

>   o That is problematic with PAE support, because there is a moment
>     during paging_init() when the physical identity mappings do not
>     work. 

Just don't printk in that moment then.

>   o Using raw physical addresses is in bad form, and doesn't always work.

On x86-64 it's that __pa() doesn't always work very early.

>   o I can't possibly see how Andrew's Changelog that using __pa
>     is more friendly to the 4G/4G split is correct.  Unless someone
>     was hard coding a virtual address previously.

Yes, it shouldn't make any difference for 4/4.

> The second hunk in early_printk.c redefines VGABASE as __va(0xb8000).
> This is correct on both x86 and x86-64 so we don't need any more
> special cases.  

Please don't do that on x86-64. On x86-64 there are two ways to reach this
address and the previous one is available earlier.  Keep the current
address for x86-64 please.

-Andi

Re: [PATCH] 2.6.4-rc1 fix x86 early_printk and make it early

[Eric W. Biederman]

Andi Kleen <ak@suse.de> writes:

> On 29 Feb 2004 22:24:12 -0700
> ebiederm@xmission.com (Eric W. Biederman) wrote:
> 
> >   o That is problematic with PAE support, because there is a moment
> >     during paging_init() when the physical identity mappings do not
> >     work. 
> 
> Just don't printk in that moment then.
> 
> >   o Using raw physical addresses is in bad form, and doesn't always work.
> 
> On x86-64 it's that __pa() doesn't always work very early.
> 
> >   o I can't possibly see how Andrew's Changelog that using __pa
> >     is more friendly to the 4G/4G split is correct.  Unless someone
> >     was hard coding a virtual address previously.
> 
> Yes, it shouldn't make any difference for 4/4.
> 
> > The second hunk in early_printk.c redefines VGABASE as __va(0xb8000).
> > This is correct on both x86 and x86-64 so we don't need any more
> > special cases.  
> 
> Please don't do that on x86-64. On x86-64 there are two ways to reach this
> address and the previous one is available earlier.  Keep the current
> address for x86-64 please.

???  I checked the x86-64 early page tables and it does appear to work.
That is why I consolidated the change.  

In particular from arch/x86_64/head.S:

.org 0x1000
ENTRY(init_level4_pgt)
	.quad	0x0000000000102007		/* -> level3_ident_pgt */
	.fill	255,8,0
	.quad	0x000000000010a007
	.fill	254,8,0
	/* (2^48-(2*1024*1024*1024))/(2^39) = 511 */
	.quad	0x0000000000103007		/* -> level3_kernel_pgt */

.org 0x3000
ENTRY(level3_kernel_pgt)
	.fill	510,8,0
	/* (2^48-(2*1024*1024*1024)-((2^39)*511))/(2^30) = 510 */
	.quad	0x0000000000105007		/* -> level2_kernel_pgt */
	.fill	1,8,0

.org 0xa000
ENTRY(level3_physmem_pgt)
	.quad	0x0000000000105007		/* -> level2_kernel_pgt (so that __va works even before pagetable_init) */

0x0000010000000000 is handled by the pointer: 0x000000000010a007
0xffffffff80000000 is handled by the pointer: 0x0000000000103007

And then they both point to: 0x0000000000103007 which is the physical mapping.

And of course there is the comment on the 0xa000 entry:
(so that __va works even before pagetable_init)

So I don't see how using __va() early in the code is incorrect.  I can
buy that __va() adds the wrong constant but that is another matter.

Eric

Re: [PATCH] 2.6.4-rc1 fix x86 early_printk and make it early

[Eric W. Biederman]

Andi Kleen <ak@suse.de> writes:

> On 29 Feb 2004 22:24:12 -0700
> ebiederm@xmission.com (Eric W. Biederman) wrote:
> 
> >   o That is problematic with PAE support, because there is a moment
> >     during paging_init() when the physical identity mappings do not
> >     work. 
> 
> Just don't printk in that moment then.
> 
> >   o Using raw physical addresses is in bad form, and doesn't always work.
> 
> On x86-64 it's that __pa() doesn't always work very early.
> 
> >   o I can't possibly see how Andrew's Changelog that using __pa
> >     is more friendly to the 4G/4G split is correct.  Unless someone
> >     was hard coding a virtual address previously.
> 
> Yes, it shouldn't make any difference for 4/4.
> 
> > The second hunk in early_printk.c redefines VGABASE as __va(0xb8000).
> > This is correct on both x86 and x86-64 so we don't need any more
> > special cases.  
> 
> Please don't do that on x86-64. On x86-64 there are two ways to reach this
> address and the previous one is available earlier.  Keep the current
> address for x86-64 please.

Andi much to my great surprise after reading through the comments 
that actually didn't work on x86-64.   So here is my corrected patch.

It still uses __va() for everything but it fixes x86-64 so that it works,
I just had to move one page table entry...

As for 0xffff000000000000 which the old page table entry applied to
I grepped the entire kernel tree and could not find anything that
applied to x86_64.  So it looks like a left over from an earlier definition
of PAGE_OFFSET.

So can we please get this applied to 2.6?  Now that it is almost purely
mini bug fixes?

Eric


diff -uNrX linux-ignore-files linux-2.6.4-rc1/arch/i386/kernel/setup.c linux-2.6.4-rc1.earlyprintk/arch/i386/kernel/setup.c
--- linux-2.6.4-rc1/arch/i386/kernel/setup.c	Sun Feb 29 14:09:16 2004
+++ linux-2.6.4-rc1.earlyprintk/arch/i386/kernel/setup.c	Sun Feb 29 19:10:40 2004
@@ -1048,6 +1048,18 @@
 {
 	unsigned long max_low_pfn;
 
+#ifdef CONFIG_EARLY_PRINTK
+	extern void setup_early_printk(char *);
+	char *str;
+
+	COMMAND_LINE[COMMAND_LINE_SIZE -1] = '\0';
+	str = strstr(COMMAND_LINE, "earlyprintk=");
+	if (str) {
+		setup_early_printk(str);
+		printk("early console enabled\n");
+	}
+#endif
+
 	memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
 	pre_setup_arch_hook();
 	early_cpu_init();
@@ -1117,19 +1129,6 @@
 	smp_alloc_memory(); /* AP processor realmode stacks in low memory*/
 #endif
 	paging_init();
-
-#ifdef CONFIG_EARLY_PRINTK
-	{

-		char *s = strstr(*cmdline_p, "earlyprintk=");
-		if (s) {
-			extern void setup_early_printk(char *);
-
-			setup_early_printk(s);
-			printk("early console enabled\n");
-		}
-	}
-#endif
-
 
 	dmi_scan_machine();
 
diff -uNrX linux-ignore-files linux-2.6.4-rc1/arch/x86_64/kernel/early_printk.c linux-2.6.4-rc1.earlyprintk/arch/x86_64/kernel/early_printk.c
--- linux-2.6.4-rc1/arch/x86_64/kernel/early_printk.c	Sun Feb 29 21:24:55 2004
+++ linux-2.6.4-rc1.earlyprintk/arch/x86_64/kernel/early_printk.c	Sun Feb 29 21:26:02 2004
@@ -7,11 +7,7 @@
 
 /* Simple VGA output */
 
-#ifdef __i386__
-#define VGABASE		__pa(__PAGE_OFFSET + 0xb8000UL)
-#else
-#define VGABASE		0xffffffff800b8000UL
-#endif
+#define VGABASE		__va(0xb8000UL)
 
 #define MAX_YPOS	25
 #define MAX_XPOS	80
diff -uNrX linux-ignore-files linux-2.6.4-rc1/arch/x86_64/kernel/head.S linux-2.6.4-rc1.earlyprintk/arch/x86_64/kernel/head.S
--- linux-2.6.4-rc1/arch/x86_64/kernel/head.S	Sun Feb 29 14:09:23 2004
+++ linux-2.6.4-rc1.earlyprintk/arch/x86_64/kernel/head.S	Tue Mar  2 01:10:52 2004
@@ -206,9 +206,10 @@
 .org 0x1000
 ENTRY(init_level4_pgt)
 	.quad	0x0000000000102007		/* -> level3_ident_pgt */
-	.fill	255,8,0
+	.fill	1,8,0
+	/* 2^40/2^39 = 2 */
 	.quad	0x000000000010a007
-	.fill	254,8,0
+	.fill	508,8,0
 	/* (2^48-(2*1024*1024*1024))/(2^39) = 511 */
 	.quad	0x0000000000103007		/* -> level3_kernel_pgt */