* [PATCH v5 10/22] x86/virt/seamldr: Abort updates if errors occurred midway
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
The TDX module update process has multiple steps, each of which may
encounter failures.
The current state machine of updates proceeds to the next step regardless
of errors. But continuing updates when errors occur midway is pointless.
Abort the update by setting a flag to indicate that a CPU has encountered
an error, forcing all CPUs to exit the execution loop. Note that failing
CPUs do not acknowledge the current step. This keeps all other CPUs waiting
in the current step (since advancing to the next step requires all CPUs to
acknowledge the current step) until they detect the fault flag and exit the
loop.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
---
v5:
- Replace failed count from atomic_t to int since it's now protected by
a lock.
v3:
- Instead of fast-forward to the final stage, exit the execution loop
directly.
---
arch/x86/virt/vmx/tdx/seamldr.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 978fcca92128..e195703398e7 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -190,6 +190,7 @@ enum module_update_state {
static struct {
enum module_update_state state;
int thread_ack;
+ int failed;
/*
* Protect update_data. Raw spinlock as it will be acquired from
* interrupt-disabled contexts.
@@ -237,12 +238,17 @@ static int do_seamldr_install_module(void *seamldr_params)
break;
}
- ack_state();
+ if (ret) {
+ scoped_guard(raw_spinlock, &update_data.lock)
+ update_data.failed++;
+ } else {
+ ack_state();
+ }
} else {
touch_nmi_watchdog();
rcu_momentary_eqs();
}
- } while (curstate != MODULE_UPDATE_DONE);
+ } while (curstate != MODULE_UPDATE_DONE && !READ_ONCE(update_data.failed));
return ret;
}
@@ -264,6 +270,7 @@ int seamldr_install_module(const u8 *data, u32 size)
if (IS_ERR(params))
return PTR_ERR(params);
+ update_data.failed = 0;
set_target_state(MODULE_UPDATE_START + 1);
return stop_machine(do_seamldr_install_module, params, cpu_online_mask);
}
--
2.47.3
^ permalink raw reply related
* [PATCH v5 09/22] x86/virt/seamldr: Introduce skeleton for TDX module updates
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
TDX module updates require careful synchronization with other TDX
operations. The requirements are (#1/#2 reflect current behavior that
must be preserved):
1. SEAMCALLs need to be callable from both process and IRQ contexts.
2. SEAMCALLs need to be able to run concurrently across CPUs
3. During updates, only update-related SEAMCALLs are permitted; all
other SEAMCALLs shouldn't be called.
4. During updates, all online CPUs must participate in the update work.
No single lock primitive satisfies all requirements. For instance,
rwlock_t handles #1/#2 but fails #4: CPUs spinning with IRQs disabled
cannot be directed to perform update work.
Use stop_machine() as it is the only well-understood mechanism that can
meet all requirements.
And TDX module updates consist of several steps (See Intel® Trust Domain
Extensions (Intel® TDX) Module Base Architecture Specification, Revision
348549-007, Chapter 4.5 "TD-Preserving TDX module Update"). Ordering
requirements between steps mandate lockstep synchronization across all
CPUs.
multi_cpu_stop() is a good example of performing a multi-step task in
lockstep. But it doesn't synchronize steps within the callback function
it takes. So, implement one based on its pattern to establish the
skeleton for TDX module updates. Specifically, add a global state
machine where each state represents a step in the update flow. The state
advances only after all CPUs acknowledge completing their work in the
current state. This acknowledgment mechanism is what ensures lockstep
execution.
Potential alternative to stop_machine()
=======================================
An alternative approach is to lock all KVM entry points and kick all
vCPUs. Here, KVM entry points refer to KVM VM/vCPU ioctl entry points,
implemented in KVM common code (virt/kvm). Adding a locking mechanism
there would affect all architectures KVM supports. And to lock only TDX
vCPUs, new logic would be needed to identify TDX vCPUs, which the KVM
common code currently lacks. This would add significant complexity and
maintenance overhead to KVM for this TDX-specific use case.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
---
v5:
- rewrite the commit message [Rick]
- use a lock to synchronize accesses to update_data [Dave]
- rename tdp_state and tdp_data to module_update_state and update_data
for clarity [Kai]
v2:
- refine the changlog to follow context-problem-solution structure
- move alternative discussions at the end of the changelog
- add a comment about state machine transition
- Move rcu_momentary_eqs() call to the else branch.
---
arch/x86/virt/vmx/tdx/seamldr.c | 77 ++++++++++++++++++++++++++++++++-
1 file changed, 75 insertions(+), 2 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 20cb6c797ce5..978fcca92128 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -7,8 +7,10 @@
#define pr_fmt(fmt) "seamldr: " fmt
#include <linux/mm.h>
+#include <linux/nmi.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
+#include <linux/stop_machine.h>
#include <asm/seamldr.h>
@@ -174,6 +176,77 @@ static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
return alloc_seamldr_params(module, module_size, sig, sig_size);
}
+/*
+ * During a TDX module update, all CPUs start from MODULE_UPDATE_START and
+ * progress to MODULE_UPDATE_DONE. Each state is associated with certain
+ * work. For some states, just one CPU needs to perform the work, while
+ * other CPUs just wait during those states.
+ */
+enum module_update_state {
+ MODULE_UPDATE_START,
+ MODULE_UPDATE_DONE,
+};
+
+static struct {
+ enum module_update_state state;
+ int thread_ack;
+ /*
+ * Protect update_data. Raw spinlock as it will be acquired from
+ * interrupt-disabled contexts.
+ */
+ raw_spinlock_t lock;
+} update_data = {
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(update_data.lock)
+};
+
+static void set_target_state(enum module_update_state state)
+{
+ /* Reset ack counter. */
+ update_data.thread_ack = num_online_cpus();
+ update_data.state = state;
+}
+
+/* Last one to ack a state moves to the next state. */
+static void ack_state(void)
+{
+ guard(raw_spinlock)(&update_data.lock);
+ update_data.thread_ack--;
+ if (!update_data.thread_ack)
+ set_target_state(update_data.state + 1);
+}
+
+/*
+ * See multi_cpu_stop() from where this multi-cpu state-machine was
+ * adopted, and the rationale for touch_nmi_watchdog().
+ */
+static int do_seamldr_install_module(void *seamldr_params)
+{
+ enum module_update_state newstate, curstate = MODULE_UPDATE_START;
+ int ret = 0;
+
+ do {
+ /* Chill out and re-read update_data. */
+ cpu_relax();
+ newstate = READ_ONCE(update_data.state);
+
+ if (newstate != curstate) {
+ curstate = newstate;
+ switch (curstate) {
+ /* TODO: add the update steps. */
+ default:
+ break;
+ }
+
+ ack_state();
+ } else {
+ touch_nmi_watchdog();
+ rcu_momentary_eqs();
+ }
+ } while (curstate != MODULE_UPDATE_DONE);
+
+ return ret;
+}
+
DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
if (!IS_ERR_OR_NULL(_T)) free_seamldr_params(_T))
@@ -191,7 +264,7 @@ int seamldr_install_module(const u8 *data, u32 size)
if (IS_ERR(params))
return PTR_ERR(params);
- /* TODO: Update TDX module here */
- return 0;
+ set_target_state(MODULE_UPDATE_START + 1);
+ return stop_machine(do_seamldr_install_module, params, cpu_online_mask);
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
--
2.47.3
^ permalink raw reply related
* [PATCH v5 08/22] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX module
update requests. This structure contains physical addresses pointing to
the module binary and its signature file (or sigstruct), along with an
update scenario field.
TDX modules are distributed in the tdx_blob format defined in
blob_structure.txt from the "Intel TDX module Binaries Repository". A
tdx_blob contains a header, sigstruct, and module binary. This is also the
format supplied by the userspace to the kernel.
Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
accordingly. This structure will be passed to P-SEAMLDR to initiate the
update.
Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
Specification" will be published separately. P-SEAMLDR compatibility
validation (such as 4KB vs 16KB sigstruct support) is left to userspace,
which must verify the P-SEAMLDR version meets the TDX module's minimum
requirements.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
---
v5:
- use a macro for tdx_blob version (0x100) [Yan]
- don't do alignment checking for the binary/sigstruct [Rick]
- drop blob's sigstruct and validation checking
- set seamldr_params.version to 1 when necessary
- drop the link to blob_structure.txt which might be unstable [Kai]
v4:
- Remove checksum verification as it is optional
- Convert comments to is_vmalloc_addr() checks [Kai]
- Explain size/alignment checks in alloc_seamldr_params() [Kai]
v3:
- Print tdx_blob version in hex [Binbin]
- Drop redundant sigstruct alignment check [Yilun]
- Note buffers passed from firmware upload infrastructure are
vmalloc()'d above alloc_seamldr_params()
---
arch/x86/virt/vmx/tdx/seamldr.c | 141 ++++++++++++++++++++++++++++++++
1 file changed, 141 insertions(+)
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 7114326d7569..20cb6c797ce5 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt) "seamldr: " fmt
#include <linux/mm.h>
+#include <linux/slab.h>
#include <linux/spinlock.h>
#include <asm/seamldr.h>
@@ -16,6 +17,33 @@
/* P-SEAMLDR SEAMCALL leaf function */
#define P_SEAMLDR_INFO 0x8000000000000000
+#define SEAMLDR_MAX_NR_MODULE_4KB_PAGES 496
+#define SEAMLDR_MAX_NR_SIG_4KB_PAGES 4
+
+/*
+ * The seamldr_params "scenario" field specifies the operation mode:
+ * 0: Install TDX module from scratch (not used by kernel)
+ * 1: Update existing TDX module to a compatible version
+ */
+#define SEAMLDR_SCENARIO_UPDATE 1
+
+/*
+ * This is called the "SEAMLDR_PARAMS" data structure and is defined
+ * in "SEAM Loader (SEAMLDR) Interface Specification".
+ *
+ * It describes the TDX module that will be installed.
+ */
+struct seamldr_params {
+ u32 version;
+ u32 scenario;
+ u64 sigstruct_pa[SEAMLDR_MAX_NR_SIG_4KB_PAGES];
+ u8 reserved[80];
+ u64 num_module_pages;
+ u64 mod_pages_pa_list[SEAMLDR_MAX_NR_MODULE_4KB_PAGES];
+} __packed;
+
+static_assert(sizeof(struct seamldr_params) == 4096);
+
/*
* Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
* interact with P-SEAMLDR simultaneously. Use raw version as the calls can
@@ -41,6 +69,114 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
+static void free_seamldr_params(struct seamldr_params *params)
+{
+ free_page((unsigned long)params);
+}
+
+static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned int module_size,
+ const void *sig, unsigned int sig_size)
+{
+ struct seamldr_params *params;
+ const u8 *ptr;
+ int i;
+
+ if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
+ return ERR_PTR(-EINVAL);
+
+ if (sig_size > SEAMLDR_MAX_NR_SIG_4KB_PAGES * SZ_4K)
+ return ERR_PTR(-EINVAL);
+
+ params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
+ if (!params)
+ return ERR_PTR(-ENOMEM);
+
+ /*
+ * Only use version 1 when required (sigstruct > 4KB) for backward
+ * compatibility with P-SEAMLDR that lacks version 1 support.
+ */
+ if (sig_size > SZ_4K)
+ params->version = 1;
+ else
+ params->version = 0;
+
+ params->scenario = SEAMLDR_SCENARIO_UPDATE;
+
+ ptr = sig;
+ for (i = 0; i < sig_size / SZ_4K; i++) {
+ /*
+ * Don't assume @sig is page-aligned although it is 4KB-aligned.
+ * Always add the in-page offset to get the physical address.
+ */
+ params->sigstruct_pa[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
+ ((unsigned long)ptr & ~PAGE_MASK);
+ ptr += SZ_4K;
+ }
+
+ params->num_module_pages = module_size / SZ_4K;
+
+ ptr = module;
+ for (i = 0; i < params->num_module_pages; i++) {
+ params->mod_pages_pa_list[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
+ ((unsigned long)ptr & ~PAGE_MASK);
+ ptr += SZ_4K;
+ }
+
+ return params;
+}
+
+/*
+ * Intel TDX module blob. Its format is defined at:
+ * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
+ *
+ * Note this structure differs from the reference above: the two variable-length
+ * fields "@sigstruct" and "@module" are represented as a single "@data" field
+ * here and split programmatically using the offset_of_module value.
+ */
+struct tdx_blob {
+ u16 version;
+ u16 checksum;
+ u32 offset_of_module;
+ u8 signature[8];
+ u32 length;
+ u32 reserved0;
+ u64 reserved1[509];
+ u8 data[];
+} __packed;
+
+/* Supported versions of the tdx_blob */
+#define TDX_BLOB_VERSION_1 0x100
+
+static struct seamldr_params *init_seamldr_params(const u8 *data, u32 size)
+{
+ const struct tdx_blob *blob = (const void *)data;
+ int module_size, sig_size;
+ const void *sig, *module;
+
+ /* Ensure the size is valid otherwise reading any field from the blob may overflow. */
+ if (size <= sizeof(struct tdx_blob) || size <= blob->offset_of_module)
+ return ERR_PTR(-EINVAL);
+
+ if (blob->version != TDX_BLOB_VERSION_1) {
+ pr_err("unsupported blob version: %x\n", blob->version);
+ return ERR_PTR(-EINVAL);
+ }
+
+ /* Split the blob into a sigstruct and a module. */
+ sig = blob->data;
+ sig_size = blob->offset_of_module - sizeof(struct tdx_blob);
+ module = data + blob->offset_of_module;
+ module_size = size - blob->offset_of_module;
+
+ if (sig_size <= 0 || module_size <= 0 || blob->length != size)
+ return ERR_PTR(-EINVAL);
+
+ return alloc_seamldr_params(module, module_size, sig, sig_size);
+}
+
+DEFINE_FREE(free_seamldr_params, struct seamldr_params *,
+ if (!IS_ERR_OR_NULL(_T)) free_seamldr_params(_T))
+
/**
* seamldr_install_module - Install a new TDX module.
* @data: Pointer to the TDX module update blob.
@@ -50,6 +186,11 @@ EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
*/
int seamldr_install_module(const u8 *data, u32 size)
{
+ struct seamldr_params *params __free(free_seamldr_params) =
+ init_seamldr_params(data, size);
+ if (IS_ERR(params))
+ return PTR_ERR(params);
+
/* TODO: Update TDX module here */
return 0;
}
--
2.47.3
^ permalink raw reply related
* [PATCH v5 07/22] coco/tdx-host: Implement firmware upload sysfs ABI for TDX module updates
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
Linux kernel supports two primary firmware update mechanisms:
- request_firmware()
- firmware upload (or fw_upload)
The former is used by microcode updates, SEV firmware updates, etc. The
latter is used by CXL and FPGA firmware updates.
One key difference between them is: request_firmware() loads a named
file from the filesystem where the filename is kernel-controlled, while
fw_upload accepts firmware data directly from userspace.
Use fw_upload for TDX module updates as loading a named file isn't
suitable for TDX (see below for more reasons). Specifically, register
TDX faux device with fw_upload framework to expose sysfs interfaces
and implement operations to process data blobs supplied by userspace.
Implementation notes:
1. P-SEAMLDR processes the entire update at once rather than
chunk-by-chunk, so .write() is called only once per update; so the
offset should be always 0.
2. An update completes synchronously within .write(), meaning
.poll_complete() is only called after the update succeeds and so always
returns success
Why fw_upload instead of request_firmware()?
============================================
The explicit file selection capabilities of fw_upload is preferred over
the implicit file selection of request_firmware() for the following
reasons:
a. Intel distributes all versions of the TDX module, allowing admins to
load any version rather than always defaulting to the latest. This
flexibility is necessary because future extensions may require reverting to
a previous version to clear fatal errors.
b. Some module version series are platform-specific. For example, the 1.5.x
series is for certain platform generations, while the 2.0.x series is
intended for others.
c. The update policy for TDX module updates is non-linear at times. The
latest TDX module may not be compatible. For example, TDX module 1.5.x
may be updated to 1.5.y but not to 1.5.y+1. This policy is documented
separately in a file released along with each TDX module release.
So, the default policy of "request_firmware()" of "always load latest", is
not suitable for TDX. Userspace needs to deploy a more sophisticated policy
check (e.g., latest may not be compatible), and there is potential
operator choice to consider.
Just have userspace pick rather than add kernel mechanism to change the
default policy of request_firmware().
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
---
v5:
- remove a tail comment [Yan]
- remove is_vmalloc_addr() check [Dave]
- use devm_add_action_or_reset() for deinit [Yilun]
- remove global tdx_fwl [Yilun]
- clarify request_firmware() doesn't take filename from userspace
[Rick]
v4:
- make tdx_fwl static [Kai]
- don't support update canceling [Yilun]
- explain why seamldr_init() doesn't return an error [Kai]
- bail out if TDX module updates are not supported [Kai]
- name the firmware "tdx_module" instead of "seamldr_upload" [Cedric]
v3:
- clear "cancel_request" in the "prepare" phase [Binbin]
- Don't fail the whole tdx-host device if seamldr_init() met an error
[Yilun]
- Add kdoc for seamldr_install_module() and verify that the input
buffer is vmalloc'd. [Yilun]
---
arch/x86/include/asm/seamldr.h | 1 +
arch/x86/include/asm/tdx.h | 6 ++
arch/x86/virt/vmx/tdx/seamldr.c | 15 +++++
drivers/virt/coco/tdx-host/Kconfig | 2 +
drivers/virt/coco/tdx-host/tdx-host.c | 87 +++++++++++++++++++++++++++
5 files changed, 111 insertions(+)
diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
index c67e5bc910a9..ac6f80f7208b 100644
--- a/arch/x86/include/asm/seamldr.h
+++ b/arch/x86/include/asm/seamldr.h
@@ -32,5 +32,6 @@ struct seamldr_info {
static_assert(sizeof(struct seamldr_info) == 256);
int seamldr_get_info(struct seamldr_info *seamldr_info);
+int seamldr_install_module(const u8 *data, u32 size);
#endif /* _ASM_X86_SEAMLDR_H */
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index cb2219302dfc..b3a7301e77c6 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -103,6 +103,12 @@ int tdx_enable(void);
const char *tdx_dump_mce_info(struct mce *m);
const struct tdx_sys_info *tdx_get_sysinfo(void);
+static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinfo)
+{
+ /* To be enabled when kernel is ready. */
+ return false;
+}
+
int tdx_guest_keyid_alloc(void);
u32 tdx_get_nr_guest_keyids(void);
void tdx_guest_keyid_free(unsigned int keyid);
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 7c0cbab2c4c0..7114326d7569 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -6,6 +6,7 @@
*/
#define pr_fmt(fmt) "seamldr: " fmt
+#include <linux/mm.h>
#include <linux/spinlock.h>
#include <asm/seamldr.h>
@@ -39,3 +40,17 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
return seamldr_call(P_SEAMLDR_INFO, &args);
}
EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
+
+/**
+ * seamldr_install_module - Install a new TDX module.
+ * @data: Pointer to the TDX module update blob.
+ * @size: Size of the TDX module update blob.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int seamldr_install_module(const u8 *data, u32 size)
+{
+ /* TODO: Update TDX module here */
+ return 0;
+}
+EXPORT_SYMBOL_FOR_MODULES(seamldr_install_module, "tdx-host");
diff --git a/drivers/virt/coco/tdx-host/Kconfig b/drivers/virt/coco/tdx-host/Kconfig
index d35d85ef91c0..ca600a39d97b 100644
--- a/drivers/virt/coco/tdx-host/Kconfig
+++ b/drivers/virt/coco/tdx-host/Kconfig
@@ -1,6 +1,8 @@
config TDX_HOST_SERVICES
tristate "TDX Host Services Driver"
depends on INTEL_TDX_HOST
+ select FW_LOADER
+ select FW_UPLOAD
default m
help
Enable access to TDX host services like module update and
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 8d46e3c039ba..1b93d20406c1 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -6,6 +6,7 @@
*/
#include <linux/device/faux.h>
+#include <linux/firmware.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/sysfs.h>
@@ -94,8 +95,94 @@ static const struct attribute_group seamldr_group = {
.attrs = seamldr_attrs,
};
+static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
+ const u8 *data, u32 size)
+{
+ return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
+ u32 offset, u32 size, u32 *written)
+{
+ int ret;
+
+ /*
+ * tdx_fw_write() always processes all data on the first call with
+ * offset == 0. Since it never returns partial success (it either
+ * succeeds completely or fails), there is no subsequent call with
+ * non-zero offsets.
+ */
+ WARN_ON_ONCE(offset);
+ ret = seamldr_install_module(data, size);
+ switch (ret) {
+ case 0:
+ *written = size;
+ return FW_UPLOAD_ERR_NONE;
+ case -EBUSY:
+ return FW_UPLOAD_ERR_BUSY;
+ case -EIO:
+ return FW_UPLOAD_ERR_HW_ERROR;
+ case -ENOSPC:
+ return FW_UPLOAD_ERR_WEAROUT;
+ case -ENOMEM:
+ return FW_UPLOAD_ERR_RW_ERROR;
+ default:
+ return FW_UPLOAD_ERR_FW_INVALID;
+ }
+}
+
+static enum fw_upload_err tdx_fw_poll_complete(struct fw_upload *fwl)
+{
+ /*
+ * TDX module updates are completed in the previous phase
+ * (tdx_fw_write()). If any error occurred, the previous phase
+ * would return an error code to abort the update process. In
+ * other words, reaching this point means the update succeeded.
+ */
+ return FW_UPLOAD_ERR_NONE;
+}
+
+/*
+ * TDX module updates cannot be cancelled. Provide a stub function since
+ * the firmware upload framework requires a .cancel operation.
+ */
+static void tdx_fw_cancel(struct fw_upload *fwl)
+{
+}
+
+static const struct fw_upload_ops tdx_fw_ops = {
+ .prepare = tdx_fw_prepare,
+ .write = tdx_fw_write,
+ .poll_complete = tdx_fw_poll_complete,
+ .cancel = tdx_fw_cancel,
+};
+
+static void seamldr_deinit(void *tdx_fwl)
+{
+ firmware_upload_unregister(tdx_fwl);
+}
+
static int seamldr_init(struct device *dev)
{
+ const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
+ struct fw_upload *tdx_fwl;
+ int ret;
+
+ if (WARN_ON_ONCE(!tdx_sysinfo))
+ return -EIO;
+
+ if (!tdx_supports_runtime_update(tdx_sysinfo))
+ return 0;
+
+ tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "tdx_module",
+ &tdx_fw_ops, NULL);
+ if (IS_ERR(tdx_fwl))
+ return PTR_ERR(tdx_fwl);
+
+ ret = devm_add_action_or_reset(dev, seamldr_deinit, tdx_fwl);
+ if (ret)
+ return ret;
+
return devm_device_add_group(dev, &seamldr_group);
}
--
2.47.3
^ permalink raw reply related
* [PATCH v5 06/22] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: x86, linux-coco, kvm, linux-kernel
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
TDX module updates require userspace to select the appropriate module
to load. Expose necessary information to facilitate this decision. Two
values are needed:
- P-SEAMLDR version: for compatibility checks between TDX module and
P-SEAMLDR
- num_remaining_updates: indicates how many updates can be performed
Expose them as tdx-host device attributes. Register these attributes
during device probe rather than creation, making it easier to hide them
when P-SEAMLDR calls are unsafe (due to CPU erratum).
Signed-off-by: Chao Gao <chao.gao@intel.com>
---
v5:
- fix typos [Binbin]
- register seamldr_group during device probe
v4:
- Make seamldr attribute permission "0400" [Dave]
- Don't include implementation details in OS ABI docs [Dave]
- Tag tdx_host_group as static [Kai]
v3:
- use #ifdef rather than .is_visible() to control P-SEAMLDR sysfs
visibility [Yilun]
---
.../ABI/testing/sysfs-devices-faux-tdx-host | 23 +++++++
drivers/virt/coco/tdx-host/tdx-host.c | 68 ++++++++++++++++++-
2 files changed, 90 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
index 2cf682b65acf..44b8356aed6b 100644
--- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -4,3 +4,26 @@ Description: (RO) Report the version of the loaded TDX module. The TDX module
version is formatted as x.y.z, where "x" is the major version,
"y" is the minor version and "z" is the update version. Versions
are used for bug reporting, TDX module updates etc.
+
+What: /sys/devices/faux/tdx_host/seamldr/version
+Contact: linux-coco@lists.linux.dev
+Description: (RO) Report the version of the loaded SEAM loader. The SEAM
+ loader version is formatted as x.y.z, where "x" is the major
+ version, "y" is the minor version and "z" is the update version.
+ Versions are used for bug reporting and compatibility checks.
+
+What: /sys/devices/faux/tdx_host/seamldr/num_remaining_updates
+Contact: linux-coco@lists.linux.dev
+Description: (RO) Report the number of remaining updates. TDX maintains a
+ log about each TDX module that has been loaded. This log has
+ a finite size, which limits the number of TDX module updates
+ that can be performed.
+
+ After each successful update, the number reduces by one. Once it
+ reaches zero, further updates will fail until next reboot. The
+ number is always zero if the P-SEAMLDR doesn't support updates.
+
+ See Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
+ Interface Specification, Revision 343755-003, Chapter 3.3
+ "SEAMLDR_INFO" and Chapter 4.2 "SEAMLDR.INSTALL" for more
+ information.
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 0424933b2560..8d46e3c039ba 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -11,6 +11,7 @@
#include <linux/sysfs.h>
#include <asm/cpu_device_id.h>
+#include <asm/seamldr.h>
#include <asm/tdx.h>
static const struct x86_cpu_id tdx_host_ids[] = {
@@ -42,6 +43,71 @@ static struct attribute *tdx_host_attrs[] = {
};
ATTRIBUTE_GROUPS(tdx_host);
+static ssize_t seamldr_version_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct seamldr_info info;
+ int ret;
+
+ ret = seamldr_get_info(&info);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%u.%u.%02u\n", info.major_version,
+ info.minor_version,
+ info.update_version);
+}
+
+static ssize_t num_remaining_updates_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct seamldr_info info;
+ int ret;
+
+ ret = seamldr_get_info(&info);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%u\n", info.num_remaining_updates);
+}
+
+/*
+ * Open-code DEVICE_ATTR_ADMIN_RO to specify a different 'show' function
+ * for P-SEAMLDR version as version_show() is used for TDX module version.
+ *
+ * Admin-only readable as reading these attributes calls into P-SEAMLDR,
+ * which may have potential performance and system impact.
+ */
+static struct device_attribute dev_attr_seamldr_version =
+ __ATTR(version, 0400, seamldr_version_show, NULL);
+static DEVICE_ATTR_ADMIN_RO(num_remaining_updates);
+
+static struct attribute *seamldr_attrs[] = {
+ &dev_attr_seamldr_version.attr,
+ &dev_attr_num_remaining_updates.attr,
+ NULL,
+};
+
+static const struct attribute_group seamldr_group = {
+ .name = "seamldr",
+ .attrs = seamldr_attrs,
+};
+
+static int seamldr_init(struct device *dev)
+{
+ return devm_device_add_group(dev, &seamldr_group);
+}
+
+static int tdx_host_probe(struct faux_device *fdev)
+{
+ return seamldr_init(&fdev->dev);
+}
+
+static const struct faux_device_ops tdx_host_ops = {
+ .probe = tdx_host_probe,
+};
+
static struct faux_device *fdev;
static int __init tdx_host_init(void)
@@ -49,7 +115,7 @@ static int __init tdx_host_init(void)
if (!x86_match_cpu(tdx_host_ids) || !tdx_get_sysinfo())
return -ENODEV;
- fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, NULL, tdx_host_groups);
+ fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, &tdx_host_ops, tdx_host_groups);
if (!fdev)
return -ENODEV;
--
2.47.3
^ permalink raw reply related
* [PATCH v5 04/22] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
The TDX architecture uses the "SEAMCALL" instruction to communicate with
SEAM mode software. Right now, the only SEAM mode software that the kernel
communicates with is the TDX module. But, there is actually another
component that runs in SEAM mode but it is separate from the TDX module:
the persistent SEAM loader or "P-SEAMLDR". Right now, the only component
that communicates with it is the BIOS which loads the TDX module itself at
boot. But, to support updating the TDX module, the kernel now needs to be
able to talk to it.
P-SEAMLDR SEAMCALLs differ from TDX module SEAMCALLs in areas such as
concurrency requirements. Add a P-SEAMLDR wrapper to handle these
differences and prepare for implementing concrete functions.
Note that unlike P-SEAMLDR, there is also a non-persistent SEAM loader
("NP-SEAMLDR"). This is an authenticated code module (ACM) that is not
callable at runtime. Only BIOS launches it to load P-SEAMLDR at boot;
the kernel does not need to interact with it for runtime update.
For details of P-SEAMLDR SEAMCALLs, see Intel® Trust Domain CPU
Architectural Extensions, Revision 343754-002, Chapter 2.3 "INSTRUCTION
SET REFERENCE".
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Tested-by: Farrah Chen <farrah.chen@intel.com>
Link: https://cdrdv2.intel.com/v1/dl/getContent/733582 # [1]
---
v5:
- Don't save/restore irq flags as P-SEAMLDR calls are made only in process
context
- clarify why raw_spinlock is used [Dave]
v4:
- Give more background about P-SEAMLDR in changelog [Dave]
- Don't handle P-SEAMLDR's "no_entropy" error [Dave]
- Assume current VMCS is preserved across P-SEAMLDR calls [Dave]
- I'm not adding Reviewed-by tags as the code has changed significantly.
v2:
- don't create a new, inferior framework to save/restore VMCS
- use human-friendly language, just "current VMCS" rather than
SDM term "current-VMCS pointer"
- don't mix guard() with goto
---
arch/x86/virt/vmx/tdx/Makefile | 2 +-
arch/x86/virt/vmx/tdx/seamldr.c | 24 ++++++++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
create mode 100644 arch/x86/virt/vmx/tdx/seamldr.c
diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile
index 90da47eb85ee..d1dbc5cc5697 100644
--- a/arch/x86/virt/vmx/tdx/Makefile
+++ b/arch/x86/virt/vmx/tdx/Makefile
@@ -1,2 +1,2 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-y += seamcall.o tdx.o
+obj-y += seamcall.o seamldr.o tdx.o
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
new file mode 100644
index 000000000000..7ed9be89017c
--- /dev/null
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * P-SEAMLDR support for TDX module management features like runtime updates
+ *
+ * Copyright (C) 2025 Intel Corporation
+ */
+#define pr_fmt(fmt) "seamldr: " fmt
+
+#include <linux/spinlock.h>
+
+#include "seamcall_internal.h"
+
+/*
+ * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
+ * interact with P-SEAMLDR simultaneously. Use raw version as the calls can
+ * be made with interrupts disabled.
+ */
+static DEFINE_RAW_SPINLOCK(seamldr_lock);
+
+static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
+{
+ guard(raw_spinlock)(&seamldr_lock);
+ return seamcall_prerr(fn, args);
+}
--
2.47.3
^ permalink raw reply related
* [PATCH v5 05/22] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
P-SEAMLDR returns its information such as version number, in response to
the SEAMLDR.INFO SEAMCALL.
This information is useful for userspace. For example, the admin can decide
which TDX module versions are compatible with the P-SEAMLDR according to
the P-SEAMLDR version.
Retrieve P-SEAMLDR information in preparation for exposing P-SEAMLDR
version and other necessary information to userspace. Export the new kAPI
for use by tdx-host.ko.
Note that there are two distinct P-SEAMLDR APIs with similar names:
SEAMLDR.INFO: Returns a SEAMLDR_INFO structure containing SEAMLDR
information such as version and remaining updates.
SEAMLDR.SEAMINFO: Returns a SEAMLDR_SEAMINFO structure containing SEAM
and system information such as Convertible Memory
Regions (CMRs) and number of CPUs and sockets.
The former is used here.
For details, see "Intel® Trust Domain Extensions - SEAM Loader (SEAMLDR)
Interface Specification" revision 343755-003.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Tested-by: Farrah Chen <farrah.chen@intel.com>
---
Kai also suggested merging this patch with the first use of the new
kAPI, so we don't need to add a comment for slow_virt_to_phys() (as the
reason can be seen from the call site). I am fine with it, but the
changelog may be a bit lengthy.
v5:
- add a comment for slow_virt_to_phys() [Kai]
v4:
- put seamldr_info on stack [Dave]
- improve changelogs to explain SEAMLDR.INFO and SEAMLDR.SEAMINFO [Dave]
- add P-SEAMLDR spec information in the changelog [Dave]
- add proper comments above ABI structure definition [Dave]
- add unused ABI structure fields rather than marking them as reserved
to better align with the specc [Dave] (I omitted "not used by kernel"
tags since there are 5-6 such fields and maintaining these tags would
be tedious.)
---
arch/x86/include/asm/seamldr.h | 36 +++++++++++++++++++++++++++++++++
arch/x86/virt/vmx/tdx/seamldr.c | 19 ++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletion(-)
create mode 100644 arch/x86/include/asm/seamldr.h
diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
new file mode 100644
index 000000000000..c67e5bc910a9
--- /dev/null
+++ b/arch/x86/include/asm/seamldr.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_SEAMLDR_H
+#define _ASM_X86_SEAMLDR_H
+
+#include <linux/types.h>
+
+/*
+ * This is called the "SEAMLDR_INFO" data structure and is defined
+ * in "SEAM Loader (SEAMLDR) Interface Specification".
+ *
+ * The SEAMLDR.INFO documentation requires this to be aligned to a
+ * 256-byte boundary.
+ */
+struct seamldr_info {
+ u32 version;
+ u32 attributes;
+ u32 vendor_id;
+ u32 build_date;
+ u16 build_num;
+ u16 minor_version;
+ u16 major_version;
+ u16 update_version;
+ u32 acm_x2apicid;
+ u32 num_remaining_updates;
+ u8 seam_info[128];
+ u8 seam_ready;
+ u8 seam_debug;
+ u8 p_seam_ready;
+ u8 reserved[93];
+} __packed __aligned(256);
+
+static_assert(sizeof(struct seamldr_info) == 256);
+
+int seamldr_get_info(struct seamldr_info *seamldr_info);
+
+#endif /* _ASM_X86_SEAMLDR_H */
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index 7ed9be89017c..7c0cbab2c4c0 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -8,8 +8,13 @@
#include <linux/spinlock.h>
+#include <asm/seamldr.h>
+
#include "seamcall_internal.h"
+/* P-SEAMLDR SEAMCALL leaf function */
+#define P_SEAMLDR_INFO 0x8000000000000000
+
/*
* Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
* interact with P-SEAMLDR simultaneously. Use raw version as the calls can
@@ -17,8 +22,20 @@
*/
static DEFINE_RAW_SPINLOCK(seamldr_lock);
-static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
+static int seamldr_call(u64 fn, struct tdx_module_args *args)
{
guard(raw_spinlock)(&seamldr_lock);
return seamcall_prerr(fn, args);
}
+
+int seamldr_get_info(struct seamldr_info *seamldr_info)
+{
+ /*
+ * Use slow_virt_to_phys() since @seamldr_info may be allocated on
+ * the stack.
+ */
+ struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
+
+ return seamldr_call(P_SEAMLDR_INFO, &args);
+}
+EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
--
2.47.3
^ permalink raw reply related
* [PATCH v5 03/22] coco/tdx-host: Expose TDX module version
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: x86, linux-coco, kvm, linux-kernel
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
For TDX module updates, userspace needs to select compatible update
versions based on the current module version. This design delegates
module selection complexity to userspace because TDX module update
policies are complex and version series are platform-specific.
For example, the 1.5.x series is for certain platform generations, while
the 2.0.x series is intended for others. And TDX module 1.5.x may be
updated to 1.5.y but not to 1.5.y+1.
Expose the TDX module version to userspace via sysfs to aid module
selection. Since the TDX faux device will drive module updates, expose
the version as its attribute.
One bonus of exposing TDX module version via sysfs is: TDX module
version information remains available even after dmesg logs are cleared.
== Background ==
The "faux device + device attribute" approach compares to other update
mechanisms as follows:
1. AMD SEV leverages an existing PCI device for the PSP to expose
metadata. TDX uses a faux device as it doesn't have PCI device
in its architecture.
2. Microcode uses per-CPU virtual devices to report microcode revisions
because CPUs can have different revisions. But, there is only a
single TDX module, so exposing the TDX module version through a global
TDX faux device is appropriate
3. ARM's CCA implementation isn't in-tree yet, but will likely follow a
similar faux device approach, though it's unclear whether they need
to expose firmware version information
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
Link: https://lore.kernel.org/all/2025073035-bulginess-rematch-b92e@gregkh/ # [1]
---
v4:
- collect reviews
- Explain other version exposure implementations and why tdx's approach differs
from them
v3:
- Justify the sysfs ABI choice and expand background on other CoCo
implementations.
---
.../ABI/testing/sysfs-devices-faux-tdx-host | 6 +++++
drivers/virt/coco/tdx-host/tdx-host.c | 26 ++++++++++++++++++-
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ABI/testing/sysfs-devices-faux-tdx-host
diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
new file mode 100644
index 000000000000..2cf682b65acf
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -0,0 +1,6 @@
+What: /sys/devices/faux/tdx_host/version
+Contact: linux-coco@lists.linux.dev
+Description: (RO) Report the version of the loaded TDX module. The TDX module
+ version is formatted as x.y.z, where "x" is the major version,
+ "y" is the minor version and "z" is the update version. Versions
+ are used for bug reporting, TDX module updates etc.
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index c77885392b09..0424933b2560 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -8,6 +8,7 @@
#include <linux/device/faux.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
+#include <linux/sysfs.h>
#include <asm/cpu_device_id.h>
#include <asm/tdx.h>
@@ -18,6 +19,29 @@ static const struct x86_cpu_id tdx_host_ids[] = {
};
MODULE_DEVICE_TABLE(x86cpu, tdx_host_ids);
+static ssize_t version_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
+ const struct tdx_sys_info_version *ver;
+
+ if (!tdx_sysinfo)
+ return -ENXIO;
+
+ ver = &tdx_sysinfo->version;
+
+ return sysfs_emit(buf, "%u.%u.%02u\n", ver->major_version,
+ ver->minor_version,
+ ver->update_version);
+}
+static DEVICE_ATTR_RO(version);
+
+static struct attribute *tdx_host_attrs[] = {
+ &dev_attr_version.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(tdx_host);
+
static struct faux_device *fdev;
static int __init tdx_host_init(void)
@@ -25,7 +49,7 @@ static int __init tdx_host_init(void)
if (!x86_match_cpu(tdx_host_ids) || !tdx_get_sysinfo())
return -ENODEV;
- fdev = faux_device_create(KBUILD_MODNAME, NULL, NULL);
+ fdev = faux_device_create_with_groups(KBUILD_MODNAME, NULL, NULL, tdx_host_groups);
if (!fdev)
return -ENODEV;
--
2.47.3
^ permalink raw reply related
* [PATCH v5 02/22] coco/tdx-host: Introduce a "tdx_host" device
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Jonathan Cameron, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
TDX depends on a platform firmware module that is invoked via instructions
similar to vmenter (i.e. enter into a new privileged "root-mode" context to
manage private memory and private device mechanisms). It is a software
construct that depends on the CPU vmxon state to enable invocation of
TDX module ABIs. Unlike other Trusted Execution Environment (TEE) platform
implementations that employ a firmware module running on a PCI device with
an MMIO mailbox for communication, TDX has no hardware device to point to
as the TEE Secure Manager (TSM).
Create a virtual device not only to align with other implementations but
also to make it easier to
- expose metadata (e.g., TDX module version, seamldr version etc) to
the userspace as device attributes
- implement firmware uploader APIs which are tied to a device. This is
needed to support TDX module runtime updates
- enable TDX Connect which will share a common infrastructure with other
platform implementations. In the TDX Connect context, every
architecture has a TSM, represented by a PCIe or virtual device. The
new "tdx_host" device will serve the TSM role.
A faux device is used for TDX because the TDX module is singular within
the system and lacks associated platform resources. Using a faux device
eliminates the need to create a stub bus.
The call to tdx_get_sysinfo() ensures that the TDX module is ready to
provide services.
Note that AMD has a PCI device for the PSP for SEV and ARM CCA will
likely have a faux device [1].
Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Link: https://lore.kernel.org/all/2025073035-bulginess-rematch-b92e@gregkh/ # [1]
---
v3:
- add Jonathan's reviewed-by
- add tdx_get_sysinfo() in module_init() to ensure the TDX module is up
and running.
- note in the changelog that both AMD and ARM have devices for coco
---
arch/x86/virt/vmx/tdx/tdx.c | 2 +-
drivers/virt/coco/Kconfig | 2 ++
drivers/virt/coco/Makefile | 1 +
drivers/virt/coco/tdx-host/Kconfig | 10 +++++++
drivers/virt/coco/tdx-host/Makefile | 1 +
drivers/virt/coco/tdx-host/tdx-host.c | 43 +++++++++++++++++++++++++++
6 files changed, 58 insertions(+), 1 deletion(-)
create mode 100644 drivers/virt/coco/tdx-host/Kconfig
create mode 100644 drivers/virt/coco/tdx-host/Makefile
create mode 100644 drivers/virt/coco/tdx-host/tdx-host.c
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 06d9709ade85..172f6d4133b5 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1435,7 +1435,7 @@ const struct tdx_sys_info *tdx_get_sysinfo(void)
return p;
}
-EXPORT_SYMBOL_FOR_KVM(tdx_get_sysinfo);
+EXPORT_SYMBOL_FOR_MODULES(tdx_get_sysinfo, "kvm-intel,tdx-host");
u32 tdx_get_nr_guest_keyids(void)
{
diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
index df1cfaf26c65..f7691f64fbe3 100644
--- a/drivers/virt/coco/Kconfig
+++ b/drivers/virt/coco/Kconfig
@@ -17,5 +17,7 @@ source "drivers/virt/coco/arm-cca-guest/Kconfig"
source "drivers/virt/coco/guest/Kconfig"
endif
+source "drivers/virt/coco/tdx-host/Kconfig"
+
config TSM
bool
diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
index cb52021912b3..b323b0ae4f82 100644
--- a/drivers/virt/coco/Makefile
+++ b/drivers/virt/coco/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_EFI_SECRET) += efi_secret/
obj-$(CONFIG_ARM_PKVM_GUEST) += pkvm-guest/
obj-$(CONFIG_SEV_GUEST) += sev-guest/
obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
+obj-$(CONFIG_INTEL_TDX_HOST) += tdx-host/
obj-$(CONFIG_ARM_CCA_GUEST) += arm-cca-guest/
obj-$(CONFIG_TSM) += tsm-core.o
obj-$(CONFIG_TSM_GUEST) += guest/
diff --git a/drivers/virt/coco/tdx-host/Kconfig b/drivers/virt/coco/tdx-host/Kconfig
new file mode 100644
index 000000000000..d35d85ef91c0
--- /dev/null
+++ b/drivers/virt/coco/tdx-host/Kconfig
@@ -0,0 +1,10 @@
+config TDX_HOST_SERVICES
+ tristate "TDX Host Services Driver"
+ depends on INTEL_TDX_HOST
+ default m
+ help
+ Enable access to TDX host services like module update and
+ extensions (e.g. TDX Connect).
+
+ Say y or m if enabling support for confidential virtual machine
+ support (CONFIG_INTEL_TDX_HOST). The module is called tdx_host.ko.
diff --git a/drivers/virt/coco/tdx-host/Makefile b/drivers/virt/coco/tdx-host/Makefile
new file mode 100644
index 000000000000..e61e749a8dff
--- /dev/null
+++ b/drivers/virt/coco/tdx-host/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TDX_HOST_SERVICES) += tdx-host.o
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
new file mode 100644
index 000000000000..c77885392b09
--- /dev/null
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TDX host user interface driver
+ *
+ * Copyright (C) 2025 Intel Corporation
+ */
+
+#include <linux/device/faux.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+
+#include <asm/cpu_device_id.h>
+#include <asm/tdx.h>
+
+static const struct x86_cpu_id tdx_host_ids[] = {
+ X86_MATCH_FEATURE(X86_FEATURE_TDX_HOST_PLATFORM, NULL),
+ {}
+};
+MODULE_DEVICE_TABLE(x86cpu, tdx_host_ids);
+
+static struct faux_device *fdev;
+
+static int __init tdx_host_init(void)
+{
+ if (!x86_match_cpu(tdx_host_ids) || !tdx_get_sysinfo())
+ return -ENODEV;
+
+ fdev = faux_device_create(KBUILD_MODNAME, NULL, NULL);
+ if (!fdev)
+ return -ENODEV;
+
+ return 0;
+}
+module_init(tdx_host_init);
+
+static void __exit tdx_host_exit(void)
+{
+ faux_device_destroy(fdev);
+}
+module_exit(tdx_host_exit);
+
+MODULE_DESCRIPTION("TDX Host Services");
+MODULE_LICENSE("GPL");
--
2.47.3
^ permalink raw reply related
* [PATCH v5 01/22] x86/virt/tdx: Move low level SEAMCALL helpers out of <asm/tdx.h>
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: linux-kernel, linux-coco, kvm
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Zhenzhong Duan, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260315135920.354657-1-chao.gao@intel.com>
From: Kai Huang <kai.huang@intel.com>
TDX host core code implements three seamcall*() helpers to make SEAMCALL
to the TDX module. Currently, they are implemented in <asm/tdx.h> and
are exposed to other kernel code which includes <asm/tdx.h>.
However, other than the TDX host core, seamcall*() are not expected to
be used by other kernel code directly. For instance, for all SEAMCALLs
that are used by KVM, the TDX host core exports a wrapper function for
each of them.
Move seamcall*() and related code out of <asm/tdx.h> and make them only
visible to TDX host core.
Since TDX host core tdx.c is already very heavy, don't put low level
seamcall*() code there but to a new dedicated "seamcall_internal.h". Also,
currently tdx.c has seamcall_prerr*() helpers which additionally print
error message when calling seamcall*() fails. Move them to
"seamcall_internal.h" as well. In such way all low level SEAMCALL helpers
are in a dedicated place, which is much more readable.
Copy the copyright notice from the original files and consolidate the
date ranges to:
Copyright (C) 2021-2023 Intel Corporation
Signed-off-by: Kai Huang <kai.huang@intel.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
---
v5:
- s/seamcall.h/seamcall_internal.h [Binbin]
- Fix an unintentional change to sc_retry() during code movement.
v4:
- Collect reviews
- add "internal" to the new header file [Dave]
- document the scope of the new header file [Dave]
- correct the copyright notice [Dave]
v2:
- new
---
arch/x86/include/asm/tdx.h | 47 ----------
arch/x86/virt/vmx/tdx/seamcall_internal.h | 109 ++++++++++++++++++++++
arch/x86/virt/vmx/tdx/tdx.c | 47 +---------
3 files changed, 111 insertions(+), 92 deletions(-)
create mode 100644 arch/x86/virt/vmx/tdx/seamcall_internal.h
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 6b338d7f01b7..cb2219302dfc 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -97,54 +97,7 @@ static inline long tdx_kvm_hypercall(unsigned int nr, unsigned long p1,
#endif /* CONFIG_INTEL_TDX_GUEST && CONFIG_KVM_GUEST */
#ifdef CONFIG_INTEL_TDX_HOST
-u64 __seamcall(u64 fn, struct tdx_module_args *args);
-u64 __seamcall_ret(u64 fn, struct tdx_module_args *args);
-u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
void tdx_init(void);
-
-#include <linux/preempt.h>
-#include <asm/archrandom.h>
-#include <asm/processor.h>
-
-typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
-
-static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
- struct tdx_module_args *args)
-{
- lockdep_assert_preemption_disabled();
-
- /*
- * SEAMCALLs are made to the TDX module and can generate dirty
- * cachelines of TDX private memory. Mark cache state incoherent
- * so that the cache can be flushed during kexec.
- *
- * This needs to be done before actually making the SEAMCALL,
- * because kexec-ing CPU could send NMI to stop remote CPUs,
- * in which case even disabling IRQ won't help here.
- */
- this_cpu_write(cache_state_incoherent, true);
-
- return func(fn, args);
-}
-
-static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
- struct tdx_module_args *args)
-{
- int retry = RDRAND_RETRY_LOOPS;
- u64 ret;
-
- do {
- preempt_disable();
- ret = __seamcall_dirty_cache(func, fn, args);
- preempt_enable();
- } while (ret == TDX_RND_NO_ENTROPY && --retry);
-
- return ret;
-}
-
-#define seamcall(_fn, _args) sc_retry(__seamcall, (_fn), (_args))
-#define seamcall_ret(_fn, _args) sc_retry(__seamcall_ret, (_fn), (_args))
-#define seamcall_saved_ret(_fn, _args) sc_retry(__seamcall_saved_ret, (_fn), (_args))
int tdx_cpu_enable(void);
int tdx_enable(void);
const char *tdx_dump_mce_info(struct mce *m);
diff --git a/arch/x86/virt/vmx/tdx/seamcall_internal.h b/arch/x86/virt/vmx/tdx/seamcall_internal.h
new file mode 100644
index 000000000000..be5f446467df
--- /dev/null
+++ b/arch/x86/virt/vmx/tdx/seamcall_internal.h
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * SEAMCALL utilities for TDX host-side operations.
+ *
+ * Provides convenient wrappers around SEAMCALL assembly with retry logic,
+ * error reporting and cache coherency tracking.
+ *
+ * Copyright (C) 2021-2023 Intel Corporation
+ */
+
+#ifndef _X86_VIRT_SEAMCALL_INTERNAL_H
+#define _X86_VIRT_SEAMCALL_INTERNAL_H
+
+#include <linux/printk.h>
+#include <linux/types.h>
+#include <asm/archrandom.h>
+#include <asm/processor.h>
+#include <asm/tdx.h>
+
+u64 __seamcall(u64 fn, struct tdx_module_args *args);
+u64 __seamcall_ret(u64 fn, struct tdx_module_args *args);
+u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
+
+typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
+
+static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
+ struct tdx_module_args *args)
+{
+ lockdep_assert_preemption_disabled();
+
+ /*
+ * SEAMCALLs are made to the TDX module and can generate dirty
+ * cachelines of TDX private memory. Mark cache state incoherent
+ * so that the cache can be flushed during kexec.
+ *
+ * This needs to be done before actually making the SEAMCALL,
+ * because kexec-ing CPU could send NMI to stop remote CPUs,
+ * in which case even disabling IRQ won't help here.
+ */
+ this_cpu_write(cache_state_incoherent, true);
+
+ return func(fn, args);
+}
+
+static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
+ struct tdx_module_args *args)
+{
+ int retry = RDRAND_RETRY_LOOPS;
+ u64 ret;
+
+ do {
+ preempt_disable();
+ ret = __seamcall_dirty_cache(func, fn, args);
+ preempt_enable();
+ } while (ret == TDX_RND_NO_ENTROPY && --retry);
+
+ return ret;
+}
+
+#define seamcall(_fn, _args) sc_retry(__seamcall, (_fn), (_args))
+#define seamcall_ret(_fn, _args) sc_retry(__seamcall_ret, (_fn), (_args))
+#define seamcall_saved_ret(_fn, _args) sc_retry(__seamcall_saved_ret, (_fn), (_args))
+
+typedef void (*sc_err_func_t)(u64 fn, u64 err, struct tdx_module_args *args);
+
+static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
+{
+ pr_err("SEAMCALL (0x%016llx) failed: 0x%016llx\n", fn, err);
+}
+
+static inline void seamcall_err_ret(u64 fn, u64 err,
+ struct tdx_module_args *args)
+{
+ seamcall_err(fn, err, args);
+ pr_err("RCX 0x%016llx RDX 0x%016llx R08 0x%016llx\n",
+ args->rcx, args->rdx, args->r8);
+ pr_err("R09 0x%016llx R10 0x%016llx R11 0x%016llx\n",
+ args->r9, args->r10, args->r11);
+}
+
+static __always_inline int sc_retry_prerr(sc_func_t func,
+ sc_err_func_t err_func,
+ u64 fn, struct tdx_module_args *args)
+{
+ u64 sret = sc_retry(func, fn, args);
+
+ if (sret == TDX_SUCCESS)
+ return 0;
+
+ if (sret == TDX_SEAMCALL_VMFAILINVALID)
+ return -ENODEV;
+
+ if (sret == TDX_SEAMCALL_GP)
+ return -EOPNOTSUPP;
+
+ if (sret == TDX_SEAMCALL_UD)
+ return -EACCES;
+
+ err_func(fn, sret, args);
+ return -EIO;
+}
+
+#define seamcall_prerr(__fn, __args) \
+ sc_retry_prerr(__seamcall, seamcall_err, (__fn), (__args))
+
+#define seamcall_prerr_ret(__fn, __args) \
+ sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
+
+#endif /* _X86_VIRT_SEAMCALL_INTERNAL_H */
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 8b8e165a2001..06d9709ade85 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -39,6 +39,8 @@
#include <asm/cpu_device_id.h>
#include <asm/processor.h>
#include <asm/mce.h>
+
+#include "seamcall_internal.h"
#include "tdx.h"
static u32 tdx_global_keyid __ro_after_init;
@@ -59,51 +61,6 @@ static LIST_HEAD(tdx_memlist);
static struct tdx_sys_info tdx_sysinfo;
-typedef void (*sc_err_func_t)(u64 fn, u64 err, struct tdx_module_args *args);
-
-static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
-{
- pr_err("SEAMCALL (0x%016llx) failed: 0x%016llx\n", fn, err);
-}
-
-static inline void seamcall_err_ret(u64 fn, u64 err,
- struct tdx_module_args *args)
-{
- seamcall_err(fn, err, args);
- pr_err("RCX 0x%016llx RDX 0x%016llx R08 0x%016llx\n",
- args->rcx, args->rdx, args->r8);
- pr_err("R09 0x%016llx R10 0x%016llx R11 0x%016llx\n",
- args->r9, args->r10, args->r11);
-}
-
-static __always_inline int sc_retry_prerr(sc_func_t func,
- sc_err_func_t err_func,
- u64 fn, struct tdx_module_args *args)
-{
- u64 sret = sc_retry(func, fn, args);
-
- if (sret == TDX_SUCCESS)
- return 0;
-
- if (sret == TDX_SEAMCALL_VMFAILINVALID)
- return -ENODEV;
-
- if (sret == TDX_SEAMCALL_GP)
- return -EOPNOTSUPP;
-
- if (sret == TDX_SEAMCALL_UD)
- return -EACCES;
-
- err_func(fn, sret, args);
- return -EIO;
-}
-
-#define seamcall_prerr(__fn, __args) \
- sc_retry_prerr(__seamcall, seamcall_err, (__fn), (__args))
-
-#define seamcall_prerr_ret(__fn, __args) \
- sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
-
/*
* Do the module global initialization once and return its result.
* It can be done on any cpu. It's always called with interrupts
--
2.47.3
^ permalink raw reply related
* [PATCH v5 00/22] Runtime TDX module update support
From: Chao Gao @ 2026-03-15 13:58 UTC (permalink / raw)
To: kvm, linux-coco, linux-doc, linux-kernel, x86
Cc: binbin.wu, dan.j.williams, dave.hansen, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, Chao Gao, Borislav Petkov, H. Peter Anvin, Ingo Molnar,
Jonathan Corbet, Shuah Khan, Thomas Gleixner
Hi Reviewers,
With this posting, I'm hoping to collect more Reviewed-by or Acked-by tags.
Please note these changes:
Patch 18 handles a CPU erratum that clears the active VMCS after
P-SEAMLDR calls. Because of this erratum, patch 6 now exposes seamldr
attributes during device probe rather than creation as unconditional
exposure would be unsafe.
Patch 22 adds error logging for update failures. It's kind of
nice-to-have, so it is placed last for easy removal if necessary.
For transparency, I should note that I used an Intel-operated AI tool to
help proofread this cover-letter and commit messages.
Changelog:
v4->v5:
- s/TDX Module/TDX module/g [Binbin/Dave]
- drop is_vmalloc_addr() checking [Dave/Rick]
- protect lockstep control data with a lock [Dave]
- clarify why raw_spinlock is used [Dave/Kai]
- drop patches that check all CPUs are online and updates are not exhausted [Dave]
- register seamldr attributes in device probe
- use devm_add_action_or_reset for seamldr deinit [Yilun]
- remove global tdx_fw [Yilun]
- clarify request_firmware() doesn't take filename from userspace [Rick]
- drop unnecessary checks when populating an update request [Rick]
- rewrite the commit message for the skeleton patch
- rewrite the commit message for the "update-sensitive operations" handling patch
- other minor code changes, changelog improvements and typo fixes [Binbin/Yan etc]
- collect review tags from Yilun/Rick/Kai/Binbin
- v4: https://lore.kernel.org/kvm/20260212143606.534586-1-chao.gao@intel.com/
This series adds support for runtime TDX module updates that preserve
running TDX guests. It is also available at:
https://github.com/gaochaointel/linux-dev/commits/tdx-module-updates-v5/
== Background ==
Intel TDX isolates Trusted Domains (TDs), or confidential guests, from the
host. A key component of Intel TDX is the TDX module, which enforces
security policies to protect the memory and CPU states of TDs from the
host. However, the TDX module is software that requires updates.
== Problems ==
Currently, the TDX module is loaded by the BIOS at boot time, and the only
way to update it is through a reboot, which results in significant system
downtime. Users expect the TDX module to be updatable at runtime without
disrupting TDX guests.
== Solution ==
On TDX platforms, P-SEAMLDR[1] is a component within the protected SEAM
range. It is loaded by the BIOS and provides the host with functions to
install a TDX module at runtime.
Implement a TDX module update facility via the fw_upload mechanism. Given
that there is variability in which module update to load based on features,
fix levels, and potentially reloading the same version for error recovery
scenarios, the explicit userspace chosen payload flexibility of fw_upload
is attractive.
This design allows the kernel to accept a bitstream instead of loading a
named file from the filesystem, as the module selection and policy
enforcement for TDX modules are quite complex (see patch "coco/tdx-host:
Implement firmware upload sysfs ABI for TDX module updates"). By doing
so, much of this complexity is shifted out of the kernel. The kernel
needs to expose information, such as the TDX module version, to
userspace. Userspace must understand the TDX module versioning scheme
and update policy to select the appropriate TDX module (see "TDX module
Versioning" below).
In the unlikely event the update fails, for example userspace picks an
incompatible update image, or the image is otherwise corrupted, all TDs
will experience SEAMCALL failures and be killed. The recovery of TD
operation from that event requires a reboot.
Given there is no mechanism to quiesce SEAMCALLs, the TDs themselves must
pause execution over an update. The most straightforward way to meet the
'pause TDs while update executes' constraint is to run the update in
stop_machine() context. All other evaluated solutions export more
complexity to KVM, or exports more fragility to userspace.
== How to test this series ==
First, load kvm-intel.ko and tdx-host.ko if they haven't been loaded:
# modprobe -r kvm_intel
# modprobe kvm_intel tdx=1
# modprobe tdx-host
Then, use the userspace tool below to select the appropriate TDX module and
install it via the interfaces exposed by this series:
# git clone https://github.com/intel/tdx-module-binaries
# cd tdx-module-binaries
# python version_select_and_load.py --update
this version changes the firmware directory name from seamldr_upload to
tdx_module, so, below change should be applied to version_select_and_load.py:
diff --git a/version_select_and_load.py b/version_select_and_load.py
index 2193bd8..6a3b604 100644
--- a/version_select_and_load.py
+++ b/version_select_and_load.py
@@ -38,7 +38,7 @@ except ImportError:
print("Error: cpuid module is not installed. Please install it using 'pip install cpuid'")
sys.exit(1)
-FIRMWARE_PATH = "/sys/class/firmware/seamldr_upload"
+FIRMWARE_PATH = "/sys/class/firmware/tdx_module"
MODULE_PATH = "/sys/devices/faux/tdx_host"
SEAMLDR_PATH = "/sys/devices/faux/tdx_host/seamldr"
allow_debug = False
== Other information relevant to Runtime TDX module updates ==
=== TDX module versioning ===
Each TDX module is assigned a version number x.y.z, where x represents the
"major" version, y the "minor" version, and z the "update" version.
Runtime TDX module updates are restricted to Z-stream releases.
Note that Z-stream releases do not necessarily guarantee compatibility. A
new release may not be compatible with all previous versions. To address this,
Intel provides a separate file containing compatibility information, which
specifies the minimum module version required for a particular update. This
information is referenced by the tool to determine if two modules are
compatible.
=== TCB Stability ===
Updates change the TCB as viewed by attestation reports. In TDX there is
a distinction between launch-time version and current version where
runtime TDX module updates cause that latter version number to change,
subject to Z-stream constraints.
The concern that a malicious host may attack confidential VMs by loading
insecure updates was addressed by Alex in [3]. Similarly, the scenario
where some "theoretical paranoid tenant" in the cloud wants to audit
updates and stop trusting the host after updates until audit completion
was also addressed in [4]. Users not in the cloud control the host machine
and can manage updates themselves, so they don't have these concerns.
See more about the implications of current TCB version changes in
attestation as summarized by Dave in [5].
=== TDX module Distribution Model ===
At a high level, Intel publishes all TDX modules on the github [2], along
with a mapping_file.json which documents the compatibility information
about each TDX module and a userspace tool to install the TDX module. OS
vendors can package these modules and distribute them. Administrators
install the package and use the tool to select the appropriate TDX module
and install it via the interfaces exposed by this series.
[1]: https://cdrdv2.intel.com/v1/dl/getContent/733584
[2]: https://github.com/intel/tdx-module-binaries
[3]: https://lore.kernel.org/all/665c5ae0-4b7c-4852-8995-255adf7b3a2f@amazon.com/
[4]: https://lore.kernel.org/all/5d1da767-491b-4077-b472-2cc3d73246d6@amazon.com/
[5]: https://lore.kernel.org/all/94d6047e-3b7c-4bc1-819c-85c16ff85abf@intel.com/
Chao Gao (21):
coco/tdx-host: Introduce a "tdx_host" device
coco/tdx-host: Expose TDX module version
x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
x86/virt/seamldr: Retrieve P-SEAMLDR information
coco/tdx-host: Expose P-SEAMLDR information via sysfs
coco/tdx-host: Implement firmware upload sysfs ABI for TDX module
updates
x86/virt/seamldr: Allocate and populate a module update request
x86/virt/seamldr: Introduce skeleton for TDX module updates
x86/virt/seamldr: Abort updates if errors occurred midway
x86/virt/seamldr: Shut down the current TDX module
x86/virt/tdx: Reset software states during TDX module shutdown
x86/virt/seamldr: Install a new TDX module
x86/virt/seamldr: Do TDX per-CPU initialization after updates
x86/virt/tdx: Restore TDX module state
x86/virt/tdx: Update tdx_sysinfo and check features post-update
x86/virt/tdx: Avoid updates during update-sensitive operations
coco/tdx-host: Don't expose P-SEAMLDR features on CPUs with erratum
x86/virt/tdx: Enable TDX module runtime updates
coco/tdx-host: Document TDX module update compatibility criteria
x86/virt/tdx: Document TDX module update
x86/virt/seamldr: Log TDX module update failures
Kai Huang (1):
x86/virt/tdx: Move low level SEAMCALL helpers out of <asm/tdx.h>
.../ABI/testing/sysfs-devices-faux-tdx-host | 75 ++++
Documentation/arch/x86/tdx.rst | 36 ++
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/seamldr.h | 37 ++
arch/x86/include/asm/tdx.h | 65 +---
arch/x86/include/asm/tdx_global_metadata.h | 5 +
arch/x86/include/asm/vmx.h | 1 +
arch/x86/kvm/vmx/tdx_errno.h | 2 -
arch/x86/virt/vmx/tdx/Makefile | 2 +-
arch/x86/virt/vmx/tdx/seamcall_internal.h | 109 ++++++
arch/x86/virt/vmx/tdx/seamldr.c | 325 ++++++++++++++++++
arch/x86/virt/vmx/tdx/tdx.c | 165 ++++++---
arch/x86/virt/vmx/tdx/tdx.h | 11 +-
arch/x86/virt/vmx/tdx/tdx_global_metadata.c | 18 +
drivers/virt/coco/Kconfig | 2 +
drivers/virt/coco/Makefile | 1 +
drivers/virt/coco/tdx-host/Kconfig | 12 +
drivers/virt/coco/tdx-host/Makefile | 1 +
drivers/virt/coco/tdx-host/tdx-host.c | 228 ++++++++++++
19 files changed, 995 insertions(+), 101 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-devices-faux-tdx-host
create mode 100644 arch/x86/include/asm/seamldr.h
create mode 100644 arch/x86/virt/vmx/tdx/seamcall_internal.h
create mode 100644 arch/x86/virt/vmx/tdx/seamldr.c
create mode 100644 drivers/virt/coco/tdx-host/Kconfig
create mode 100644 drivers/virt/coco/tdx-host/Makefile
create mode 100644 drivers/virt/coco/tdx-host/tdx-host.c
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] coco/guest: Remove unneeded selection of CRYPTO
From: Dan Williams @ 2026-03-14 22:04 UTC (permalink / raw)
To: Eric Biggers, Dan Williams, linux-coco
In-Reply-To: <20260314205533.GA45660@quark>
Eric Biggers wrote:
> On Thu, Jan 08, 2026 at 06:26:22PM -0800, Eric Biggers wrote:
> > On Wed, Dec 03, 2025 at 09:55:12PM -0800, Eric Biggers wrote:
> > > All that's needed here is CRYPTO_HASH_INFO. It used to be the case that
> > > CRYPTO_HASH_INFO was visible only when CRYPTO, but that was fixed by
> > > commit aacb37f597d0 ("lib/crypto: hash_info: Move hash_info.c into
> > > lib/crypto/"). Now CRYPTO_HASH_INFO can be selected directly.
> > >
> > > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > > ---
> > > drivers/virt/coco/guest/Kconfig | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/virt/coco/guest/Kconfig b/drivers/virt/coco/guest/Kconfig
> > > index 3d5e1d05bf34..da570dc4bd48 100644
> > > --- a/drivers/virt/coco/guest/Kconfig
> > > +++ b/drivers/virt/coco/guest/Kconfig
> > > @@ -11,7 +11,6 @@ config TSM_REPORTS
> > > tristate
> > >
> > > config TSM_MEASUREMENTS
> > > select TSM_GUEST
> > > select CRYPTO_HASH_INFO
> > > - select CRYPTO
> > > bool
> > >
> >
> > Any interest in applying this patch?
>
> Ping.
>
> If there continues to be no response, I'll take this patch via
> libcrypto-next.
Apologies, not sure how I missed this.
Acked-by: Dan Williams <dan.j.williams@intel.com>
...and yes, fine for this to go through libcrypto-next.
^ permalink raw reply
* Re: [PATCH] coco/guest: Remove unneeded selection of CRYPTO
From: Eric Biggers @ 2026-03-14 20:55 UTC (permalink / raw)
To: Dan Williams, linux-coco
In-Reply-To: <20260109022620.GB2790@sol>
On Thu, Jan 08, 2026 at 06:26:22PM -0800, Eric Biggers wrote:
> On Wed, Dec 03, 2025 at 09:55:12PM -0800, Eric Biggers wrote:
> > All that's needed here is CRYPTO_HASH_INFO. It used to be the case that
> > CRYPTO_HASH_INFO was visible only when CRYPTO, but that was fixed by
> > commit aacb37f597d0 ("lib/crypto: hash_info: Move hash_info.c into
> > lib/crypto/"). Now CRYPTO_HASH_INFO can be selected directly.
> >
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> > drivers/virt/coco/guest/Kconfig | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/virt/coco/guest/Kconfig b/drivers/virt/coco/guest/Kconfig
> > index 3d5e1d05bf34..da570dc4bd48 100644
> > --- a/drivers/virt/coco/guest/Kconfig
> > +++ b/drivers/virt/coco/guest/Kconfig
> > @@ -11,7 +11,6 @@ config TSM_REPORTS
> > tristate
> >
> > config TSM_MEASUREMENTS
> > select TSM_GUEST
> > select CRYPTO_HASH_INFO
> > - select CRYPTO
> > bool
> >
>
> Any interest in applying this patch?
Ping.
If there continues to be no response, I'll take this patch via
libcrypto-next.
- Eric
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Lukas Wunner @ 2026-03-14 18:37 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, jgg, Donald Hunter, Jakub Kicinski
In-Reply-To: <20260303000207.1836586-9-dan.j.williams@intel.com>
On Mon, Mar 02, 2026 at 04:01:56PM -0800, Dan Williams wrote:
> +definitions:
> + -
> + type: const
> + name: max-object-size
> + value: 0x01000000
[...]
> + -
> + name: val
> + type: binary
> + checks:
> + max-len: max-obj-size
The length of a netlink attribute is a 16-bit value, so a 16 MByte value
(0x01000000) won't fit.
Moreover you're referencing max-obj-size but are defining max-object-size.
This doesn't look like it's ever been tested, so at the very least
it should be marked RFC in the subject to convey that it's not yet
in a cut-and-dried state.
The two top-most commits on my development branch have solved the
size problem and may serve as a template:
https://github.com/l1k/linux/commits/doe
Thanks,
Lukas
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Jakub Kicinski @ 2026-03-14 18:12 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, jgg, Donald Hunter
In-Reply-To: <20260303000207.1836586-9-dan.j.williams@intel.com>
On Mon, 2 Mar 2026 16:01:56 -0800 Dan Williams wrote:
> The implementation adheres to the guideline from:
> Documentation/userspace-api/netlink/genetlink-legacy.rst
>
> New Netlink families should never respond to a DO operation with
> multiple replies, with ``NLM_F_MULTI`` set. Use a filtered dump
> instead.
My understanding of F_MULTI is that deserializer is supposed to
continue deserializing into current object. IOW if we have:
struct does_this {
int really;
int have_to;
int be_netlink;
};
You can send "really" and "be_netlink" in one message and "have_to"
in the next, and receiver should reconstruct them into a single struct.
If F_MULTI is not set - receiver assumes that the next message is a new
struct. And the whole dump returns a list of structs.
So IOW I think what you're doing is a bit too.. inventive.
Do you have plans to add more commands?
The read-only stuff feels like it could be a sysfs API?
The main strength of Netlink is "do" commands with multiple optional
attrs.
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Dan Williams @ 2026-03-14 1:32 UTC (permalink / raw)
To: Jason Gunthorpe, Dan Williams
Cc: Greg KH, linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, Christoph Hellwig, Marek Szyprowski,
Robin Murphy, Roman Kisel, Samuel Ortiz, Rafael J. Wysocki,
Danilo Krummrich
In-Reply-To: <20260313202421.GG1586734@nvidia.com>
Jason Gunthorpe wrote:
> On Fri, Mar 13, 2026 at 12:56:07PM -0700, Dan Williams wrote:
> > > 0 Blocked and disabled
> > > The device cannot attack the system, enforced by the OS not loading a
> > > driver or mapping the MMIO and IOMMU fully blocking everything from it.
> >
> > In terms of details I am trying to think through whether the device
> > actually changes its ->trust level in reaction to a driver attaching, or
> > whether the block and disabled state is implicit in not being driver
> > bound.
>
> I am thinking of it as an independent property. When the device is
> first discovered it gets a level by default, userspace can change the
> level but only when not bound. The level restricts what the kernel
> will do with the device, 0 would mean "do not allow a driver to bind"
The problem is that for all the buses that do not currently have a
"device authorization" concept only userspace can decide that a device
should skip bind by default. For that, I propose module autoprobe policy
[1]. Not yet convinced the kernel needs its own per-device "no bind"
policy.
However, I do think userspace would like to know if the IOMMU subsystem
has blocked device DMA while unbound.
[1]: http://lore.kernel.org/20260303000207.1836586-6-dan.j.williams@intel.com
> > > 1 In use, attacks from a hostile device are possible
> > > A driver can operate the device and is expected to defend against
> > > attacks from the device itself. The IOMMU restricts the device to only
> > > access driver approved data (no ATS, DMA strict only, CC shared
> > > only, interrupt remapping security, bounce partial DMA mappings, etc)
> >
> > This is a better way to convey the current "force_swiotlb" settings that
> > TVMs deploy in their arch code.
>
> SWIOTLB that is needed to make the DMA API work because the device
> cannot reach CC private memory is orthogonal - the TDISP state (or
> lack of) should directly drive that in the DMA API.
>
> The DMA API just wants a flag in the struct device that says if the
> device can access encrypted memory or only decrypted.
You mean separate "trusted to access private" and "currently enabled to
access private" properties? I am trying to think of a situation where
"dev->trust >= 3" and a flag saying "disable bouncing for encrypted
memory" would ever disagree.
> > I am assuming that each bus implementation may have a different way to
> > get the device to the various trust levels.
>
> I was actually thinking no, it is just a generic orthogonal driver
> core property.
Property? Agreed. uAPI? Not so sure...
> > For example, the uAPI for PCI TDISP requires associating a device with a
> > TSM and asking the TSM to push the device to trust level 3.
>
> The other way, you can't get to level 3 unless the TSM subsystem ACK's
> it. So TSM independently does its bit then userspace can set the level
> to 3.
That bit though has lock-to-run consistency expectations. So if the
kernel does not yet fully trust the device by time the relying party is
satisfied, and the uAPI to transition the device into the TCB (level 3)
is driver-core generic it raises TOCTOU issues in my mind. The
driver-core would need to ask the bus "user now trusts this device, do
you?".
Aneesh and I are currently debating on Discord whether the kernel needs
to protect against guest userspace confusing itself. Part of me says no,
especially with sysfs, if multiple threads are racing "unlock,
update/re-measure, lock, accept", then userspace gets to keep the
pieces.
However, to Aneesh's point we could protect against that with a
transactional uAPI like netlink that can express "trust if and only if
the device has not been relocked before final accept" by passing a
cookie obtained at lock to accept. That would be awkward to coordinate
with driver-core generic uAPI for trust.
> If it sets RUN and 2 that should work and have some kind of meaning,
> just not be super useful.
>
> > Another bus like thunderbolt may want to imply that "authorized"
> > that uses challenge response (tb_domain_challenge_switch_key)
> > enables trust level 2, but otherwise only enables trust level 1.
>
> For thunderbolt/hot plug I imagine the kernel would default all
> devices to level 0. Userspace would do its thing, using whatever other
> uAPIs, and then set the level to 1 or 2. Then the driver starts.
>
> This way nothing is coupled and the kernel can offer all kinds of
> different uAPI for device verification. Userspaces picks the
> appropriate one and acks it with the level change.
Thunderbolt already has authorized uAPI. I expect adding dev->trust
support to thunderbolt is more related to ATS privilege and private
memory privilege.
> > Yes, no mitigations against spoofing the device interface without TDISP.
> > However, I would also assume that level 2 is the ATS-on trust level
> > outside of TDISP cases.
>
> Yes, level 2 would be the break where the device is required to not do
> wild PCIe packets to maintain kernel integrity.
>
> > > #2 can happen in bare metal where a OS may activate link encryption
> > > and attest the device, but doesn't have CC private/shared memory.
> >
> > Bare metal would still need to figure out how to send T=1 MMIO cycles
> > and check with some boot attestation that it can trust its MMIO mappings
> > are indeed targeting the device. So let's say trust level 2 is
> > everything but private MMIO and private DMA.
>
> bare metal has no T=1 and no "private" at all. It just sets up link
> encryption, excludes a MIM, attests the peer, then opens the iommu.
Ok, we are on the same page as to what a theoretical level 2 would mean.
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Jason Gunthorpe @ 2026-03-13 20:24 UTC (permalink / raw)
To: Dan Williams
Cc: Greg KH, linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, Christoph Hellwig, Marek Szyprowski,
Robin Murphy, Roman Kisel, Samuel Ortiz, Rafael J. Wysocki,
Danilo Krummrich
In-Reply-To: <69b46bd7935d9_b2b6100b7@dwillia2-mobl4.notmuch>
On Fri, Mar 13, 2026 at 12:56:07PM -0700, Dan Williams wrote:
> > 0 Blocked and disabled
> > The device cannot attack the system, enforced by the OS not loading a
> > driver or mapping the MMIO and IOMMU fully blocking everything from it.
>
> In terms of details I am trying to think through whether the device
> actually changes its ->trust level in reaction to a driver attaching, or
> whether the block and disabled state is implicit in not being driver
> bound.
I am thinking of it as an independent property. When the device is
first discovered it gets a level by default, userspace can change the
level but only when not bound. The level restricts what the kernel
will do with the device, 0 would mean "do not allow a driver to bind"
> > 1 In use, attacks from a hostile device are possible
> > A driver can operate the device and is expected to defend against
> > attacks from the device itself. The IOMMU restricts the device to only
> > access driver approved data (no ATS, DMA strict only, CC shared
> > only, interrupt remapping security, bounce partial DMA mappings, etc)
>
> This is a better way to convey the current "force_swiotlb" settings that
> TVMs deploy in their arch code.
SWIOTLB that is needed to make the DMA API work because the device
cannot reach CC private memory is orthogonal - the TDISP state (or
lack of) should directly drive that in the DMA API.
The DMA API just wants a flag in the struct device that says if the
device can access encrypted memory or only decrypted.
> I am assuming that each bus implementation may have a different way to
> get the device to the various trust levels.
I was actually thinking no, it is just a generic orthogonal driver
core property.
> For example, the uAPI for PCI TDISP requires associating a device with a
> TSM and asking the TSM to push the device to trust level 3.
The other way, you can't get to level 3 unless the TSM subsystem ACK's
it. So TSM independently does its bit then userspace can set the level
to 3.
If it sets RUN and 2 that should work and have some kind of meaning,
just not be super useful.
> Another bus like thunderbolt may want to imply that "authorized"
> that uses challenge response (tb_domain_challenge_switch_key)
> enables trust level 2, but otherwise only enables trust level 1.
For thunderbolt/hot plug I imagine the kernel would default all
devices to level 0. Userspace would do its thing, using whatever other
uAPIs, and then set the level to 1 or 2. Then the driver starts.
This way nothing is coupled and the kernel can offer all kinds of
different uAPI for device verification. Userspaces picks the
appropriate one and acks it with the level change.
> Yes, no mitigations against spoofing the device interface without TDISP.
> However, I would also assume that level 2 is the ATS-on trust level
> outside of TDISP cases.
Yes, level 2 would be the break where the device is required to not do
wild PCIe packets to maintain kernel integrity.
> > #2 can happen in bare metal where a OS may activate link encryption
> > and attest the device, but doesn't have CC private/shared memory.
>
> Bare metal would still need to figure out how to send T=1 MMIO cycles
> and check with some boot attestation that it can trust its MMIO mappings
> are indeed targeting the device. So let's say trust level 2 is
> everything but private MMIO and private DMA.
bare metal has no T=1 and no "private" at all. It just sets up link
encryption, excludes a MIM, attests the peer, then opens the iommu.
Jason
^ permalink raw reply
* Re: [PATCH v2 1/3] cpu/bugs: Allow forcing Automatic IBRS with SNP enabled using spectre_v2=eibrs
From: Pawan Gupta @ 2026-03-13 20:04 UTC (permalink / raw)
To: Kim Phillips
Cc: linux-kernel, kvm, linux-coco, x86, Sean Christopherson,
Paolo Bonzini, K Prateek Nayak, Nikunj A Dadhania, Tom Lendacky,
Michael Roth, Borislav Petkov, Borislav Petkov, Naveen Rao,
David Kaplan, stable
In-Reply-To: <20260311130611.2201214-2-kim.phillips@amd.com>
On Wed, Mar 11, 2026 at 08:06:09AM -0500, Kim Phillips wrote:
> To allow this, do the SNP check in spectre_v2_select_mitigation()
> processing instead of the original commit's implementation in
> cpu_set_bug_bits().
>
> Since SPECTRE_V2_CMD_AUTO logic falls through to SPECTRE_V2_CMD_FORCE,
> double-check if SPECTRE_V2_CMD_FORCE is used before allowing
> SPECTRE_V2_EIBRS with SNP enabled.
>
> Also mute SPECTRE_V2_IBRS_PERF_MSG if SNP is enabled on an AutoIBRS
> capable machine, since, in that case, the message doesn't apply.
>
> Fixes: acaa4b5c4c85 ("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
> Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Borislav Petkov (AMD) <bp@alien8.de>
> Cc: stable@kernel.org
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>
> ---
> v2:
> - Address Dave Hansen's comment to adhere to using the IBRS_ENHANCED
> Intel feature flag also for AutoIBRS.
>
> v1:
> https://lore.kernel.org/kvm/20260224180157.725159-2-kim.phillips@amd.com/
>
> arch/x86/kernel/cpu/bugs.c | 12 ++++++++++--
> arch/x86/kernel/cpu/common.c | 6 +-----
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 83f51cab0b1e..957e0df38d90 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -2181,7 +2181,14 @@ static void __init spectre_v2_select_mitigation(void)
> break;
> fallthrough;
> case SPECTRE_V2_CMD_FORCE:
> - if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> + /*
> + * Unless forced, don't use AutoIBRS when SNP is enabled
> + * because it degrades host userspace indirect branch performance.
> + */
> + if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) &&
> + (!boot_cpu_has(X86_FEATURE_SEV_SNP) ||
> + (boot_cpu_has(X86_FEATURE_SEV_SNP) &&
> + spectre_v2_cmd == SPECTRE_V2_CMD_FORCE))) {
This is forcing AutoIBRS when spectre_v2=on (meaning force), but the
subject says to allow forcing with spectre_v2=eibrs, which one is it?
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Dan Williams @ 2026-03-13 19:56 UTC (permalink / raw)
To: Jason Gunthorpe, Dan Williams
Cc: Greg KH, linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, Christoph Hellwig, Marek Szyprowski,
Robin Murphy, Roman Kisel, Samuel Ortiz, Rafael J. Wysocki,
Danilo Krummrich
In-Reply-To: <20260313133235.GC1586734@nvidia.com>
Jason Gunthorpe wrote:
> On Thu, Mar 12, 2026 at 09:11:32PM -0700, Dan Williams wrote:
> > Greg KH wrote:
> > > On Mon, Mar 02, 2026 at 04:01:51PM -0800, Dan Williams wrote:
> > > > An "accepted" device is one that is allowed to access private memory within
> > > > a Trusted Computing Boundary (TCB). The concept of "acceptance" is distinct
> > > > from other device properties like usb_device::authorized, or
> > > > tb_switch::authorized. The entry to the accepted state is a violent
> > > > operation in which the device will reject MMIO requests that are not
> > > > encrypted, and the device enters a new IOMMU protection domain to allow it
> > > > to access addresses that were previously off-limits.
> > >
> > > Trying to mix/match "acceptance" with "authorized" is going to be a
> > > nightmare, what's the combination that can happen here over time?
> >
> > I do think Linux needs to mix/match these concepts. "Authorization" is a
> > kernel policy to operate a device at all. "Acceptance" is a mechanism
> > to operate a device within a hardware TCB boundary.
>
> I'm not sure about these words either, I would revise your table to be
> more OS centric, the device can be in one of four security levels:
I like this framing.
> 0 Blocked and disabled
> The device cannot attack the system, enforced by the OS not loading a
> driver or mapping the MMIO and IOMMU fully blocking everything from it.
In terms of details I am trying to think through whether the device
actually changes its ->trust level in reaction to a driver attaching, or
whether the block and disabled state is implicit in not being driver
bound.
It does strike me that this value could be used to convey whether a
given arch's IOMMU driver indeed arranges for devices to be IOMMU
blocked while driver detached. In that case you could see, "oh, devices
are not DMA blocked by default" as we talked about in the ATS-always-on
thread [1].
[1]: http://lore.kernel.org/20260128130520.GV1134360@nvidia.com
> 1 In use, attacks from a hostile device are possible
> A driver can operate the device and is expected to defend against
> attacks from the device itself. The IOMMU restricts the device to only
> access driver approved data (no ATS, DMA strict only, CC shared
> only, interrupt remapping security, bounce partial DMA mappings, etc)
This is a better way to convey the current "force_swiotlb" settings that
TVMs deploy in their arch code.
> 2 In use, no attacks from the device
> The device does what the driver says and is not hostile. The driver
> does not have to defend itself, the IOMMU can run in faster & lower
> security modes (ATS on, DMA-FQ, Identity, still CC shared only)
> * Basically our default security level today
>
> 3 In use, no attacks, and access to CC private memory
> Like #2 and now the IOMMU allows access to CC private memory too.
I am assuming that each bus implementation may have a different way to
get the device to the various trust levels.
For example, the uAPI for PCI TDISP requires associating a device with a
TSM and asking the TSM to push the device to trust level 3. Another bus
like thunderbolt may want to imply that "authorized" that uses challenge
response (tb_domain_challenge_switch_key) enables trust level 2, but
otherwise only enables trust level 1.
> [*] I'm inclunding all attacks with "hostile device", including MIM on
> the PCIe link, compromised/fake device, attacks from a VMM through a
> virtual device, etc.
>
> From a CC VM perspective 0 is at boot, 1 is an out of TCB device, 2
> doesn't exist (without TDISP there is no way to keep the
> hypervisor from attacking?),
Yes, no mitigations against spoofing the device interface without TDISP.
However, I would also assume that level 2 is the ATS-on trust level
outside of TDISP cases.
> and 3 is a full accepted TDSIP device.
>
> #2 can happen in bare metal where a OS may activate link encryption
> and attest the device, but doesn't have CC private/shared memory.
Bare metal would still need to figure out how to send T=1 MMIO cycles
and check with some boot attestation that it can trust its MMIO mappings
are indeed targeting the device. So let's say trust level 2 is
everything but private MMIO and private DMA.
> From a uAPI perspective I'm not sold on having two bools, I think a
> level string would be more flexible. TSM and CC properties are
> orthogonal, except you can't select #3 without the TSM saying it is in
> RUN.
Perhaps the concern is less 2 bools in the uAPI and more the concern
that 'struct pci_dev::untrusted', 'struct tb_switch::authorized',
'struct usb_dev::authorized' and this new 'struct
device_private::cc_accepted' are getting convoluted.
> Internally we'd probably turn that dev->trusted thing into an
> enum and teach the iommu layer to treat it more dynamically.
I will take a stab at some patches in this direction and at least
demonstrate how 'struct pci_dev::untrusted' can be merged with what CC
wants to add on top.
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Jason Gunthorpe @ 2026-03-13 19:07 UTC (permalink / raw)
To: Dan Williams
Cc: Greg KH, linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu,
bhelgaas, alistair23, lukas, Christoph Hellwig, Marek Szyprowski,
Robin Murphy, Roman Kisel, Samuel Ortiz, Rafael J. Wysocki,
Danilo Krummrich
In-Reply-To: <69b45d178ae17_b2b6100f2@dwillia2-mobl4.notmuch>
On Fri, Mar 13, 2026 at 11:53:11AM -0700, Dan Williams wrote:
> Jason's framing of an enum rather than a boolean for "trust" seems
> workable to me and melds "authorization" and "CC acceptance" into one
> concept.
I think you can also fold the auto-probe into this as well.
The kernel would have some default policy for what enum value to set
upon discovery and instead of 'disable auto probe' you'd arrange to
set trust level 0 which would block driver binding and probing
inherently.
Policy in userspace then has to increase the trust level which could
trigger an auto-bind.
> > > Instead, give userspace all the tools it needs to deploy policy about
> > > when to operate a device. When it does decide to operate the device give
> > > it the mechanism to add confidentiality, integrity and performance to
> > > that operation.
> >
> > Yes, this is a policy decision, and if you are only saying this is about
> > "which IOMMU should we select", then that's a dma layer configuration
> > option. Let's not call that "acceptance" please.
>
> Done.
AFAICT "which IOMMU should we select" should entirely be driven by the
TDISP state being in RUN
Jason
^ permalink raw reply
* Re: [PATCH v2 03/19] device core: Introduce confidential device acceptance
From: Dan Williams @ 2026-03-13 18:53 UTC (permalink / raw)
To: Greg KH, Dan Williams
Cc: linux-coco, linux-pci, aik, aneesh.kumar, yilun.xu, bhelgaas,
alistair23, lukas, jgg, Christoph Hellwig, Jason Gunthorpe,
Marek Szyprowski, Robin Murphy, Roman Kisel, Samuel Ortiz,
Rafael J. Wysocki, Danilo Krummrich
In-Reply-To: <2026031319-payee-photo-bdd9@gregkh>
Greg KH wrote:
> On Thu, Mar 12, 2026 at 09:11:32PM -0700, Dan Williams wrote:
> > Greg KH wrote:
> > > On Mon, Mar 02, 2026 at 04:01:51PM -0800, Dan Williams wrote:
> > > > An "accepted" device is one that is allowed to access private memory within
> > > > a Trusted Computing Boundary (TCB). The concept of "acceptance" is distinct
> > > > from other device properties like usb_device::authorized, or
> > > > tb_switch::authorized. The entry to the accepted state is a violent
> > > > operation in which the device will reject MMIO requests that are not
> > > > encrypted, and the device enters a new IOMMU protection domain to allow it
> > > > to access addresses that were previously off-limits.
> > >
> > > Trying to mix/match "acceptance" with "authorized" is going to be a
> > > nightmare, what's the combination that can happen here over time?
> >
> > I do think Linux needs to mix/match these concepts. "Authorization" is a
> > kernel policy to operate a device at all. "Acceptance" is a mechanism
> > to operate a device within a hardware TCB boundary.
> >
> > So, the truth table of combinations would be:
> >
> > accepted authorized result
> > 0 0 logically and physically disconnected device
> >
> > 0 1 connected device, DMA is bounce buffered
> > and loses confidentiality, integrity,
> > and performance
> >
> > 1 0 logically disconnected device, but a
> > relying party trusts that the device is
> > not spoofing authorization.
> >
> > 1 1 connected device, DMA is direct and gains
> > confidentiality, integrity and performance
> >
> > To say it another way, when the above distinguishes "logically" vs
> > "physically" disconnected it is whether the device interface can be
> > verified to not be under adversarial control. An unaccepted device can
> > do limited damage, but still can bounce buffer secrets out of the TCB if
> > so directed.
>
> I really don't agree with this, but I can't think of why at the moment.
> I feel like you are looking at this purely in the TCB point of view,
> while I don't feel that is something that should be considered "special"
> at all here. Linux has, for the most part, always trusted the hardware,
> and now you are wanting to not trust the hardware.
I think framing "trust" as an enum rather than a boolean better
addresses this problem.
> for some things and parts of the kernel. Which is great, it's
> something that I have wanted to change for a very long time now, but
> let's do it right if at all possible.
>
> Give me a few days to come up with a better reply, let me think about
> this some more...
Sure, but I also think we might already be converging, more below...
> > > We need to either "trust" or "not trust" the device, and the bus can
> > > decide what to do with that value (if anything). The DMA layer can then
> > > use that value to do:
> >
> > Trust is separate. For example, there are deployed use cases today where
> > the device is trusted, but unaccepted. Acceptance support for those
> > cases is mostly a performance optimization to be able to stop performing
> > software encryption on top of DMA bounce buffering.
>
> If "acceptance" is just a performance issue, I think you all need to go
> back to the marketing people as that's probably not what they intended
> to have happen here. For some reason I thought they were selling this
> as "security", not "speed" :)
Do not get me wrong there are several threat models mitigated by having
hardware assurance that all communications with the device are
confidentiality and integrity protected. Hardware assurances that
attempts to subvert those protections result in hardware error states is
part of the value. However, you can approximate a subset of those
protections with high overhead workarounds.
> > > > Subsystems like the DMA mapping layer, that need to modify their behavior
> > > > based on the accept state, may only have access to the base 'struct
> > > > device'.
> > >
> > > ^this.
> >
> > The DMA layer is not operating on a trust concept it is effectively
> > being told to select an IOMMU.
>
> Ok, then that's independent of "acceptance", that is "use this IOMMU vs.
> that one" type of thing which is just a "basic configuration for speed"
> type of thing as you mention above :)
>
> Let's not confuse that with anything else like "acceptance" please.
That is fine, and I think you and Jason may be hitting on the same
concern.
>
> > > > It is also likely that the concept of TCB acceptance grows beyond
> > > > PCI devices over time. For these reasons, introduce the concept of
> > > > acceptance in 'struct device_private' which is device common, but only
> > > > suitable for buses and built-in infrastructure to consume.
> > >
> > > Busses are what can control this, but please, let's not make this a
> > > cc-only type thing. We have the idea of trust starting to propagate
> > > through a number of different busses, let's get it right here, so we
> > > don't have to have all of these different bus-specific hacks like we do
> > > today.
> >
> > The conflation of "trust" and "acceptance" has been the main stumbling
> > block of past proposals. As you have said before "kernel drivers trust
> > their devices". That precedent is not being touched in this proposal.
>
> Ah, but I WANT to touch that. Let's FINALLY solve that! Or at the very
> least, provide the infrastructure in the driver core to allow busses
> that want to do that, to be able to do so.
Jason's framing of an enum rather than a boolean for "trust" seems
workable to me and melds "authorization" and "CC acceptance" into one
concept.
> > Instead, give userspace all the tools it needs to deploy policy about
> > when to operate a device. When it does decide to operate the device give
> > it the mechanism to add confidentiality, integrity and performance to
> > that operation.
>
> Yes, this is a policy decision, and if you are only saying this is about
> "which IOMMU should we select", then that's a dma layer configuration
> option. Let's not call that "acceptance" please.
Done.
...and there is precedent for a "trust" enum, lockdown levels. In that
case as well there are a menu of priveleges that can be incrementally
enabled by a policy.
> > This is a "CC-only type thing" because only CC partitions the system
> > into two device domains. One where "trusted unaccepted" devices can
> > operate without CC protections and "trusted accepted" devices can
> > operate with CC protections and direct DMA.
>
> In other words, it's an IOMMU switch, so why not use the switch
> infrastructure? </me runs away...>
>
> anyway, let me think about this some more...
Will do, however in the meantime I am going to speculate that this "trust
as enum" idea is workable and start drafting some patches towards that.
^ permalink raw reply
* Re: [PATCH v2 08/19] PCI/TSM: Add "evidence" support
From: Dan Williams @ 2026-03-13 18:06 UTC (permalink / raw)
To: Xu Yilun, Dan Williams
Cc: linux-coco, linux-pci, gregkh, aik, aneesh.kumar, bhelgaas,
alistair23, lukas, jgg, Donald Hunter, Jakub Kicinski
In-Reply-To: <abPh3d4+opAkot3p@yilunxu-OptiPlex-7050>
Xu Yilun wrote:
> > +void pci_tsm_init_evidence(struct pci_tsm_evidence *evidence, int slot,
> > + enum hash_algo digest_algo)
> > +{
> > + evidence->slot = slot;
> > + evidence->generation = 1;
> > + evidence->digest_algo = digest_algo;
> > + init_rwsem(&evidence->lock);
>
> IIUC, this function is for link tsm driver, is it?
It is meant to be generic for both, and an "optional" support
library for the low-level TSM drivers.
- Host PCI/TSM evidence interface collects the blobs
- Guest PCI/TSM evidence interface retrieves the digests via private TSM
GHCI
- Some infrastructure (either arch specific GHCI or new common GHCI)
pushes the blobs from Host to Guest so that guest PCI/TSM evidence
gathering can also get the blobs.
> But in the following patch, devsec tsm would consume
> pci_tsm_mmio_alloc() which uses evidence->lock. So my solution is to
> initialize the lock on tsm construction.
So I did flub the "->evidence == 0" check, and yes initializing the lock
by default looks like the right answer to that problem.
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Edgecombe, Rick P @ 2026-03-13 17:43 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
x86@kernel.org, dave.hansen@linux.intel.com, kas@kernel.org,
binbin.wu@linux.intel.com, Weiny, Ira,
tony.lindgren@linux.intel.com, mingo@redhat.com, Verma, Vishal L,
nik.borisov@suse.com, seanjc@google.com,
linux-kernel@vger.kernel.org, Annapurve, Vishal, sagis@google.com,
Duan, Zhenzhong, tglx@kernel.org, paulmck@kernel.org,
Chatre, Reinette, bp@alien8.de, yilun.xu@linux.intel.com,
Williams, Dan J, hpa@zytor.com
In-Reply-To: <abQW/1Cfxl9TdJ8D@intel.com>
On Fri, 2026-03-13 at 21:54 +0800, Chao Gao wrote:
> Or the step details might be irrelevant. Perhaps:
>
> TDX module update consists of several steps. Ordering requirements between
> steps mandate lockstep synchronization across all CPUs.
It seems like enough to understand this patch. Then would you put a little blurb
about the ordering of each step in the later patches?
>
> > > > > 1. The entire update process must use stop_machine() to synchronize with
> > > > > other TDX workloads
> > > > > 2. Update steps must be performed in a step-locked manner
> > > > >
> > > > > To prepare for implementing concrete TDX Module update steps, establish
> > > > > the framework by mimicking multi_cpu_stop(), which is a good example of
> > > > > performing a multi-step task in step-locked manner.
> > > > >
> > > >
> > > > Offline Chao pointed that Paul suggested this after considering refactoring out
> > > > the common code. I think it might still be worth mentioning why you can't use
> > > > multi_cpu_stop() directly. I guess there are some differences. what are they.
> > >
> > > To be clear, Paul didn't actually suggest this approach. His feedback indicated
> > > he wasn't concerned about duplicating some of multi_cpu_stop()'s code, i.e., no
> > > need to refactor out some common code.
> >
> > Right, sorry for oversimplifying.
> >
> > >
> > > https://lore.kernel.org/all/a7affba9-0cea-4493-b868-392158b59d83@paulmck-laptop/#t
> > >
> > > We can't use multi_cpu_stop() directly because it only provides lockstep
> > > execution for its own infrastructure, not for the function it runs. If we
> > > passed a function that performs steps A, B, and C to multi_cpu_stop(), there's
> > > no guarantee that all CPUs complete step A before any CPU begins step B.
> >
> > If it could be said more concisely, it seems relevant.
>
> How about:
>
> multi_cpu_stop() executes in lockstep but doesn't synchronize steps within the
> callback function it takes. So, implement one based on its pattern.
Yea.
^ permalink raw reply
* Re: [PATCH v2 3/3] KVM: SEV: Add support for SNP BTB Isolation
From: Tom Lendacky @ 2026-03-13 16:50 UTC (permalink / raw)
To: Kim Phillips, linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Michael Roth, Borislav Petkov, Borislav Petkov,
Naveen Rao, David Kaplan, Pawan Gupta
In-Reply-To: <20260311130611.2201214-4-kim.phillips@amd.com>
On 3/11/26 08:06, Kim Phillips wrote:
> This feature ensures SNP guest Branch Target Buffers (BTBs) are not
> affected by context outside that guest. CPU hardware tracks each
> guest's BTB entries and can flush the BTB if it has been determined
> to be contaminated with any prediction information originating outside
> the particular guest's context.
>
> To mitigate possible performance penalties incurred by these flushes,
> it is recommended that the hypervisor run with SPEC_CTRL[IBRS] set.
> Note that using Automatic IBRS is not an equivalent option here, since
> it behaves differently when SEV-SNP is active. See commit acaa4b5c4c85
> ("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
> for more details.
>
> Indicate support for BTB Isolation in sev_supported_vmsa_features,
> bit 7.
>
> SNP-active guests can enable (BTB) Isolation through SEV_Status
> bit 9 (SNPBTBIsolation).
>
> For more info, refer to page 615, Section 15.36.17 "Side-Channel
> Protection", AMD64 Architecture Programmer's Manual Volume 2: System
> Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
>
> Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
> Signed-off-by: Kim Phillips <kim.phillips@amd.com>
> ---
> v2: No changes
> v1: https://lore.kernel.org/kvm/20260224180157.725159-4-kim.phillips@amd.com/
>
> arch/x86/include/asm/svm.h | 1 +
> arch/x86/kvm/svm/sev.c | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index edde36097ddc..2038461c1316 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -305,6 +305,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
> #define SVM_SEV_FEAT_RESTRICTED_INJECTION BIT(3)
> #define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
> #define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
> +#define SVM_SEV_FEAT_BTB_ISOLATION BIT(7)
> #define SVM_SEV_FEAT_SECURE_TSC BIT(9)
>
> #define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 3f9c1aa39a0a..ac29cf47dd08 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3167,6 +3167,9 @@ void __init sev_hardware_setup(void)
>
> if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
> sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
> +
> + if (sev_snp_enabled)
> + sev_supported_vmsa_features |= SVM_SEV_FEAT_BTB_ISOLATION;
This would also need to update the SVM_SEV_FEAT_SNP_ONLY_MASK that Sean
suggested/created in the IBPB-On-Entry series.
Thanks,
Tom
> }
>
> void sev_hardware_unsetup(void)
^ permalink raw reply
* Re: [PATCH 1/1] firmware: smccc: add support for Live Firmware Activation (LFA)
From: Andre Przywara @ 2026-03-13 14:39 UTC (permalink / raw)
To: Nirmoy Das, Salman Nabi, vvidwans, sudeep.holla, mark.rutland,
lpieralisi
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <19aeb934-7e36-4f30-8f7f-a8ae74a797f5@nvidia.com>
Hi Nirmoy,
On 3/13/26 10:46, Nirmoy Das wrote:
> Hi Salman and Andre,
>
>
> We found an bug while testing LFA. See below:
>
> On 19.01.26 14:27, Salman Nabi wrote:
>> The Arm Live Firmware Activation (LFA) is a specification [1] to describe
>> activating firmware components without a reboot. Those components
>> (like TF-A's BL31, EDK-II, TF-RMM, secure paylods) would be updated the
>> usual way: via fwupd, FF-A or other secure storage methods, or via some
>> IMPDEF Out-Of-Bound method. The user can then activate this new firmware,
>> at system runtime, without requiring a reboot.
>> The specification covers the SMCCC interface to list and query available
>> components and eventually trigger the activation.
[ .... ]
>> +
>> + update_fw_images_tree();
>> +
>> + /*
>> + * Removing non-valid image directories at the end of an activation.
>> + * We can't remove the sysfs attributes while in the respective
>> + * _store() handler, so have to postpone the list removal to a
>> + * workqueue.
>> + */
>> + INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
>
>
> This can get invoke multiple times so re-initializing a work item that
> may already be queued or running
>
> is unsafe. This should be moved to lfa_init() so it is only called once.
> I suggest:
Ah, good point, thanks for spotting and reporting. Will fold this into
the next post!
Cheers,
Andre
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/
> lfa_fw.c
> index 90727a66e49a5..135358113104c 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -653,7 +653,6 @@ static int update_fw_images_tree(void)
> * _store() handler, so have to postpone the list removal to a
> * workqueue.
> */
> - INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
> queue_work(fw_images_update_wq, &fw_images_update_work);
>
> return 0;
> @@ -680,7 +679,7 @@ static void lfa_notify_handler(acpi_handle handle,
> u32 event, void *data)
> * of all activable and pending images.
> */
> do {
> - /* Reset activable image flag */
> + flush_workqueue(fw_images_update_wq);
> found_activable_image = false;
> list_for_each_entry(attrs, &lfa_fw_images, image_node) {
> if (attrs->fw_seq_id == -1)
> @@ -782,6 +781,8 @@ static int __init lfa_init(void)
> return -ENOMEM;
> }
>
> + INIT_WORK(&fw_images_update_work, remove_invalid_fw_images);
> +
> pr_info("Live Firmware Activation: detected v%ld.%ld\n",
> reg.a0 >> 16, reg.a0 & 0xffff);
>
>
> Regards,
>
> Nirmoy
>
>> + queue_work(fw_images_update_wq, &fw_images_update_work);
>> + mutex_unlock(&lfa_lock);
>> +
>> + return ret;
>> +}
>> +
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox