Re: [PATCH v3 3/3] crypto: ccp - Add AMD Seamless Firmware Servicing (SFS) driver

[Tom Lendacky <thomas.lendacky@amd.com>]

On 8/20/25 17:19, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> AMD Seamless Firmware Servicing (SFS) is a secure method to allow
> non-persistent updates to running firmware and settings without
> requiring BIOS reflash and/or system reset.
> 
> SFS does not address anything that runs on the x86 processors and
> it can be used to update ASP firmware, modules, register settings
> and update firmware for other microprocessors like TMPM, etc.
> 
> SFS driver support adds ioctl support to communicate the SFS
> commands to the ASP/PSP by using the TEE mailbox interface.
> 
> The Seamless Firmware Servicing (SFS) driver is added as a
> PSP sub-device.
> 
> For detailed information, please look at the SFS specifications:
> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58604.pdf
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  drivers/crypto/ccp/Makefile         |   3 +-
>  drivers/crypto/ccp/psp-dev.c        |  20 ++
>  drivers/crypto/ccp/psp-dev.h        |   8 +-
>  drivers/crypto/ccp/sfs.c            | 302 ++++++++++++++++++++++++++++
>  drivers/crypto/ccp/sfs.h            |  47 +++++
>  include/linux/psp-platform-access.h |   2 +
>  include/uapi/linux/psp-sfs.h        |  87 ++++++++
>  7 files changed, 467 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/crypto/ccp/sfs.c
>  create mode 100644 drivers/crypto/ccp/sfs.h
>  create mode 100644 include/uapi/linux/psp-sfs.h
> 

> +
> +	/*
> +	 * SFS command buffer must be mapped as non-cacheable.
> +	 */
> +	ret = set_memory_uc((unsigned long)sfs_dev->command_buf, SFS_NUM_PAGES_CMDBUF);
> +	if (ret) {
> +		dev_dbg(dev, "Set memory uc failed\n");
> +		goto cleanup_cmd_buf;
> +	}

You should restore the memory attribute before freeing it in
sfs_dev_destroy() and below in the cleanup.

Thanks,
Tom

> +
> +	dev_dbg(dev, "Command buffer 0x%px marked uncacheable\n", sfs_dev->command_buf);
> +
> +	psp->sfs_data = sfs_dev;
> +	sfs_dev->dev = dev;
> +	sfs_dev->psp = psp;
> +
> +	ret = sfs_misc_init(sfs_dev);
> +	if (ret)
> +		goto cleanup_cmd_buf;
> +
> +	dev_notice(sfs_dev->dev, "SFS support is available\n");
> +
> +	return 0;
> +
> +cleanup_cmd_buf:
> +	snp_free_hv_fixed_pages(page);
> +
> +cleanup_dev:
> +	psp->sfs_data = NULL;
> +	devm_kfree(dev, sfs_dev);
> +
> +	return ret;
> +}