[PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[Kumar Gala]

Move to using PAGE_OFFSET instead of TASK_SIZE or KERNELBASE value on
6xx/40x/44x/fsl-booke to determine if the faulting address is a kernel or
user space address.  This mimics how the macro is_kernel_addr() works.

---

I've tested this on fsl_booke & 603 systems.

 arch/powerpc/kernel/head_32.S        |   18 +++++++++---------
 arch/powerpc/kernel/head_40x.S       |    6 +++---
 arch/powerpc/kernel/head_44x.S       |    6 +++---
 arch/powerpc/kernel/head_fsl_booke.S |   11 ++++-------
 4 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index d83f04e..a5b13ae 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -469,12 +469,12 @@ InstructionTLBMiss:
 	mfctr	r0
 	/* Get PTE (linux-style) and check access */
 	mfspr	r3,SPRN_IMISS
-	lis	r1,KERNELBASE@h		/* check if kernel address */
-	cmplw	0,r3,r1
+	lis	r1,PAGE_OFFSET@h		/* check if kernel address */
+	cmplw	0,r1,r3
 	mfspr	r2,SPRN_SPRG3
 	li	r1,_PAGE_USER|_PAGE_PRESENT /* low addresses tested as user */
 	lwz	r2,PGDIR(r2)
-	blt+	112f
+	bge-	112f
 	mfspr	r2,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
 	rlwimi	r1,r2,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 	lis	r2,swapper_pg_dir@ha	/* if kernel address, use */
@@ -543,12 +543,12 @@ DataLoadTLBMiss:
 	mfctr	r0
 	/* Get PTE (linux-style) and check access */
 	mfspr	r3,SPRN_DMISS
-	lis	r1,KERNELBASE@h		/* check if kernel address */
-	cmplw	0,r3,r1
+	lis	r1,PAGE_OFFSET@h		/* check if kernel address */
+	cmplw	0,r1,r3
 	mfspr	r2,SPRN_SPRG3
 	li	r1,_PAGE_USER|_PAGE_PRESENT /* low addresses tested as user */
 	lwz	r2,PGDIR(r2)
-	blt+	112f
+	bge-	112f
 	mfspr	r2,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
 	rlwimi	r1,r2,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 	lis	r2,swapper_pg_dir@ha	/* if kernel address, use */
@@ -615,12 +615,12 @@ DataStoreTLBMiss:
 	mfctr	r0
 	/* Get PTE (linux-style) and check access */
 	mfspr	r3,SPRN_DMISS
-	lis	r1,KERNELBASE@h		/* check if kernel address */
-	cmplw	0,r3,r1
+	lis	r1,PAGE_OFFSET@h		/* check if kernel address */
+	cmplw	0,r1,r3
 	mfspr	r2,SPRN_SPRG3
 	li	r1,_PAGE_RW|_PAGE_USER|_PAGE_PRESENT /* access flags */
 	lwz	r2,PGDIR(r2)
-	blt+	112f
+	bge-	112f
 	mfspr	r2,SPRN_SRR1		/* and MSR_PR bit from SRR1 */
 	rlwimi	r1,r2,32-12,29,29	/* shift MSR_PR to _PAGE_USER posn */
 	lis	r2,swapper_pg_dir@ha	/* if kernel address, use */
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index e312824..cfefc2d 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -289,7 +289,7 @@ label:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
+	lis	r11, PAGE_OFFSET@h
 	cmplw	r10, r11
 	blt+	3f
 	lis	r11, swapper_pg_dir@h
@@ -481,7 +481,7 @@ label:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
+	lis	r11, PAGE_OFFSET@h
 	cmplw	r10, r11
 	blt+	3f
 	lis	r11, swapper_pg_dir@h
@@ -581,7 +581,7 @@ label:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
+	lis	r11, PAGE_OFFSET@h
 	cmplw	r10, r11
 	blt+	3f
 	lis	r11, swapper_pg_dir@h
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index 864d63f..409db61 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -319,7 +319,7 @@ interrupt_base:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
+	lis	r11, PAGE_OFFSET@h
 	cmplw	r10, r11
 	blt+	3f
 	lis	r11, swapper_pg_dir@h
@@ -458,7 +458,7 @@ interrupt_base:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
+	lis	r11, PAGE_OFFSET@h
 	cmplw	r10, r11
 	blt+	3f
 	lis	r11, swapper_pg_dir@h
@@ -528,7 +528,7 @@ interrupt_base:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
+	lis	r11, PAGE_OFFSET@h
 	cmplw	r10, r11
 	blt+	3f
 	lis	r11, swapper_pg_dir@h
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index ee33ddd..4b98227 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -461,8 +461,7 @@ interrupt_base:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
-	ori	r11, r11, TASK_SIZE@l
+	lis	r11, PAGE_OFFSET@h
 	cmplw	0, r10, r11
 	bge	2f

@@ -584,8 +583,7 @@ interrupt_base:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
-	ori	r11, r11, TASK_SIZE@l
+	lis	r11, PAGE_OFFSET@h
 	cmplw	5, r10, r11
 	blt	5, 3f
 	lis	r11, swapper_pg_dir@h
@@ -645,8 +643,7 @@ interrupt_base:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
-	lis	r11, TASK_SIZE@h
-	ori	r11, r11, TASK_SIZE@l
+	lis	r11, PAGE_OFFSET@h
 	cmplw	5, r10, r11
 	blt	5, 3f
 	lis	r11, swapper_pg_dir@h
@@ -744,7 +741,7 @@ data_access:
  *	r10 - EA of fault
  *	r11 - TLB (info from Linux PTE)
  *	r12, r13 - available to use
- *	CR5 - results of addr < TASK_SIZE
+ *	CR5 - results of addr >= PAGE_OFFSET
  *	MAS0, MAS1 - loaded with proper value when we get here
  *	MAS2, MAS3 - will need additional info from Linux PTE
  *	Upon exit, we reload everything and RFI.
-- 
1.5.2.4

Re: [PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[David Gibson]

On Thu, Oct 11, 2007 at 01:42:30PM -0500, Kumar Gala wrote:
> Move to using PAGE_OFFSET instead of TASK_SIZE or KERNELBASE value on
> 6xx/40x/44x/fsl-booke to determine if the faulting address is a kernel or
> user space address.  This mimics how the macro is_kernel_addr()
> works.

Actually it's ambiguous whether TASK_SIZE or PAGE_OFFSET is correct in
most of these cases (KERNELBASE is certainly wrong, though).

TASK_SIZE is the top of the userspace mapped area, PAGE_OFFSET is the
bottom of the linear mapping.  So, strictly speaking there are 3 paths
for the miss handlers: < TASK_SIZE => user mapping, >= PAGE_OFFSET =>
kernel mapping, between the two => immediate fault.

We get away with a two way comparison on 32-bit because, a) they have
the same value and b) none of the pagetables, user or kernel, should
have any entries in the in between region so we'll end up in
do_page_fault in the end, anyway.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[Josh Boyer]

