Skip to contentSkip to conversation

powerpc/64s: Fix for radix TLB invalidation bug

PATCH Applied Nicholas Piggin 3 participants 7 messages Nov 26, 2020 – Dec 4, 2020

Subsystems (3)
lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

cover powerpc/64s: Fix for radix TLB invalidation bug

This fixes a tricky bug that was noticed by TLB multi-hits in a guest stress testing CPU hotplug, but TLB invalidation means any kind of data corruption is possible.

Thanks,
Nick

Nicholas Piggin (4):
powerpc/64s: Fix hash ISA v3.0 TLBIEL instruction generation powerpc/64s/pseries: Fix hash tlbiel_all_isa300 for guest kernels kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling powerpc/64s: Trim offlined CPUs from mm_cpumasks

 arch/powerpc/include/asm/book3s/64/mmu.h     | 12 ++++++++++
 arch/powerpc/mm/book3s64/hash_native.c       | 23 +++++++++++++-------
 arch/powerpc/mm/book3s64/mmu_context.c       | 20 +++++++++++++++++
 arch/powerpc/platforms/powermac/smp.c        |  2 ++
 arch/powerpc/platforms/powernv/smp.c         |  3 +++
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  3 +++
 kernel/cpu.c                                 |  6 ++++-
 7 files changed, 60 insertions(+), 9 deletions(-)
Signature · 1 line
2.23.0
author reviewer
#1 lore raw

PATCH 1/4 powerpc/64s: Fix hash ISA v3.0 TLBIEL instruction generation

A typo has the R field of the instruction assigned by lucky dip a la register allocator.

Fixes d4748276ae14c ("powerpc/64s: Improve local TLB flush for boot and MCE on POWER9")
---
 arch/powerpc/mm/book3s64/hash_native.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/mm/book3s64/hash_native.c +1 -1
index 0203cdf48c54..97fa42d7027e 100644
@@ -68,7 +68,7 @@ static __always_inline void tlbiel_hash_set_isa300(unsigned int set, unsigned in
 	rs = ((unsigned long)pid << PPC_BITLSHIFT(31));
 
 	asm volatile(PPC_TLBIEL(%0, %1, %2, %3, %4)
-		     : : "r"(rb), "r"(rs), "i"(ric), "i"(prs), "r"(r)
+		     : : "r"(rb), "r"(rs), "i"(ric), "i"(prs), "i"(r)
 		     : "memory");
 }
 
Signature · 1 line
2.23.0
author reviewer
#2 lore raw

PATCH 2/4 powerpc/64s/pseries: Fix hash tlbiel_all_isa300 for guest kernels

In reply to Nicholas Piggin

tlbiel_all() can not be usable in !HVMODE when running hash presently, remove HV privileged flushes when running in guest to make it usable.

---
 arch/powerpc/mm/book3s64/hash_native.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)
arch/powerpc/mm/book3s64/hash_native.c +14 -7
index 97fa42d7027e..52e170bd95ae 100644
@@ -92,16 +92,15 @@ static void tlbiel_all_isa300(unsigned int num_sets, unsigned int is)
 	asm volatile("ptesync": : :"memory");
 
 	/*
-	 * Flush the first set of the TLB, and any caching of partition table
-	 * entries. Then flush the remaining sets of the TLB. Hash mode uses
-	 * partition scoped TLB translations.
+	 * Flush the partition table cache if this is HV mode.
 	 */
-	tlbiel_hash_set_isa300(0, is, 0, 2, 0);
-	for (set = 1; set < num_sets; set++)
-		tlbiel_hash_set_isa300(set, is, 0, 0, 0);
+	if (early_cpu_has_feature(CPU_FTR_HVMODE))
+		tlbiel_hash_set_isa300(0, is, 0, 2, 0);
 
 	/*
-	 * Now invalidate the process table cache.
+	 * Now invalidate the process table cache. UPRT=0 HPT modes (what
+	 * current hardware implements) do not use the process table, but
+	 * add the flushes anyway.
 	 *
 	 * From ISA v3.0B p. 1078:
 	 *     The following forms are invalid.
@@ -110,6 +109,14 @@ static void tlbiel_all_isa300(unsigned int num_sets, unsigned int is)
 	 */
 	tlbiel_hash_set_isa300(0, is, 0, 2, 1);
 
+	/*
+	 * Then flush the sets of the TLB proper. Hash mode uses
+	 * partition scoped TLB translations, which may be flushed
+	 * in !HV mode.
+	 */
+	for (set = 0; set < num_sets; set++)
+		tlbiel_hash_set_isa300(set, is, 0, 0, 0);
+
 	ppc_after_tlbiel_barrier();
 
 	asm volatile(PPC_ISA_3_0_INVALIDATE_ERAT "; isync" : : :"memory");
Signature · 1 line
2.23.0
author reviewer
#3 lore raw

PATCH 3/4 kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling

In reply to Nicholas Piggin

powerpc/64s keeps a counter in the mm which counts bits set in mm_cpumask as well as other things. This means it can't use generic code to clear bits out of the mask and doesn't adjust the arch specific counter.

Add an arch override that allows powerpc/64s to use clear_tasks_mm_cpumask().

---
 kernel/cpu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
kernel/cpu.c +5 -1
index 6ff2578ecf17..2b8d7a5db383 100644
@@ -815,6 +815,10 @@ void __init cpuhp_threads_init(void)
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
+#ifndef arch_clear_mm_cpumask_cpu
+#define arch_clear_mm_cpumask_cpu(cpu, mm) cpumask_clear_cpu(cpu, mm_cpumask(mm))
+#endif
+
 /**
  * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU
  * @cpu: a CPU id
@@ -850,7 +854,7 @@ void clear_tasks_mm_cpumask(int cpu)
 		t = find_lock_task_mm(p);
 		if (!t)
 			continue;
-		cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
+		arch_clear_mm_cpumask_cpu(cpu, t->mm);
 		task_unlock(t);
 	}
 	rcu_read_unlock();
Signature · 1 line
2.23.0
author reviewer
#4 lore raw

PATCH 4/4 powerpc/64s: Trim offlined CPUs from mm_cpumasks

In reply to Nicholas Piggin

When offlining a CPU, powerpc/64s does not flush TLBs, rather it just leaves the CPU set in mm_cpumasks, so it continues to receive TLBIEs to manage its TLBs.

However the exit_flush_lazy_tlbs() function expects that after returning, all CPUs (except self) have flushed TLBs for that mm, in which case TLBIEL can be used for this flush. This breaks for offline CPUs because they don't get the IPI to flush their TLB. This can lead to stale translations.

Fix this by clearing the CPU from mm_cpumasks, then flushing all TLBs before going offline.

These offlined CPU bits stuck in the cpumask also prevents the cpumask from being trimmed back to local mode, which means continual broadcast IPIs or TLBIEs are needed for TLB flushing. This patch prevents that situation too.

A cast of many were involved in working this out, but in particular Milton, Aneesh, Paul made key discoveries.

Fixes 0cef77c7798a7 ("powerpc/64s/radix: flush remote CPUs out of single-threaded mm_cpumask")
---
 arch/powerpc/include/asm/book3s/64/mmu.h     | 12 ++++++++++++
 arch/powerpc/mm/book3s64/mmu_context.c       | 20 ++++++++++++++++++++
 arch/powerpc/platforms/powermac/smp.c        |  2 ++
 arch/powerpc/platforms/powernv/smp.c         |  3 +++
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  3 +++
 5 files changed, 40 insertions(+)
arch/powerpc/include/asm/book3s/64/mmu.h +12 -0
index e0b52940e43c..750918451dd2 100644
@@ -242,6 +242,18 @@ extern void radix_init_pseries(void);
 static inline void radix_init_pseries(void) { };
 #endif
 
+#ifdef CONFIG_HOTPLUG_CPU
+#define arch_clear_mm_cpumask_cpu(cpu, mm)				\
+	do {								\
+		if (cpumask_test_cpu(cpu, mm_cpumask(mm))) {		\
+			atomic_dec(&(mm)->context.active_cpus);		\
+			cpumask_clear_cpu(cpu, mm_cpumask(mm));		\
+		}							\
+	} while (0)
+
+void cleanup_cpu_mmu_context(void);
+#endif
+
 static inline int get_user_context(mm_context_t *ctx, unsigned long ea)
 {
 	int index = ea >> MAX_EA_BITS_PER_CONTEXT;
arch/powerpc/mm/book3s64/mmu_context.c +20 -0
index 1c54821de7bf..0c8557220ae2 100644
@@ -17,6 +17,7 @@
 #include <linux/export.h>
 #include <linux/gfp.h>
 #include <linux/slab.h>
+#include <linux/cpu.h>
 
 #include <asm/mmu_context.h>
 #include <asm/pgalloc.h>
@@ -307,3 +308,22 @@ void radix__switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
 	isync();
 }
 #endif
+
+/**
+ * cleanup_cpu_mmu_context - Clean up MMU details for this CPU (newly offlined)
+ *
+ * This clears the CPU from mm_cpumask for all processes, and then flushes the
+ * local TLB to ensure TLB coherency in case the CPU is onlined again.
+ *
+ * KVM guest translations are not necessarily flushed here. If KVM started
+ * using mm_cpumask or the Linux APIs which do, this would have to be resolved.
+ */
+#ifdef CONFIG_HOTPLUG_CPU
+void cleanup_cpu_mmu_context(void)
+{
+	int cpu = smp_processor_id();
+
+	clear_tasks_mm_cpumask(cpu);
+	tlbiel_all();
+}
+#endif
arch/powerpc/platforms/powermac/smp.c +2 -0
index 74ebe664b016..adae2a6712e1 100644
@@ -911,6 +911,8 @@ static int smp_core99_cpu_disable(void)
 
 	mpic_cpu_set_priority(0xf);
 
