Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init

Sean Christopherson
2025-12-12T18:56:53Z

On Tue, Dec 09, 2025, Chao Gao wrote:
> On Fri, Dec 05, 2025 at 05:10:50PM -0800, Sean Christopherson wrote:
> > static int __init __tdx_bringup(void)
> > {
> > const struct tdx_sys_info_td_conf *td_conf;
> >@@ -3417,34 +3362,18 @@ static int __init __tdx_bringup(void)
> > }
> > }
> >
> >- /*
> >- * Enabling TDX requires enabling hardware virtualization first,
> >- * as making SEAMCALLs requires CPU being in post-VMXON state.
> >- */
> >- r = kvm_enable_virtualization();
> >- if (r)
> >- return r;
> >-
> >- cpus_read_lock();
> >- r = __do_tdx_bringup();
> >- cpus_read_unlock();
> >-
> >- if (r)
> >- goto tdx_bringup_err;
> >-
> >- r = -EINVAL;
> > /* Get TDX global information for later use */
> > tdx_sysinfo = tdx_get_sysinfo();
> >- if (WARN_ON_ONCE(!tdx_sysinfo))
> >- goto get_sysinfo_err;
> >+ if (!tdx_sysinfo)
> >+ return -EINVAL;
>
> ...
>
> >- /*
> >- * Ideally KVM should probe whether TDX module has been loaded
> >- * first and then try to bring it up. But TDX needs to use SEAMCALL
> >- * to probe whether the module is loaded (there is no CPUID or MSR
> >- * for that), and making SEAMCALL requires enabling virtualization
> >- * first, just like the rest steps of bringing up TDX module.
> >- *
> >- * So, for simplicity do everything in __tdx_bringup(); the first
> >- * SEAMCALL will return -ENODEV when the module is not loaded. The
> >- * only complication is having to make sure that initialization
> >- * SEAMCALLs don't return TDX_SEAMCALL_VMFAILINVALID in other
> >- * cases.
> >- */
> > r = __tdx_bringup();
> >- if (r) {
> >- /*
> >- * Disable TDX only but don't fail to load module if the TDX
> >- * module could not be loaded. No need to print message saying
> >- * "module is not loaded" because it was printed when the first
> >- * SEAMCALL failed. Don't bother unwinding the S-EPT hooks or
> >- * vm_size, as kvm_x86_ops have already been finalized (and are
> >- * intentionally not exported). The S-EPT code is unreachable,
> >- * and allocating a few more bytes per VM in a should-be-rare
> >- * failure scenario is a non-issue.
> >- */
> >- if (r == -ENODEV)
> >- goto success_disable_tdx;
>
> Previously, loading kvm-intel.ko (with tdx=1) would succeed even if there was
> no TDX module loaded by BIOS. IIUC, the behavior changes here; the lack of TDX
> module becomes fatal and kvm-intel.ko loading would fail.
>
> Is this intentional?

Nope, definitely not intentional. I think this as fixup?

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index d0161dc3d184..4e0372f12e6d 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3365,7 +3365,7 @@ static int __init __tdx_bringup(void)
/* Get TDX global information for later use */
tdx_sysinfo = tdx_get_sysinfo();
if (!tdx_sysinfo)
- return -EINVAL;
+ return -ENODEV;

/* Check TDX module and KVM capabilities */
if (!tdx_get_supported_attrs(&tdx_sysinfo->td_conf) ||
@@ -3470,8 +3470,20 @@ int __init tdx_bringup(void)
}

r = __tdx_bringup();
- if (r)
- enable_tdx = 0;
+ if (r) {
+ /*
+ * Disable TDX only but don't fail to load module if the TDX
+ * module could not be loaded. No need to print message saying
+ * "module is not loaded" because it was printed when the first
+ * SEAMCALL failed. Don't bother unwinding the S-EPT hooks or
+ * vm_size, as kvm_x86_ops have already been finalized (and are
+ * intentionally not exported). The S-EPT code is unreachable,
+ * and allocating a few more bytes per VM in a should-be-rare
+ * failure scenario is a non-issue.
+ */
+ if (r == -ENODEV)
+ goto success_disable_tdx;
+ }

return r;