[PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[Daniel Laird]

Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550

Signed-off-by: Daniel Laird <daniel.j.laird@nxp.com>
---
 tlb-r4k.c |    4 +++
 tlbex.c   |   69 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)
---
Index: linux-2.6.22.1/arch/mips/mm/tlbex.c
===================================================================
--- linux-2.6.22.1/arch/mips/mm/tlbex.c    (revision 8)
+++ linux-2.6.22.1/arch/mips/mm/tlbex.c    (working copy)
@@ -435,6 +435,9 @@
     label_nopage_tlbm,
     label_smp_pgtable_change,
     label_r3000_write_probe_fail,
+#ifdef CONFIG_PNX8550
+    label_pnx8550_bac_reset
+#endif
 };
 
 struct label {
@@ -470,6 +473,9 @@
 L_LA(_nopage_tlbm)
 L_LA(_smp_pgtable_change)
 L_LA(_r3000_write_probe_fail)
+#ifdef CONFIG_PNX8550
+L_LA(_pnx8550_bac_reset)
+#endif
 
 /* convenience macros for instructions */
 #ifdef CONFIG_64BIT
@@ -698,6 +704,14 @@
     r_mips_pc16(r, *p, l);
     i_bgez(p, reg, 0);
 }
+#ifdef CONFIG_PNX8550
+static void il_beq(u32 **p, struct reloc **r, unsigned int reg1,
+           unsigned int reg2, enum label_id l)
+{
+    r_mips_pc16(r, *p, l);
+    i_beq(p, reg1, reg2, 0);
+}
+#endif
 
 /* The only general purpose registers allowed in TLB handlers. */
 #define K0        26
@@ -734,6 +748,57 @@
 static __initdata struct label labels[128];
 static __initdata struct reloc relocs[128];
 
+#ifdef CONFIG_PNX8550
+static void __init build_pnx8550_bug_fix( u32 **p, struct label **l, 
struct reloc **r)
+{
+#define MFC0(_reg, _cp, _sel)    \
+        ((cop0_op)  << OP_SH    \
+         | (mfc_op) << RS_SH    \
+         | (_reg)   << RT_SH    \
+         | (_cp)    << RD_SH    \
+         | (_sel))
+
+#define MTC0(_reg, _cp, _sel)    \
+        ((cop0_op)  << OP_SH    \
+         | (mtc_op) << RS_SH    \
+         | (_reg)   << RT_SH    \
+         | (_cp)    << RD_SH    \
+         | (_sel))
+
+    /* load epc and badvaddr to k0 and k1 */
+    i_MFC0(p, K0, C0_EPC);
+    i_MFC0(p, K1, C0_BADVADDR);
+
+    /* branch if code entry  */
+    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
+    i_addiu(p, K0, K0, 4);
+
+    /* branch if code entry in BDS */
+    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
+    i_nop(p);
+    /* Write data tlb entry 11..31 */
+    i_tlbwr(p);
+    i_eret(p);
+    /* BAC Reset */
+    l_pnx8550_bac_reset(l, *p);
+    **p = MFC0(K0, C0_CONFIG, 7);
+    (*p)++;
+    i_ori(p, K0, K0, (1<<14));
+
+    **p = MTC0(K0, C0_CONFIG, 7);
+    (*p)++;
+
+    /* read random reg, sub 11, div by 2 */
+    i_MFC0(p, K1, C0_RANDOM);
+    i_addiu(p, K1, K1, -11);
+    i_srl(p, K1, K1, 1);
+
+    /* use as index for tlbwi */
+    i_MTC0(p, K1, C0_INDEX);
+    i_tlbwi(p);
+}
+#endif
+
 /*
  * The R3000 TLB handler is simple.
  */
@@ -1261,8 +1326,12 @@
 
     build_get_ptep(&p, K0, K1);
     build_update_entries(&p, K0, K1);
