| 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: <CAAhR5DGFxidA=MuhLrixdHv+D_=KYQquBE2quNCNMZvzijXLiw@mail.gmail.com>
Date: Mon, 16 Jun 2025 17:55:50 -0500
From: Sagi Shahar <sagis@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: linux-coco@...ts.linux.dev, x86@...nel.org, kvm@...r.kernel.org,
seanjc@...gle.com, pbonzini@...hat.com, eddie.dong@...el.com,
kirill.shutemov@...el.com, dave.hansen@...el.com, dan.j.williams@...el.com,
kai.huang@...el.com, isaku.yamahata@...el.com, elena.reshetova@...el.com,
rick.p.edgecombe@...el.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" <kirill.shutemov@...ux.intel.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 08/20] x86/virt/seamldr: Implement FW_UPLOAD sysfs ABI
for TD-Preserving Updates
On Fri, May 23, 2025 at 4:55 AM Chao Gao <chao.gao@...el.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@...el.com>
> Tested-by: Farrah Chen <farrah.chen@...el.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
>
>
Powered by blists - more mailing lists