KVM: x86: Use actual kvm_cpuid.base for clearing KVM_FEATURE_PV_UNHALT
authorVitaly Kuznetsov <vkuznets@redhat.com>
Wed, 28 Feb 2024 10:18:36 +0000 (11:18 +0100)
committerSean Christopherson <seanjc@google.com>
Wed, 6 Mar 2024 17:50:15 +0000 (09:50 -0800)
Commit ee3a5f9e3d9b ("KVM: x86: Do runtime CPUID update before updating
vcpu->arch.cpuid_entries") moved tweaking of the supplied CPUID
data earlier in kvm_set_cpuid() but __kvm_update_cpuid_runtime() actually
uses 'vcpu->arch.kvm_cpuid' (though __kvm_find_kvm_cpuid_features()) which
gets set later in kvm_set_cpuid(). In some cases, e.g. when kvm_set_cpuid()
is called for the first time and 'vcpu->arch.kvm_cpuid' is clear,
__kvm_find_kvm_cpuid_features() fails to find KVM PV feature entry and the
logic which clears KVM_FEATURE_PV_UNHALT after enabling
KVM_X86_DISABLE_EXITS_HLT does not work.

The logic, introduced by the commit ee3a5f9e3d9b ("KVM: x86: Do runtime
CPUID update before updating vcpu->arch.cpuid_entries") must stay: the
supplied CPUID data is tweaked by KVM first (__kvm_update_cpuid_runtime())
and checked later (kvm_check_cpuid()) and the actual data
(vcpu->arch.cpuid_*, vcpu->arch.kvm_cpuid, vcpu->arch.xen.cpuid,..) is only
updated on success.

Switch to searching for KVM_SIGNATURE in the supplied CPUID data to
discover KVM PV feature entry instead of using stale 'vcpu->arch.kvm_cpuid'.

While on it, drop pointless "&& (best->eax & (1 << KVM_FEATURE_PV_UNHALT)"
check when clearing KVM_FEATURE_PV_UNHALT bit.

Fixes: ee3a5f9e3d9b ("KVM: x86: Do runtime CPUID update before updating vcpu->arch.cpuid_entries")
Reported-and-tested-by: Li RongQing <lirongqing@baidu.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Link: https://lore.kernel.org/r/20240228101837.93642-3-vkuznets@redhat.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
arch/x86/kvm/cpuid.c

index 0e1e3e5df6dd43786bc3b4f226e5e4bfc749fb1b..bfc0bfcb2bc60dd2860fa4617f3dc92dea1f3a97 100644 (file)
@@ -224,22 +224,22 @@ static struct kvm_hypervisor_cpuid kvm_get_hypervisor_cpuid(struct kvm_vcpu *vcp
                                          vcpu->arch.cpuid_nent, sig);
 }
 
-static struct kvm_cpuid_entry2 *__kvm_find_kvm_cpuid_features(struct kvm_vcpu *vcpu,
-                                             struct kvm_cpuid_entry2 *entries, int nent)
+static struct kvm_cpuid_entry2 *__kvm_find_kvm_cpuid_features(struct kvm_cpuid_entry2 *entries,
+                                                             int nent, u32 kvm_cpuid_base)
 {
-       u32 base = vcpu->arch.kvm_cpuid.base;
-
-       if (!base)
-               return NULL;
-
-       return cpuid_entry2_find(entries, nent, base | KVM_CPUID_FEATURES,
+       return cpuid_entry2_find(entries, nent, kvm_cpuid_base | KVM_CPUID_FEATURES,
                                 KVM_CPUID_INDEX_NOT_SIGNIFICANT);
 }
 
 static struct kvm_cpuid_entry2 *kvm_find_kvm_cpuid_features(struct kvm_vcpu *vcpu)
 {
-       return __kvm_find_kvm_cpuid_features(vcpu, vcpu->arch.cpuid_entries,
-                                            vcpu->arch.cpuid_nent);
+       u32 base = vcpu->arch.kvm_cpuid.base;
+
+       if (!base)
+               return NULL;
+
+       return __kvm_find_kvm_cpuid_features(vcpu->arch.cpuid_entries,
+                                            vcpu->arch.cpuid_nent, base);
 }
 
 void kvm_update_pv_runtime(struct kvm_vcpu *vcpu)
@@ -273,6 +273,7 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e
                                       int nent)
 {
        struct kvm_cpuid_entry2 *best;
+       struct kvm_hypervisor_cpuid kvm_cpuid;
 
        best = cpuid_entry2_find(entries, nent, 1, KVM_CPUID_INDEX_NOT_SIGNIFICANT);
        if (best) {
@@ -299,10 +300,12 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e
                     cpuid_entry_has(best, X86_FEATURE_XSAVEC)))
                best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
 
-       best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent);
-       if (kvm_hlt_in_guest(vcpu->kvm) && best &&
-               (best->eax & (1 << KVM_FEATURE_PV_UNHALT)))
-               best->eax &= ~(1 << KVM_FEATURE_PV_UNHALT);
+       kvm_cpuid = __kvm_get_hypervisor_cpuid(entries, nent, KVM_SIGNATURE);
+       if (kvm_cpuid.base) {
+               best = __kvm_find_kvm_cpuid_features(entries, nent, kvm_cpuid.base);
+               if (kvm_hlt_in_guest(vcpu->kvm) && best)
+                       best->eax &= ~(1 << KVM_FEATURE_PV_UNHALT);
+       }
 
        if (!kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT)) {
                best = cpuid_entry2_find(entries, nent, 0x1, KVM_CPUID_INDEX_NOT_SIGNIFICANT);