I think I have 8xx working...

I think I have 8xx working...

[Rex Feany]

The biggest problem for me turned out to be the MMU context IDs being
clamped to 32 when the 8xx only has 16. With this, things are a bit more
stable :)

diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index c2f93dc..15e00c5 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -404,7 +404,8 @@ void __init mmu_context_init(void)
 	}
 
 #ifdef DEBUG_CLAMP_LAST_CONTEXT
-	last_context = DEBUG_CLAMP_LAST_CONTEXT;
+	if (last_context > DEBUG_CLAMP_LAST_CONTEXT)
+	    last_context = DEBUG_CLAMP_LAST_CONTEXT;
 #endif
 	/*
 	 * Allocate the maps used by context management
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index e7dae82..26fb6b9 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -40,7 +40,7 @@
 #include <asm/uaccess.h>
 #include <asm/tlbflush.h>
 #include <asm/siginfo.h>
-
+#include <mm/mmu_decl.h>
 
 #ifdef CONFIG_KPROBES
 static inline int notify_page_fault(struct pt_regs *regs)
@@ -246,6 +246,12 @@ good_area:
 		goto bad_area;
 #endif /* CONFIG_6xx */
 #if defined(CONFIG_8xx)
+	/* 8xx sometimes need to load a invalid/non-present TLBs.
+	 * These must be invalidated separately as linux mm don't.
+	 */
+	if (error_code & 0x40000000) /* no translation? */
+		_tlbil_va(address, 0, 0, 0);
+
         /* The MPC8xx seems to always set 0x80000000, which is
          * "undefined".  Of those that can be set, this is the only
          * one which seems bad.

Re: I think I have 8xx working...

[Benjamin Herrenschmidt]

On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote:
> The biggest problem for me turned out to be the MMU context IDs being
> clamped to 32 when the 8xx only has 16. With this, things are a bit more
> stable :)

Ugh ? The clamp went upstream ? That sucks... let me fix that asap

Cheers,
Ben.

> diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
> index c2f93dc..15e00c5 100644
> --- a/arch/powerpc/mm/mmu_context_nohash.c
> +++ b/arch/powerpc/mm/mmu_context_nohash.c
> @@ -404,7 +404,8 @@ void __init mmu_context_init(void)
>  	}
>  
>  #ifdef DEBUG_CLAMP_LAST_CONTEXT
> -	last_context = DEBUG_CLAMP_LAST_CONTEXT;
> +	if (last_context > DEBUG_CLAMP_LAST_CONTEXT)
> +	    last_context = DEBUG_CLAMP_LAST_CONTEXT;
>  #endif
>  	/*
>  	 * Allocate the maps used by context management
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index e7dae82..26fb6b9 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -40,7 +40,7 @@
>  #include <asm/uaccess.h>
>  #include <asm/tlbflush.h>
>  #include <asm/siginfo.h>
> -
> +#include <mm/mmu_decl.h>
>  
>  #ifdef CONFIG_KPROBES
>  static inline int notify_page_fault(struct pt_regs *regs)
> @@ -246,6 +246,12 @@ good_area:
>  		goto bad_area;
>  #endif /* CONFIG_6xx */
>  #if defined(CONFIG_8xx)
> +	/* 8xx sometimes need to load a invalid/non-present TLBs.
> +	 * These must be invalidated separately as linux mm don't.
> +	 */
> +	if (error_code & 0x40000000) /* no translation? */
> +		_tlbil_va(address, 0, 0, 0);
> +
>          /* The MPC8xx seems to always set 0x80000000, which is
>           * "undefined".  Of those that can be set, this is the only
>           * one which seems bad.

Re: I think I have 8xx working...

[Benjamin Herrenschmidt]

On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote:
> The biggest problem for me turned out to be the MMU context IDs being
> clamped to 32 when the 8xx only has 16. With this, things are a bit more
> stable :)

This is with Joakim patches or without ?

Ben.

> diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
> index c2f93dc..15e00c5 100644
> --- a/arch/powerpc/mm/mmu_context_nohash.c
> +++ b/arch/powerpc/mm/mmu_context_nohash.c
> @@ -404,7 +404,8 @@ void __init mmu_context_init(void)
>  	}
>  
>  #ifdef DEBUG_CLAMP_LAST_CONTEXT
> -	last_context = DEBUG_CLAMP_LAST_CONTEXT;
> +	if (last_context > DEBUG_CLAMP_LAST_CONTEXT)
> +	    last_context = DEBUG_CLAMP_LAST_CONTEXT;
>  #endif
>  	/*
>  	 * Allocate the maps used by context management
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index e7dae82..26fb6b9 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -40,7 +40,7 @@
>  #include <asm/uaccess.h>
>  #include <asm/tlbflush.h>
>  #include <asm/siginfo.h>
> -
> +#include <mm/mmu_decl.h>
>  
>  #ifdef CONFIG_KPROBES
>  static inline int notify_page_fault(struct pt_regs *regs)
> @@ -246,6 +246,12 @@ good_area:
>  		goto bad_area;
>  #endif /* CONFIG_6xx */
>  #if defined(CONFIG_8xx)
> +	/* 8xx sometimes need to load a invalid/non-present TLBs.
> +	 * These must be invalidated separately as linux mm don't.
> +	 */
> +	if (error_code & 0x40000000) /* no translation? */
> +		_tlbil_va(address, 0, 0, 0);
> +
>          /* The MPC8xx seems to always set 0x80000000, which is
>           * "undefined".  Of those that can be set, this is the only
>           * one which seems bad.

Re: I think I have 8xx working...

[Rex Feany]

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

> On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote:
> > The biggest problem for me turned out to be the MMU context IDs being
> > clamped to 32 when the 8xx only has 16. With this, things are a bit more
> > stable :)
> 
> This is with Joakim patches or without ?

with just one: adding tlbil_va() to do_page_fault().

take care!
/rex.

Re: I think I have 8xx working...

[Benjamin Herrenschmidt]

On Wed, 2009-10-14 at 18:08 -0700, Rex Feany wrote:
> Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):
> 
> > On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote:
> > > The biggest problem for me turned out to be the MMU context IDs being
> > > clamped to 32 when the 8xx only has 16. With this, things are a bit more
> > > stable :)
> > 
> > This is with Joakim patches or without ?
> 
> with just one: adding tlbil_va() to do_page_fault().

Ah cool, at least that keeps my sanity, I didn't get how it could work
at all without that bit :-)

Scott, Joakim: It will be interesting to figure out exactly what is the
condition where that is necessary. It definitely should make the one
in set_pte_filter() or ptep_set_access_flags_filter() unnecessary.

Joakim, you mentioned increased amount of TLB errors or misses when
doing that too often, that sounds a bit weird and worth investigating

Finally we could implement preload from update_mmu_cache() but that's a
different matter alltogether.

Cheers,
Ben.

Re: I think I have 8xx working...

[Joakim Tjernlund]

Rex Feany <RFeany@mrv.com> wrote on 15/10/2009 03:08:55:
>
> Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):
>
> > On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote:
> > > The biggest problem for me turned out to be the MMU context IDs being
> > > clamped to 32 when the 8xx only has 16. With this, things are a bit more
> > > stable :)
> >
> > This is with Joakim patches or without ?
>
> with just one: adding tlbil_va() to do_page_fault().

Looking forward too hear about the the other patches too :)

Re: I think I have 8xx working...

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 15/10/2009 03:12:50:

>
> On Wed, 2009-10-14 at 18:08 -0700, Rex Feany wrote:
> > Thus spake Benjamin Herrenschmidt (benh@kernel.crashing.org):
> >
> > > On Wed, 2009-10-14 at 17:41 -0700, Rex Feany wrote:
> > > > The biggest problem for me turned out to be the MMU context IDs being
> > > > clamped to 32 when the 8xx only has 16. With this, things are a bit more
> > > > stable :)
> > >
> > > This is with Joakim patches or without ?
> >
> > with just one: adding tlbil_va() to do_page_fault().
>
> Ah cool, at least that keeps my sanity, I didn't get how it could work
> at all without that bit :-)
>
> Scott, Joakim: It will be interesting to figure out exactly what is the
> condition where that is necessary. It definitely should make the one
> in set_pte_filter() or ptep_set_access_flags_filter() unnecessary.
>
> Joakim, you mentioned increased amount of TLB errors or misses when
> doing that too often, that sounds a bit weird and worth investigating

I didn't say that, did I? More like:
if I don't do tlbil_va at all I get a lot of extra/duplicate TLB errors
for the same address. Adding the patch makes these go away.
I guess one could do tlbil_va unconditionally but I didn't
see any improvements from doing that, but this was all on 2.4

>
> Finally we could implement preload from update_mmu_cache() but that's a
> different matter alltogether.
>
> Cheers,
> Ben.
>
>
>
>

Re: I think I have 8xx working...

[Benjamin Herrenschmidt]

On Thu, 2009-10-15 at 07:42 +0200, Joakim Tjernlund wrote:
> I didn't say that, did I? More like:
> if I don't do tlbil_va at all I get a lot of extra/duplicate TLB
> errors
> for the same address. Adding the patch makes these go away.
> I guess one could do tlbil_va unconditionally but I didn't
> see any improvements from doing that, but this was all on 2.4

Ok so I misparsed you. I was advocating going unconditional here
and though you said it would increase the miss rate.

Cheers,
Ben