[PATCH 0/3 3.19] HTW fixes

[PATCH 0/3 3.19] HTW fixes

[Markos Chandras]

Hi,
	
A few HTW fixes for 3.19 and stable branches.

Markos Chandras (3):
  MIPS: asm: pgtable: Add c0 hazards on HTW start/stop sequences
  MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}
  MIPS: asm: pgtable: Prevent HTW race when updating PTEs

 arch/mips/include/asm/cpu-info.h    |  5 +++++
 arch/mips/include/asm/mmu_context.h |  7 ++++++-
 arch/mips/include/asm/pgtable.h     | 37 ++++++++++++++++++++++---------------
 arch/mips/kernel/cpu-probe.c        |  4 +++-
 4 files changed, 36 insertions(+), 17 deletions(-)

-- 
2.2.2

[PATCH 0/3 3.19] HTW fixes

[Markos Chandras]

Hi,
	
A few HTW fixes for 3.19 and stable branches.

Markos Chandras (3):
  MIPS: asm: pgtable: Add c0 hazards on HTW start/stop sequences
  MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}
  MIPS: asm: pgtable: Prevent HTW race when updating PTEs

 arch/mips/include/asm/cpu-info.h    |  5 +++++
 arch/mips/include/asm/mmu_context.h |  7 ++++++-
 arch/mips/include/asm/pgtable.h     | 37 ++++++++++++++++++++++---------------
 arch/mips/kernel/cpu-probe.c        |  4 +++-
 4 files changed, 36 insertions(+), 17 deletions(-)

-- 
2.2.2

[PATCH 1/3] MIPS: asm: pgtable: Add c0 hazards on HTW start/stop sequences

[Markos Chandras]

