14 messages, 6 authors, 2018-05-17 · open the first message on its own page
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-04-05 17:56:44
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(-) -- 2.16.3
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-04-05 17:56:47
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=2048 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 <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index e1c083fbe434..78e6a392330f 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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"); }
--
2.16.3
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-04-05 17:56:50
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 <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv_rm_mmu.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 78e6a392330f..0221a0f74f07 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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");
--
2.16.3
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-04-06 05:39:44
On Fri, 6 Apr 2018 03:56:31 +1000 Nicholas Piggin [off-list ref] 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 <npiggin@gmail.com> --- arch/powerpc/kvm/book3s_hv_rm_mmu.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index 78e6a392330f..0221a0f74f07 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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);
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
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2018-04-06 06:12:32
Nicholas Piggin [off-list ref] writes:
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index 78e6a392330f..0221a0f74f07 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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; +}
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
From: Balbir Singh <bsingharora@gmail.com>
Date: 2018-04-08 10:17:50
On Fri, Apr 6, 2018 at 3:56 AM, Nicholas Piggin [off-list ref] wrote:
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=2048 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.
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.
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-04-08 13:42:12
On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh [off-list ref] wrote:
On Fri, Apr 6, 2018 at 3:56 AM, Nicholas Piggin [off-list ref] wrote:quoted
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=2048 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.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.
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
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2018-04-10 03:21:19
Nicholas Piggin [off-list ref] writes:
On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh [off-list ref] wrote:quoted
On Fri, Apr 6, 2018 at 3:56 AM, Nicholas Piggin [off-list ref] wrote:quoted
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=2048 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.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
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
From: Naveen N. Rao <hidden>
Date: 2018-04-10 05:55:14
Michael Ellerman wrote:
Nicholas Piggin [off-list ref] writes: =20quoted
On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh [off-list ref] wrote:quoted
On Fri, Apr 6, 2018 at 3:56 AM, Nicholas Piggin [off-list ref] wro=
te:
quoted
quoted
quoted
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=3D2048 NUMA PowerNV CPU: 53 PID: 6582 Comm: qemu-system-ppc Not tainted 4.16.0-01530-g4=
3d1859f0994
quoted
quoted
quoted
NIP: c0000000000155ac LR: c0000000000c2430 CTR: c000000000015580 REGS: c000000fff76dd80 TRAP: 0200 Not tainted (4.16.0-01530-g43d=
1859f0994)
quoted
quoted
quoted
MSR: 9000000000201003 <SF,HV,ME,RI,LE> CR: 48082222 XER: 0000000=
0
quoted
quoted
quoted
CFAR: 0000000102900ef0 DAR: d00017fffd941a28 DSISR: 00000040 SOFTE:=
3
quoted
quoted
quoted
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. =20=20 Could you share the stack trace as well? I've not observed this in my t=
esting.
quoted
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
May be I don't have as many cpus. I presume your talking about the per =
cpu
quoted
quoted
data offsets for per cpu trace data?It looked like it was dereferencing virtually mapped per-cpu data, yes. Probably the perf_events deref.=20 Naveen has posted a series to (hopefully) fix this, which just missed the merge window: =20 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=20 static tracepoints. We could convert trace_tlbie() to a TRACE_EVENT_CONDITION() and guard it=20 within a check for paca->ftrace_enabled, but that would only be useful=20 if the below callsites can ever be hit outside of KVM guest mode. - Naveen =
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2018-04-10 06:10:51
On Tue, 10 Apr 2018 11:25:02 +0530 "Naveen N. Rao" [off-list ref] wrote:
Michael Ellerman wrote:quoted
Nicholas Piggin [off-list ref] writes:quoted
On Sun, 8 Apr 2018 20:17:47 +1000 Balbir Singh [off-list ref] wrote:quoted
On Fri, Apr 6, 2018 at 3:56 AM, Nicholas Piggin [off-list ref] wrote:quoted
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=2048 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.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
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/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
From: Michael Ellerman <hidden>
Date: 2018-04-11 14:49:30
On Thu, 2018-04-05 at 17:56:30 UTC, Nicholas Piggin wrote:
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=2048 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 <npiggin@gmail.com>Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/19ce7909ed11c49f7eddf59e7f49cd cheers
From: Paul Mackerras <hidden>
Date: 2018-05-10 09:57:42
On Fri, Apr 06, 2018 at 04:12:32PM +1000, Michael Ellerman wrote:
Nicholas Piggin [off-list ref] writes:quoted
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index 78e6a392330f..0221a0f74f07 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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; +}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?
Yes; that code was written when we still supported HV KVM on 970, but we ripped that out some time ago. Paul.
From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2018-05-14 04:04:15
Paul Mackerras [off-list ref] writes:
On Fri, Apr 06, 2018 at 04:12:32PM +1000, Michael Ellerman wrote:quoted
Nicholas Piggin [off-list ref] writes:quoted
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index 78e6a392330f..0221a0f74f07 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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; +}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?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
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 17498e9a26e4..7756b0c6da75 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h@@ -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;
diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c
index 78e6a392330f..89d909b3b881 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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");
From: Paul Mackerras <hidden>
Date: 2018-05-17 03:54:00
On Mon, May 14, 2018 at 02:04:10PM +1000, Michael Ellerman wrote: [snip]
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. cheersdiff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 17498e9a26e4..7756b0c6da75 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h@@ -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;diff --git a/arch/powerpc/kvm/book3s_hv_rm_mmu.c b/arch/powerpc/kvm/book3s_hv_rm_mmu.c index 78e6a392330f..89d909b3b881 100644 --- a/arch/powerpc/kvm/book3s_hv_rm_mmu.c +++ b/arch/powerpc/kvm/book3s_hv_rm_mmu.c@@ -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");
Seems reasonable; is that a patch submission? :) Paul.