arch/arm/kernel/perf_callchain.c | 28 ++------------ arch/arm64/Kconfig | 1 + arch/arm64/include/asm/kvm_host.h | 8 +++- arch/arm64/kernel/perf_callchain.c | 18 ++++++--- arch/arm64/kvm/Makefile | 2 +- arch/arm64/kvm/arm.c | 13 ++++++- 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 | 17 +++++--- arch/x86/events/intel/core.c | 7 ++-- arch/x86/include/asm/kvm_host.h | 4 +- arch/x86/kvm/pmu.c | 2 +- arch/x86/kvm/pmu.h | 1 + arch/x86/kvm/svm/svm.c | 2 +- arch/x86/kvm/vmx/vmx.c | 25 ++++++++++-- arch/x86/kvm/x86.c | 54 +++----------------------- arch/x86/kvm/x86.h | 12 +++--- arch/x86/xen/pmu.c | 2 +- include/kvm/arm_pmu.h | 1 + include/linux/kvm_host.h | 12 ++++++ include/linux/perf_event.h | 33 ++++++++++++---- init/Kconfig | 3 ++ kernel/events/core.c | 28 ++++++++------ virt/kvm/kvm_main.c | 42 ++++++++++++++++++++ 28 files changed, 204 insertions(+), 231 deletions(-) delete mode 100644 arch/arm64/kvm/perf.c
This started out as a small series[1] to fix a KVM bug related to Intel PT
interrupt handling and snowballed horribly.
The main problem being addressed is that the perf_guest_cbs are shared by
all CPUs, can be nullified by KVM during module unload, and are not
protected against concurrent access from NMI context.
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 AFAICT the compiler never reloads perf_guest_cbs in this sequence.
The compiler does reload perf_guest_cbs for any future dereferences, 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 it can be unloaded.
But with help, e.g. READ_ONCE(perf_guest_cbs), unloading kvm_intel can
trigger a NULL pointer derference (see below). Manual intervention aside,
the bug is a bit of a time bomb, e.g. my patch 3 from the original PT
handling series would have omitted the ->is_in_guest() guard.
This series fixes the problem by making the callbacks per-CPU, and
registering/unregistering the callbacks only with preemption disabled
(except for the Xen case, which doesn't unregister).
This approach also allows for several nice cleanups in this series.
KVM x86 and arm64 can share callbacks, KVM x86 drops its somewhat
redundant current_vcpu, and the retpoline that is currently hit when KVM
is loaded (due to always checking ->is_in_guest()) goes away (it's still
there when running as Xen Dom0).
Changing to per-CPU callbacks also provides a good excuse to excise
copy+paste code from architectures that can't possibly have guest
callbacks.
This series conflicts horribly with a proposed patch[2] to use static
calls for perf_guest_cbs. But that patch is broken as it completely
fails to handle unregister, and it's not clear to me whether or not
it can correctly handle unregister without fixing the underlying race
(I don't know enough about the code patching for static calls).
This tweak
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 1eb45139fcc6..202e5ad97f82 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2954,7 +2954,7 @@ unsigned long perf_misc_flags(struct pt_regs *regs)
{
int misc = 0;
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+ if (READ_ONCE(perf_guest_cbs) && READ_ONCE(perf_guest_cbs)->is_in_guest()) {
if (perf_guest_cbs->is_user_mode())
misc |= PERF_RECORD_MISC_GUEST_USER;
else
while spamming 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
[1] https://lkml.kernel.org/r/20210823193709.55886-1-seanjc@google.com
[2] https://lkml.kernel.org/r/20210806133802.3528-2-lingshan.zhu@intel.com
Sean Christopherson (15):
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
perf: Track guest callbacks on a per-CPU basis
KVM: x86: Register perf callbacks only when actively handling
interrupt
KVM: Use dedicated flag to track if KVM is handling an NMI from guest
KVM: x86: Drop current_vcpu in favor of kvm_running_vcpu
KVM: arm64: Register/unregister perf callbacks at vcpu load/put
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 bit of code into pmu.c
perf: Disallow bulk unregistering of guest callbacks and do cleanup
perf: KVM: Indicate "in guest" via NULL ->is_in_guest callback
arch/arm/kernel/perf_callchain.c | 28 ++------------
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/kvm_host.h | 8 +++-
arch/arm64/kernel/perf_callchain.c | 18 ++++++---
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/arm.c | 13 ++++++-
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 | 17 +++++---
arch/x86/events/intel/core.c | 7 ++--
arch/x86/include/asm/kvm_host.h | 4 +-
arch/x86/kvm/pmu.c | 2 +-
arch/x86/kvm/pmu.h | 1 +
arch/x86/kvm/svm/svm.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 25 ++++++++++--
arch/x86/kvm/x86.c | 54 +++-----------------------
arch/x86/kvm/x86.h | 12 +++---
arch/x86/xen/pmu.c | 2 +-
include/kvm/arm_pmu.h | 1 +
include/linux/kvm_host.h | 12 ++++++
include/linux/perf_event.h | 33 ++++++++++++----
init/Kconfig | 3 ++
kernel/events/core.c | 28 ++++++++------
virt/kvm/kvm_main.c | 42 ++++++++++++++++++++
28 files changed, 204 insertions(+), 231 deletions(-)
delete mode 100644 arch/arm64/kvm/perf.c
--
2.33.0.259.gc128427fd7-goog
+ STATIC BRANCH/CALL friends.
On 27/8/2021 8:57 am, Sean Christopherson wrote:
> This started out as a small series[1] to fix a KVM bug related to Intel PT
> interrupt handling and snowballed horribly.
>
> The main problem being addressed is that the perf_guest_cbs are shared by
> all CPUs, can be nullified by KVM during module unload, and are not
> protected against concurrent access from NMI context.
Shouldn't this be a generic issue of the static_call() usage ?
At the beginning, we set up the static entry assuming perf_guest_cbs != NULL:
if (perf_guest_cbs && perf_guest_cbs->handle_intel_pt_intr) {
static_call_update(x86_guest_handle_intel_pt_intr,
perf_guest_cbs->handle_intel_pt_intr);
}
and then we unset the perf_guest_cbs and do the static function call like this:
DECLARE_STATIC_CALL(x86_guest_handle_intel_pt_intr,
*(perf_guest_cbs->handle_intel_pt_intr));
static int handle_pmi_common(struct pt_regs *regs, u64 status)
{
...
if (!static_call(x86_guest_handle_intel_pt_intr)())
intel_pt_interrupt();
...
}
Can we make static_call() back to the original "(void *)&__static_call_return0"
in this case ?
>
> 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 AFAICT the compiler never reloads perf_guest_cbs in this sequence.
> The compiler does reload perf_guest_cbs for any future dereferences, 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 it can be unloaded.
>
> But with help, e.g. READ_ONCE(perf_guest_cbs), unloading kvm_intel can
> trigger a NULL pointer derference (see below). Manual intervention aside,
> the bug is a bit of a time bomb, e.g. my patch 3 from the original PT
> handling series would have omitted the ->is_in_guest() guard.
>
> This series fixes the problem by making the callbacks per-CPU, and
> registering/unregistering the callbacks only with preemption disabled
> (except for the Xen case, which doesn't unregister).
>
> This approach also allows for several nice cleanups in this series.
> KVM x86 and arm64 can share callbacks, KVM x86 drops its somewhat
> redundant current_vcpu, and the retpoline that is currently hit when KVM
> is loaded (due to always checking ->is_in_guest()) goes away (it's still
> there when running as Xen Dom0).
>
> Changing to per-CPU callbacks also provides a good excuse to excise
> copy+paste code from architectures that can't possibly have guest
> callbacks.
>
> This series conflicts horribly with a proposed patch[2] to use static
> calls for perf_guest_cbs. But that patch is broken as it completely
> fails to handle unregister, and it's not clear to me whether or not
> it can correctly handle unregister without fixing the underlying race
> (I don't know enough about the code patching for static calls).
>
> This tweak
>
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 1eb45139fcc6..202e5ad97f82 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -2954,7 +2954,7 @@ unsigned long perf_misc_flags(struct pt_regs *regs)
> {
> int misc = 0;
>
> - if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
> + if (READ_ONCE(perf_guest_cbs) && READ_ONCE(perf_guest_cbs)->is_in_guest()) {
> if (perf_guest_cbs->is_user_mode())
> misc |= PERF_RECORD_MISC_GUEST_USER;
> else
>
> while spamming 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
>
> [1] https://lkml.kernel.org/r/20210823193709.55886-1-seanjc@google.com
> [2] https://lkml.kernel.org/r/20210806133802.3528-2-lingshan.zhu@intel.com
>
> Sean Christopherson (15):
> 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
> perf: Track guest callbacks on a per-CPU basis
> KVM: x86: Register perf callbacks only when actively handling
> interrupt
> KVM: Use dedicated flag to track if KVM is handling an NMI from guest
> KVM: x86: Drop current_vcpu in favor of kvm_running_vcpu
> KVM: arm64: Register/unregister perf callbacks at vcpu load/put
> 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 bit of code into pmu.c
> perf: Disallow bulk unregistering of guest callbacks and do cleanup
> perf: KVM: Indicate "in guest" via NULL ->is_in_guest callback
>
> arch/arm/kernel/perf_callchain.c | 28 ++------------
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/kvm_host.h | 8 +++-
> arch/arm64/kernel/perf_callchain.c | 18 ++++++---
> arch/arm64/kvm/Makefile | 2 +-
> arch/arm64/kvm/arm.c | 13 ++++++-
> 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 | 17 +++++---
> arch/x86/events/intel/core.c | 7 ++--
> arch/x86/include/asm/kvm_host.h | 4 +-
> arch/x86/kvm/pmu.c | 2 +-
> arch/x86/kvm/pmu.h | 1 +
> arch/x86/kvm/svm/svm.c | 2 +-
> arch/x86/kvm/vmx/vmx.c | 25 ++++++++++--
> arch/x86/kvm/x86.c | 54 +++-----------------------
> arch/x86/kvm/x86.h | 12 +++---
> arch/x86/xen/pmu.c | 2 +-
> include/kvm/arm_pmu.h | 1 +
> include/linux/kvm_host.h | 12 ++++++
> include/linux/perf_event.h | 33 ++++++++++++----
> init/Kconfig | 3 ++
> kernel/events/core.c | 28 ++++++++------
> virt/kvm/kvm_main.c | 42 ++++++++++++++++++++
> 28 files changed, 204 insertions(+), 231 deletions(-)
> delete mode 100644 arch/arm64/kvm/perf.c
>
On Fri, Aug 27, 2021 at 02:52:25PM +0800, Like Xu wrote:
> + STATIC BRANCH/CALL friends.
>
> On 27/8/2021 8:57 am, Sean Christopherson wrote:
> > This started out as a small series[1] to fix a KVM bug related to Intel PT
> > interrupt handling and snowballed horribly.
> >
> > The main problem being addressed is that the perf_guest_cbs are shared by
> > all CPUs, can be nullified by KVM during module unload, and are not
> > protected against concurrent access from NMI context.
>
> Shouldn't this be a generic issue of the static_call() usage ?
>
> At the beginning, we set up the static entry assuming perf_guest_cbs != NULL:
>
> if (perf_guest_cbs && perf_guest_cbs->handle_intel_pt_intr) {
> static_call_update(x86_guest_handle_intel_pt_intr,
> perf_guest_cbs->handle_intel_pt_intr);
> }
>
> and then we unset the perf_guest_cbs and do the static function call like this:
>
> DECLARE_STATIC_CALL(x86_guest_handle_intel_pt_intr,
> *(perf_guest_cbs->handle_intel_pt_intr));
> static int handle_pmi_common(struct pt_regs *regs, u64 status)
> {
> ...
> if (!static_call(x86_guest_handle_intel_pt_intr)())
> intel_pt_interrupt();
> ...
> }
You just have to make sure all static_call() invocations that started
before unreg are finished before continuing with the unload.
synchronize_rcu() can help with that.
This is module unload 101. Nothing specific to static_call().
On 27/8/2021 3:44 pm, Peter Zijlstra wrote:
> On Fri, Aug 27, 2021 at 02:52:25PM +0800, Like Xu wrote:
>> + STATIC BRANCH/CALL friends.
>>
>> On 27/8/2021 8:57 am, Sean Christopherson wrote:
>>> This started out as a small series[1] to fix a KVM bug related to Intel PT
>>> interrupt handling and snowballed horribly.
>>>
>>> The main problem being addressed is that the perf_guest_cbs are shared by
>>> all CPUs, can be nullified by KVM during module unload, and are not
>>> protected against concurrent access from NMI context.
>>
>> Shouldn't this be a generic issue of the static_call() usage ?
>>
>> At the beginning, we set up the static entry assuming perf_guest_cbs != NULL:
>>
>> if (perf_guest_cbs && perf_guest_cbs->handle_intel_pt_intr) {
>> static_call_update(x86_guest_handle_intel_pt_intr,
>> perf_guest_cbs->handle_intel_pt_intr);
>> }
>>
>> and then we unset the perf_guest_cbs and do the static function call like this:
>>
>> DECLARE_STATIC_CALL(x86_guest_handle_intel_pt_intr,
>> *(perf_guest_cbs->handle_intel_pt_intr));
>> static int handle_pmi_common(struct pt_regs *regs, u64 status)
>> {
>> ...
>> if (!static_call(x86_guest_handle_intel_pt_intr)())
>> intel_pt_interrupt();
>> ...
>> }
>
> You just have to make sure all static_call() invocations that started
> before unreg are finished before continuing with the unload.
> synchronize_rcu() can help with that.
Do you mean something like that:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 64e310ff4f3a..e7d310af7509 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8465,6 +8465,7 @@ void kvm_arch_exit(void)
#endif
kvm_lapic_exit();
perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
+ synchronize_rcu();
if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e466fc8176e1..63ae56c5d133 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6508,6 +6508,7 @@ EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
{
perf_guest_cbs = NULL;
+ arch_perf_update_guest_cbs();
return 0;
}
EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
>
> This is module unload 101. Nothing specific to static_call().
>
On Fri, Aug 27, 2021 at 04:01:45PM +0800, Like Xu wrote:
> On 27/8/2021 3:44 pm, Peter Zijlstra wrote:
> > You just have to make sure all static_call() invocations that started
> > before unreg are finished before continuing with the unload.
> > synchronize_rcu() can help with that.
>
> Do you mean something like that:
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 64e310ff4f3a..e7d310af7509 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8465,6 +8465,7 @@ void kvm_arch_exit(void)
> #endif
> kvm_lapic_exit();
> perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
> + synchronize_rcu();
>
> if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
> cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index e466fc8176e1..63ae56c5d133 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6508,6 +6508,7 @@ EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
> int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
> {
> perf_guest_cbs = NULL;
> + arch_perf_update_guest_cbs();
I'm thinking the synchronize_rcu() should go here, and access to
perf_guest_cbs should be wrapped to yell when called with preemption
enabled.
But yes..
> return 0;
> }
> EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
>
> >
> > This is module unload 101. Nothing specific to static_call().
> >
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 <artem.kashkanov@intel.com>
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(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 86539c1686fa..fb6015f97f9e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8426,8 +8426,6 @@ int kvm_arch_init(void *opaque)
kvm_timer_init();
- perf_register_guest_info_callbacks(&kvm_guest_cbs);
-
if (boot_cpu_has(X86_FEATURE_XSAVE)) {
host_xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
supported_xcr0 = host_xcr0 & KVM_SUPPORTED_XCR0;
@@ -8461,7 +8459,6 @@ void kvm_arch_exit(void)
clear_hv_tscchange_cb();
#endif
kvm_lapic_exit();
- perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
cpufreq_unregister_notifier(&kvmclock_cpufreq_notifier_block,
@@ -11064,6 +11061,8 @@ int kvm_arch_hardware_setup(void *opaque)
memcpy(&kvm_x86_ops, ops->runtime_ops, sizeof(kvm_x86_ops));
kvm_ops_static_call_update();
+ perf_register_guest_info_callbacks(&kvm_guest_cbs);
+
if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
supported_xss = 0;
@@ -11091,6 +11090,8 @@ int kvm_arch_hardware_setup(void *opaque)
void kvm_arch_hardware_unsetup(void)
{
+ perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
+
static_call(kvm_x86_hardware_unsetup)();
}
--
2.33.0.259.gc128427fd7-goog
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 <like.xu.linux@gmail.com>
Reported-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Reported-by: Artem Kashkanov <artem.kashkanov@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/pmu.h | 1 +
arch/x86/kvm/vmx/vmx.c | 1 +
arch/x86/kvm/x86.c | 5 ++++-
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 09b256db394a..1ea4943a73d7 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1494,6 +1494,7 @@ struct kvm_x86_init_ops {
int (*disabled_by_bios)(void);
int (*check_processor_compatibility)(void);
int (*hardware_setup)(void);
+ bool (*intel_pt_intr_in_guest)(void);
struct kvm_x86_ops *runtime_ops;
};
diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index 0e4f2b1fa9fb..b06dbbd7eeeb 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -41,6 +41,7 @@ struct kvm_pmu_ops {
void (*reset)(struct kvm_vcpu *vcpu);
void (*deliver_pmi)(struct kvm_vcpu *vcpu);
void (*cleanup)(struct kvm_vcpu *vcpu);
+ void (*handle_intel_pt_intr)(void);
};
static inline u64 pmc_bitmask(struct kvm_pmc *pmc)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index fada1055f325..f19d72136f77 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7896,6 +7896,7 @@ static struct kvm_x86_init_ops vmx_init_ops __initdata = {
.disabled_by_bios = vmx_disabled_by_bios,
.check_processor_compatibility = vmx_check_processor_compat,
.hardware_setup = hardware_setup,
+ .intel_pt_intr_in_guest = vmx_pt_mode_is_host_guest,
.runtime_ops = &vmx_x86_ops,
};
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fb6015f97f9e..ffc6c2d73508 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8305,7 +8305,7 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
.is_in_guest = kvm_is_in_guest,
.is_user_mode = kvm_is_user_mode,
.get_guest_ip = kvm_get_guest_ip,
- .handle_intel_pt_intr = kvm_handle_intel_pt_intr,
+ .handle_intel_pt_intr = NULL,
};
#ifdef CONFIG_X86_64
@@ -11061,6 +11061,8 @@ int kvm_arch_hardware_setup(void *opaque)
memcpy(&kvm_x86_ops, ops->runtime_ops, sizeof(kvm_x86_ops));
kvm_ops_static_call_update();
+ 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);
if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
@@ -11091,6 +11093,7 @@ int kvm_arch_hardware_setup(void *opaque)
void kvm_arch_hardware_unsetup(void)
{
perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
+ kvm_guest_cbs.handle_intel_pt_intr = NULL;
static_call(kvm_x86_hardware_unsetup)();
}
--
2.33.0.259.gc128427fd7-goog
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
cotenant 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 | 11 ++---------
5 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 41911585ae0c..ed940aec89e0 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -670,8 +670,8 @@ unsigned long kvm_mmio_read_buf(const void *buf, unsigned int len);
int kvm_handle_mmio_return(struct kvm_vcpu *vcpu);
int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa);
-int kvm_perf_init(void);
-int kvm_perf_teardown(void);
+void kvm_perf_init(void);
+void kvm_perf_teardown(void);
long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
index 151c31fb9860..039fe59399a2 100644
--- a/arch/arm64/kvm/perf.c
+++ b/arch/arm64/kvm/perf.c
@@ -48,15 +48,15 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
.get_guest_ip = kvm_get_guest_ip,
};
-int kvm_perf_init(void)
+void kvm_perf_init(void)
{
if (kvm_pmu_probe_pmuver() != 0xf && !is_protected_kvm_enabled())
static_branch_enable(&kvm_arm_pmu_available);
- return perf_register_guest_info_callbacks(&kvm_guest_cbs);
+ perf_register_guest_info_callbacks(&kvm_guest_cbs);
}
-int kvm_perf_teardown(void)
+void kvm_perf_teardown(void)
{
- return perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
+ perf_unregister_guest_info_callbacks();
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ffc6c2d73508..bae951344e28 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11092,7 +11092,7 @@ int kvm_arch_hardware_setup(void *opaque)
void kvm_arch_hardware_unsetup(void)
{
- perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
+ perf_unregister_guest_info_callbacks();
kvm_guest_cbs.handle_intel_pt_intr = NULL;
static_call(kvm_x86_hardware_unsetup)();
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 2d510ad750ed..05c0efba3cd1 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1237,8 +1237,8 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,
u16 flags);
extern struct perf_guest_info_callbacks *perf_guest_cbs;
-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);
+extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
+extern void perf_unregister_guest_info_callbacks(void);
extern void perf_event_exec(void);
extern void perf_event_comm(struct task_struct *tsk, bool exec);
@@ -1481,10 +1481,9 @@ perf_sw_event(u32 event_id, u64 nr, struct pt_regs *regs, u64 addr) { }
static inline void
perf_bp_event(struct perf_event *event, void *data) { }
-static inline int perf_register_guest_info_callbacks
-(struct perf_guest_info_callbacks *callbacks) { return 0; }
-static inline int perf_unregister_guest_info_callbacks
-(struct perf_guest_info_callbacks *callbacks) { return 0; }
+static inline void perf_register_guest_info_callbacks
+(struct perf_guest_info_callbacks *callbacks) { }
+static inline void perf_unregister_guest_info_callbacks(void) { }
static inline void perf_event_mmap(struct vm_area_struct *vma) { }
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 464917096e73..baae796612b9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6482,24 +6482,17 @@ static void perf_pending_event(struct irq_work *entry)
perf_swevent_put_recursion_context(rctx);
}
-/*
- * We assume there is only KVM supporting the callbacks.
- * Later on, we might change it to a list if there is
- * another virtualization implementation supporting the callbacks.
- */
struct perf_guest_info_callbacks *perf_guest_cbs;
-int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
+void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
{
perf_guest_cbs = cbs;
- return 0;
}
EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
-int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
+void perf_unregister_guest_info_callbacks(void)
{
perf_guest_cbs = NULL;
- return 0;
}
EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
--
2.33.0.259.gc128427fd7-goog
Introduce HAVE_GUEST_PERF_EVENTS and require architectures to select it
to allow register guest callbacks in perf. Future patches will convert
the callbacks to per-CPU definitions. 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 | 28 ++++------------------------
arch/arm64/Kconfig | 1 +
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 +
include/linux/perf_event.h | 4 ++++
init/Kconfig | 3 +++
kernel/events/core.c | 2 ++
9 files changed, 19 insertions(+), 69 deletions(-)
diff --git a/arch/arm/kernel/perf_callchain.c b/arch/arm/kernel/perf_callchain.c
index 3b69a76d341e..bc6b246ab55e 100644
--- a/arch/arm/kernel/perf_callchain.c
+++ b/arch/arm/kernel/perf_callchain.c
@@ -64,11 +64,6 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs
{
struct frame_tail __user *tail;
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
- /* We don't support guest os callchain now */
- return;
- }
-
perf_callchain_store(entry, regs->ARM_pc);
if (!current->mm)
@@ -100,20 +95,12 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
{
struct stackframe fr;
- if (perf_guest_cbs && perf_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);
}
unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
- return perf_guest_cbs->get_guest_ip();
-
return instruction_pointer(regs);
}
@@ -121,17 +108,10 @@ unsigned long perf_misc_flags(struct pt_regs *regs)
{
int misc = 0;
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
- if (perf_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;
return misc;
}
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b5b13a932561..72a201a686c5 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -190,6 +190,7 @@ config ARM64
select HAVE_NMI
select HAVE_PATA_PLATFORM
select HAVE_PERF_EVENTS
+ select HAVE_GUEST_PERF_EVENTS
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
select HAVE_REGS_AND_STACK_ACCESS_API
diff --git a/arch/csky/kernel/perf_callchain.c b/arch/csky/kernel/perf_callchain.c
index ab55e98ee8f6..92057de08f4f 100644
--- a/arch/csky/kernel/perf_callchain.c
+++ b/arch/csky/kernel/perf_callchain.c
@@ -88,10 +88,6 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
{
unsigned long fp = 0;
- /* C-SKY does not support virtualization. */
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
- return;
-
fp = regs->regs[4];
perf_callchain_store(entry, regs->pc);
@@ -112,12 +108,6 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
{
struct stackframe fr;
- /* C-SKY does not support virtualization. */
- if (perf_guest_cbs && perf_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);
diff --git a/arch/nds32/kernel/perf_event_cpu.c b/arch/nds32/kernel/perf_event_cpu.c
index 0ce6f9f307e6..a78a879e7ef1 100644
--- a/arch/nds32/kernel/perf_event_cpu.c
+++ b/arch/nds32/kernel/perf_event_cpu.c
@@ -1371,11 +1371,6 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry,
leaf_fp = 0;
- if (perf_guest_cbs && perf_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;
@@ -1481,10 +1476,6 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
{
struct stackframe fr;
- if (perf_guest_cbs && perf_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;
@@ -1493,10 +1484,6 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
- /* However, NDS32 does not support virtualization */
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
- return perf_guest_cbs->get_guest_ip();
-
return instruction_pointer(regs);
}
@@ -1504,18 +1491,10 @@ unsigned long perf_misc_flags(struct pt_regs *regs)
{
int misc = 0;
- /* However, NDS32 does not support virtualization */
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
- if (perf_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;
return misc;
}
diff --git a/arch/riscv/kernel/perf_callchain.c b/arch/riscv/kernel/perf_callchain.c
index 0bb1854dce83..1fc075b8f764 100644
--- a/arch/riscv/kernel/perf_callchain.c
+++ b/arch/riscv/kernel/perf_callchain.c
@@ -58,10 +58,6 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
{
unsigned long fp = 0;
- /* RISC-V does not support perf in guest mode. */
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
- return;
-
fp = regs->s0;
perf_callchain_store(entry, regs->epc);
@@ -78,11 +74,5 @@ static bool fill_callchain(void *entry, unsigned long pc)
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
- /* RISC-V does not support perf in guest mode. */
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
- pr_warn("RISC-V does not support perf in guest mode!");
- return;
- }
-
walk_stackframe(NULL, regs, fill_callchain, entry);
}
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 49270655e827..a4de4aa7a3df 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -229,6 +229,7 @@ config X86
select HAVE_PERF_EVENTS
select HAVE_PERF_EVENTS_NMI
select HAVE_HARDLOCKUP_DETECTOR_PERF if PERF_EVENTS && HAVE_PERF_EVENTS_NMI
+ select HAVE_GUEST_PERF_EVENTS
select HAVE_PCI
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 05c0efba3cd1..5eab690622ca 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1236,9 +1236,11 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,
enum perf_bpf_event_type type,
u16 flags);
+#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
extern struct perf_guest_info_callbacks *perf_guest_cbs;
extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
extern void perf_unregister_guest_info_callbacks(void);
+#endif /* CONFIG_HAVE_GUEST_PERF_EVENTS */
extern void perf_event_exec(void);
extern void perf_event_comm(struct task_struct *tsk, bool exec);
@@ -1481,9 +1483,11 @@ perf_sw_event(u32 event_id, u64 nr, struct pt_regs *regs, u64 addr) { }
static inline void
perf_bp_event(struct perf_event *event, void *data) { }
+#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
static inline void perf_register_guest_info_callbacks
(struct perf_guest_info_callbacks *callbacks) { }
static inline void perf_unregister_guest_info_callbacks(void) { }
+#endif
static inline void perf_event_mmap(struct vm_area_struct *vma) { }
diff --git a/init/Kconfig b/init/Kconfig
index 55f9f7738ebb..9ef51ae53977 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1776,6 +1776,9 @@ config HAVE_PERF_EVENTS
help
See tools/perf/design.txt for details.
+config HAVE_GUEST_PERF_EVENTS
+ bool
+
config PERF_USE_VMALLOC
bool
help
diff --git a/kernel/events/core.c b/kernel/events/core.c
index baae796612b9..9820df7ee455 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6482,6 +6482,7 @@ static void perf_pending_event(struct irq_work *entry)
perf_swevent_put_recursion_context(rctx);
}
+#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
struct perf_guest_info_callbacks *perf_guest_cbs;
void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
@@ -6495,6 +6496,7 @@ void perf_unregister_guest_info_callbacks(void)
perf_guest_cbs = NULL;
}
EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
+#endif
static void
perf_output_sample_regs(struct perf_output_handle *handle,
--
2.33.0.259.gc128427fd7-goog
Use a per-CPU pointer to track perf's guest callbacks so that KVM can set
the callbacks more precisely and avoid a lurking NULL pointer dereference.
On x86, KVM supports being built as a module and thus can be unloaded.
And because the shared callbacks are referenced from IRQ/NMI context,
unloading KVM can run concurrently with perf, and thus all of perf's
checks for a NULL perf_guest_cbs are flawed as perf_guest_cbs could be
nullified between the check and dereference.
In practice, this has not been problematic because the callbacks are
always guarded with a "perf_guest_cbs && perf_guest_cbs->is_in_guest()"
pattern, and it's extremely unlikely the compiler will choost to reload
perf_guest_cbs in that particular sequence. Because is_in_guest() is
obviously true only when KVM is running a guest, perf always wins the
race to the guarded code (which does often reload perf_guest_cbs) as KVM
has to stop running all guests and do a heavy teardown before unloading.
Cc: Zhu Lingshan <lingshan.zhu@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm64/kernel/perf_callchain.c | 18 ++++++++++++------
arch/x86/events/core.c | 17 +++++++++++------
arch/x86/events/intel/core.c | 8 +++++---
include/linux/perf_event.h | 2 +-
kernel/events/core.c | 12 +++++++++---
5 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/arch/arm64/kernel/perf_callchain.c b/arch/arm64/kernel/perf_callchain.c
index 4a72c2727309..38555275c6a2 100644
--- a/arch/arm64/kernel/perf_callchain.c
+++ b/arch/arm64/kernel/perf_callchain.c
@@ -102,7 +102,9 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,
void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_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)
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
struct stackframe frame;
- 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;
}
@@ -160,18 +163,21 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
- return perf_guest_cbs->get_guest_ip();
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
+
+ if (guest_cbs && guest_cbs->is_in_guest())
+ return guest_cbs->get_guest_ip();
return instruction_pointer(regs);
}
unsigned long perf_misc_flags(struct pt_regs *regs)
{
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
int misc = 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;
else
misc |= PERF_RECORD_MISC_GUEST_KERNEL;
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 1eb45139fcc6..34155a52e498 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2761,10 +2761,11 @@ static bool perf_hw_regs(struct pt_regs *regs)
void
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
struct unwind_state state;
unsigned long addr;
- 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 *ent
void
perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
struct stack_frame frame;
const struct stack_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)
unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
- if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
- return perf_guest_cbs->get_guest_ip();
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
+
+ if (guest_cbs && guest_cbs->is_in_guest())
+ return guest_cbs->get_guest_ip();
return regs->ip + code_segment_base(regs);
}
unsigned long perf_misc_flags(struct pt_regs *regs)
{
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
int misc = 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;
else
misc |= PERF_RECORD_MISC_GUEST_KERNEL;
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index fca7a6e2242f..96001962c24d 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2784,6 +2784,7 @@ static void intel_pmu_reset(void)
static int handle_pmi_common(struct pt_regs *regs, u64 status)
{
+ struct perf_guest_info_callbacks *guest_cbs;
struct perf_sample_data data;
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
int bit;
@@ -2852,9 +2853,10 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
*/
if (__test_and_clear_bit(GLOBAL_STATUS_TRACE_TOPAPMI_BIT, (unsigned long *)&status)) {
handled++;
- if (unlikely(perf_guest_cbs && perf_guest_cbs->is_in_guest() &&
- perf_guest_cbs->handle_intel_pt_intr))
- perf_guest_cbs->handle_intel_pt_intr();
+ guest_cbs = this_cpu_read(perf_guest_cbs);
+ if (unlikely(guest_cbs && guest_cbs->is_in_guest() &&
+ guest_cbs->handle_intel_pt_intr))
+ guest_cbs->handle_intel_pt_intr();
else
intel_pt_interrupt();
}
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 5eab690622ca..c98253dae037 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1237,7 +1237,7 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,
u16 flags);
#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
-extern struct perf_guest_info_callbacks *perf_guest_cbs;
+DECLARE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
extern void perf_unregister_guest_info_callbacks(void);
#endif /* CONFIG_HAVE_GUEST_PERF_EVENTS */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9820df7ee455..9bc1375d6ed9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6483,17 +6483,23 @@ static void perf_pending_event(struct irq_work *entry)
}
#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
-struct perf_guest_info_callbacks *perf_guest_cbs;
+DEFINE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
{
- perf_guest_cbs = cbs;
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ per_cpu(perf_guest_cbs, cpu) = cbs;
}
EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
void perf_unregister_guest_info_callbacks(void)
{
- perf_guest_cbs = NULL;
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ per_cpu(perf_guest_cbs, cpu) = NULL;
}
EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
#endif
--
2.33.0.259.gc128427fd7-goog
On Thu, Aug 26, 2021 at 05:57:08PM -0700, Sean Christopherson wrote: > Use a per-CPU pointer to track perf's guest callbacks so that KVM can set > the callbacks more precisely and avoid a lurking NULL pointer dereference. I'm completely failing to see how per-cpu helps anything here... > On x86, KVM supports being built as a module and thus can be unloaded. > And because the shared callbacks are referenced from IRQ/NMI context, > unloading KVM can run concurrently with perf, and thus all of perf's > checks for a NULL perf_guest_cbs are flawed as perf_guest_cbs could be > nullified between the check and dereference. No longer allowing KVM to be a module would be *AWESOME*. I detest how much we have to export for KVM :/ Still, what stops KVM from doing a coherent unreg? Even the static_call() proposed in the other patch, unreg can do static_call_update() + synchronize_rcu() to ensure everybody sees the updated pointer (would require a quick audit to see all users are with preempt disabled, but I think your using per-cpu here already imposes the same).
On Fri, Aug 27, 2021, Peter Zijlstra wrote: > On Thu, Aug 26, 2021 at 05:57:08PM -0700, Sean Christopherson wrote: > > Use a per-CPU pointer to track perf's guest callbacks so that KVM can set > > the callbacks more precisely and avoid a lurking NULL pointer dereference. > > I'm completely failing to see how per-cpu helps anything here... It doesn't help until KVM is converted to set the per-cpu pointer in flows that are protected against preemption, and more specifically when KVM only writes to the pointer from the owning CPU. > > On x86, KVM supports being built as a module and thus can be unloaded. > > And because the shared callbacks are referenced from IRQ/NMI context, > > unloading KVM can run concurrently with perf, and thus all of perf's > > checks for a NULL perf_guest_cbs are flawed as perf_guest_cbs could be > > nullified between the check and dereference. > > No longer allowing KVM to be a module would be *AWESOME*. I detest how > much we have to export for KVM :/ > > Still, what stops KVM from doing a coherent unreg? Even the > static_call() proposed in the other patch, unreg can do > static_call_update() + synchronize_rcu() to ensure everybody sees the > updated pointer (would require a quick audit to see all users are with > preempt disabled, but I think your using per-cpu here already imposes > the same). Ignoring static call for the moment, I don't see how the unreg side can be safe using a bare single global pointer. There is no way for KVM to prevent an NMI from running in parallel on a different CPU. If there's a more elegant solution, especially something that can be backported, e.g. an rcu-protected pointer, I'm all for it. I went down the per-cpu path because it allowed for cleanups in KVM, but similar cleanups can be done without per-cpu perf callbacks. As for static calls, I certainly have no objection to employing static calls for the callbacks, but IMO we should not be relying on static call for correctness, i.e. the existing bug needs to be fixed first.
On Fri, Aug 27, 2021 at 02:49:50PM +0000, Sean Christopherson wrote: > On Fri, Aug 27, 2021, Peter Zijlstra wrote: > > On Thu, Aug 26, 2021 at 05:57:08PM -0700, Sean Christopherson wrote: > > > Use a per-CPU pointer to track perf's guest callbacks so that KVM can set > > > the callbacks more precisely and avoid a lurking NULL pointer dereference. > > > > I'm completely failing to see how per-cpu helps anything here... > > It doesn't help until KVM is converted to set the per-cpu pointer in flows that > are protected against preemption, and more specifically when KVM only writes to > the pointer from the owning CPU. So the 'problem' I have with this is that sane (!KVM using) people, will still have to suffer that load, whereas with the static_call() we patch in an 'xor %rax,%rax' and only have immediate code flow. > Ignoring static call for the moment, I don't see how the unreg side can be safe > using a bare single global pointer. There is no way for KVM to prevent an NMI > from running in parallel on a different CPU. If there's a more elegant solution, > especially something that can be backported, e.g. an rcu-protected pointer, I'm > all for it. I went down the per-cpu path because it allowed for cleanups in KVM, > but similar cleanups can be done without per-cpu perf callbacks. If all the perf_guest_cbs dereferences are with preemption disabled (IRQs disabled, IRQ context, NMI context included), then the sequence: WRITE_ONCE(perf_guest_cbs, NULL); synchronize_rcu(); Ensures that all prior observers of perf_guest_csb will have completed and future observes must observe the NULL value.
On Fri, Aug 27, 2021, Peter Zijlstra wrote: > On Fri, Aug 27, 2021 at 02:49:50PM +0000, Sean Christopherson wrote: > > On Fri, Aug 27, 2021, Peter Zijlstra wrote: > > > On Thu, Aug 26, 2021 at 05:57:08PM -0700, Sean Christopherson wrote: > > > > Use a per-CPU pointer to track perf's guest callbacks so that KVM can set > > > > the callbacks more precisely and avoid a lurking NULL pointer dereference. > > > > > > I'm completely failing to see how per-cpu helps anything here... > > > > It doesn't help until KVM is converted to set the per-cpu pointer in flows that > > are protected against preemption, and more specifically when KVM only writes to > > the pointer from the owning CPU. > > So the 'problem' I have with this is that sane (!KVM using) people, will > still have to suffer that load, whereas with the static_call() we patch > in an 'xor %rax,%rax' and only have immediate code flow. Again, I've no objection to the static_call() approach. I didn't even see the patch until I had finished testing my series :-/ > > Ignoring static call for the moment, I don't see how the unreg side can be safe > > using a bare single global pointer. There is no way for KVM to prevent an NMI > > from running in parallel on a different CPU. If there's a more elegant solution, > > especially something that can be backported, e.g. an rcu-protected pointer, I'm > > all for it. I went down the per-cpu path because it allowed for cleanups in KVM, > > but similar cleanups can be done without per-cpu perf callbacks. > > If all the perf_guest_cbs dereferences are with preemption disabled > (IRQs disabled, IRQ context, NMI context included), then the sequence: > > WRITE_ONCE(perf_guest_cbs, NULL); > synchronize_rcu(); > > Ensures that all prior observers of perf_guest_csb will have completed > and future observes must observe the NULL value. That alone won't be sufficient, as the read side also needs to ensure it doesn't reload perf_guest_cbs between NULL checks and dereferences. But that's easy enough to solve with a READ_ONCE and maybe a helper to make it more cumbersome to use perf_guest_cbs directly. How about this for a series? 1. Use READ_ONCE/WRITE_ONCE + synchronize_rcu() to fix the underlying bug 2. Fix KVM PT interrupt handler bug 3. Kill off perf_guest_cbs usage in architectures that don't need the callbacks 4. Replace ->is_in_guest()/->is_user_mode() with ->state(), and s/get_guest_ip/get_ip 5. Implement static_call() support 6. Cleanups, if there are any 6..N KVM cleanups, e.g. to eliminate current_vcpu and share x86+arm64 callbacks
Register KVM's perf callback only when handling an interrupt that may be
a PMI (sadly this includes IRQs), and unregister the callback immediately
after handling the interrupt (or closing the window). Registering the
callback on a per-CPU basis (with preemption disabled!), fixes a mostly
theoretical bug where perf could dereference a NULL pointer due to KVM
unloading and unregistering the callbacks in between perf queries of the
callback functions. The precise registration will also allow for future
cleanups and optimizations, e.g. the existence of the callbacks can serve
as the "in guest" check.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 27 +++++++++++++++++----------
arch/x86/kvm/x86.h | 10 ++++++++++
include/linux/perf_event.h | 2 ++
kernel/events/core.c | 12 ++++++++++++
4 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bae951344e28..bc4ee6ea7752 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8274,28 +8274,31 @@ int kvm_is_in_guest(void)
static int kvm_is_user_mode(void)
{
- int user_mode = 3;
+ struct kvm_vcpu *vcpu = __this_cpu_read(current_vcpu);
- if (__this_cpu_read(current_vcpu))
- user_mode = static_call(kvm_x86_get_cpl)(__this_cpu_read(current_vcpu));
+ if (WARN_ON_ONCE(!vcpu))
+ return 0;
- return user_mode != 0;
+ return static_call(kvm_x86_get_cpl)(vcpu) != 0;
}
static unsigned long kvm_get_guest_ip(void)
{
- unsigned long ip = 0;
+ struct kvm_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))
+ return 0;
- return ip;
+ return kvm_rip_read(vcpu);
}
static void kvm_handle_intel_pt_intr(void)
{
struct kvm_vcpu *vcpu = __this_cpu_read(current_vcpu);
+ if (WARN_ON_ONCE(!vcpu))
+ return;
+
kvm_make_request(KVM_REQ_PMI, vcpu);
__set_bit(MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT,
(unsigned long *)&vcpu->arch.pmu.global_status);
@@ -8308,6 +8311,12 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
.handle_intel_pt_intr = NULL,
};
+void kvm_register_perf_callbacks(void)
+{
+ __perf_register_guest_info_callbacks(&kvm_guest_cbs);
+}
+EXPORT_SYMBOL_GPL(kvm_register_perf_callbacks);
+
#ifdef CONFIG_X86_64
static void pvclock_gtod_update_fn(struct work_struct *work)
{
@@ -11063,7 +11072,6 @@ int kvm_arch_hardware_setup(void *opaque)
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);
if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
supported_xss = 0;
@@ -11092,7 +11100,6 @@ int kvm_arch_hardware_setup(void *opaque)
void kvm_arch_hardware_unsetup(void)
{
- perf_unregister_guest_info_callbacks();
kvm_guest_cbs.handle_intel_pt_intr = NULL;
static_call(kvm_x86_hardware_unsetup)();
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 7d66d63dc55a..5cedc0e8a5d5 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -387,15 +387,25 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)
return kvm->arch.cstate_in_guest;
}
+void kvm_register_perf_callbacks(void);
+static inline void kvm_unregister_perf_callbacks(void)
+{
+ __perf_unregister_guest_info_callbacks();
+}
+
DECLARE_PER_CPU(struct kvm_vcpu *, current_vcpu);
static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu)
{
__this_cpu_write(current_vcpu, vcpu);
+
+ kvm_register_perf_callbacks();
}
static inline void kvm_after_interrupt(struct kvm_vcpu *vcpu)
{
+ kvm_unregister_perf_callbacks();
+
__this_cpu_write(current_vcpu, NULL);
}
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index c98253dae037..7a367bf1b78d 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1238,6 +1238,8 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,
#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
DECLARE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
+extern void __perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs);
+extern void __perf_unregister_guest_info_callbacks(void);
extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
extern void perf_unregister_guest_info_callbacks(void);
#endif /* CONFIG_HAVE_GUEST_PERF_EVENTS */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9bc1375d6ed9..2f28d9d8dc94 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6485,6 +6485,18 @@ static void perf_pending_event(struct irq_work *entry)
#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
DEFINE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
+void __perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
+{
+ __this_cpu_write(perf_guest_cbs, cbs);
+}
+EXPORT_SYMBOL_GPL(__perf_register_guest_info_callbacks);
+
+void __perf_unregister_guest_info_callbacks(void)
+{
+ __this_cpu_write(perf_guest_cbs, NULL);
+}
+EXPORT_SYMBOL_GPL(__perf_unregister_guest_info_callbacks);
+
void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
{
int cpu;
--
2.33.0.259.gc128427fd7-goog
On Thu, Aug 26, 2021 at 05:57:09PM -0700, Sean Christopherson wrote:
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 9bc1375d6ed9..2f28d9d8dc94 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6485,6 +6485,18 @@ static void perf_pending_event(struct irq_work *entry)
> #ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
> DEFINE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
>
> +void __perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
> +{
> + __this_cpu_write(perf_guest_cbs, cbs);
> +}
> +EXPORT_SYMBOL_GPL(__perf_register_guest_info_callbacks);
> +
> +void __perf_unregister_guest_info_callbacks(void)
> +{
> + __this_cpu_write(perf_guest_cbs, NULL);
> +}
> +EXPORT_SYMBOL_GPL(__perf_unregister_guest_info_callbacks);
This is 100% broken, and a prime example of why I hate modules.
It provides an interface for all modules, and completely fails to
validate even the most basic usage.
By using __this_cpu*() it omits the preemption checks, so you can call
this with preemption enabled, no problem.
By not checking the previous state, multiple modules can call this
interleaved without issue.
Basically assume any EXPORTed function is hostile, binary modules and
out-of-tree modules *are* just that. It's a cesspit out there.
Add a dedicated flag to detect the case where KVM's PMC overflow
callback was originally invoked in response to an NMI that arrived while
the guest was running. Using current_vcpu is less precise as IRQs also
set current_vcpu (though presumably KVM's callback should not be reached
in that case), and more importantly, this will allow dropping
current_vcpu as the perf callbacks can switch to kvm_running_vcpu now
that the perf callbacks are precisely registered, i.e. kvm_running_vcpu
doesn't need to be used to detect if a PMI arrived in the guest.
Fixes: dd60d217062f ("KVM: x86: Fix perf timer mode IP reporting")
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/svm/svm.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 2 +-
arch/x86/kvm/x86.c | 4 ++--
arch/x86/kvm/x86.h | 4 +++-
6 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1ea4943a73d7..465b35736d9b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -763,6 +763,7 @@ struct kvm_vcpu_arch {
unsigned nmi_pending; /* NMI queued after currently running handler */
bool nmi_injected; /* Trying to inject an NMI this entry */
bool smi_pending; /* SMI queued after currently running handler */
+ bool handling_nmi_from_guest;
struct kvm_mtrr mtrr_state;
u64 pat;
@@ -1874,8 +1875,6 @@ int kvm_skip_emulated_instruction(struct kvm_vcpu *vcpu);
int kvm_complete_insn_gp(struct kvm_vcpu *vcpu, int err);
void __kvm_request_immediate_exit(struct kvm_vcpu *vcpu);
-int kvm_is_in_guest(void);
-
void __user *__x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa,
u32 size);
bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 0772bad9165c..2b8934b452ea 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -87,7 +87,7 @@ static void kvm_perf_overflow_intr(struct perf_event *perf_event,
* woken up. So we should wake it, but this is impossible from
* NMI context. Do it from irq work instead.
*/
- if (!kvm_is_in_guest())
+ if (!pmc->vcpu->arch.handling_nmi_from_guest)
irq_work_queue(&pmc_to_pmu(pmc)->irq_work);
else
kvm_make_request(KVM_REQ_PMI, pmc->vcpu);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 1a70e11f0487..3fc6767e5fd8 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3843,7 +3843,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
}
if (unlikely(svm->vmcb->control.exit_code == SVM_EXIT_NMI))
- kvm_before_interrupt(vcpu);
+ kvm_before_interrupt(vcpu, true);
kvm_load_host_xsave_state(vcpu);
stgi();
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f19d72136f77..f08980ef7c44 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6344,7 +6344,7 @@ void vmx_do_interrupt_nmi_irqoff(unsigned long entry);
static void handle_interrupt_nmi_irqoff(struct kvm_vcpu *vcpu,
unsigned long entry)
{
- kvm_before_interrupt(vcpu);
+ kvm_before_interrupt(vcpu, entry == (unsigned long)asm_exc_nmi_noist);
vmx_do_interrupt_nmi_irqoff(entry);
kvm_after_interrupt(vcpu);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index bc4ee6ea7752..d4d91944fde7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8267,7 +8267,7 @@ static void kvm_timer_init(void)
DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);
EXPORT_PER_CPU_SYMBOL_GPL(current_vcpu);
-int kvm_is_in_guest(void)
+static int kvm_is_in_guest(void)
{
return __this_cpu_read(current_vcpu) != NULL;
}
@@ -9678,7 +9678,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
* interrupts on processors that implement an interrupt shadow, the
* stat.exits increment will do nicely.
*/
- kvm_before_interrupt(vcpu);
+ kvm_before_interrupt(vcpu, false);
local_irq_enable();
++vcpu->stat.exits;
local_irq_disable();
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 5cedc0e8a5d5..4c5ba4128b38 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -395,9 +395,10 @@ static inline void kvm_unregister_perf_callbacks(void)
DECLARE_PER_CPU(struct kvm_vcpu *, current_vcpu);
-static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu)
+static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu, bool is_nmi)
{
__this_cpu_write(current_vcpu, vcpu);
+ WRITE_ONCE(vcpu->arch.handling_nmi_from_guest, is_nmi);
kvm_register_perf_callbacks();
}
@@ -406,6 +407,7 @@ static inline void kvm_after_interrupt(struct kvm_vcpu *vcpu)
{
kvm_unregister_perf_callbacks();
+ WRITE_ONCE(vcpu->arch.handling_nmi_from_guest, false);
__this_cpu_write(current_vcpu, NULL);
}
--
2.33.0.259.gc128427fd7-goog
Now that KVM registers perf callbacks only when the CPU is "in guest",
use kvm_running_vcpu instead of current_vcpu to retrieve the associated
vCPU and drop current_vcpu.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/x86.c | 12 +++++-------
arch/x86/kvm/x86.h | 4 ----
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d4d91944fde7..e337aef60793 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8264,17 +8264,15 @@ static void kvm_timer_init(void)
kvmclock_cpu_online, kvmclock_cpu_down_prep);
}
-DEFINE_PER_CPU(struct kvm_vcpu *, current_vcpu);
-EXPORT_PER_CPU_SYMBOL_GPL(current_vcpu);
-
static int kvm_is_in_guest(void)
{
- return __this_cpu_read(current_vcpu) != NULL;
+ /* x86's callbacks are registered only when handling a guest NMI. */
+ return true;
}
static int kvm_is_user_mode(void)
{
- struct kvm_vcpu *vcpu = __this_cpu_read(current_vcpu);
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
if (WARN_ON_ONCE(!vcpu))
return 0;
@@ -8284,7 +8282,7 @@ static int kvm_is_user_mode(void)
static unsigned long kvm_get_guest_ip(void)
{
- struct kvm_vcpu *vcpu = __this_cpu_read(current_vcpu);
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
if (WARN_ON_ONCE(!vcpu))
return 0;
@@ -8294,7 +8292,7 @@ static unsigned long kvm_get_guest_ip(void)
static void kvm_handle_intel_pt_intr(void)
{
- struct kvm_vcpu *vcpu = __this_cpu_read(current_vcpu);
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
if (WARN_ON_ONCE(!vcpu))
return;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 4c5ba4128b38..f13f15d2fab8 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -393,11 +393,8 @@ static inline void kvm_unregister_perf_callbacks(void)
__perf_unregister_guest_info_callbacks();
}
-DECLARE_PER_CPU(struct kvm_vcpu *, current_vcpu);
-
static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu, bool is_nmi)
{
- __this_cpu_write(current_vcpu, vcpu);
WRITE_ONCE(vcpu->arch.handling_nmi_from_guest, is_nmi);
kvm_register_perf_callbacks();
@@ -408,7 +405,6 @@ static inline void kvm_after_interrupt(struct kvm_vcpu *vcpu)
kvm_unregister_perf_callbacks();
WRITE_ONCE(vcpu->arch.handling_nmi_from_guest, false);
- __this_cpu_write(current_vcpu, NULL);
}
--
2.33.0.259.gc128427fd7-goog
Register/unregister perf callbacks at vcpu_load()/vcpu_put() instead of
keeping the callbacks registered for all eternity after loading KVM.
This will allow future cleanups and optimizations as the registration
of the callbacks signifies "in guest". This will also allow moving the
callbacks into common KVM as they arm64 and x86 now have semantically
identical callback implementations.
Note, KVM could likely be more precise in its registration, but that's a
cleanup for the future.
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 | 36 ++++++++++++++-----------------
3 files changed, 31 insertions(+), 22 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index ed940aec89e0..007c38d77fd9 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -671,7 +671,17 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu);
int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa);
void kvm_perf_init(void);
-void kvm_perf_teardown(void);
+
+#ifdef CONFIG_PERF_EVENTS
+void kvm_register_perf_callbacks(void);
+static inline void kvm_unregister_perf_callbacks(void)
+{
+ __perf_unregister_guest_info_callbacks();
+}
+#else
+static inline void kvm_register_perf_callbacks(void) {}
+static inline void kvm_unregister_perf_callbacks(void) {}
+#endif
long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);
gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index e9a2b8f27792..ec386971030d 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -429,10 +429,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
if (vcpu_has_ptrauth(vcpu))
vcpu_ptrauth_disable(vcpu);
kvm_arch_vcpu_load_debug_state_flags(vcpu);
+
+ kvm_register_perf_callbacks();
}
void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
{
+ kvm_unregister_perf_callbacks();
kvm_arch_vcpu_put_debug_state_flags(vcpu);
kvm_arch_vcpu_put_fp(vcpu);
if (has_vhe())
@@ -2155,7 +2158,7 @@ int kvm_arch_init(void *opaque)
/* NOP: Compiling as a module not supported */
void kvm_arch_exit(void)
{
- kvm_perf_teardown();
+
}
static int __init early_kvm_mode_cfg(char *arg)
diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
index 039fe59399a2..2556b0a3b096 100644
--- a/arch/arm64/kvm/perf.c
+++ b/arch/arm64/kvm/perf.c
@@ -13,33 +13,30 @@
DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
+#ifdef CONFIG_PERF_EVENTS
static int kvm_is_in_guest(void)
{
- return kvm_get_running_vcpu() != NULL;
+ return true;
}
static int kvm_is_user_mode(void)
{
- struct kvm_vcpu *vcpu;
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
- vcpu = kvm_get_running_vcpu();
+ if (WARN_ON_ONCE(!vcpu))
+ return 0;
- if (vcpu)
- return !vcpu_mode_priv(vcpu);
-
- return 0;
+ return !vcpu_mode_priv(vcpu);
}
static unsigned long kvm_get_guest_ip(void)
{
- struct kvm_vcpu *vcpu;
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
- vcpu = kvm_get_running_vcpu();
+ if (WARN_ON_ONCE(!vcpu))
+ return 0;
- if (vcpu)
- return *vcpu_pc(vcpu);
-
- return 0;
+ return *vcpu_pc(vcpu);
}
static struct perf_guest_info_callbacks kvm_guest_cbs = {
@@ -48,15 +45,14 @@ static struct perf_guest_info_callbacks kvm_guest_cbs = {
.get_guest_ip = kvm_get_guest_ip,
};
+void kvm_register_perf_callbacks(void)
+{
+ __perf_register_guest_info_callbacks(&kvm_guest_cbs);
+}
+#endif /* CONFIG_PERF_EVENTS*/
+
void kvm_perf_init(void)
{
if (kvm_pmu_probe_pmuver() != 0xf && !is_protected_kvm_enabled())
static_branch_enable(&kvm_arm_pmu_available);
-
- perf_register_guest_info_callbacks(&kvm_guest_cbs);
-}
-
-void kvm_perf_teardown(void)
-{
- perf_unregister_guest_info_callbacks();
}
--
2.33.0.259.gc128427fd7-goog
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 | 1 +
arch/x86/kvm/x86.c | 48 +++++----------------------------
arch/x86/kvm/x86.h | 6 -----
include/linux/kvm_host.h | 12 +++++++++
virt/kvm/kvm_main.c | 46 +++++++++++++++++++++++++++++++
5 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 465b35736d9b..63553a1f43ee 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -36,6 +36,7 @@
#include <asm/hyperv-tlfs.h>
#define __KVM_HAVE_ARCH_VCPU_DEBUGFS
+#define __KVM_WANT_PERF_CALLBACKS
#define KVM_MAX_VCPUS 288
#define KVM_SOFT_MAX_VCPUS 240
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e337aef60793..7cb0f04e24ee 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8264,32 +8264,6 @@ static void kvm_timer_init(void)
kvmclock_cpu_online, kvmclock_cpu_down_prep);
}
-static int kvm_is_in_guest(void)
-{
- /* x86's callbacks are registered only when handling a guest NMI. */
- return true;
-}
-
-static int kvm_is_user_mode(void)
-{
- struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
-
- if (WARN_ON_ONCE(!vcpu))
- return 0;
-
- return static_call(kvm_x86_get_cpl)(vcpu) != 0;
-}
-
-static unsigned long kvm_get_guest_ip(void)
-{
- struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
-
- if (WARN_ON_ONCE(!vcpu))
- return 0;
-
- return kvm_rip_read(vcpu);
-}
-
static void kvm_handle_intel_pt_intr(void)
{
struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
@@ -8302,19 +8276,6 @@ static void kvm_handle_intel_pt_intr(void)
(unsigned long *)&vcpu->arch.pmu.global_status);
}
-static struct perf_guest_info_callbacks kvm_guest_cbs = {
- .is_in_guest = kvm_is_in_guest,
- .is_user_mode = kvm_is_user_mode,
- .get_guest_ip = kvm_get_guest_ip,
- .handle_intel_pt_intr = NULL,
-};
-
-void kvm_register_perf_callbacks(void)
-{
- __perf_register_guest_info_callbacks(&kvm_guest_cbs);
-}
-EXPORT_SYMBOL_GPL(kvm_register_perf_callbacks);
-
#ifdef CONFIG_X86_64
static void pvclock_gtod_update_fn(struct work_struct *work)
{
@@ -11069,7 +11030,7 @@ int kvm_arch_hardware_setup(void *opaque)
kvm_ops_static_call_update();
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;
+ kvm_set_intel_pt_intr_handler(kvm_handle_intel_pt_intr);
if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
supported_xss = 0;
@@ -11098,7 +11059,7 @@ int kvm_arch_hardware_setup(void *opaque)
void kvm_arch_hardware_unsetup(void)
{
- kvm_guest_cbs.handle_intel_pt_intr = NULL;
+ kvm_set_intel_pt_intr_handler(NULL);
static_call(kvm_x86_hardware_unsetup)();
}
@@ -11725,6 +11686,11 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
return vcpu->arch.preempted_in_kernel;
}
+unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu)
+{
+ return kvm_rip_read(vcpu);
+}
+
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
{
return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index f13f15d2fab8..e1fe738c3827 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -387,12 +387,6 @@ static inline bool kvm_cstate_in_guest(struct kvm *kvm)
return kvm->arch.cstate_in_guest;
}
-void kvm_register_perf_callbacks(void);
-static inline void kvm_unregister_perf_callbacks(void)
-{
- __perf_unregister_guest_info_callbacks();
-}
-
static inline void kvm_before_interrupt(struct kvm_vcpu *vcpu, bool is_nmi)
{
WRITE_ONCE(vcpu->arch.handling_nmi_from_guest, is_nmi);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e4d712e9f760..0db9af0b628c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1163,6 +1163,18 @@ static inline bool kvm_arch_intc_initialized(struct kvm *kvm)
}
#endif
+#ifdef __KVM_WANT_PERF_CALLBACKS
+
+void kvm_set_intel_pt_intr_handler(void (*handler)(void));
+unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu);
+
+void kvm_register_perf_callbacks(void);
+static inline void kvm_unregister_perf_callbacks(void)
+{
+ __perf_unregister_guest_info_callbacks();
+}
+#endif
+
int kvm_arch_init_vm(struct kvm *kvm, unsigned long type);
void kvm_arch_destroy_vm(struct kvm *kvm);
void kvm_arch_sync_events(struct kvm *kvm);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 3e67c93ca403..13c4f58a75e5 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5460,6 +5460,52 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
return &kvm_running_vcpu;
}
+#ifdef __KVM_WANT_PERF_CALLBACKS
+static int kvm_is_in_guest(void)
+{
+ /* Registration of KVM's callback signifies "in guest". */
+ return true;
+}
+
+static int kvm_is_user_mode(void)
+{
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
+
+ if (WARN_ON_ONCE(!vcpu))
+ return 0;
+
+ return !kvm_arch_vcpu_in_kernel(vcpu);
+}
+
+static unsigned long kvm_get_guest_ip(void)
+{
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
+
+ if (WARN_ON_ONCE(!vcpu))
+ return 0;
+
+ return kvm_arch_vcpu_get_ip(vcpu);
+}
+
+static struct perf_guest_info_callbacks kvm_guest_cbs = {
+ .is_in_guest = kvm_is_in_guest,
+ .is_user_mode = kvm_is_user_mode,
+ .get_guest_ip = kvm_get_guest_ip,
+ .handle_intel_pt_intr = NULL,
+};
+
+void kvm_set_intel_pt_intr_handler(void (*handler)(void))
+{
+ kvm_guest_cbs.handle_intel_pt_intr = handler;
+}
+
+void kvm_register_perf_callbacks(void)
+{
+ __perf_register_guest_info_callbacks(&kvm_guest_cbs);
+}
+EXPORT_SYMBOL_GPL(kvm_register_perf_callbacks);
+#endif
+
struct kvm_cpu_compat_check {
void *opaque;
int *ret;
--
2.33.0.259.gc128427fd7-goog
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 | 1 -
arch/x86/kvm/vmx/vmx.c | 24 +++++++++++++++++++++---
arch/x86/kvm/x86.c | 17 -----------------
virt/kvm/kvm_main.c | 1 +
4 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 63553a1f43ee..daa33147650a 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1496,7 +1496,6 @@ struct kvm_x86_init_ops {
int (*disabled_by_bios)(void);
int (*check_processor_compatibility)(void);
int (*hardware_setup)(void);
- bool (*intel_pt_intr_in_guest)(void);
struct kvm_x86_ops *runtime_ops;
};
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f08980ef7c44..4665a272249a 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7535,6 +7535,8 @@ static void vmx_migrate_timers(struct kvm_vcpu *vcpu)
static void hardware_unsetup(void)
{
+ kvm_set_intel_pt_intr_handler(NULL);
+
if (nested)
nested_vmx_hardware_unsetup();
@@ -7685,6 +7687,18 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
.vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector,
};
+static void vmx_handle_intel_pt_intr(void)
+{
+ struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
+
+ if (WARN_ON_ONCE(!vcpu))
+ return;
+
+ kvm_make_request(KVM_REQ_PMI, vcpu);
+ __set_bit(MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT,
+ (unsigned long *)&vcpu->arch.pmu.global_status);
+}
+
static __init void vmx_setup_user_return_msrs(void)
{
@@ -7886,9 +7900,14 @@ static __init int hardware_setup(void)
vmx_set_cpu_caps();
r = alloc_kvm_area();
- if (r)
+ if (r) {
nested_vmx_hardware_unsetup();
- return r;
+ return r;
+ }
+
+ if (pt_mode == PT_MODE_HOST_GUEST)
+ kvm_set_intel_pt_intr_handler(vmx_handle_intel_pt_intr);
+ return 0;
}
static struct kvm_x86_init_ops vmx_init_ops __initdata = {
@@ -7896,7 +7915,6 @@ static struct kvm_x86_init_ops vmx_init_ops __initdata = {
.disabled_by_bios = vmx_disabled_by_bios,
.check_processor_compatibility = vmx_check_processor_compat,
.hardware_setup = hardware_setup,
- .intel_pt_intr_in_guest = vmx_pt_mode_is_host_guest,
.runtime_ops = &vmx_x86_ops,
};
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7cb0f04e24ee..11c7a02f839c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8264,18 +8264,6 @@ static void kvm_timer_init(void)
kvmclock_cpu_online, kvmclock_cpu_down_prep);
}
-static void kvm_handle_intel_pt_intr(void)
-{
- struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
-
- if (WARN_ON_ONCE(!vcpu))
- return;
-
- kvm_make_request(KVM_REQ_PMI, vcpu);
- __set_bit(MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT,
- (unsigned long *)&vcpu->arch.pmu.global_status);
-}
-
#ifdef CONFIG_X86_64
static void pvclock_gtod_update_fn(struct work_struct *work)
{
@@ -11029,9 +11017,6 @@ int kvm_arch_hardware_setup(void *opaque)
memcpy(&kvm_x86_ops, ops->runtime_ops, sizeof(kvm_x86_ops));
kvm_ops_static_call_update();
- if (ops->intel_pt_intr_in_guest && ops->intel_pt_intr_in_guest())
- kvm_set_intel_pt_intr_handler(kvm_handle_intel_pt_intr);
-
if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
supported_xss = 0;
@@ -11059,8 +11044,6 @@ int kvm_arch_hardware_setup(void *opaque)
void kvm_arch_hardware_unsetup(void)
{
- kvm_set_intel_pt_intr_handler(NULL);
-
static_call(kvm_x86_hardware_unsetup)();
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 13c4f58a75e5..e0b1c9386926 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5498,6 +5498,7 @@ void kvm_set_intel_pt_intr_handler(void (*handler)(void))
{
kvm_guest_cbs.handle_intel_pt_intr = handler;
}
+EXPORT_SYMBOL_GPL(kvm_set_intel_pt_intr_handler);
void kvm_register_perf_callbacks(void)
{
--
2.33.0.259.gc128427fd7-goog
On Thu, Aug 26, 2021 at 05:57:14PM -0700, Sean Christopherson wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 13c4f58a75e5..e0b1c9386926 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -5498,6 +5498,7 @@ void kvm_set_intel_pt_intr_handler(void (*handler)(void))
> {
> kvm_guest_cbs.handle_intel_pt_intr = handler;
> }
> +EXPORT_SYMBOL_GPL(kvm_set_intel_pt_intr_handler);
This is another one of those broken exports.
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 | 6 +----
arch/arm64/kvm/arm.c | 5 ++++
arch/arm64/kvm/perf.c | 38 -------------------------------
3 files changed, 6 insertions(+), 43 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 007c38d77fd9..12e8d789e1ac 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -673,11 +673,7 @@ int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa);
void kvm_perf_init(void);
#ifdef CONFIG_PERF_EVENTS
-void kvm_register_perf_callbacks(void);
-static inline void kvm_unregister_perf_callbacks(void)
-{
- __perf_unregister_guest_info_callbacks();
-}
+#define __KVM_WANT_PERF_CALLBACKS
#else
static inline void kvm_register_perf_callbacks(void) {}
static inline void kvm_unregister_perf_callbacks(void) {}
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index ec386971030d..dfc8078dd4f9 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -503,6 +503,11 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
return vcpu_mode_priv(vcpu);
}
+unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu)
+{
+ return *vcpu_pc(vcpu);
+}
+
/* Just ensure a guest exit from a particular CPU */
static void exit_vm_noop(void *info)
{
diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
index 2556b0a3b096..ad9fdc2f2f70 100644
--- a/arch/arm64/kvm/perf.c
+++ b/arch/arm64/kvm/perf.c
@@ -13,44 +13,6 @@
DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
-#ifdef CONFIG_PERF_EVENTS
-static int kvm_is_in_guest(void)
-{
- return true;
-}
-
-static int kvm_is_user_mode(void)
-{
- struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
-
- if (WARN_ON_ONCE(!vcpu))
- return 0;
-
- return !vcpu_mode_priv(vcpu);
-}
-
-static unsigned long kvm_get_guest_ip(void)
-{
- struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
-
- if (WARN_ON_ONCE(!vcpu))
- return 0;
-
- return *vcpu_pc(vcpu);
-}
-
-static struct perf_guest_info_callbacks kvm_guest_cbs = {
- .is_in_guest = kvm_is_in_guest,
- .is_user_mode = kvm_is_user_mode,
- .get_guest_ip = kvm_get_guest_ip,
-};
-
-void kvm_register_perf_callbacks(void)
-{
- __perf_register_guest_info_callbacks(&kvm_guest_cbs);
-}
-#endif /* CONFIG_PERF_EVENTS*/
-
void kvm_perf_init(void)
{
if (kvm_pmu_probe_pmuver() != 0xf && !is_protected_kvm_enabled())
--
2.33.0.259.gc128427fd7-goog
Fold that last few remnants of perf.c into pmu.c and rename the init
helper as appropriate.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm64/include/asm/kvm_host.h | 2 --
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/arm.c | 3 ++-
arch/arm64/kvm/perf.c | 20 --------------------
arch/arm64/kvm/pmu.c | 8 ++++++++
include/kvm/arm_pmu.h | 1 +
6 files changed, 12 insertions(+), 24 deletions(-)
delete mode 100644 arch/arm64/kvm/perf.c
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 12e8d789e1ac..86c0fdd11ad2 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -670,8 +670,6 @@ unsigned long kvm_mmio_read_buf(const void *buf, unsigned int len);
int kvm_handle_mmio_return(struct kvm_vcpu *vcpu);
int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa);
-void kvm_perf_init(void);
-
#ifdef CONFIG_PERF_EVENTS
#define __KVM_WANT_PERF_CALLBACKS
#else
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 989bb5dad2c8..0bcc378b7961 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -12,7 +12,7 @@ obj-$(CONFIG_KVM) += hyp/
kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
$(KVM)/vfio.o $(KVM)/irqchip.o $(KVM)/binary_stats.o \
- arm.o mmu.o mmio.o psci.o perf.o hypercalls.o pvtime.o \
+ arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
inject_fault.o va_layout.o handle_exit.o \
guest.o debug.o reset.o sys_regs.o \
vgic-sys-reg-v3.o fpsimd.o pmu.o \
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index dfc8078dd4f9..57e637dee71d 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1747,7 +1747,8 @@ static int init_subsystems(void)
if (err)
goto out;
- kvm_perf_init();
+ kvm_pmu_init();
+
kvm_sys_reg_table_init();
out:
diff --git a/arch/arm64/kvm/perf.c b/arch/arm64/kvm/perf.c
deleted file mode 100644
index ad9fdc2f2f70..000000000000
--- a/arch/arm64/kvm/perf.c
+++ /dev/null
@@ -1,20 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Based on the x86 implementation.
- *
- * Copyright (C) 2012 ARM Ltd.
- * Author: Marc Zyngier <marc.zyngier@arm.com>
- */
-
-#include <linux/perf_event.h>
-#include <linux/kvm_host.h>
-
-#include <asm/kvm_emulate.h>
-
-DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
-
-void kvm_perf_init(void)
-{
- if (kvm_pmu_probe_pmuver() != 0xf && !is_protected_kvm_enabled())
- static_branch_enable(&kvm_arm_pmu_available);
-}
diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 03a6c1f4a09a..d98b57a17043 100644
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -7,6 +7,14 @@
#include <linux/perf_event.h>
#include <asm/kvm_hyp.h>
+DEFINE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
+
+void kvm_pmu_init(void)
+{
+ if (kvm_pmu_probe_pmuver() != 0xf && !is_protected_kvm_enabled())
+ static_branch_enable(&kvm_arm_pmu_available);
+}
+
/*
* Given the perf event attributes and system type, determine
* if we are going to need to switch counters at guest entry/exit.
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 864b9997efb2..42270676498d 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -14,6 +14,7 @@
#define ARMV8_PMU_MAX_COUNTER_PAIRS ((ARMV8_PMU_MAX_COUNTERS + 1) >> 1)
DECLARE_STATIC_KEY_FALSE(kvm_arm_pmu_available);
+void kvm_pmu_init(void);
static __always_inline bool kvm_arm_support_pmu_v3(void)
{
--
2.33.0.259.gc128427fd7-goog
Drop the helper that allows bulk unregistering of the per-CPU callbacks
now that KVM, the only entity that actually unregisters callbacks, uses
the per-CPU helpers. Bulk unregistering is inherently unsafe as there
are no protections against nullifying a pointer for a CPU that is using
said pointer in a PMI handler.
Opportunistically tweak names to better reflect reality.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/xen/pmu.c | 2 +-
include/linux/kvm_host.h | 2 +-
include/linux/perf_event.h | 9 +++------
kernel/events/core.c | 31 +++++++++++--------------------
virt/kvm/kvm_main.c | 2 +-
5 files changed, 17 insertions(+), 29 deletions(-)
diff --git a/arch/x86/xen/pmu.c b/arch/x86/xen/pmu.c
index e13b0b49fcdf..57834de043c3 100644
--- a/arch/x86/xen/pmu.c
+++ b/arch/x86/xen/pmu.c
@@ -548,7 +548,7 @@ void xen_pmu_init(int cpu)
per_cpu(xenpmu_shared, cpu).flags = 0;
if (cpu == 0) {
- perf_register_guest_info_callbacks(&xen_guest_cbs);
+ perf_register_guest_info_callbacks_all_cpus(&xen_guest_cbs);
xen_pmu_arch_init();
}
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 0db9af0b628c..d68a49d5fc53 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1171,7 +1171,7 @@ unsigned long kvm_arch_vcpu_get_ip(struct kvm_vcpu *vcpu);
void kvm_register_perf_callbacks(void);
static inline void kvm_unregister_perf_callbacks(void)
{
- __perf_unregister_guest_info_callbacks();
+ perf_unregister_guest_info_callbacks();
}
#endif
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 7a367bf1b78d..db701409a62f 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1238,10 +1238,9 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,
#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
DECLARE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
-extern void __perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs);
-extern void __perf_unregister_guest_info_callbacks(void);
-extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
+extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs);
extern void perf_unregister_guest_info_callbacks(void);
+extern void perf_register_guest_info_callbacks_all_cpus(struct perf_guest_info_callbacks *cbs);
#endif /* CONFIG_HAVE_GUEST_PERF_EVENTS */
extern void perf_event_exec(void);
@@ -1486,9 +1485,7 @@ static inline void
perf_bp_event(struct perf_event *event, void *data) { }
#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
-static inline void perf_register_guest_info_callbacks
-(struct perf_guest_info_callbacks *callbacks) { }
-static inline void perf_unregister_guest_info_callbacks(void) { }
+extern void perf_register_guest_info_callbacks_all_cpus(struct perf_guest_info_callbacks *cbs);
#endif
static inline void perf_event_mmap(struct vm_area_struct *vma) { }
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2f28d9d8dc94..f1964096c4c2 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6485,35 +6485,26 @@ static void perf_pending_event(struct irq_work *entry)
#ifdef CONFIG_HAVE_GUEST_PERF_EVENTS
DEFINE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
-void __perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
-{
- __this_cpu_write(perf_guest_cbs, cbs);
-}
-EXPORT_SYMBOL_GPL(__perf_register_guest_info_callbacks);
-
-void __perf_unregister_guest_info_callbacks(void)
-{
- __this_cpu_write(perf_guest_cbs, NULL);
-}
-EXPORT_SYMBOL_GPL(__perf_unregister_guest_info_callbacks);
-
void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
{
- int cpu;
-
- for_each_possible_cpu(cpu)
- per_cpu(perf_guest_cbs, cpu) = cbs;
+ __this_cpu_write(perf_guest_cbs, cbs);
}
EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
void perf_unregister_guest_info_callbacks(void)
{
- int cpu;
-
- for_each_possible_cpu(cpu)
- per_cpu(perf_guest_cbs, cpu) = NULL;
+ __this_cpu_write(perf_guest_cbs, NULL);
}
EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
+
+void perf_register_guest_info_callbacks_all_cpus(struct perf_guest_info_callbacks *cbs)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ per_cpu(perf_guest_cbs, cpu) = cbs;
+}
+EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks_all_cpus);
#endif
static void
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e0b1c9386926..1bcc3eab510b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5502,7 +5502,7 @@ EXPORT_SYMBOL_GPL(kvm_set_intel_pt_intr_handler);
void kvm_register_perf_callbacks(void)
{
- __perf_register_guest_info_callbacks(&kvm_guest_cbs);
+ perf_register_guest_info_callbacks(&kvm_guest_cbs);
}
EXPORT_SYMBOL_GPL(kvm_register_perf_callbacks);
#endif
--
2.33.0.259.gc128427fd7-goog
Interpret a null ->is_in_guest callback as meaning "in guest" and use
the new semantics in KVM, which currently returns 'true' unconditionally
in its implementation of ->is_in_guest(). This avoids a retpoline on
the indirect call for PMIs that arrive in a KVM guest, and also provides
a handy excuse for a wrapper around retrieval of perf_get_guest_cbs,
e.g. to reduce the probability of an errant direct read of perf_guest_cbs.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/events/core.c | 16 ++++++++--------
arch/x86/events/intel/core.c | 5 ++---
include/linux/perf_event.h | 17 +++++++++++++++++
virt/kvm/kvm_main.c | 9 ++-------
4 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 34155a52e498..b60c339ae06b 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2761,11 +2761,11 @@ static bool perf_hw_regs(struct pt_regs *regs)
void
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
- struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
+ struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct unwind_state state;
unsigned long addr;
- if (guest_cbs && guest_cbs->is_in_guest()) {
+ if (guest_cbs) {
/* TODO: We don't support guest os callchain now */
return;
}
@@ -2865,11 +2865,11 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent
void
perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
- struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
+ struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct stack_frame frame;
const struct stack_frame __user *fp;
- if (guest_cbs && guest_cbs->is_in_guest()) {
+ if (guest_cbs) {
/* TODO: We don't support guest os callchain now */
return;
}
@@ -2946,9 +2946,9 @@ static unsigned long code_segment_base(struct pt_regs *regs)
unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
- struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
+ struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
- if (guest_cbs && guest_cbs->is_in_guest())
+ if (guest_cbs)
return guest_cbs->get_guest_ip();
return regs->ip + code_segment_base(regs);
@@ -2956,10 +2956,10 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
unsigned long perf_misc_flags(struct pt_regs *regs)
{
- struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
+ struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
int misc = 0;
- if (guest_cbs && guest_cbs->is_in_guest()) {
+ if (guest_cbs) {
if (guest_cbs->is_user_mode())
misc |= PERF_RECORD_MISC_GUEST_USER;
else
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 96001962c24d..9a8c18b51a96 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2853,9 +2853,8 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
*/
if (__test_and_clear_bit(GLOBAL_STATUS_TRACE_TOPAPMI_BIT, (unsigned long *)&status)) {
handled++;
- guest_cbs = this_cpu_read(perf_guest_cbs);
- if (unlikely(guest_cbs && guest_cbs->is_in_guest() &&
- guest_cbs->handle_intel_pt_intr))
+ guest_cbs = perf_get_guest_cbs();
+ if (unlikely(guest_cbs && guest_cbs->handle_intel_pt_intr))
guest_cbs->handle_intel_pt_intr();
else
intel_pt_interrupt();
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index db701409a62f..6e3a10784d24 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1241,6 +1241,23 @@ DECLARE_PER_CPU(struct perf_guest_info_callbacks *, perf_guest_cbs);
extern void perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs);
extern void perf_unregister_guest_info_callbacks(void);
extern void perf_register_guest_info_callbacks_all_cpus(struct perf_guest_info_callbacks *cbs);
+/*
+ * Returns guest callbacks for the current CPU if callbacks are registered and
+ * the PMI fired while a guest was running, otherwise returns NULL.
+ */
+static inline struct perf_guest_info_callbacks *perf_get_guest_cbs(void)
+{
+ struct perf_guest_info_callbacks *guest_cbs = this_cpu_read(perf_guest_cbs);
+
+ /*
+ * Implementing is_in_guest is optional if the callbacks are registered
+ * only when "in guest".
+ */
+ if (guest_cbs && (!guest_cbs->is_in_guest || guest_cbs->is_in_guest()))
+ return guest_cbs;
+
+ return NULL;
+}
#endif /* CONFIG_HAVE_GUEST_PERF_EVENTS */
extern void perf_event_exec(void);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1bcc3eab510b..fa83d3846785 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5461,12 +5461,6 @@ struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
}
#ifdef __KVM_WANT_PERF_CALLBACKS
-static int kvm_is_in_guest(void)
-{
- /* Registration of KVM's callback signifies "in guest". */
- return true;
-}
-
static int kvm_is_user_mode(void)
{
struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
@@ -5488,7 +5482,8 @@ static unsigned long kvm_get_guest_ip(void)
}
static struct perf_guest_info_callbacks kvm_guest_cbs = {
- .is_in_guest = kvm_is_in_guest,
+ /* Registration of KVM's callback signifies "in guest". */
+ .is_in_guest = NULL,
.is_user_mode = kvm_is_user_mode,
.get_guest_ip = kvm_get_guest_ip,
.handle_intel_pt_intr = NULL,
--
2.33.0.259.gc128427fd7-goog
© 2016 - 2024 Red Hat, Inc.