Re: BUILD_BUG_ON failed for kvm_vmx struct size sanity check in vmx.c

[Sean Christopherson <seanjc@google.com>]

On Fri, Aug 28, 2026, Sean Christopherson wrote:
> On Thu, Aug 27, 2026, Jean-Christophe Guillain wrote:
> > Hello,
> > 
> > Since yesterday, my daily compilation of Linus tree fails with this
> > error:
> > 
> >   CC [M]  arch/x86/kvm/vmx/vmx.o
> > In file included from <command-line>:
> > arch/x86/kvm/vmx/vmx.c: In function ‘vmx_init’:
> > ././include/linux/compiler_types.h:702:45: error: call to
> > ‘__compiletime_assert_1411’ declared with attribute error: BUILD_BUG_ON
> > failed: get_order(sizeof(struct kvm_vmx) - SIZE_OF_MEMSLOTS_HASHTABLE)
> > && !IS_ENABLED(CONFIG_DEBUG_KERNEL) && !IS_ENABLED(CONFIG_KASAN)
> >   702 |         _compiletime_assert(condition, msg,
> > __compiletime_assert_, __COUNTER__)
> 
> The immediate problem is more than likely commit 97d65b544f48 ("KVM: Check for
> duplicate vcpu_id as early as possible"), which increased the size by 512 bytes.

/facepalm

Note, my claim wasn't 512 bytes, it was 256 bytes.  From commit 97d65b544f48
("KVM: Check for duplicate vcpu_id as early as possible"):

  the worst case scenario is 256 bytes per-VM (on x86, which allows up to 16KiB vCPU IDs)

Dmytro was the author, but that blurb came from me.

And I didn't _completely_ fail at math, just mostly failed at math.  I was write
that it would require 256 somethings, the small problem is that it's 256 "unsigned
longs", not 256 bytes (I did the mental math to compute the size of the array in
number of entries, not in number of bytes).

So it's not 256 bytes, it's 2048 bytes, which is basically just a rounding error,
right?  Right!?!?.  *sigh*

In other words, the compile-time assertions added by commit ac777fbf064f ("KVM:
x86: Use kvzalloc() to allocate VM struct") are doing exactly what they're intended
to do, but I failed at math and at incorporating the sanity check into my test flow.

Looking at my semi-temporary xarray idea, that approach doesn't make much sense.
An xarray node is currently 576 bytes, so in practice it would either bleed more
or less the same number of bytes, or would result in alloc+free on every vCPU
creation.

Stepping back, all of this complexity exists purely so that the guts of vCPU
creation can run without holding kvm->lock.  AFAIK, everyone, myself included,
has assumed that dropping kvm->lock was a performance optimization, e.g. to allow
userspace to create vCPUs concurrently for latency purposes.  But I've scraped
through all of the VMMs I know about, and several I didn't until now, and unless
I'm misreading Rust code, no VMM actually does concurrent vCPU creation.

And looking at the history of this code, before commit 11ec28047118 ("KVM: Convert
vm lock to a mutex"), kvm->lock was a spinlock.  I.e. KVM *had* to drop kvm->lock
when doing the bulk of vCPU creation, otherwise it couldn't do normal memory
allocations.  When kvm->lock got turned into a mutex, no one update vCPU creation
to take advtange of that.  And 19 years later, we all just assumed the complexity
was for performance reasons.

Furthermore, naively parallelizing vCPU creation in userspace is likely a net
negative due to the overheads of task creation.  Unless a VMM specificaly avoids
extra overhead related to parallelization, e.g. spawns each vCPU's thread before
creating the vCPU, creating vCPUs concurrently is a net *negative* up until about
~64 vCPUs, after which the times are a wash.

The speed of light is faster if KVM doesn't hold kvm-lock, but at vCPU counts of
~24 or less, it's probably in the noise when considering total VM creation time.
E.g. 190us vs. 390us for 8 vCPUs, 1170us vs. 420us for 24 vCPUs.

On top of all that, we've had a *lot* of fatal bugs (found by syzkaller) related
to vCPUs being created while trying to do per-VM operations (basically, see every
flow that locks all vCPUs).  I.e. the parallel vCPU creation "support" is actively
harmful.

So, rather than trying to come up with a scheme that is performant without being
too complex, I'm planning on fixing this by simply holding kvm->lock for the entire
duration of vCPU creation, so that KVM doesn't need to do extra tracking and can
instead rely purely on the array of online vCPUs.  Then we can revert 97d65b544f48
and do more cleanups on top.

I'll send a series (hopefully tomorrow) after testing.

P.S. I was also wrong (though nowhere near as badly) in commit ac777fbf064f when
     I said the size of kvm_{svm,vmx,tdx} was below 2KiB.  That was true, but only
     for a KVM built without Hyper-V or Xen emulation.  With those it was and still
     is about 2.7KiB.  So bigger, but still in the realm of "shouldn't cross the
     4KiB boundary anytime soon".