Skip to contentSkip to conversation

KVM powerpc tlbie scalability improvement

PATCH Applied Nicholas Piggin 6 participants 14 messages Apr 5, 2018 – May 17, 2018

Subsystems (2)
PowerPC KVM PowerPC
lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

cover KVM powerpc tlbie scalability improvement

Any reason we still need to take the tlbie lock on modern processors?

Nicholas Piggin (2):
KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode
KVM: PPC: Book3S HV: lockless tlbie for HPT hcalls

 arch/powerpc/kvm/book3s_hv_rm_mmu.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
Signature · 1 line
2.16.3
author reviewer
#1 lore raw

PATCH 1/2 KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode

This crashes with a "Bad real address for load" attempting to load from the vmalloc region in realmode (faulting address is in DAR).

  Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
  LE SMP NR_CPUS 48 NUMA PowerNV
  CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g43d1859f0994
  NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
  REGS: c000000fff76dd80 TRAP: 0200   Not tainted  (4.16.0-01530-g43d1859f0994)
  MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
  CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
  NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
  LR [c0000000000c2430] do_tlbies+0x230/0x2f0

I suspect the reason is the per-cpu data is not in the linear chunk. This could be restored if that was able to be fixed, but for now, just remove the tracepoints.

Fixes 0428491cba ("powerpc/mm: Trace tlbie(l) instructions")
Signed-off-by Nicholas Piggin <[email protected]>
---
 arch/powerpc/kvm/book3s_hv_rm_mmu.c | 4 ----
 1 file changed, 4 deletions(-)
arch/powerpc/kvm/book3s_hv_rm_mmu.c +0 -4
index e1c083fbe434..78e6a392330f 100644
@@ -470,8 +470,6 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		for (i = 0; i < npages; ++i) {
 			asm volatile(PPC_TLBIE_5(%0,%1,0,0,0) : :
 				     "r" (rbvalues[i]), "r" (kvm->arch.lpid));
-			trace_tlbie(kvm->arch.lpid, 0, rbvalues[i],
-				kvm->arch.lpid, 0, 0, 0);
 		}
 
 		if (cpu_has_feature(CPU_FTR_P9_TLBIE_BUG)) {

@@ -492,8 +490,6 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		for (i = 0; i < npages; ++i) {
 			asm volatile(PPC_TLBIEL(%0,%1,0,0,0) : :
 				     "r" (rbvalues[i]), "r" (0));
-			trace_tlbie(kvm->arch.lpid, 1, rbvalues[i],
-				0, 0, 0, 0);
 		}
 		asm volatile("ptesync" : : : "memory");
 	}
Signature · 1 line
2.16.3
#2 lore raw

Re: [PATCH 1/2] KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode

Quoted 13 lines · Nicholas Piggin

This crashes with a "Bad real address for load" attempting to load from the vmalloc region in realmode (faulting address is in DAR).

  Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
  LE SMP NR_CPUS 48 NUMA PowerNV
  CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g43d1859f0994
  NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
  REGS: c000000fff76dd80 TRAP: 0200   Not tainted  (4.16.0-01530-g43d1859f0994)
  MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
  CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
  NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
  LR [c0000000000c2430] do_tlbies+0x230/0x2f0
Quoted 3 lines

I suspect the reason is the per-cpu data is not in the linear chunk. This could be restored if that was able to be fixed, but for now, just remove the tracepoints.

Could you share the stack trace as well? I've not observed this in my testing. May be I don't have as many cpus. I presume your talking about the per cpu data offsets for per cpu trace data?

Balbir Singh.

author reviewer
#3 lore raw

Re: [PATCH 1/2] KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode

Quoted 14 lines · On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh <[email protected]> wrote:
Quoted 13 lines · Nicholas Piggin

This crashes with a "Bad real address for load" attempting to load from the vmalloc region in realmode (faulting address is in DAR).

  Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
  LE SMP NR_CPUS 48 NUMA PowerNV
  CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g43d1859f0994
  NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
  REGS: c000000fff76dd80 TRAP: 0200   Not tainted  (4.16.0-01530-g43d1859f0994)
  MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
  CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
  NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
  LR [c0000000000c2430] do_tlbies+0x230/0x2f0
Quoted 5 lines
Quoted 3 lines

