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

["Huang, Kai" <kai.huang@intel.com>]

On Thu, 2025-05-22 at 06:40 -0700, Sean Christopherson wrote:
> 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.

This is true.  Agreed.

> 
> 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

Right.  No argument around this.  I agree if there are multiple features wanting
to update then it could be dangerous.  Thanks for the insight :-)