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@xxxxxxxxxx Right. No argument around this. I agree if there are multiple features wanting to update then it could be dangerous. Thanks for the insight :-)
