Skip to contentSkip to conversation

TLB flush concerns with POWER9/radix for init and machine checks

PATCH New Nicholas Piggin 1 participant 1 message Apr 30, 2017

Subsystems (2)
lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

PATCH 1/1 TLB flush concerns with POWER9/radix for init and machine checks

Hi,

I've had a few concerns with the TLB flushing for POWER9 as implemented for cpu_setup/cpu_restore and machine check exception flushes.

- tlbiel is undefined if R does not match the current radix mode. The TLB clearing in cpu_setup should match hash mode, but in machine check and cpu_restore, it seems like it will be the wrong mode on radix hosts.

- Radix mode should have to flush process and partition scope to flush all TLBs for machine checks.

- PWC and table caches were not being invalidated. I'm not sure if TLB type MCE should have to flush these, but at least if setup expects to have stale TLB entries then we should flush these as well.

This is what I've come up with so far to improve it. For POWER[789] we have a hash__ and radix__tlbiel_all. This is no longer a cputable hook because it's a slow path case so just make it a function.

The very early TLB flush in cpu_setup_power.S for those CPUs is removed, and instead it is moved to early_init_mmu right before caller turns on relocation. There does not seem to be a reason to flush TLB before relocation.

TLB flush is removed from the ->cpu_restore hook. This gets called from idle wakeup for a full state loss. Is it really necessary to flush the TLBs in that case? We can add this back if necessary but it would be good to understand why it's required.

Another concern but something I haven't addressed is tlbie (broadcast) being executed before all CPUs have their MMU registers set up and matching. update_hid_for_radix() for example (which also does not really have R matching the current mode AFAIKS).

---
 arch/powerpc/include/asm/book3s/64/tlbflush-hash.h |  1 +
 .../powerpc/include/asm/book3s/64/tlbflush-radix.h |  3 +
 arch/powerpc/include/asm/book3s/64/tlbflush.h      | 14 ++++
 arch/powerpc/include/asm/cputable.h                | 12 ----
 arch/powerpc/kernel/cpu_setup_power.S              | 43 ------------
 arch/powerpc/kernel/cputable.c                     | 14 ----
 arch/powerpc/kernel/mce_power.c                    | 61 +----------------
 arch/powerpc/kvm/book3s_hv_ras.c                   |  6 +-
 arch/powerpc/mm/hash_native_64.c                   | 79 ++++++++++++++++++++++
 arch/powerpc/mm/hash_utils_64.c                    |  4 ++
 arch/powerpc/mm/pgtable-radix.c                    |  4 ++
 arch/powerpc/mm/tlb-radix.c                        | 54 +++++++++++++++
 12 files changed, 163 insertions(+), 132 deletions(-)
arch/powerpc/include/asm/book3s/64/tlbflush-hash.h +1 -0
index 2f6373144e2c..c02ece27fd7b 100644
@@ -50,6 +50,7 @@ static inline void arch_leave_lazy_mmu_mode(void)
 
 #define arch_flush_lazy_mmu_mode()      do {} while (0)
 
+extern void hash__tlbiel_all(unsigned int action);
 
 extern void flush_hash_page(unsigned long vpn, real_pte_t pte, int psize,
 			    int ssize, unsigned long flags);
arch/powerpc/include/asm/book3s/64/tlbflush-radix.h +3 -0
index cc7fbde4f53c..e7b767a3b2fa 100644
@@ -10,6 +10,8 @@ static inline int mmu_get_ap(int psize)
 	return mmu_psize_defs[psize].ap;
 }
 
+extern void radix__tlbiel_all(unsigned int action);
+
 extern void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
 					   unsigned long start, unsigned long end);
 extern void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long start,
@@ -44,4 +46,5 @@ extern void radix__flush_tlb_lpid(unsigned long lpid);
 extern void radix__flush_tlb_all(void);
 extern void radix__flush_tlb_pte_p9_dd1(unsigned long old_pte, struct mm_struct *mm,
 					unsigned long address);
+
 #endif
arch/powerpc/include/asm/book3s/64/tlbflush.h +14 -0
index 72b925f97bab..edf2bf28269a 100644
@@ -7,6 +7,20 @@
 #include <asm/book3s/64/tlbflush-hash.h>
 #include <asm/book3s/64/tlbflush-radix.h>
 
+/* TLB flush actions. Used as argument to tlbiel_all() */
+enum {
+	TLB_INVAL_SCOPE_GLOBAL = 0,	/* invalidate all TLBs */
+	TLB_INVAL_SCOPE_LPID = 1,	/* invalidate TLBs for current LPID */
+};
+
+static inline void tlbiel_all(unsigned int action)
+{
+	if (radix_enabled())
+		radix__tlbiel_all(action);
+	else
+		hash__tlbiel_all(action);
+}
+
 #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
 static inline void flush_pmd_tlb_range(struct vm_area_struct *vma,
 				       unsigned long start, unsigned long end)
arch/powerpc/include/asm/cputable.h +0 -12
index 1f6847b107e4..8205ce79c173 100644
@@ -106,12 +106,6 @@ struct cpu_spec {
 	 * called in real mode to handle SLB and TLB errors.
 	 */
 	long		(*machine_check_early)(struct pt_regs *regs);
-
-	/*
-	 * Processor specific routine to flush tlbs.
-	 */
-	void		(*flush_tlb)(unsigned int action);
-
 };
 
 extern struct cpu_spec		*cur_cpu_spec;
@@ -130,12 +124,6 @@ extern void cpu_feature_keys_init(void);
 static inline void cpu_feature_keys_init(void) { }
 #endif
 
-/* TLB flush actions. Used as argument to cpu_spec.flush_tlb() hook */
-enum {
-	TLB_INVAL_SCOPE_GLOBAL = 0,	/* invalidate all TLBs */
-	TLB_INVAL_SCOPE_LPID = 1,	/* invalidate TLBs for current LPID */
-};
-
 #endif /* __ASSEMBLY__ */
 
 /* CPU kernel features */
arch/powerpc/kernel/cpu_setup_power.S +0 -43

85 lines not on this page · raw message

arch/powerpc/kernel/cputable.c +0 -14
index e79b9daa873c..437ff57c5101 100644
@@ -72,9 +72,6 @@ extern void __setup_cpu_power8(unsigned long offset, struct cpu_spec* spec);
 extern void __restore_cpu_power8(void);
 extern void __setup_cpu_power9(unsigned long offset, struct cpu_spec* spec);
 extern void __restore_cpu_power9(void);
-extern void __flush_tlb_power7(unsigned int action);
-extern void __flush_tlb_power8(unsigned int action);
-extern void __flush_tlb_power9(unsigned int action);
 extern long __machine_check_early_realmode_p7(struct pt_regs *regs);
 extern long __machine_check_early_realmode_p8(struct pt_regs *regs);
 extern long __machine_check_early_realmode_p9(struct pt_regs *regs);
@@ -365,7 +362,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_cpu_type	= "ppc64/ibm-compat-v1",
 		.cpu_setup		= __setup_cpu_power7,
 		.cpu_restore		= __restore_cpu_power7,
-		.flush_tlb		= __flush_tlb_power7,
 		.machine_check_early	= __machine_check_early_realmode_p7,
 		.platform		= "power7",
 	},
@@ -383,7 +379,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_cpu_type	= "ppc64/ibm-compat-v1",
 		.cpu_setup		= __setup_cpu_power8,
 		.cpu_restore		= __restore_cpu_power8,
-		.flush_tlb		= __flush_tlb_power8,
 		.machine_check_early	= __machine_check_early_realmode_p8,
 		.platform		= "power8",
 	},
@@ -401,7 +396,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_cpu_type	= "ppc64/ibm-compat-v1",
 		.cpu_setup		= __setup_cpu_power9,
 		.cpu_restore		= __restore_cpu_power9,
-		.flush_tlb		= __flush_tlb_power9,
 		.platform		= "power9",
 	},
 	{	/* Power7 */
@@ -420,7 +414,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_POWER4,
 		.cpu_setup		= __setup_cpu_power7,
 		.cpu_restore		= __restore_cpu_power7,
-		.flush_tlb		= __flush_tlb_power7,
 		.machine_check_early	= __machine_check_early_realmode_p7,
 		.platform		= "power7",
 	},
@@ -440,7 +433,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_POWER4,
 		.cpu_setup		= __setup_cpu_power7,
 		.cpu_restore		= __restore_cpu_power7,
-		.flush_tlb		= __flush_tlb_power7,
 		.machine_check_early	= __machine_check_early_realmode_p7,
 		.platform		= "power7+",
 	},
@@ -460,7 +452,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_INVALID,
 		.cpu_setup		= __setup_cpu_power8,
 		.cpu_restore		= __restore_cpu_power8,
-		.flush_tlb		= __flush_tlb_power8,
 		.machine_check_early	= __machine_check_early_realmode_p8,
 		.platform		= "power8",
 	},
@@ -480,7 +471,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_INVALID,
 		.cpu_setup		= __setup_cpu_power8,
 		.cpu_restore		= __restore_cpu_power8,
