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

[Huang, Kai]

On Fri, 2026-07-17 at 18:20 +0200, Dmytro Maluka wrote:
> On Fri, Jul 17, 2026 at 12:59:38PM +0000, Huang, Kai wrote:
> > > 
> > > An easy fix would be to clear the pid_table entry in vmx_vcpu_free().
> > > However that would be still problematic, for the following reason:
> > > userspace may try to create a vCPU with the same vcpu_id as an existing
> > > one; vmx_vcpu_create() will succeed, and only after that
> > > kvm_vm_ioctl_create_vcpu() will check for the duplicate vcpu_id and
> > > fail with -EEXIST and then free the vCPU in the failure path. So in this
> > > failure path, vmx_vcpu_free() would clear the pid_table entry for that
> > > already existing good vCPU, i.e. effectively disable IPIv for that vCPU.
> > 
> > IMHO this seems a bit fragile?  If something similar to pid_table coming up in
> > the future, we could end up with a similar problem.
> > 
> > The "duplicated vcpu_id check" seems the ones that should happen as early as
> > possible.  Is it better to move the "duplicated vcpu_id check" earlier before
> > vmx_vcpu_create(), e.g., even before the kvm_arch_vcpu_precreate()?  In this
> > case we can do the pid_table cleanup in vmx_vcpu_free() I think.
> 
> Unfortunately it is not that simple. As I understand, the reason why the
> "duplicated vcpu_id check" is done later is that it needs to be done
> atomically together with inserting the vCPU into kvm->vcpu_array after
> it, i.e. both the check and the insertion need to be done with kvm->lock
> held (rather than releasing kvm->lock and taking it again between the
> two operations).
> 
> IOW, if we just move the "duplicated vcpu_id check" earlier (before the
> first unlock of kvm->lock), we have a race:
> 
> 1. vCPU A is being created but not installed in kvm->vcpu_array yet.
> 2. vCPU B with the same vcpu_id is being created. It passes the
>    duplicated vcpu_id check, since the check doesn't find vCPU A in
>    kvm->vcpu_array.
> 3. vCPU A is installed in kvm->vcpu_array, vCPU creation succeeds.
> 4. vCPU B with the same vcpu_id is installed in kvm->vcpu_array, vCPU
>    creation succeeds.
> 
> And we cannot just move the kvm->vcpu_array insertion earlier (before
> the first unlock of kvm->lock), at least because the vCPU is not even
> allocated at that point.

Hmm right, and AFACIT vCPU A needs to actually bump up the kvm->online_vcpus in
order for the vCPU B to detect the vCPU of same vcpu_id has been created because
kvm_get_vcpu_by_id() will only scan online vCPUs.

Thanks for pointing out!

> 
> 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 don't think we should do that, and your approach is much safer:

Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx>