MIPS: MSA and Huge Page fix

MIPS: MSA and Huge Page fix

[Huang Pei]

correct spelling and adjust commit message to discribe the problem

move patch 2 from V1 into another thread

no function change

[PATCH] MIPS: add missing MSACSR and upper MSA initialization

[Huang Pei]

init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
task on current cpu can leak into current task and cause unpredictable
behavior when MSA context not initialized.

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/kernel/traps.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 38aa07ccdbcc..f8334b63e4c8 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1287,6 +1287,14 @@ static int enable_restore_fp_context(int msa)
 		err = own_fpu_inatomic(1);
 		if (msa && !err) {
 			enable_msa();
+			/* with MSA enabled, userspace can see MSACSR 
+			 * and MSA regs, but the values in them are from
+			 * other task before current task, restore them
+			 * from saved fp/msa context */
+			write_msa_csr(current->thread.fpu.msacsr);
+			/* own_fpu_inatomic(1) just restore low 64bit,
+			 * fix the high 64bit */
+			init_msa_upper();
 			set_thread_flag(TIF_USEDMSA);
 			set_thread_flag(TIF_MSA_CTX_LIVE);
 		}
-- 
2.17.1

[PATCH] Revert "MIPS: Flush wrong invalid FTLB entry for huge page"

[Huang Pei]

This reverts commit 0115f6cbf26663c86496bc56eeea293f85b77897.

The problem fixed in 0115f6cbf26663c86496bc56eeea293f85b77897 is that
it is expected to replace a base page mapping entry in FTLB with a
huge page mapping entry using tlbwi, but doing it is not permitted by
hardware.

The same problem is hit by __update_tlb from do_page_fault first, but it
DOES NOT work it out, and cause another TLB Invalid and give
0115f6cbf26663c86496bc56eeea293f85b77897 the chane to fix it. Let
__update_tlb fix it and remove this extra TLB Invalid exception.

Move the fix from handle_tlb[lms] to __update_tlb, and now huge page
fault path only take 1 TLB Miss plus 1 TLB Invalid, instead of 1 TLB Miss
plus 2 TLB Invalid

Whether fixed by 0115f6cbf26663c86496bc56eeea293f85b77897 or by __update_tlb,
the root cause is TLB Miss handler writes invalid mapping entry into TLB
without checking _PAGE_PRESENT, and this can not be fixed unless letting
CP0 Entrylo0 & Entrylo1 map one page instead of two, like mapping Huge Page

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/mm/tlb-r4k.c | 15 ++++++++++++++-
 arch/mips/mm/tlbex.c   | 25 ++++---------------------
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/arch/mips/mm/tlb-r4k.c b/arch/mips/mm/tlb-r4k.c
index 38e2894d5fa3..cb8afa326b2c 100644
--- a/arch/mips/mm/tlb-r4k.c
+++ b/arch/mips/mm/tlb-r4k.c
@@ -328,6 +328,7 @@ void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t pte)
 	/* this could be a huge page  */
 	if (pmd_huge(*pmdp)) {
 		unsigned long lo;
+		unsigned long entryhi;
 		write_c0_pagemask(PM_HUGE_MASK);
 		ptep = (pte_t *)pmdp;
 		lo = pte_to_entrylo(pte_val(*ptep));
@@ -335,7 +336,19 @@ void __update_tlb(struct vm_area_struct * vma, unsigned long address, pte_t pte)
 		write_c0_entrylo1(lo + (HPAGE_SIZE >> 7));
 
 		mtc0_tlbw_hazard();
-		if (idx < 0)
+		if (idx >= current_cpu_data.tlbsizevtlb) {
+		/* hit in FTLB.
+		 * Invalid it then tlbwr, since FTLB hold only base page*/
+			entryhi = read_c0_entryhi();
+			write_c0_entryhi(MIPS_ENTRYHI_EHINV);
+			tlb_write_indexed();
+			tlbw_use_hazard();
+			write_c0_entryhi(entryhi);
+
+		}
+
+
+		if (idx < 0 || idx >= current_cpu_data.tlbsizevtlb)
 			tlb_write_random();
 		else
 			tlb_write_indexed();
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
index 14f8ba93367f..9c4cd08c00d3 100644
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -762,8 +762,7 @@ static void build_huge_update_entries(u32 **p, unsigned int pte,
 static void build_huge_handler_tail(u32 **p, struct uasm_reloc **r,
 				    struct uasm_label **l,
 				    unsigned int pte,
-				    unsigned int ptr,
-				    unsigned int flush)
+				    unsigned int ptr)
 {
 #ifdef CONFIG_SMP
 	UASM_i_SC(p, pte, 0, ptr);
@@ -772,22 +771,6 @@ static void build_huge_handler_tail(u32 **p, struct uasm_reloc **r,
 #else
 	UASM_i_SW(p, pte, 0, ptr);
 #endif
-	if (cpu_has_ftlb && flush) {
-		BUG_ON(!cpu_has_tlbinv);
-
-		UASM_i_MFC0(p, ptr, C0_ENTRYHI);
-		uasm_i_ori(p, ptr, ptr, MIPS_ENTRYHI_EHINV);
-		UASM_i_MTC0(p, ptr, C0_ENTRYHI);
-		build_tlb_write_entry(p, l, r, tlb_indexed);
-
-		uasm_i_xori(p, ptr, ptr, MIPS_ENTRYHI_EHINV);
-		UASM_i_MTC0(p, ptr, C0_ENTRYHI);
-		build_huge_update_entries(p, pte, ptr);
-		build_huge_tlb_write_entry(p, l, r, pte, tlb_random, 0);
-
-		return;
-	}
-
 	build_huge_update_entries(p, pte, ptr);
 	build_huge_tlb_write_entry(p, l, r, pte, tlb_indexed, 0);
 }
@@ -2278,7 +2261,7 @@ static void build_r4000_tlb_load_handler(void)
 		uasm_l_tlbl_goaround2(&l, p);
 	}
 	uasm_i_ori(&p, wr.r1, wr.r1, (_PAGE_ACCESSED | _PAGE_VALID));
-	build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2, 1);
+	build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2);
 #endif
 
 	uasm_l_nopage_tlbl(&l, p);
