[PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates

Chao Gao posted 26 patches 2 weeks ago
[PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
Posted by Chao Gao 2 weeks ago
The firmware upload framework provides a standard mechanism for firmware
updates by allowing device drivers to expose sysfs interfaces for
user-initiated updates.

Register with this framework to expose sysfs interfaces for TDX Module
updates and implement operations to process data blobs supplied by
userspace.

Note that:
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. TDX Module Updates complete synchronously within .write(), meaning
   .poll_complete() is only called after successful updates and therefore
   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>
Tested-by: Farrah Chen <farrah.chen@intel.com>
---
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        |   2 +
 arch/x86/include/asm/tdx.h            |   5 ++
 arch/x86/virt/vmx/tdx/seamldr.c       |  19 ++++
 drivers/virt/coco/tdx-host/Kconfig    |   2 +
 drivers/virt/coco/tdx-host/tdx-host.c | 124 +++++++++++++++++++++++++-
 5 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/seamldr.h b/arch/x86/include/asm/seamldr.h
index d1e9f6e16e8d..692bde5e9bb4 100644
--- a/arch/x86/include/asm/seamldr.h
+++ b/arch/x86/include/asm/seamldr.h
@@ -20,8 +20,10 @@ struct seamldr_info {
 
 #ifdef CONFIG_INTEL_TDX_MODULE_UPDATE
 const struct seamldr_info *seamldr_get_info(void);
+int seamldr_install_module(const u8 *data, u32 size);
 #else
 static inline const struct seamldr_info *seamldr_get_info(void) { return NULL; }
+static inline int seamldr_install_module(const u8 *data, u32 size) { return -EOPNOTSUPP; }
 #endif
 
 #endif
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index cb2219302dfc..ffadbf64d0c1 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -103,6 +103,11 @@ 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)
+{
+	return false; /* To be enabled when kernel is ready */
+}
+
 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 6a83ae405fac..af7a6621e5e0 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/irqflags.h>
+#include <linux/mm.h>
 #include <linux/types.h>
 
 #include <asm/seamldr.h>
@@ -69,3 +70,21 @@ const struct seamldr_info *seamldr_get_info(void)
 	return seamldr_call(P_SEAMLDR_INFO, &args) ? NULL : &seamldr_info;
 }
 EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
+
+/**
+ * seamldr_install_module - Install a new TDX module
+ * @data: Pointer to the TDX module binary data. It should be vmalloc'd
+ *        memory.
+ * @size: Size of the TDX module binary data
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int seamldr_install_module(const u8 *data, u32 size)
+{
+	if (!is_vmalloc_addr(data))
+		return -EINVAL;
+
+	/* 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 6a9199e6c2c6..59aaca2252b0 100644
--- a/drivers/virt/coco/tdx-host/Kconfig
+++ b/drivers/virt/coco/tdx-host/Kconfig
@@ -12,6 +12,8 @@ config TDX_HOST_SERVICES
 config INTEL_TDX_MODULE_UPDATE
 	bool "Intel TDX module runtime update"
 	depends on TDX_HOST_SERVICES
+	select FW_LOADER
+	select FW_UPLOAD
 	help
 	  This enables the kernel to support TDX module runtime update. This
 	  allows the admin to update the TDX module to another compatible
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index f4ce89522806..06487de2ebfe 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>
@@ -20,6 +21,13 @@ static const struct x86_cpu_id tdx_host_ids[] = {
 };
 MODULE_DEVICE_TABLE(x86cpu, tdx_host_ids);
 
+struct tdx_fw_upload_status {
+	bool cancel_request;
+};
+
+struct fw_upload *tdx_fwl;
+static struct tdx_fw_upload_status tdx_fw_upload_status;
+
 static ssize_t version_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
@@ -100,6 +108,120 @@ static const struct attribute_group *tdx_host_groups[] = {
 	NULL,
 };
 
+static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
+					 const u8 *data, u32 size)
+{
+	struct tdx_fw_upload_status *status = fwl->dd_handle;
+
+	status->cancel_request = false;
+
+	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)
+{
+	struct tdx_fw_upload_status *status = fwl->dd_handle;
+	int ret;
+
+	if (status->cancel_request) {
+		status->cancel_request = false;
+		return FW_UPLOAD_ERR_CANCELED;
+	}
+
+	/*
+	 * 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;
+}
+
+static void tdx_fw_cancel(struct fw_upload *fwl)
+{
+	struct tdx_fw_upload_status *status = fwl->dd_handle;
+
+	status->cancel_request = true;
+}
+
+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_init(struct device *dev)
+{
+	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
+	int ret;
+
+	if (WARN_ON_ONCE(!tdx_sysinfo))
+		return;
+
+	if (!IS_ENABLED(CONFIG_INTEL_TDX_MODULE_UPDATE))
+		return;
+
+	if (!tdx_supports_runtime_update(tdx_sysinfo))
+		pr_info("Current TDX Module cannot be updated. Consider BIOS updates\n");
+
+	tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
+					   &tdx_fw_ops, &tdx_fw_upload_status);
+	ret = PTR_ERR_OR_ZERO(tdx_fwl);
+	if (ret)
+		pr_err("failed to register module uploader %d\n", ret);
+}
+
+static void seamldr_deinit(void)
+{
+	if (tdx_fwl)
+		firmware_upload_unregister(tdx_fwl);
+}
+
+static int tdx_host_probe(struct faux_device *fdev)
+{
+	seamldr_init(&fdev->dev);
+	return 0;
+}
+
+static void tdx_host_remove(struct faux_device *fdev)
+{
+	seamldr_deinit();
+}
+
+static struct faux_device_ops tdx_host_ops = {
+	.probe		= tdx_host_probe,
+	.remove		= tdx_host_remove,
+};
+
 static struct faux_device *fdev;
 
 static int __init tdx_host_init(void)
@@ -107,7 +229,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
Re: [PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
Posted by Xing, Cedric 12 hours ago
On 1/23/2026 8:55 AM, Chao Gao wrote:
[...]
> +
> +static void seamldr_init(struct device *dev)
> +{
> +	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> +	int ret;
> +
> +	if (WARN_ON_ONCE(!tdx_sysinfo))
> +		return;
> +
> +	if (!IS_ENABLED(CONFIG_INTEL_TDX_MODULE_UPDATE))
> +		return;
> +
> +	if (!tdx_supports_runtime_update(tdx_sysinfo))
> +		pr_info("Current TDX Module cannot be updated. Consider BIOS updates\n");
> +
> +	tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
I can't speak for others but the name "seamldr_upload" here doesn't look intuitive to me. Given this 
FW node will show up in /sys/class/firmware/, I'd name it "tdx_module" to indicate to the user 
clearly that this is for updating the TDX module.

-Cedric
Re: [PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
Posted by Xu Yilun 1 week ago
> +static enum fw_upload_err tdx_fw_write(struct fw_upload *fwl, const u8 *data,
> +				       u32 offset, u32 size, u32 *written)
> +{
> +	struct tdx_fw_upload_status *status = fwl->dd_handle;
> +	int ret;
> +
> +	if (status->cancel_request) {
> +		status->cancel_request = false;
> +		return FW_UPLOAD_ERR_CANCELED;

We don't allow partial write, we stop_machine while writing, so we
cannot possibly cancel the update in progress, so we only check the
cancel_request once before first write. That means cancel is useless for
our case. Is it better we delete all the cancel logic &
struct tdx_fw_upload_status?

> +	}
> +
> +	/*
> +	 * 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);

...

> +static void tdx_fw_cancel(struct fw_upload *fwl)
> +{

Unfortunately fw_upload core doesn't allow .cancel unimplemented, leave
it as a dummy stub is OK, since this callback just request cancel,
doesn't care whether the cancel succeeds or fails in the end.

If you agree, add some comments in this function.

> +}
> +
> +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_init(struct device *dev)
> +{
> +	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> +	int ret;
> +
> +	if (WARN_ON_ONCE(!tdx_sysinfo))
> +		return;

We already does tdx_get_sysinfo() on module_init, is it better we have
a global tdx_sysinfo pointer in this driver, so that we don't have to
retrieve it again and again.
Re: [PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
Posted by Huang, Kai 1 week, 3 days ago
> 2. TDX Module Updates complete synchronously within .write(), meaning
>    .poll_complete() is only called after successful updates and therefore
>    always returns success

Nit:

Why "updates" instead of "update"?  Is there multiple updates possible
within .write()?

[...]

> 
>  
> +struct tdx_fw_upload_status {
> +	bool cancel_request;
> +};
> +
> +struct fw_upload *tdx_fwl;

Can 'tdx_fwl' be static?

[...]

> 
> +static void seamldr_init(struct device *dev)
> +{
> +	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> +	int ret;
> +
> +	if (WARN_ON_ONCE(!tdx_sysinfo))
> +		return;
> +
> +	if (!IS_ENABLED(CONFIG_INTEL_TDX_MODULE_UPDATE))
> +		return;
> +
> +	if (!tdx_supports_runtime_update(tdx_sysinfo))
> +		pr_info("Current TDX Module cannot be updated. Consider BIOS updates\n");

What's the point of continuing if runtime update is not supported?

> +
> +	tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
> +					   &tdx_fw_ops, &tdx_fw_upload_status);
> +	ret = PTR_ERR_OR_ZERO(tdx_fwl);
> +	if (ret)
> +		pr_err("failed to register module uploader %d\n", ret);
> +}
> +
> +static void seamldr_deinit(void)
> +{
> +	if (tdx_fwl)
> +		firmware_upload_unregister(tdx_fwl);
> +}
> +
> +static int tdx_host_probe(struct faux_device *fdev)
> +{
> +	seamldr_init(&fdev->dev);

IMHO you need a comment to explain why seamldr_init() doesn't return error
and tdx_host_probe() already returns success?

> +	return 0;
> +}
> +
> +static void tdx_host_remove(struct faux_device *fdev)
> +{
> +	seamldr_deinit();
> +}
> +
> +static struct faux_device_ops tdx_host_ops = {
> +	.probe		= tdx_host_probe,
> +	.remove		= tdx_host_remove,
> +};
> +
>  static struct faux_device *fdev;
>  
>  static int __init tdx_host_init(void)
> @@ -107,7 +229,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;
>  
Re: [PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
Posted by Tony Lindgren 1 week, 4 days ago
On Fri, Jan 23, 2026 at 06:55:18AM -0800, Chao Gao wrote:
> The firmware upload framework provides a standard mechanism for firmware
> updates by allowing device drivers to expose sysfs interfaces for
> user-initiated updates.
> 
> Register with this framework to expose sysfs interfaces for TDX Module
> updates and implement operations to process data blobs supplied by
> userspace.

Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>