Implement a fw_upload interface to coordinate TD-Preserving updates. 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 TD-Preserving is non-linear at times. The latest
TDX module may not be TD-Preserving capable. 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 (i.e. latest may not be TD-Preserving capable), 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>
---
 arch/x86/Kconfig                |  2 +
 arch/x86/virt/vmx/tdx/seamldr.c | 77 +++++++++++++++++++++++++++++++++
 arch/x86/virt/vmx/tdx/seamldr.h |  2 +
 arch/x86/virt/vmx/tdx/tdx.c     |  4 ++
 4 files changed, 85 insertions(+)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 8b1e0986b7f8..31385104a6ee 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1935,6 +1935,8 @@ config INTEL_TDX_HOST
 config INTEL_TDX_MODULE_UPDATE
 	bool "Intel TDX module runtime update"
 	depends on INTEL_TDX_HOST
+	select FW_LOADER
+	select FW_UPLOAD
 	help
 	  This enables the kernel to support TDX module runtime update. This allows
 	  the admin to upgrade the TDX module to a newer one without the need to
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index b628555daf55..da862e71ebce 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -8,6 +8,8 @@
 
 #include <linux/cleanup.h>
 #include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/gfp.h>
 #include <linux/kobject.h>
 #include <linux/sysfs.h>
 
@@ -32,6 +34,15 @@ struct seamldr_info {
 	u8	reserved1[224];
 } __packed;
 
+
+#define TDX_FW_STATE_BITS	32
+#define TDX_FW_CANCEL		0
+struct tdx_status {
+	DECLARE_BITMAP(fw_state, TDX_FW_STATE_BITS);
+};
+
+struct fw_upload *tdx_fwl;
+static struct tdx_status tdx_status;
 static struct seamldr_info seamldr_info __aligned(256);
 
 static inline int seamldr_call(u64 fn, struct tdx_module_args *args)
@@ -101,3 +112,69 @@ int get_seamldr_info(void)
 
 	return seamldr_call(P_SEAMLDR_INFO, &args);
 }
+
+static int seamldr_install_module(const u8 *data, u32 size)
+{
+	return -EOPNOTSUPP;
+}
+
+static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
+					 const u8 *data, u32 size)
+{
+	struct tdx_status *status = fwl->dd_handle;
+
+	if (test_and_clear_bit(TDX_FW_CANCEL, status->fw_state))
+		return FW_UPLOAD_ERR_CANCELED;
+
+	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_status *status = fwl->dd_handle;
+
+	if (test_and_clear_bit(TDX_FW_CANCEL, status->fw_state))
+		return FW_UPLOAD_ERR_CANCELED;
+
+	/*
+	 * No partial write will be returned to callers so @offset should
+	 * always be zero.
+	 */
+	WARN_ON_ONCE(offset);
+	if (seamldr_install_module(data, size))
+		return FW_UPLOAD_ERR_FW_INVALID;
+
+	*written = size;
+	return FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err tdx_fw_poll_complete(struct fw_upload *fwl)
+{
+	return FW_UPLOAD_ERR_NONE;
+}
+
+static void tdx_fw_cancel(struct fw_upload *fwl)
+{
+	struct tdx_status *status = fwl->dd_handle;
+
+	set_bit(TDX_FW_CANCEL, status->fw_state);
+}
+
+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,
+};
+
+void seamldr_init(struct device *dev)
+{
+	int ret;
+
+	tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
+					   &tdx_fw_ops, &tdx_status);
+	ret = PTR_ERR_OR_ZERO(tdx_fwl);
+	if (ret)
+		pr_err("failed to register module uploader %d\n", ret);
+}
diff --git a/arch/x86/virt/vmx/tdx/seamldr.h b/arch/x86/virt/vmx/tdx/seamldr.h
index 15597cb5036d..00fa3a4e9155 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.h
+++ b/arch/x86/virt/vmx/tdx/seamldr.h
@@ -6,9 +6,11 @@
 extern struct attribute_group seamldr_group;
 #define SEAMLDR_GROUP (&seamldr_group)
 int get_seamldr_info(void);