When we use htw_{start,stop}() outside of htw_reset(), we need
to ensure that c0 changes have been propagated properly before
we attempt to continue with subsequence memory operations.

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/include/asm/pgtable.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index d6d1928539b1..7f7c558de9fc 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -99,16 +99,20 @@ extern void paging_init(void);
 
 #define htw_stop()							\
 do {									\
-	if (cpu_has_htw)						\
+	if (cpu_has_htw) {						\
 		write_c0_pwctl(read_c0_pwctl() &			\
 			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
+		back_to_back_c0_hazard();				\
+	}								\
 } while(0)
 
 #define htw_start()							\
 do {									\
-	if (cpu_has_htw)						\
+	if (cpu_has_htw) {						\
 		write_c0_pwctl(read_c0_pwctl() |			\
 			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
+		back_to_back_c0_hazard();				\
+	}								\
 } while(0)
 
 
@@ -116,9 +120,7 @@ do {									\
 do {									\
 	if (cpu_has_htw) {						\
 		htw_stop();						\
-		back_to_back_c0_hazard();				\
 		htw_start();						\
-		back_to_back_c0_hazard();				\
 	}								\
 } while(0)
 
-- 
2.2.2

[PATCH 1/3] MIPS: asm: pgtable: Add c0 hazards on HTW start/stop sequences

[Markos Chandras]

When we use htw_{start,stop}() outside of htw_reset(), we need
to ensure that c0 changes have been propagated properly before
we attempt to continue with subsequence memory operations.

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/include/asm/pgtable.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index d6d1928539b1..7f7c558de9fc 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -99,16 +99,20 @@ extern void paging_init(void);
 
 #define htw_stop()							\
 do {									\
-	if (cpu_has_htw)						\
+	if (cpu_has_htw) {						\
 		write_c0_pwctl(read_c0_pwctl() &			\
 			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
+		back_to_back_c0_hazard();				\
+	}								\
 } while(0)
 
 #define htw_start()							\
 do {									\
-	if (cpu_has_htw)						\
+	if (cpu_has_htw) {						\
 		write_c0_pwctl(read_c0_pwctl() |			\
 			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
+		back_to_back_c0_hazard();				\
+	}								\
 } while(0)
 
 
@@ -116,9 +120,7 @@ do {									\
 do {									\
 	if (cpu_has_htw) {						\
 		htw_stop();						\
-		back_to_back_c0_hazard();				\
 		htw_start();						\
-		back_to_back_c0_hazard();				\
 	}								\
 } while(0)
 
-- 
2.2.2

[PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

activate_mm() and switch_mm() call get_new_mmu_context() which in turn
can enable the HTW before the entryhi is changed with the new ASID.
Since the latter will enable the HTW in local_flush_tlb_all(),
then there is a small timing window where the HTW is running with the
new ASID but with an old pgd since the TLBMISS_HANDLER_SETUP_PGD
hasn't assigned a new one yet. In order to prevent that, we introduce a
simple htw counter to avoid starting HTW accidentally due to nested
htw_{start,stop}() sequences. Moreover, since various IPI calls can
enforce TLB flushing operations on a different core, such an operation
may interrupt another htw_{stop,start} in progress leading inconsistent
updates of the htw_seq variable. In order to avoid that, we disable the
interrupts whenever we update that variable.

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/include/asm/cpu-info.h    |  5 +++++
 arch/mips/include/asm/mmu_context.h |  7 ++++++-
 arch/mips/include/asm/pgtable.h     | 17 ++++++++++++++---
 arch/mips/kernel/cpu-probe.c        |  4 +++-
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index a6c9ccb33c5c..c3f4f2d2e108 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -84,6 +84,11 @@ struct cpuinfo_mips {
 	 * (shifted by _CACHE_SHIFT)
 	 */
 	unsigned int		writecombine;
+	/*
+	 * Simple counter to prevent enabling HTW in nested
+	 * htw_start/htw_stop calls
+	 */
+	unsigned int		htw_seq;
 } __attribute__((aligned(SMP_CACHE_BYTES)));
 
 extern struct cpuinfo_mips cpu_data[];
diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
index 2f82568a3ee4..bc01579a907a 100644
--- a/arch/mips/include/asm/mmu_context.h
+++ b/arch/mips/include/asm/mmu_context.h
@@ -25,7 +25,6 @@ do {									\
 	if (cpu_has_htw) {						\
 		write_c0_pwbase(pgd);					\
 		back_to_back_c0_hazard();				\
-		htw_reset();						\
 	}								\
 } while (0)
 
@@ -142,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	unsigned long flags;
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
@@ -154,6 +154,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	 */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -180,6 +181,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, cpu);
 
@@ -189,6 +191,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 	/* mark mmu ownership change */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -203,6 +206,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 	unsigned long flags;
 
 	local_irq_save(flags);
+	htw_stop();
 
 	if (cpumask_test_cpu(cpu, mm_cpumask(mm)))  {
 		get_new_mmu_context(mm, cpu);
@@ -211,6 +215,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 		/* will get a new context next time */
 		cpu_context(cpu, mm) = 0;
 	}
+	htw_start();
 	local_irq_restore(flags);
 }
 
diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index 7f7c558de9fc..d2c7e9e7447e 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -99,19 +99,30 @@ extern void paging_init(void);
 
 #define htw_stop()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
+		local_irq_save(flags);					\
+		raw_current_cpu_data.htw_seq++;				\
 		write_c0_pwctl(read_c0_pwctl() &			\
 			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
 		back_to_back_c0_hazard();				\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
 #define htw_start()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
-		write_c0_pwctl(read_c0_pwctl() |			\
-			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
-		back_to_back_c0_hazard();				\
+		local_irq_save(flags);					\
+		if (!--raw_current_cpu_data.htw_seq) {			\
+			write_c0_pwctl(read_c0_pwctl() |		\
+				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
+			back_to_back_c0_hazard();			\
+		}							\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index dc49cf30c2db..5d6e59f20750 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
 	if (config3 & MIPS_CONF3_MSA)
 		c->ases |= MIPS_ASE_MSA;
 	/* Only tested on 32-bit cores */
-	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
+	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
+		c->htw_seq = 0;
 		c->options |= MIPS_CPU_HTW;
+	}
 
 	return config3 & MIPS_CONF_M;
 }
-- 
2.2.2

[PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

activate_mm() and switch_mm() call get_new_mmu_context() which in turn
can enable the HTW before the entryhi is changed with the new ASID.
Since the latter will enable the HTW in local_flush_tlb_all(),
then there is a small timing window where the HTW is running with the
new ASID but with an old pgd since the TLBMISS_HANDLER_SETUP_PGD
hasn't assigned a new one yet. In order to prevent that, we introduce a
simple htw counter to avoid starting HTW accidentally due to nested
htw_{start,stop}() sequences. Moreover, since various IPI calls can
enforce TLB flushing operations on a different core, such an operation
may interrupt another htw_{stop,start} in progress leading inconsistent
updates of the htw_seq variable. In order to avoid that, we disable the
interrupts whenever we update that variable.

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/include/asm/cpu-info.h    |  5 +++++
 arch/mips/include/asm/mmu_context.h |  7 ++++++-
 arch/mips/include/asm/pgtable.h     | 17 ++++++++++++++---
 arch/mips/kernel/cpu-probe.c        |  4 +++-
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index a6c9ccb33c5c..c3f4f2d2e108 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -84,6 +84,11 @@ struct cpuinfo_mips {
 	 * (shifted by _CACHE_SHIFT)
 	 */
 	unsigned int		writecombine;
+	/*
+	 * Simple counter to prevent enabling HTW in nested
+	 * htw_start/htw_stop calls
+	 */
+	unsigned int		htw_seq;
 } __attribute__((aligned(SMP_CACHE_BYTES)));
 
 extern struct cpuinfo_mips cpu_data[];
diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
index 2f82568a3ee4..bc01579a907a 100644
--- a/arch/mips/include/asm/mmu_context.h
+++ b/arch/mips/include/asm/mmu_context.h
@@ -25,7 +25,6 @@ do {									\
 	if (cpu_has_htw) {						\
 		write_c0_pwbase(pgd);					\
 		back_to_back_c0_hazard();				\
-		htw_reset();						\
 	}								\
 } while (0)
 
@@ -142,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	unsigned long flags;
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
@@ -154,6 +154,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	 */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -180,6 +181,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, cpu);
 
@@ -189,6 +191,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 	/* mark mmu ownership change */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -203,6 +206,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 	unsigned long flags;
 
 	local_irq_save(flags);
+	htw_stop();
 
 	if (cpumask_test_cpu(cpu, mm_cpumask(mm)))  {
 		get_new_mmu_context(mm, cpu);
@@ -211,6 +215,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 		/* will get a new context next time */
 		cpu_context(cpu, mm) = 0;
 	}
+	htw_start();
 	local_irq_restore(flags);
 }
 
diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index 7f7c558de9fc..d2c7e9e7447e 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -99,19 +99,30 @@ extern void paging_init(void);
 
 #define htw_stop()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
+		local_irq_save(flags);					\
+		raw_current_cpu_data.htw_seq++;				\
 		write_c0_pwctl(read_c0_pwctl() &			\
 			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
 		back_to_back_c0_hazard();				\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
 #define htw_start()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
-		write_c0_pwctl(read_c0_pwctl() |			\
-			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
-		back_to_back_c0_hazard();				\
+		local_irq_save(flags);					\
+		if (!--raw_current_cpu_data.htw_seq) {			\
+			write_c0_pwctl(read_c0_pwctl() |		\
+				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
+			back_to_back_c0_hazard();			\
+		}							\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index dc49cf30c2db..5d6e59f20750 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
 	if (config3 & MIPS_CONF3_MSA)
 		c->ases |= MIPS_ASE_MSA;
 	/* Only tested on 32-bit cores */
-	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
+	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
+		c->htw_seq = 0;
 		c->options |= MIPS_CPU_HTW;
+	}
 
 	return config3 & MIPS_CONF_M;
 }
-- 
2.2.2

[PATCH 3/3] MIPS: asm: pgtable: Prevent HTW race when updating PTEs

[Markos Chandras]

Whenever we modify a page table entry, we need to ensure that the HTW
will not fetch a stable entry. And for that to happen we need to ensure
that HTW is stopped before we modify the said entry otherwise the HTW
may already be in the process of reading that entry and fetching the
old information. As a result of which, we replace the htw_reset() calls
with htw_{stop,start} in more appropriate places. This also removes the
remaining users of htw_reset() and as a result we drop that macro

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/include/asm/pgtable.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index d2c7e9e7447e..4f92607ce36e 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -127,14 +127,6 @@ do {									\
 } while(0)
 
 
-#define htw_reset()							\
-do {									\
-	if (cpu_has_htw) {						\
-		htw_stop();						\
-		htw_start();						\
-	}								\
-} while(0)
-
 extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
 	pte_t pteval);
 
@@ -166,12 +158,13 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *pt
 {
 	pte_t null = __pte(0);
 
+	htw_stop();
 	/* Preserve global status for the pair */
 	if (ptep_buddy(ptep)->pte_low & _PAGE_GLOBAL)
 		null.pte_low = null.pte_high = _PAGE_GLOBAL;
 
 	set_pte_at(mm, addr, ptep, null);
-	htw_reset();
+	htw_start();
 }
 #else
 
@@ -201,6 +194,7 @@ static inline void set_pte(pte_t *ptep, pte_t pteval)
 
 static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
 {
+	htw_stop();
 #if !defined(CONFIG_CPU_R3000) && !defined(CONFIG_CPU_TX39XX)
 	/* Preserve global status for the pair */
 	if (pte_val(*ptep_buddy(ptep)) & _PAGE_GLOBAL)
@@ -208,7 +202,7 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *pt
 	else
 #endif
 		set_pte_at(mm, addr, ptep, __pte(0));
-	htw_reset();
+	htw_start();
 }
 #endif
 
-- 
2.2.2

[PATCH 3/3] MIPS: asm: pgtable: Prevent HTW race when updating PTEs

[Markos Chandras]

Whenever we modify a page table entry, we need to ensure that the HTW
will not fetch a stable entry. And for that to happen we need to ensure
that HTW is stopped before we modify the said entry otherwise the HTW
may already be in the process of reading that entry and fetching the
old information. As a result of which, we replace the htw_reset() calls
with htw_{stop,start} in more appropriate places. This also removes the
remaining users of htw_reset() and as a result we drop that macro

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/include/asm/pgtable.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index d2c7e9e7447e..4f92607ce36e 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -127,14 +127,6 @@ do {									\
 } while(0)
 
 
-#define htw_reset()							\
-do {									\
-	if (cpu_has_htw) {						\
-		htw_stop();						\
-		htw_start();						\
-	}								\
-} while(0)
-
 extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
 	pte_t pteval);
 
@@ -166,12 +158,13 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *pt
 {
 	pte_t null = __pte(0);
 
+	htw_stop();
 	/* Preserve global status for the pair */
 	if (ptep_buddy(ptep)->pte_low & _PAGE_GLOBAL)
 		null.pte_low = null.pte_high = _PAGE_GLOBAL;
 
 	set_pte_at(mm, addr, ptep, null);
-	htw_reset();
+	htw_start();
 }
 #else
 
@@ -201,6 +194,7 @@ static inline void set_pte(pte_t *ptep, pte_t pteval)
 
 static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
 {
+	htw_stop();
 #if !defined(CONFIG_CPU_R3000) && !defined(CONFIG_CPU_TX39XX)
 	/* Preserve global status for the pair */
 	if (pte_val(*ptep_buddy(ptep)) & _PAGE_GLOBAL)
@@ -208,7 +202,7 @@ static inline void pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *pt
 	else
 #endif
 		set_pte_at(mm, addr, ptep, __pte(0));
-	htw_reset();
+	htw_start();
 }
 #endif
 
-- 
2.2.2

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 5865 bytes --]

Hi Markos,

On 26/01/15 09:40, Markos Chandras wrote:
> activate_mm() and switch_mm() call get_new_mmu_context() which in turn
> can enable the HTW before the entryhi is changed with the new ASID.
> Since the latter will enable the HTW in local_flush_tlb_all(),
> then there is a small timing window where the HTW is running with the
> new ASID but with an old pgd since the TLBMISS_HANDLER_SETUP_PGD
> hasn't assigned a new one yet. In order to prevent that, we introduce a
> simple htw counter to avoid starting HTW accidentally due to nested
> htw_{start,stop}() sequences. Moreover, since various IPI calls can
> enforce TLB flushing operations on a different core, such an operation
> may interrupt another htw_{stop,start} in progress leading inconsistent
> updates of the htw_seq variable. In order to avoid that, we disable the
> interrupts whenever we update that variable.
> 
> Cc: <stable@vger.kernel.org> # 3.17+
> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
> ---
>  arch/mips/include/asm/cpu-info.h    |  5 +++++
>  arch/mips/include/asm/mmu_context.h |  7 ++++++-
>  arch/mips/include/asm/pgtable.h     | 17 ++++++++++++++---
>  arch/mips/kernel/cpu-probe.c        |  4 +++-
>  4 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
> index a6c9ccb33c5c..c3f4f2d2e108 100644
> --- a/arch/mips/include/asm/cpu-info.h
> +++ b/arch/mips/include/asm/cpu-info.h
> @@ -84,6 +84,11 @@ struct cpuinfo_mips {
>  	 * (shifted by _CACHE_SHIFT)
>  	 */
>  	unsigned int		writecombine;
> +	/*
> +	 * Simple counter to prevent enabling HTW in nested
> +	 * htw_start/htw_stop calls
> +	 */
> +	unsigned int		htw_seq;
>  } __attribute__((aligned(SMP_CACHE_BYTES)));
>  
>  extern struct cpuinfo_mips cpu_data[];
> diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
> index 2f82568a3ee4..bc01579a907a 100644
> --- a/arch/mips/include/asm/mmu_context.h
> +++ b/arch/mips/include/asm/mmu_context.h
> @@ -25,7 +25,6 @@ do {									\
>  	if (cpu_has_htw) {						\
>  		write_c0_pwbase(pgd);					\
>  		back_to_back_c0_hazard();				\
> -		htw_reset();						\
>  	}								\
>  } while (0)
>  
> @@ -142,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  	unsigned long flags;
>  	local_irq_save(flags);
>  
> +	htw_stop();
>  	/* Check if our ASID is of an older version and thus invalid */
>  	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
>  		get_new_mmu_context(next, cpu);
> @@ -154,6 +154,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  	 */
>  	cpumask_clear_cpu(cpu, mm_cpumask(prev));
>  	cpumask_set_cpu(cpu, mm_cpumask(next));
> +	htw_start();
>  
>  	local_irq_restore(flags);
>  }
> @@ -180,6 +181,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
>  
>  	local_irq_save(flags);
>  
> +	htw_stop();
>  	/* Unconditionally get a new ASID.  */
>  	get_new_mmu_context(next, cpu);
>  
> @@ -189,6 +191,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
>  	/* mark mmu ownership change */
>  	cpumask_clear_cpu(cpu, mm_cpumask(prev));
>  	cpumask_set_cpu(cpu, mm_cpumask(next));
> +	htw_start();
>  
>  	local_irq_restore(flags);
>  }
> @@ -203,6 +206,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
>  	unsigned long flags;
>  
>  	local_irq_save(flags);
> +	htw_stop();
>  
>  	if (cpumask_test_cpu(cpu, mm_cpumask(mm)))  {
>  		get_new_mmu_context(mm, cpu);
> @@ -211,6 +215,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
>  		/* will get a new context next time */
>  		cpu_context(cpu, mm) = 0;
>  	}
> +	htw_start();
>  	local_irq_restore(flags);
>  }
>  
> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
> index 7f7c558de9fc..d2c7e9e7447e 100644
> --- a/arch/mips/include/asm/pgtable.h
> +++ b/arch/mips/include/asm/pgtable.h
> @@ -99,19 +99,30 @@ extern void paging_init(void);
>  
>  #define htw_stop()							\
>  do {									\
> +	unsigned long flags;						\
> +									\
>  	if (cpu_has_htw) {						\
> +		local_irq_save(flags);					\

duplicate?

> +		raw_current_cpu_data.htw_seq++;				\

not "if (!raw_current_cpu_data.htw_seq++)) {"?

>  		write_c0_pwctl(read_c0_pwctl() &			\
>  			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
>  		back_to_back_c0_hazard();				\
> +		local_irq_restore(flags);				\
>  	}								\
>  } while(0)
>  
>  #define htw_start()							\
>  do {									\
> +	unsigned long flags;						\
> +									\
>  	if (cpu_has_htw) {						\
> -		write_c0_pwctl(read_c0_pwctl() |			\
> -			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
> -		back_to_back_c0_hazard();				\
> +		local_irq_save(flags);					\
> +		if (!--raw_current_cpu_data.htw_seq) {			\
> +			write_c0_pwctl(read_c0_pwctl() |		\
> +				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
> +			back_to_back_c0_hazard();			\
> +		}							\
> +		local_irq_restore(flags);				\
>  	}								\
>  } while(0)
>  
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index dc49cf30c2db..5d6e59f20750 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
>  	if (config3 & MIPS_CONF3_MSA)
>  		c->ases |= MIPS_ASE_MSA;
>  	/* Only tested on 32-bit cores */
> -	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
> +	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
> +		c->htw_seq = 0;

is that necessary, since cpu_data[] is global?

>  		c->options |= MIPS_CPU_HTW;
> +	}
>  
>  	return config3 & MIPS_CONF_M;
>  }
> 

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 5865 bytes --]