@@ -2334,7 +2317,7 @@ static void build_r4000_tlb_store_handler(void)
 	build_tlb_probe_entry(&p);
 	uasm_i_ori(&p, wr.r1, wr.r1,
 		   _PAGE_ACCESSED | _PAGE_MODIFIED | _PAGE_VALID | _PAGE_DIRTY);
-	build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2, 1);
+	build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2);
 #endif
 
 	uasm_l_nopage_tlbs(&l, p);
@@ -2391,7 +2374,7 @@ static void build_r4000_tlb_modify_handler(void)
 	build_tlb_probe_entry(&p);
 	uasm_i_ori(&p, wr.r1, wr.r1,
 		   _PAGE_ACCESSED | _PAGE_MODIFIED | _PAGE_VALID | _PAGE_DIRTY);
-	build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2, 0);
+	build_huge_handler_tail(&p, &r, &l, wr.r1, wr.r2);
 #endif
 
 	uasm_l_nopage_tlbm(&l, p);
-- 
2.17.1

Re: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

[Sergei Shtylyov]

Hello!

On 25.08.2020 7:38, Huang Pei wrote:

> init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
> just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
> MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
> task on current cpu can leak into current task and cause unpredictable
> behavior when MSA context not initialized.
> 
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>   arch/mips/kernel/traps.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index 38aa07ccdbcc..f8334b63e4c8 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -1287,6 +1287,14 @@ static int enable_restore_fp_context(int msa)
>   		err = own_fpu_inatomic(1);
>   		if (msa && !err) {
>   			enable_msa();
> +			/* with MSA enabled, userspace can see MSACSR
> +			 * and MSA regs, but the values in them are from
> +			 * other task before current task, restore them
> +			 * from saved fp/msa context */
> +			write_msa_csr(current->thread.fpu.msacsr);
> +			/* own_fpu_inatomic(1) just restore low 64bit,
> +			 * fix the high 64bit */

    The preferred multi-line comment style is:

/*
  * bla
  * bla
  */

> +			init_msa_upper();
>   			set_thread_flag(TIF_USEDMSA);
>   			set_thread_flag(TIF_MSA_CTX_LIVE);
>   		}

MBR, Sergei

Re: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

[Huang Pei]

On Tue, Aug 25, 2020 at 01:05:31PM +0300, Sergei Shtylyov wrote:

OK, got it, anything else?

