Re: [PATCH v3 2/3] KVM: x86: Use kvzalloc() to allocate VM struct

[Sean Christopherson <seanjc@google.com>]

On Wed, May 21, 2025, Kai Huang wrote:
> On Wed, 2025-05-21 at 10:12 -0700, Sean Christopherson wrote:
> > > e.g., if we export kvm_x86_ops, we could unwind them.
> > 
> > Maaaybe.  I mean, yes, we could fully unwind kvm_x86_ops, but doing so would make
> > the overall code far more brittle.  E.g. simply updating kvm_x86_ops won't suffice,
> > as the static_calls also need to be patched, and we would have to be very careful
> > not to touch anything in kvm_x86_ops that might have been consumed between here
> > and the call to tdx_bringup().
> 
> Right.  Maybe exporting kvm_ops_update() is better.

A bit, but KVM would still need to be careful not to modify the parts of
vt_x86_ops that have already been consumed.

While I agree that leaving TDX breadcrumbs in kvm_x86_ops when TDX is disabled is
undesirable, the behavior is known, i.e. we know exactly what TDX state is being
left behind.  And failure to load the TDX Module should be very, very rare for
any setup that is actually trying to enable TDX.

Whereas providing a way to modify kvm_x86_ops creates the possibility for "bad"
updates.  KVM's initialization code is a lot like the kernel's boot code (and
probably most bootstrapping code): it's inherently fragile because avoiding
dependencies is practically impossible.

E.g. I ran into a relevant ordering problem[*] just a few days ago, where checking
for VMX capabilities during PMU initialization always failed because the VMCS
config hadn't yet been parsed.  Those types of bugs are especially dangerous
because they're very easy to overlook when modifying existing code, e.g. the
only sign that anything is broken is an optional feature being missing.

[*] https://lore.kernel.org/all/aCU2YEpU0dOk7RTk@google.com