Avoid potential hazard on Context register
PATCH 1/1 Avoid potential hazard on Context register
set_saved_sp reads Context register. Avoid reading stale value from earlier incomplete write
---
arch/mips/kernel/head.S | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/mips/kernel/head.S +1 -0
@@ -191,6 +191,7 @@ NESTED(kernel_entry, 16, sp) # kernel entry point /* Set the SP after an empty pt_regs. */ PTR_LI sp, _THREAD_SIZE - 32 - PT_SIZE PTR_ADDU sp, $28 + back_to_back_c0_hazard set_saved_sp sp, t0, t1 PTR_SUBU sp, 4 * SZREG # init stack pointer
Quoted 2 lines · Chris Dearman
set_saved_spreads Context register. Avoid reading stale value from earlier incomplete write
Which is executed fairly frequently. So it'll be cheaper to fix the issue in the writers. Will post patch in a few minutes.
I'm curious, did you actually manage to trigger this one? The time between the instructions should be fairly long but then again the 34K has been good for a few surprises!
Ralf
There is no hazard barrier between writes to c0_context and subsequent read accesses. This is a fairly theoretical hole as c0_context is only written on CPU bootup and other, unrelated code will almost certainly execute a hazard barrier somewhen between the write and read access. Even if not, the window is probably in the thousands of cycles so likely too large to actually consistute a pipeline hazard.
Reported and initial patch by Chris Dearman <[email protected]>.
arch/mips/include/asm/mmu_context.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
arch/mips/include/asm/mmu_context.h +3 -0
@@ -16,6 +16,7 @@#include <linux/smp.h> #include <linux/slab.h> #include <asm/cacheflush.h> +#include <asm/hazards.h> #include <asm/tlbflush.h> #ifdef CONFIG_MIPS_MT_SMTC #include <asm/mipsmtregs.h>
@@ -36,11 +37,13 @@ extern unsigned long pgd_current[];#ifdef CONFIG_32BIT #define TLBMISS_HANDLER_SETUP () \ write_c0_context((unsigned long) smp_processor_id() << 25); \ + back_to_back_c0_hazard(); \ TLBMISS_HANDLER_SETUP_PGD(swapper_pg_dir)#endif #ifdef CONFIG_64BIT #define TLBMISS_HANDLER_SETUP () \ write_c0_context((unsigned long) smp_processor_id() << 26); \ + back_to_back_c0_hazard(); \ TLBMISS_HANDLER_SETUP_PGD(swapper_pg_dir)#endif
Quoted 3 lines · Ralf Baechle
There is no hazard barrier between writes to
c0_contextand subsequent read accesses. This is a fairly theoretical hole asc0_contextis only written on CPU bootup and other, unrelated code will almost certainly
It was actually in the bootup code where I saw the problem, and this patch doesn't deal with that case:
Quoted 7 lines
MTC0 zero,
CP0_CONTEXT# clear context register
PTR_LA$28,init_thread_union
/* Set the SP after an emptypt_regs. */
PTR_LIsp,_THREAD_SIZE- 32 -PT_SIZE
PTR_ADDUsp, $28
back_to_back_c0_hazard
set_saved_spsp, t0, t1
The problem I observed is that the Context valuse used by set_saved_sp is whatever it inherits from YAMON.
Chris
Signature · 2 lines
Chris Dearman Desk: +1 408 530 5092 Cell: +1 650 224 8603 MIPS Technologies Inc 955 East Arques Ave, Sunnyvale CA 94085
Quoted 7 lines · Chris Dearman
Quoted 3 lines · Ralf Baechle
There is no hazard barrier between writes to
c0_contextand subsequent read accesses. This is a fairly theoretical hole asc0_contextis only written on CPU bootup and other, unrelated code will almost certainlyIt was actually in the bootup code where I saw the problem, and this patch doesn't deal with that case:
Quoted 8 lines
Quoted 5 lines
MTC0 zero,
CP0_CONTEXT# clear context
registerPTR_LA$28,init_thread_union/* Set
the SP after an emptypt_regs. */PTR_LIsp,_THREAD_SIZE- 32 -PT_SIZEPTR_ADDUsp, $28back_to_back_c0_hazardset_saved_spsp, t0, t1The problem I observed is that the Context valuse used by
set_saved_spis whatever it inherits from YAMON.
So we need a double hazard barrier like below.
Ralf
There is no hazard barrier between writes to c0_context and subsequent read accesses. This is a fairly theoretical hole as c0_context is only written on CPU bootup and other, unrelated code will almost certainly execute a hazard barrier somewhen between the write and read access. Even if not, the window is probably in the thousands of cycles so likely too large to actually consistute a pipeline hazard.
Reported and initial patch by Chris Dearman <[email protected]>.
arch/mips/include/asm/mmu_context.h | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
arch/mips/include/asm/mmu_context.h +5 -0
@@ -16,6 +16,7 @@#include <linux/smp.h> #include <linux/slab.h> #include <asm/cacheflush.h> +#include <asm/hazards.h> #include <asm/tlbflush.h> #ifdef CONFIG_MIPS_MT_SMTC #include <asm/mipsmtregs.h>
@@ -35,12 +36,16 @@ extern unsigned long pgd_current[];#ifdef CONFIG_32BIT #define TLBMISS_HANDLER_SETUP () \ + back_to_back_c0_hazard(); \ write_c0_context((unsigned long) smp_processor_id() << 25); \ + back_to_back_c0_hazard(); \ TLBMISS_HANDLER_SETUP_PGD(swapper_pg_dir)#endif #ifdef CONFIG_64BIT #define TLBMISS_HANDLER_SETUP () \ + back_to_back_c0_hazard(); \ write_c0_context((unsigned long) smp_processor_id() << 26); \ + back_to_back_c0_hazard(); \ TLBMISS_HANDLER_SETUP_PGD(swapper_pg_dir)#endif
Quoted 3 lines · Ralf Baechle
I'm curious, did you actually manage to trigger this one? The time between the instructions should be fairly long but then again the 34K has been good for a few surprises!
Yes... It actually showed up in 74k silicon at particular clock frequencies. The dual issue pipeline does make it susceptible to these "theoretical" problems ;)
Chris
Signature · 2 lines
Chris Dearman Desk: +1 408 530 5092 Cell: +1 650 224 8603 MIPS Technologies Inc 955 East Arques Ave, Sunnyvale CA 94085