Hi Markos,

On 26/01/15 09:40, Markos Chandras wrote:
> activate_mm() and switch_mm() call get_new_mmu_context() which in turn
> can enable the HTW before the entryhi is changed with the new ASID.
> Since the latter will enable the HTW in local_flush_tlb_all(),
> then there is a small timing window where the HTW is running with the
> new ASID but with an old pgd since the TLBMISS_HANDLER_SETUP_PGD
> hasn't assigned a new one yet. In order to prevent that, we introduce a
> simple htw counter to avoid starting HTW accidentally due to nested
> htw_{start,stop}() sequences. Moreover, since various IPI calls can
> enforce TLB flushing operations on a different core, such an operation
> may interrupt another htw_{stop,start} in progress leading inconsistent
> updates of the htw_seq variable. In order to avoid that, we disable the
> interrupts whenever we update that variable.
> 
> Cc: <stable@vger.kernel.org> # 3.17+
> Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
> ---
>  arch/mips/include/asm/cpu-info.h    |  5 +++++
>  arch/mips/include/asm/mmu_context.h |  7 ++++++-
>  arch/mips/include/asm/pgtable.h     | 17 ++++++++++++++---
>  arch/mips/kernel/cpu-probe.c        |  4 +++-
>  4 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
> index a6c9ccb33c5c..c3f4f2d2e108 100644
> --- a/arch/mips/include/asm/cpu-info.h
> +++ b/arch/mips/include/asm/cpu-info.h
> @@ -84,6 +84,11 @@ struct cpuinfo_mips {
>  	 * (shifted by _CACHE_SHIFT)
>  	 */
>  	unsigned int		writecombine;
> +	/*
> +	 * Simple counter to prevent enabling HTW in nested
> +	 * htw_start/htw_stop calls
> +	 */
> +	unsigned int		htw_seq;
>  } __attribute__((aligned(SMP_CACHE_BYTES)));
>  
>  extern struct cpuinfo_mips cpu_data[];
> diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
> index 2f82568a3ee4..bc01579a907a 100644
> --- a/arch/mips/include/asm/mmu_context.h
> +++ b/arch/mips/include/asm/mmu_context.h
> @@ -25,7 +25,6 @@ do {									\
>  	if (cpu_has_htw) {						\
>  		write_c0_pwbase(pgd);					\
>  		back_to_back_c0_hazard();				\
> -		htw_reset();						\
>  	}								\
>  } while (0)
>  
> @@ -142,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  	unsigned long flags;
>  	local_irq_save(flags);
>  
> +	htw_stop();
>  	/* Check if our ASID is of an older version and thus invalid */
>  	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
>  		get_new_mmu_context(next, cpu);
> @@ -154,6 +154,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>  	 */
>  	cpumask_clear_cpu(cpu, mm_cpumask(prev));
>  	cpumask_set_cpu(cpu, mm_cpumask(next));
> +	htw_start();
>  
>  	local_irq_restore(flags);
>  }
> @@ -180,6 +181,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
>  
>  	local_irq_save(flags);
>  
> +	htw_stop();
>  	/* Unconditionally get a new ASID.  */
>  	get_new_mmu_context(next, cpu);
>  
> @@ -189,6 +191,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
>  	/* mark mmu ownership change */
>  	cpumask_clear_cpu(cpu, mm_cpumask(prev));
>  	cpumask_set_cpu(cpu, mm_cpumask(next));
> +	htw_start();
>  
>  	local_irq_restore(flags);
>  }
> @@ -203,6 +206,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
>  	unsigned long flags;
>  
>  	local_irq_save(flags);
> +	htw_stop();
>  
>  	if (cpumask_test_cpu(cpu, mm_cpumask(mm)))  {
>  		get_new_mmu_context(mm, cpu);
> @@ -211,6 +215,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
>  		/* will get a new context next time */
>  		cpu_context(cpu, mm) = 0;
>  	}
> +	htw_start();
>  	local_irq_restore(flags);
>  }
>  
> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
> index 7f7c558de9fc..d2c7e9e7447e 100644
> --- a/arch/mips/include/asm/pgtable.h
> +++ b/arch/mips/include/asm/pgtable.h
> @@ -99,19 +99,30 @@ extern void paging_init(void);
>  
>  #define htw_stop()							\
>  do {									\
> +	unsigned long flags;						\
> +									\
>  	if (cpu_has_htw) {						\
> +		local_irq_save(flags);					\

duplicate?

> +		raw_current_cpu_data.htw_seq++;				\

not "if (!raw_current_cpu_data.htw_seq++)) {"?

>  		write_c0_pwctl(read_c0_pwctl() &			\
>  			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
>  		back_to_back_c0_hazard();				\
> +		local_irq_restore(flags);				\
>  	}								\
>  } while(0)
>  
>  #define htw_start()							\
>  do {									\
> +	unsigned long flags;						\
> +									\
>  	if (cpu_has_htw) {						\
> -		write_c0_pwctl(read_c0_pwctl() |			\
> -			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
> -		back_to_back_c0_hazard();				\
> +		local_irq_save(flags);					\
> +		if (!--raw_current_cpu_data.htw_seq) {			\
> +			write_c0_pwctl(read_c0_pwctl() |		\
> +				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
> +			back_to_back_c0_hazard();			\
> +		}							\
> +		local_irq_restore(flags);				\
>  	}								\
>  } while(0)
>  
> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
> index dc49cf30c2db..5d6e59f20750 100644
> --- a/arch/mips/kernel/cpu-probe.c
> +++ b/arch/mips/kernel/cpu-probe.c
> @@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
>  	if (config3 & MIPS_CONF3_MSA)
>  		c->ases |= MIPS_ASE_MSA;
>  	/* Only tested on 32-bit cores */
> -	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
> +	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
> +		c->htw_seq = 0;

is that necessary, since cpu_data[] is global?

>  		c->options |= MIPS_CPU_HTW;
> +	}
>  
>  	return config3 & MIPS_CONF_M;
>  }
> 

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On 26/01/15 11:36, James Hogan wrote:
>> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
>> index 7f7c558de9fc..d2c7e9e7447e 100644
>> --- a/arch/mips/include/asm/pgtable.h
>> +++ b/arch/mips/include/asm/pgtable.h
>> @@ -99,19 +99,30 @@ extern void paging_init(void);
>>  
>>  #define htw_stop()							\
>>  do {									\
>> +	unsigned long flags;						\
>> +									\
>>  	if (cpu_has_htw) {						\
>> +		local_irq_save(flags);					\
> 
> duplicate?

Whoops, ignore that question (it applied to older version of patch).

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On 26/01/15 11:36, James Hogan wrote:
>> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
>> index 7f7c558de9fc..d2c7e9e7447e 100644
>> --- a/arch/mips/include/asm/pgtable.h
>> +++ b/arch/mips/include/asm/pgtable.h
>> @@ -99,19 +99,30 @@ extern void paging_init(void);
>>  
>>  #define htw_stop()							\
>>  do {									\
>> +	unsigned long flags;						\
>> +									\
>>  	if (cpu_has_htw) {						\
>> +		local_irq_save(flags);					\
> 
> duplicate?

Whoops, ignore that question (it applied to older version of patch).

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

On 01/26/2015 11:36 AM, James Hogan wrote:

> 
>> +		raw_current_cpu_data.htw_seq++;				\
> 
> not "if (!raw_current_cpu_data.htw_seq++)) {"?
Why?

on _stop() calls you just increment it. The _start() will do the right
thing then.

I think what you suggest it to move the if() condition from the _start()
to _stop().

> 
>>  		write_c0_pwctl(read_c0_pwctl() &			\
>>  			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
>>  		back_to_back_c0_hazard();				\
>> +		local_irq_restore(flags);				\
>>  	}								\
>>  } while(0)
>>  
>>  #define htw_start()							\
>>  do {									\
>> +	unsigned long flags;						\
>> +									\
>>  	if (cpu_has_htw) {						\
>> -		write_c0_pwctl(read_c0_pwctl() |			\
>> -			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
>> -		back_to_back_c0_hazard();				\
>> +		local_irq_save(flags);					\
>> +		if (!--raw_current_cpu_data.htw_seq) {			\
>> +			write_c0_pwctl(read_c0_pwctl() |		\
>> +				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
>> +			back_to_back_c0_hazard();			\
>> +		}							\
>> +		local_irq_restore(flags);				\
>>  	}								\
>>  } while(0)
>>  
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index dc49cf30c2db..5d6e59f20750 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
>>  	if (config3 & MIPS_CONF3_MSA)
>>  		c->ases |= MIPS_ASE_MSA;
>>  	/* Only tested on 32-bit cores */
>> -	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
>> +	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
>> +		c->htw_seq = 0;
> 
> is that necessary, since cpu_data[] is global?

I checked and it is not placed in the .data instead of .bss section so i
was not sure if it is initialized properly.

-- 
markos

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

On 01/26/2015 11:36 AM, James Hogan wrote:

> 
>> +		raw_current_cpu_data.htw_seq++;				\
> 
> not "if (!raw_current_cpu_data.htw_seq++)) {"?
Why?

on _stop() calls you just increment it. The _start() will do the right
thing then.

I think what you suggest it to move the if() condition from the _start()
to _stop().

> 
>>  		write_c0_pwctl(read_c0_pwctl() &			\
>>  			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
>>  		back_to_back_c0_hazard();				\
>> +		local_irq_restore(flags);				\
>>  	}								\
>>  } while(0)
>>  
>>  #define htw_start()							\
>>  do {									\
>> +	unsigned long flags;						\
>> +									\
>>  	if (cpu_has_htw) {						\
>> -		write_c0_pwctl(read_c0_pwctl() |			\
>> -			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
>> -		back_to_back_c0_hazard();				\
>> +		local_irq_save(flags);					\
>> +		if (!--raw_current_cpu_data.htw_seq) {			\
>> +			write_c0_pwctl(read_c0_pwctl() |		\
>> +				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
>> +			back_to_back_c0_hazard();			\
>> +		}							\
>> +		local_irq_restore(flags);				\
>>  	}								\
>>  } while(0)
>>  
>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>> index dc49cf30c2db..5d6e59f20750 100644
>> --- a/arch/mips/kernel/cpu-probe.c
>> +++ b/arch/mips/kernel/cpu-probe.c
>> @@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
>>  	if (config3 & MIPS_CONF3_MSA)
>>  		c->ases |= MIPS_ASE_MSA;
>>  	/* Only tested on 32-bit cores */
>> -	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
>> +	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
>> +		c->htw_seq = 0;
> 
> is that necessary, since cpu_data[] is global?

I checked and it is not placed in the .data instead of .bss section so i
was not sure if it is initialized properly.

-- 
markos

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

On 01/26/2015 11:47 AM, Markos Chandras wrote:
> On 01/26/2015 11:36 AM, James Hogan wrote:
> 
>>
>>> +		raw_current_cpu_data.htw_seq++;				\
>>
>> not "if (!raw_current_cpu_data.htw_seq++)) {"?
> Why?
> 
> on _stop() calls you just increment it. The _start() will do the right
> thing then.
> 
> I think what you suggest it to move the if() condition from the _start()
> to _stop().
> 
>>
>>>  		write_c0_pwctl(read_c0_pwctl() &			\
>>>  			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
>>>  		back_to_back_c0_hazard();				\
>>> +		local_irq_restore(flags);				\
>>>  	}								\
>>>  } while(0)
>>>  
>>>  #define htw_start()							\
>>>  do {									\
>>> +	unsigned long flags;						\
>>> +									\
>>>  	if (cpu_has_htw) {						\
>>> -		write_c0_pwctl(read_c0_pwctl() |			\
>>> -			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
>>> -		back_to_back_c0_hazard();				\
>>> +		local_irq_save(flags);					\
>>> +		if (!--raw_current_cpu_data.htw_seq) {			\
>>> +			write_c0_pwctl(read_c0_pwctl() |		\
>>> +				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
>>> +			back_to_back_c0_hazard();			\
>>> +		}							\
>>> +		local_irq_restore(flags);				\
>>>  	}								\
>>>  } while(0)
>>>  
>>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>>> index dc49cf30c2db..5d6e59f20750 100644
>>> --- a/arch/mips/kernel/cpu-probe.c
>>> +++ b/arch/mips/kernel/cpu-probe.c
>>> @@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
>>>  	if (config3 & MIPS_CONF3_MSA)
>>>  		c->ases |= MIPS_ASE_MSA;
>>>  	/* Only tested on 32-bit cores */
>>> -	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
>>> +	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
>>> +		c->htw_seq = 0;
>>
>> is that necessary, since cpu_data[] is global?
> 
> I checked and it is not placed in the .data instead of .bss section so i
> was not sure if it is initialized properly.
> 
sorry i meant it's placed in ".data" instead of ".bss" section.

-- 
markos

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

On 01/26/2015 11:47 AM, Markos Chandras wrote:
> On 01/26/2015 11:36 AM, James Hogan wrote:
> 
>>
>>> +		raw_current_cpu_data.htw_seq++;				\
>>
>> not "if (!raw_current_cpu_data.htw_seq++)) {"?
> Why?
> 
> on _stop() calls you just increment it. The _start() will do the right
> thing then.
> 
> I think what you suggest it to move the if() condition from the _start()
> to _stop().
> 
>>
>>>  		write_c0_pwctl(read_c0_pwctl() &			\
>>>  			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
>>>  		back_to_back_c0_hazard();				\
>>> +		local_irq_restore(flags);				\
>>>  	}								\
>>>  } while(0)
>>>  
>>>  #define htw_start()							\
>>>  do {									\
>>> +	unsigned long flags;						\
>>> +									\
>>>  	if (cpu_has_htw) {						\
>>> -		write_c0_pwctl(read_c0_pwctl() |			\
>>> -			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
>>> -		back_to_back_c0_hazard();				\
>>> +		local_irq_save(flags);					\
>>> +		if (!--raw_current_cpu_data.htw_seq) {			\
>>> +			write_c0_pwctl(read_c0_pwctl() |		\
>>> +				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
>>> +			back_to_back_c0_hazard();			\
>>> +		}							\
>>> +		local_irq_restore(flags);				\
>>>  	}								\
>>>  } while(0)
>>>  
>>> diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
>>> index dc49cf30c2db..5d6e59f20750 100644
>>> --- a/arch/mips/kernel/cpu-probe.c
>>> +++ b/arch/mips/kernel/cpu-probe.c
>>> @@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
>>>  	if (config3 & MIPS_CONF3_MSA)
>>>  		c->ases |= MIPS_ASE_MSA;
>>>  	/* Only tested on 32-bit cores */
>>> -	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
>>> +	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
>>> +		c->htw_seq = 0;
>>
>> is that necessary, since cpu_data[] is global?
> 
> I checked and it is not placed in the .data instead of .bss section so i
> was not sure if it is initialized properly.
> 
sorry i meant it's placed in ".data" instead of ".bss" section.

-- 
markos

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On 26/01/15 11:47, Markos Chandras wrote:
> On 01/26/2015 11:36 AM, James Hogan wrote:
> 
>>
>>> +		raw_current_cpu_data.htw_seq++;				\
>>
>> not "if (!raw_current_cpu_data.htw_seq++)) {"?
> Why?
> 
> on _stop() calls you just increment it. The _start() will do the right
> thing then.
> 
> I think what you suggest it to move the if() condition from the _start()
> to _stop().

I just mean you only need to disable htw the first time its called. I
guess the extra branch every time could be worse than the extra disable
and ehb when nesting does occur, so its probably premature optimisation.

Up to you.

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On 26/01/15 11:47, Markos Chandras wrote:
> On 01/26/2015 11:36 AM, James Hogan wrote:
> 
>>
>>> +		raw_current_cpu_data.htw_seq++;				\
>>
>> not "if (!raw_current_cpu_data.htw_seq++)) {"?
> Why?
> 
> on _stop() calls you just increment it. The _start() will do the right
> thing then.
> 
> I think what you suggest it to move the if() condition from the _start()
> to _stop().

I just mean you only need to disable htw the first time its called. I
guess the extra branch every time could be worse than the extra disable
and ehb when nesting does occur, so its probably premature optimisation.

Up to you.

Cheers
James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

On 01/26/2015 12:03 PM, James Hogan wrote:
> On 26/01/15 11:47, Markos Chandras wrote:
>> On 01/26/2015 11:36 AM, James Hogan wrote:
>>
>>>
>>>> +		raw_current_cpu_data.htw_seq++;				\
>>>
>>> not "if (!raw_current_cpu_data.htw_seq++)) {"?
>> Why?
>>
>> on _stop() calls you just increment it. The _start() will do the right
>> thing then.
>>
>> I think what you suggest it to move the if() condition from the _start()
>> to _stop().
> 
> I just mean you only need to disable htw the first time its called. I
> guess the extra branch every time could be worse than the extra disable
> and ehb when nesting does occur, so its probably premature optimisation.
> 
> Up to you.
> 
> Cheers
> James
> 
good idea i will change it.

-- 
markos

Re: [PATCH 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

On 01/26/2015 12:03 PM, James Hogan wrote:
> On 26/01/15 11:47, Markos Chandras wrote:
>> On 01/26/2015 11:36 AM, James Hogan wrote:
>>
>>>
>>>> +		raw_current_cpu_data.htw_seq++;				\
>>>
>>> not "if (!raw_current_cpu_data.htw_seq++)) {"?
>> Why?
>>
>> on _stop() calls you just increment it. The _start() will do the right
>> thing then.
>>
>> I think what you suggest it to move the if() condition from the _start()
>> to _stop().
> 
> I just mean you only need to disable htw the first time its called. I
> guess the extra branch every time could be worse than the extra disable
> and ehb when nesting does occur, so its probably premature optimisation.
> 
> Up to you.
> 
> Cheers
> James
> 
good idea i will change it.

-- 
markos

[PATCH v2 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

activate_mm() and switch_mm() call get_new_mmu_context() which in turn
can enable the HTW before the entryhi is changed with the new ASID.
Since the latter will enable the HTW in local_flush_tlb_all(),
then there is a small timing window where the HTW is running with the
new ASID but with an old pgd since the TLBMISS_HANDLER_SETUP_PGD
hasn't assigned a new one yet. In order to prevent that, we introduce a
simple htw counter to avoid starting HTW accidentally due to nested
htw_{start,stop}() sequences. Moreover, since various IPI calls can
enforce TLB flushing operations on a different core, such an operation
may interrupt another htw_{stop,start} in progress leading inconsistent
updates of the htw_seq variable. In order to avoid that, we disable the
interrupts whenever we update that variable.

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
Changes since v1:
- Avoid stopping the HTW if it is already stopped.
---
 arch/mips/include/asm/cpu-info.h    |  5 +++++
 arch/mips/include/asm/mmu_context.h |  7 ++++++-
 arch/mips/include/asm/pgtable.h     | 24 ++++++++++++++++++------
 arch/mips/kernel/cpu-probe.c        |  4 +++-
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index a6c9ccb33c5c..c3f4f2d2e108 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -84,6 +84,11 @@ struct cpuinfo_mips {
 	 * (shifted by _CACHE_SHIFT)
 	 */
 	unsigned int		writecombine;
+	/*
+	 * Simple counter to prevent enabling HTW in nested
+	 * htw_start/htw_stop calls
+	 */
+	unsigned int		htw_seq;
 } __attribute__((aligned(SMP_CACHE_BYTES)));
 
 extern struct cpuinfo_mips cpu_data[];
diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
index 2f82568a3ee4..bc01579a907a 100644
--- a/arch/mips/include/asm/mmu_context.h
+++ b/arch/mips/include/asm/mmu_context.h
@@ -25,7 +25,6 @@ do {									\
 	if (cpu_has_htw) {						\
 		write_c0_pwbase(pgd);					\
 		back_to_back_c0_hazard();				\
-		htw_reset();						\
 	}								\
 } while (0)
 
@@ -142,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	unsigned long flags;
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
@@ -154,6 +154,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	 */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -180,6 +181,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, cpu);
 
@@ -189,6 +191,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 	/* mark mmu ownership change */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -203,6 +206,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 	unsigned long flags;
 
 	local_irq_save(flags);
+	htw_stop();
 
 	if (cpumask_test_cpu(cpu, mm_cpumask(mm)))  {
 		get_new_mmu_context(mm, cpu);
@@ -211,6 +215,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 		/* will get a new context next time */
 		cpu_context(cpu, mm) = 0;
 	}
+	htw_start();
 	local_irq_restore(flags);
 }
 
diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index 7f7c558de9fc..eacb022067a6 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -99,19 +99,31 @@ extern void paging_init(void);
 
 #define htw_stop()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
-		write_c0_pwctl(read_c0_pwctl() &			\
-			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
-		back_to_back_c0_hazard();				\
+		local_irq_save(flags);					\
+		if(!raw_current_cpu_data.htw_seq++) {			\
+			write_c0_pwctl(read_c0_pwctl() &		\
+				       ~(1 << MIPS_PWCTL_PWEN_SHIFT));	\
+			back_to_back_c0_hazard();			\
+		}							\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
 #define htw_start()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
-		write_c0_pwctl(read_c0_pwctl() |			\
-			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
-		back_to_back_c0_hazard();				\
+		local_irq_save(flags);					\
+		if (!--raw_current_cpu_data.htw_seq) {			\
+			write_c0_pwctl(read_c0_pwctl() |		\
+				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
+			back_to_back_c0_hazard();			\
+		}							\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index dc49cf30c2db..5d6e59f20750 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
 	if (config3 & MIPS_CONF3_MSA)
 		c->ases |= MIPS_ASE_MSA;
 	/* Only tested on 32-bit cores */
-	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
+	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
+		c->htw_seq = 0;
 		c->options |= MIPS_CPU_HTW;
+	}
 
 	return config3 & MIPS_CONF_M;
 }
-- 
2.2.2

[PATCH v2 2/3] MIPS: HTW: Prevent accidental HTW start due to nested htw_{start,stop}

[Markos Chandras]

activate_mm() and switch_mm() call get_new_mmu_context() which in turn
can enable the HTW before the entryhi is changed with the new ASID.
Since the latter will enable the HTW in local_flush_tlb_all(),
then there is a small timing window where the HTW is running with the
new ASID but with an old pgd since the TLBMISS_HANDLER_SETUP_PGD
hasn't assigned a new one yet. In order to prevent that, we introduce a
simple htw counter to avoid starting HTW accidentally due to nested
htw_{start,stop}() sequences. Moreover, since various IPI calls can
enforce TLB flushing operations on a different core, such an operation
may interrupt another htw_{stop,start} in progress leading inconsistent
updates of the htw_seq variable. In order to avoid that, we disable the
interrupts whenever we update that variable.

Cc: <stable@vger.kernel.org> # 3.17+
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
Changes since v1:
- Avoid stopping the HTW if it is already stopped.
---
 arch/mips/include/asm/cpu-info.h    |  5 +++++
 arch/mips/include/asm/mmu_context.h |  7 ++++++-
 arch/mips/include/asm/pgtable.h     | 24 ++++++++++++++++++------
 arch/mips/kernel/cpu-probe.c        |  4 +++-
 4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index a6c9ccb33c5c..c3f4f2d2e108 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -84,6 +84,11 @@ struct cpuinfo_mips {
 	 * (shifted by _CACHE_SHIFT)
 	 */
 	unsigned int		writecombine;
+	/*
+	 * Simple counter to prevent enabling HTW in nested
+	 * htw_start/htw_stop calls
+	 */
+	unsigned int		htw_seq;
 } __attribute__((aligned(SMP_CACHE_BYTES)));
 
 extern struct cpuinfo_mips cpu_data[];
diff --git a/arch/mips/include/asm/mmu_context.h b/arch/mips/include/asm/mmu_context.h
index 2f82568a3ee4..bc01579a907a 100644
--- a/arch/mips/include/asm/mmu_context.h
+++ b/arch/mips/include/asm/mmu_context.h
@@ -25,7 +25,6 @@ do {									\
 	if (cpu_has_htw) {						\
 		write_c0_pwbase(pgd);					\
 		back_to_back_c0_hazard();				\
-		htw_reset();						\
 	}								\
 } while (0)
 
@@ -142,6 +141,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	unsigned long flags;
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Check if our ASID is of an older version and thus invalid */
 	if ((cpu_context(cpu, next) ^ asid_cache(cpu)) & ASID_VERSION_MASK)
 		get_new_mmu_context(next, cpu);
@@ -154,6 +154,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	 */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -180,6 +181,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 
 	local_irq_save(flags);
 
+	htw_stop();
 	/* Unconditionally get a new ASID.  */
 	get_new_mmu_context(next, cpu);
 
@@ -189,6 +191,7 @@ activate_mm(struct mm_struct *prev, struct mm_struct *next)
 	/* mark mmu ownership change */
 	cpumask_clear_cpu(cpu, mm_cpumask(prev));
 	cpumask_set_cpu(cpu, mm_cpumask(next));
+	htw_start();
 
 	local_irq_restore(flags);
 }
@@ -203,6 +206,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 	unsigned long flags;
 
 	local_irq_save(flags);
+	htw_stop();
 
 	if (cpumask_test_cpu(cpu, mm_cpumask(mm)))  {
 		get_new_mmu_context(mm, cpu);
@@ -211,6 +215,7 @@ drop_mmu_context(struct mm_struct *mm, unsigned cpu)
 		/* will get a new context next time */
 		cpu_context(cpu, mm) = 0;
 	}
+	htw_start();
 	local_irq_restore(flags);
 }
 
diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
index 7f7c558de9fc..eacb022067a6 100644
--- a/arch/mips/include/asm/pgtable.h
+++ b/arch/mips/include/asm/pgtable.h
@@ -99,19 +99,31 @@ extern void paging_init(void);
 
 #define htw_stop()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
-		write_c0_pwctl(read_c0_pwctl() &			\
-			       ~(1 << MIPS_PWCTL_PWEN_SHIFT));		\
-		back_to_back_c0_hazard();				\
+		local_irq_save(flags);					\
+		if(!raw_current_cpu_data.htw_seq++) {			\
+			write_c0_pwctl(read_c0_pwctl() &		\
+				       ~(1 << MIPS_PWCTL_PWEN_SHIFT));	\
+			back_to_back_c0_hazard();			\
+		}							\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
 #define htw_start()							\
 do {									\
+	unsigned long flags;						\
+									\
 	if (cpu_has_htw) {						\
-		write_c0_pwctl(read_c0_pwctl() |			\
-			       (1 << MIPS_PWCTL_PWEN_SHIFT));		\
-		back_to_back_c0_hazard();				\
+		local_irq_save(flags);					\
+		if (!--raw_current_cpu_data.htw_seq) {			\
+			write_c0_pwctl(read_c0_pwctl() |		\
+				       (1 << MIPS_PWCTL_PWEN_SHIFT));	\
+			back_to_back_c0_hazard();			\
+		}							\
+		local_irq_restore(flags);				\
 	}								\
 } while(0)
 
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index dc49cf30c2db..5d6e59f20750 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -367,8 +367,10 @@ static inline unsigned int decode_config3(struct cpuinfo_mips *c)
 	if (config3 & MIPS_CONF3_MSA)
 		c->ases |= MIPS_ASE_MSA;
 	/* Only tested on 32-bit cores */
-	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT))
+	if ((config3 & MIPS_CONF3_PW) && config_enabled(CONFIG_32BIT)) {
+		c->htw_seq = 0;
 		c->options |= MIPS_CPU_HTW;
+	}
 
 	return config3 & MIPS_CONF_M;
 }
-- 
2.2.2