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

[Sean Christopherson <seanjc@google.com>]

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;