I suspect the reason is the per-cpu data is not in the linear chunk. This could be restored if that was able to be fixed, but for now, just remove the tracepoints.

Could you share the stack trace as well? I've not observed this in my testing.

I can't seem to find it, I can try reproduce tomorrow. It was coming from h_remove hcall from the guest. It's 176 logical CPUs.

Quoted 2 lines

May be I don't have as many cpus. I presume your talking about the per cpu data offsets for per cpu trace data?

It looked like it was dereferencing virtually mapped per-cpu data, yes. Probably the perf_events deref.

Thanks,
Nick

reviewer
#4 lore raw

Re: [PATCH 1/2] KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode

Quoted 23 lines · Nicholas Piggin
Quoted 14 lines · On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh <[email protected]> wrote:
Quoted 13 lines · Nicholas Piggin

This crashes with a "Bad real address for load" attempting to load from the vmalloc region in realmode (faulting address is in DAR).

  Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
  LE SMP NR_CPUS 48 NUMA PowerNV
  CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g43d1859f0994
  NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
  REGS: c000000fff76dd80 TRAP: 0200   Not tainted  (4.16.0-01530-g43d1859f0994)
  MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
  CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
  NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
  LR [c0000000000c2430] do_tlbies+0x230/0x2f0
Quoted 5 lines
Quoted 3 lines

I suspect the reason is the per-cpu data is not in the linear chunk. This could be restored if that was able to be fixed, but for now, just remove the tracepoints.

Could you share the stack trace as well? I've not observed this in my testing.

Quoted 8 lines

I can't seem to find it, I can try reproduce tomorrow. It was coming from h_remove hcall from the guest. It's 176 logical CPUs.

Quoted 2 lines

May be I don't have as many cpus. I presume your talking about the per cpu data offsets for per cpu trace data?

It looked like it was dereferencing virtually mapped per-cpu data, yes. Probably the perf_events deref.

Naveen has posted a series to (hopefully) fix this, which just missed the merge window:

https://patchwork.ozlabs.org/patch/894757/

cheers

#5 lore raw

Re: [PATCH 1/2] KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode

Quoted 31 lines · Michael Ellerman
Quoted 29 lines · Nicholas Piggin
Quoted 14 lines · On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh <[email protected]> wrote:
Quoted 13 lines · Nicholas Piggin

This crashes with a "Bad real address for load" attempting to load from the vmalloc region in realmode (faulting address is in DAR).

  Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
  LE SMP NR_CPUS 48 NUMA PowerNV
  CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g43d1859f0994
  NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
  REGS: c000000fff76dd80 TRAP: 0200   Not tainted  (4.16.0-01530-g43d1859f0994)
  MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
  CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
  NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
  LR [c0000000000c2430] do_tlbies+0x230/0x2f0
Quoted 5 lines
Quoted 3 lines

I suspect the reason is the per-cpu data is not in the linear chunk. This could be restored if that was able to be fixed, but for now, just remove the tracepoints.

Could you share the stack trace as well? I've not observed this in my testing.

I can't seem to find it, I can try reproduce tomorrow. It was coming from h_remove hcall from the guest. It's 176 logical CPUs.

Quoted 2 lines

May be I don't have as many cpus. I presume your talking about the per cpu data offsets for per cpu trace data?

Quoted 7 lines
Quoted 2 lines

It looked like it was dereferencing virtually mapped per-cpu data, yes. Probably the perf_events deref.

Naveen has posted a series to (hopefully) fix this, which just missed the merge window:

https://patchwork.ozlabs.org/patch/894757/