+#ifndef CONFIG_PNX8550
     build_tlb_write_entry(&p, &l, &r, tlb_random);
     l_leave(&l, p);
+#else
+    build_pnx8550_bug_fix(&p, &l, &r);
+#endif
     i_eret(&p); /* return from trap */
 
 #ifdef CONFIG_64BIT
Index: linux-2.6.22.1/arch/mips/mm/tlb-r4k.c
===================================================================
--- linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (revision 8)
+++ linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (working copy)
@@ -456,7 +456,11 @@
      */
     probe_tlb(config);
     write_c0_pagemask(PM_DEFAULT_MASK);
+#ifdef CONFIG_SOC_PNX8550
+    write_c0_wired(11);
+#else
     write_c0_wired(0);
+#endif
     write_c0_framemask(0);
     temp_tlb_entry = current_cpu_data.tlbsize - 1;

Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[Daniel Laird]

Update Patch

Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550

Signed-off-by: Daniel Laird <daniel.j.laird@nxp.com>
---
 tlb-r4k.c |    4 +++
 tlbex.c   |   71 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
---
Index: linux-2.6.22.1/arch/mips/mm/tlbex.c
===================================================================
--- linux-2.6.22.1/arch/mips/mm/tlbex.c    (revision 8)
+++ linux-2.6.22.1/arch/mips/mm/tlbex.c    (working copy)
@@ -435,6 +435,9 @@
     label_nopage_tlbm,
     label_smp_pgtable_change,
     label_r3000_write_probe_fail,
+#ifdef CONFIG_PNX8550
+    label_pnx8550_bac_reset
+#endif
 };
 
 struct label {
@@ -470,6 +473,9 @@
 L_LA(_nopage_tlbm)
 L_LA(_smp_pgtable_change)
 L_LA(_r3000_write_probe_fail)
+#ifdef CONFIG_PNX8550
+L_LA(_pnx8550_bac_reset)
+#endif
 
 /* convenience macros for instructions */
 #ifdef CONFIG_64BIT
@@ -698,6 +704,14 @@
     r_mips_pc16(r, *p, l);
     i_bgez(p, reg, 0);
 }
+#ifdef CONFIG_PNX8550
+static void il_beq(u32 **p, struct reloc **r, unsigned int reg1,
+           unsigned int reg2, enum label_id l)
+{
+    r_mips_pc16(r, *p, l);
+    i_beq(p, reg1, reg2, 0);
+}
+#endif
 
 /* The only general purpose registers allowed in TLB handlers. */
 #define K0        26
@@ -705,6 +719,7 @@
 
 /* Some CP0 registers */
 #define C0_INDEX    0, 0
+#define C0_RANDOM   1, 0
 #define C0_ENTRYLO0    2, 0
 #define C0_TCBIND    2, 2
 #define C0_ENTRYLO1    3, 0
@@ -712,6 +727,7 @@
 #define C0_BADVADDR    8, 0
 #define C0_ENTRYHI    10, 0
 #define C0_EPC        14, 0
+#define C0_CONFIG    16, 0
 #define C0_XCONTEXT    20, 0
 
 #ifdef CONFIG_64BIT
@@ -734,6 +750,57 @@
 static __initdata struct label labels[128];
 static __initdata struct reloc relocs[128];
 
