Re: [PATCH v6 00/22] Runtime TDX module update support

[Chao Gao <chao.gao@intel.com>]

On Thu, Mar 26, 2026 at 01:43:51AM -0700, Chao Gao wrote:
>Hi Reviewers,
>
>Please review patches 6 and 17; others already have 2+ RB tags.
>
>Patch 6 was reworked to use is_visible() for attribute visibility (which is
>the standard practice), so previous RB tags were dropped. Patch 17 has
>fewer reviews so far and needs another look.
>
>I believe this series is quite mature and also self-contained (no impact to
>the rest of kernel unless an update is triggered through the dedicated
>sysfs ABIs). I'm hoping it can be merged for 7.1.
>
>Changelog:
>v5->v6:

Below is the diff between v5 and v6:

diff --git a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
index 97840db794c0..e1a2f3b2ea65 100644
--- a/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
+++ b/Documentation/ABI/testing/sysfs-devices-faux-tdx-host
@@ -24,9 +24,8 @@ Description:	(RO) Report the number of remaining updates. TDX maintains a
		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.
+		Interface Specification, Chapter "SEAMLDR_INFO" and Chapter
+		"SEAMLDR.INSTALL" for more information.
 
 What:		/sys/devices/faux/tdx_host/firmware/tdx_module
 Contact:	linux-coco@lists.linux.dev
@@ -58,14 +57,15 @@ Description:	(RO) See Documentation/ABI/testing/sysfs-class-firmware for
		baseline expectations for this file. The <ERROR> part in the
		<STATUS>:<ERROR> format can be:
 
-		   "device-busy": Compatibility checks failed.
+		   "device-busy": Conflicting operations are in progress, e.g., TD
+				  build or TD migration.
 
		   "read-write-error": Memory allocation failed.
 
-		   "hw-error": Cannot communicate with P-SEAMLDR or TDX module.
+		   "hw-error": Communication with P-SEAMLDR or TDX module failed
+			       or update limit exhausted.
 
		   "firmware-invalid": The provided TDX module update is invalid,
-		                       or the number of updates reached the limit,
				       or other unexpected errors occurred.
 
		"hw-error" or "firmware-invalid" may be fatal, causing all TDs
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 386097b2e01b..6351d2c21513 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -116,11 +116,6 @@ static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinf
	return sysinfo->features.tdx_features0 & TDX_FEATURES0_TD_PRESERVING;
 }
 
-static inline bool tdx_supports_update_compatibility(const struct tdx_sys_info *sysinfo)
-{
-	return sysinfo->features.tdx_features0 & TDX_FEATURES0_UPDATE_COMPAT;
-}
-
 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 4e1ad06506cc..276330179783 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -51,7 +51,8 @@ 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
- * be made with interrupts disabled.
+ * be made with interrupts disabled, where plain spinlocks are prohibited in
+ * PREEMPT_RT kernels as they become sleeping locks.
  */
 static DEFINE_RAW_SPINLOCK(seamldr_lock);
 
@@ -73,6 +74,13 @@ int seamldr_get_info(struct seamldr_info *seamldr_info)
 }
 EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
 
