All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Gao <chao.gao@intel.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
	Kiryl Shutsemau <kas@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	<linux-kernel@vger.kernel.org>, <linux-coco@lists.linux.dev>,
	<kvm@vger.kernel.org>, Dan Williams <dan.j.williams@intel.com>
Subject: Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init
Date: Tue, 9 Dec 2025 15:06:32 +0800	[thread overview]
Message-ID: <aTfKeNMIiF8NCRlO@intel.com> (raw)
In-Reply-To: <20251206011054.494190-4-seanjc@google.com>

On Fri, Dec 05, 2025 at 05:10:50PM -0800, Sean Christopherson wrote:
>Now that VMXON can be done without bouncing through KVM, do TDX-Module
>initialization during subsys init (specifically before module_init() so
>that it runs before KVM when both are built-in).  Aside from the obvious
>benefits of separating core TDX code from KVM, this will allow tagging a
>pile of TDX functions and globals as being __init and __ro_after_init.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>
>---
> Documentation/arch/x86/tdx.rst |  26 -----
> arch/x86/include/asm/tdx.h     |   4 -
> arch/x86/kvm/vmx/tdx.c         | 169 ++++++--------------------------
> arch/x86/virt/vmx/tdx/tdx.c    | 170 ++++++++++++++++++---------------
> arch/x86/virt/vmx/tdx/tdx.h    |   8 --
> 5 files changed, 124 insertions(+), 253 deletions(-)
>
>diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
>index 61670e7df2f7..2e0a15d6f7d1 100644
>--- a/Documentation/arch/x86/tdx.rst
>+++ b/Documentation/arch/x86/tdx.rst
>@@ -60,32 +60,6 @@ Besides initializing the TDX module, a per-cpu initialization SEAMCALL
> must be done on one cpu before any other SEAMCALLs can be made on that
> cpu.
> 
>-The kernel provides two functions, tdx_enable() and tdx_cpu_enable() to
>-allow the user of TDX to enable the TDX module and enable TDX on local
>-cpu respectively.
>-
>-Making SEAMCALL requires VMXON has been done on that CPU.  Currently only
>-KVM implements VMXON.  For now both tdx_enable() and tdx_cpu_enable()
>-don't do VMXON internally (not trivial), but depends on the caller to
>-guarantee that.
>-
>-To enable TDX, the caller of TDX should: 1) temporarily disable CPU
>-hotplug; 2) do VMXON and tdx_enable_cpu() on all online cpus; 3) call
>-tdx_enable().  For example::
>-
>-        cpus_read_lock();
>-        on_each_cpu(vmxon_and_tdx_cpu_enable());
>-        ret = tdx_enable();
>-        cpus_read_unlock();
>-        if (ret)
>-                goto no_tdx;
>-        // TDX is ready to use
>-
>-And the caller of TDX must guarantee the tdx_cpu_enable() has been
>-successfully done on any cpu before it wants to run any other SEAMCALL.
>-A typical usage is do both VMXON and tdx_cpu_enable() in CPU hotplug
>-online callback, and refuse to online if tdx_cpu_enable() fails.
>-

With this patch applied, other parts of the document will be stale and need
updates, i.e.,:

diff --git a/Documentation/arch/x86/tdx.rst b/Documentation/arch/x86/tdx.rst
index 2e0a15d6f7d1..3d5bc68e3bcb 100644
--- a/Documentation/arch/x86/tdx.rst
+++ b/Documentation/arch/x86/tdx.rst
@@ -66,12 +66,12 @@ If the TDX module is initialized successfully, dmesg shows something
 like below::
 
   [..] virt/tdx: 262668 KBs allocated for PAMT
-  [..] virt/tdx: module initialized
+  [..] virt/tdx: TDX-Module initialized
 
 If the TDX module failed to initialize, dmesg also shows it failed to
 initialize::
 
-  [..] virt/tdx: module initialization failed ...
+  [..] virt/tdx: TDX-Module initialization failed ...
 
 TDX Interaction to Other Kernel Components
 ------------------------------------------
