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

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> * Ingo Molnar <mingo@xxxxxxx> wrote:
>
>   
> > * Ingo Molnar <mingo@xxxxxxx> 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 :-/
>   

:-/ indeed.  One #ifdef/include-hell bug, one typo, and one... well, see 
my other mail.  I think my _PAGE_GLOBAL change is actually valid, and 
the bug is in change_page_attr after all.

   J

> 	Ingo
>
> ---------------->
> Subject: x86: move all asm/pgtable constants into one place, fix
> From: Ingo Molnar <mingo@xxxxxxx>
>
> fix.
>
> Signed-off-by: Ingo Molnar <mingo@xxxxxxx>
> ---
>  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
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/