From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:36:30
This is a combination of ~2 series to fix bugs in the perf+KVM callbacks,
optimize the callbacks by employing static_call, and do a variety of
cleanup in both perf and KVM.
Patch 1 fixes a mostly-theoretical bug where perf can deref a NULL
pointer if KVM unregisters its callbacks while they're being accessed.
In practice, compilers tend to avoid problematic reloads of the pointer
and the PMI handler doesn't lose the race against module unloading,
i.e doesn't hit a use-after-free.
Patches 2 and 3 fix an Intel PT handling bug where KVM incorrectly
eats PT interrupts when PT is supposed to be owned entirely by the host.
Patches 4-7 clean up perf's callback infrastructure and switch to
static_call for arm64 and x86 (the only survivors).
Patches 8-13 clean up related KVM code and unify the arm64/x86 callbacks.
Based on "git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue", commit
680c7e3be6a3 ("KVM: x86: Exit to userspace ...").
v2 (relatively to static_call v10)
- Split the patch into the semantic change (multiplexed ->state) and
introduction of static_call.
- Don't use '0' for "not a guest RIP".
- Handle unregister path.
- Drop changes for architectures that can be culled entirely.
v2 (relative to v1)
- Drop per-cpu approach. [Peter]
- Fix mostly-theoretical reload and use-after-free with READ_ONCE(),
WRITE_ONCE(), and synchronize_rcu(). [Peter]
- Avoid new exports like the plague. [Peter]
v1:
- https://lkml.kernel.org/r/20210827005718.585190-1-seanjc@google.com
v10 static_call:
- https://lkml.kernel.org/r/20210806133802.3528-2-lingshan.zhu@intel.com
Like Xu (2):
perf/core: Rework guest callbacks to prepare for static_call support
perf/core: Use static_call to optimize perf_guest_info_callbacks
Sean Christopherson (11):
perf: Ensure perf_guest_cbs aren't reloaded between !NULL check and
deref
KVM: x86: Register perf callbacks after calling vendor's
hardware_setup()
KVM: x86: Register Processor Trace interrupt hook iff PT enabled in
guest
perf: Stop pretending that perf can handle multiple guest callbacks
perf: Force architectures to opt-in to guest callbacks
KVM: x86: Drop current_vcpu for kvm_running_vcpu + kvm_arch_vcpu
variable
KVM: x86: More precisely identify NMI from guest when handling PMI
KVM: Move x86's perf guest info callbacks to generic KVM
KVM: x86: Move Intel Processor Trace interrupt handler to vmx.c
KVM: arm64: Convert to the generic perf callbacks
KVM: arm64: Drop perf.c and fold its tiny bits of code into arm.c /
pmu.c
arch/arm/kernel/perf_callchain.c | 28 ++------------
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/kvm_host.h | 13 ++++++-
arch/arm64/kernel/perf_callchain.c | 28 +++++++++++---
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/arm.c | 11 +++++-
arch/arm64/kvm/perf.c | 62 ------------------------------
arch/arm64/kvm/pmu.c | 8 ++++
arch/csky/kernel/perf_callchain.c | 10 -----
arch/nds32/kernel/perf_event_cpu.c | 29 ++------------
arch/riscv/kernel/perf_callchain.c | 10 -----
arch/x86/Kconfig | 1 +
arch/x86/events/core.c | 36 ++++++++++++++---
arch/x86/events/intel/core.c | 7 ++--
arch/x86/include/asm/kvm_host.h | 8 +++-
arch/x86/kvm/pmu.c | 2 +-
arch/x86/kvm/svm/svm.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 25 +++++++++++-
arch/x86/kvm/x86.c | 58 +++++-----------------------
arch/x86/kvm/x86.h | 17 ++++++--
arch/x86/xen/pmu.c | 32 +++++++--------
include/kvm/arm_pmu.h | 1 +
include/linux/kvm_host.h | 10 +++++
include/linux/perf_event.h | 26 ++++++++-----
init/Kconfig | 3 ++
kernel/events/core.c | 24 ++++++------
virt/kvm/kvm_main.c | 40 +++++++++++++++++++
27 files changed, 245 insertions(+), 249 deletions(-)
delete mode 100644 arch/arm64/kvm/perf.c
--
2.33.0.259.gc128427fd7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:36:35
Protect perf_guest_cbs with READ_ONCE/WRITE_ONCE to ensure it's not
reloaded between a !NULL check and a dereference, and wait for all
readers via syncrhonize_rcu() to prevent use-after-free, e.g. if the
callbacks are being unregistered during module unload. Because the
callbacks are global, it's possible for readers to run in parallel with
an unregister operation.
The bug has escaped notice because all dereferences of perf_guest_cbs
follow the same "perf_guest_cbs && perf_guest_cbs->is_in_guest()" pattern,
and it's extremely unlikely a compiler will reload perf_guest_cbs in this
sequence. Compilers do reload perf_guest_cbs for future derefs, e.g. for
->is_user_mode(), but the ->is_in_guest() guard all but guarantees the
PMI handler will win the race, e.g. to nullify perf_guest_cbs, KVM has to
completely exit the guest and teardown down all VMs before KVM start its
module unload / unregister sequence.
But with help, unloading kvm_intel can trigger a NULL pointer derference,
e.g. wrapping perf_guest_cbs with READ_ONCE in perf_misc_flags() while
spamming kvm_intel module load/unload leads to:
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] PREEMPT SMP
CPU: 6 PID: 1825 Comm: stress Not tainted 5.14.0-rc2+ #459
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
RIP: 0010:perf_misc_flags+0x1c/0x70
Call Trace:
perf_prepare_sample+0x53/0x6b0
perf_event_output_forward+0x67/0x160
__perf_event_overflow+0x52/0xf0
handle_pmi_common+0x207/0x300
intel_pmu_handle_irq+0xcf/0x410
perf_event_nmi_handler+0x28/0x50
nmi_handle+0xc7/0x260
default_do_nmi+0x6b/0x170
exc_nmi+0x103/0x130
asm_exc_nmi+0x76/0xbf
Fixes: 39447b386c84 ("perf: Enhance perf to allow for guest statistic collection from host")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm/kernel/perf_callchain.c | 17 +++++++++++------
arch/arm64/kernel/perf_callchain.c | 18 ++++++++++++------
arch/csky/kernel/perf_callchain.c | 6 ++++--
arch/nds32/kernel/perf_event_cpu.c | 17 +++++++++++------
arch/riscv/kernel/perf_callchain.c | 7 +++++--
arch/x86/events/core.c | 17 +++++++++++------
arch/x86/events/intel/core.c | 9 ++++++---
include/linux/perf_event.h | 8 ++++++++
kernel/events/core.c | 9 +++++++--
9 files changed, 75 insertions(+), 33 deletions(-)
@@ -62,9 +62,10 @@ user_backtrace(struct frame_tail __user *tail,voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structframe_tail__user*tail;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){/* We don't support guest os callchain now */return;}
@@ -98,9 +99,10 @@ callchain_trace(struct stackframe *fr,voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframefr;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){/* We don't support guest os callchain now */return;}
@@ -102,7 +102,9 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();++if(guest_cbs&&guest_cbs->is_in_guest()){/* We don't support guest os callchain now */return;}
@@ -147,9 +149,10 @@ static bool callchain_trace(void *data, unsigned long pc)voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframeframe;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){/* We don't support guest os callchain now */return;}
@@ -86,10 +86,11 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();unsignedlongfp=0;/* C-SKY does not support virtualization. */-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest())+if(guest_cbs&&guest_cbs->is_in_guest())return;fp=regs->regs[4];
@@ -110,10 +111,11 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframefr;/* C-SKY does not support virtualization. */-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){pr_warn("C-SKY does not support perf in guest mode!");return;}
@@ -1371,7 +1372,7 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry,leaf_fp=0;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){/* We don't support guest os callchain now */return;}
@@ -1479,9 +1480,10 @@ voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframefr;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){/* We don't support guest os callchain now */return;}
@@ -1493,20 +1495,23 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,unsignedlongperf_instruction_pointer(structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();+/* However, NDS32 does not support virtualization */-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest())-returnperf_guest_cbs->get_guest_ip();+if(guest_cbs&&guest_cbs->is_in_guest())+returnguest_cbs->get_guest_ip();returninstruction_pointer(regs);}unsignedlongperf_misc_flags(structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();intmisc=0;/* However, NDS32 does not support virtualization */-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){-if(perf_guest_cbs->is_user_mode())+if(guest_cbs&&guest_cbs->is_in_guest()){+if(guest_cbs->is_user_mode())misc|=PERF_RECORD_MISC_GUEST_USER;elsemisc|=PERF_RECORD_MISC_GUEST_KERNEL;
@@ -56,10 +56,11 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();unsignedlongfp=0;/* RISC-V does not support perf in guest mode. */-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest())+if(guest_cbs&&guest_cbs->is_in_guest())return;fp=regs->s0;
@@ -78,8 +79,10 @@ static bool fill_callchain(void *entry, unsigned long pc)voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();+/* RISC-V does not support perf in guest mode. */-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){pr_warn("RISC-V does not support perf in guest mode!");return;}
@@ -2761,10 +2761,11 @@ static bool perf_hw_regs(struct pt_regs *regs)voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structunwind_statestate;unsignedlongaddr;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){/* TODO: We don't support guest os callchain now */return;}
@@ -2864,10 +2865,11 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *entvoidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstack_frameframe;conststructstack_frame__user*fp;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->is_in_guest()){/* TODO: We don't support guest os callchain now */return;}
@@ -2944,18 +2946,21 @@ static unsigned long code_segment_base(struct pt_regs *regs)unsignedlongperf_instruction_pointer(structpt_regs*regs){-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest())-returnperf_guest_cbs->get_guest_ip();+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();++if(guest_cbs&&guest_cbs->is_in_guest())+returnguest_cbs->get_guest_ip();returnregs->ip+code_segment_base(regs);}unsignedlongperf_misc_flags(structpt_regs*regs){+structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();intmisc=0;-if(perf_guest_cbs&&perf_guest_cbs->is_in_guest()){-if(perf_guest_cbs->is_user_mode())+if(guest_cbs&&guest_cbs->is_in_guest()){+if(guest_cbs->is_user_mode())misc|=PERF_RECORD_MISC_GUEST_USER;elsemisc|=PERF_RECORD_MISC_GUEST_KERNEL;
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:36:43
Wait to register perf callbacks until after doing vendor hardaware setup.
VMX's hardware_setup() configures Intel Processor Trace (PT) mode, and a
future fix to register the Intel PT guest interrupt hook if and only if
Intel PT is exposed to the guest will consume the configured PT mode.
Delaying registration to hardware setup is effectively a nop as KVM's perf
hooks all pivot on the per-CPU current_vcpu, which is non-NULL only when
KVM is handling an IRQ/NMI in a VM-Exit path. I.e. current_vcpu will be
NULL throughout both kvm_arch_init() and kvm_arch_hardware_setup().
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Artem Kashkanov <redacted>
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:36:46
Override the Processor Trace (PT) interrupt handler for guest mode if and
only if PT is configured for host+guest mode, i.e. is being used
independently by both host and guest. If PT is configured for system
mode, the host fully controls PT and must handle all events.
Fixes: 8479e04e7d6b ("KVM: x86: Inject PMI for KVM guest")
Cc: stable@vger.kernel.org
Cc: Like Xu <redacted>
Reported-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reported-by: Artem Kashkanov <redacted>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/vmx/vmx.c | 1 +
arch/x86/kvm/x86.c | 5 ++++-
3 files changed, 6 insertions(+), 1 deletion(-)
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:36:50
Drop the 'int' return value from the perf (un)register callbacks helpers
and stop pretending perf can support multiple callbacks. The 'int'
returns are not future proofing anything as none of the callers take
action on an error. It's also not obvious that there will ever be
co-tenant hypervisors, and if there are, that allowing multiple callbacks
to be registered is desirable or even correct.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm64/include/asm/kvm_host.h | 4 ++--
arch/arm64/kvm/perf.c | 8 ++++----
arch/x86/kvm/x86.c | 2 +-
include/linux/perf_event.h | 11 +++++------
kernel/events/core.c | 14 +++-----------
5 files changed, 15 insertions(+), 24 deletions(-)
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:36:53
From: Like Xu <redacted>
Use static_call to optimize perf's guest callbacks on arm64 and x86,
which are now the only architectures that define the callbacks. Use
DEFINE_STATIC_CALL_RET0 as the default/NULL for all guest callbacks, as
the callback semantics are that a return value '0' means "not in guest".
static_call obviously avoids the overhead of CONFIG_RETPOLINE=y, but is
also advantageous versus other solutions, e.g. per-cpu callbacks, in that
a per-cpu memory load is not needed to detect the !guest case.
Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Originally-by: Peter Zijlstra (Intel) [off-list ref]
Signed-off-by: Like Xu <redacted>
Signed-off-by: Zhu Lingshan <redacted>
[sean: split out patch, drop __weak, tweak updaters, rewrite changelog]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm64/kernel/perf_callchain.c | 31 +++++++++++++++---------
arch/x86/events/core.c | 38 ++++++++++++++++++++++--------
arch/x86/events/intel/core.c | 7 +++---
include/linux/perf_event.h | 9 +------
kernel/events/core.c | 2 ++
5 files changed, 54 insertions(+), 33 deletions(-)
@@ -99,12 +100,24 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,}#endif /* CONFIG_COMPAT */+DEFINE_STATIC_CALL_RET0(arm64_guest_state,*(perf_guest_cbs->state));+DEFINE_STATIC_CALL_RET0(arm64_guest_get_ip,*(perf_guest_cbs->get_ip));++voidarch_perf_update_guest_cbs(structperf_guest_info_callbacks*guest_cbs)+{+if(guest_cbs){+static_call_update(arm64_guest_state,guest_cbs->state);+static_call_update(arm64_guest_get_ip,guest_cbs->get_ip);+}else{+static_call_update(arm64_guest_state,(void*)&__static_call_return0);+static_call_update(arm64_guest_get_ip,(void*)&__static_call_return0);+}+}+voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();--if(guest_cbs&&guest_cbs->state()){+if(static_call(arm64_guest_state)()){/* We don't support guest os callchain now */return;}
@@ -149,10 +162,9 @@ static bool callchain_trace(void *data, unsigned long pc)voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframeframe;-if(guest_cbs&&guest_cbs->state()){+if(static_call(arm64_guest_state)()){/* We don't support guest os callchain now */return;}
@@ -2761,11 +2784,10 @@ static bool perf_hw_regs(struct pt_regs *regs)voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structunwind_statestate;unsignedlongaddr;-if(guest_cbs&&guest_cbs->state()){+if(static_call(x86_guest_state)()){/* TODO: We don't support guest os callchain now */return;}
@@ -2865,11 +2887,10 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *entvoidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstack_frameframe;conststructstack_frame__user*fp;-if(guest_cbs&&guest_cbs->state()){+if(static_call(x86_guest_state)()){/* TODO: We don't support guest os callchain now */return;}
@@ -2946,18 +2967,15 @@ static unsigned long code_segment_base(struct pt_regs *regs)unsignedlongperf_instruction_pointer(structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();--if(guest_cbs&&guest_cbs->state())-returnguest_cbs->get_ip();+if(static_call(x86_guest_state)())+returnstatic_call(x86_guest_get_ip)();returnregs->ip+code_segment_base(regs);}unsignedlongperf_misc_flags(structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();-unsignedintguest_state=guest_cbs?guest_cbs->state():0;+unsignedintguest_state=static_call(x86_guest_state)();intmisc=0;if(guest_state){
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:37:00
Use the generic kvm_running_vcpu plus a new 'handling_intr_from_guest'
variable in kvm_arch_vcpu instead of the semi-redundant current_vcpu.
kvm_before/after_interrupt() must be called while the vCPU is loaded,
(which protects against preemption), thus kvm_running_vcpu is guaranteed
to be non-NULL when handling_intr_from_guest is non-zero.
Switching to kvm_get_running_vcpu() will allows moving KVM's perf
callbacks to generic code, and the new flag will be used in a future
patch to more precisely identify the "NMI from guest" case.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 3 +--
arch/x86/kvm/pmu.c | 2 +-
arch/x86/kvm/x86.c | 21 ++++++++++++---------
arch/x86/kvm/x86.h | 10 ++++++----
4 files changed, 20 insertions(+), 16 deletions(-)
@@ -763,6 +763,7 @@ struct kvm_vcpu_arch {unsignednmi_pending;/* NMI queued after currently running handler */boolnmi_injected;/* Trying to inject an NMI this entry */boolsmi_pending;/* SMI queued after currently running handler */+u8handling_intr_from_guest;structkvm_mtrrmtrr_state;u64pat;
@@ -1874,8 +1875,6 @@ int kvm_skip_emulated_instruction(struct kvm_vcpu *vcpu);intkvm_complete_insn_gp(structkvm_vcpu*vcpu,interr);void__kvm_request_immediate_exit(structkvm_vcpu*vcpu);-unsignedintkvm_guest_state(void);-void__user*__x86_set_memory_region(structkvm*kvm,intid,gpa_tgpa,u32size);boolkvm_vcpu_is_reset_bsp(structkvm_vcpu*vcpu);
@@ -8284,9 +8286,10 @@ unsigned int kvm_guest_state(void)staticunsignedlongkvm_guest_get_ip(void){-structkvm_vcpu*vcpu=__this_cpu_read(current_vcpu);+structkvm_vcpu*vcpu=kvm_get_running_vcpu();-if(WARN_ON_ONCE(!vcpu))+/* Retrieving the IP must be guarded by a call to kvm_guest_state(). */+if(WARN_ON_ONCE(!kvm_pmi_in_guest(vcpu)))return0;returnkvm_rip_read(vcpu);
@@ -8294,10 +8297,10 @@ static unsigned long kvm_guest_get_ip(void)staticunsignedintkvm_handle_intel_pt_intr(void){-structkvm_vcpu*vcpu=__this_cpu_read(current_vcpu);+structkvm_vcpu*vcpu=kvm_get_running_vcpu();/* '0' on failure so that the !PT case can use a RET0 static call. */-if(!vcpu)+if(!kvm_pmi_in_guest(vcpu))return0;kvm_make_request(KVM_REQ_PMI,vcpu);
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:37:09
From: Like Xu <redacted>
To prepare for using static_calls to optimize perf's guest callbacks,
replace ->is_in_guest and ->is_user_mode with a new multiplexed hook
->state, tweak ->handle_intel_pt_intr to play nice with being called when
there is no active guest, and drop "guest" from ->is_in_guest.
Return '0' from ->state and ->handle_intel_pt_intr to indicate "not in
guest" so that DEFINE_STATIC_CALL_RET0 can be used to define the static
calls, i.e. no callback == !guest.
Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Originally-by: Peter Zijlstra (Intel) [off-list ref]
Signed-off-by: Like Xu <redacted>
Signed-off-by: Zhu Lingshan <redacted>
[sean: extracted from static_call patch, fixed get_ip() bug, wrote changelog]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm64/kernel/perf_callchain.c | 13 +++++-----
arch/arm64/kvm/perf.c | 35 +++++++++++---------------
arch/x86/events/core.c | 13 +++++-----
arch/x86/events/intel/core.c | 5 +---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/pmu.c | 2 +-
arch/x86/kvm/x86.c | 40 ++++++++++++++++--------------
arch/x86/xen/pmu.c | 32 ++++++++++--------------
include/linux/perf_event.h | 10 +++++---
kernel/events/core.c | 1 +
10 files changed, 74 insertions(+), 79 deletions(-)
@@ -104,7 +104,7 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,{structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();-if(guest_cbs&&guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->state()){/* We don't support guest os callchain now */return;}
@@ -152,7 +152,7 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframeframe;-if(guest_cbs&&guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->state()){/* We don't support guest os callchain now */return;}
@@ -165,8 +165,8 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs){structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();-if(guest_cbs&&guest_cbs->is_in_guest())-returnguest_cbs->get_guest_ip();+if(guest_cbs&&guest_cbs->state())+returnguest_cbs->get_ip();returninstruction_pointer(regs);}
@@ -174,10 +174,11 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)unsignedlongperf_misc_flags(structpt_regs*regs){structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();+unsignedintguest_state=guest_cbs?guest_cbs->state():0;intmisc=0;-if(guest_cbs&&guest_cbs->is_in_guest()){-if(guest_cbs->is_user_mode())+if(guest_state){+if(guest_state&PERF_GUEST_USER)misc|=PERF_RECORD_MISC_GUEST_USER;elsemisc|=PERF_RECORD_MISC_GUEST_KERNEL;
@@ -2765,7 +2765,7 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *restructunwind_statestate;unsignedlongaddr;-if(guest_cbs&&guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->state()){/* TODO: We don't support guest os callchain now */return;}
@@ -2869,7 +2869,7 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regsstructstack_frameframe;conststructstack_frame__user*fp;-if(guest_cbs&&guest_cbs->is_in_guest()){+if(guest_cbs&&guest_cbs->state()){/* TODO: We don't support guest os callchain now */return;}
@@ -2948,8 +2948,8 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs){structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();-if(guest_cbs&&guest_cbs->is_in_guest())-returnguest_cbs->get_guest_ip();+if(guest_cbs&&guest_cbs->state())+returnguest_cbs->get_ip();returnregs->ip+code_segment_base(regs);}
@@ -2957,10 +2957,11 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)unsignedlongperf_misc_flags(structpt_regs*regs){structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();+unsignedintguest_state=guest_cbs?guest_cbs->state():0;intmisc=0;-if(guest_cbs&&guest_cbs->is_in_guest()){-if(guest_cbs->is_user_mode())+if(guest_state){+if(guest_state&PERF_GUEST_USER)misc|=PERF_RECORD_MISC_GUEST_USER;elsemisc|=PERF_RECORD_MISC_GUEST_KERNEL;
@@ -8267,44 +8267,48 @@ static void kvm_timer_init(void)DEFINE_PER_CPU(structkvm_vcpu*,current_vcpu);EXPORT_PER_CPU_SYMBOL_GPL(current_vcpu);-intkvm_is_in_guest(void)+unsignedintkvm_guest_state(void){-return__this_cpu_read(current_vcpu)!=NULL;-}+structkvm_vcpu*vcpu=__this_cpu_read(current_vcpu);+unsignedintstate;-staticintkvm_is_user_mode(void)-{-intuser_mode=3;+if(!vcpu)+return0;-if(__this_cpu_read(current_vcpu))-user_mode=static_call(kvm_x86_get_cpl)(__this_cpu_read(current_vcpu));+state=PERF_GUEST_ACTIVE;+if(static_call(kvm_x86_get_cpl)(vcpu))+state|=PERF_GUEST_USER;-returnuser_mode!=0;+returnstate;}-staticunsignedlongkvm_get_guest_ip(void)+staticunsignedlongkvm_guest_get_ip(void){-unsignedlongip=0;+structkvm_vcpu*vcpu=__this_cpu_read(current_vcpu);-if(__this_cpu_read(current_vcpu))-ip=kvm_rip_read(__this_cpu_read(current_vcpu));+if(WARN_ON_ONCE(!vcpu))+return0;-returnip;+returnkvm_rip_read(vcpu);}-staticvoidkvm_handle_intel_pt_intr(void)+staticunsignedintkvm_handle_intel_pt_intr(void){structkvm_vcpu*vcpu=__this_cpu_read(current_vcpu);+/* '0' on failure so that the !PT case can use a RET0 static call. */+if(!vcpu)+return0;+kvm_make_request(KVM_REQ_PMI,vcpu);__set_bit(MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT,(unsignedlong*)&vcpu->arch.pmu.global_status);+return1;}staticstructperf_guest_info_callbackskvm_guest_cbs={-.is_in_guest=kvm_is_in_guest,-.is_user_mode=kvm_is_user_mode,-.get_guest_ip=kvm_get_guest_ip,+.state=kvm_guest_state,+.get_ip=kvm_guest_get_ip,.handle_intel_pt_intr=NULL,};
@@ -413,34 +413,29 @@ int pmu_apic_update(uint32_t val)}/* perf callbacks */-staticintxen_is_in_guest(void)+staticunsignedintxen_guest_state(void){conststructxen_pmu_data*xenpmu_data=get_xenpmu_data();+unsignedintstate=0;if(!xenpmu_data){pr_warn_once("%s: pmudata not initialized\n",__func__);-return0;+returnstate;}if(!xen_initial_domain()||(xenpmu_data->domain_id>=DOMID_SELF))-return0;+returnstate;-return1;-}+state|=PERF_GUEST_ACTIVE;-staticintxen_is_user_mode(void)-{-conststructxen_pmu_data*xenpmu_data=get_xenpmu_data();--if(!xenpmu_data){-pr_warn_once("%s: pmudata not initialized\n",__func__);-return0;+if(xenpmu_data->pmu.pmu_flags&PMU_SAMPLE_PV){+if(xenpmu_data->pmu.pmu_flags&PMU_SAMPLE_USER)+state|=PERF_GUEST_USER;+}elseif(xenpmu_data->pmu.r.regs.cpl&3){+state|=PERF_GUEST_USER;}-if(xenpmu_data->pmu.pmu_flags&PMU_SAMPLE_PV)-return(xenpmu_data->pmu.pmu_flags&PMU_SAMPLE_USER);-else-return!!(xenpmu_data->pmu.r.regs.cpl&3);+returnstate;}staticunsignedlongxen_get_guest_ip(void)
@@ -456,9 +451,8 @@ static unsigned long xen_get_guest_ip(void)}staticstructperf_guest_info_callbacksxen_guest_cbs={-.is_in_guest=xen_is_in_guest,-.is_user_mode=xen_is_user_mode,-.get_guest_ip=xen_get_guest_ip,+.state=xen_guest_state,+.get_ip=xen_get_guest_ip,};/* Convert registers from Xen's format to Linux' */
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:37:11
Differntiate between IRQ and NMI for KVM's PMC overflow callback, which
was originally invoked in response to an NMI that arrived while the guest
was running, but was inadvertantly changed to fire on IRQs as well when
support for perf without PMU/NMI was added to KVM. In practice, this
should be a nop as the PMC overflow callback shouldn't be reached, but
it's a cheap and easy fix that also better documents the situation.
Note, this also doesn't completely prevent false positives if perf
somehow ends up calling into KVM, e.g. an NMI can arrive in host after
KVM sets its flag.
Fixes: dd60d217062f ("KVM: x86: Fix perf timer mode IP reporting")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/svm.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 4 +++-
arch/x86/kvm/x86.c | 2 +-
arch/x86/kvm/x86.h | 13 ++++++++++---
4 files changed, 15 insertions(+), 6 deletions(-)
@@ -387,9 +387,16 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)returnkvm->arch.cstate_in_guest;}-staticinlinevoidkvm_before_interrupt(structkvm_vcpu*vcpu)+enumkvm_intr_type{+/* Values are arbitrary, but must be non-zero. */+KVM_HANDLING_IRQ=1,+KVM_HANDLING_NMI,+};++staticinlinevoidkvm_before_interrupt(structkvm_vcpu*vcpu,+enumkvm_intr_typeintr){-WRITE_ONCE(vcpu->arch.handling_intr_from_guest,1);+WRITE_ONCE(vcpu->arch.handling_intr_from_guest,(u8)intr);}staticinlinevoidkvm_after_interrupt(structkvm_vcpu*vcpu)
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:37:15
Introduce HAVE_GUEST_PERF_EVENTS and require architectures to select it
to allow registering guest callbacks in perf. Future patches will convert
the callbacks to static_call. Rather than churn a bunch of arch code (that
was presumably copy+pasted from x86), remove it wholesale as it's useless
and at best wasting cycles.
Wrap even the stubs with an #ifdef to avoid an arch sneaking in a bogus
registration with CONFIG_PERF_EVENTS=n.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm/kernel/perf_callchain.c | 33 ++++-------------------------
arch/arm64/Kconfig | 1 +
arch/csky/kernel/perf_callchain.c | 12 -----------
arch/nds32/kernel/perf_event_cpu.c | 34 ++++--------------------------
arch/riscv/kernel/perf_callchain.c | 13 ------------
arch/x86/Kconfig | 1 +
include/linux/perf_event.h | 4 ++++
init/Kconfig | 3 +++
kernel/events/core.c | 2 ++
9 files changed, 19 insertions(+), 84 deletions(-)
@@ -62,14 +62,8 @@ user_backtrace(struct frame_tail __user *tail,voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structframe_tail__user*tail;-if(guest_cbs&&guest_cbs->is_in_guest()){-/* We don't support guest os callchain now */-return;-}-perf_callchain_store(entry,regs->ARM_pc);if(!current->mm)
@@ -99,44 +93,25 @@ callchain_trace(struct stackframe *fr,voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframefr;-if(guest_cbs&&guest_cbs->is_in_guest()){-/* We don't support guest os callchain now */-return;-}-arm_get_current_stackframe(regs,&fr);walk_stackframe(&fr,callchain_trace,entry);}unsignedlongperf_instruction_pointer(structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();--if(guest_cbs&&guest_cbs->is_in_guest())-returnguest_cbs->get_guest_ip();-returninstruction_pointer(regs);}unsignedlongperf_misc_flags(structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();intmisc=0;-if(guest_cbs&&guest_cbs->is_in_guest()){-if(guest_cbs->is_user_mode())-misc|=PERF_RECORD_MISC_GUEST_USER;-else-misc|=PERF_RECORD_MISC_GUEST_KERNEL;-}else{-if(user_mode(regs))-misc|=PERF_RECORD_MISC_USER;-else-misc|=PERF_RECORD_MISC_KERNEL;-}+if(user_mode(regs))+misc|=PERF_RECORD_MISC_USER;+else+misc|=PERF_RECORD_MISC_KERNEL;returnmisc;}
@@ -86,13 +86,8 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();unsignedlongfp=0;-/* C-SKY does not support virtualization. */-if(guest_cbs&&guest_cbs->is_in_guest())-return;-fp=regs->regs[4];perf_callchain_store(entry,regs->pc);
@@ -111,15 +106,8 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframefr;-/* C-SKY does not support virtualization. */-if(guest_cbs&&guest_cbs->is_in_guest()){-pr_warn("C-SKY does not support perf in guest mode!");-return;-}-fr.fp=regs->regs[4];fr.lr=regs->lr;walk_stackframe(&fr,entry);
@@ -1372,11 +1371,6 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry,leaf_fp=0;-if(guest_cbs&&guest_cbs->is_in_guest()){-/* We don't support guest os callchain now */-return;-}-perf_callchain_store(entry,regs->ipc);fp=regs->fp;gp=regs->gp;
@@ -1480,13 +1474,8 @@ voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();structstackframefr;-if(guest_cbs&&guest_cbs->is_in_guest()){-/* We don't support guest os callchain now */-return;-}fr.fp=regs->fp;fr.lp=regs->lp;fr.sp=regs->sp;
@@ -1495,32 +1484,17 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,unsignedlongperf_instruction_pointer(structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();--/* However, NDS32 does not support virtualization */-if(guest_cbs&&guest_cbs->is_in_guest())-returnguest_cbs->get_guest_ip();-returninstruction_pointer(regs);}unsignedlongperf_misc_flags(structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();intmisc=0;-/* However, NDS32 does not support virtualization */-if(guest_cbs&&guest_cbs->is_in_guest()){-if(guest_cbs->is_user_mode())-misc|=PERF_RECORD_MISC_GUEST_USER;-else-misc|=PERF_RECORD_MISC_GUEST_KERNEL;-}else{-if(user_mode(regs))-misc|=PERF_RECORD_MISC_USER;-else-misc|=PERF_RECORD_MISC_KERNEL;-}+if(user_mode(regs))+misc|=PERF_RECORD_MISC_USER;+else+misc|=PERF_RECORD_MISC_KERNEL;returnmisc;}
@@ -56,13 +56,8 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,voidperf_callchain_user(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();unsignedlongfp=0;-/* RISC-V does not support perf in guest mode. */-if(guest_cbs&&guest_cbs->is_in_guest())-return;-fp=regs->s0;perf_callchain_store(entry,regs->epc);
@@ -79,13 +74,5 @@ static bool fill_callchain(void *entry, unsigned long pc)voidperf_callchain_kernel(structperf_callchain_entry_ctx*entry,structpt_regs*regs){-structperf_guest_info_callbacks*guest_cbs=perf_get_guest_cbs();--/* RISC-V does not support perf in guest mode. */-if(guest_cbs&&guest_cbs->is_in_guest()){-pr_warn("RISC-V does not support perf in guest mode!");-return;-}-walk_stackframe(NULL,regs,fill_callchain,entry);}
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:37:17
Move x86's perf guest callbacks into common KVM, as they are semantically
identical to arm64's callbacks (the only other such KVM callbacks).
arm64 will convert to the common versions in a future patch.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 4 +++
arch/x86/kvm/x86.c | 53 +++++++--------------------------
include/linux/kvm_host.h | 12 ++++++++
virt/kvm/kvm_main.c | 40 +++++++++++++++++++++++++
4 files changed, 67 insertions(+), 42 deletions(-)
@@ -8264,43 +8264,12 @@ static void kvm_timer_init(void)kvmclock_cpu_online,kvmclock_cpu_down_prep);}-staticinlineboolkvm_pmi_in_guest(structkvm_vcpu*vcpu)-{-returnvcpu&&vcpu->arch.handling_intr_from_guest;-}--staticunsignedintkvm_guest_state(void)-{-structkvm_vcpu*vcpu=kvm_get_running_vcpu();-unsignedintstate;--if(!kvm_pmi_in_guest(vcpu))-return0;--state=PERF_GUEST_ACTIVE;-if(static_call(kvm_x86_get_cpl)(vcpu))-state|=PERF_GUEST_USER;--returnstate;-}--staticunsignedlongkvm_guest_get_ip(void)-{-structkvm_vcpu*vcpu=kvm_get_running_vcpu();--/* Retrieving the IP must be guarded by a call to kvm_guest_state(). */-if(WARN_ON_ONCE(!kvm_pmi_in_guest(vcpu)))-return0;--returnkvm_rip_read(vcpu);-}-staticunsignedintkvm_handle_intel_pt_intr(void){structkvm_vcpu*vcpu=kvm_get_running_vcpu();/* '0' on failure so that the !PT case can use a RET0 static call. */-if(!kvm_pmi_in_guest(vcpu))+if(!kvm_arch_pmi_in_guest(vcpu))return0;kvm_make_request(KVM_REQ_PMI,vcpu);
@@ -8309,12 +8278,6 @@ static unsigned int kvm_handle_intel_pt_intr(void)return1;}-staticstructperf_guest_info_callbackskvm_guest_cbs={-.state=kvm_guest_state,-.get_ip=kvm_guest_get_ip,-.handle_intel_pt_intr=NULL,-};-#ifdef CONFIG_X86_64staticvoidpvclock_gtod_update_fn(structwork_struct*work){
@@ -11068,9 +11031,11 @@ int kvm_arch_hardware_setup(void *opaque)memcpy(&kvm_x86_ops,ops->runtime_ops,sizeof(kvm_x86_ops));kvm_ops_static_call_update();+/* Temporary ugliness. */if(ops->intel_pt_intr_in_guest&&ops->intel_pt_intr_in_guest())-kvm_guest_cbs.handle_intel_pt_intr=kvm_handle_intel_pt_intr;-perf_register_guest_info_callbacks(&kvm_guest_cbs);+kvm_register_perf_callbacks(kvm_handle_intel_pt_intr);+else+kvm_register_perf_callbacks(NULL);if(!kvm_cpu_cap_has(X86_FEATURE_XSAVES))supported_xss=0;
@@ -11099,8 +11064,7 @@ int kvm_arch_hardware_setup(void *opaque)voidkvm_arch_hardware_unsetup(void){-perf_unregister_guest_info_callbacks();-kvm_guest_cbs.handle_intel_pt_intr=NULL;+kvm_unregister_perf_callbacks();static_call(kvm_x86_hardware_unsetup)();}
@@ -5460,6 +5460,46 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)return&kvm_running_vcpu;}+#ifdef __KVM_WANT_PERF_CALLBACKS+staticunsignedintkvm_guest_state(void)+{+structkvm_vcpu*vcpu=kvm_get_running_vcpu();+unsignedintstate;++if(!kvm_arch_pmi_in_guest(vcpu))+return0;++state=PERF_GUEST_ACTIVE;+if(!kvm_arch_vcpu_in_kernel(vcpu))+state|=PERF_GUEST_USER;++returnstate;+}++staticunsignedlongkvm_guest_get_ip(void)+{+structkvm_vcpu*vcpu=kvm_get_running_vcpu();++/* Retrieving the IP must be guarded by a call to kvm_guest_state(). */+if(WARN_ON_ONCE(!kvm_arch_pmi_in_guest(vcpu)))+return0;++returnkvm_arch_vcpu_get_ip(vcpu);+}++staticstructperf_guest_info_callbackskvm_guest_cbs={+.state=kvm_guest_state,+.get_ip=kvm_guest_get_ip,+.handle_intel_pt_intr=NULL,+};++voidkvm_register_perf_callbacks(unsignedint(*pt_intr_handler)(void))+{+kvm_guest_cbs.handle_intel_pt_intr=pt_intr_handler;+perf_register_guest_info_callbacks(&kvm_guest_cbs);+}+#endif+structkvm_cpu_compat_check{void*opaque;int*ret;
--
2.33.0.259.gc128427fd7-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:37:23
Now that all state needed for VMX's PT interrupt handler is exposed to
vmx.c (specifically the currently running vCPU), move the handler into
vmx.c where it belongs.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/vmx/vmx.c | 22 +++++++++++++++++++++-
arch/x86/kvm/x86.c | 20 +-------------------
include/linux/kvm_host.h | 2 --
4 files changed, 23 insertions(+), 23 deletions(-)
@@ -7687,6 +7687,20 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {.vcpu_deliver_sipi_vector=kvm_vcpu_deliver_sipi_vector,};+staticunsignedintvmx_handle_intel_pt_intr(void)+{+structkvm_vcpu*vcpu=kvm_get_running_vcpu();++/* '0' on failure so that the !PT case can use a RET0 static call. */+if(!kvm_arch_pmi_in_guest(vcpu))+return0;++kvm_make_request(KVM_REQ_PMI,vcpu);+__set_bit(MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT,+(unsignedlong*)&vcpu->arch.pmu.global_status);+return1;+}+static__initvoidvmx_setup_user_return_msrs(void){
@@ -8264,20 +8264,6 @@ static void kvm_timer_init(void)kvmclock_cpu_online,kvmclock_cpu_down_prep);}-staticunsignedintkvm_handle_intel_pt_intr(void)-{-structkvm_vcpu*vcpu=kvm_get_running_vcpu();--/* '0' on failure so that the !PT case can use a RET0 static call. */-if(!kvm_arch_pmi_in_guest(vcpu))-return0;--kvm_make_request(KVM_REQ_PMI,vcpu);-__set_bit(MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT,-(unsignedlong*)&vcpu->arch.pmu.global_status);-return1;-}-#ifdef CONFIG_X86_64staticvoidpvclock_gtod_update_fn(structwork_struct*work){
@@ -11031,11 +11017,7 @@ int kvm_arch_hardware_setup(void *opaque)memcpy(&kvm_x86_ops,ops->runtime_ops,sizeof(kvm_x86_ops));kvm_ops_static_call_update();-/* Temporary ugliness. */-if(ops->intel_pt_intr_in_guest&&ops->intel_pt_intr_in_guest())-kvm_register_perf_callbacks(kvm_handle_intel_pt_intr);-else-kvm_register_perf_callbacks(NULL);+kvm_register_perf_callbacks(ops->handle_intel_pt_intr);if(!kvm_cpu_cap_has(X86_FEATURE_XSAVES))supported_xss=0;
@@ -1744,7 +1744,9 @@ static int init_subsystems(void)if(err)gotoout;-kvm_perf_init();+kvm_pmu_init();+kvm_register_perf_callbacks(NULL);+kvm_sys_reg_table_init();out:
@@ -2160,7 +2162,7 @@ int kvm_arch_init(void *opaque)/* NOP: Compiling as a module not supported */voidkvm_arch_exit(void){-kvm_perf_teardown();+kvm_unregister_perf_callbacks();}staticint__initearly_kvm_mode_cfg(char*arg)
From: Sean Christopherson <seanjc@google.com> Date: 2021-08-28 00:37:31
Drop arm64's version of the callbacks in favor of the callbacks provided
by generic KVM, which are semantically identical. Implement the "get ip"
hook as needed.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm64/include/asm/kvm_host.h | 12 +++++++++++
arch/arm64/kvm/arm.c | 5 +++++
arch/arm64/kvm/perf.c | 34 ++-----------------------------
3 files changed, 19 insertions(+), 32 deletions(-)
@@ -673,6 +673,18 @@ int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa);voidkvm_perf_init(void);voidkvm_perf_teardown(void);+#ifdef CONFIG_PERF_EVENTS+#define __KVM_WANT_PERF_CALLBACKS+staticinlineboolkvm_arch_pmi_in_guest(structkvm_vcpu*vcpu)+{+/* Any callback while a vCPU is loaded is considered to be in guest. */+return!!vcpu;+}+#else+staticinlinevoidkvm_register_perf_callbacks(void){}+staticinlinevoidkvm_unregister_perf_callbacks(void){}+#endif+longkvm_hypercall_pv_features(structkvm_vcpu*vcpu);gpa_tkvm_init_stolen_time(structkvm_vcpu*vcpu);voidkvm_update_stolen_time(structkvm_vcpu*vcpu);
@@ -500,6 +500,11 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)returnvcpu_mode_priv(vcpu);}+unsignedlongkvm_arch_vcpu_get_ip(structkvm_vcpu*vcpu)+{+return*vcpu_pc(vcpu);+}+/* Just ensure a guest exit from a particular CPU */staticvoidexit_vm_noop(void*info){
@@ -1237,6 +1237,14 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,u16flags);externstructperf_guest_info_callbacks*perf_guest_cbs;+staticinlinestructperf_guest_info_callbacks*perf_get_guest_cbs(void)+{+/* Reg/unreg perf_guest_cbs waits for readers via synchronize_rcu(). */+lockdep_assert_preemption_disabled();++/* Prevent reloading between a !NULL check and dereferences. */+returnREAD_ONCE(perf_guest_cbs);+}
Nice..
quoted hunk
extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
From: Peter Zijlstra <peterz@infradead.org> Date: 2021-08-28 20:14:47
On Fri, Aug 27, 2021 at 05:35:45PM -0700, Sean Christopherson wrote:
Like Xu (2):
perf/core: Rework guest callbacks to prepare for static_call support
perf/core: Use static_call to optimize perf_guest_info_callbacks
Sean Christopherson (11):
perf: Ensure perf_guest_cbs aren't reloaded between !NULL check and
deref
KVM: x86: Register perf callbacks after calling vendor's
hardware_setup()
KVM: x86: Register Processor Trace interrupt hook iff PT enabled in
guest
perf: Stop pretending that perf can handle multiple guest callbacks
perf: Force architectures to opt-in to guest callbacks
KVM: x86: Drop current_vcpu for kvm_running_vcpu + kvm_arch_vcpu
variable
KVM: x86: More precisely identify NMI from guest when handling PMI
KVM: Move x86's perf guest info callbacks to generic KVM
KVM: x86: Move Intel Processor Trace interrupt handler to vmx.c
KVM: arm64: Convert to the generic perf callbacks
KVM: arm64: Drop perf.c and fold its tiny bits of code into arm.c /
pmu.c
From: Sean Christopherson <seanjc@google.com> Date: 2021-09-16 21:38:14
On Sat, Aug 28, 2021, Peter Zijlstra wrote:
On Fri, Aug 27, 2021 at 05:35:45PM -0700, Sean Christopherson wrote:
quoted
Like Xu (2):
perf/core: Rework guest callbacks to prepare for static_call support
perf/core: Use static_call to optimize perf_guest_info_callbacks
Sean Christopherson (11):
perf: Ensure perf_guest_cbs aren't reloaded between !NULL check and
deref
KVM: x86: Register perf callbacks after calling vendor's
hardware_setup()
KVM: x86: Register Processor Trace interrupt hook iff PT enabled in
guest
perf: Stop pretending that perf can handle multiple guest callbacks
perf: Force architectures to opt-in to guest callbacks
KVM: x86: Drop current_vcpu for kvm_running_vcpu + kvm_arch_vcpu
variable
KVM: x86: More precisely identify NMI from guest when handling PMI
KVM: Move x86's perf guest info callbacks to generic KVM
KVM: x86: Move Intel Processor Trace interrupt handler to vmx.c
KVM: arm64: Convert to the generic perf callbacks
KVM: arm64: Drop perf.c and fold its tiny bits of code into arm.c /
pmu.c
Argh, sorry, I somehow managed to miss all of your replies. I'll get back to
this series next week. Thanks for the quick response!
Lets keep the whole intel_pt crud inside x86...
In theory, I like the idea of burying intel_pt inside x86 (and even in Intel+VMX code
for the most part), but the actual implementation is a bit gross. Because of the
whole "KVM can be a module" thing, either the static call and __static_call_return0
would need to be exported, or a new register/unregister pair would have to be exported.
The unregister path would also need its own synchronize_rcu(). In general, I
don't love duplicating the logic, but it's not the end of the world.
Either way works for me. Paolo or Peter, do either of you have a preference?
FWIW, the param needs to be a raw function, not a function pointer.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
You're waiting for all NULL users to go away? :-) IOW, we can do without
this synchronize_rcu() call.
Doh, right. I was thinking KVM needed to wait for in-progress NMI to exit to
ensure guest PT interrupts are handled correctly, but obviously the NMI handler
needs to exit for that CPU to get into a guest...
if (WARN_ON_ONCE(perf_guest_cbs != cbs))
return -EBUSY;
?
Works for me. I guess I'm more optimistic about people not being morons :-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Ah, nice! We can go even further to:
depends on HAVE_PERF_EVENTS && HAVE_KVM
though I'm pretty sure all architectures that select HAVE_KVM also select
HAVE_PERF_EVENTS.
Huh. arm64 doesn't select HAVE_KVM even though it selects almost literally every
other HAVE_KVM_* config. arm64 has some other weirdness with CONFIG_KVM, I'll add
a patch or two to fix that stuff and amend this patch as above.
Thanks again!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Peter Zijlstra <peterz@infradead.org> Date: 2021-09-17 07:29:53
On Thu, Sep 16, 2021 at 09:37:43PM +0000, Sean Christopherson wrote:
On Sat, Aug 28, 2021, Peter Zijlstra wrote:
Argh, sorry, I somehow managed to miss all of your replies. I'll get back to
this series next week. Thanks for the quick response!
quoted
Lets keep the whole intel_pt crud inside x86...
In theory, I like the idea of burying intel_pt inside x86 (and even in
Intel+VMX code for the most part), but the actual implementation is a
bit gross. Because of the whole "KVM can be a module" thing,
ARGH!! we should really fix that. I've heard other archs have made much
better choices here.
either
the static call and __static_call_return0 would need to be exported,
or a new register/unregister pair would have to be exported.
So I don't mind exporting __static_call_return0, but exporting a raw
static_call is much like exporting a function pointer :/
The unregister path would also need its own synchronize_rcu(). In general, I
don't love duplicating the logic, but it's not the end of the world.
Either way works for me. Paolo or Peter, do either of you have a preference?
Can we de-feature kvm as a module and only have this PT functionality
when built-in? :-)
From: Sean Christopherson <seanjc@google.com> Date: 2021-09-17 16:54:01
On Fri, Sep 17, 2021, Peter Zijlstra wrote:
On Thu, Sep 16, 2021 at 09:37:43PM +0000, Sean Christopherson wrote:
So I don't mind exporting __static_call_return0, but exporting a raw
static_call is much like exporting a function pointer :/
Ya, that part is quite gross.
quoted
The unregister path would also need its own synchronize_rcu(). In general, I
don't love duplicating the logic, but it's not the end of the world.
Either way works for me. Paolo or Peter, do either of you have a preference?
Can we de-feature kvm as a module and only have this PT functionality
when built-in? :-)
I agree that many of the for-KVM exports are ugly, especially several of the
perf exports, but I will fight tooth and nail to keep KVM-as-a-module. It is
invaluable for development and testing, and in the not-too-distant future there
is KVM-maintenance related functionality that we'd like to implement that relies
on KVM being a module.
I would be more than happy to help explore approaches that reduce the for-KVM
exports, but I am strongly opposed to defeaturing KVM-as-a-module. I have a few
nascent ideas for eliminating a handful of a random exports, but no clever ideas
for eliminating perf's for-KVM exports.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2021-09-20 12:05:46
On 17/09/21 09:28, Peter Zijlstra wrote:
quoted
In theory, I like the idea of burying intel_pt inside x86 (and even in
Intel+VMX code for the most part), but the actual implementation is a
bit gross. Because of the whole "KVM can be a module" thing,
ARGH!! we should really fix that. I've heard other archs have made much
better choices here.
I think that's only ARM, and even then it is only because of limitations
of the hardware which mostly apply only if VHE is not in use.
If anything, it's ARM that should support module build in VHE mode
(Linux would still need to know whether it will be running at EL1 or
EL2, but KVM's functionality is as self-contained as on x86 in the VHE
case).
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-09-20 12:22:10
On Mon, 20 Sep 2021 13:05:25 +0100,
Paolo Bonzini [off-list ref] wrote:
On 17/09/21 09:28, Peter Zijlstra wrote:
quoted
quoted
In theory, I like the idea of burying intel_pt inside x86 (and even in
Intel+VMX code for the most part), but the actual implementation is a
bit gross. Because of the whole "KVM can be a module" thing,
ARGH!! we should really fix that. I've heard other archs have made much
better choices here.
I think that's only ARM, and even then it is only because of
limitations of the hardware which mostly apply only if VHE is not in
use.
If anything, it's ARM that should support module build in VHE mode
(Linux would still need to know whether it will be running at EL1 or
EL2, but KVM's functionality is as self-contained as on x86 in the VHE
case).
I don't see this happening anytime soon. At least not before we
declare the arm64 single kernel image policy to be obsolete.
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2021-09-20 13:18:42
On 20/09/21 14:22, Marc Zyngier wrote:
quoted
I think that's only ARM, and even then it is only because of
limitations of the hardware which mostly apply only if VHE is not in
use.
If anything, it's ARM that should support module build in VHE mode
(Linux would still need to know whether it will be running at EL1 or
EL2, but KVM's functionality is as self-contained as on x86 in the VHE
case).
I don't see this happening anytime soon. At least not before we
declare the arm64 single kernel image policy to be obsolete.
--verbose please. :) I am sure you're right, but I don't understand the
link between the two.
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-09-20 13:40:45
On Mon, 20 Sep 2021 14:18:30 +0100,
Paolo Bonzini [off-list ref] wrote:
On 20/09/21 14:22, Marc Zyngier wrote:
quoted
quoted
I think that's only ARM, and even then it is only because of
limitations of the hardware which mostly apply only if VHE is not in
use.
If anything, it's ARM that should support module build in VHE mode
(Linux would still need to know whether it will be running at EL1 or
EL2, but KVM's functionality is as self-contained as on x86 in the VHE
case).
I don't see this happening anytime soon. At least not before we
declare the arm64 single kernel image policy to be obsolete.
--verbose please. :) I am sure you're right, but I don't understand
the link between the two.
To start making KVM/arm64 modular, you'd have to build it such as
there is no support for the nVHE hypervisor anymore. Which would mean
two different configs (one that can only work with VHE, and one for
the rest) and contradicts the current single kernel image policy.
It is bad enough that we have to support 3 sets of page sizes.
Doubling the validation space for the sake of being able to unload KVM
seems a dubious prospect.
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2021-09-20 18:29:24
On 20/09/21 15:40, Marc Zyngier wrote:
quoted
quoted
At least not before we
declare the arm64 single kernel image policy to be obsolete.
--verbose please.:) I am sure you're right, but I don't understand
the link between the two.
To start making KVM/arm64 modular, you'd have to build it such as
there is no support for the nVHE hypervisor anymore. Which would mean
two different configs (one that can only work with VHE, and one for
the rest) and contradicts the current single kernel image policy.
Ah okay, I interpreted the policy as "it's possible to build a single
kernel image but it would be possible to build an image for a subset of
the features as well".
In that case you could have one config that can work either with or
without VHE (and supports y/n) and one config that can only work with
VHE (and supports y/m/n). The code to enter VHE EL2 would of course
always be builtin.
It is bad enough that we have to support 3 sets of page sizes.
Doubling the validation space for the sake of being able to unload KVM
seems a dubious prospect.
It's not even a configuration that matches kconfig very well, since it
does have a way to build something *only as a module*, but not a way to
build something only as built-in.
That said, if you had the possibility to unload/reload KVM, you'll
quickly become unable to live without it. :)
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Paolo Bonzini <pbonzini@redhat.com> Date: 2021-09-21 16:44:42
On 28/08/21 21:47, Peter Zijlstra wrote:
quoted
+config HAVE_GUEST_PERF_EVENTS
+ bool
depends on HAVE_KVM
It won't really do anything, since Kconfig does not detects conflicts
between select' and 'depends on' clauses.
Rather, should the symbol be selected by KVM, instead of ARM64 and X86?
Paolo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Sean Christopherson <seanjc@google.com> Date: 2021-09-21 21:30:03
On Tue, Sep 21, 2021, Paolo Bonzini wrote:
On 28/08/21 21:47, Peter Zijlstra wrote:
quoted
quoted
+config HAVE_GUEST_PERF_EVENTS
+ bool
depends on HAVE_KVM
It won't really do anything, since Kconfig does not detects conflicts
between select' and 'depends on' clauses.
It does throw a WARN, though the build doesn't fail.
WARNING: unmet direct dependencies detected for HAVE_GUEST_PERF_EVENTS
Depends on [n]: HAVE_KVM [=n] && HAVE_PERF_EVENTS [=y]
Selected by [y]:
- ARM64 [=y]
WARNING: unmet direct dependencies detected for HAVE_GUEST_PERF_EVENTS
Depends on [n]: HAVE_KVM [=n] && HAVE_PERF_EVENTS [=y]
Selected by [y]:
- ARM64 [=y]
WARNING: unmet direct dependencies detected for HAVE_GUEST_PERF_EVENTS
Depends on [n]: HAVE_KVM [=n] && HAVE_PERF_EVENTS [=y]
Selected by [y]:
- ARM64 [=y]
Rather, should the symbol be selected by KVM, instead of ARM64 and X86?
By KVM, you mean KVM in arm64 and x86, correct? Because HAVE_GUEST_PERF_EVENTS
should not be selected for s390, PPC, or MIPS.
Oh, and Xen also uses the callbacks on x86, which means the HAVE_KVM part is
arguabably wrong, even though it's guaranteed to be true for the XEN_PV case.
I'll drop that dependency and send out a separate series to clean up the arm64
side of HAVE_KVM.
The reason I didn't bury HAVE_GUEST_PERF_EVENTS under KVM (and XEN_PV) is that
there are number of references to the callbacks throught perf and I didn't want
to create #ifdef hell.
But I think I figured out a not-awful solution. If there are wrappers+stubs for
the guest callback users, then the new Kconfig can be selected on-demand instead
of unconditionally by arm64 and x86. That has the added bonus of eliminating
the relevant code paths for !KVM (and !XEN_PV on x86), with or without static_call.
It also obviates the needs for __KVM_WANT_GUEST_PERF_EVENTS or whatever I called
that thing.
It more or less requires defining the static calls in generic perf, but I think
that actually ends up being good thing as it consolidates more code without
introducing more #ifdefs. The diffstats for the static_call() conversions are
also quite nice.
include/linux/perf_event.h | 28 ++++++----------------------
kernel/events/core.c | 15 +++++++++++++++
2 files changed, 21 insertions(+), 22 deletions(-)
I'll try to get a new version out today or tomorrow.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel