Paolo Bonzini <pbonzini@xxxxxxxxxx> writes: > On 17/06/20 13:38, Vitaly Kuznetsov wrote: >> >> For KVM_GET_MSR_INDEX_LIST, the promise is "guest msrs that are >> supported" and I'm not exactly sure what this means. Personally, I see >> no point in returning MSRs which can't be read with KVM_GET_MSRS (as >> this also means the guest can't read them) and KVM selftests seem to >> rely on that (vcpu_save_state()) but this is not a documented feature. > > Yes, this is intended. KVM_GET_MSR_INDEX_LIST is not the full list of > supported MSRs or KVM_GET_MSRS (especially PMU MSRs are missing) but it > certainly should be a sufficient condition for KVM_GET_MSRS support. > > In this case your patch is sort-of correct because AMD machines won't > have X86_FEATURE_PDCM. However, even in that case there are two things > we can do that are better: > > 1) force-set X86_FEATURE_PDCM in vmx_set_cpu_caps instead of having it > in kvm_set_cpu_caps. The latter is incorrect because if AMD for > whatever reason added it we'd lack the support. This would be basically > a refined version of your patch. > > 2) emulate the MSR on AMD too (returning zero) if somebody for whatever > reason enables PDCM in there too: this would include returning it in > KVM_GET_FEATURE_MSR_INDEX_LIST, and using kvm_get_msr_feature to set a > default value in kvm_pmu_refresh. The feature bit then would be > force-set in kvm_set_cpu_caps. This would be nicer since we have the > value in vcpu->arch already instead of struct vcpu_vmx. Let's try the hard way :-) I'll send v2 implementing 2) (hope I got the idea right), thanks! -- Vitaly