+void seamldr_init(struct device *dev);
 #else
 #define SEAMLDR_GROUP NULL
 static inline int get_seamldr_info(void) { return 0; }
+static inline void seamldr_init(struct device *dev) { }
 #endif
 
 #endif
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index aa6a23d46494..22ffc15b4299 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1178,6 +1178,10 @@ static void tdx_subsys_init(void)
 		goto err_bus;
 	}
 
+	struct device *dev_root __free(put_device) = bus_get_dev_root(&tdx_subsys);
+	if (dev_root)
+		seamldr_init(dev_root);
+
 	return;
 
 err_bus:
-- 
2.47.1

On Fri, May 23, 2025 at 4:55 AM Chao Gao <chao.gao@intel.com> wrote:
>
> Implement a fw_upload interface to coordinate TD-Preserving updates. 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 TD-Preserving is non-linear at times. The latest
> TDX module may not be TD-Preserving capable. 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 (i.e. latest may not be TD-Preserving capable), 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>
> ---
>  arch/x86/Kconfig                |  2 +
>  arch/x86/virt/vmx/tdx/seamldr.c | 77 +++++++++++++++++++++++++++++++++
>  arch/x86/virt/vmx/tdx/seamldr.h |  2 +
>  arch/x86/virt/vmx/tdx/tdx.c     |  4 ++
>  4 files changed, 85 insertions(+)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 8b1e0986b7f8..31385104a6ee 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1935,6 +1935,8 @@ config INTEL_TDX_HOST
>  config INTEL_TDX_MODULE_UPDATE
>         bool "Intel TDX module runtime update"
>         depends on INTEL_TDX_HOST
> +       select FW_LOADER
> +       select FW_UPLOAD
>         help
>           This enables the kernel to support TDX module runtime update. This allows
>           the admin to upgrade the TDX module to a newer one without the need to
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
> index b628555daf55..da862e71ebce 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -8,6 +8,8 @@
>
>  #include <linux/cleanup.h>
>  #include <linux/device.h>
> +#include <linux/firmware.h>
> +#include <linux/gfp.h>
>  #include <linux/kobject.h>
>  #include <linux/sysfs.h>
>
> @@ -32,6 +34,15 @@ struct seamldr_info {
>         u8      reserved1[224];
>  } __packed;
>
> +
> +#define TDX_FW_STATE_BITS      32
> +#define TDX_FW_CANCEL          0
> +struct tdx_status {
> +       DECLARE_BITMAP(fw_state, TDX_FW_STATE_BITS);
> +};
> +
> +struct fw_upload *tdx_fwl;
> +static struct tdx_status tdx_status;
>  static struct seamldr_info seamldr_info __aligned(256);
>
>  static inline int seamldr_call(u64 fn, struct tdx_module_args *args)
> @@ -101,3 +112,69 @@ int get_seamldr_info(void)
>
>         return seamldr_call(P_SEAMLDR_INFO, &args);
>  }
> +
> +static int seamldr_install_module(const u8 *data, u32 size)
> +{
> +       return -EOPNOTSUPP;
> +}
> +
> +static enum fw_upload_err tdx_fw_prepare(struct fw_upload *fwl,
> +                                        const u8 *data, u32 size)
> +{
> +       struct tdx_status *status = fwl->dd_handle;
> +
> +       if (test_and_clear_bit(TDX_FW_CANCEL, status->fw_state))
> +               return FW_UPLOAD_ERR_CANCELED;
> +
> +       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_status *status = fwl->dd_handle;
> +
> +       if (test_and_clear_bit(TDX_FW_CANCEL, status->fw_state))
> +               return FW_UPLOAD_ERR_CANCELED;
> +
> +       /*
> +        * No partial write will be returned to callers so @offset should
> +        * always be zero.
> +        */
> +       WARN_ON_ONCE(offset);
> +       if (seamldr_install_module(data, size))
> +               return FW_UPLOAD_ERR_FW_INVALID;
> +
> +       *written = size;
> +       return FW_UPLOAD_ERR_NONE;
> +}
> +
> +static enum fw_upload_err tdx_fw_poll_complete(struct fw_upload *fwl)
> +{
> +       return FW_UPLOAD_ERR_NONE;
> +}
> +
> +static void tdx_fw_cancel(struct fw_upload *fwl)
> +{
> +       struct tdx_status *status = fwl->dd_handle;
> +
> +       set_bit(TDX_FW_CANCEL, status->fw_state);
> +}
> +
> +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,
> +};
> +
> +void seamldr_init(struct device *dev)
> +{
> +       int ret;
> +
> +       tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
> +                                          &tdx_fw_ops, &tdx_status);
> +       ret = PTR_ERR_OR_ZERO(tdx_fwl);
> +       if (ret)
> +               pr_err("failed to register module uploader %d\n", ret);
> +}
> diff --git a/arch/x86/virt/vmx/tdx/seamldr.h b/arch/x86/virt/vmx/tdx/seamldr.h
> index 15597cb5036d..00fa3a4e9155 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.h
> +++ b/arch/x86/virt/vmx/tdx/seamldr.h
> @@ -6,9 +6,11 @@
>  extern struct attribute_group seamldr_group;
>  #define SEAMLDR_GROUP (&seamldr_group)
>  int get_seamldr_info(void);
> +void seamldr_init(struct device *dev);
>  #else
>  #define SEAMLDR_GROUP NULL
>  static inline int get_seamldr_info(void) { return 0; }
> +static inline void seamldr_init(struct device *dev) { }
>  #endif
>
>  #endif
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index aa6a23d46494..22ffc15b4299 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1178,6 +1178,10 @@ static void tdx_subsys_init(void)
>                 goto err_bus;
>         }
>
> +       struct device *dev_root __free(put_device) = bus_get_dev_root(&tdx_subsys);

