linux-kernel - Re: [PATCH v3 10/26] coco/tdx-host: Implement FW

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aXy7FTKDBfZ4jXt1@yilunxu-OptiPlex-7050>
Date: Fri, 30 Jan 2026 22:07:17 +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,
	kvm@...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, zhenzhong.duan@...el.com, seanjc@...gle.com,
	rick.p.edgecombe@...el.com, kas@...nel.org,
	dave.hansen@...ux.intel.com, vishal.l.verma@...el.com,
	Farrah Chen <farrah.chen@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v3 10/26] coco/tdx-host: Implement FW_UPLOAD sysfs ABI
 for TDX Module updates

> +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.