I'm afraid that won't actually help here :(
That series is specific to the function tracer, while this is using static tracepoints.

We could convert trace_tlbie() to a TRACE_EVENT_CONDITION() and guard it within a check for paca->ftrace_enabled, but that would only be useful if the below callsites can ever be hit outside of KVM guest mode.

- Naveen

author reviewer
#6 lore raw

Re: [PATCH 1/2] KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode

Quoted 40 lines · On Tue, 10 Apr 2018 11:25:02 +0530 "Naveen N. Rao"…
Quoted 31 lines · Michael Ellerman
Quoted 29 lines · Nicholas Piggin
Quoted 14 lines · On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh <[email protected]> wrote:
Quoted 13 lines · Nicholas Piggin

This crashes with a "Bad real address for load" attempting to load from the vmalloc region in realmode (faulting address is in DAR).

  Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
  LE SMP NR_CPUS 48 NUMA PowerNV
  CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g43d1859f0994
  NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
  REGS: c000000fff76dd80 TRAP: 0200   Not tainted  (4.16.0-01530-g43d1859f0994)
  MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
  CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
  NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
  LR [c0000000000c2430] do_tlbies+0x230/0x2f0
Quoted 5 lines
Quoted 3 lines

I suspect the reason is the per-cpu data is not in the linear chunk. This could be restored if that was able to be fixed, but for now, just remove the tracepoints.

Could you share the stack trace as well? I've not observed this in my testing.

I can't seem to find it, I can try reproduce tomorrow. It was coming from h_remove hcall from the guest. It's 176 logical CPUs.

Quoted 2 lines

May be I don't have as many cpus. I presume your talking about the per cpu data offsets for per cpu trace data?

Quoted 7 lines
Quoted 2 lines

It looked like it was dereferencing virtually mapped per-cpu data, yes. Probably the perf_events deref.

Naveen has posted a series to (hopefully) fix this, which just missed the merge window:

https://patchwork.ozlabs.org/patch/894757/

Quoted 7 lines

I'm afraid that won't actually help here :(
That series is specific to the function tracer, while this is using static tracepoints.

We could convert trace_tlbie() to a TRACE_EVENT_CONDITION() and guard it within a check for paca->ftrace_enabled, but that would only be useful if the below callsites can ever be hit outside of KVM guest mode.

Right, removing the trace points is the right thing to do here.

Doing tracing in real mode would be a whole effort itself, I'd expect. Or disabling realmode handling of HPT hcalls if trace points are active.

Thanks,
Nick

Applied
#7 lore raw

Re: [1/2] KVM: PPC: Book3S HV: trace_tlbie must not be called in realmode

In reply to Nicholas Piggin
Quoted 13 lines · Nicholas Piggin

This crashes with a "Bad real address for load" attempting to load from the vmalloc region in realmode (faulting address is in DAR).

  Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
  LE SMP NR_CPUS 48 NUMA PowerNV
  CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g43d1859f0994
  NIP:  c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580
  REGS: c000000fff76dd80 TRAP: 0200   Not tainted  (4.16.0-01530-g43d1859f0994)
  MSR:  9000000000201003 <SF,HV,ME,RI,LE>  CR: 48082222  XER: 00000000
  CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE: 3
  NIP [c0000000000155ac] perf_trace_tlbie+0x2c/0x1a0
  LR [c0000000000c2430] do_tlbies+0x230/0x2f0
Quoted 6 lines

I suspect the reason is the per-cpu data is not in the linear chunk. This could be restored if that was able to be fixed, but for now, just remove the tracepoints.

Fixes 0428491cba ("powerpc/mm: Trace tlbie(l) instructions")
Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/19ce7909ed11c49f7eddf59e7f49cd

cheers

author reviewer
#8 lore raw

PATCH 2/2 KVM: PPC: Book3S HV: lockless tlbie for HPT hcalls

In reply to Nicholas Piggin

tlbies to an LPAR do not have to be serialised since POWER4, MMU_FTR_LOCKLESS_TLBIE can be used to avoid the spin lock in do_tlbies.

Testing was done on a POWER9 system in HPT mode, with a -smp 32 guest in HPT mode. 32 instances of the powerpc fork benchmark from selftests were run with --fork, and the results measured.

Without this patch, total throughput was about 13.5K/sec, and this is the top of the host profile:

   74.52%  [k] do_tlbies
    2.95%  [k] kvmppc_book3s_hv_page_fault
    1.80%  [k] calc_checksum
    1.80%  [k] kvmppc_vcpu_run_hv
    1.49%  [k] kvmppc_run_core

After this patch, throughput was about 51K/sec, with this profile:

   21.28%  [k] do_tlbies
    5.26%  [k] kvmppc_run_core
    4.88%  [k] kvmppc_book3s_hv_page_fault
    3.30%  [k] _raw_spin_lock_irqsave
    3.25%  [k] gup_pgd_range
Signed-off-by Nicholas Piggin <[email protected]>
---
 arch/powerpc/kvm/book3s_hv_rm_mmu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
arch/powerpc/kvm/book3s_hv_rm_mmu.c +10 -1
index 78e6a392330f..0221a0f74f07 100644
@@ -439,6 +439,9 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	unsigned int tmp, old;
 	unsigned int token = LOCK_TOKEN;
 
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return 1;
+
 	asm volatile("1:lwarx	%1,0,%2\n"
 		     "	cmpwi	cr0,%1,0\n"
 		     "	bne	2f\n"

@@ -452,6 +455,12 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	return old = 0;
 }
 
+static inline void unlock_tlbie_after_sync(unsigned int *lock)
+{
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return;
+}
+
 static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		      long npages, int global, bool need_sync)
 {

@@ -483,7 +492,7 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		}
 
 		asm volatile("eieio; tlbsync; ptesync" : : : "memory");
-		kvm->arch.tlbie_lock = 0;
+		unlock_tlbie_after_sync(&kvm->arch.tlbie_lock);
 	} else {
 		if (need_sync)
 			asm volatile("ptesync" : : : "memory");
Signature · 1 line
2.16.3
author reviewer
#9 lore raw

Re: [PATCH 2/2] KVM: PPC: Book3S HV: lockless tlbie for HPT hcalls

Quoted 58 lines · On Fri, 6 Apr 2018 03:56:31 +1000 Nicholas Piggin <[email protected]> wrote:

tlbies to an LPAR do not have to be serialised since POWER4, MMU_FTR_LOCKLESS_TLBIE can be used to avoid the spin lock in do_tlbies.

Testing was done on a POWER9 system in HPT mode, with a -smp 32 guest in HPT mode. 32 instances of the powerpc fork benchmark from selftests were run with --fork, and the results measured.

Without this patch, total throughput was about 13.5K/sec, and this is the top of the host profile:

   74.52%  [k] do_tlbies
    2.95%  [k] kvmppc_book3s_hv_page_fault
    1.80%  [k] calc_checksum
    1.80%  [k] kvmppc_vcpu_run_hv
    1.49%  [k] kvmppc_run_core

After this patch, throughput was about 51K/sec, with this profile:

   21.28%  [k] do_tlbies
    5.26%  [k] kvmppc_run_core
    4.88%  [k] kvmppc_book3s_hv_page_fault
    3.30%  [k] _raw_spin_lock_irqsave
    3.25%  [k] gup_pgd_range
Signed-off-by Nicholas Piggin <[email protected]>
---
 arch/powerpc/kvm/book3s_hv_rm_mmu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
arch/powerpc/kvm/book3s_hv_rm_mmu.c +9 -0
index 78e6a392330f..0221a0f74f07 100644
@@ -439,6 +439,9 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	unsigned int tmp, old;
 	unsigned int token = LOCK_TOKEN;
 
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return 1;
+
 	asm volatile("1:lwarx	%1,0,%2\n"
 		     "	cmpwi	cr0,%1,0\n"
 		     "	bne	2f\n"

@@ -452,6 +455,12 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	return old = 0;
 }
 
+static inline void unlock_tlbie_after_sync(unsigned int *lock)
+{
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return;
+}
+
 static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		      long npages, int global, bool need_sync)
 {

arch/powerpc/kvm/book3s_hv_rm_mmu.c static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues, · line 495

Quoted 5 lines
arch/powerpc/kvm/book3s_hv_rm_mmu.c +1 -1
@@ -483,7 +492,7 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		}
 
 		asm volatile("eieio; tlbsync; ptesync" : : : "memory");