+#ifdef CONFIG_PNX8550
+static void __init build_pnx8550_bug_fix( u32 **p, struct label **l, 
struct reloc **r)
+{
+#define MFC0(_reg, _cp, _sel)    \
+    ((cop0_op)  << OP_SH    \
+    | (mfc_op) << RS_SH    \
+    | (_reg)   << RT_SH    \
+    | (_cp)    << RD_SH    \
+    | (_sel))
+
+#define MTC0(_reg, _cp, _sel)    \
+    ((cop0_op)  << OP_SH    \
+    | (mtc_op) << RS_SH    \
+    | (_reg)   << RT_SH    \
+    | (_cp)    << RD_SH    \
+    | (_sel))
+
+    /* load epc and badvaddr to k0 and k1 */
+    i_MFC0(p, K0, C0_EPC);
+    i_MFC0(p, K1, C0_BADVADDR);
+
+    /* branch if code entry  */
+    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
+    i_addiu(p, K0, K0, 4);
+
+    /* branch if code entry in BDS */
+    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
+    i_nop(p);
+    /* Write data tlb entry 11..31 */
+    i_tlbwr(p);
+    i_eret(p);
+    /* BAC Reset */
+    l_pnx8550_bac_reset(l, *p);
+    **p = MFC0(K0, C0_CONFIG, 7);
+    (*p)++;
+    i_ori(p, K0, K0, (1<<14));
+
+    **p = MTC0(K0, C0_CONFIG, 7);
+    (*p)++;
+
+    /* read random reg, sub 11, div by 2 */
+    i_MFC0(p, K1, C0_RANDOM);
+    i_addiu(p, K1, K1, -11);
+    i_srl(p, K1, K1, 1);
+
+    /* use as index for tlbwi */
+    i_MTC0(p, K1, C0_INDEX);
+    i_tlbwi(p);
+}
+#endif
+
 /*
  * The R3000 TLB handler is simple.
  */
@@ -1261,8 +1328,12 @@
 
     build_get_ptep(&p, K0, K1);
     build_update_entries(&p, K0, K1);
+#ifndef CONFIG_PNX8550
     build_tlb_write_entry(&p, &l, &r, tlb_random);
     l_leave(&l, p);
+#else
+    build_pnx8550_bug_fix(&p, &l, &r);
+#endif
     i_eret(&p); /* return from trap */
 
 #ifdef CONFIG_64BIT
Index: linux-2.6.22.1/arch/mips/mm/tlb-r4k.c
===================================================================
--- linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (revision 8)
+++ linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (working copy)
@@ -456,7 +456,11 @@
      */
     probe_tlb(config);
     write_c0_pagemask(PM_DEFAULT_MASK);
+#ifdef CONFIG_SOC_PNX8550
+    write_c0_wired(11);
+#else
     write_c0_wired(0);
+#endif
     write_c0_framemask(0);
     temp_tlb_entry = current_cpu_data.tlbsize - 1;
 

Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[Jan-Benedict Glaw]

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

On Thu, 2007-07-19 16:24:29 +0100, Daniel Laird <daniel.j.laird@nxp.com> wrote:
> Update Patch
> 
> Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550

Both versions are whitespace damaged...

> Index: linux-2.6.22.1/arch/mips/mm/tlb-r4k.c
> ===================================================================
> --- linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (revision 8)
> +++ linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (working copy)
> @@ -456,7 +456,11 @@
>      */
>     probe_tlb(config);
>     write_c0_pagemask(PM_DEFAULT_MASK);
> +#ifdef CONFIG_SOC_PNX8550
> +    write_c0_wired(11);
> +#else
>     write_c0_wired(0);
> +#endif
>     write_c0_framemask(0);
>     temp_tlb_entry = current_cpu_data.tlbsize - 1;

Magic constants?

MfG, JBG

-- 
      Jan-Benedict Glaw      jbglaw@lug-owl.de              +49-172-7608481
  Signature of:                           Wenn ich wach bin, träume ich.
  the second  :

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[Maciej W. Rozycki]

On Thu, 19 Jul 2007, Daniel Laird wrote:

> @@ -435,6 +435,9 @@
>     label_nopage_tlbm,
>     label_smp_pgtable_change,
>     label_r3000_write_probe_fail,
> +#ifdef CONFIG_PNX8550
> +    label_pnx8550_bac_reset
> +#endif
> };
> 
> struct label {

 You have formatting problems (mailer suspected), a comma is missing from 
the above fragment and the whole proposal is a horrible #ifdef maze.  Can 
you please rewrite it in a more orderly fashion?

  Maciej

Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[Thiemo Seufer]

Daniel Laird wrote:
> Update Patch
>
> Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
>
> Signed-off-by: Daniel Laird <daniel.j.laird@nxp.com>
> ---
> tlb-r4k.c |    4 +++
> tlbex.c   |   71 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 75 insertions(+)
> ---
> Index: linux-2.6.22.1/arch/mips/mm/tlbex.c
> ===================================================================
> --- linux-2.6.22.1/arch/mips/mm/tlbex.c    (revision 8)
> +++ linux-2.6.22.1/arch/mips/mm/tlbex.c    (working copy)
> @@ -435,6 +435,9 @@
>     label_nopage_tlbm,
>     label_smp_pgtable_change,
>     label_r3000_write_probe_fail,
> +#ifdef CONFIG_PNX8550
> +    label_pnx8550_bac_reset
> +#endif

Please don't use #ifdef where it isn't needed ...

[snip]
> +static void __init build_pnx8550_bug_fix( u32 **p, struct label **l, 
> struct reloc **r)
> +{
> +#define MFC0(_reg, _cp, _sel)    \
> +    ((cop0_op)  << OP_SH    \
> +    | (mfc_op) << RS_SH    \
> +    | (_reg)   << RT_SH    \
> +    | (_cp)    << RD_SH    \
> +    | (_sel))
> +
> +#define MTC0(_reg, _cp, _sel)    \
> +    ((cop0_op)  << OP_SH    \
> +    | (mtc_op) << RS_SH    \
> +    | (_reg)   << RT_SH    \
> +    | (_cp)    << RD_SH    \
> +    | (_sel))

... don't reinvent the wheel ...

[snip]
> @@ -1261,8 +1328,12 @@
>     build_get_ptep(&p, K0, K1);
>     build_update_entries(&p, K0, K1);
> +#ifndef CONFIG_PNX8550
>     build_tlb_write_entry(&p, &l, &r, tlb_random);
>     l_leave(&l, p);
> +#else
> +    build_pnx8550_bug_fix(&p, &l, &r);
> +#endif

... and make that a runtime check.

>     i_eret(&p); /* return from trap */
> #ifdef CONFIG_64BIT
> Index: linux-2.6.22.1/arch/mips/mm/tlb-r4k.c
> ===================================================================
> --- linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (revision 8)
> +++ linux-2.6.22.1/arch/mips/mm/tlb-r4k.c    (working copy)
> @@ -456,7 +456,11 @@
>      */
>     probe_tlb(config);
>     write_c0_pagemask(PM_DEFAULT_MASK);
> +#ifdef CONFIG_SOC_PNX8550
> +    write_c0_wired(11);
> +#else
>     write_c0_wired(0);
> +#endif

11 wired entries sounds excessive to me. Is it really necessary to
kill that much performance?


Thiemo

Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[Sergei Shtylyov]

Hello.

Daniel Laird wrote:

 > Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550

    Would've been god to somehow document it too...

> Signed-off-by: Daniel Laird <daniel.j.laird@nxp.com>

> Index: linux-2.6.22.1/arch/mips/mm/tlbex.c
> ===================================================================
> --- linux-2.6.22.1/arch/mips/mm/tlbex.c    (revision 8)
> +++ linux-2.6.22.1/arch/mips/mm/tlbex.c    (working copy)
[...]
> @@ -705,6 +719,7 @@
> 
> /* Some CP0 registers */
> #define C0_INDEX    0, 0
> +#define C0_RANDOM   1, 0
> #define C0_ENTRYLO0    2, 0
> #define C0_TCBIND    2, 2
> #define C0_ENTRYLO1    3, 0
> @@ -712,6 +727,7 @@
> #define C0_BADVADDR    8, 0
> #define C0_ENTRYHI    10, 0
> #define C0_EPC        14, 0
> +#define C0_CONFIG    16, 0
> #define C0_XCONTEXT    20, 0
> 
> #ifdef CONFIG_64BIT
> @@ -734,6 +750,57 @@
> static __initdata struct label labels[128];
> static __initdata struct reloc relocs[128];
> 
> +#ifdef CONFIG_PNX8550
> +static void __init build_pnx8550_bug_fix( u32 **p, struct label **l, 
> struct reloc **r)
> +{
> +#define MFC0(_reg, _cp, _sel)    \
> +    ((cop0_op)  << OP_SH    \
> +    | (mfc_op) << RS_SH    \
> +    | (_reg)   << RT_SH    \
> +    | (_cp)    << RD_SH    \
> +    | (_sel))
> +
> +#define MTC0(_reg, _cp, _sel)    \
> +    ((cop0_op)  << OP_SH    \
> +    | (mtc_op) << RS_SH    \
> +    | (_reg)   << RT_SH    \
> +    | (_cp)    << RD_SH    \
> +    | (_sel))
> +

    Macro defs inside function, isn't that er... too much?
    And don't we already have macro M() doing exactly what these two macros 
are doing?

> +    /* load epc and badvaddr to k0 and k1 */
> +    i_MFC0(p, K0, C0_EPC);
> +    i_MFC0(p, K1, C0_BADVADDR);
> +
> +    /* branch if code entry  */
> +    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
> +    i_addiu(p, K0, K0, 4);
> +
> +    /* branch if code entry in BDS */
> +    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
> +    i_nop(p);
> +    /* Write data tlb entry 11..31 */
> +    i_tlbwr(p);
> +    i_eret(p);
> +    /* BAC Reset */
> +    l_pnx8550_bac_reset(l, *p);
> +    **p = MFC0(K0, C0_CONFIG, 7);
> +    (*p)++;

    Hmmm, why we need another version of i_MFC0? After looking at the currecnt 
code it seemed to me that i_M[FT]C0 are just using i_[d]m[ft]c0 incorrectly -- 
those are difined to have 3 opcode args (by being built as I_u1u2u3() but only 
get passed 2 args.  Hmm, I don't understand how i_[d]m[ft]c0() invocations 
used to work before since they're alsway passing only 2 opcode args where 3 
are requiered...
    Ah, that must be due to those tricky macros above -- with commas inside.
David, then this trickery is unacceptable -- add the needed macro instead; and 
there's no need to use i_M[FT]C0() on a 32-bit only CPU, just use i_m[ft]c0().

> +    i_ori(p, K0, K0, (1<<14));
> +
> +    **p = MTC0(K0, C0_CONFIG, 7);
> +    (*p)++;

    So, this also won't do.

[...]

WBR, Sergei

Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[danieljlaird]

Cheers for all comments, will
1.) remove reinvented code
2.) Add explanation of exact bug (corruption of TLB)
3.) Use runtime checking instead of #ifdef
4.) Not use Thunderbird to post patches!

