| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aWcIt9AUMo2N9RKw@yilunxu-OptiPlex-7050>
Date: Wed, 14 Jan 2026 11:08:39 +0800
From: Xu Yilun <yilun.xu@...ux.intel.com>
To: Chao Gao <chao.gao@...el.com>
Cc: linux-coco@...ts.linux.dev, linux-kernel@...r.kernel.org,
x86@...nel.org, reinette.chatre@...el.com, ira.weiny@...el.com,
kai.huang@...el.com, dan.j.williams@...el.com, sagis@...gle.com,
vannapurve@...gle.com, paulmck@...nel.org, nik.borisov@...e.com,
Farrah Chen <farrah.chen@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>,
"Kirill A. Shutemov" <kas@...nel.org>
Subject: Re: [PATCH v2 08/21] coco/tdx-host: Implement FW_UPLOAD sysfs ABI
for TDX Module updates
On Tue, Sep 30, 2025 at 07:52:52PM -0700, 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.
>
> 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@...el.com>
> Tested-by: Farrah Chen <farrah.chen@...el.com>
> ---
> arch/x86/Kconfig | 2 +
> arch/x86/include/asm/seamldr.h | 2 +
> arch/x86/include/asm/tdx.h | 5 ++
> arch/x86/virt/vmx/tdx/seamldr.c | 7 ++
> drivers/virt/coco/tdx-host/tdx-host.c | 122 +++++++++++++++++++++++++-
> 5 files changed, 137 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 6b47383d2958..2bf4bb3dfe71 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1908,6 +1908,8 @@ config INTEL_TDX_HOST
> 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 the same or any newer
> 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 7ad026618a23..2422904079a3 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -107,6 +107,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 08c2e3fe6071..69c059194c61 100644
> --- a/arch/x86/virt/vmx/tdx/seamldr.c
> +++ b/arch/x86/virt/vmx/tdx/seamldr.c
> @@ -69,3 +69,10 @@ const struct seamldr_info *seamldr_get_info(void)
> return seamldr_call(P_SEAMLDR_INFO, &args) ? NULL : &seamldr_info;
> }
> EXPORT_SYMBOL_GPL_FOR_MODULES(seamldr_get_info, "tdx-host");
> +
> +int seamldr_install_module(const u8 *data, u32 size)
> +{
> + /* TODO: Update TDX Module here */
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL_FOR_MODULES(seamldr_install_module, "tdx-host");
> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> index 42570c5b221b..418e90797689 100644
> --- a/drivers/virt/coco/tdx-host/tdx-host.c
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> @@ -10,6 +10,7 @@
> #include <linux/mod_devicetable.h>
> #include <linux/sysfs.h>
> #include <linux/device/faux.h>
> +#include <linux/firmware.h>
> #include <asm/cpu_device_id.h>
> #include <asm/seamldr.h>
> #include <asm/tdx.h>
> @@ -21,6 +22,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 struct faux_device *fdev;
Make the fdev declaration right before tdx_host_init(), try best to keep
the update stuff in one bluk.
[...]
> +static int seamldr_init(struct device *dev)
> +{
> + const struct seamldr_info *seamldr_info = seamldr_get_info();
> + const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> + int ret;
> +
> + if (!tdx_sysinfo || !seamldr_info)
> + return -ENXIO;
> +
> + if (!tdx_supports_runtime_update(tdx_sysinfo)) {
> + pr_info("Current TDX Module cannot be updated. Consider BIOS updates\n");
> + return -EOPNOTSUPP;
I don't think we fail out the whole tdx-host here. We should skip the
optional feature if it is not supported to allow other features work.
E.g. the TDX Module version, the P-SEAMLOAD version, TDX Connect.
> + }
> +
> + if (!seamldr_info->num_remaining_updates) {
> + pr_info("P-SEAMLDR doesn't support TDX Module updates\n");
> + return -EOPNOTSUPP;
> + }
Ditto. And keeping num_remaining_updates sysfs node visible and returning 0
is valuable, it clearly tells why update is impossible and aligns with
the situation when the user keeps on updating and exhausts the available
updates.
> +
> + 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);
> +
> + return ret;
> +}
Powered by blists - more mailing lists