[PATCH 0/4] 8xx: Optimize TLB Miss code.

[PATCH 0/4] 8xx: Optimize TLB Miss code.

[Joakim Tjernlund]

This set of tries to optimize the TLB code on 8xx even
more. If they work, it should be a noticable performance
boost.

I would be very happy if you could test them for me.

Joakim Tjernlund (4):
  8xx: Optimze TLB Miss handlers
  8xx: Avoid testing for kernel space in ITLB Miss.
  8xx: Don't touch ACCESSED when no SWAP.
  8xx: Use SPRG2 and DAR registers to stash r11 and cr.

 arch/powerpc/kernel/head_8xx.S |   70 +++++++++++++++++++++++++++-------------
 1 files changed, 47 insertions(+), 23 deletions(-)

[PATCH 1/4] 8xx: Optimze TLB Miss handlers

[Joakim Tjernlund]

This removes a couple of insn's from the TLB Miss
handlers whithout changing functionality.
---
 arch/powerpc/kernel/head_8xx.S |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 3ef743f..ecc4a02 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -343,17 +343,14 @@ InstructionTLBMiss:
 	cmpwi	cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
 	bne-	cr0, 2f
 
-	/* Clear PP lsb, 0x400 */
-	rlwinm 	r10, r10, 0, 22, 20
-
 	/* The Linux PTE won't go exactly into the MMU TLB.
-	 * Software indicator bits 22 and 28 must be clear.
+	 * Software indicator bits 21 and 28 must be clear.
 	 * Software indicator bits 24, 25, 26, and 27 must be
 	 * set.  All other Linux PTE bits control the behavior
 	 * of the MMU.
 	 */
 	li	r11, 0x00f0
-	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
+	rlwimi	r10, r11, 0, 0x07f8	/* Set 24-27, clear 21-23,28 */
 	DO_8xx_CPU6(0x2d80, r3)
 	mtspr	SPRN_MI_RPN, r10	/* Update TLB entry */
 
@@ -444,9 +441,7 @@ DataStoreTLBMiss:
 
 	/* Honour kernel RO, User NA */
 	/* 0x200 == Extended encoding, bit 22 */
-	/* r11 =  (r10 & _PAGE_USER) >> 2 */
-	rlwinm	r11, r10, 32-2, 0x200
-	or	r10, r11, r10
+	rlwimi	r10, r10, 32-2, 0x200 /* Copy USER to bit 22, 0x200 */
 	/* r11 =  (r10 & _PAGE_RW) >> 1 */
 	rlwinm	r11, r10, 32-1, 0x200
 	or	r10, r11, r10
-- 
1.6.4.4

[PATCH 2/4] 8xx: Avoid testing for kernel space in ITLB Miss.

[Joakim Tjernlund]

Only modules will cause ITLB Misses as we always pin
the first 8MB of kernel memory.
---
 arch/powerpc/kernel/head_8xx.S |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index ecc4a02..84ca1d9 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -318,12 +318,16 @@ InstructionTLBMiss:
 	/* If we are faulting a kernel address, we have to use the
 	 * kernel page tables.
 	 */
+#ifdef CONFIG_MODULES
+	/* Only modules will cause ITLB Misses as we always
+	 * pin the first 8MB of kernel memory */
 	andi.	r11, r10, 0x0800	/* Address >= 0x80000000 */
 	beq	3f
 	lis	r11, swapper_pg_dir@h
 	ori	r11, r11, swapper_pg_dir@l
 	rlwimi	r10, r11, 0, 2, 19
 3:
+#endif
 	lwz	r11, 0(r10)	/* Get the level 1 entry */
 	rlwinm.	r10, r11,0,0,19	/* Extract page descriptor page address */
 	beq	2f		/* If zero, don't try to find a pte */
-- 
1.6.4.4

[PATCH 3/4] 8xx: Don't touch ACCESSED when no SWAP.

[Joakim Tjernlund]

Only the swap function cares about the ACCESSED bit in
the pte. Do not waste cycles updateting ACCESSED when swap
is not compiled into the kernel.
---
 arch/powerpc/kernel/head_8xx.S |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 84ca1d9..6478a96 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -343,10 +343,11 @@ InstructionTLBMiss:
 	mfspr	r11, SPRN_MD_TWC	/* ....and get the pte address */
 	lwz	r10, 0(r11)	/* Get the pte */
 
+#ifdef CONFIG_SWAP
 	andi.	r11, r10, _PAGE_ACCESSED | _PAGE_PRESENT
 	cmpwi	cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
 	bne-	cr0, 2f
-
+#endif
 	/* The Linux PTE won't go exactly into the MMU TLB.
 	 * Software indicator bits 21 and 28 must be clear.
 	 * Software indicator bits 24, 25, 26, and 27 must be
@@ -439,10 +440,11 @@ DataStoreTLBMiss:
 	 * r11 = ((r10 & PRESENT) & ((r10 & ACCESSED) >> 5));
 	 * r10 = (r10 & ~PRESENT) | r11;
 	 */
+#ifdef CONFIG_SWAP
 	rlwinm	r11, r10, 32-5, _PAGE_PRESENT
 	and	r11, r11, r10
 	rlwimi	r10, r11, 0, _PAGE_PRESENT
-
+#endif
 	/* Honour kernel RO, User NA */
 	/* 0x200 == Extended encoding, bit 22 */
 	rlwimi	r10, r10, 32-2, 0x200 /* Copy USER to bit 22, 0x200 */
-- 
1.6.4.4

[PATCH 4/4] 8xx: Use SPRG2 and DAR registers to stash r11 and cr.

[Joakim Tjernlund]

This avoids storing these registers in memory.
CPU6 errata will still use the old way.
Remove some G2 leftover accesses from 2.4
---
 arch/powerpc/kernel/head_8xx.S |   49 +++++++++++++++++++++++++++++----------
 1 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 6478a96..1f1a04b 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -71,9 +71,6 @@ _ENTRY(_start);
  * in the first level table, but that would require many changes to the
  * Linux page directory/table functions that I don't want to do right now.
  *
- * I used to use SPRG2 for a temporary register in the TLB handler, but it
- * has since been put to other uses.  I now use a hack to save a register
- * and the CCR at memory location 0.....Someday I'll fix this.....
  *	-- Dan
  */
 	.globl	__start
@@ -302,8 +299,13 @@ InstructionTLBMiss:
 	DO_8xx_CPU6(0x3f80, r3)
 	mtspr	SPRN_M_TW, r10	/* Save a couple of working registers */
 	mfcr	r10
+#ifdef CONFIG_8xx_CPU6
 	stw	r10, 0(r0)
 	stw	r11, 4(r0)
+#else
+	mtspr	SPRN_DAR, r10
+	mtspr	SPRN_SPRG2, r11
+#endif
 	mfspr	r10, SPRN_SRR0	/* Get effective address of fault */
 #ifdef CONFIG_8xx_CPU15
 	addi	r11, r10, 0x1000
@@ -359,13 +361,19 @@ InstructionTLBMiss:
 	DO_8xx_CPU6(0x2d80, r3)
 	mtspr	SPRN_MI_RPN, r10	/* Update TLB entry */
 
-	mfspr	r10, SPRN_M_TW	/* Restore registers */
+	/* Restore registers */
+#ifndef CONFIG_8xx_CPU6
+	mfspr	r10, SPRN_DAR
+	mtcr	r10
+	mtspr	SPRN_DAR, r11	/* Tag DAR */
+	mfspr	r11, SPRN_SPRG2
+#else
 	lwz	r11, 0(r0)
 	mtcr	r11
 	lwz	r11, 4(r0)
-#ifdef CONFIG_8xx_CPU6
 	lwz	r3, 8(r0)
 #endif
+	mfspr	r10, SPRN_M_TW
 	rfi
 2:
 	mfspr	r11, SPRN_SRR1
@@ -375,13 +383,20 @@ InstructionTLBMiss:
 	rlwinm	r11, r11, 0, 0xffff
 	mtspr	SPRN_SRR1, r11
 
-	mfspr	r10, SPRN_M_TW	/* Restore registers */
+	/* Restore registers */
+#ifndef CONFIG_8xx_CPU6
+	mfspr	r10, SPRN_DAR
+	mtcr	r10
+	li	r11, 0x00f0
+	mtspr	SPRN_DAR, r11	/* Tag DAR */
+	mfspr	r11, SPRN_SPRG2
+#else
 	lwz	r11, 0(r0)
 	mtcr	r11
 	lwz	r11, 4(r0)
-#ifdef CONFIG_8xx_CPU6
 	lwz	r3, 8(r0)
 #endif
+	mfspr	r10, SPRN_M_TW
 	b	InstructionAccess
 
 	. = 0x1200
@@ -392,8 +407,13 @@ DataStoreTLBMiss:
 	DO_8xx_CPU6(0x3f80, r3)
 	mtspr	SPRN_M_TW, r10	/* Save a couple of working registers */
 	mfcr	r10
+#ifdef CONFIG_8xx_CPU6
 	stw	r10, 0(r0)
 	stw	r11, 4(r0)
+#else
+	mtspr	SPRN_DAR, r10
+	mtspr	SPRN_SPRG2, r11
+#endif
 	mfspr	r10, SPRN_M_TWB	/* Get level 1 table entry address */
 
 	/* If we are faulting a kernel address, we have to use the
@@ -461,18 +481,24 @@ DataStoreTLBMiss:
 	 * of the MMU.
 	 */
 2:	li	r11, 0x00f0
-	mtspr	SPRN_DAR,r11	/* Tag DAR */
 	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
 	DO_8xx_CPU6(0x3d80, r3)
 	mtspr	SPRN_MD_RPN, r10	/* Update TLB entry */
 
-	mfspr	r10, SPRN_M_TW	/* Restore registers */
+	/* Restore registers */
+#ifndef CONFIG_8xx_CPU6
+	mfspr	r10, SPRN_DAR
+	mtcr	r10
+	mtspr	SPRN_DAR, r11	/* Tag DAR */
+	mfspr	r11, SPRN_SPRG2
+#else
+	mtspr	SPRN_DAR, r11	/* Tag DAR */
 	lwz	r11, 0(r0)
 	mtcr	r11
 	lwz	r11, 4(r0)
-#ifdef CONFIG_8xx_CPU6
 	lwz	r3, 8(r0)
 #endif
+	mfspr	r10, SPRN_M_TW
 	rfi
 
 /* This is an instruction TLB error on the MPC8xx.  This could be due
@@ -684,9 +710,6 @@ start_here:
 	tophys(r4,r2)
 	addi	r4,r4,THREAD	/* init task's THREAD */
 	mtspr	SPRN_SPRG_THREAD,r4
-	li	r3,0
-	/* XXX What is that for ? SPRG2 appears otherwise unused on 8xx */
-	mtspr	SPRN_SPRG2,r3	/* 0 => r1 has kernel sp */
 
 	/* stack */
 	lis	r1,init_thread_union@ha
-- 
1.6.4.4

Re: [PATCH 1/4] 8xx: Optimze TLB Miss handlers

[Scott Wood]

On Fri, Feb 26, 2010 at 09:29:40AM +0100, Joakim Tjernlund wrote:
> This removes a couple of insn's from the TLB Miss
> handlers whithout changing functionality.
> ---

Did a quick test of the patchset, seems to work OK (without CONFIG_SWAP or
CONFIG_MODULES).  Didn't try with CONFIG_8xx_CPU6.

-Scott

Re: [PATCH 1/4] 8xx: Optimze TLB Miss handlers

[Kumar Gala]

On Feb 26, 2010, at 2:29 AM, Joakim Tjernlund wrote:

> 	li	r11, 0x00f0
> -	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
> +	rlwimi	r10, r11, 0, 0x07f8	/* Set 24-27, clear 21-23,28 */
> 	DO_8xx_CPU6(0x2d80, r3)
> 	mtspr	SPRN_MI_RPN, r10	/* Update TLB entry */

Cool, didn't know 'as' supported this notation.

- k

Re: [PATCH 1/4] 8xx: Optimze TLB Miss handlers

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2010/02/26 20:50:18:
>
> On Fri, Feb 26, 2010 at 09:29:40AM +0100, Joakim Tjernlund wrote:
> > This removes a couple of insn's from the TLB Miss
> > handlers whithout changing functionality.
> > ---
>
> Did a quick test of the patchset, seems to work OK (without CONFIG_SWAP or
> CONFIG_MODULES).  Didn't try with CONFIG_8xx_CPU6.

Cool, thanks a lot!

Not sure anyone is using 2.6 with CPU6 errata. Seems it was fixed years ago.

Should I resend the whole series with SOB line or just include it here?

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

Re: [PATCH 1/4] 8xx: Optimze TLB Miss handlers

[Joakim Tjernlund]

Kumar Gala <galak@kernel.crashing.org> wrote on 2010/02/26 21:10:31:
>
>
> On Feb 26, 2010, at 2:29 AM, Joakim Tjernlund wrote:
>
> >    li   r11, 0x00f0
> > -   rlwimi   r10, r11, 0, 24, 28   /* Set 24-27, clear 28 */
> > +   rlwimi   r10, r11, 0, 0x07f8   /* Set 24-27, clear 21-23,28 */
> >    DO_8xx_CPU6(0x2d80, r3)
> >    mtspr   SPRN_MI_RPN, r10   /* Update TLB entry */
>
> Cool, didn't know 'as' supported this notation.

Yeah, it was Scott who gave me the clue and from what I can tell it
is an official syntax form. I find much easier to understand.

 Jocke

Re: [PATCH 2/4] 8xx: Avoid testing for kernel space in ITLB Miss.

[Benjamin Herrenschmidt]

On Fri, 2010-02-26 at 09:29 +0100, Joakim Tjernlund wrote:
> +#ifdef CONFIG_MODULES
> +       /* Only modules will cause ITLB Misses as we always
> +        * pin the first 8MB of kernel memory */
>         andi.   r11, r10, 0x0800        /* Address >= 0x80000000 */
>         beq     3f
>         lis     r11, swapper_pg_dir@h
>         ori     r11, r11, swapper_pg_dir@l
>         rlwimi  r10, r11, 0, 2, 19
>  3:
> +#endif

You can optimize that further I think...

You can probably just remove the code above, and add something to
do_page_fault() that lazily copies the kernel PGD entries from
swapper_pg_dir to the app pgdir. (You can even pre-fill that when
creating a new mm).

Cheers,
Ben.

Re: [PATCH 3/4] 8xx: Don't touch ACCESSED when no SWAP.

[Benjamin Herrenschmidt]

On Fri, 2010-02-26 at 09:29 +0100, Joakim Tjernlund wrote:
> Only the swap function cares about the ACCESSED bit in
> the pte. Do not waste cycles updateting ACCESSED when swap
> is not compiled into the kernel.
> ---

Your changeset comment is a bit misleading since the code isn't actually
updating ACCESSED... it's testing if ACCESSED is set and goes to the
higher level fault if not (which might then update ACCESSED).

Cheers,
Ben.

>  arch/powerpc/kernel/head_8xx.S |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index 84ca1d9..6478a96 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -343,10 +343,11 @@ InstructionTLBMiss:
>  	mfspr	r11, SPRN_MD_TWC	/* ....and get the pte address */
>  	lwz	r10, 0(r11)	/* Get the pte */
>  
> +#ifdef CONFIG_SWAP
>  	andi.	r11, r10, _PAGE_ACCESSED | _PAGE_PRESENT
>  	cmpwi	cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
>  	bne-	cr0, 2f
> -
> +#endif
>  	/* The Linux PTE won't go exactly into the MMU TLB.
>  	 * Software indicator bits 21 and 28 must be clear.
>  	 * Software indicator bits 24, 25, 26, and 27 must be
> @@ -439,10 +440,11 @@ DataStoreTLBMiss:
>  	 * r11 = ((r10 & PRESENT) & ((r10 & ACCESSED) >> 5));
>  	 * r10 = (r10 & ~PRESENT) | r11;
>  	 */
> +#ifdef CONFIG_SWAP
>  	rlwinm	r11, r10, 32-5, _PAGE_PRESENT
>  	and	r11, r11, r10
>  	rlwimi	r10, r11, 0, _PAGE_PRESENT
> -
> +#endif
>  	/* Honour kernel RO, User NA */
>  	/* 0x200 == Extended encoding, bit 22 */
>  	rlwimi	r10, r10, 32-2, 0x200 /* Copy USER to bit 22, 0x200 */

Re: [PATCH 2/4] 8xx: Avoid testing for kernel space in ITLB Miss.

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2010/03/16 22:19:36:
>
> On Fri, 2010-02-26 at 09:29 +0100, Joakim Tjernlund wrote:
> > +#ifdef CONFIG_MODULES
> > +       /* Only modules will cause ITLB Misses as we always
> > +        * pin the first 8MB of kernel memory */
> >         andi.   r11, r10, 0x0800        /* Address >= 0x80000000 */
> >         beq     3f
> >         lis     r11, swapper_pg_dir@h
> >         ori     r11, r11, swapper_pg_dir@l
> >         rlwimi  r10, r11, 0, 2, 19
> >  3:
> > +#endif
>
> You can optimize that further I think...
>
> You can probably just remove the code above, and add something to
> do_page_fault() that lazily copies the kernel PGD entries from
> swapper_pg_dir to the app pgdir. (You can even pre-fill that when
> creating a new mm).

I did look at this at some point and could not figure out how
to do this.

Re: [PATCH 3/4] 8xx: Don't touch ACCESSED when no SWAP.

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 2010/03/16 22:20:52:
>
> On Fri, 2010-02-26 at 09:29 +0100, Joakim Tjernlund wrote:
> > Only the swap function cares about the ACCESSED bit in
> > the pte. Do not waste cycles updateting ACCESSED when swap
> > is not compiled into the kernel.
> > ---
>
> Your changeset comment is a bit misleading since the code isn't actually
> updating ACCESSED... it's testing if ACCESSED is set and goes to the
> higher level fault if not (which might then update ACCESSED).

Right, I did have one or two variants that did update ACCESSED that
I experimented with, I guess that I got a bit confused by that.

The jury is still out on whether this patch is an improvement or not.

    Jocke

[PATCH 1/4] 8xx: Optimze TLB Miss handlers

[Joakim Tjernlund]

This removes a couple of insn's from the TLB Miss
handlers whithout changing functionality.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/kernel/head_8xx.S |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 3ef743f..ecc4a02 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -343,17 +343,14 @@ InstructionTLBMiss:
 	cmpwi	cr0, r11, _PAGE_ACCESSED | _PAGE_PRESENT
 	bne-	cr0, 2f
 
-	/* Clear PP lsb, 0x400 */
-	rlwinm 	r10, r10, 0, 22, 20
-
 	/* The Linux PTE won't go exactly into the MMU TLB.
-	 * Software indicator bits 22 and 28 must be clear.
+	 * Software indicator bits 21 and 28 must be clear.
 	 * Software indicator bits 24, 25, 26, and 27 must be
 	 * set.  All other Linux PTE bits control the behavior
 	 * of the MMU.
 	 */
 	li	r11, 0x00f0
-	rlwimi	r10, r11, 0, 24, 28	/* Set 24-27, clear 28 */
+	rlwimi	r10, r11, 0, 0x07f8	/* Set 24-27, clear 21-23,28 */
 	DO_8xx_CPU6(0x2d80, r3)
 	mtspr	SPRN_MI_RPN, r10	/* Update TLB entry */
 
@@ -444,9 +441,7 @@ DataStoreTLBMiss:
 
 	/* Honour kernel RO, User NA */
 	/* 0x200 == Extended encoding, bit 22 */
-	/* r11 =  (r10 & _PAGE_USER) >> 2 */
-	rlwinm	r11, r10, 32-2, 0x200
-	or	r10, r11, r10
+	rlwimi	r10, r10, 32-2, 0x200 /* Copy USER to bit 22, 0x200 */
 	/* r11 =  (r10 & _PAGE_RW) >> 1 */
 	rlwinm	r11, r10, 32-1, 0x200
 	or	r10, r11, r10
-- 
1.6.4.4