[PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[Benjamin Herrenschmidt]

The test to check whether we have _PAGE_SPECIAL defined is broken,
since we always define it, just not always to a meaninful value :-)

That broke 8xx and 40x under some circumstances.

This fixes it by adding _PAGE_SPECIAL for both of these since they
had a free PTE bit, and removing the condition around advertising
it.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Bernhard, Rex, please let me know if that works for you.


diff --git a/arch/powerpc/include/asm/pte-40x.h b/arch/powerpc/include/asm/pte-40x.h
index 6c3e1f4..ec0b0b0 100644
--- a/arch/powerpc/include/asm/pte-40x.h
+++ b/arch/powerpc/include/asm/pte-40x.h
@@ -43,6 +43,7 @@
 #define	_PAGE_NO_CACHE	0x004	/* I: caching is inhibited */
 #define	_PAGE_WRITETHRU	0x008	/* W: caching is write-through */
 #define	_PAGE_USER	0x010	/* matches one of the zone permission bits */
+#define	_PAGE_SPECIAL	0x020	/* software: Special page */
 #define	_PAGE_RW	0x040	/* software: Writes permitted */
 #define	_PAGE_DIRTY	0x080	/* software: dirty page */
 #define _PAGE_HWWRITE	0x100	/* hardware: Dirty & RW, set in exception */
diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/pte-8xx.h
index 94e9797..dd5ea95 100644
--- a/arch/powerpc/include/asm/pte-8xx.h
+++ b/arch/powerpc/include/asm/pte-8xx.h
@@ -32,6 +32,7 @@
 #define _PAGE_FILE	0x0002	/* when !present: nonlinear file mapping */
 #define _PAGE_NO_CACHE	0x0002	/* I: cache inhibit */
 #define _PAGE_SHARED	0x0004	/* No ASID (context) compare */
+#define _PAGE_SPECIAL	0x0008	/* SW entry, forced to 0 by the TLB miss */
 
 /* These five software bits must be masked out when the entry is loaded
  * into the TLB.
diff --git a/arch/powerpc/include/asm/pte-common.h b/arch/powerpc/include/asm/pte-common.h
index c3b6507..f2b3701 100644
--- a/arch/powerpc/include/asm/pte-common.h
+++ b/arch/powerpc/include/asm/pte-common.h
@@ -25,9 +25,6 @@
 #ifndef _PAGE_WRITETHRU
 #define _PAGE_WRITETHRU	0
 #endif
-#ifndef _PAGE_SPECIAL
-#define _PAGE_SPECIAL	0
-#endif
 #ifndef _PAGE_4K_PFN
 #define _PAGE_4K_PFN		0
 #endif
@@ -179,7 +176,5 @@ extern unsigned long bad_call_to_PMD_PAGE_SIZE(void);
 #define HAVE_PAGE_AGP
 
 /* Advertise support for _PAGE_SPECIAL */
-#ifdef _PAGE_SPECIAL
 #define __HAVE_ARCH_PTE_SPECIAL
-#endif
 

Re: [PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[Weirich, Bernhard]

Works perfectly here. Thank you for the patch, I was not feeling confident =
enough to play around with the page bits :)

Bernhard WEIRICH
Software Development
________________________________________
Von: Benjamin Herrenschmidt [benh@kernel.crashing.org]
Gesendet: Mittwoch, 23. September 2009 06:12
An: Weirich, Bernhard; Rex Feany
Cc: linuxppc-dev list
Betreff: [PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

The test to check whether we have _PAGE_SPECIAL defined is broken,
since we always define it, just not always to a meaninful value :-)

That broke 8xx and 40x under some circumstances.

This fixes it by adding _PAGE_SPECIAL for both of these since they
had a free PTE bit, and removing the condition around advertising
it.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Bernhard, Rex, please let me know if that works for you.


diff --git a/arch/powerpc/include/asm/pte-40x.h b/arch/powerpc/include/asm/=
pte-40x.h
index 6c3e1f4..ec0b0b0 100644
--- a/arch/powerpc/include/asm/pte-40x.h
+++ b/arch/powerpc/include/asm/pte-40x.h
@@ -43,6 +43,7 @@
 #define        _PAGE_NO_CACHE  0x004   /* I: caching is inhibited */
 #define        _PAGE_WRITETHRU 0x008   /* W: caching is write-through */
 #define        _PAGE_USER      0x010   /* matches one of the zone permissi=
on bits */
+#define        _PAGE_SPECIAL   0x020   /* software: Special page */
 #define        _PAGE_RW        0x040   /* software: Writes permitted */
 #define        _PAGE_DIRTY     0x080   /* software: dirty page */
 #define _PAGE_HWWRITE  0x100   /* hardware: Dirty & RW, set in exception *=
