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

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 for Android: free password hash cracker in your pocket

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

Message-ID: <e32770ac-2bc8-4e09-aad2-0fc219abfc8b@amd.com>
Date: Mon, 8 Sep 2025 16:32:24 -0500
From: "Kalra, Ashish" <ashish.kalra@....com>
To: Tom Lendacky <thomas.lendacky@....com>, tglx@...utronix.de,
 mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
 hpa@...or.com, seanjc@...gle.com, pbonzini@...hat.com,
 herbert@...dor.apana.org.au
Cc: nikunj@....com, davem@...emloft.net, aik@....com, ardb@...nel.org,
 john.allen@....com, michael.roth@....com, Neeraj.Upadhyay@....com,
 linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
 linux-crypto@...r.kernel.org
Subject: Re: [PATCH v3 3/3] crypto: ccp - Add AMD Seamless Firmware Servicing
 (SFS) driver

Hello Tom,

On 9/8/2025 4:18 PM, Tom Lendacky wrote:
> On 8/20/25 17:19, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@....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@....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;
>> +	}
> 

Yes, i was restoring the memory attribute before freeing, but then i realized that if the buffer
is transitioned to HV_Fixed and can't be freed but can still potentially be re-used then who will
setup the buffer to Uncacheable again, but i guess that should not be an issue as SFS driver will 
do that setup again after being unloaded/reloaded again, so i will go ahead and restore the memory
attribute here and in sfs_dev_destroy().

Thanks,
Ashish

> 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;
>> +}