On Tue, Aug 02, 2022, Like Xu wrote:
> On 28/7/2022 7:34 am, Sean Christopherson wrote:
> > -bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu)
> > -{
> > - struct x86_pmu_lbr *lbr = vcpu_to_lbr_records(vcpu);
> > -
> > - return lbr->nr && (vcpu_get_perf_capabilities(vcpu) & PMU_CAP_LBR_FMT);
> > -}
> > -
> > static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> > {
> > struct x86_pmu_lbr *records = vcpu_to_lbr_records(vcpu);
> > @@ -590,7 +583,9 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
> > bitmap_set(pmu->all_valid_pmc_idx,
> > INTEL_PMC_MAX_GENERIC, pmu->nr_arch_fixed_counters);
> > - if (cpuid_model_is_consistent(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);
>
> As one of evil source to add CPUID walk in the critical path:
>
> The x86_perf_get_lbr() is one of the perf interfaces, KVM cannot always trust
> that the number of returned lbr_desc->records.nr is always > 0, and if not,
> we have to tweak perf_capabilities inside KVM which violates user input again.
>
> Do you have more inputs to address this issue ?
First, drop the unnecessary stub and return value from x86_perf_get_lbr(). KVM
selects PERF_EVENTS, so the stub and thus error path can't be hit. I'll add
patches to the series to do this.
Second, check the number of perf LBRs in vmx_get_perf_capabilities() and advertise
PMU_CAP_LBR_FMT iff perf fully supports LBRs.
---
From: Sean Christopherson <seanjc@xxxxxxxxxx>
Date: Tue, 2 Aug 2022 07:45:33 -0700
Subject: [PATCH] KVM: VMX: Advertise PMU LBRs if and only if perf supports
LBRs
Advertise LBR support to userspace via MSR_IA32_PERF_CAPABILITIES if and
only if perf fully supports LBRs. Perf may disable LBRs (by zeroing the
number of LBRs) even on platforms the allegedly support LBRs, e.g. if
probing any LBR MSRs during setup fails.
Fixes: be635e34c284 ("KVM: vmx/pmu: Expose LBR_FMT in the MSR_IA32_PERF_CAPABILITIES")
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
---
arch/x86/kvm/vmx/capabilities.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index c5e5dfef69c7..d2fdaf888d33 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -404,6 +404,7 @@ static inline bool vmx_pebs_supported(void)
static inline 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)
@@ -412,7 +413,9 @@ static inline u64 vmx_get_perf_capabilities(void)
if (boot_cpu_has(X86_FEATURE_PDCM))
rdmsrl(MSR_IA32_PERF_CAPABILITIES, host_perf_cap);
- perf_cap |= host_perf_cap & PMU_CAP_LBR_FMT;
+ x86_perf_get_lbr(&lbr);
+ if (lbr.nr)
+ perf_cap |= host_perf_cap & PMU_CAP_LBR_FMT;
if (vmx_pebs_supported()) {
perf_cap |= host_perf_cap & PERF_CAP_PEBS_MASK;
base-commit: 1f011a0755c2135b035cdee3b54e3adc426ec95c
--
