[PATCH] TX49 MFC0 bug workaround

[PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

If mfc0 $12 follows store and the mfc0 is last instruction of a
page and fetching the next instruction causes TLB miss, the result
of the mfc0 might wrongly contain EXL bit.

ERT-TX49H2-027, ERT-TX49H3-012, ERT-TX49HL3-006, ERT-TX49H4-008

Workaround: mask EXL bit of the result or place a nop before mfc0.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/include/asm-mips/interrupt.h b/include/asm-mips/interrupt.h
index 0da5818..951ee7a 100644
--- a/include/asm-mips/interrupt.h
+++ b/include/asm-mips/interrupt.h
@@ -13,6 +13,7 @@
 
 #include <linux/config.h>
 #include <asm/hazards.h>
+#include <asm/war.h>
 
 __asm__ (
 	"	.macro	local_irq_enable				\n"
@@ -55,8 +56,13 @@ __asm__ (
 	"	di							\n"
 #else
 	"	mfc0	$1,$12						\n"
+#if TX49XX_MFC0_WAR && defined(MODULE)
+	"	ori	$1,3						\n"
+	"	xori	$1,3						\n"
+#else
 	"	ori	$1,1						\n"
 	"	xori	$1,1						\n"
+#endif
 	"	.set	noreorder					\n"
 	"	mtc0	$1,$12						\n"
 #endif
@@ -96,8 +102,13 @@ __asm__ (
 	"	andi	\\result, 1					\n"
 #else
 	"	mfc0	\\result, $12					\n"
+#if TX49XX_MFC0_WAR && defined(MODULE)
+	"	ori	$1, \\result, 3					\n"
+	"	xori	$1, 3						\n"
+#else
 	"	ori	$1, \\result, 1					\n"
 	"	xori	$1, 1						\n"
+#endif
 	"	.set	noreorder					\n"
 	"	mtc0	$1, $12						\n"
 #endif
@@ -136,8 +147,13 @@ __asm__ (
 #else
 	"	mfc0	$1, $12						\n"
 	"	andi	\\flags, 1					\n"
+#if TX49XX_MFC0_WAR && defined(MODULE)
+	"	ori	$1, 3						\n"
+	"	xori	$1, 3						\n"
+#else
 	"	ori	$1, 1						\n"
 	"	xori	$1, 1						\n"
+#endif
 	"	or	\\flags, $1					\n"
 	"	mtc0	\\flags, $12					\n"
 #endif
diff --git a/include/asm-mips/war.h b/include/asm-mips/war.h
index ad374bd..859520a 100644
--- a/include/asm-mips/war.h
+++ b/include/asm-mips/war.h
@@ -169,6 +169,19 @@
 #endif
 
 /*
+ * If mfc0 $12 follows store and the mfc0 is last instruction of a
+ * page and fetching the next instruction causes TLB miss, the result
+ * of the mfc0 might wrongly contain EXL bit.
+ *
+ * ERT-TX49H2-027, ERT-TX49H3-012, ERT-TX49HL3-006, ERT-TX49H4-008
+ *
+ * Workaround: mask EXL bit of the result or place a nop before mfc0.
+ */
+#ifdef CONFIG_CPU_TX49XX
+#define TX49XX_MFC0_WAR 1
+#endif
+
+/*
  * On the RM9000 there is a problem which makes the CreateDirtyExclusive
  * cache operation unusable on SMP systems.
  */
@@ -228,6 +241,9 @@
 #ifndef TX49XX_ICACHE_INDEX_INV_WAR
 #define TX49XX_ICACHE_INDEX_INV_WAR	0
 #endif
+#ifndef TX49XX_MFC0_WAR
+#define TX49XX_MFC0_WAR	0
+#endif
 #ifndef RM9000_CDEX_SMP_WAR
 #define RM9000_CDEX_SMP_WAR		0
 #endif

Re: [PATCH] TX49 MFC0 bug workaround

[Maciej W. Rozycki]

On Fri, 3 Feb 2006, Atsushi Nemoto wrote:

> Workaround: mask EXL bit of the result or place a nop before mfc0.
[...]
> @@ -55,8 +56,13 @@ __asm__ (
>  	"	di							\n"
>  #else
>  	"	mfc0	$1,$12						\n"
> +#if TX49XX_MFC0_WAR && defined(MODULE)
> +	"	ori	$1,3						\n"
> +	"	xori	$1,3						\n"
> +#else
>  	"	ori	$1,1						\n"
>  	"	xori	$1,1						\n"
> +#endif
>  	"	.set	noreorder					\n"
>  	"	mtc0	$1,$12						\n"
>  #endif

 Hmm, wouldn't that "nop" alternative be simpler?

  Maciej

Re: [PATCH] TX49 MFC0 bug workaround

[Ralf Baechle]

On Fri, Feb 03, 2006 at 01:34:01AM +0900, Atsushi Nemoto wrote:

> If mfc0 $12 follows store and the mfc0 is last instruction of a
> page and fetching the next instruction causes TLB miss, the result
> of the mfc0 might wrongly contain EXL bit.
> 
> ERT-TX49H2-027, ERT-TX49H3-012, ERT-TX49HL3-006, ERT-TX49H4-008
> 
> Workaround: mask EXL bit of the result or place a nop before mfc0.

Applied,

  Ralf

Re: [PATCH] TX49 MFC0 bug workaround

[Ralf Baechle]

On Thu, Feb 02, 2006 at 04:38:37PM +0000, Maciej W. Rozycki wrote:
> Date:	Thu, 2 Feb 2006 16:38:37 +0000 (GMT)
> From:	"Maciej W. Rozycki" <macro@linux-mips.org>
> To:	Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> Cc:	linux-mips@linux-mips.org, ralf@linux-mips.org
> Subject: Re: [PATCH] TX49 MFC0 bug workaround
> Content-Type: TEXT/PLAIN; charset=US-ASCII
> 
> On Fri, 3 Feb 2006, Atsushi Nemoto wrote:
> 
> > Workaround: mask EXL bit of the result or place a nop before mfc0.
> [...]
> > @@ -55,8 +56,13 @@ __asm__ (
> >  	"	di							\n"
> >  #else
> >  	"	mfc0	$1,$12						\n"
> > +#if TX49XX_MFC0_WAR && defined(MODULE)
> > +	"	ori	$1,3						\n"
> > +	"	xori	$1,3						\n"
> > +#else
> >  	"	ori	$1,1						\n"
> >  	"	xori	$1,1						\n"
> > +#endif
> >  	"	.set	noreorder					\n"
> >  	"	mtc0	$1,$12						\n"
> >  #endif
> 
>  Hmm, wouldn't that "nop" alternative be simpler?

Simpler maybe - but this variant has zero runtime overhead.

  Ralf

Re: [PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

>>>>> On Thu, 2 Feb 2006 16:56:56 +0000, Ralf Baechle <ralf@linux-mips.org> said:

>> Hmm, wouldn't that "nop" alternative be simpler?

ralf> Simpler maybe - but this variant has zero runtime overhead.

Yes.  I do not want do add extra cycles.

However, It can be more readable since we can safely mask bit[5:1] (as
local_irq_enable() does).  Like this:

__asm__ (
	"	.macro	local_irq_disable\n"
	"	.set	push						\n"
	"	.set	noat						\n"
#ifdef CONFIG_CPU_MIPSR2
	"	di							\n"
#else
	"	mfc0	$1,$12						\n"
	"	ori	$1,0x1f						\n"
	"	xori	$1,0x1f						\n"
	"	.set	noreorder					\n"
	"	mtc0	$1,$12						\n"
#endif
	"	irq_disable_hazard					\n"
	"	.set	pop						\n"
	"	.endm							\n");


Is this preferred?  We can get rid of all TX49XX_MFC0_WAR on this way.

---
Atsushi Nemoto

Re: [PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

>>>>> On Fri, 03 Feb 2006 02:04:28 +0900 (JST), Atsushi Nemoto <anemo@mba.ocn.ne.jp> said:

anemo> However, It can be more readable since we can safely mask
anemo> bit[5:1] (as local_irq_enable() does).  Like this:

Correction: bit[4:1] (0x1e)

Re: [PATCH] TX49 MFC0 bug workaround

[Ralf Baechle]

On Fri, Feb 03, 2006 at 02:04:28AM +0900, Atsushi Nemoto wrote:

> Is this preferred?  We can get rid of all TX49XX_MFC0_WAR on this way.

It should be ok for any R4000-style status register.  I'm not sure about
R3000 - but I'm sure Maciej will know.  If he agrees I'd say let's go for
it.

  Ralf

Re: [PATCH] TX49 MFC0 bug workaround

[Sergei Shtylylov]

Hello.

Atsushi Nemoto wrote:
> If mfc0 $12 follows store and the mfc0 is last instruction of a
> page and fetching the next instruction causes TLB miss, the result
> of the mfc0 might wrongly contain EXL bit.

    Hmm, a TLB miss in fetching from KSEG0?!

> ERT-TX49H2-027, ERT-TX49H3-012, ERT-TX49HL3-006, ERT-TX49H4-008
> 
> Workaround: mask EXL bit of the result or place a nop before mfc0.

    Is this workaround really needed?

> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

WBR, Sergei

Re: [PATCH] TX49 MFC0 bug workaround

[Ralf Baechle]

On Thu, Feb 02, 2006 at 09:46:25PM +0300, Sergei Shtylylov wrote:

> Atsushi Nemoto wrote:
> >If mfc0 $12 follows store and the mfc0 is last instruction of a
> >page and fetching the next instruction causes TLB miss, the result
> >of the mfc0 might wrongly contain EXL bit.
> 
>    Hmm, a TLB miss in fetching from KSEG0?!

It'll hit loadable modules which run in the mapped KSEG2/3 spaces.

  Ralf

Re: [PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

>>>>> On Thu, 02 Feb 2006 21:46:25 +0300, Sergei Shtylylov <sshtylyov@ru.mvista.com> said:
>> If mfc0 $12 follows store and the mfc0 is last instruction of a
>> page and fetching the next instruction causes TLB miss, the result
>> of the mfc0 might wrongly contain EXL bit.

sshtylyov>     Hmm, a TLB miss in fetching from KSEG0?!

We can call these inline functions from modules running on KSEG2.

---
Atsushi Nemoto

Re: [PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

>>>>> On Thu, 2 Feb 2006 17:24:34 +0000, Ralf Baechle <ralf@linux-mips.org> said:
ralf> It should be ok for any R4000-style status register.  I'm not
ralf> sure about R3000 - but I'm sure Maciej will know.  If he agrees
ralf> I'd say let's go for it.

It should be OK for all CPU while STI/CLI/KMODE macro always clear
bit[4:1] of status register.  Could you confirm, Maciej ?

So here is a patch against current GIT.


Always clear bit[4:1] of status register.  This makes interrupt.h TX49
bug proof.  TX49XX_MFC0_WAR is not needed anymore.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/include/asm-mips/interrupt.h b/include/asm-mips/interrupt.h
index 951ee7a..7743487 100644
--- a/include/asm-mips/interrupt.h
+++ b/include/asm-mips/interrupt.h
@@ -13,7 +13,6 @@
 
 #include <linux/config.h>
 #include <asm/hazards.h>
-#include <asm/war.h>
 
 __asm__ (
 	"	.macro	local_irq_enable				\n"
@@ -48,6 +47,17 @@ static inline void local_irq_enable(void
  * R4000/R4400 need three nops, the R4600 two nops and the R10000 needs
  * no nops at all.
  */
+/*
+ * For TX49, operating only IE bit is not enough.
+ *
+ * If mfc0 $12 follows store and the mfc0 is last instruction of a
+ * page and fetching the next instruction causes TLB miss, the result
+ * of the mfc0 might wrongly contain EXL bit.
+ *
+ * ERT-TX49H2-027, ERT-TX49H3-012, ERT-TX49HL3-006, ERT-TX49H4-008
+ *
+ * Workaround: mask EXL bit of the result or place a nop before mfc0.
+ */
 __asm__ (
 	"	.macro	local_irq_disable\n"
 	"	.set	push						\n"
@@ -56,13 +66,8 @@ __asm__ (
 	"	di							\n"
 #else
 	"	mfc0	$1,$12						\n"
-#if TX49XX_MFC0_WAR && defined(MODULE)
-	"	ori	$1,3						\n"
-	"	xori	$1,3						\n"
-#else
-	"	ori	$1,1						\n"
-	"	xori	$1,1						\n"
-#endif
+	"	ori	$1,0x1f						\n"
+	"	xori	$1,0x1f						\n"
 	"	.set	noreorder					\n"
 	"	mtc0	$1,$12						\n"
 #endif
@@ -102,13 +107,8 @@ __asm__ (
 	"	andi	\\result, 1					\n"
 #else
 	"	mfc0	\\result, $12					\n"
-#if TX49XX_MFC0_WAR && defined(MODULE)
-	"	ori	$1, \\result, 3					\n"
-	"	xori	$1, 3						\n"
-#else
-	"	ori	$1, \\result, 1					\n"
-	"	xori	$1, 1						\n"
-#endif
+	"	ori	$1, \\result, 0x1f				\n"
+	"	xori	$1, 0x1f					\n"
 	"	.set	noreorder					\n"
 	"	mtc0	$1, $12						\n"
 #endif
@@ -147,13 +147,8 @@ __asm__ (
 #else
 	"	mfc0	$1, $12						\n"
 	"	andi	\\flags, 1					\n"
-#if TX49XX_MFC0_WAR && defined(MODULE)
-	"	ori	$1, 3						\n"
-	"	xori	$1, 3						\n"
-#else
-	"	ori	$1, 1						\n"
-	"	xori	$1, 1						\n"
-#endif
+	"	ori	$1, 0x1f					\n"
+	"	xori	$1, 0x1f					\n"
 	"	or	\\flags, $1					\n"
 	"	mtc0	\\flags, $12					\n"
 #endif
diff --git a/include/asm-mips/war.h b/include/asm-mips/war.h
index 859520a..229afaa 100644
--- a/include/asm-mips/war.h
+++ b/include/asm-mips/war.h
@@ -169,19 +169,6 @@
 #endif
 
 /*
- * If mfc0 $12 follows store and the mfc0 is last instruction of a
- * page and fetching the next instruction causes TLB miss, the result
- * of the mfc0 might wrongly contain EXL bit.
- *
- * ERT-TX49H2-027, ERT-TX49H3-012, ERT-TX49HL3-006, ERT-TX49H4-008
- *
- * Workaround: mask EXL bit of the result or place a nop before mfc0.
- */
-#ifdef CONFIG_CPU_TX49XX
-#define TX49XX_MFC0_WAR 1
-#endif
-
-/*
  * On the RM9000 there is a problem which makes the CreateDirtyExclusive
  * cache operation unusable on SMP systems.
  */

Re: [PATCH] TX49 MFC0 bug workaround

[Sergei Shtylylov]

Hello.

Atsushi Nemoto wrote:

>>>>>>On Thu, 02 Feb 2006 21:46:25 +0300, Sergei Shtylylov <sshtylyov@ru.mvista.com> said:
>>>
>>>If mfc0 $12 follows store and the mfc0 is last instruction of a
>>>page and fetching the next instruction causes TLB miss, the result
>>>of the mfc0 might wrongly contain EXL bit.
> 
> 
> sshtylyov>     Hmm, a TLB miss in fetching from KSEG0?!
> 
> We can call these inline functions from modules running on KSEG2.

    Hm, I'm still learning Linux/MIPS, and have overlooked #ifdef MODULE. :-<

    If I don't mistake, the offending code is in local_irq_disable, 
local_irq_save, and local_irq_restore macros. The effect would be a crash on 
any exception taken once interrupts get disabled in a module (*and* that code 
happens to fall on a page boundary)... nasty. :-(

> ---
> Atsushi Nemoto

WBR, Sergei

Re: [PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

>>>>> On Fri, 03 Feb 2006 05:12:47 +0300, Sergei Shtylylov <sshtylyov@ru.mvista.com> said:
sshtylyov>     If I don't mistake, the offending code is in
sshtylyov> local_irq_disable, local_irq_save, and local_irq_restore
sshtylyov> macros. The effect would be a crash on any exception taken
sshtylyov> once interrupts get disabled in a module (*and* that code
sshtylyov> happens to fall on a page boundary)... nasty. :-(

Right.  And it can really happen.

---
Atsushi Nemoto

Re: [PATCH] TX49 MFC0 bug workaround

[Sergei Shtylylov]

Hello.

Ralf Baechle wrote:

>>>Workaround: mask EXL bit of the result or place a nop before mfc0.
>>
>>[...]
>>
>>>@@ -55,8 +56,13 @@ __asm__ (
>>> 	"	di							\n"
>>> #else
>>> 	"	mfc0	$1,$12						\n"
>>>+#if TX49XX_MFC0_WAR && defined(MODULE)
>>>+	"	ori	$1,3						\n"
>>>+	"	xori	$1,3						\n"
>>>+#else
>>> 	"	ori	$1,1						\n"
>>> 	"	xori	$1,1						\n"
>>>+#endif
>>> 	"	.set	noreorder					\n"
>>> 	"	mtc0	$1,$12						\n"
>>> #endif

>> Hmm, wouldn't that "nop" alternative be simpler?

> Simpler maybe - but this variant has zero runtime overhead.

    And.. how do you imagine placing a NOP (which surely just moves MFC0 down 
so that it's a 1st insn. on the next page). What if it'll move it to the 
errata prone address from a safe one instead?

WBR, Sergei

Re: [PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

>>>>> On Fri, 03 Feb 2006 05:26:48 +0300, Sergei Shtylylov <sshtylyov@ru.mvista.com> said:
sshtylyov>     And.. how do you imagine placing a NOP (which surely
sshtylyov> just moves MFC0 down so that it's a 1st insn. on the next
sshtylyov> page). What if it'll move it to the errata prone address
sshtylyov> from a safe one instead?

The NOP will break the "If mfc0 $12 follows store" condition.

Actually, the condition is more strict.  This is a code sequence from
the errata. (It seems English version is not updated yet...)

	Load/Store instruction
	Load/Store/Sync instruction
	Mfc0 rt,rd	; rd is Status/Cause
	-- page boundary --
	nop		; TLB mapped area

For Cause register case, Linux modules should never read it so it
would not be a problem.

---
Atsushi Nemoto

Re: [PATCH] TX49 MFC0 bug workaround

[Maciej W. Rozycki]

On Fri, 3 Feb 2006, Atsushi Nemoto wrote:

> It should be OK for all CPU while STI/CLI/KMODE macro always clear
> bit[4:1] of status register.  Could you confirm, Maciej ?

 Well, as long as RESTORE_SOME in <asm/stackframe.h> correctly restores 
the IEc, KUc, IEp and KUp bits in the status register this change should 
be OK for R3k-class processors.

  Maciej

Re: [PATCH] TX49 MFC0 bug workaround

[Ralf Baechle]

On Fri, Feb 03, 2006 at 11:10:12AM +0900, Atsushi Nemoto wrote:

> So here is a patch against current GIT.

Okay, applied.

  Ralf

Re: [PATCH] TX49 MFC0 bug workaround

[Atsushi Nemoto]

>>>>> On Fri, 3 Feb 2006 14:44:20 +0000, Ralf Baechle <ralf@linux-mips.org> said:
>> So here is a patch against current GIT.

ralf> Okay, applied.

Thanks.  And please apply this also...


Remove TX49XX_MFC0_WAR completely.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/include/asm-mips/war.h b/include/asm-mips/war.h
index 229afaa..ad374bd 100644
--- a/include/asm-mips/war.h
+++ b/include/asm-mips/war.h
@@ -228,9 +228,6 @@
 #ifndef TX49XX_ICACHE_INDEX_INV_WAR
 #define TX49XX_ICACHE_INDEX_INV_WAR	0
 #endif
-#ifndef TX49XX_MFC0_WAR
-#define TX49XX_MFC0_WAR	0
-#endif
 #ifndef RM9000_CDEX_SMP_WAR
 #define RM9000_CDEX_SMP_WAR		0
 #endif

Re: [PATCH] TX49 MFC0 bug workaround

[Ralf Baechle]

On Mon, Feb 06, 2006 at 06:36:55PM +0900, Atsushi Nemoto wrote:

> Thanks.  And please apply this also...
> 
> 
> Remove TX49XX_MFC0_WAR completely.

Applied, thanks.

  Ralf

Re: [PATCH] TX49 MFC0 bug workaround

[Sergei Shtylylov]

Hello.

Atsushi Nemoto wrote:
>>>>>>On Fri, 03 Feb 2006 05:12:47 +0300, Sergei Shtylylov <sshtylyov@ru.mvista.com> said:
> 
> sshtylyov>     If I don't mistake, the offending code is in
> sshtylyov> local_irq_disable, local_irq_save, and local_irq_restore
> sshtylyov> macros. The effect would be a crash on any exception taken
> sshtylyov> once interrupts get disabled in a module (*and* that code
> sshtylyov> happens to fall on a page boundary)... nasty. :-(

    ... and yet worse: any external interrupts actually blocked from CPU 
forever by Status.EXL set (which then will never be cleared after the errata 
triggers).

> Right.  And it can really happen.

> ---
> Atsushi Nemoto

WBR, Sergei

Re: [PATCH] TX49 MFC0 bug workaround

[Sergei Shtylylov]

Hello.

Atsushi Nemoto wrote:
>>>>>>On Fri, 03 Feb 2006 05:12:47 +0300, Sergei Shtylylov <sshtylyov@ru.mvista.com> said:
> 
> sshtylyov>     If I don't mistake, the offending code is in
> sshtylyov> local_irq_disable, local_irq_save, and local_irq_restore
> sshtylyov> macros. The effect would be a crash on any exception taken
> sshtylyov> once interrupts get disabled in a module (*and* that code
> sshtylyov> happens to fall on a page boundary)... nasty. :-(

    ... and yet worse: any external interrupts actually blocked from CPU 
forever by Status.EXL set (which then will never be cleared after the errata 
triggers).

> Right.  And it can really happen.

> ---
> Atsushi Nemoto

WBR, Sergei