public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Jim Mattson <jmattson@google.com>,
	mlevitsk@redhat.com
Subject: Re: [PATCH 5/5] KVM: x86: Defer runtime updates of dynamic CPUID bits until CPUID emulation
Date: Mon, 1 Dec 2025 11:36:15 -0800	[thread overview]
Message-ID: <aS3uLyKFawM8V-ed@google.com> (raw)
In-Reply-To: <aS26gBXQnHjgSDW5@google.com>

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@google.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;


  reply	other threads:[~2025-12-01 19:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-11  1:32 [PATCH 0/5] KVM: x86: Address xstate_required_size() perf regression Sean Christopherson
2024-12-11  1:32 ` [PATCH 1/5] KVM: x86: Cache CPUID.0xD XSTATE offsets+sizes during module init Sean Christopherson
2024-12-11  1:32 ` [PATCH 2/5] KVM: x86: Use for-loop to iterate over XSTATE size entries Sean Christopherson
2024-12-11  1:33 ` [PATCH 3/5] KVM: x86: Apply TSX_CTRL_CPUID_CLEAR if and only if the vCPU has RTM or HLE Sean Christopherson
2024-12-13 14:39   ` Jim Mattson
2024-12-11  1:33 ` [PATCH 4/5] KVM: x86: Query X86_FEATURE_MWAIT iff userspace owns the CPUID feature bit Sean Christopherson
2024-12-11  1:33 ` [PATCH 5/5] KVM: x86: Defer runtime updates of dynamic CPUID bits until CPUID emulation Sean Christopherson
2025-11-28 11:32   ` Igor Mammedov
2025-12-01 15:55     ` Sean Christopherson
2025-12-01 19:36       ` Sean Christopherson [this message]
2024-12-11 16:42 ` [PATCH 0/5] KVM: x86: Address xstate_required_size() perf regression Sean Christopherson
2024-12-13 18:58 ` Paolo Bonzini
2024-12-16 20:04 ` Jim Mattson
2024-12-16 23:16   ` Sean Christopherson
2025-02-15  0:50 ` Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aS3uLyKFawM8V-ed@google.com \
    --to=seanjc@google.com \
    --cc=imammedo@redhat.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox