* [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
@ 2007-07-19 14:19 Daniel Laird
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Laird @ 2007-07-19 14:19 UTC (permalink / raw)
To: linux-mips
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;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
@ 2007-07-19 15:24 Daniel Laird
2007-07-19 15:28 ` Jan-Benedict Glaw
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Daniel Laird @ 2007-07-19 15:24 UTC (permalink / raw)
To: linux-mips
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;
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
2007-07-19 15:24 Daniel Laird
@ 2007-07-19 15:28 ` Jan-Benedict Glaw
2007-07-19 15:31 ` Maciej W. Rozycki
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Jan-Benedict Glaw @ 2007-07-19 15:28 UTC (permalink / raw)
To: Daniel Laird; +Cc: linux-mips
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
2007-07-19 15:24 Daniel Laird
2007-07-19 15:28 ` Jan-Benedict Glaw
@ 2007-07-19 15:31 ` Maciej W. Rozycki
2007-07-19 16:28 ` Thiemo Seufer
2007-07-19 17:32 ` Sergei Shtylyov
3 siblings, 0 replies; 8+ messages in thread
From: Maciej W. Rozycki @ 2007-07-19 15:31 UTC (permalink / raw)
To: Daniel Laird; +Cc: linux-mips
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
2007-07-19 15:24 Daniel Laird
2007-07-19 15:28 ` Jan-Benedict Glaw
2007-07-19 15:31 ` Maciej W. Rozycki
@ 2007-07-19 16:28 ` Thiemo Seufer
2007-07-19 17:32 ` Sergei Shtylyov
3 siblings, 0 replies; 8+ messages in thread
From: Thiemo Seufer @ 2007-07-19 16:28 UTC (permalink / raw)
To: Daniel Laird; +Cc: linux-mips
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
2007-07-19 15:24 Daniel Laird
` (2 preceding siblings ...)
2007-07-19 16:28 ` Thiemo Seufer
@ 2007-07-19 17:32 ` Sergei Shtylyov
2007-07-19 20:16 ` danieljlaird
3 siblings, 1 reply; 8+ messages in thread
From: Sergei Shtylyov @ 2007-07-19 17:32 UTC (permalink / raw)
To: Daniel Laird; +Cc: linux-mips
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
2007-07-19 17:32 ` Sergei Shtylyov
@ 2007-07-19 20:16 ` danieljlaird
2007-07-19 20:16 ` danieljlaird
0 siblings, 1 reply; 8+ messages in thread
From: danieljlaird @ 2007-07-19 20:16 UTC (permalink / raw)
To: linux-mips
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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550
2007-07-19 20:16 ` danieljlaird
@ 2007-07-19 20:16 ` danieljlaird
0 siblings, 0 replies; 8+ messages in thread
From: danieljlaird @ 2007-07-19 20:16 UTC (permalink / raw)
To: linux-mips
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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2007-07-19 20:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-19 14:19 [PATCH] Fix known HW bug with MIPS core on NXP/Philips PNX8550 Daniel Laird
-- strict thread matches above, loose matches on Subject: below --
2007-07-19 15:24 Daniel Laird
2007-07-19 15:28 ` Jan-Benedict Glaw
2007-07-19 15:31 ` Maciej W. Rozycki
2007-07-19 16:28 ` Thiemo Seufer
2007-07-19 17:32 ` Sergei Shtylyov
2007-07-19 20:16 ` danieljlaird
2007-07-19 20:16 ` danieljlaird
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox