All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmytro Maluka <dmaluka@chromium.org>
To: "Huang, Kai" <kai.huang@intel.com>
Cc: "seanjc@google.com" <seanjc@google.com>,
	"Gao, Chao" <chao.gao@intel.com>,
	"aashish@aashishsharma.net" <aashish@aashishsharma.net>,
	"guang.zeng@intel.com" <guang.zeng@intel.com>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"vineeth@bitbyteword.org" <vineeth@bitbyteword.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"jaszczyk@chromium.org" <jaszczyk@chromium.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"Dong, Chuanxiao" <chuanxiao.dong@intel.com>
Subject: Re: [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation
Date: Fri, 17 Jul 2026 18:20:09 +0200	[thread overview]
Message-ID: <alpWONLi8h2T5WhZ@google.com> (raw)
In-Reply-To: <792366b7d918faca3f40bccab56bab965e7e34f5.camel@intel.com>

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.

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?

  reply	other threads:[~2026-07-17 16:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16 16:08 [PATCH] KVM: VMX: Postpone IPIv setup after successful vCPU creation Dmytro Maluka
2026-07-16 16:29 ` sashiko-bot
2026-07-16 17:24   ` Dmytro Maluka
2026-07-17 12:59 ` Huang, Kai
2026-07-17 16:20   ` Dmytro Maluka [this message]
2026-07-17 21:21     ` Huang, Kai

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=alpWONLi8h2T5WhZ@google.com \
    --to=dmaluka@chromium.org \
    --cc=aashish@aashishsharma.net \
    --cc=chao.gao@intel.com \
    --cc=chuanxiao.dong@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=guang.zeng@intel.com \
    --cc=jaszczyk@chromium.org \
    --cc=kai.huang@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=vineeth@bitbyteword.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.