linux-mips.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Added missing EHB in mtc0 -> mfc0 sequence.
@ 2019-06-15  0:35 Dmitry Korotin
  2019-06-20 22:21 ` Paul Burton
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Korotin @ 2019-06-15  0:35 UTC (permalink / raw)
  To: Paul Burton; +Cc: linux-mips, stable, Dmitry Korotin

    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(-)

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");
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Added missing EHB in mtc0 -> mfc0 sequence.
  2019-06-15  0:35 [PATCH] Added missing EHB in mtc0 -> mfc0 sequence Dmitry Korotin
@ 2019-06-20 22:21 ` Paul Burton
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Burton @ 2019-06-20 22:21 UTC (permalink / raw)
  To: Dmitry Korotin; +Cc: Paul Burton, linux-mips, stable

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Added missing EHB in mtc0 -> mfc0 sequence.
  2019-06-24 19:05 Dmitry Korotin
@ 2019-06-24 21:15 ` Paul Burton
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Burton @ 2019-06-24 21:15 UTC (permalink / raw)
  To: Dmitry Korotin
  Cc: Paul Burton, linux-mips, stable, Dmitry Korotin, linux-mips

Hello,

Dmitry Korotin wrote:
> Added missing EHB (Execution Hazard Barrier) in mtc0 -> mfc0 sequence.
> 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.
> 
> Reproduciable on P5600 & P6600.
> 
> Mips documentation Volume III (rev 6.03) table 8.1.
> 
> Signed-off-by: Dmitry Korotin <dkorotin@wavecomp.com>

Applied to mips-fixes.

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] Added missing EHB in mtc0 -> mfc0 sequence.
@ 2019-06-24 19:05 Dmitry Korotin
  2019-06-24 21:15 ` Paul Burton
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Korotin @ 2019-06-24 19:05 UTC (permalink / raw)
  To: Paul Burton; +Cc: linux-mips, stable, Dmitry Korotin

Added missing EHB (Execution Hazard Barrier) in mtc0 -> mfc0 sequence.
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.

Reproduciable on P5600 & P6600.

Mips documentation Volume III (rev 6.03) table 8.1.

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

diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index 65b6e85..144ceb0 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,17 @@ 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_jr(&p, 31);
+		uasm_i_ehb(&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_jr(&p, 31);
+		uasm_i_ehb(&p);
 	}
 #else
 #ifdef CONFIG_SMP
@@ -1625,13 +1633,16 @@ 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_jr(&p, 31);
+		uasm_i_ehb(&p);
+	} else {
+		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");
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-06-24 21:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-15  0:35 [PATCH] Added missing EHB in mtc0 -> mfc0 sequence Dmitry Korotin
2019-06-20 22:21 ` Paul Burton
2019-06-24 19:05 Dmitry Korotin
2019-06-24 21:15 ` Paul Burton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).