[PATCH] 8xx: fix user space TLB walk in dcbX fixup

[PATCH] 8xx: fix user space TLB walk in dcbX fixup

[Joakim Tjernlund]

The newly added fixup for buggy dcbX insn's has
a bug that always trigger a kernel TLB walk so a user space
dcbX insn will cause a Kernel Machine Check if it hits DTLB error.

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

I found this problem in 2.4 and forward ported it to 2.6. I
cannot test it so I cannot be 100% sure I got it right.

 arch/powerpc/kernel/head_8xx.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index ce327c5..91bef6e 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -542,11 +542,11 @@ DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR
 FixupDAR:/* Entry point for dcbx workaround. */
 	/* fetch instruction from memory. */
 	mfspr	r10, SPRN_SRR0
+	andis.	r11, r10, 0x8000	/* Address >= 0x80000000 */
 	DO_8xx_CPU6(0x3780, r3)
 	mtspr	SPRN_MD_EPN, r10
 	mfspr	r11, SPRN_M_TWB	/* Get level 1 table entry address */
-	cmplwi	cr0, r11, 0x0800
-	blt-	3f		/* Branch if user space */
+	beq-	3f		/* Branch if user space */
 	lis	r11, (swapper_pg_dir-PAGE_OFFSET)@h
 	ori	r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
 	rlwimi	r11, r10, 32-20, 0xffc /* r11 = r11&~0xffc|(r10>>20)&0xffc */
-- 
1.6.4.4

Re: [PATCH] 8xx: fix user space TLB walk in dcbX fixup

[Benjamin Herrenschmidt]

On Fri, 2010-01-08 at 17:46 +0100, Joakim Tjernlund wrote:
> The newly added fixup for buggy dcbX insn's has
> a bug that always trigger a kernel TLB walk so a user space
> dcbX insn will cause a Kernel Machine Check if it hits DTLB error.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
> 
> I found this problem in 2.4 and forward ported it to 2.6. I
> cannot test it so I cannot be 100% sure I got it right.
> 
>  arch/powerpc/kernel/head_8xx.S |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Do you have something to make sure that TASK_SIZE is never bigger than
2G ? Else userspace could be all the way to 0xbfffffff ...

Cheers,
Ben.

> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index ce327c5..91bef6e 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -542,11 +542,11 @@ DARFixed:/* Return from dcbx instruction bug workaround, r10 holds value of DAR
>  FixupDAR:/* Entry point for dcbx workaround. */
>  	/* fetch instruction from memory. */
>  	mfspr	r10, SPRN_SRR0
> +	andis.	r11, r10, 0x8000	/* Address >= 0x80000000 */
>  	DO_8xx_CPU6(0x3780, r3)
>  	mtspr	SPRN_MD_EPN, r10
>  	mfspr	r11, SPRN_M_TWB	/* Get level 1 table entry address */
> -	cmplwi	cr0, r11, 0x0800
> -	blt-	3f		/* Branch if user space */
> +	beq-	3f		/* Branch if user space */
>  	lis	r11, (swapper_pg_dir-PAGE_OFFSET)@h
>  	ori	r11, r11, (swapper_pg_dir-PAGE_OFFSET)@l
>  	rlwimi	r11, r10, 32-20, 0xffc /* r11 = r11&~0xffc|(r10>>20)&0xffc */

Re: [PATCH] 8xx: fix user space TLB walk in dcbX fixup

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 12/01/2010 03:40:45:
>
> On Fri, 2010-01-08 at 17:46 +0100, Joakim Tjernlund wrote:
> > The newly added fixup for buggy dcbX insn's has
> > a bug that always trigger a kernel TLB walk so a user space
> > dcbX insn will cause a Kernel Machine Check if it hits DTLB error.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >
> > I found this problem in 2.4 and forward ported it to 2.6. I
> > cannot test it so I cannot be 100% sure I got it right.
> >
> >  arch/powerpc/kernel/head_8xx.S |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
>
> Do you have something to make sure that TASK_SIZE is never bigger than
> 2G ? Else userspace could be all the way to 0xbfffffff ...

No, but this is 8xx :) The TLB handlers has the same "limitation" and has always
been so.

      Jocke

Re: [PATCH] 8xx: fix user space TLB walk in dcbX fixup

[Benjamin Herrenschmidt]

On Tue, 2010-01-12 at 08:07 +0100, Joakim Tjernlund wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 12/01/2010 03:40:45:
> >
> > On Fri, 2010-01-08 at 17:46 +0100, Joakim Tjernlund wrote:
> > > The newly added fixup for buggy dcbX insn's has
> > > a bug that always trigger a kernel TLB walk so a user space
> > > dcbX insn will cause a Kernel Machine Check if it hits DTLB error.
> > >
> > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > > ---
> > >
> > > I found this problem in 2.4 and forward ported it to 2.6. I
> > > cannot test it so I cannot be 100% sure I got it right.
> > >
> > >  arch/powerpc/kernel/head_8xx.S |    4 ++--
> > >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > Do you have something to make sure that TASK_SIZE is never bigger than
> > 2G ? Else userspace could be all the way to 0xbfffffff ...
> 
> No, but this is 8xx :) The TLB handlers has the same "limitation" and has always
> been so.

You should send a patch to express that limitation in KConfig :-)

Cheers,
Ben.

Re: [PATCH] 8xx: fix user space TLB walk in dcbX fixup

[Joakim Tjernlund]

Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 12/01/2010 09:44:09:
>
> On Tue, 2010-01-12 at 08:07 +0100, Joakim Tjernlund wrote:
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote on 12/01/2010 03:40:45:
> > >
> > > On Fri, 2010-01-08 at 17:46 +0100, Joakim Tjernlund wrote:
> > > > The newly added fixup for buggy dcbX insn's has
> > > > a bug that always trigger a kernel TLB walk so a user space
> > > > dcbX insn will cause a Kernel Machine Check if it hits DTLB error.
> > > >
> > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > > > ---
> > > >
> > > > I found this problem in 2.4 and forward ported it to 2.6. I
> > > > cannot test it so I cannot be 100% sure I got it right.
> > > >
> > > >  arch/powerpc/kernel/head_8xx.S |    4 ++--
> > > >  1 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > Do you have something to make sure that TASK_SIZE is never bigger than
> > > 2G ? Else userspace could be all the way to 0xbfffffff ...
> >
> > No, but this is 8xx :) The TLB handlers has the same "limitation" and has always
> > been so.
>
> You should send a patch to express that limitation in KConfig :-)

Yeah, but this is nothing new to 8xx users.
The TLB handlers (and possible other parts too) has always had this limitation.
I don't think anyone cares since this is a small embedded CPU.

       Jocke