/
diff --git a/arch/powerpc/include/asm/pte-8xx.h b/arch/powerpc/include/asm/=
pte-8xx.h
index 94e9797..dd5ea95 100644
--- a/arch/powerpc/include/asm/pte-8xx.h
+++ b/arch/powerpc/include/asm/pte-8xx.h
@@ -32,6 +32,7 @@
 #define _PAGE_FILE     0x0002  /* when !present: nonlinear file mapping */
 #define _PAGE_NO_CACHE 0x0002  /* I: cache inhibit */
 #define _PAGE_SHARED   0x0004  /* No ASID (context) compare */
+#define _PAGE_SPECIAL  0x0008  /* SW entry, forced to 0 by the TLB miss */

 /* These five software bits must be masked out when the entry is loaded
  * into the TLB.
diff --git a/arch/powerpc/include/asm/pte-common.h b/arch/powerpc/include/a=
sm/pte-common.h
index c3b6507..f2b3701 100644
--- a/arch/powerpc/include/asm/pte-common.h
+++ b/arch/powerpc/include/asm/pte-common.h
@@ -25,9 +25,6 @@
 #ifndef _PAGE_WRITETHRU
 #define _PAGE_WRITETHRU        0
 #endif
-#ifndef _PAGE_SPECIAL
-#define _PAGE_SPECIAL  0
-#endif
 #ifndef _PAGE_4K_PFN
 #define _PAGE_4K_PFN           0
 #endif
@@ -179,7 +176,5 @@ extern unsigned long bad_call_to_PMD_PAGE_SIZE(void);
 #define HAVE_PAGE_AGP

 /* Advertise support for _PAGE_SPECIAL */
-#ifdef _PAGE_SPECIAL
 #define __HAVE_ARCH_PTE_SPECIAL
-#endif

Re: [PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[Rex Feany]

Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):

> Bernhard, Rex, please let me know if that works for you.

it doesn't work for me, it crashes differently then before though!

This patch worked for me:
> 
> Signed-off-by: Bernhard Weirich <bernhard.weirich@riedel.net>
> --- linux/arch/powerpc/include/asm/pte-common.h 2009-09-16 11:23:44.000000000 +0200
> +++ linux/arch/powerpc/include/asm/pte-common.h 2009-09-22 11:13:08.000000000 +0200
> @@ -176,7 +176,7 @@
>  #define HAVE_PAGE_AGP
>  
>  /* Advertise support for _PAGE_SPECIAL */
> -#ifdef _PAGE_SPECIAL
> +#if _PAGE_SPECIAL != 0
>  #define __HAVE_ARCH_PTE_SPECIAL
>  #endif
> 

without either patch:

boots into userspace, starts running my code
which uses mmap to access an fpga:

BUG: Bad page map in process loadfpga  pte:ff1008d3 pmd:02103001
addr:30028000 vm_flags:400844fb anon_vma:(null) mapping:c30000e8 index:ff100
vma->vm_ops->fault: 0x0
vma->vm_file->f_op->mmap: mmap_mem+0x0/0xa8
Call Trace:
[c2107db0] [c00067d0] show_stack+0x44/0x14c (unreliable)
[c2107df0] [c0056854] print_bad_pte+0x1b4/0x1d0
[c2107e20] [c0056898] vm_normal_page+0x28/0x50
[c2107e30] [c0056c30] unmap_vmas+0x1b8/0x510
[c2107eb0] [c005b17c] unmap_region+0x94/0x124
[c2107ef0] [c005c110] do_munmap+0x264/0x2dc
[c2107f20] [c005c1c0] sys_munmap+0x38/0x5c
[c2107f40] [c000d0dc] ret_from_syscall+0x0/0x38

trunk+benh complicated patch:

crashes earlier:

BUG: Bad page map in process swapper  pte:001f88e9 pmd:020d8001
addr:7ffffff6 vm_flags:00100177 anon_vma:c20d95e0 mapping:(null) index:7ffff
Call Trace:
[c341fd20] [c00067d0] show_stack+0x44/0x14c (unreliable)
[c341fd60] [c0056860] print_bad_pte+0x1b4/0x1d0
[c341fd90] [c00568c8] vm_normal_page+0x4c/0x78
[c341fda0] [c0056988] follow_page+0x94/0x1b8
[c341fdc0] [c005854c] __get_user_pages+0x324/0x3c4
[c341fe20] [c006c12c] get_arg_page+0x40/0xac
[c341fe40] [c006c374] copy_strings+0xec/0x1d4
[c341fe80] [c006c47c] copy_strings_kernel+0x20/0x38
[c341fea0] [c006d3cc] do_execve+0x110/0x240
[c341fee0] [c00070c4] sys_execve+0x50/0x7c
[c341ff00] [c000d0dc] ret_from_syscall+0x0/0x38
[c341ffc0] [c00023ac] init_post+0x5c/0x168
[c341ffe0] [c01df22c] kernel_init+0x10c/0x124
[c341fff0] [c000cf3c] kernel_thread+0x4c/0x68