On Fri, 2007-10-12 at 13:30 +1000, David Gibson wrote:
> On Thu, Oct 11, 2007 at 01:42:30PM -0500, Kumar Gala wrote:
> > Move to using PAGE_OFFSET instead of TASK_SIZE or KERNELBASE value on
> > 6xx/40x/44x/fsl-booke to determine if the faulting address is a kernel or
> > user space address.  This mimics how the macro is_kernel_addr()
> > works.
> 
> Actually it's ambiguous whether TASK_SIZE or PAGE_OFFSET is correct in
> most of these cases (KERNELBASE is certainly wrong, though).
> 
> TASK_SIZE is the top of the userspace mapped area, PAGE_OFFSET is the
> bottom of the linear mapping.  So, strictly speaking there are 3 paths
> for the miss handlers: < TASK_SIZE => user mapping, >= PAGE_OFFSET =>
> kernel mapping, between the two => immediate fault.
> 
> We get away with a two way comparison on 32-bit because, a) they have
> the same value and b) none of the pagetables, user or kernel, should
> have any entries in the in between region so we'll end up in
> do_page_fault in the end, anyway.

Kumar's other patch removes the gap.  He changed the default
CONFIG_TASK_SIZE to 0xc0000000.

josh

Re: [PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[David Gibson]

On Fri, Oct 12, 2007 at 06:56:24AM -0500, Josh Boyer wrote:
> On Fri, 2007-10-12 at 13:30 +1000, David Gibson wrote:
> > On Thu, Oct 11, 2007 at 01:42:30PM -0500, Kumar Gala wrote:
> > > Move to using PAGE_OFFSET instead of TASK_SIZE or KERNELBASE value on
> > > 6xx/40x/44x/fsl-booke to determine if the faulting address is a kernel or
> > > user space address.  This mimics how the macro is_kernel_addr()
> > > works.
> > 
> > Actually it's ambiguous whether TASK_SIZE or PAGE_OFFSET is correct in
> > most of these cases (KERNELBASE is certainly wrong, though).
> > 
> > TASK_SIZE is the top of the userspace mapped area, PAGE_OFFSET is the
> > bottom of the linear mapping.  So, strictly speaking there are 3 paths
> > for the miss handlers: < TASK_SIZE => user mapping, >= PAGE_OFFSET =>
> > kernel mapping, between the two => immediate fault.
> > 
> > We get away with a two way comparison on 32-bit because, a) they have
> > the same value and b) none of the pagetables, user or kernel, should
> > have any entries in the in between region so we'll end up in
> > do_page_fault in the end, anyway.
> 
> Kumar's other patch removes the gap.  He changed the default
> CONFIG_TASK_SIZE to 0xc0000000.

That's (a) and only removes the gap in the default configuration..

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[Josh Boyer]

On Mon, 2007-10-15 at 10:54 +1000, David Gibson wrote:
> On Fri, Oct 12, 2007 at 06:56:24AM -0500, Josh Boyer wrote:
> > On Fri, 2007-10-12 at 13:30 +1000, David Gibson wrote:
> > > On Thu, Oct 11, 2007 at 01:42:30PM -0500, Kumar Gala wrote:
> > > > Move to using PAGE_OFFSET instead of TASK_SIZE or KERNELBASE value on
> > > > 6xx/40x/44x/fsl-booke to determine if the faulting address is a kernel or
> > > > user space address.  This mimics how the macro is_kernel_addr()
> > > > works.
> > > 
> > > Actually it's ambiguous whether TASK_SIZE or PAGE_OFFSET is correct in
> > > most of these cases (KERNELBASE is certainly wrong, though).
> > > 
> > > TASK_SIZE is the top of the userspace mapped area, PAGE_OFFSET is the
> > > bottom of the linear mapping.  So, strictly speaking there are 3 paths
> > > for the miss handlers: < TASK_SIZE => user mapping, >= PAGE_OFFSET =>
> > > kernel mapping, between the two => immediate fault.
> > > 
> > > We get away with a two way comparison on 32-bit because, a) they have
> > > the same value and b) none of the pagetables, user or kernel, should
> > > have any entries in the in between region so we'll end up in
> > > do_page_fault in the end, anyway.
> > 
> > Kumar's other patch removes the gap.  He changed the default
> > CONFIG_TASK_SIZE to 0xc0000000.
> 
> That's (a) and only removes the gap in the default configuration..

I believe the idea was that as defconfigs get updated for 2.6.24, they
would pick up the new default.

josh

Re: [PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[David Gibson]

On Sun, Oct 14, 2007 at 08:01:19PM -0500, Josh Boyer wrote:
> On Mon, 2007-10-15 at 10:54 +1000, David Gibson wrote:
> > On Fri, Oct 12, 2007 at 06:56:24AM -0500, Josh Boyer wrote:
> > > On Fri, 2007-10-12 at 13:30 +1000, David Gibson wrote:
> > > > On Thu, Oct 11, 2007 at 01:42:30PM -0500, Kumar Gala wrote:
> > > > > Move to using PAGE_OFFSET instead of TASK_SIZE or KERNELBASE value on
> > > > > 6xx/40x/44x/fsl-booke to determine if the faulting address is a kernel or
> > > > > user space address.  This mimics how the macro is_kernel_addr()
> > > > > works.
> > > > 
> > > > Actually it's ambiguous whether TASK_SIZE or PAGE_OFFSET is correct in
> > > > most of these cases (KERNELBASE is certainly wrong, though).
> > > > 
> > > > TASK_SIZE is the top of the userspace mapped area, PAGE_OFFSET is the
> > > > bottom of the linear mapping.  So, strictly speaking there are 3 paths
> > > > for the miss handlers: < TASK_SIZE => user mapping, >= PAGE_OFFSET =>
> > > > kernel mapping, between the two => immediate fault.
> > > > 
> > > > We get away with a two way comparison on 32-bit because, a) they have
> > > > the same value and b) none of the pagetables, user or kernel, should
> > > > have any entries in the in between region so we'll end up in
> > > > do_page_fault in the end, anyway.
> > > 
> > > Kumar's other patch removes the gap.  He changed the default
> > > CONFIG_TASK_SIZE to 0xc0000000.
> > 
> > That's (a) and only removes the gap in the default configuration..
> 
> I believe the idea was that as defconfigs get updated for 2.6.24, they
> would pick up the new default.

Yes, but if someone overrides CONFIG_TASK_SIZE, it should still work,
yes?  Or else this should not be a CONFIG option at all.  Which it
will, of course, because of (b), but one should still be aware of the
theoretical 3-way branch when touching this code, even if it can be
reduced to  2-way branch in practice.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] [POWERPC] Use PAGE_OFFSET to tell if an address is user/kernel in SW TLB handlers

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 2438 bytes --]

On Mon, 2007-10-15 at 11:45 +1000, David Gibson wrote:
> On Sun, Oct 14, 2007 at 08:01:19PM -0500, Josh Boyer wrote:
> > On Mon, 2007-10-15 at 10:54 +1000, David Gibson wrote:
> > > On Fri, Oct 12, 2007 at 06:56:24AM -0500, Josh Boyer wrote:
> > > > On Fri, 2007-10-12 at 13:30 +1000, David Gibson wrote:
> > > > > On Thu, Oct 11, 2007 at 01:42:30PM -0500, Kumar Gala wrote:
> > > > > > Move to using PAGE_OFFSET instead of TASK_SIZE or KERNELBASE value on
> > > > > > 6xx/40x/44x/fsl-booke to determine if the faulting address is a kernel or
> > > > > > user space address.  This mimics how the macro is_kernel_addr()
> > > > > > works.
> > > > > 
> > > > > Actually it's ambiguous whether TASK_SIZE or PAGE_OFFSET is correct in
> > > > > most of these cases (KERNELBASE is certainly wrong, though).
> > > > > 
> > > > > TASK_SIZE is the top of the userspace mapped area, PAGE_OFFSET is the
> > > > > bottom of the linear mapping.  So, strictly speaking there are 3 paths
> > > > > for the miss handlers: < TASK_SIZE => user mapping, >= PAGE_OFFSET =>
> > > > > kernel mapping, between the two => immediate fault.
> > > > > 
> > > > > We get away with a two way comparison on 32-bit because, a) they have
> > > > > the same value and b) none of the pagetables, user or kernel, should
> > > > > have any entries in the in between region so we'll end up in
> > > > > do_page_fault in the end, anyway.
> > > > 
> > > > Kumar's other patch removes the gap.  He changed the default
> > > > CONFIG_TASK_SIZE to 0xc0000000.
> > > 
> > > That's (a) and only removes the gap in the default configuration..
> > 
> > I believe the idea was that as defconfigs get updated for 2.6.24, they
> > would pick up the new default.
> 
> Yes, but if someone overrides CONFIG_TASK_SIZE, it should still work,
> yes?  Or else this should not be a CONFIG option at all.  Which it
> will, of course, because of (b), but one should still be aware of the
> theoretical 3-way branch when touching this code, even if it can be
> reduced to  2-way branch in practice.

There's a construct involving the characters "/" and "*" which I believe
can solve this problem.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]