2024-03-07 01:14:01

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH 0/3] KVM: VMX: Disable LBRs if CPU doesn't have callstacks

Disable LBR virtualization if the CPU (or I guess perf) doesn't support
LBR callstacks, as KVM unconditionally creates the associated perf LBR
event with PERF_SAMPLE_BRANCH_CALL_STACK. That results in perf rejecting
the event, and cause LBR virtualization to silently fail.

This was detected by running vmx_pmu_caps_test on older hardware. I didn't
tag it for stable because I can't imagine anyone is trying to use KVM's LBR
virtualization on pre-HSW.

Sean Christopherson (3):
KVM: VMX: Snapshot LBR capabilities during module initialization
perf/x86/intel: Expose existence of callback support to KVM
KVM: VMX: Disable LBR virtualization if the CPU doesn't support LBR
callstacks

arch/x86/events/intel/lbr.c | 1 +
arch/x86/include/asm/perf_event.h | 1 +
arch/x86/kvm/vmx/pmu_intel.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 17 +++++++++++++----
arch/x86/kvm/vmx/vmx.h | 2 ++
5 files changed, 18 insertions(+), 5 deletions(-)


base-commit: 0c64952fec3ea01cb5b09f00134200f3e7ab40d5
--
2.44.0.278.ge034bb2e1d-goog



2024-03-07 01:14:15

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH 1/3] KVM: VMX: Snapshot LBR capabilities during module initialization

Snapshot VMX's LBR capabilities once during module initialization instead
of calling into perf every time a vCPU reconfigures its vPMU. This will
allow massaging the LBR capabilities, e.g. if the CPU doesn't support
callstacks, without having to remember to update multiple locations.

Opportunistically tag vmx_get_perf_capabilities() with __init, as it's
only called from vmx_set_cpu_caps().

Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/vmx/pmu_intel.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 9 +++++----
arch/x86/kvm/vmx/vmx.h | 2 ++
3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 12ade343a17e..be40474de6e4 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -535,7 +535,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
perf_capabilities = vcpu_get_perf_capabilities(vcpu);
if (cpuid_model_is_consistent(vcpu) &&
(perf_capabilities & PMU_CAP_LBR_FMT))
- x86_perf_get_lbr(&lbr_desc->records);
+ memcpy(&lbr_desc->records, &vmx_lbr_caps, sizeof(vmx_lbr_caps));
else
lbr_desc->records.nr = 0;

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 7a74388f9ecf..2a7cd66988a5 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -217,6 +217,8 @@ module_param(ple_window_max, uint, 0444);
int __read_mostly pt_mode = PT_MODE_SYSTEM;
module_param(pt_mode, int, S_IRUGO);

+struct x86_pmu_lbr __ro_after_init vmx_lbr_caps;
+
static DEFINE_STATIC_KEY_FALSE(vmx_l1d_should_flush);
static DEFINE_STATIC_KEY_FALSE(vmx_l1d_flush_cond);
static DEFINE_MUTEX(vmx_l1d_flush_mutex);
@@ -7844,10 +7846,9 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
vmx_update_exception_bitmap(vcpu);
}

-static u64 vmx_get_perf_capabilities(void)
+static __init u64 vmx_get_perf_capabilities(void)
{
u64 perf_cap = PMU_CAP_FW_WRITES;
- struct x86_pmu_lbr lbr;
u64 host_perf_cap = 0;

if (!enable_pmu)
@@ -7857,8 +7858,8 @@ static u64 vmx_get_perf_capabilities(void)
rdmsrl(MSR_IA32_PERF_CAPABILITIES, host_perf_cap);

if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR)) {
- x86_perf_get_lbr(&lbr);
- if (lbr.nr)
+ x86_perf_get_lbr(&vmx_lbr_caps);
+ if (vmx_lbr_caps.nr)
perf_cap |= host_perf_cap & PMU_CAP_LBR_FMT;
}

diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 65786dbe7d60..cc10df53966e 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -109,6 +109,8 @@ struct lbr_desc {
bool msr_passthrough;
};

