On Wed, Nov 27, 2024 at 05:34:17PM -0800, Sean Christopherson wrote: >Switch all queries (except XSAVES) of guest features from guest CPUID to >guest capabilities, i.e. replace all calls to guest_cpuid_has() with calls >to guest_cpu_cap_has(). > >Keep guest_cpuid_has() around for XSAVES, but subsume its helper >guest_cpuid_get_register() and add a compile-time assertion to prevent >using guest_cpuid_has() for any other feature. Add yet another comment >for XSAVE to explain why KVM is allowed to query its raw guest CPUID. > >Opportunistically drop the unused guest_cpuid_clear(), as there should be >no circumstance in which KVM needs to _clear_ a guest CPUID feature now >that everything is tracked via cpu_caps. E.g. KVM may need to _change_ >a feature to emulate dynamic CPUID flags, but KVM should never need to >clear a feature in guest CPUID to prevent it from being used by the guest. > >Delete the last remnants of the governed features framework, as the lone >holdout was vmx_adjust_secondary_exec_control()'s divergent behavior for >governed vs. ungoverned features. > >Note, replacing guest_cpuid_has() checks with guest_cpu_cap_has() when >computing reserved CR4 bits is a nop when viewed as a whole, as KVM's >capabilities are already incorporated into the calculation, i.e. if a >feature is present in guest CPUID but unsupported by KVM, its CR4 bit >was already being marked as reserved, checking guest_cpu_cap_has() simply >double-stamps that it's a reserved bit. ... > >Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx> >Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> >--- > arch/x86/kvm/cpuid.c | 4 +- > arch/x86/kvm/cpuid.h | 76 ++++++++++++-------------------- > arch/x86/kvm/governed_features.h | 22 --------- > arch/x86/kvm/hyperv.c | 2 +- > arch/x86/kvm/lapic.c | 4 +- > arch/x86/kvm/smm.c | 10 ++--- > arch/x86/kvm/svm/pmu.c | 8 ++-- > arch/x86/kvm/svm/sev.c | 4 +- > arch/x86/kvm/svm/svm.c | 20 ++++----- > arch/x86/kvm/vmx/hyperv.h | 2 +- > arch/x86/kvm/vmx/nested.c | 12 ++--- > arch/x86/kvm/vmx/pmu_intel.c | 4 +- > arch/x86/kvm/vmx/sgx.c | 14 +++--- > arch/x86/kvm/vmx/vmx.c | 47 +++++++++----------- > arch/x86/kvm/x86.c | 66 +++++++++++++-------------- > 15 files changed, 124 insertions(+), 171 deletions(-) > delete mode 100644 arch/x86/kvm/governed_features.h > >diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >index d3c3e1327ca1..8d088a888a0d 100644 >--- a/arch/x86/kvm/cpuid.c >+++ b/arch/x86/kvm/cpuid.c >@@ -416,7 +416,7 @@ void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > * and can install smaller shadow pages if the host lacks 1GiB support. > */ > allow_gbpages = tdp_enabled ? boot_cpu_has(X86_FEATURE_GBPAGES) : >- guest_cpuid_has(vcpu, X86_FEATURE_GBPAGES); >+ guest_cpu_cap_has(vcpu, X86_FEATURE_GBPAGES); > guest_cpu_cap_change(vcpu, X86_FEATURE_GBPAGES, allow_gbpages); > > best = kvm_find_cpuid_entry(vcpu, 1); >@@ -441,7 +441,7 @@ void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu) > > #define __kvm_cpu_cap_has(UNUSED_, f) kvm_cpu_cap_has(f) > vcpu->arch.cr4_guest_rsvd_bits = __cr4_reserved_bits(__kvm_cpu_cap_has, UNUSED_) | >- __cr4_reserved_bits(guest_cpuid_has, vcpu); >+ __cr4_reserved_bits(guest_cpu_cap_has, vcpu); So, actually, __cr4_reserved_bits(__kvm_cpu_cap_has, UNUSED_) can be dropped. Is there any reason to keep it? It makes perfect sense to just look up the guest cpu_caps given it already takes KVM caps into consideration. > #undef __kvm_cpu_cap_has > > kvm_hv_set_cpuid(vcpu, kvm_cpuid_has_hyperv(vcpu));