Cheers
Dan


----- Original Message -----
From: "Sergei Shtylyov" <sshtylyov@ru.mvista.com>
To: "Daniel Laird" <daniel.j.laird@nxp.com>
Cc: <linux-mips@linux-mips.org>
Sent: Thursday, July 19, 2007 6:32 PM
Subject: Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

> Hello.
>
> Daniel Laird wrote:
>
> > Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
>
>    Would've been god to somehow document it too...
>
>> Signed-off-by: Daniel Laird <daniel.j.laird@nxp.com>
>
>> Index: linux-2.6.22.1/arch/mips/mm/tlbex.c
>> ===================================================================
>> --- linux-2.6.22.1/arch/mips/mm/tlbex.c    (revision 8)
>> +++ linux-2.6.22.1/arch/mips/mm/tlbex.c    (working copy)
> [...]
>> @@ -705,6 +719,7 @@
>>
>> /* Some CP0 registers */
>> #define C0_INDEX    0, 0
>> +#define C0_RANDOM   1, 0
>> #define C0_ENTRYLO0    2, 0
>> #define C0_TCBIND    2, 2
>> #define C0_ENTRYLO1    3, 0
>> @@ -712,6 +727,7 @@
>> #define C0_BADVADDR    8, 0
>> #define C0_ENTRYHI    10, 0
>> #define C0_EPC        14, 0
>> +#define C0_CONFIG    16, 0
>> #define C0_XCONTEXT    20, 0
>>
>> #ifdef CONFIG_64BIT
>> @@ -734,6 +750,57 @@
>> static __initdata struct label labels[128];
>> static __initdata struct reloc relocs[128];
>>
>> +#ifdef CONFIG_PNX8550
>> +static void __init build_pnx8550_bug_fix( u32 **p, struct label **l, 
>> struct reloc **r)
>> +{
>> +#define MFC0(_reg, _cp, _sel)    \
>> +    ((cop0_op)  << OP_SH    \
>> +    | (mfc_op) << RS_SH    \
>> +    | (_reg)   << RT_SH    \
>> +    | (_cp)    << RD_SH    \
>> +    | (_sel))
>> +
>> +#define MTC0(_reg, _cp, _sel)    \
>> +    ((cop0_op)  << OP_SH    \
>> +    | (mtc_op) << RS_SH    \
>> +    | (_reg)   << RT_SH    \
>> +    | (_cp)    << RD_SH    \
>> +    | (_sel))
>> +
>
>    Macro defs inside function, isn't that er... too much?
>    And don't we already have macro M() doing exactly what these two macros 
> are doing?
>
>> +    /* load epc and badvaddr to k0 and k1 */
>> +    i_MFC0(p, K0, C0_EPC);
>> +    i_MFC0(p, K1, C0_BADVADDR);
>> +
>> +    /* branch if code entry  */
>> +    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
>> +    i_addiu(p, K0, K0, 4);
>> +
>> +    /* branch if code entry in BDS */
>> +    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
>> +    i_nop(p);
>> +    /* Write data tlb entry 11..31 */
>> +    i_tlbwr(p);
>> +    i_eret(p);
>> +    /* BAC Reset */
>> +    l_pnx8550_bac_reset(l, *p);
>> +    **p = MFC0(K0, C0_CONFIG, 7);
>> +    (*p)++;
>
>    Hmmm, why we need another version of i_MFC0? After looking at the 
> currecnt code it seemed to me that i_M[FT]C0 are just using i_[d]m[ft]c0 
> incorrectly -- 
> those are difined to have 3 opcode args (by being built as I_u1u2u3() but 
> only get passed 2 args.  Hmm, I don't understand how i_[d]m[ft]c0() 
> invocations used to work before since they're alsway passing only 2 opcode 
> args where 3 are requiered...
>    Ah, that must be due to those tricky macros above -- with commas 
> inside.
> David, then this trickery is unacceptable -- add the needed macro instead; 
> and there's no need to use i_M[FT]C0() on a 32-bit only CPU, just use 
> i_m[ft]c0().
>
>> +    i_ori(p, K0, K0, (1<<14));
>> +
>> +    **p = MTC0(K0, C0_CONFIG, 7);
>> +    (*p)++;
>
>    So, this also won't do.
>
> [...]
>
> WBR, Sergei
>
> 

Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

[danieljlaird]

Cheers for all comments, will
1.) remove reinvented code
2.) Add explanation of exact bug (corruption of TLB)
3.) Use runtime checking instead of #ifdef
4.) Not use Thunderbird to post patches!

Cheers
Dan


----- Original Message -----
From: "Sergei Shtylyov" <sshtylyov@ru.mvista.com>
To: "Daniel Laird" <daniel.j.laird@nxp.com>
Cc: <linux-mips@linux-mips.org>
Sent: Thursday, July 19, 2007 6:32 PM
Subject: Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550

> Hello.
>
> Daniel Laird wrote:
>
> > Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
>
>    Would've been god to somehow document it too...
>
>> Signed-off-by: Daniel Laird <daniel.j.laird@nxp.com>
>
>> Index: linux-2.6.22.1/arch/mips/mm/tlbex.c
>> ===================================================================
>> --- linux-2.6.22.1/arch/mips/mm/tlbex.c    (revision 8)
>> +++ linux-2.6.22.1/arch/mips/mm/tlbex.c    (working copy)
> [...]
>> @@ -705,6 +719,7 @@
>>
>> /* Some CP0 registers */
>> #define C0_INDEX    0, 0
>> +#define C0_RANDOM   1, 0
>> #define C0_ENTRYLO0    2, 0
>> #define C0_TCBIND    2, 2
>> #define C0_ENTRYLO1    3, 0
>> @@ -712,6 +727,7 @@
>> #define C0_BADVADDR    8, 0
>> #define C0_ENTRYHI    10, 0
>> #define C0_EPC        14, 0
>> +#define C0_CONFIG    16, 0
>> #define C0_XCONTEXT    20, 0
>>
>> #ifdef CONFIG_64BIT
>> @@ -734,6 +750,57 @@
>> static __initdata struct label labels[128];
>> static __initdata struct reloc relocs[128];
>>
>> +#ifdef CONFIG_PNX8550
>> +static void __init build_pnx8550_bug_fix( u32 **p, struct label **l, 
>> struct reloc **r)
>> +{
>> +#define MFC0(_reg, _cp, _sel)    \
>> +    ((cop0_op)  << OP_SH    \
>> +    | (mfc_op) << RS_SH    \
>> +    | (_reg)   << RT_SH    \
>> +    | (_cp)    << RD_SH    \
>> +    | (_sel))
>> +
>> +#define MTC0(_reg, _cp, _sel)    \
>> +    ((cop0_op)  << OP_SH    \
>> +    | (mtc_op) << RS_SH    \
>> +    | (_reg)   << RT_SH    \
>> +    | (_cp)    << RD_SH    \
>> +    | (_sel))
>> +
>
>    Macro defs inside function, isn't that er... too much?
>    And don't we already have macro M() doing exactly what these two macros 
> are doing?
>
>> +    /* load epc and badvaddr to k0 and k1 */
>> +    i_MFC0(p, K0, C0_EPC);
>> +    i_MFC0(p, K1, C0_BADVADDR);
>> +
>> +    /* branch if code entry  */
>> +    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
>> +    i_addiu(p, K0, K0, 4);
>> +
>> +    /* branch if code entry in BDS */
>> +    il_beq(p, r, K0, K1, label_pnx8550_bac_reset);
>> +    i_nop(p);
>> +    /* Write data tlb entry 11..31 */
>> +    i_tlbwr(p);
>> +    i_eret(p);
>> +    /* BAC Reset */
>> +    l_pnx8550_bac_reset(l, *p);
>> +    **p = MFC0(K0, C0_CONFIG, 7);
>> +    (*p)++;
>
>    Hmmm, why we need another version of i_MFC0? After looking at the 
> currecnt code it seemed to me that i_M[FT]C0 are just using i_[d]m[ft]c0 
> incorrectly -- 
> those are difined to have 3 opcode args (by being built as I_u1u2u3() but 
> only get passed 2 args.  Hmm, I don't understand how i_[d]m[ft]c0() 
> invocations used to work before since they're alsway passing only 2 opcode 
> args where 3 are requiered...
>    Ah, that must be due to those tricky macros above -- with commas 
> inside.
> David, then this trickery is unacceptable -- add the needed macro instead; 
> and there's no need to use i_M[FT]C0() on a 32-bit only CPU, just use 
> i_m[ft]c0().
>
>> +    i_ori(p, K0, K0, (1<<14));
>> +
>> +    **p = MTC0(K0, C0_CONFIG, 7);
>> +    (*p)++;
>
>    So, this also won't do.
>
> [...]
>
> WBR, Sergei
>
>