[04/10] 8xx: Always pin kernel instruction TLB

Various kernel asm modifies SRR0/SRR1 just before executing
a rfi. If such code crosses a page boundary you risk a TLB miss
which will clobber SRR0/SRR1. Avoid this by always pinning
kernel instruction TLB space.

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

On Fri, 2009-11-20 at 11:21 +0100, Joakim Tjernlund wrote:
> Various kernel asm modifies SRR0/SRR1 just before executing
> a rfi. If such code crosses a page boundary you risk a TLB miss
> which will clobber SRR0/SRR1. Avoid this by always pinning
> kernel instruction TLB space.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  arch/powerpc/kernel/head_8xx.S |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index a9f1ace..e70503d 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -705,7 +705,7 @@ start_here:
>   */
>  initial_mmu:
>  	tlbia			/* Invalidate all TLB entries */
> -#ifdef CONFIG_PIN_TLB
> +#if 1 /* CONFIG_PIN_TLB */
>  	lis	r8, MI_RSV4I@h
>  	ori	r8, r8, 0x1c00
>  #else

Not nice. Either remove the config option or make sure all those code
sequences are appropriately aligned so it doesn't happen. I recommend
the later :-)

I'll apply the other patches.

Cheers,
Ben.

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 09/12/2009 05:19:59:

> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> To: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Cc: Scott Wood <scottwood@freescale.com>, "linuxppc-dev@ozlabs.org" <linuxppc-
> dev@ozlabs.org>, Rex Feany <RFeany@mrv.com>
> Date: 09/12/2009 05:20
> Subject: Re: [PATCH 04/10] 8xx: Always pin kernel instruction TLB
>
> On Fri, 2009-11-20 at 11:21 +0100, Joakim Tjernlund wrote:
> > Various kernel asm modifies SRR0/SRR1 just before executing
> > a rfi. If such code crosses a page boundary you risk a TLB miss
> > which will clobber SRR0/SRR1. Avoid this by always pinning
> > kernel instruction TLB space.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >  arch/powerpc/kernel/head_8xx.S |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> > index a9f1ace..e70503d 100644
> > --- a/arch/powerpc/kernel/head_8xx.S
> > +++ b/arch/powerpc/kernel/head_8xx.S
> > @@ -705,7 +705,7 @@ start_here:
> >   */
> >  initial_mmu:
> >     tlbia         /* Invalidate all TLB entries */
> > -#ifdef CONFIG_PIN_TLB
> > +#if 1 /* CONFIG_PIN_TLB */
> >     lis   r8, MI_RSV4I@h
> >     ori   r8, r8, 0x1c00
> >  #else
>
> Not nice. Either remove the config option or make sure all those code
> sequences are appropriately aligned so it doesn't happen. I recommend
> the later :-)

The later isn't as simple :) I believe the bulk of such code in entry_32.S.
Anyhow, the config option is still valid as if enabled
it will pin several DTLB's too. Scott had some concerns about removing the
config option completely so this was the next best thing.

>
> I'll apply the other patches.
OK, great.

On Wed, 2009-12-09 at 08:39 +0100, Joakim Tjernlund wrote:
> The later isn't as simple :) I believe the bulk of such code in
> entry_32.S.

Yeah but it would be useful for hash I suppose if one really wants to
boot with nobats. Though at least on hash most of the time we have ways
to recover by mean of MSR:RI being cleared, which your TLB miss code
doesn't check...

> Anyhow, the config option is still valid as if enabled
> it will pin several DTLB's too. Scott had some concerns about removing
> the config option completely so this was the next best thing.

Well, if you want to pin at least one entry, then just remove the #if
completely but don't leave a #if 1 :-)

Cheers,
Ben.

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 09/12/2009 09:56:35:
>
> On Wed, 2009-12-09 at 08:39 +0100, Joakim Tjernlund wrote:
> > The later isn't as simple :) I believe the bulk of such code in
> > entry_32.S.
>
> Yeah but it would be useful for hash I suppose if one really wants to
> boot with nobats. Though at least on hash most of the time we have ways
> to recover by mean of MSR:RI being cleared, which your TLB miss code
> doesn't check...

Never looked at this so I cannot comment.

>
> > Anyhow, the config option is still valid as if enabled
> > it will pin several DTLB's too. Scott had some concerns about removing
> > the config option completely so this was the next best thing.
>
> Well, if you want to pin at least one entry, then just remove the #if
> completely but don't leave a #if 1 :-)

I could, but I wanted to have an easy way back to the old way if
the need arises.
Anyway, I am rather busy ATM with more pressing matters so I would
appreciate if you could take the patch as is.

 Jocke