+static int seamldr_install(const struct seamldr_params *params)
+{
+	struct tdx_module_args args = { .rcx = __pa(params) };
+
+	return seamldr_call(P_SEAMLDR_INSTALL, &args);
+}
+
 static void free_seamldr_params(struct seamldr_params *params)
 {
	free_page((unsigned long)params);
@@ -109,8 +117,9 @@ static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned
	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.
+		 * @sig is 4KB-aligned, but that does not imply PAGE_SIZE
+		 * alignment when PAGE_SIZE != SZ_4K. Always include the
+		 * in-page offset.
		 */
		params->sigstruct_pa[i] = (vmalloc_to_pfn(ptr) << PAGE_SHIFT) +
					  ((unsigned long)ptr & ~PAGE_MASK);
@@ -136,6 +145,10 @@ static struct seamldr_params *alloc_seamldr_params(const void *module, unsigned
  * 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.
+ *
+ * Note @offset_of_module is relative to the start of struct tdx_blob, not
+ * @data, and @length is the total length of the blob, not the length of
+ * @data.
  */
 struct tdx_blob {
	u16	version;
@@ -196,7 +209,7 @@ enum module_update_state {
 static struct {
	enum module_update_state state;
	int thread_ack;
-	int failed;
+	bool failed;
	/*
	 * Protect update_data. Raw spinlock as it will be acquired from
	 * interrupt-disabled contexts.
@@ -234,7 +247,6 @@ static void print_update_failure_message(void)
 static int do_seamldr_install_module(void *seamldr_params)
 {
	enum module_update_state newstate, curstate = MODULE_UPDATE_START;
-	struct tdx_module_args args = {};
	int cpu = smp_processor_id();
	bool primary;
	int ret = 0;
@@ -254,8 +266,7 @@ static int do_seamldr_install_module(void *seamldr_params)
					ret = tdx_module_shutdown();
				break;
			case MODULE_UPDATE_CPU_INSTALL:
-				args.rcx = __pa(seamldr_params);
-				ret = seamldr_call(P_SEAMLDR_INSTALL, &args);
+				ret = seamldr_install(seamldr_params);
				break;
			case MODULE_UPDATE_CPU_INIT:
				ret = tdx_cpu_enable();
@@ -269,8 +280,7 @@ static int do_seamldr_install_module(void *seamldr_params)
			}
 
			if (ret) {
-				scoped_guard(raw_spinlock, &update_data.lock)
-					update_data.failed++;
+				WRITE_ONCE(update_data.failed, true);
				if (curstate > MODULE_UPDATE_SHUTDOWN)
					print_update_failure_message();
			} else {
@@ -314,7 +324,7 @@ int seamldr_install_module(const u8 *data, u32 size)
	if (IS_ERR(params))
		return PTR_ERR(params);
 
-	update_data.failed = 0;
+	update_data.failed = false;
	set_target_state(MODULE_UPDATE_START + 1);
	ret = stop_machine(do_seamldr_install_module, params, cpu_online_mask);
	if (ret)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index b76b8c393425..3f4221098b78 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1194,7 +1194,7 @@ int tdx_module_shutdown(void)
	 */
	args.rcx = tdx_sysinfo.handoff.module_hv;
 
-	if (tdx_supports_update_compatibility(&tdx_sysinfo))
+	if (tdx_sysinfo.features.tdx_features0 & TDX_FEATURES0_UPDATE_COMPAT)
		args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
 
	ret = seamcall(TDH_SYS_SHUTDOWN, &args);
@@ -1214,11 +1214,12 @@ int tdx_module_shutdown(void)
	sysinit_ret = 0;
 
	/*
-	 * By reaching here CPUHP is disabled and all present CPUs
-	 * are online. It's safe to just loop all online CPUs and
-	 * reset the per-cpu flag.
+	 * Since the TDX module is shut down and gone, mark all CPUs
+	 * (including offlined ones) as uninitialied. This is called in
+	 * stop_machine() (where CPU hotplug is disabled), preventing
+	 * races with other tdx_lp_initialized accesses.
	 */
-	for_each_online_cpu(cpu)
+	for_each_possible_cpu(cpu)
		per_cpu(tdx_lp_initialized, cpu) = false;
	return 0;
 }
diff --git a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
index d6a4fa8deb5e..1b6f9b80b197 100644
--- a/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
+++ b/arch/x86/virt/vmx/tdx/tdx_global_metadata.c
@@ -102,13 +102,15 @@ static int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *sysinfo_td_conf
 
 static int get_tdx_sys_info_handoff(struct tdx_sys_info_handoff *sysinfo_handoff)
 {
-	int ret = 0;
+	int ret;
	u64 val;
 
-	if (!ret && !(ret = read_sys_metadata_field(0x8900000100000000, &val)))
-		sysinfo_handoff->module_hv = val;
+	ret = read_sys_metadata_field(0x8900000100000000, &val);
+	if (ret)
+		return ret;
 
-	return ret;
+	sysinfo_handoff->module_hv = val;
+	return 0;
 }
 
 static int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 8cf3cc99024a..f236119c2748 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -21,6 +21,12 @@ static const struct x86_cpu_id tdx_host_ids[] = {
 };
 MODULE_DEVICE_TABLE(x86cpu, tdx_host_ids);
 
+/*
+ * TDX module and P-SEAMLDR version convention: "major.minor.update"
+ * (e.g., "1.5.08") with zero-padded two-digit update field.
+ */
+#define TDX_VERSION_FMT "%u.%u.%02u"
+
 static ssize_t version_show(struct device *dev, struct device_attribute *attr,
			    char *buf)
 {
@@ -32,9 +38,9 @@ static ssize_t version_show(struct device *dev, struct device_attribute *attr,
 
	ver = &tdx_sysinfo->version;
 
-	return sysfs_emit(buf, "%u.%u.%02u\n", ver->major_version,
-					       ver->minor_version,
-					       ver->update_version);
+	return sysfs_emit(buf, TDX_VERSION_FMT"\n", ver->major_version,
+						    ver->minor_version,
+						    ver->update_version);
 }
 static DEVICE_ATTR_RO(version);
 
@@ -42,7 +48,10 @@ static struct attribute *tdx_host_attrs[] = {
	&dev_attr_version.attr,
	NULL,
 };
-ATTRIBUTE_GROUPS(tdx_host);
+
+static const struct attribute_group tdx_host_group = {
+	.attrs = tdx_host_attrs,
+};
 
 static ssize_t seamldr_version_show(struct device *dev, struct device_attribute *attr,
				    char *buf)
@@ -54,9 +63,9 @@ static ssize_t seamldr_version_show(struct device *dev, struct device_attribute
	if (ret)
		return ret;
 
-	return sysfs_emit(buf, "%u.%u.%02u\n", info.major_version,
-					       info.minor_version,
-					       info.update_version);
+	return sysfs_emit(buf, TDX_VERSION_FMT"\n", info.major_version,
+						    info.minor_version,
+						    info.update_version);
 }
 
 static ssize_t num_remaining_updates_show(struct device *dev,
@@ -90,9 +99,41 @@ static struct attribute *seamldr_attrs[] = {
	NULL,
 };
 
+static bool can_expose_seamldr(void)
+{
+	const struct tdx_sys_info *sysinfo = tdx_get_sysinfo();
+
+	if (!sysinfo)
+		return false;
+
+	/*
+	 * Calling P-SEAMLDR on CPUs with the seamret_invd_vmcs bug clears
+	 * the current VMCS, which breaks KVM. Verify the erratum is not
+	 * present before exposing P-SEAMLDR features.
+	 */
+	if (boot_cpu_has_bug(X86_BUG_SEAMRET_INVD_VMCS))
+		return false;
+
+	return tdx_supports_runtime_update(sysinfo);
+}
+
+static bool seamldr_group_visible(struct kobject *kobj)
+{
+	return can_expose_seamldr();
+}
+
+DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(seamldr);
+
 static const struct attribute_group seamldr_group = {
	.name = "seamldr",
	.attrs = seamldr_attrs,
+	.is_visible = SYSFS_GROUP_VISIBLE(seamldr),
+};
+
+static const struct attribute_group *tdx_host_groups[] = {
+	&tdx_host_group,
+	&seamldr_group,
+	NULL,
 };
 
 static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
@@ -122,8 +163,6 @@ static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
		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:
@@ -164,22 +203,9 @@ static void seamldr_deinit(void *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;
-
-	/*
-	 * Calling P-SEAMLDR on CPUs with the seamret_invd_vmcs bug clears
-	 * the current VMCS, which breaks KVM. Verify the erratum is not
-	 * present before exposing P-SEAMLDR features.
-	 */
-	if (boot_cpu_has_bug(X86_BUG_SEAMRET_INVD_VMCS))
+	if (!can_expose_seamldr())
		return 0;
 
	tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "tdx_module",
@@ -187,11 +213,7 @@ static int seamldr_init(struct device *dev)
	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);
+	return devm_add_action_or_reset(dev, seamldr_deinit, tdx_fwl);
 }
 
 static int tdx_host_probe(struct faux_device *fdev)