this is running on a MPC 875, and I'm happy to test any patches or
add any debugging code if it helps.

thanks!
/rex

Re: [PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[Benjamin Herrenschmidt]

On Wed, 2009-09-23 at 17:53 -0700, Rex Feany wrote:
> this is running on a MPC 875, and I'm happy to test any patches or
> add any debugging code if it helps.

Hrm... smells like something screwy in the 8xx PTE handling.

Let me have another look.

Cheers,
Ben.

Re: [PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[Benjamin Herrenschmidt]

On Wed, 2009-09-23 at 17:53 -0700, Rex Feany wrote:
> Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):
> 
> > Bernhard, Rex, please let me know if that works for you.
> 
> it doesn't work for me, it crashes differently then before though!

Hrm. This is really strange...

> trunk+benh complicated patch:
> 
> crashes earlier:
> 
> BUG: Bad page map in process swapper  pte:001f88e9 pmd:020d8001

It's complaining because for some reason a PTE ends up with
_PAGE_SPECIAL set which indeed shouldn't be the case for a stack
page which is what is being accessed in our case. However, I have
no idea how that bit ended up being set. The generic code shouldn't
be setting it, and I looked several times at the TLB miss handling
on 8xx and can't find how in hell it would inadvertently set that
bit in the PTE...

Scott, I don't have any 8xx HW, any chance you can have a look at
what's going on here ?

The other bits in there look fine. So something must be whacking
_PAGE_SPECIAL in somewhere or at least whacking 0x8 in there.

Also, _PAGE_HWWRITE isn't set, but the DataTLBError code will
set it when setting dirty, so the page must have been created
pre-dirtied or something... all very weird I must say.

> addr:7ffffff6 vm_flags:00100177 anon_vma:c20d95e0 mapping:(null) index:7ffff
> Call Trace:
> [c341fd20] [c00067d0] show_stack+0x44/0x14c (unreliable)
> [c341fd60] [c0056860] print_bad_pte+0x1b4/0x1d0
> [c341fd90] [c00568c8] vm_normal_page+0x4c/0x78
> [c341fda0] [c0056988] follow_page+0x94/0x1b8
> [c341fdc0] [c005854c] __get_user_pages+0x324/0x3c4
> [c341fe20] [c006c12c] get_arg_page+0x40/0xac
> [c341fe40] [c006c374] copy_strings+0xec/0x1d4
> [c341fe80] [c006c47c] copy_strings_kernel+0x20/0x38
> [c341fea0] [c006d3cc] do_execve+0x110/0x240
> [c341fee0] [c00070c4] sys_execve+0x50/0x7c
> [c341ff00] [c000d0dc] ret_from_syscall+0x0/0x38
> [c341ffc0] [c00023ac] init_post+0x5c/0x168
> [c341ffe0] [c01df22c] kernel_init+0x10c/0x124
> [c341fff0] [c000cf3c] kernel_thread+0x4c/0x68
> 
> this is running on a MPC 875, and I'm happy to test any patches or
> add any debugging code if it helps.
> 
> thanks!
> /rex

Re: [PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[Rex Feany]

Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):

> Hrm. This is really strange...

Is this a problem? in pte-8xx.h you added:

+#define _PAGE_SPECIAL  0x0008  /* SW entry, forced to 0 by the TLB miss */

but there is already this:

#define _PAGE_EXEC      0x0008  /* software: i-cache coherency required */

take care!
/rex.

Re: [PATCH] powerpc/mm: Fix 40x and 8xx vs. _PAGE_SPECIAL

[Benjamin Herrenschmidt]

On Wed, 2009-09-23 at 19:38 -0700, Rex Feany wrote:
> Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):
> 
> > Hrm. This is really strange...
> 
> Is this a problem? in pte-8xx.h you added:
> 
> +#define _PAGE_SPECIAL  0x0008  /* SW entry, forced to 0 by the TLB miss */
> 
> but there is already this:
> 
> #define _PAGE_EXEC      0x0008  /* software: i-cache coherency required */

Ohhh... you aren't using upstream ! That explains :-)

I removed _PAGE_EXEC from 8xx (since it doesn't use it for anything)
recently.

I think the best option is to go for Bernhard's patch for "stable" and
my patch for upstream, so that "stable" gets the bug fix without the
feature of _PAGE_SPECIAL and upstream gets it both.

Cheers,
Ben.