diff mbox series

Added missing EHB in mtc0 -> mfc0 sequence.

Message ID CY4PR22MB02453FCB184A7ED1534D0C2CAFE90@CY4PR22MB0245.namprd22.prod.outlook.com (mailing list archive)
State Superseded
Headers show
Series Added missing EHB in mtc0 -> mfc0 sequence. | expand

Commit Message

Dmitry Korotin June 15, 2019, 12:35 a.m. UTC
Added missing EHB (Execution Hazard Barrier) in mtc0 -> mfc0 sequence.
    Mips documentation Volume III (rev 6.03) table 8.1.

Signed-off-by: Dmitry Korotin <dkorotin@wavecomp.com>
---
 arch/mips/mm/tlbex.c |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)

Comments

Paul Burton June 20, 2019, 10:21 p.m. UTC | #1
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
diff mbox series

Patch

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);
 	} 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);
 	}
 #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);
 #endif
 	if (p >= (u32 *)tlbmiss_handler_setup_pgd_end)
 		panic("tlbmiss_handler_setup_pgd space exceeded");