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

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

[Sean Christopherson]

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

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

[Sean Christopherson]

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 <seanjc@google.com>
---
 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

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

[Sean Christopherson]

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 <seanjc@google.com>
---
 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

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

[Sean Christopherson]

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 <mizhang@google.com>
Cc: Jim Mattson <jmattson@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 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

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

[Mingwei Zhang]

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 <seanjc@google.com>
> ---
Reviewed-by: Mingwei Zhang <mizhang@google.com>

>  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
> 

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

[Mingwei Zhang]

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 <seanjc@google.com>
Reviewed-by: Mingwei Zhang <mizhang@google.com>
> ---
>  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
> 

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

[Mingwei Zhang]

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 <mizhang@google.com>
> Cc: Jim Mattson <jmattson@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Tested-by: Mingwei Zhang <mizhang@google.com>
> ---
>  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
> 

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

[Sean Christopherson]

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