KVM: VMX: Always treat MSR_IA32_PERF_CAPABILITIES as a valid PMU MSR
PATCH 1/1 KVM: VMX: Always treat MSR_IA32_PERF_CAPABILITIES as a valid PMU MSR
Unconditionally return true when querying the validity of MSR_IA32_PERF_CAPABILITIES so as to defer the validity check to intel_pmu_{get,set}_msr(), which can properly give the MSR a pass when the access is initiated from host userspace. The MSR is emulated so there is no underlying hardware dependency to worry about.
---
KVM selftests are completely hosed for me, everything fails on KVM_GET_MSRS.
arch/x86/kvm/vmx/pmu_intel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kvm/vmx/pmu_intel.c +1 -1
@@ -181,7 +181,7 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr) ret = pmu->version >1 ;break ;case MSR_IA32_PERF_CAPABILITIES : - ret =guest_cpuid_has (vcpu,X86_FEATURE_PDCM ); + ret =1 ;break ;default : ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
Signature · 1 line
2.26.0
Quoted 4 lines · Sean Christopherson
Unconditionally return true when querying the validity of
MSR_IA32_PERF_CAPABILITIESso as to defer the validity check to intel_pmu_{get,set}_msr(), which can properly give the MSR a pass when the access is initiated from host userspace.
Regardless of the MSR is emulated or not, is it a really good assumption that the guest cpuids are not properly ready when we do initialization from host userspace ?
Quoted 9 lines
The MSR is emulated so
there is no underlying hardware dependency to worry about.Fixes 27461da31089a ("KVM: x86/pmu: Support full width counting")Cc Like Xu <[email protected]>Signed-off-by Sean Christopherson <[email protected]>---
KVM selftests are completely hosed for me, everything fails on
KVM_GET_MSRS.
At least I tried "make --silent -C tools/testing/selftests/kvm run_tests" and how do I reproduce the "everything fails" for this issue ?
Thanks,
Like Xu
Quoted 17 lines
arch/x86/kvm/vmx/pmu_intel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/x86/kvm/vmx/pmu_intel.c +1
-1@@ -181,7 +181,7 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr) ret = pmu->version >1 ;break ;case MSR_IA32_PERF_CAPABILITIES : - ret =guest_cpuid_has (vcpu,X86_FEATURE_PDCM ); + ret =1 ;break ;default : ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
Quoted 9 lines · Like
Quoted 4 lines · Sean Christopherson
Unconditionally return true when querying the validity of
MSR_IA32_PERF_CAPABILITIESso as to defer the validity check to intel_pmu_{get,set}_msr(), which can properly give the MSR a pass when the access is initiated from host userspace.Regardless of the MSR is emulated or not, is it a really good assumption that the guest cpuids are not properly ready when we do initialization from host userspace
?
I don't know if I would call it a "good assumption" so much as a "necessary assumption". KVM_{GET,SET}_MSRS are allowed, and must function correctly, if they're called prior to KVM_SET_CPUID{2}.
Quoted 11 lines
Quoted 9 lines
The MSR is emulated so
there is no underlying hardware dependency to worry about.Fixes 27461da31089a ("KVM: x86/pmu: Support full width counting")Cc Like Xu <[email protected]>Signed-off-by Sean Christopherson <[email protected]>---
KVM selftests are completely hosed for me, everything fails on
KVM_GET_MSRS.At least I tried "make --silent -C
tools/testing/selftests/kvmrun_tests" and how do I reproduce the "everything fails" for this issue ?
Hmm, I did nothing more than run the tests on a HSW system.
Quoted 20 lines
Thanks,
Like XuQuoted 17 lines
arch/x86/kvm/vmx/pmu_intel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/x86/kvm/vmx/pmu_intel.c +1
-1@@ -181,7 +181,7 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr) ret = pmu->version >1 ;break ;case MSR_IA32_PERF_CAPABILITIES : - ret =guest_cpuid_has (vcpu,X86_FEATURE_PDCM ); + ret =1 ;break ;default : ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
Paolo Bonzini
maintainerQuoted 6 lines · Sean Christopherson
Quoted 5 lines · Like
Quoted 4 lines · Sean Christopherson
Unconditionally return true when querying the validity of
MSR_IA32_PERF_CAPABILITIESso as to defer the validity check to intel_pmu_{get,set}_msr(), which can properly give the MSR a pass when the access is initiated from host userspace.
Quoted 8 lines
Quoted 4 lines
Regardless of the MSR is emulated or not, is it a really good assumption that the guest cpuids are not properly ready when we do initialization from host userspace
?I don't know if I would call it a "good assumption" so much as a "necessary assumption". KVM_{GET,SET}_MSRS are allowed, and must function correctly, if they're called prior to KVM_SET_CPUID{2}.
Generally speaking this is not the case for the PMU; get_gp_pmc for example depends on pmu->nr_arch_gp_counters which is initialized based on CPUID leaf 0xA.
The assumption that this patch fixes is that you can blindly take the output of KVM_GET_MSR_INDEX_LIST and pass it to KVM_{GET,SET}_MSRS.
Paolo
Quoted 17 lines · Paolo Bonzini
Quoted 6 lines · Sean Christopherson
Quoted 5 lines · Like
Quoted 4 lines · Sean Christopherson
Unconditionally return true when querying the validity of
MSR_IA32_PERF_CAPABILITIESso as to defer the validity check to intel_pmu_{get,set}_msr(), which can properly give the MSR a pass when the access is initiated from host userspace.Quoted 8 lines
Quoted 4 lines
Regardless of the MSR is emulated or not, is it a really good assumption that the guest cpuids are not properly ready when we do initialization from host userspace
?I don't know if I would call it a "good assumption" so much as a "necessary assumption". KVM_{GET,SET}_MSRS are allowed, and must function correctly, if they're called prior to KVM_SET_CPUID{2}.
Quoted 6 lines
Generally speaking this is not the case for the PMU;
get_gp_pmcfor example depends onpmu->nr_arch_gp_counterswhich is initialized based on CPUID leaf0xA.The assumption that this patch fixes is that you can blindly take the output of
KVM_GET_MSR_INDEX_LISTand pass it to KVM_{GET,SET}_MSRS.
Is that an assumption or an invariant?
Paolo Bonzini
maintainerQuoted 11 lines · Jim Mattson
Quoted 9 lines
Quoted 3 lines
I don't know if I would call it a "good assumption" so much as a "necessary assumption". KVM_{GET,SET}_MSRS are allowed, and must function correctly, if they're called prior to KVM_SET_CPUID{2}.
Generally speaking this is not the case for the PMU;
get_gp_pmcfor example depends onpmu->nr_arch_gp_counterswhich is initialized based on CPUID leaf0xA.The assumption that this patch fixes is that you can blindly take the output of
KVM_GET_MSR_INDEX_LISTand pass it to KVM_{GET,SET}_MSRS.Is that an assumption or an invariant?
Both, I guess (a valid assumption for userspace, an invariant to be respected for the kernel code).
The part where we don't fare to well, is that a bunch of MSRs that need save/restore are _not_ included in KVM_GET_MSR_INDEX_LIST (and the PMU is the biggest if not the only offender there).
Paolo