linux-kernel - Re: [PATCH 5/5] KVM: x86: Defer runtime updates of dynamic CPUID bits until CPUID emulation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aS3uLyKFawM8V-ed@google.com>
Date: Mon, 1 Dec 2025 11:36:15 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Igor Mammedov <imammedo@...hat.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Jim Mattson <jmattson@...gle.com>, mlevitsk@...hat.com
Subject: Re: [PATCH 5/5] KVM: x86: Defer runtime updates of dynamic CPUID bits
 until CPUID emulation

On Mon, Dec 01, 2025, Sean Christopherson wrote:
> On Fri, Nov 28, 2025, Igor Mammedov wrote:
> > On Tue, 10 Dec 2024 17:33:02 -0800
> > Sean Christopherson <seanjc@...gle.com> wrote:
> > 
> > Sean,
> > 
> > this patch broke vCPU hotplug (still broken with current master),
> > after repeated plug/unplug of the same vCPU in a loop, QEMU exits
> > due to error in vcpu initialization:
> > 
> >     r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);                         
> >     if (r) {                                                                     
> >         goto fail;                                                               
> >     }
> > 
> > Reproducer (host in question is Haswell but it's been seen on other hosts as well):
> > for it to trigger the issue it must be Q35 machine with UEFI firmware
> > (the rest doesn't seem to matter)
> 
> Gah, sorry.  I managed to handle KVM_GET_CPUID2, so I suspect I thought the update
> in kvm_cpuid_check_equal() would take care of things, but that only operates on
> the new entries.
> 
> Can you test the below?  In the meantime, I'll see if I can enhance the CPUID
> selftest to detect the issue and verify the fix.
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index d563a948318b..dd6534419074 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -509,11 +509,18 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,
>         u32 vcpu_caps[NR_KVM_CPU_CAPS];
>         int r;
>  
> +       /*
> +        * Apply pending runtime CPUID updates to the current CPUID entries to
> +        * avoid false positives due mismatches on KVM-owned feature flags.
> +        */
> +       if (vcpu->arch.cpuid_dynamic_bits_dirty)
> +               kvm_update_cpuid_runtime(vcpu);
> +
>         /*
>          * Swap the existing (old) entries with the incoming (new) entries in
>          * order to massage the new entries, e.g. to account for dynamic bits
> -        * that KVM controls, without clobbering the current guest CPUID, which
> -        * KVM needs to preserve in order to unwind on failure.
> +        * that KVM controls, without losing the current guest CPUID, which KVM
> +        * needs to preserve in order to unwind on failure.
>          *
>          * Similarly, save the vCPU's current cpu_caps so that the capabilities
>          * can be updated alongside the CPUID entries when performing runtime

Verified the bug and the fix with the below selftest change.  I'll post proper
patches after full testing, and cross my fingers it fixes the hotplug issue :-)

diff --git a/tools/testing/selftests/kvm/x86/cpuid_test.c b/tools/testing/selftests/kvm/x86/cpuid_test.c
index 7b3fda6842bc..f9ed14996977 100644
--- a/tools/testing/selftests/kvm/x86/cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86/cpuid_test.c
@@ -155,6 +155,7 @@ struct kvm_cpuid2 *vcpu_alloc_cpuid(struct kvm_vm *vm, vm_vaddr_t *p_gva, struct
 static void set_cpuid_after_run(struct kvm_vcpu *vcpu)
 {
        struct kvm_cpuid_entry2 *ent;
+       struct kvm_sregs sregs;
        int rc;
        u32 eax, ebx, x;
 
@@ -162,6 +163,20 @@ static void set_cpuid_after_run(struct kvm_vcpu *vcpu)
        rc = __vcpu_set_cpuid(vcpu);
        TEST_ASSERT(!rc, "Setting unmodified CPUID after KVM_RUN failed: %d", rc);
 
+       /*
+        * Toggle CR4 bits that affect dynamic CPUID feature flags to verify
+        * setting unmodified CPUID succeeds with runtime CPUID updates.
+        */
+       vcpu_sregs_get(vcpu, &sregs);
+       if (kvm_cpu_has(X86_FEATURE_XSAVE))
+               sregs.cr4 ^= X86_CR4_OSXSAVE;
+       if (kvm_cpu_has(X86_FEATURE_PKU))
+               sregs.cr4 ^= X86_CR4_PKE;
+       vcpu_sregs_set(vcpu, &sregs);
+
+       rc = __vcpu_set_cpuid(vcpu);
+       TEST_ASSERT(!rc, "Setting unmodified CPUID after KVM_RUN failed: %d", rc);
+
        /* Changing CPU features is forbidden */
        ent = vcpu_get_cpuid_entry(vcpu, 0x7);
        ebx = ent->ebx;