@@ -102,11 +102,6 @@ removal but depends on the BIOS to behave correctly.
 CPU Hotplug
 ~~~~~~~~~~~
 
-TDX module requires the per-cpu initialization SEAMCALL must be done on
-one cpu before any other SEAMCALLs can be made on that cpu.  The kernel
-provides tdx_cpu_enable() to let the user of TDX to do it when the user
-wants to use a new cpu for TDX task.
-
 TDX doesn't support physical (ACPI) CPU hotplug.  During machine boot,
 TDX verifies all boot-time present logical CPUs are TDX compatible before
 enabling TDX.  A non-buggy BIOS should never support hot-add/removal of

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

>-
>+	if (r)
> 		enable_tdx = 0;
>-	}
> 
> 	return r;
> 
>@@ -3596,6 +3480,15 @@ int __init tdx_bringup(void)
> 	return 0;
> }

  parent reply	other threads:[~2025-12-09  7:06 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-06  1:10 [PATCH v2 0/7] KVM: x86/tdx: Have TDX handle VMXON during bringup Sean Christopherson
2025-12-06  1:10 ` [PATCH v2 1/7] KVM: x86: Move kvm_rebooting to x86 Sean Christopherson
2025-12-09  7:46   ` Chao Gao
2026-01-05 17:48   ` Dave Hansen
2025-12-06  1:10 ` [PATCH v2 2/7] KVM: x86: Extract VMXON and EFER.SVME enablement to kernel Sean Christopherson
2025-12-07  7:22   ` dan.j.williams
2025-12-09 20:01     ` Sean Christopherson
2025-12-10  7:41       ` dan.j.williams
2025-12-10 14:20         ` Sean Christopherson
2025-12-24 11:07           ` Xu Yilun
2025-12-30 22:59             ` Sean Christopherson
2025-12-09  5:48   ` Chao Gao
2025-12-17  6:57     ` Xu Yilun
2025-12-17 19:01       ` Sean Christopherson
2025-12-19  2:14         ` Xu Yilun
2025-12-19 15:40           ` Sean Christopherson
2025-12-19 17:30             ` Dave Hansen
2025-12-19 21:12             ` Huang, Kai
2026-01-27  2:46             ` Binbin Wu
2025-12-19 17:45   ` Dave Hansen
2025-12-19 18:35     ` Sean Christopherson
2025-12-19 18:48       ` Dave Hansen
2025-12-06  1:10 ` [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init Sean Christopherson
2025-12-07  7:25   ` dan.j.williams
2025-12-08 23:17     ` Sean Christopherson
2025-12-09  1:34       ` dan.j.williams
2025-12-09  7:06   ` Chao Gao [this message]
2025-12-12 18:56     ` Sean Christopherson
2025-12-06  1:10 ` [PATCH v2 4/7] x86/virt/tdx: Tag a pile of functions as __init, and globals as __ro_after_init Sean Christopherson
2025-12-09  4:17   ` dan.j.williams
2025-12-09  7:26   ` Chao Gao
2025-12-06  1:10 ` [PATCH v2 5/7] x86/virt/tdx: KVM: Consolidate TDX CPU hotplug handling Sean Christopherson
2025-12-09  4:19   ` dan.j.williams
2025-12-06  1:10 ` [PATCH v2 6/7] x86/virt/tdx: Use ida_is_empty() to detect if any TDs may be running Sean Christopherson
2025-12-09  4:19   ` dan.j.williams
2025-12-09  7:33   ` Chao Gao
2025-12-06  1:10 ` [PATCH v2 7/7] KVM: Bury kvm_{en,dis}able_virtualization() in kvm_main.c once more Sean Christopherson
2025-12-09  4:20   ` dan.j.williams
2025-12-09  7:37   ` Chao Gao
2025-12-08  2:49 ` [PATCH v2 0/7] KVM: x86/tdx: Have TDX handle VMXON during bringup Chao Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aTfKeNMIiF8NCRlO@intel.com \
    --to=chao.gao@intel.com \
    --cc=bp@alien8.de \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=kas@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.