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

[Xu Yilun]

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