From 0f4003eb418737df75cb8be79d4da34b1500f3d7 Mon Sep 17 00:00:00 2001
From: Huang Pei <huangpei@loongson.cn>
Date: Fri, 21 Aug 2020 10:48:40 +0800
Subject: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
task on current cpu can leak into current task and cause unpredictable
behavior when MSA context not initialized.

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/kernel/traps.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 38aa07ccdbcc..e843b38486b8 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1287,6 +1287,16 @@ static int enable_restore_fp_context(int msa)
 		err = own_fpu_inatomic(1);
 		if (msa && !err) {
 			enable_msa();
+			/* with MSA enabled, userspace can see MSACSR 
+			 * and MSA regs, but the values in them are from
+			 * other task before current task, restore them
+			 * from saved fp/msa context
+			 */
+			write_msa_csr(current->thread.fpu.msacsr);
+			/* own_fpu_inatomic(1) just restore low 64bit,
+			 * fix the high 64bit
+			 */
+			init_msa_upper();
 			set_thread_flag(TIF_USEDMSA);
 			set_thread_flag(TIF_MSA_CTX_LIVE);
 		}
-- 
2.17.1

> Hello!
> 
> On 25.08.2020 7:38, Huang Pei wrote:
> 
> > init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
> > just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
> > MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
> > task on current cpu can leak into current task and cause unpredictable
> > behavior when MSA context not initialized.
> > 
> > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > ---
> >   arch/mips/kernel/traps.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> > index 38aa07ccdbcc..f8334b63e4c8 100644
> > --- a/arch/mips/kernel/traps.c
> > +++ b/arch/mips/kernel/traps.c
> > @@ -1287,6 +1287,14 @@ static int enable_restore_fp_context(int msa)
> >   		err = own_fpu_inatomic(1);
> >   		if (msa && !err) {
> >   			enable_msa();
> > +			/* with MSA enabled, userspace can see MSACSR
> > +			 * and MSA regs, but the values in them are from
> > +			 * other task before current task, restore them
> > +			 * from saved fp/msa context */
> > +			write_msa_csr(current->thread.fpu.msacsr);
> > +			/* own_fpu_inatomic(1) just restore low 64bit,
> > +			 * fix the high 64bit */
> 
>    The preferred multi-line comment style is:
> 
> /*
>  * bla
>  * bla
>  */
> 
> > +			init_msa_upper();
> >   			set_thread_flag(TIF_USEDMSA);
> >   			set_thread_flag(TIF_MSA_CTX_LIVE);
> >   		}
> 
> MBR, Sergei

Re: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

[Sergei Shtylyov]

On 8/26/20 1:10 PM, Huang Pei wrote:

> From 0f4003eb418737df75cb8be79d4da34b1500f3d7 Mon Sep 17 00:00:00 2001
> From: Huang Pei <huangpei@loongson.cn>
> Date: Fri, 21 Aug 2020 10:48:40 +0800
> Subject: [PATCH] MIPS: add missing MSACSR and upper MSA initialization
> 
> init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
> just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
> MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
> task on current cpu can leak into current task and cause unpredictable
> behavior when MSA context not initialized.
> 
> Signed-off-by: Huang Pei <huangpei@loongson.cn>
> ---
>  arch/mips/kernel/traps.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index 38aa07ccdbcc..e843b38486b8 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -1287,6 +1287,16 @@ static int enable_restore_fp_context(int msa)
>  		err = own_fpu_inatomic(1);
>  		if (msa && !err) {
>  			enable_msa();
> +			/* with MSA enabled, userspace can see MSACSR 
> +			 * and MSA regs, but the values in them are from
> +			 * other task before current task, restore them
> +			 * from saved fp/msa context
> +			 */
> +			write_msa_csr(current->thread.fpu.msacsr);
> +			/* own_fpu_inatomic(1) just restore low 64bit,
> +			 * fix the high 64bit
> +			 */

   This comment style is acceptable only for the networking code, all the other code
should have the 1st comment line empty.

[...]

MBR, Sergei

Re: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

[Huang Pei]

On Wed, Aug 26, 2020 at 04:17:43PM +0300, Sergei Shtylyov wrote:

From 218f23077352a7befd2aaad3fa46b93112d4051b Mon Sep 17 00:00:00 2001
From: Huang Pei <huangpei@loongson.cn>
Date: Fri, 21 Aug 2020 10:48:40 +0800
Subject: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
task on current cpu can leak into current task and cause unpredictable
behavior when MSA context not initialized.

Signed-off-by: Huang Pei <huangpei@loongson.cn>
---
 arch/mips/kernel/traps.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 38aa07ccdbcc..cf788591f091 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1287,6 +1287,18 @@ static int enable_restore_fp_context(int msa)
 		err = own_fpu_inatomic(1);
 		if (msa && !err) {
 			enable_msa();
+			/*
+			 * with MSA enabled, userspace can see MSACSR
+			 * and MSA regs, but the values in them are from
+			 * other task before current task, restore them
+			 * from saved fp/msa context
+			 */
+			write_msa_csr(current->thread.fpu.msacsr);
+			/*
+			 * own_fpu_inatomic(1) just restore low 64bit,
+			 * fix the high 64bit
+			 */
+			init_msa_upper();
 			set_thread_flag(TIF_USEDMSA);
 			set_thread_flag(TIF_MSA_CTX_LIVE);
 		}
-- 
2.17.1

> On 8/26/20 1:10 PM, Huang Pei wrote:
> 
> > From 0f4003eb418737df75cb8be79d4da34b1500f3d7 Mon Sep 17 00:00:00 2001
> > From: Huang Pei <huangpei@loongson.cn>
> > Date: Fri, 21 Aug 2020 10:48:40 +0800
> > Subject: [PATCH] MIPS: add missing MSACSR and upper MSA initialization
> > 
> > init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
> > just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
> > MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
> > task on current cpu can leak into current task and cause unpredictable
> > behavior when MSA context not initialized.
> > 
> > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> > ---
> >  arch/mips/kernel/traps.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> > index 38aa07ccdbcc..e843b38486b8 100644
> > --- a/arch/mips/kernel/traps.c
> > +++ b/arch/mips/kernel/traps.c
> > @@ -1287,6 +1287,16 @@ static int enable_restore_fp_context(int msa)
> >  		err = own_fpu_inatomic(1);
> >  		if (msa && !err) {
> >  			enable_msa();
> > +			/* with MSA enabled, userspace can see MSACSR 
> > +			 * and MSA regs, but the values in them are from
> > +			 * other task before current task, restore them
> > +			 * from saved fp/msa context
> > +			 */
> > +			write_msa_csr(current->thread.fpu.msacsr);
> > +			/* own_fpu_inatomic(1) just restore low 64bit,
> > +			 * fix the high 64bit
> > +			 */
> 
>    This comment style is acceptable only for the networking code, all the other code
> should have the 1st comment line empty.
> 
> [...]
> 
> MBR, Sergei

Re: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

[Thomas Bogendoerfer]

On Thu, Aug 27, 2020 at 07:41:31PM +0800, Huang Pei wrote:
> On Wed, Aug 26, 2020 at 04:17:43PM +0300, Sergei Shtylyov wrote:
> 
> >From 218f23077352a7befd2aaad3fa46b93112d4051b Mon Sep 17 00:00:00 2001
> From: Huang Pei <huangpei@loongson.cn>
> Date: Fri, 21 Aug 2020 10:48:40 +0800
> Subject: [PATCH] MIPS: add missing MSACSR and upper MSA initialization
> 
> init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
> just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
> MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
> task on current cpu can leak into current task and cause unpredictable
> behavior when MSA context not initialized.
> 
> Signed-off-by: Huang Pei <huangpei@loongson.cn>

Please submit in a new email and not in respone to another mail, and
add a Fixes tag.  Otherwise the fix looks correct to me. Thank you.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH] MIPS: add missing MSACSR and upper MSA initialization

[Huang Pei]

On Fri, Aug 28, 2020 at 09:41:18AM +0200, Thomas Bogendoerfer wrote:
> On Thu, Aug 27, 2020 at 07:41:31PM +0800, Huang Pei wrote:
> > On Wed, Aug 26, 2020 at 04:17:43PM +0300, Sergei Shtylyov wrote:
> > 
> > >From 218f23077352a7befd2aaad3fa46b93112d4051b Mon Sep 17 00:00:00 2001
> > From: Huang Pei <huangpei@loongson.cn>
> > Date: Fri, 21 Aug 2020 10:48:40 +0800
> > Subject: [PATCH] MIPS: add missing MSACSR and upper MSA initialization
> > 
> > init_fp_ctx just initialize the fp/msa context, and own_fp_inatomic
> > just restore FCSR and 64bit FP regs from it, but miss MSACSR and upper
> > MSA regs for MSA, so MSACSR and MSA upper regs's value from previous
> > task on current cpu can leak into current task and cause unpredictable
> > behavior when MSA context not initialized.
> > 
> > Signed-off-by: Huang Pei <huangpei@loongson.cn>
> 
> Please submit in a new email and not in respone to another mail, and
> add a Fixes tag.  Otherwise the fix looks correct to me. Thank you.
> 
> Thomas.
> 
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]

Got it, send this patch alone in a new mail

Huang Pei,