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