+extern struct x86_pmu_lbr vmx_lbr_caps;
+
/*
* The nested_vmx structure is part of vcpu_vmx, and holds information we need
* for correct emulation of VMX (i.e., nested VMX) on this vcpu.
--
2.44.0.278.ge034bb2e1d-goog


2024-03-07 01:14:32

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH 2/3] perf/x86/intel: Expose existence of callback support to KVM

Add a "has_callstack" field to the x86_pmu_lbr structure used to pass
information to KVM, and set it accordingly in x86_perf_get_lbr(). KVM
will use has_callstack to avoid trying to create perf LBR events with
PERF_SAMPLE_BRANCH_CALL_STACK on CPUs that don't support callstacks.

Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/events/intel/lbr.c | 1 +
arch/x86/include/asm/perf_event.h | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 78cd5084104e..4367aa77cb8d 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1693,6 +1693,7 @@ void x86_perf_get_lbr(struct x86_pmu_lbr *lbr)
lbr->from = x86_pmu.lbr_from;
lbr->to = x86_pmu.lbr_to;
lbr->info = x86_pmu.lbr_info;
+ lbr->has_callstack = x86_pmu_has_lbr_callstack();
}
EXPORT_SYMBOL_GPL(x86_perf_get_lbr);

diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 3736b8a46c04..7f1e17250546 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -555,6 +555,7 @@ struct x86_pmu_lbr {
unsigned int from;
unsigned int to;
unsigned int info;
+ bool has_callstack;
};

extern void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap);
--
2.44.0.278.ge034bb2e1d-goog


2024-03-07 01:14:54

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH 3/3] KVM: VMX: Disable LBR virtualization if the CPU doesn't support LBR callstacks

Disable LBR virtualization if the CPU doesn't support callstacks, which
were introduced in HSW (see commit e9d7f7cd97c4 ("perf/x86/intel: Add
basic Haswell LBR call stack support"), as KVM unconditionally configures
the perf LBR event with PERF_SAMPLE_BRANCH_CALL_STACK, i.e. LBR
virtualization always fails on pre-HSW CPUs.

Simply disable LBR support on such CPUs, as it has never worked, i.e.
there is no risk of breaking an existing setup, and figuring out a way
to performantly context switch LBRs on old CPUs is not worth the effort.

Fixes: be635e34c284 ("KVM: vmx/pmu: Expose LBR_FMT in the MSR_IA32_PERF_CAPABILITIES")
Cc: Mingwei Zhang <[email protected]>
Cc: Jim Mattson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/vmx/vmx.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 2a7cd66988a5..25a7652bee7c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7859,7 +7859,15 @@ static __init u64 vmx_get_perf_capabilities(void)

if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR)) {
x86_perf_get_lbr(&vmx_lbr_caps);
- if (vmx_lbr_caps.nr)
+
+ /*
+ * KVM requires LBR callstack support, as the overhead due to
+ * context switching LBRs without said support is too high.
+ * See intel_pmu_create_guest_lbr_event() for more info.
+ */
+ if (!vmx_lbr_caps.has_callstack)
+ memset(&vmx_lbr_caps, 0, sizeof(vmx_lbr_caps));
+ else if (vmx_lbr_caps.nr)
perf_cap |= host_perf_cap & PMU_CAP_LBR_FMT;
}

--
2.44.0.278.ge034bb2e1d-goog


2024-03-18 22:50:56

by Mingwei Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/3] perf/x86/intel: Expose existence of callback support to KVM

On Wed, Mar 06, 2024, Sean Christopherson wrote:
> Add a "has_callstack" field to the x86_pmu_lbr structure used to pass
> information to KVM, and set it accordingly in x86_perf_get_lbr(). KVM
> will use has_callstack to avoid trying to create perf LBR events with
> PERF_SAMPLE_BRANCH_CALL_STACK on CPUs that don't support callstacks.
>
> Signed-off-by: Sean Christopherson <[email protected]>
Reviewed-by: Mingwei Zhang <[email protected]>
> ---
> arch/x86/events/intel/lbr.c | 1 +
> arch/x86/include/asm/perf_event.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 78cd5084104e..4367aa77cb8d 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1693,6 +1693,7 @@ void x86_perf_get_lbr(struct x86_pmu_lbr *lbr)
> lbr->from = x86_pmu.lbr_from;
> lbr->to = x86_pmu.lbr_to;
> lbr->info = x86_pmu.lbr_info;
> + lbr->has_callstack = x86_pmu_has_lbr_callstack();
> }
> EXPORT_SYMBOL_GPL(x86_perf_get_lbr);
>
> diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
> index 3736b8a46c04..7f1e17250546 100644
> --- a/arch/x86/include/asm/perf_event.h
> +++ b/arch/x86/include/asm/perf_event.h
> @@ -555,6 +555,7 @@ struct x86_pmu_lbr {
> unsigned int from;
> unsigned int to;
> unsigned int info;
> + bool has_callstack;
> };
>
> extern void perf_get_x86_pmu_capability(struct x86_pmu_capability *cap);
> --
> 2.44.0.278.ge034bb2e1d-goog
>

2024-03-18 22:51:21

by Mingwei Zhang

[permalink] [raw]
Subject: Re: [PATCH 3/3] KVM: VMX: Disable LBR virtualization if the CPU doesn't support LBR callstacks

On Wed, Mar 06, 2024, Sean Christopherson wrote:
> Disable LBR virtualization if the CPU doesn't support callstacks, which
> were introduced in HSW (see commit e9d7f7cd97c4 ("perf/x86/intel: Add
> basic Haswell LBR call stack support"), as KVM unconditionally configures
> the perf LBR event with PERF_SAMPLE_BRANCH_CALL_STACK, i.e. LBR
> virtualization always fails on pre-HSW CPUs.
>
> Simply disable LBR support on such CPUs, as it has never worked, i.e.
> there is no risk of breaking an existing setup, and figuring out a way
> to performantly context switch LBRs on old CPUs is not worth the effort.
>
> Fixes: be635e34c284 ("KVM: vmx/pmu: Expose LBR_FMT in the MSR_IA32_PERF_CAPABILITIES")
> Cc: Mingwei Zhang <[email protected]>
> Cc: Jim Mattson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Mingwei Zhang <[email protected]>
> ---
> arch/x86/kvm/vmx/vmx.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 2a7cd66988a5..25a7652bee7c 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7859,7 +7859,15 @@ static __init u64 vmx_get_perf_capabilities(void)
>
> if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR)) {
> x86_perf_get_lbr(&vmx_lbr_caps);
> - if (vmx_lbr_caps.nr)
> +
> + /*
> + * KVM requires LBR callstack support, as the overhead due to
> + * context switching LBRs without said support is too high.
> + * See intel_pmu_create_guest_lbr_event() for more info.
> + */
> + if (!vmx_lbr_caps.has_callstack)
> + memset(&vmx_lbr_caps, 0, sizeof(vmx_lbr_caps));
> + else if (vmx_lbr_caps.nr)
> perf_cap |= host_perf_cap & PMU_CAP_LBR_FMT;
> }
>
> --
> 2.44.0.278.ge034bb2e1d-goog
>

2024-03-18 22:51:45

by Mingwei Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/3] KVM: VMX: Snapshot LBR capabilities during module initialization

On Wed, Mar 06, 2024, Sean Christopherson wrote:
> Snapshot VMX's LBR capabilities once during module initialization instead
> of calling into perf every time a vCPU reconfigures its vPMU. This will
> allow massaging the LBR capabilities, e.g. if the CPU doesn't support
> callstacks, without having to remember to update multiple locations.
>
> Opportunistically tag vmx_get_perf_capabilities() with __init, as it's
> only called from vmx_set_cpu_caps().
>
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
Reviewed-by: Mingwei Zhang <[email protected]>

