Fix known HW bug with MIPS core on NXP/Philips PNX8550
Update Patch
Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
---tlb-r4k.c | 4 +++tlbex.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
---
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +3 -0
@@ -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
arch/mips/mm/tlb-r4k.c (working copy) renamed from arch/mips/mm/tlb-r4k.c (revision 8) +4 -0
@@ -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;
Quoted 3 lines · Daniel Laird
Update Patch
Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
Both versions are whitespace damaged...
Quoted 5 lines
Index:
linux-2.6.22.1/arch/mips/mm/tlb-r4k.c
===================================================================
arch/mips/mm/tlb-r4k.c (working copy) line 465
Quoted 10 lines
arch/mips/mm/tlb-r4k.c (working copy) renamed from arch/mips/mm/tlb-r4k.c (revision 8) +4
-0@@ -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
Signature · 3 lines
Jan-Benedict Glaw [email protected] +49-172-7608481 Signature of: Wenn ich wach bin, träume ich. the second :
arch/mips/mm/tlbex.c (working copy) line 443
Quoted 10 lines · Daniel Laird
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +3
-0@@ -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
Quoted 16 lines · Daniel Laird
Update Patch
Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
Signed-off-by Daniel Laird <[email protected]>---
tlb-r4k.c| 4 +++tlbex.c| 71
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++2 files changed, 75 insertions(+)---
Index:linux-2.6.22.1/arch/mips/mm/tlbex.c
===================================================================
arch/mips/mm/tlbex.c (working copy) line 440
Quoted 6 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +3
-0@@ -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]
Quoted 6 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +5
-0+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 \
arch/mips/mm/tlbex.c (working copy)
Quoted 10 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +10
-0+ | (_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]
arch/mips/mm/tlbex.c (working copy) line 1335
Quoted 9 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +4
-0@@ -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.
Quoted 7 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +0
-0i_eret(&p); /* return from trap */ #ifdef CONFIG_64BITIndex:
linux-2.6.22.1/arch/mips/mm/tlb-r4k.c
===================================================================
arch/mips/mm/tlb-r4k.c (working copy) line 463
Quoted 8 lines
arch/mips/mm/tlb-r4k.c (working copy) renamed from arch/mips/mm/tlb-r4k.c (revision 8) +4
-0@@ -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
Hello.
Quoted 1 line · Daniel Laird
Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
Would've been god to somehow document it too...
Quoted 6 lines
Signed-off-by Daniel Laird <[email protected]>Index:
linux-2.6.22.1/arch/mips/mm/tlbex.c
===================================================================arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +0
-0
[...]
Quoted 28 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +9
-0@@ -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 \
arch/mips/mm/tlbex.c (working copy) line 770
Quoted 10 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +10
-0+ | (_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?
Quoted 8 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +8
-0+ /* 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); +
arch/mips/mm/tlbex.c (working copy) line 788
Quoted 10 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +10
-0+ /* 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().
arch/mips/mm/tlbex.c (working copy) line 792
Quoted 4 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +4
-0+ i_ori(p, K0, K0, (1<<14)); + + **p = MTC0(K0, C0_CONFIG, 7); + (*p)++;
So, this also won't do.
[...]
WBR, Sergei
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" <[email protected]>
To: "Daniel Laird" <[email protected]>
Cc: <[email protected]>
Sent: Thursday, July 19, 2007 6:32 PM
Subject: Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
Quoted 95 lines
Hello.
Quoted 1 line · Daniel Laird
Fix known bug with MIPS core when using TLB on NXP/Philips PNX8550
Would've been god to somehow document it too...
Quoted 6 lines
Signed-off-by Daniel Laird <[email protected]>Index:
linux-2.6.22.1/arch/mips/mm/tlbex.c
===================================================================arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +0
-0[...]
Quoted 28 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +9
-0@@ -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 \arch/mips/mm/tlbex.c (working copy) line 770
Quoted 10 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +10
-0+ | (_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 macroM()doing exactly what these two macros
are doing?Quoted 8 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +8
-0+ /* 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); +arch/mips/mm/tlbex.c (working copy) line 788
Quoted 10 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +10
-0+ /* 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 asI_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().arch/mips/mm/tlbex.c (working copy) line 792
Quoted 4 lines
arch/mips/mm/tlbex.c (working copy) renamed from arch/mips/mm/tlbex.c (revision 8) +4
-0+ i_ori(p, K0, K0, (1<<14)); + + **p = MTC0(K0, C0_CONFIG, 7); + (*p)++;So, this also won't do.
[...]
WBR, Sergei