Re: [PATCH 00 of 10] x86: unify asm/pgtable.h

[Ingo Molnar <mingo@elte.hu>]

* Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > > >>>  #define __PAGE_KERNEL_EXEC						\
> > > >>> -	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_GLOBAL)
> > > >>> +	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED)
> > > >>>     
> > > >
> > > > This shouldn't be necessary.  The old 64-bit code defined everything 
> > > > without _PAGE_GLOBAL, but then used a MAKE_GLOBAL() macro to OR it 
> > > > in later.  This seemed a bit roundabout to me, so I just put it in 
> > > > from the outset.
> > 
> > actually, this is wrong.
> > 
> > a couple of places use __PAGE_* values, which you've now changed to 
> > include the _PAGE_GLOBAL flag.
> 
> yep, fixing this resolves the crash.

but then it crashes init:

 init[1]: segfault at ffffffffff600400 ip ffffffffff600400 sp 
 7fff81bedda8 error5printk: 7740690 messages suppressed.

because you changed __PAGE_KERNEL_VSYSCALL as well, from:

 #define __PAGE_KERNEL_VSYSCALL \
         (_PAGE_PRESENT | _PAGE_USER | _PAGE_ACCESSED)

to:

 #define __PAGE_KERNEL_VSYSCALL          (__PAGE_KERNEL_RO | _PAGE_USER)

but __PAGE_KERNEL_RO is an NX one, so the vsyscall cannot execute 
instructions.

so this wants to be:

 #define __PAGE_KERNEL_VSYSCALL          (__PAGE_KERNEL_RX | _PAGE_USER)

this was the last bug and the resulting kernel boots fine. 3 nasty bugs 
in one patch :-/

	Ingo

---------------->
Subject: x86: move all asm/pgtable constants into one place, fix
From: Ingo Molnar <mingo@elte.hu>

fix.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/asm-x86/pgtable.h |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Index: linux-x86.q/include/asm-x86/pgtable.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/pgtable.h
+++ linux-x86.q/include/asm-x86/pgtable.h
@@ -68,27 +68,27 @@ extern unsigned long long __PAGE_KERNEL,
 #endif	/* __ASSEMBLER__ */
 #else
 #define __PAGE_KERNEL_EXEC						\
-	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_GLOBAL)
+	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED)
 #define __PAGE_KERNEL		(__PAGE_KERNEL_EXEC | _PAGE_NX)
 #endif
 
 #define __PAGE_KERNEL_RO		(__PAGE_KERNEL & ~_PAGE_RW)
 #define __PAGE_KERNEL_RX		(__PAGE_KERNEL_EXEC & ~_PAGE_RW)
 #define __PAGE_KERNEL_NOCACHE		(__PAGE_KERNEL | _PAGE_PCD | _PAGE_PWT)
-#define __PAGE_KERNEL_VSYSCALL		(__PAGE_KERNEL_RO | _PAGE_USER)
+#define __PAGE_KERNEL_VSYSCALL		(__PAGE_KERNEL_RX | _PAGE_USER)
 #define __PAGE_KERNEL_VSYSCALL_NOCACHE	(__PAGE_KERNEL_VSYSCALL | _PAGE_PCD | _PAGE_PWT)
 #define __PAGE_KERNEL_LARGE		(__PAGE_KERNEL | _PAGE_PSE)
 #define __PAGE_KERNEL_LARGE_EXEC	(__PAGE_KERNEL_EXEC | _PAGE_PSE)
 
-#define PAGE_KERNEL			__pgprot(__PAGE_KERNEL)
-#define PAGE_KERNEL_RO			__pgprot(__PAGE_KERNEL_RO)
-#define PAGE_KERNEL_EXEC		__pgprot(__PAGE_KERNEL_EXEC)
-#define PAGE_KERNEL_RX			__pgprot(__PAGE_KERNEL_RX)
-#define PAGE_KERNEL_NOCACHE		__pgprot(__PAGE_KERNEL_NOCACHE)
-#define PAGE_KERNEL_LARGE		__pgprot(__PAGE_KERNEL_LARGE)
-#define PAGE_KERNEL_LARGE_EXEC		__pgprot(__PAGE_KERNEL_LARGE_EXEC)
-#define PAGE_KERNEL_VSYSCALL		__pgprot(__PAGE_KERNEL_VSYSCALL)
-#define PAGE_KERNEL_VSYSCALL_NOCACHE	__pgprot(__PAGE_KERNEL_VSYSCALL_NOCACHE)
+#define PAGE_KERNEL			__pgprot(__PAGE_KERNEL | _PAGE_GLOBAL)
+#define PAGE_KERNEL_RO			__pgprot(__PAGE_KERNEL_RO | _PAGE_GLOBAL)
+#define PAGE_KERNEL_EXEC		__pgprot(__PAGE_KERNEL_EXEC | _PAGE_GLOBAL)
+#define PAGE_KERNEL_RX			__pgprot(__PAGE_KERNEL_RX | _PAGE_GLOBAL)
+#define PAGE_KERNEL_NOCACHE		__pgprot(__PAGE_KERNEL_NOCACHE | _PAGE_GLOBAL)
+#define PAGE_KERNEL_LARGE		__pgprot(__PAGE_KERNEL_LARGE | _PAGE_GLOBAL)
+#define PAGE_KERNEL_LARGE_EXEC		__pgprot(__PAGE_KERNEL_LARGE_EXEC | _PAGE_GLOBAL)
+#define PAGE_KERNEL_VSYSCALL		__pgprot(__PAGE_KERNEL_VSYSCALL | _PAGE_GLOBAL)
+#define PAGE_KERNEL_VSYSCALL_NOCACHE	__pgprot(__PAGE_KERNEL_VSYSCALL_NOCACHE | _PAGE_GLOBAL)
 
 /*         xwr */
 #define __P000	PAGE_NONE