-		.flush_tlb		= __flush_tlb_power8,
 		.machine_check_early	= __machine_check_early_realmode_p8,
 		.platform		= "power8",
 	},
@@ -500,7 +490,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_INVALID,
 		.cpu_setup		= __setup_cpu_power8,
 		.cpu_restore		= __restore_cpu_power8,
-		.flush_tlb		= __flush_tlb_power8,
 		.machine_check_early	= __machine_check_early_realmode_p8,
 		.platform		= "power8",
 	},
@@ -520,7 +509,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_INVALID,
 		.cpu_setup		= __setup_cpu_power8,
 		.cpu_restore		= __restore_cpu_power8,
-		.flush_tlb		= __flush_tlb_power8,
 		.machine_check_early	= __machine_check_early_realmode_p8,
 		.platform		= "power8",
 	},
@@ -540,7 +528,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_INVALID,
 		.cpu_setup		= __setup_cpu_power9,
 		.cpu_restore		= __restore_cpu_power9,
-		.flush_tlb		= __flush_tlb_power9,
 		.machine_check_early	= __machine_check_early_realmode_p9,
 		.platform		= "power9",
 	},
@@ -560,7 +547,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.oprofile_type		= PPC_OPROFILE_INVALID,
 		.cpu_setup		= __setup_cpu_power9,
 		.cpu_restore		= __restore_cpu_power9,
-		.flush_tlb		= __flush_tlb_power9,
 		.machine_check_early	= __machine_check_early_realmode_p9,
 		.platform		= "power9",
 	},
arch/powerpc/kernel/mce_power.c +2 -59

73 lines not on this page · raw message

arch/powerpc/kvm/book3s_hv_ras.c +2 -4
index 7ef0993214f3..3e7783cff0c5 100644
@@ -87,8 +87,7 @@ static long kvmppc_realmode_mc_power7(struct kvm_vcpu *vcpu)
 				   DSISR_MC_SLB_PARITY | DSISR_MC_DERAT_MULTI);
 		}
 		if (dsisr & DSISR_MC_TLB_MULTI) {
-			if (cur_cpu_spec && cur_cpu_spec->flush_tlb)
-				cur_cpu_spec->flush_tlb(TLB_INVAL_SCOPE_LPID);
+			tlbiel_all(TLB_INVAL_SCOPE_LPID);
 			dsisr &= ~DSISR_MC_TLB_MULTI;
 		}
 		/* Any other errors we don't understand? */
@@ -105,8 +104,7 @@ static long kvmppc_realmode_mc_power7(struct kvm_vcpu *vcpu)
 		reload_slb(vcpu);
 		break;
 	case SRR1_MC_IFETCH_TLBMULTI:
-		if (cur_cpu_spec && cur_cpu_spec->flush_tlb)
-			cur_cpu_spec->flush_tlb(TLB_INVAL_SCOPE_LPID);
+		tlbiel_all(TLB_INVAL_SCOPE_LPID);
 		break;
 	default:
 		handled = 0;
arch/powerpc/mm/hash_native_64.c +79 -0

85 lines not on this page · raw message

arch/powerpc/mm/hash_utils_64.c +4 -0
index f2095ce9d4b0..c98e085a25ec 100644
@@ -1044,6 +1044,8 @@ void __init hash__early_init_mmu(void)
 	pr_info("Initializing hash mmu with SLB\n");
 	/* Initialize SLB management */
 	slb_initialize();
+
+	tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
 }
 
 #ifdef CONFIG_SMP
@@ -1063,6 +1065,8 @@ void hash__early_init_mmu_secondary(void)
 	}
 	/* Initialize SLB */
 	slb_initialize();
+
+	tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
 }
 #endif /* CONFIG_SMP */
 
arch/powerpc/mm/pgtable-radix.c +4 -0
index c28165d8970b..fbf9e8fb20ba 100644
@@ -426,6 +426,8 @@ void __init radix__early_init_mmu(void)
 
 	radix_init_iamr();
 	radix_init_pgtable();
+
+	tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
 }
 
 void radix__early_init_mmu_secondary(void)
@@ -447,6 +449,8 @@ void radix__early_init_mmu_secondary(void)
 		radix_init_amor();
 	}
 	radix_init_iamr();
+
+	tlbiel_all(TLB_INVAL_SCOPE_GLOBAL);
 }
 
 void radix__mmu_cleanup_all(void)
arch/powerpc/mm/tlb-radix.c +54 -0

60 lines not on this page · raw message

Signature · 1 line
2.11.0