Re: [PATCH] Added missing EHB in mtc0 -> mfc0 sequence.

Hi Dmitry,

On Sat, Jun 15, 2019 at 12:35:39AM +0000, Dmitry Korotin wrote:

Added missing EHB (Execution Hazard Barrier) in mtc0 -> mfc0 sequence.
Mips documentation Volume III (rev 6.03) table 8.1.

It would be good to describe the problem you saw here - ie. mention that without this execution hazard barrier it's possible for the value read back from the KScratch register to be the value from before the mtc0.

Also probably good to mention which CPUs the problem has been seen on.

Information like this can be really useful when making decisions about stable backports, or for others who come across the patch later & just want to figure out why you wrote it.

Signed-off-by: Dmitry Korotin dkorotin@wavecomp.com

arch/mips/mm/tlbex.c | 32 ++++++++++++++++++++++---------- 1 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c index 65b6e85..bf7f131 100644 --- a/arch/mips/mm/tlbex.c +++ b/arch/mips/mm/tlbex.c @@ -391,6 +391,7 @@ static struct work_registers build_get_work_registers(u32 **p) static void build_restore_work_registers(u32 **p) { if (scratch_reg >= 0) {

• uasm_i_ehb(p);
  

  UASM_i_MFC0(p, 1, c0_kscratch(), scratch_reg); return; }

@@ -668,10 +669,12 @@ static void build_restore_pagemask(u32 **p, struct uasm_reloc **r, uasm_i_mtc0(p, 0, C0_PAGEMASK); uasm_il_b(p, r, lid); }

• if (scratch_reg >= 0)
  

• if (scratch_reg >= 0) {
  

• 	uasm_i_ehb(p);
  UASM_i_MFC0(p, 1, c0_kscratch(), scratch_reg);
  

• else
  

• } else {
  UASM_i_LW(p, 1, scratchpad_offset(0), 0);
  

• }
  

  } else { /* Reset default page size */ if (PM_DEFAULT_MASK >> 16) {

@@ -938,10 +941,12 @@ void build_get_pmde64(u32 **p, struct uasm_label **l, struct uasm_reloc **r, uasm_i_jr(p, ptr); if (mode == refill_scratch) {

• 	if (scratch_reg >= 0)
  

• 	if (scratch_reg >= 0) {
  

• 		uasm_i_ehb(p);
  	UASM_i_MFC0(p, 1, c0_kscratch(), scratch_reg);
  

• 	else
  

• 	} else {
  	UASM_i_LW(p, 1, scratchpad_offset(0), 0);
  

• 	}
  

  } else { uasm_i_nop(p); }

@@ -1258,6 +1263,7 @@ struct mips_huge_tlb_info { UASM_i_MTC0(p, odd, C0_ENTRYLO1); /* load it */ if (c0_scratch_reg >= 0) {

• uasm_i_ehb(p);
  

  UASM_i_MFC0(p, scratch, c0_kscratch(), c0_scratch_reg); build_tlb_write_entry(p, l, r, tlb_random); uasm_l_leave(l, *p);

@@ -1603,15 +1609,19 @@ static void build_setup_pgd(void) uasm_i_dinsm(&p, a0, 0, 29, 64 - 29); uasm_l_tlbl_goaround1(&l, p); UASM_i_SLL(&p, a0, a0, 11);

• uasm_i_jr(&p, 31);
  

  UASM_i_MTC0(&p, a0, C0_CONTEXT);

• uasm_i_ehb(&p);
  

• uasm_i_jr(&p, 31);
  

• uasm_i_nop(&p);
  

Could the ehb go in the JR's delay slot here?

} else { /* PGD in c0_KScratch */

• uasm_i_jr(&p, 31);
  

  if (cpu_has_ldpte) UASM_i_MTC0(&p, a0, C0_PWBASE); else UASM_i_MTC0(&p, a0, c0_kscratch(), pgd_reg);

• uasm_i_ehb(&p);
  

• uasm_i_jr(&p, 31);
  

• uasm_i_nop(&p);
  

Likewise here.

} #else #ifdef CONFIG_SMP @@ -1625,13 +1635,15 @@ static void build_setup_pgd(void) UASM_i_LA_mostly(&p, a2, pgdc); UASM_i_SW(&p, a0, uasm_rel_lo(pgdc), a2); #endif /* SMP */

• uasm_i_jr(&p, 31);

/* if pgd_reg is allocated, save PGD also to scratch register */

• if (pgd_reg != -1)

• if (pgd_reg != -1) { UASM_i_MTC0(&p, a0, c0_kscratch(), pgd_reg);

• else

• uasm_i_nop(&p);
  

• uasm_i_ehb(&p);
  

• }
• uasm_i_jr(&p, 31);
• uasm_i_nop(&p);

And here too.

Thanks, Paul