Re: [PATCH 3/3] KVM: VMX: Initialize TDX during KVM module load

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/11/2024 9:22 am, Sean Christopherson wrote:
On Thu, Oct 31, 2024, Kai Huang wrote:
On Wed, 2024-10-30 at 08:19 -0700, Sean Christopherson wrote:
+void __init tdx_bringup(void)
+{
+	enable_tdx = enable_tdx && !__tdx_bringup();

Ah.  I don't love this approach because it mixes "failure" due to an unsupported
configuration, with failure due to unexpected issues.  E.g. if enabling virtualization
fails, loading KVM-the-module absolutely should fail too, not simply disable TDX.

Thanks for the comments.

I see your point.  However for "enabling virtualization failure" kvm_init() will
also try to do (default behaviour), so if it fails it will result in module
loading failure eventually.  So while I guess it would be slightly better to
make module loading fail if "enabling virtualization fails" in TDX, it is a nit
issue to me.

I think "enabling virtualization failure" is the only "unexpected issue" that
should result in module loading failure.  For any other TDX-specific
initialization failure (e.g., any memory allocation in future patches) it's
better to only disable TDX.

I disagree.  The platform owner wants TDX to be enabled, KVM shouldn't silently
disable TDX because of a transient, unrelated failure.

If TDX _can't_ be supported, e.g. because EPT or MMIO SPTE caching was explicitly
disable, then that's different.  And that's the general pattern throughout KVM.
If a requested feature isn't supported, then KVM continues on updates the module
param accordingly.  But if something outright fails during setup, KVM aborts the
entire sequence.

So I can change to "make loading KVM-the-module fail if enabling virtualization
fails in TDX", but I want to confirm this is what you want?

I would prefer the logic to be: reject loading kvm-intel.ko if an operation that
would normally succeed, fails.

OK will change to what you suggested. I'll need to take a deeper look though since later patches will add more checks.

Thanks for the comments!





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux