Re: [Patch v3 6/7] crypto/ccp: Register with fw_uploader and always fail

[Tom Lendacky]

On 10/5/26 11:15, Pratik R. Sampat wrote:
> In preparation for SEV-SNP DOWNLOAD_FIRMWARE_EX live firmware update
> support, add an 'sev' firmware loader that always fails with EBUSY.
> 
> Co-developed-by: Tycho Andersen (AMD) <tycho@xxxxxxxxxx>
> Signed-off-by: Tycho Andersen (AMD) <tycho@xxxxxxxxxx>
> Signed-off-by: Pratik R. Sampat <prsampat@xxxxxxx>

Just some minor comments below.

Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>

> ---
>  drivers/crypto/ccp/sev-dev.c | 80 ++++++++++++++++++++++++++++++++++++
>  drivers/crypto/ccp/sev-dev.h |  2 +
>  2 files changed, 82 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index c40401c861d3..88cf60a9640e 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -2212,6 +2212,80 @@ static int sev_update_firmware(struct device *dev)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_FW_UPLOAD
> +static enum fw_upload_err sev_fw_upload_prepare(struct fw_upload *fw_upload,
> +						const u8 *data, u32 size)
> +{
> +	return FW_UPLOAD_ERR_NONE;
> +}
> +
> +static enum fw_upload_err sev_fw_upload_write(struct fw_upload *fw_upload,
> +					      const u8 *data, u32 offset,
> +					      u32 size, u32 *written)
> +{
> +	return FW_UPLOAD_ERR_BUSY;
> +}
> +
> +static enum fw_upload_err sev_fw_upload_poll_complete(struct fw_upload *fw_upload)
> +{
> +	return FW_UPLOAD_ERR_NONE;
> +}
> +
> +static void sev_fw_upload_cancel(struct fw_upload *fw_upload)
> +{
> +	/* intentional no-op */
> +}
> +
> +static const struct fw_upload_ops sev_fw_upload_ops = {
> +	.prepare = sev_fw_upload_prepare,
> +	.write = sev_fw_upload_write,
> +	.poll_complete = sev_fw_upload_poll_complete,
> +	.cancel = sev_fw_upload_cancel,
> +};
> +
> +/* Serializes registration and unregistration of sev->fwl */
> +static DEFINE_MUTEX(sev_fwl_lock);
> +
> +static void register_sev_fw_uploader(struct sev_device *sev)
> +{
> +	struct fw_upload *fwl;
> +
> +	guard(mutex)(&sev_fwl_lock);
> +
> +	/*
> +	 * The interface only drives DOWNLOAD_FIRMWARE_EX, which is an SNP
> +	 * command, so do not advertise it where SNP is unavailable or the
> +	 * interface already exists.
> +	 */
> +	if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP) || sev->fwl)
> +		return;
> +
> +	fwl = firmware_upload_register(sev->dev, "sev", &sev_fw_upload_ops,
> +				       sev);

Make this all one line.

> +	if (IS_ERR(fwl)) {
> +		dev_err(sev->dev, "SEV firmware upload registration failure: %ld\n",
> +			PTR_ERR(fwl));

and this one.

Thanks,
Tom

> +		return;
> +	}
> +
> +	sev->fwl = fwl;
> +}
> +
> +static void unregister_sev_fw_uploader(struct sev_device *sev)
> +{
> +	guard(mutex)(&sev_fwl_lock);
> +
> +	if (!sev->fwl)
> +		return;
> +
> +	firmware_upload_unregister(sev->fwl);
> +	sev->fwl = NULL;
> +}
> +#else	/* CONFIG_FW_UPLOAD */
> +static void register_sev_fw_uploader(struct sev_device *sev) { }
> +static void unregister_sev_fw_uploader(struct sev_device *sev) { }
> +#endif	/* CONFIG_FW_UPLOAD */
> +
>  static int __sev_snp_shutdown_locked(int *error, bool panic)
>  {
>  	struct psp_device *psp = psp_master;
> @@ -3018,6 +3092,8 @@ void sev_dev_destroy(struct psp_device *psp)
>  	if (!sev)
>  		return;
>  
> +	unregister_sev_fw_uploader(sev);
> +
>  	sev_firmware_shutdown(sev);
>  
>  	if (sev->misc)
> @@ -3081,6 +3157,8 @@ void sev_pci_init(void)
>  			 api_major, api_minor, build,
>  			 sev->api_major, sev->api_minor, sev->build);
>  
> +	register_sev_fw_uploader(sev);
> +
>  	return;
>  
>  err:
> @@ -3096,6 +3174,8 @@ void sev_pci_exit(void)
>  	if (!sev)
>  		return;
>  
> +	unregister_sev_fw_uploader(sev);
> +
>  	sev_firmware_shutdown(sev);
>  }
>  
> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
> index d5e596606def..7ec692e2147e 100644
> --- a/drivers/crypto/ccp/sev-dev.h
> +++ b/drivers/crypto/ccp/sev-dev.h
> @@ -69,6 +69,8 @@ struct sev_device {
>  
>  	struct tsm_dev *tsmdev;
>  	struct sev_tio_status *tio_status;
> +
> +	struct fw_upload *fwl;
>  };
>  
>  int sev_dev_init(struct psp_device *psp);