dev_root definition here causes compilation error:

arch/x86/virt/vmx/tdx/tdx.c:1181:3: error: cannot jump from this goto
statement to its label
                goto err_bus;
                ^
arch/x86/virt/vmx/tdx/tdx.c:1184:17: note: jump bypasses
initialization of variable with __attribute__((cleanup))
        struct device *dev_root __free(put_device) =
bus_get_dev_root(&tdx_subsys);

> +       if (dev_root)
> +               seamldr_init(dev_root);
> +
>         return;
>
>  err_bus:
> --
> 2.47.1
>
>

On Mon, Jun 16, 2025 at 05:55:50PM -0500, Sagi Shahar wrote:
>> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
>> index aa6a23d46494..22ffc15b4299 100644
>> --- a/arch/x86/virt/vmx/tdx/tdx.c
>> +++ b/arch/x86/virt/vmx/tdx/tdx.c
>> @@ -1178,6 +1178,10 @@ static void tdx_subsys_init(void)
>>                 goto err_bus;
>>         }
>>
>> +       struct device *dev_root __free(put_device) = bus_get_dev_root(&tdx_subsys);
>
>dev_root definition here causes compilation error:
>
>arch/x86/virt/vmx/tdx/tdx.c:1181:3: error: cannot jump from this goto
>statement to its label
>                goto err_bus;
>                ^
>arch/x86/virt/vmx/tdx/tdx.c:1184:17: note: jump bypasses
>initialization of variable with __attribute__((cleanup))
>        struct device *dev_root __free(put_device) =
>bus_get_dev_root(&tdx_subsys);

Thank you for reporting this. The goto label is unnecessary, and I'll remove it
from patch 4 (which adds this goto).

I'm curious about which compiler you are using because I don't encounter this
error with "gcc version 11.5.0 20240719 (Red Hat 11.5.0-5) (GCC)".

>
>> +       if (dev_root)
>> +               seamldr_init(dev_root);
>> +
>>         return;
>>
>>  err_bus:
>> --
>> 2.47.1
>>
>>
>