> arch/x86/kvm/vmx/pmu_intel.c | 2 +-
> arch/x86/kvm/vmx/vmx.c | 9 +++++----
> arch/x86/kvm/vmx/vmx.h | 2 ++
> 3 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 12ade343a17e..be40474de6e4 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -535,7 +535,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
> perf_capabilities = vcpu_get_perf_capabilities(vcpu);
> if (cpuid_model_is_consistent(vcpu) &&
> (perf_capabilities & PMU_CAP_LBR_FMT))
> - x86_perf_get_lbr(&lbr_desc->records);
> + memcpy(&lbr_desc->records, &vmx_lbr_caps, sizeof(vmx_lbr_caps));
> else
> lbr_desc->records.nr = 0;
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 7a74388f9ecf..2a7cd66988a5 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -217,6 +217,8 @@ module_param(ple_window_max, uint, 0444);
> int __read_mostly pt_mode = PT_MODE_SYSTEM;
> module_param(pt_mode, int, S_IRUGO);
>
> +struct x86_pmu_lbr __ro_after_init vmx_lbr_caps;
> +
> static DEFINE_STATIC_KEY_FALSE(vmx_l1d_should_flush);
> static DEFINE_STATIC_KEY_FALSE(vmx_l1d_flush_cond);
> static DEFINE_MUTEX(vmx_l1d_flush_mutex);
> @@ -7844,10 +7846,9 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> vmx_update_exception_bitmap(vcpu);
> }
>
> -static u64 vmx_get_perf_capabilities(void)
> +static __init u64 vmx_get_perf_capabilities(void)
> {
> u64 perf_cap = PMU_CAP_FW_WRITES;
> - struct x86_pmu_lbr lbr;
> u64 host_perf_cap = 0;
>
> if (!enable_pmu)
> @@ -7857,8 +7858,8 @@ static u64 vmx_get_perf_capabilities(void)
> rdmsrl(MSR_IA32_PERF_CAPABILITIES, host_perf_cap);
>
> if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR)) {
> - x86_perf_get_lbr(&lbr);
> - if (lbr.nr)
> + x86_perf_get_lbr(&vmx_lbr_caps);
> + if (vmx_lbr_caps.nr)
> perf_cap |= host_perf_cap & PMU_CAP_LBR_FMT;
> }
>
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 65786dbe7d60..cc10df53966e 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -109,6 +109,8 @@ struct lbr_desc {
> bool msr_passthrough;
> };
>
> +extern struct x86_pmu_lbr vmx_lbr_caps;
> +
> /*
> * The nested_vmx structure is part of vcpu_vmx, and holds information we need
> * for correct emulation of VMX (i.e., nested VMX) on this vcpu.
> --
> 2.44.0.278.ge034bb2e1d-goog
>

2024-04-09 02:03:11

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH 0/3] KVM: VMX: Disable LBRs if CPU doesn't have callstacks

On Wed, 06 Mar 2024 17:13:41 -0800, Sean Christopherson wrote:
> Disable LBR virtualization if the CPU (or I guess perf) doesn't support
> LBR callstacks, as KVM unconditionally creates the associated perf LBR
> event with PERF_SAMPLE_BRANCH_CALL_STACK. That results in perf rejecting
> the event, and cause LBR virtualization to silently fail.
>
> This was detected by running vmx_pmu_caps_test on older hardware. I didn't
> tag it for stable because I can't imagine anyone is trying to use KVM's LBR
> virtualization on pre-HSW.
>
> [...]

Applied to kvm-x86 fixes, as I can't imagine anyone objecting to patch 2.

[1/3] KVM: VMX: Snapshot LBR capabilities during module initialization
https://github.com/kvm-x86/linux/commit/2a94a2761236
[2/3] perf/x86/intel: Expose existence of callback support to KVM
https://github.com/kvm-x86/linux/commit/0c0241c12332
[3/3] KVM: VMX: Disable LBR virtualization if the CPU doesn't support LBR callstacks
https://github.com/kvm-x86/linux/commit/0eb2416c8111

--
https://github.com/kvm-x86/linux/tree/next