+	cleanup_cpu_mmu_context();
+
 	return 0;
 }
 
arch/powerpc/platforms/powernv/smp.c +3 -0
index 54c4ba45c7ce..cbb67813cd5d 100644
@@ -143,6 +143,9 @@ static int pnv_smp_cpu_disable(void)
 		xive_smp_disable_cpu();
 	else
 		xics_migrate_irqs_away();
+
+	cleanup_cpu_mmu_context();
+
 	return 0;
 }
 
arch/powerpc/platforms/pseries/hotplug-cpu.c +3 -0
index f2837e33bf5d..a02012f1b04a 100644
@@ -90,6 +90,9 @@ static int pseries_cpu_disable(void)
 		xive_smp_disable_cpu();
 	else
 		xics_migrate_irqs_away();
+
+	cleanup_cpu_mmu_context();
+
 	return 0;
 }
 
Signature · 1 line
2.23.0
Reviewed-by on the v1 series
#5 lore raw
In reply to Nicholas Piggin
Quoted 13 lines · Nicholas Piggin

This fixes a tricky bug that was noticed by TLB multi-hits in a guest stress testing CPU hotplug, but TLB invalidation means any kind of data corruption is possible.

Thanks,
Nick

Nicholas Piggin (4):
powerpc/64s: Fix hash ISA v3.0 TLBIEL instruction generation powerpc/64s/pseries: Fix hash tlbiel_all_isa300 for guest kernels kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling powerpc/64s: Trim offlined CPUs from mm_cpumasks

Quoted 9 lines
 arch/powerpc/include/asm/book3s/64/mmu.h     | 12 ++++++++++
 arch/powerpc/mm/book3s64/hash_native.c       | 23 +++++++++++++-------
 arch/powerpc/mm/book3s64/mmu_context.c       | 20 +++++++++++++++++
 arch/powerpc/platforms/powermac/smp.c        |  2 ++
 arch/powerpc/platforms/powernv/smp.c         |  3 +++
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  3 +++
 kernel/cpu.c                                 |  6 ++++-
 7 files changed, 60 insertions(+), 9 deletions(-)

You can add for the series

-aneesh

#6 lore raw
In reply to Nicholas Piggin
Quoted 8 lines · Nicholas Piggin

This fixes a tricky bug that was noticed by TLB multi-hits in a guest stress testing CPU hotplug, but TLB invalidation means any kind of data corruption is possible.

Thanks,
Nick

[...]

Applied to powerpc/fixes.

[1/4] powerpc/64s: Fix hash ISA v3.0 TLBIEL instruction generation

      https://git.kernel.org/powerpc/c/5844cc25fd121074de7895181a2fa1ce100a0fdd
[2/4] powerpc/64s/pseries: Fix hash tlbiel_all_isa300 for guest kernels
      https://git.kernel.org/powerpc/c/c0b27c517acf8a2b359dd373a7e7e88b01a8308e
[3/4] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling
      https://git.kernel.org/powerpc/c/8ff00399b153440c1c83e20c43020385b416415b
[4/4] powerpc/64s: Trim offlined CPUs from mm_cpumasks
      https://git.kernel.org/powerpc/c/01b0f0eae0812e80efeee4ee17687e5386335e08

cheers