-		kvm->arch.tlbie_lock = 0;
+		unlock_tlbie_after_sync(&kvm->arch.tlbie_lock);

Well that's a silly bug in the !LOCKLESS path, that was supposed to move to unlock, of course. Will fix it up after some time for comments.

Thanks,
Nick

reviewer
#10 lore raw

Re: [PATCH 2/2] KVM: PPC: Book3S HV: lockless tlbie for HPT hcalls

In reply to Nicholas Piggin
Quoted 15 lines · Nicholas Piggin
arch/powerpc/kvm/book3s_hv_rm_mmu.c +3 -0
index 78e6a392330f..0221a0f74f07 100644
@@ -439,6 +439,9 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	unsigned int tmp, old;
 	unsigned int token = LOCK_TOKEN;
 
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return 1;
+
 	asm volatile("1:lwarx	%1,0,%2\n"
 		     "	cmpwi	cr0,%1,0\n"
 		     "	bne	2f\n"

arch/powerpc/kvm/book3s_hv_rm_mmu.c static inline int try_lock_tlbie(unsigned int *lock) · line 462

Quoted 8 lines
arch/powerpc/kvm/book3s_hv_rm_mmu.c +5 -0
@@ -452,6 +455,12 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	return old = 0;
 }
 
+static inline void unlock_tlbie_after_sync(unsigned int *lock)
+{
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return;
+}

So this is a bit hard to follow:

#define MMU_FTRS_DEFAULT_HPTE_ARCH_V2 \
MMU_FTR_HPTE_TABLE | MMU_FTR_PPCAS_ARCH_V2
#define MMU_FTRS_POWER MMU_FTRS_DEFAULT_HPTE_ARCH_V2
#define MMU_FTRS_PPC970 MMU_FTRS_POWER | MMU_FTR_TLBIE_CROP_VA // does NOT
#define MMU_FTRS_POWER5 MMU_FTRS_POWER | MMU_FTR_LOCKLESS_TLBIE
#define MMU_FTRS_POWER6 MMU_FTRS_POWER5 | MMU_FTR_KERNEL_RO | MMU_FTR_68_BIT_VA // includes lockless TLBIE
#define MMU_FTRS_POWER7 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_POWER8 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_POWER9 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_CELL MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | // does NOT
MMU_FTR_CI_LARGE_PAGE
#define MMU_FTRS_PA6T MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | \ // does NOT
MMU_FTR_CI_LARGE_PAGE | MMU_FTR_NO_SLBIE_B

So it's only 970, Cell and Pasemi that *don't* have lockless TLBIE.

And KVM HV doesn't doesn't run on any of those.

So we can just not check for the feature in the KVM HV code.

Am I right?

cheers

#11 lore raw

Re: [PATCH 2/2] KVM: PPC: Book3S HV: lockless tlbie for HPT hcalls

Quoted 42 lines · Michael Ellerman
Quoted 15 lines · Nicholas Piggin
arch/powerpc/kvm/book3s_hv_rm_mmu.c +3 -0
index 78e6a392330f..0221a0f74f07 100644
@@ -439,6 +439,9 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	unsigned int tmp, old;
 	unsigned int token = LOCK_TOKEN;
 
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return 1;
+
 	asm volatile("1:lwarx	%1,0,%2\n"
 		     "	cmpwi	cr0,%1,0\n"
 		     "	bne	2f\n"

arch/powerpc/kvm/book3s_hv_rm_mmu.c static inline int try_lock_tlbie(unsigned int *lock) · line 462

Quoted 8 lines
arch/powerpc/kvm/book3s_hv_rm_mmu.c +5 -0
@@ -452,6 +455,12 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	return old = 0;
 }
 
+static inline void unlock_tlbie_after_sync(unsigned int *lock)
+{
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return;
+}

So this is a bit hard to follow:

#define MMU_FTRS_DEFAULT_HPTE_ARCH_V2 \
MMU_FTR_HPTE_TABLE | MMU_FTR_PPCAS_ARCH_V2
#define MMU_FTRS_POWER MMU_FTRS_DEFAULT_HPTE_ARCH_V2
#define MMU_FTRS_PPC970 MMU_FTRS_POWER | MMU_FTR_TLBIE_CROP_VA // does NOT
#define MMU_FTRS_POWER5 MMU_FTRS_POWER | MMU_FTR_LOCKLESS_TLBIE
#define MMU_FTRS_POWER6 MMU_FTRS_POWER5 | MMU_FTR_KERNEL_RO | MMU_FTR_68_BIT_VA // includes lockless TLBIE
#define MMU_FTRS_POWER7 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_POWER8 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_POWER9 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_CELL MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | // does NOT
MMU_FTR_CI_LARGE_PAGE
#define MMU_FTRS_PA6T MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | \ // does NOT
MMU_FTR_CI_LARGE_PAGE | MMU_FTR_NO_SLBIE_B

Quoted 7 lines

So it's only 970, Cell and Pasemi that *don't* have lockless TLBIE.

And KVM HV doesn't doesn't run on any of those.

So we can just not check for the feature in the KVM HV code.

Am I right?

Yes; that code was written when we still supported HV KVM on 970, but we ripped that out some time ago.

Paul.

reviewer
#12 lore raw

Re: [PATCH 2/2] KVM: PPC: Book3S HV: lockless tlbie for HPT hcalls

Quoted 45 lines · Paul Mackerras
Quoted 44 lines · Michael Ellerman
Quoted 15 lines · Nicholas Piggin
arch/powerpc/kvm/book3s_hv_rm_mmu.c +3 -0
index 78e6a392330f..0221a0f74f07 100644
@@ -439,6 +439,9 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	unsigned int tmp, old;
 	unsigned int token = LOCK_TOKEN;
 
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return 1;
+
 	asm volatile("1:lwarx	%1,0,%2\n"
 		     "	cmpwi	cr0,%1,0\n"
 		     "	bne	2f\n"

arch/powerpc/kvm/book3s_hv_rm_mmu.c static inline int try_lock_tlbie(unsigned int *lock) · line 462

Quoted 8 lines
arch/powerpc/kvm/book3s_hv_rm_mmu.c +5 -0
@@ -452,6 +455,12 @@ static inline int try_lock_tlbie(unsigned int *lock)
 	return old = 0;
 }
 
+static inline void unlock_tlbie_after_sync(unsigned int *lock)
+{
+	if (mmu_has_feature(MMU_FTR_LOCKLESS_TLBIE))
+		return;
+}

So this is a bit hard to follow:

#define MMU_FTRS_DEFAULT_HPTE_ARCH_V2 \
MMU_FTR_HPTE_TABLE | MMU_FTR_PPCAS_ARCH_V2
#define MMU_FTRS_POWER MMU_FTRS_DEFAULT_HPTE_ARCH_V2
#define MMU_FTRS_PPC970 MMU_FTRS_POWER | MMU_FTR_TLBIE_CROP_VA // does NOT
#define MMU_FTRS_POWER5 MMU_FTRS_POWER | MMU_FTR_LOCKLESS_TLBIE
#define MMU_FTRS_POWER6 MMU_FTRS_POWER5 | MMU_FTR_KERNEL_RO | MMU_FTR_68_BIT_VA // includes lockless TLBIE
#define MMU_FTRS_POWER7 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_POWER8 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_POWER9 MMU_FTRS_POWER6 // includes lockless TLBIE
#define MMU_FTRS_CELL MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | // does NOT
MMU_FTR_CI_LARGE_PAGE
#define MMU_FTRS_PA6T MMU_FTRS_DEFAULT_HPTE_ARCH_V2 | \ // does NOT
MMU_FTR_CI_LARGE_PAGE | MMU_FTR_NO_SLBIE_B

So it's only 970, Cell and Pasemi that *don't* have lockless TLBIE.

Quoted 8 lines
Quoted 5 lines

And KVM HV doesn't doesn't run on any of those.

So we can just not check for the feature in the KVM HV code.

Am I right?

Yes; that code was written when we still supported HV KVM on 970, but we ripped that out some time ago.

OK good, in commit:

c17b98cf6028 ("KVM: PPC: Book3S HV: Remove code for PPC970 processors") (Dec 2014)

So we should be able to do the patch below.

cheers

arch/powerpc/include/asm/kvm_host.h +0 -1
index 17498e9a26e4..7756b0c6da75 100644
@@ -269,7 +269,6 @@ struct kvm_arch {
 	unsigned long host_lpcr;
 	unsigned long sdr1;
 	unsigned long host_sdr1;
-	int tlbie_lock;
 	unsigned long lpcr;
 	unsigned long vrma_slb_v;
 	int mmu_ready;
arch/powerpc/kvm/book3s_hv_rm_mmu.c +0 -21
index 78e6a392330f..89d909b3b881 100644
@@ -434,24 +434,6 @@ static inline int is_mmio_hpte(unsigned long v, unsigned long r)
 		(HPTE_R_KEY_HI | HPTE_R_KEY_LO));
 }
 
-static inline int try_lock_tlbie(unsigned int *lock)
-{
-	unsigned int tmp, old;
-	unsigned int token = LOCK_TOKEN;
-
-	asm volatile("1:lwarx	%1,0,%2\n"
-		     "	cmpwi	cr0,%1,0\n"
-		     "	bne	2f\n"
-		     "  stwcx.	%3,0,%2\n"
-		     "	bne-	1b\n"
-		     "  isync\n"
-		     "2:"
-		     : "=&r" (tmp), "=&r" (old)
-		     : "r" (lock), "r" (token)
-		     : "cc", "memory");
-	return old = 0;
-}
-
 static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		      long npages, int global, bool need_sync)
 {

@@ -463,8 +445,6 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 	 * the RS field, this is backwards-compatible with P7 and P8.
 	 */
 	if (global) {
-		while (!try_lock_tlbie(&kvm->arch.tlbie_lock))
-			cpu_relax();
 		if (need_sync)
 			asm volatile("ptesync" : : : "memory");
 		for (i = 0; i < npages; ++i) {

@@ -483,7 +463,6 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		}
 
 		asm volatile("eieio; tlbsync; ptesync" : : : "memory");
-		kvm->arch.tlbie_lock = 0;
 	} else {
 		if (need_sync)
 			asm volatile("ptesync" : : : "memory");
#13 lore raw

Re: [PATCH 2/2] KVM: PPC: Book3S HV: lockless tlbie for HPT hcalls

On Mon, May 14, 2018 at 02:04:10PM +1000, Michael Ellerman wrote:
[snip]

Quoted 60 lines

OK good, in commit:

c17b98cf6028 ("KVM: PPC: Book3S HV: Remove code for PPC970 processors") (Dec 2014)

So we should be able to do the patch below.

cheers

arch/powerpc/include/asm/kvm_host.h +0 -1
index 17498e9a26e4..7756b0c6da75 100644
@@ -269,7 +269,6 @@ struct kvm_arch {
 	unsigned long host_lpcr;
 	unsigned long sdr1;
 	unsigned long host_sdr1;
-	int tlbie_lock;
 	unsigned long lpcr;
 	unsigned long vrma_slb_v;
 	int mmu_ready;
arch/powerpc/kvm/book3s_hv_rm_mmu.c +0 -20
index 78e6a392330f..89d909b3b881 100644
@@ -434,24 +434,6 @@ static inline int is_mmio_hpte(unsigned long v, unsigned long r)
 		(HPTE_R_KEY_HI | HPTE_R_KEY_LO));
 }
 
-static inline int try_lock_tlbie(unsigned int *lock)
-{
-	unsigned int tmp, old;
-	unsigned int token = LOCK_TOKEN;
-
-	asm volatile("1:lwarx	%1,0,%2\n"
-		     "	cmpwi	cr0,%1,0\n"
-		     "	bne	2f\n"
-		     "  stwcx.	%3,0,%2\n"
-		     "	bne-	1b\n"
-		     "  isync\n"
-		     "2:"
-		     : "=&r" (tmp), "=&r" (old)
-		     : "r" (lock), "r" (token)
-		     : "cc", "memory");
-	return old = 0;
-}
-
 static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		      long npages, int global, bool need_sync)
 {

@@ -463,8 +445,6 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 	 * the RS field, this is backwards-compatible with P7 and P8.
 	 */
 	if (global) {
-		while (!try_lock_tlbie(&kvm->arch.tlbie_lock))
-			cpu_relax();
 		if (need_sync)
 			asm volatile("ptesync" : : : "memory");
 		for (i = 0; i < npages; ++i) {

arch/powerpc/kvm/book3s_hv_rm_mmu.c static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues, · line 468

Quoted 7 lines
arch/powerpc/kvm/book3s_hv_rm_mmu.c +0 -1
@@ -483,7 +463,6 @@ static void do_tlbies(struct kvm *kvm, unsigned long *rbvalues,
 		}
 
 		asm volatile("eieio; tlbsync; ptesync" : : : "memory");
-		kvm->arch.tlbie_lock = 0;
 	} else {
 		if (need_sync)
 			asm volatile("ptesync" : : : "memory");

Seems reasonable; is that a patch submission? :)

Paul.