All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors
@ 2021-06-02  4:04 Nicholas Piggin
  2021-06-02 20:34 ` Fabiano Rosas
  2021-06-24 13:59 ` Michael Ellerman
  0 siblings, 2 replies; 3+ messages in thread
From: Nicholas Piggin @ 2021-06-02  4:04 UTC (permalink / raw)
  To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Suraj Jitindar Singh

From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

The POWER9 vCPU TLB management code assumes all threads in a core share
a TLB, and that TLBIEL execued by one thread will invalidate TLBs for
all threads. This is not the case for SMT8 capable POWER9 and POWER10
(big core) processors, where the TLB is split between groups of threads.
This results in TLB multi-hits, random data corruption, etc.

Fix this by introducing cpu_first_tlb_thread_sibling etc., to determine
which siblings share TLBs, and use that in the guest TLB flushing code.

[npiggin@gmail.com: add changelog and comment]

Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/cputhreads.h | 30 +++++++++++++++++++++++++++
 arch/powerpc/kvm/book3s_hv.c          | 13 ++++++------
 arch/powerpc/kvm/book3s_hv_builtin.c  |  2 +-
 arch/powerpc/kvm/book3s_hv_rm_mmu.c   |  2 +-
 4 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/cputhreads.h b/arch/powerpc/include/asm/cputhreads.h
index 98c8bd155bf9..b167186aaee4 100644
--- a/arch/powerpc/include/asm/cputhreads.h
+++ b/arch/powerpc/include/asm/cputhreads.h
@@ -98,6 +98,36 @@ static inline int cpu_last_thread_sibling(int cpu)
 	return cpu | (threads_per_core - 1);
 }
 
+/*
+ * tlb_thread_siblings are siblings which share a TLB. This is not
+ * architected, is not something a hypervisor could emulate and a future
+ * CPU may change behaviour even in compat mode, so this should only be
+ * used on PowerNV, and only with care.
+ */
+static inline int cpu_first_tlb_thread_sibling(int cpu)
+{
+	if (cpu_has_feature(CPU_FTR_ARCH_300) && (threads_per_core == 8))
+		return cpu & ~0x6;	/* Big Core */
+	else
+		return cpu_first_thread_sibling(cpu);
+}
+
+static inline int cpu_last_tlb_thread_sibling(int cpu)
+{
+	if (cpu_has_feature(CPU_FTR_ARCH_300) && (threads_per_core == 8))
+		return cpu | 0x6;	/* Big Core */
+	else
+		return cpu_last_thread_sibling(cpu);
+}
+
+static inline int cpu_tlb_thread_sibling_step(void)
+{
+	if (cpu_has_feature(CPU_FTR_ARCH_300) && (threads_per_core == 8))
+		return 2;		/* Big Core */
+	else
+		return 1;
+}
+
 static inline u32 get_tensr(void)
 {
 #ifdef	CONFIG_BOOKE
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 28a80d240b76..0a8398a63f80 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2657,7 +2657,7 @@ static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu)
 	cpumask_t *cpu_in_guest;
 	int i;
 
-	cpu = cpu_first_thread_sibling(cpu);
+	cpu = cpu_first_tlb_thread_sibling(cpu);
 	if (nested) {
 		cpumask_set_cpu(cpu, &nested->need_tlb_flush);
 		cpu_in_guest = &nested->cpu_in_guest;
@@ -2671,9 +2671,10 @@ static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu)
 	 * the other side is the first smp_mb() in kvmppc_run_core().
 	 */
 	smp_mb();
-	for (i = 0; i < threads_per_core; ++i)
-		if (cpumask_test_cpu(cpu + i, cpu_in_guest))
-			smp_call_function_single(cpu + i, do_nothing, NULL, 1);
+	for (i = cpu; i <= cpu_last_tlb_thread_sibling(cpu);
+					i += cpu_tlb_thread_sibling_step())
+		if (cpumask_test_cpu(i, cpu_in_guest))
+			smp_call_function_single(i, do_nothing, NULL, 1);
 }
 
 static void kvmppc_prepare_radix_vcpu(struct kvm_vcpu *vcpu, int pcpu)
@@ -2704,8 +2705,8 @@ static void kvmppc_prepare_radix_vcpu(struct kvm_vcpu *vcpu, int pcpu)
 	 */
 	if (prev_cpu != pcpu) {
 		if (prev_cpu >= 0 &&
-		    cpu_first_thread_sibling(prev_cpu) !=
-		    cpu_first_thread_sibling(pcpu))
+		    cpu_first_tlb_thread_sibling(prev_cpu) !=
+		    cpu_first_tlb_thread_sibling(pcpu))
 			radix_flush_cpu(kvm, prev_cpu, vcpu);
 		if (nested)
 			nested->prev_cpu[vcpu->arch.nested_vcpu_id] = pcpu;
diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
index 7a0e33a9c980..3edc25c89092 100644
--- a/arch/powerpc/kvm/book3s_hv_builtin.c
+++ b/arch/powerpc/kvm/book3s_hv_builtin.c
@@ -800,7 +800,7 @@ void kvmppc_check_need_tlb_flush(struct kvm *kvm, int pcpu,
 	 * Thus we make all 4 threads use the same bit.
 	 */
 	if (cpu_has_feature(CPU_FTR_ARCH_300))
-		pcpu = cpu_first_thread_sibling(pcpu);
+		pcpu = cpu_first_tlb_thread_sibling(pcpu);
 
 	if (nested)
 		need_tlb_flush = &nested->need_tlb_flush;
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 7af7c70f1468..407dbf21bcbc 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
@@ -67,7 +67,7 @@ static int global_invalidates(struct kvm *kvm)
 		 * so use the bit for the first thread to represent the core.
 		 */
 		if (cpu_has_feature(CPU_FTR_ARCH_300))
-			cpu = cpu_first_thread_sibling(cpu);
+			cpu = cpu_first_tlb_thread_sibling(cpu);
 		cpumask_clear_cpu(cpu, &kvm->arch.need_tlb_flush);
 	}
 
-- 
2.23.0


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

* Re: [PATCH] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors
  2021-06-02  4:04 [PATCH] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors Nicholas Piggin
@ 2021-06-02 20:34 ` Fabiano Rosas
  2021-06-24 13:59 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Fabiano Rosas @ 2021-06-02 20:34 UTC (permalink / raw)
  To: Nicholas Piggin, kvm-ppc
  Cc: linuxppc-dev, Nicholas Piggin, Suraj Jitindar Singh

Nicholas Piggin <npiggin@gmail.com> writes:

> From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>
> The POWER9 vCPU TLB management code assumes all threads in a core share
> a TLB, and that TLBIEL execued by one thread will invalidate TLBs for
> all threads. This is not the case for SMT8 capable POWER9 and POWER10
> (big core) processors, where the TLB is split between groups of threads.
> This results in TLB multi-hits, random data corruption, etc.
>
> Fix this by introducing cpu_first_tlb_thread_sibling etc., to determine
> which siblings share TLBs, and use that in the guest TLB flushing code.
>
> [npiggin@gmail.com: add changelog and comment]
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

> ---
>  arch/powerpc/include/asm/cputhreads.h | 30 +++++++++++++++++++++++++++
>  arch/powerpc/kvm/book3s_hv.c          | 13 ++++++------
>  arch/powerpc/kvm/book3s_hv_builtin.c  |  2 +-
>  arch/powerpc/kvm/book3s_hv_rm_mmu.c   |  2 +-
>  4 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputhreads.h b/arch/powerpc/include/asm/cputhreads.h
> index 98c8bd155bf9..b167186aaee4 100644
> --- a/arch/powerpc/include/asm/cputhreads.h
> +++ b/arch/powerpc/include/asm/cputhreads.h
> @@ -98,6 +98,36 @@ static inline int cpu_last_thread_sibling(int cpu)
>  	return cpu | (threads_per_core - 1);
>  }
>
> +/*
> + * tlb_thread_siblings are siblings which share a TLB. This is not
> + * architected, is not something a hypervisor could emulate and a future
> + * CPU may change behaviour even in compat mode, so this should only be
> + * used on PowerNV, and only with care.
> + */
> +static inline int cpu_first_tlb_thread_sibling(int cpu)
> +{
> +	if (cpu_has_feature(CPU_FTR_ARCH_300) && (threads_per_core == 8))
> +		return cpu & ~0x6;	/* Big Core */
> +	else
> +		return cpu_first_thread_sibling(cpu);
> +}
> +
> +static inline int cpu_last_tlb_thread_sibling(int cpu)
> +{
> +	if (cpu_has_feature(CPU_FTR_ARCH_300) && (threads_per_core == 8))
> +		return cpu | 0x6;	/* Big Core */
> +	else
> +		return cpu_last_thread_sibling(cpu);
> +}
> +
> +static inline int cpu_tlb_thread_sibling_step(void)
> +{
> +	if (cpu_has_feature(CPU_FTR_ARCH_300) && (threads_per_core == 8))
> +		return 2;		/* Big Core */
> +	else
> +		return 1;
> +}
> +
>  static inline u32 get_tensr(void)
>  {
>  #ifdef	CONFIG_BOOKE
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 28a80d240b76..0a8398a63f80 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -2657,7 +2657,7 @@ static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu)
>  	cpumask_t *cpu_in_guest;
>  	int i;
>
> -	cpu = cpu_first_thread_sibling(cpu);
> +	cpu = cpu_first_tlb_thread_sibling(cpu);
>  	if (nested) {
>  		cpumask_set_cpu(cpu, &nested->need_tlb_flush);
>  		cpu_in_guest = &nested->cpu_in_guest;
> @@ -2671,9 +2671,10 @@ static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu)
>  	 * the other side is the first smp_mb() in kvmppc_run_core().
>  	 */
>  	smp_mb();
> -	for (i = 0; i < threads_per_core; ++i)
> -		if (cpumask_test_cpu(cpu + i, cpu_in_guest))
> -			smp_call_function_single(cpu + i, do_nothing, NULL, 1);
> +	for (i = cpu; i <= cpu_last_tlb_thread_sibling(cpu);
> +					i += cpu_tlb_thread_sibling_step())
> +		if (cpumask_test_cpu(i, cpu_in_guest))
> +			smp_call_function_single(i, do_nothing, NULL, 1);
>  }
>
>  static void kvmppc_prepare_radix_vcpu(struct kvm_vcpu *vcpu, int pcpu)
> @@ -2704,8 +2705,8 @@ static void kvmppc_prepare_radix_vcpu(struct kvm_vcpu *vcpu, int pcpu)
>  	 */
>  	if (prev_cpu != pcpu) {
>  		if (prev_cpu >= 0 &&
> -		    cpu_first_thread_sibling(prev_cpu) !=
> -		    cpu_first_thread_sibling(pcpu))
> +		    cpu_first_tlb_thread_sibling(prev_cpu) !=
> +		    cpu_first_tlb_thread_sibling(pcpu))
>  			radix_flush_cpu(kvm, prev_cpu, vcpu);
>  		if (nested)
>  			nested->prev_cpu[vcpu->arch.nested_vcpu_id] = pcpu;
> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
> index 7a0e33a9c980..3edc25c89092 100644
> --- a/arch/powerpc/kvm/book3s_hv_builtin.c
> +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
> @@ -800,7 +800,7 @@ void kvmppc_check_need_tlb_flush(struct kvm *kvm, int pcpu,
>  	 * Thus we make all 4 threads use the same bit.
>  	 */
>  	if (cpu_has_feature(CPU_FTR_ARCH_300))
> -		pcpu = cpu_first_thread_sibling(pcpu);
> +		pcpu = cpu_first_tlb_thread_sibling(pcpu);
>
>  	if (nested)
>  		need_tlb_flush = &nested->need_tlb_flush;
> diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> index 7af7c70f1468..407dbf21bcbc 100644
> --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
> @@ -67,7 +67,7 @@ static int global_invalidates(struct kvm *kvm)
>  		 * so use the bit for the first thread to represent the core.
>  		 */
>  		if (cpu_has_feature(CPU_FTR_ARCH_300))
> -			cpu = cpu_first_thread_sibling(cpu);
> +			cpu = cpu_first_tlb_thread_sibling(cpu);
>  		cpumask_clear_cpu(cpu, &kvm->arch.need_tlb_flush);
>  	}

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

* Re: [PATCH] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors
  2021-06-02  4:04 [PATCH] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors Nicholas Piggin
  2021-06-02 20:34 ` Fabiano Rosas
@ 2021-06-24 13:59 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2021-06-24 13:59 UTC (permalink / raw)
  To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev, Suraj Jitindar Singh

On Wed, 2 Jun 2021 14:04:41 +1000, Nicholas Piggin wrote:
> The POWER9 vCPU TLB management code assumes all threads in a core share
> a TLB, and that TLBIEL execued by one thread will invalidate TLBs for
> all threads. This is not the case for SMT8 capable POWER9 and POWER10
> (big core) processors, where the TLB is split between groups of threads.
> This results in TLB multi-hits, random data corruption, etc.
> 
> Fix this by introducing cpu_first_tlb_thread_sibling etc., to determine
> which siblings share TLBs, and use that in the guest TLB flushing code.
> 
> [...]

Applied to powerpc/topic/ppc-kvm.

[1/1] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors
      https://git.kernel.org/powerpc/c/77bbbc0cf84834ed130838f7ac1988567f4d0288

cheers

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

end of thread, other threads:[~2021-06-24 14:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02  4:04 [PATCH] KVM: PPC: Book3S HV: Fix TLB management on SMT8 POWER9 and POWER10 processors Nicholas Piggin
2021-06-02 20:34 ` Fabiano Rosas
2021-06-24 13:59 ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.