On Fri, Dec 13, 2024, Chao Gao wrote:
> On Wed, Nov 27, 2024 at 05:34:17PM -0800, Sean Christopherson wrote:
> >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.
Hmm, good point. I agree that that keeping the __kvm_cpu_cap_has() checks is
unnecessary, though I'm tempted to turn it into a WARN. E.g. to guard against
stuffing a feature into cpu_caps without thinking through the implications.
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index edef30359c19..3cbf384aeb7a 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -460,9 +460,16 @@ void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
kvm_pmu_refresh(vcpu);
+ vcpu->arch.cr4_guest_rsvd_bits = __cr4_reserved_bits(guest_cpu_cap_has, vcpu);
+ /*
+ * KVM's capabilities are incorporated into the vCPU's capabilities,
+ * and letting the guest to use a CR4-based feature that KVM doesn't
+ * support isn't allowed as KVM either needs to explicitly emulate the
+ * feature or set the CR4 bit in hardware.
+ */
#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_cpu_cap_has, vcpu);
+ WARN_ON_ONCE(~vcpu->arch.cr4_guest_rsvd_bits &
+ __cr4_reserved_bits(__kvm_cpu_cap_has, UNUSED_));
#undef __kvm_cpu_cap_has
kvm_hv_set_cpuid(vcpu, kvm_cpuid_has_hyperv(vcpu));
