On Thu, Jul 04, 2024, Maxim Levitsky wrote: > On Fri, 2024-05-17 at 10:39 -0700, Sean Christopherson wrote: > > - cpuid_entry_change(best, X86_FEATURE_OSPKE, > > - kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE)); > > + kvm_update_feature_runtime(vcpu, best, X86_FEATURE_OSPKE, > > + kvm_is_cr4_bit_set(vcpu, X86_CR4_PKE)); > > + > > > > best = kvm_find_cpuid_entry_index(vcpu, 0xD, 0); > > if (best) > > > I am not 100% sure that we need to do this. > > Runtime cpuid changes are a hack that Intel did back then, due to various > reasons, These changes don't really change the feature set that CPU supports, > but merly as you like to say 'massage' the output of the CPUID instruction to > make the unmodified OS happy usually. > > Thus it feels to me that CPU caps should not include the dynamic features, > and neither KVM should use the value of these as a source for truth, but > rather the underlying source of the truth (e.g CR4). > > But if you insist, I don't really have a very strong reason to object this. FWIW, I think I agree that CR4 should be the source of truth, but it's largely a moot point because KVM doesn't actually check OSXSAVE or OSPKE, as KVM never emulates the relevant instructions. So for those, it's indeed not strictly necessary. Unfortunately, KVM has established ABI for checking X86_FEATURE_MWAIT when "emulating" MONITOR and MWAIT, i.e. KVM can't use vcpu->arch.ia32_misc_enable_msr as the source of truth. So for MWAIT, KVM does need to update CPU caps (or carry even more awful MWAIT code), at which point extending the behavior to the CR4 features (and to X86_FEATURE_APIC) is practically free.
