| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aTfKeNMIiF8NCRlO@intel.com>
Date: Tue, 9 Dec 2025 15:06:32 +0800
From: Chao Gao <chao.gao@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
<x86@...nel.org>, Kiryl Shutsemau <kas@...nel.org>, Paolo Bonzini
<pbonzini@...hat.com>, <linux-kernel@...r.kernel.org>,
<linux-coco@...ts.linux.dev>, <kvm@...r.kernel.org>, Dan Williams
<dan.j.williams@...el.com>
Subject: Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module
initialization during subsys init
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@...gle.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;
> }
Powered by blists - more mailing lists