Re: [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation

[Dmytro Maluka]

On Mon, Jul 20, 2026 at 03:17:39PM -0700, Sean Christopherson wrote:
> On Fri, Jul 17, 2026, Kai Huang wrote:
> > On Fri, 2026-07-17 at 18:20 +0200, Dmytro Maluka wrote:
> > > I guess we could track the used vcpu_ids separately in another xarray
> > > (or a bitmap) which could be checked and updated by the early
> > > "duplicated vcpu_id check" before the first unlock of kvm->lock. But do
> > > we want to pay the memory price for that?
> 
> I'm comfortable paying the cost for a bitmap.  If a system can actually support
> thousands of vCPU IDs, then burning a few hundred bytes per VM is all but
> guaranteed to be a non-issue.  And for smaller systems, e.g. on arm64 where a
> VM can have at most 512 vCPU IDs, the bitmap costs a measely 64 bytes.

I was just a bit concerned that, for example, on my laptop, even though
there is only a dozen of physical CPUs, there is still
CONFIG_KVM_MAX_NR_VCPUS=4096 in my Debian's default kernel config, and
KVM_VCPU_ID_RATIO is still hardcoded to 4, so the bitmap size would be
4096 * 4 / 8 = 2KB which just feels "disproportionate" for such a corner
case as this sanity check.

But yeah, that doesn't seem to be a real problem (it's not like we
routinely run thousands of VMs on such systems).

> And if the wastefulness of a persistent bitmap is a concern, we could easly use
> an xarray to track only "pending" vCPUs, so that the steady state cost is ~zero.
> Actually, given how easy that is (famous last words), unless removing from an
> xarray doesn't free memory soon-ish, I'd say go straight to a semi-permanent
> xarray to avoid bikeshedding over the cost of the bitmap.

After some thinking, I think I prefer the bitmap. The pending_vcpu_ids
xarray feels like a premature optimization.

> > I don't think we should do that, and your approach is much safer:
> 
> I disagree.  I actually arrived at the bitmap solution before reading this.  My
> concern isn't so much about IPI virtualization, it's about the lurking danger of
> an unverified vcpu_id.  E.g. see Naveen's suggestion in this thread of simply
> letting vcpu_load() initialize the per-VM table, which doesn't work because x86
> calls vcpu_load() as part of vCPU creation.  I wouldn't be all that surprised if
> there's another bug or two in KVM where colliding 
> 
> Ha!  Case in point, s390 had what is effectively the *exact* same bug and fixed
> it in the *exact* same way (hooking vcpu_postcreate()) over a decade ago in
> commit 255088244929 ("KVM: s390: fix SCA related races and double use"), and that
> obviously did nothing to help x86 from repeating the same mistake.
> 
> In other words, I want to fix this entire class of bugs, not play a game of
> whack-a-mole with bugs that humans are all but guaranteed to overlook.
> 
> My other concern with the proposed change is that the vCPU becomes reachable
> before long before kvm_arch_vcpu_postcreate().  Which should be fine" for IPI
> virtualization, but sets a precedence I'd rather not exist, because initializing
> vCPU state _after_ it's reachable is rarely correct.  E.g. msr_kvm_poll_control
> is initialized in postcreate for some reason, and that's technically buggy
> because it's possible, albeit extremely unlikely, that MSR_KVM_POLL_CONTROL could
> be written by userspace before postcreate() runs.

Yeah, agree on both points. I myself was a bit uncomfortable with my
vcpu_postcreate approach implying some unwritten ill-defined rules on
what things _must_ be done in vcpu_postcreate (rather than in
vcpu_create) and what things _must not_ be done in it.

> E.g. for the xarray approacy (sketch only, completely untested):
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index fdbd697d0337..e34ca5920393 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -791,6 +791,7 @@ struct kvm {
>         /* The current active memslot set for each address space */
>         struct kvm_memslots __rcu *memslots[KVM_MAX_NR_ADDRESS_SPACES];
>         struct xarray vcpu_array;
> +       struct xarray pending_vcpu_ids;
>         /*
>          * Protected by slots_lock, but can be read outside if an
>          * incorrect answer is acceptable.
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 2df8ee9ecf6c..8bc4f7ffd76d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4179,6 +4179,12 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>                 return r;
>         }
>  
> +       r = xa_insert(&kvm->pending_vcpu_ids, id, xa_mk_value(id), GFP_KERNEL);
> +       if (r) {
> +               mutex_unlock(&kvm->lock);
> +               return r;
> +       }
> +
>         kvm->created_vcpus++;
>         mutex_unlock(&kvm->lock);
>  
> @@ -4249,6 +4255,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>          */
>         smp_wmb();
>         atomic_inc(&kvm->online_vcpus);
> +       xa_erase(&kvm->pending_vcpu_ids, id);
>         mutex_unlock(&vcpu->mutex);
>  
>         mutex_unlock(&kvm->lock);
> @@ -4273,6 +4280,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>  vcpu_decrement:
>         mutex_lock(&kvm->lock);
>         kvm->created_vcpus--;
> +       xa_erase(&kvm->pending_vcpu_ids, id);
>         mutex_unlock(&kvm->lock);
>         return r;
>  }
> 
> 
> or if that doesn't work, the more naive bitmap approach:
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index fdbd697d0337..eadde580e27f 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -791,6 +791,8 @@ struct kvm {
>         /* The current active memslot set for each address space */
>         struct kvm_memslots __rcu *memslots[KVM_MAX_NR_ADDRESS_SPACES];
>         struct xarray vcpu_array;
> +       DECLARE_BITMAP(vcpu_ids, KVM_MAX_VCPU_IDS);
> +
>         /*
>          * Protected by slots_lock, but can be read outside if an
>          * incorrect answer is acceptable.
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 2df8ee9ecf6c..c735698e76cf 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4179,6 +4179,11 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>                 return r;
>         }
>  
> +       if (__test_and_set_bit(id, kvm->vcpu_ids)) {
> +               mutex_unlock(&kvm->lock);
> +               return -EEXIST;
> +       }
> +
>         kvm->created_vcpus++;
>         mutex_unlock(&kvm->lock);
>  
> @@ -4213,7 +4218,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>  
>         mutex_lock(&kvm->lock);
>  
> -       if (kvm_get_vcpu_by_id(kvm, id)) {
> +       if (WARN_ON_ONCE(kvm_get_vcpu_by_id(kvm, id))) {
>                 r = -EEXIST;
>                 goto unlock_vcpu_destroy;
>         }
> @@ -4273,6 +4278,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>  vcpu_decrement:
>         mutex_lock(&kvm->lock);
>         kvm->created_vcpus--;
> +       __clear_bit(id, kvm->vcpu_ids);
>         mutex_unlock(&kvm->lock);
>         return r;
>  }