All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Binbin Wu <binbin.wu@linux.intel.com>
Cc: sashiko-reviews@lists.linux.dev,
	Paolo Bonzini <pbonzini@redhat.com>,
	 kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init
Date: Wed, 8 Jul 2026 06:09:32 -0700	[thread overview]
Message-ID: <ak5L2csITyi1tGp0@google.com> (raw)
In-Reply-To: <a018bb95-cda5-4431-9bed-e60223087d97@linux.intel.com>

On Wed, Jul 08, 2026, Binbin Wu wrote:
> On 7/8/2026 10:35 AM, sashiko-bot@kernel.org wrote:
> > This isn't a bug introduced by this patch, but does the KVM_TDX_INIT_VM
> > initialization path have a lock inversion?
> > 
> > Looking at the call chain triggered by the KVM_TDX_INIT_VM ioctl, 
> > tdx_vm_ioctl() first acquires the inner KVM locks via 
> > tdx_acquire_vm_state_locks():
> > 
> > arch/x86/kvm/vmx/tdx.c:tdx_vm_ioctl() {
> >     ...
> >     CLASS(tdx_vm_state_guard, guard)(kvm);
> >     ...
> >     r = tdx_td_init(kvm, &tdx_cmd);
> >     ...
> > }
> > 
> > The tdx_vm_state_guard acquires kvm->lock, all vcpu->mutex locks, and 
> > kvm->slots_lock. 
> > 
> > Then tdx_td_init() continues and calls __tdx_td_init(), where 
> > cpus_read_lock() is acquired directly:
> > 
> > arch/x86/kvm/vmx/tdx.c:__tdx_td_init() {
> >     ...
> >     cpus_read_lock();
> >     ...
> > }
> > 
> > Since KVM locking rules require cpus_read_lock() to be the outermost lock,
> > does acquiring it while holding kvm->lock and the other inner locks create
> > a deadlock risk against CPU hotplug or other paths that acquire these locks
> > in the correct order?
> > 
> 
> The virt/kvm/locking.rst doesn't explicitly say cpus_read_lock() needs to be the
> outermost lock, but
> documentation:        cpus_read_lock() is taken outside kvm_lock
>                       kvm->lock is taken outside kvm->slots_lock
> ​set_nx_huge_pages():  takes kvm_lock outside of kvm->slots_lock
> 
> So it's transitive that cpus_read_lock() should be taken outside of
> kvm->slots_lock while KVM_TDX_INIT_VM takes cpus_read_lock() inside
> kvm->slots_lock.
> 
> So there could be a deadlock case:
> tdx_td_init()               set_nx_huge_pages()        __kvmclock_cpufreq_notifier
>   lock(&kvm->slots_lock)                             
>                               lock(kvm_lock)
>                                                          lock(cpu_hotplug_lock)
>                               lock(&kvm->slots_lock)
>                                                          lock(kvm_lock)
>   lock(cpu_hotplug_lock)
> 
> There was a discussion that is related to this issue:
> https://lore.kernel.org/kvm/20250124191109.205955-2-pbonzini@redhat.com/
> It seems the discussion is still open?

Yeah, mostly because the deadlock is so extremely unlikely.

> Back to this issue, does it make sense to take cpus_read_lock() before 
> kvm->lock during KVM_TDX_INIT_VM as following:

I would say "not yet".  I consider this a set_nx_huge_pages() problem, not a TDX
problem.

  reply	other threads:[~2026-07-08 13:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08  2:29 [PATCH] KVM: x86: TDX: Use validated CPUID entry count for TD init Binbin Wu
2026-07-08  2:35 ` sashiko-bot
2026-07-08  8:50   ` Binbin Wu
2026-07-08 13:09     ` Sean Christopherson [this message]
2026-07-08  8:42 ` Thorsten Blum
2026-07-08  9:04   ` Binbin Wu
2026-07-08 16:20     ` Edgecombe, Rick P
2026-07-09  1:50       ` Binbin Wu
2026-07-09 13:41         ` Sean Christopherson
2026-07-10  0:12           ` Binbin Wu

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=ak5L2csITyi1tGp0@google.com \
    --to=seanjc@google.com \
    --cc=binbin.wu@linux.intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.