[PATCH 0/2] crypto: ccp - Add AMD Seamless Firmware Servicing (SFS) driver

[PATCH 0/2] crypto: ccp - Add AMD Seamless Firmware Servicing (SFS) driver

[Ashish Kalra]

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.

Includes a pre-patch for the SEV driver to add new API interface
to extend the hypervisor fixed pages list passed to SNP_INIT_EX
to allow other PSP sub-devices such as the SFS driver to add 
their HV_Fixed pages to this list.

For detailed information, please look at the SFS specifications:
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58604.pdf

Ashish Kalra (2):
  crypto: ccp - Add new API for extending HV_Fixed Pages
  crypto: ccp - Add AMD Seamless Firmware Servicing (SFS) driver

 drivers/crypto/ccp/Makefile         |   3 +-
 drivers/crypto/ccp/psp-dev.c        |  20 ++
 drivers/crypto/ccp/psp-dev.h        |   8 +-
 drivers/crypto/ccp/sev-dev.c        |  88 ++++++++
 drivers/crypto/ccp/sev-dev.h        |   3 +
 drivers/crypto/ccp/sfs.c            | 316 ++++++++++++++++++++++++++++
 drivers/crypto/ccp/sfs.h            |  53 +++++
 include/linux/psp-platform-access.h |   2 +
 include/uapi/linux/psp-sfs.h        |  87 ++++++++
 9 files changed, 578 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

-- 
2.34.1

[PATCH 1/2] crypto: ccp - Add new API for extending HV_Fixed Pages

[Ashish Kalra]

From: Ashish Kalra <ashish.kalra@amd.com>

Implement new API to add support for extending the HV_Fixed pages list
passed to SNP_INIT_EX.

Adds a simple list based interface to extend the HV_Fixed pages list
for PSP sub-devices such as the SFS driver.

Suggested-by: Thomas.Lendacky@amd.com <Thomas.Lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 88 ++++++++++++++++++++++++++++++++++++
 drivers/crypto/ccp/sev-dev.h |  3 ++
 2 files changed, 91 insertions(+)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index e058ba027792..c3ff40cd7a96 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -82,6 +82,14 @@ MODULE_FIRMWARE("amd/amd_sev_fam19h_model1xh.sbin"); /* 4th gen EPYC */
 static bool psp_dead;
 static int psp_timeout;
 
+struct snp_hv_fixed_pages_entry {
+	u64 base;
+	int npages;
+	struct list_head list;
+};
+static LIST_HEAD(snp_hv_fixed_pages);
+static DEFINE_SPINLOCK(snp_hv_fixed_pages_lock);
+
 /* Trusted Memory Region (TMR):
  *   The TMR is a 1MB area that must be 1MB aligned.  Use the page allocator
  *   to allocate the memory, which will return aligned memory for the specified
@@ -1073,6 +1081,76 @@ static void snp_set_hsave_pa(void *arg)
 	wrmsrq(MSR_VM_HSAVE_PA, 0);
 }
 
+int snp_insert_hypervisor_fixed_pages_list(u64 paddr, int npages)
+{
+	struct snp_hv_fixed_pages_entry *entry;
+
+	spin_lock(&snp_hv_fixed_pages_lock);
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry) {
+		spin_unlock(&snp_hv_fixed_pages_lock);
+		return -ENOMEM;
+	}
+	entry->base = paddr;
+	entry->npages = npages;
+	list_add_tail(&entry->list, &snp_hv_fixed_pages);
+
+	spin_unlock(&snp_hv_fixed_pages_lock);
+
+	return 0;
+}
+
+void snp_delete_hypervisor_fixed_pages_list(u64 paddr)
+{
+	struct snp_hv_fixed_pages_entry *entry, *nentry;
+
+	spin_lock(&snp_hv_fixed_pages_lock);
+	list_for_each_entry_safe(entry, nentry, &snp_hv_fixed_pages, list) {
+		if (entry->base == paddr) {
+			list_del(&entry->list);
+			kfree(entry);
+			break;
+		}
+	}
+	spin_unlock(&snp_hv_fixed_pages_lock);
+}
+
+static int snp_extend_hypervisor_fixed_pages(struct sev_data_range_list *range_list)
+{
+	struct sev_data_range *range = &range_list->ranges[range_list->num_elements];
+	struct snp_hv_fixed_pages_entry *entry;
+	int new_element_count, ret = 0;
+
+	spin_lock(&snp_hv_fixed_pages_lock);
+	if (list_empty(&snp_hv_fixed_pages))
+		goto out;
+
+	new_element_count = list_count_nodes(&snp_hv_fixed_pages) +
+			    range_list->num_elements;
+
+	/*
+	 * Ensure the list of HV_FIXED pages that will be passed to firmware
+	 * do not exceed the page-sized argument buffer.
+	 */
+	if (new_element_count * sizeof(struct sev_data_range) +
+	    sizeof(struct sev_data_range_list) > PAGE_SIZE) {
+		ret = -E2BIG;
+		goto out;
+	}
+
+	list_for_each_entry(entry, &snp_hv_fixed_pages, list) {
+		range->base = entry->base;
+		range->page_count = entry->npages;
+		range++;
+	}
+	range_list->num_elements = new_element_count;
+out:
+	spin_unlock(&snp_hv_fixed_pages_lock);
+
+	return ret;
+}
+
 static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
 {
 	struct sev_data_range_list *range_list = arg;
@@ -1163,6 +1241,16 @@ static int __sev_snp_init_locked(int *error)
 			return rc;
 		}
 
+		/*
+		 * Extend the HV_Fixed pages list with HV_Fixed pages added from other
+		 * PSP sub-devices such as SFS. Warn if the list can't be extended
+		 * but continue with SNP_INIT_EX.
+		 */
+		rc = snp_extend_hypervisor_fixed_pages(snp_range_list);
+		if (rc)
+			dev_warn(sev->dev,
+				 "SEV: SNP_INIT_EX extend HV_Fixed pages failed rc = %d\n", rc);
+
 		memset(&data, 0, sizeof(data));
 		data.init_rmp = 1;
 		data.list_paddr_en = 1;
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index 3e4e5574e88a..444d7fffd801 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -65,4 +65,7 @@ void sev_dev_destroy(struct psp_device *psp);
 void sev_pci_init(void);
 void sev_pci_exit(void);
 
+int snp_insert_hypervisor_fixed_pages_list(u64 paddr, int npages);
+void snp_delete_hypervisor_fixed_pages_list(u64 paddr);
+
 #endif /* __SEV_DEV_H */
-- 
2.34.1

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

[Ashish Kalra]

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            | 316 ++++++++++++++++++++++++++++
 drivers/crypto/ccp/sfs.h            |  53 +++++
 include/linux/psp-platform-access.h |   2 +
 include/uapi/linux/psp-sfs.h        |  87 ++++++++
 7 files changed, 487 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

diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
index 394484929dae..9b876bfb1289 100644
--- a/drivers/crypto/ccp/Makefile
+++ b/drivers/crypto/ccp/Makefile
@@ -13,7 +13,8 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o \
                                    tee-dev.o \
                                    platform-access.o \
                                    dbc.o \
-                                   hsti.o
+                                   hsti.o \
+				   sfs.o
 
 obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
 ccp-crypto-objs := ccp-crypto-main.o \
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index 1c5a7189631e..8c4ad225ad67 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -17,6 +17,7 @@
 #include "psp-dev.h"
 #include "sev-dev.h"
 #include "tee-dev.h"
+#include "sfs.h"
 #include "platform-access.h"
 #include "dbc.h"
 #include "hsti.h"
@@ -182,6 +183,17 @@ static int psp_check_tee_support(struct psp_device *psp)
 	return 0;
 }
 
+static int psp_check_sfs_support(struct psp_device *psp)
+{
+	/* Check if device supports SFS feature */
+	if (!psp->capability.sev) {
+		dev_dbg(psp->dev, "psp does not support SFS\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int psp_init(struct psp_device *psp)
 {
 	int ret;
@@ -198,6 +210,12 @@ static int psp_init(struct psp_device *psp)
 			return ret;
 	}
 
+	if (!psp_check_sfs_support(psp)) {
+		ret = sfs_dev_init(psp);
+		if (ret)
+			return ret;
+	}
+
 	if (psp->vdata->platform_access) {
 		ret = platform_access_dev_init(psp);
 		if (ret)
@@ -302,6 +320,8 @@ void psp_dev_destroy(struct sp_device *sp)
 
 	tee_dev_destroy(psp);
 
+	sfs_dev_destroy(psp);
+
 	dbc_dev_destroy(psp);
 
 	platform_access_dev_destroy(psp);
diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
index e43ce87ede76..268c83f298cb 100644
--- a/drivers/crypto/ccp/psp-dev.h
+++ b/drivers/crypto/ccp/psp-dev.h
@@ -32,7 +32,8 @@ union psp_cap_register {
 		unsigned int sev			:1,
 			     tee			:1,
 			     dbc_thru_ext		:1,
-			     rsvd1			:4,
+			     sfs			:1,
+			     rsvd1			:3,
 			     security_reporting		:1,
 			     fused_part			:1,
 			     rsvd2			:1,
@@ -68,6 +69,7 @@ struct psp_device {
 	void *tee_data;
 	void *platform_access_data;
 	void *dbc_data;
+	void *sfs_data;
 
 	union psp_cap_register capability;
 };
@@ -118,12 +120,16 @@ struct psp_ext_request {
  * @PSP_SUB_CMD_DBC_SET_UID:		Set UID for DBC
  * @PSP_SUB_CMD_DBC_GET_PARAMETER:	Get parameter from DBC
  * @PSP_SUB_CMD_DBC_SET_PARAMETER:	Set parameter for DBC
+ * @PSP_SUB_CMD_SFS_GET_FW_VERS:	Get firmware versions for ASP and other MP
+ * @PSP_SUB_CMD_SFS_UPDATE:		Command to load, verify and execute SFS package
  */
 enum psp_sub_cmd {
 	PSP_SUB_CMD_DBC_GET_NONCE	= PSP_DYNAMIC_BOOST_GET_NONCE,
 	PSP_SUB_CMD_DBC_SET_UID		= PSP_DYNAMIC_BOOST_SET_UID,
 	PSP_SUB_CMD_DBC_GET_PARAMETER	= PSP_DYNAMIC_BOOST_GET_PARAMETER,
 	PSP_SUB_CMD_DBC_SET_PARAMETER	= PSP_DYNAMIC_BOOST_SET_PARAMETER,
+	PSP_SUB_CMD_SFS_GET_FW_VERS	= PSP_SFS_GET_FW_VERSIONS,
+	PSP_SUB_CMD_SFS_UPDATE		= PSP_SFS_UPDATE,
 };
 
 int psp_extended_mailbox_cmd(struct psp_device *psp, unsigned int timeout_msecs,
diff --git a/drivers/crypto/ccp/sfs.c b/drivers/crypto/ccp/sfs.c
new file mode 100644
index 000000000000..cbca01a884e1
--- /dev/null
+++ b/drivers/crypto/ccp/sfs.c
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AMD Secure Processor Seamless Firmware Servicing support.
+ *
+ * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
+ *
+ * Author: Ashish Kalra <ashish.kalra@amd.com>
+ */
+
+#include <linux/firmware.h>
+
+#include "sfs.h"
+#include "sev-dev.h"
+
+#define SFS_DEFAULT_TIMEOUT		(10 * MSEC_PER_SEC)
+#define SFS_MAX_PAYLOAD_SIZE		(2 * 1024 * 1024)
+#define ORDER_2MB 9
+
+/* SFS Status values */
+#define SFS_SUCCESS			0x00
+#define SFS_INVALID_TOTAL_SIZE		0x02
+#define SFS_INVALID_PKG_SIZE		0x04
+#define SFS_DISABLED			0x05
+#define SFS_INVALID_CUST_SIGN		0x06
+#define SFS_INVALID_AMD_SIGN		0x07
+#define SFS_INTERNAL_ERROR		0x08
+#define SFS_CUST_SIGN_NOT_ALLOWED	0x09
+#define SFS_INVALID_BASE_PATCH_LVL	0x0A
+#define SFS_INVALID_CURR_PATCH_LVL	0x0B
+#define SFS_INVALID_NEW_PATCH_LVL	0x0C
+#define SFS_INVALID_SUBCOMMAND		0x0D
+#define SFS_PROTECTION_FAIL		0x0E
+#define SFS_BUSY			0x0F
+#define SFS_FW_VERSION_MISMATCH	0x10
+#define SFS_SYS_VERSION_MISMATCH	0x11
+#define SFS_SEV_STILL_INITIALIZED	0x12
+
+static bool sfs_initialized;
+
+static int send_sfs_cmd(struct psp_sfs_device *sfs_dev, int msg)
+{
+	int ret;
+
+	*sfs_dev->result = 0;
+	sfs_dev->command_hdr->ext_req.header.sub_cmd_id = msg;
+
+	ret = psp_extended_mailbox_cmd(sfs_dev->psp,
+					SFS_DEFAULT_TIMEOUT,
+					(struct psp_ext_request *)sfs_dev->command_hdr);
+	if (ret == -EIO) {
+		dev_dbg(sfs_dev->dev,
+			 "msg 0x%x failed with PSP error: 0x%x\n",
+			 msg, *sfs_dev->result);
+		dev_dbg(sfs_dev->dev,
+			 "msg 0x%x extended status: 0x%x\n",
+			 msg, *(u32 *)sfs_dev->payload);
+	}
+
+	return ret;
+}
+
+static int send_sfs_get_fw_versions(struct psp_sfs_device *sfs_dev)
+{
+	int ret;
+
+	sfs_dev->payload_size = &sfs_dev->command_hdr->ext_req.header.payload_size;
+	sfs_dev->result = &sfs_dev->command_hdr->ext_req.header.status;
+	sfs_dev->payload = &sfs_dev->command_hdr->ext_req.buf;
+	sfs_dev->pkg_hdr = (void *)sfs_dev->command_hdr + PAGE_SIZE;
+	sfs_dev->header_size = sizeof(struct psp_ext_req_buffer_hdr);
+
+	/*
+	 * SFS_GET_FW_VERSIONS command needs the output buffer to be
+	 * initialized to 0xC7 in every byte.
+	 */
+	memset(sfs_dev->pkg_hdr, 0xc7, PAGE_SIZE);
+	*sfs_dev->payload_size = 2 * PAGE_SIZE;
+
+	ret = send_sfs_cmd(sfs_dev, PSP_SFS_GET_FW_VERSIONS);
+
+	return ret;
+}
+
+static int send_sfs_update_package(struct psp_sfs_device *sfs_dev, char *payload_name)
+{
+	char payload_path[PAYLOAD_NAME_SIZE];
+	const struct firmware *firmware;
+	unsigned long package_size;
+	int ret;
+
+	sprintf(payload_path, "amd/%s", payload_name);
+
+	ret = firmware_request_nowarn(&firmware, payload_path, sfs_dev->dev);
+	if (ret < 0) {
+		dev_warn(sfs_dev->dev, "firmware request fail %d\n", ret);
+		return -ENOENT;
+	}
+
+	/* SFS Update Package should be 64KB aligned */
+	package_size = ALIGN(firmware->size + PAGE_SIZE, 0x10000U);
+
+	/*
+	 * SFS command buffer is a pre-allocated 2MB buffer, fail update package
+	 * if SFS payload is larger than the pre-allocated command buffer.
+	 */
+	if (package_size > SFS_MAX_PAYLOAD_SIZE) {
+		dev_warn(sfs_dev->dev,
+			 "SFS payload size %ld larger than maximum supported payload size of 2MB\n",
+			 package_size);
+		return -ENOMEM;
+	}
+
+	sfs_dev->payload_size = &sfs_dev->command_hdr->ext_req.header.payload_size;
+	sfs_dev->result = &sfs_dev->command_hdr->ext_req.header.status;
+	sfs_dev->payload = &sfs_dev->command_hdr->ext_req.buf;
+	sfs_dev->pkg_hdr = (void *)sfs_dev->command_hdr + PAGE_SIZE;
+	sfs_dev->header_size = sizeof(struct psp_ext_req_buffer_hdr);
+
+	/*
+	 * Copy firmware data to a kernel allocated contiguous
+	 * memory region.
+	 */
+	memcpy(sfs_dev->pkg_hdr, firmware->data, firmware->size);
+	*sfs_dev->payload_size = package_size;
+
+	ret = send_sfs_cmd(sfs_dev, PSP_SFS_UPDATE);
+
+	release_firmware(firmware);
+	return ret;
+}
+
+static long sfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	struct psp_device *psp_master = psp_get_master_device();
+	void __user *argp = (void __user *)arg;
+	char payload_name[PAYLOAD_NAME_SIZE];
+	struct psp_sfs_device *sfs_dev;
+	int ret;
+
+	if (!psp_master || !psp_master->sfs_data)
+		return -ENODEV;
+	sfs_dev = psp_master->sfs_data;
+
+	if (!sfs_initialized)
+		return -EINVAL;
+
+	mutex_lock(&sfs_dev->ioctl_mutex);
+
+	switch (cmd) {
+	case SFSIOCFWVERS:
+		dev_dbg(sfs_dev->dev, "in SFSIOCFWVERS\n");
+
+		ret = send_sfs_get_fw_versions(sfs_dev);
+		if (ret && ret != -EIO)
+			goto unlock;
+		/*
+		 * return SFS status and extended status back to userspace
+		 * if PSP status indicated command error.
+		 */
+		if (copy_to_user(argp, sfs_dev->pkg_hdr, PAGE_SIZE))
+			ret = -EFAULT;
+		if (copy_to_user(argp + PAGE_SIZE, sfs_dev->result, sizeof(u32)))
+			ret = -EFAULT;
+		if (copy_to_user(argp + PAGE_SIZE + sizeof(u32), sfs_dev->payload, sizeof(u32)))
+			ret = -EFAULT;
+		break;
+	case SFSIOCUPDATEPKG:
+		dev_dbg(sfs_dev->dev, "in SFSIOCUPDATEPKG\n");
+
+		if (copy_from_user(payload_name, argp, PAYLOAD_NAME_SIZE)) {
+			ret = -EFAULT;
+			goto unlock;
+		}
+
+		ret = send_sfs_update_package(sfs_dev, payload_name);
+		if (ret && ret != -EIO)
+			goto unlock;
+		/*
+		 * return SFS status and extended status back to userspace
+		 * if PSP status indicated command error.
+		 */
+		if (copy_to_user(argp + PAYLOAD_NAME_SIZE, sfs_dev->result, sizeof(u32)))
+			ret = -EFAULT;
+		if (copy_to_user(argp + PAYLOAD_NAME_SIZE + sizeof(u32), sfs_dev->payload,
+				 sizeof(u32)))
+			ret = -EFAULT;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+unlock:
+	mutex_unlock(&sfs_dev->ioctl_mutex);
+
+	return ret;
+}
+
+static const struct file_operations sfs_fops = {
+	.owner	= THIS_MODULE,
+	.unlocked_ioctl = sfs_ioctl,
+};
+
+void sfs_dev_destroy(struct psp_device *psp)
+{
+	struct psp_sfs_device *sfs_dev = psp->sfs_data;
+	int ret;
+
+	if (!sfs_dev)
+		return;
+
+	/*
+	 * TODO: free pre-allocated 2MB command buffer,
+	 * if SEV-SNP is initialized the command buffer has
+	 * been marked HV_Fixed and HV_Fixed pages remain
+	 * in that state till system reset, they cannot be
+	 * released back to the page allocator.
+	 *
+	 */
+	snp_delete_hypervisor_fixed_pages_list(page_to_pfn(sfs_dev->page) << PAGE_SHIFT);
+
+	ret = set_memory_wb((unsigned long)page_address(sfs_dev->page), 512);
+	if (ret)
+		dev_dbg(sfs_dev->dev, "set memory wb failed\n");
+
+	sfs_initialized = false;
+	misc_deregister(&sfs_dev->char_dev);
+	mutex_destroy(&sfs_dev->ioctl_mutex);
+	psp->sfs_data = NULL;
+}
+
+int sfs_dev_init(struct psp_device *psp)
+{
+	struct device *dev = psp->dev;
+	struct psp_sfs_device *sfs_dev;
+	struct page *page;
+	u64 cmd_buf_paddr;
+	int ret;
+
+	/*
+	 * SFS feature support can be detected on multiple devices but the SFS
+	 * FW commands must be issued on the master. During probe, we do not
+	 * know the master hence we create /dev/sfs on the first device probe.
+	 */
+	if (sfs_initialized)
+		return 0;
+
+	sfs_dev = devm_kzalloc(dev, sizeof(*sfs_dev), GFP_KERNEL);
+	if (!sfs_dev)
+		return -ENOMEM;
+
+	BUILD_BUG_ON(sizeof(struct sfs_command) > PAGE_SIZE);
+
+	/*
+	 * Pre-allocate static 2MB command buffer for all SFS commands.
+	 */
+	page = alloc_pages(GFP_KERNEL | __GFP_ZERO, ORDER_2MB);
+	if (!page)
+		return -ENOMEM;
+	sfs_dev->page = page;
+	sfs_dev->command_hdr = page_address(page);
+	cmd_buf_paddr = page_to_pfn(sfs_dev->page) << PAGE_SHIFT;
+
+	/*
+	 * If SEV-SNP is enabled the SFS command buffer needs to
+	 * transitioned to HV_Fixed page state.
+	 */
+	dev_dbg(sfs_dev->dev, "cmdbuf page pa 0x%llx to be marked as HV_Fixed\n",
+		cmd_buf_paddr);
+
+	ret = snp_insert_hypervisor_fixed_pages_list(cmd_buf_paddr, 512);
+	if (ret) {
+		dev_dbg(sfs_dev->dev, "cmdbuf page failed insertion in HV-Fixed page list\n");
+		goto cleanup_cmd_buf;
+	}
+
+	/*
+	 * Buffers used for communication with different processors, x86 to ASP
+	 * in this case must be mapped as non-cacheable.
+	 */
+	ret = set_memory_uc((unsigned long)page_address(page), 512);
+	if (ret) {
+		dev_dbg(sfs_dev->dev, "set memory uc failed\n");
+		goto cleanup_cmd_buf_after_hv_fixed;
+	}
+
+	dev_dbg(sfs_dev->dev, "cmdbuf page va 0x%lx marked as UnCacheable\n",
+		(unsigned long)sfs_dev->command_hdr);
+
+	psp->sfs_data = sfs_dev;
+	sfs_dev->dev = dev;
+	sfs_dev->psp = psp;
+
+	dev_dbg(sfs_dev->dev, "seamless firmware serviving support is available\n");
+
+	sfs_dev->char_dev.minor = MISC_DYNAMIC_MINOR;
+	sfs_dev->char_dev.name = "sfs";
+	sfs_dev->char_dev.fops = &sfs_fops;
+	sfs_dev->char_dev.mode = 0600;
+	ret = misc_register(&sfs_dev->char_dev);
+	if (ret)
+		goto cleanup_cmd_buf_after_hv_fixed;
+
+	mutex_init(&sfs_dev->ioctl_mutex);
+	sfs_initialized = true;
+
+	return 0;
+
+cleanup_cmd_buf_after_hv_fixed:
+	snp_delete_hypervisor_fixed_pages_list(cmd_buf_paddr);
+cleanup_cmd_buf:
+	__free_pages(page, ORDER_2MB);
+	psp->sfs_data = NULL;
+	devm_kfree(dev, sfs_dev);
+
+	return ret;
+}
diff --git a/drivers/crypto/ccp/sfs.h b/drivers/crypto/ccp/sfs.h
new file mode 100644
index 000000000000..89b7792af076
--- /dev/null
+++ b/drivers/crypto/ccp/sfs.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AMD Platform Security Processor (PSP) Seamless Firmware (SFS) Support.
+ *
+ * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
+ *
+ * Author: Ashish Kalra <ashish.kalra@amd.com>
+ */
+
+#ifndef __SFS_H__
+#define __SFS_H__
+
+#include <uapi/linux/psp-sfs.h>
+
+#include <linux/device.h>
+#include <linux/miscdevice.h>
+#include <linux/psp-sev.h>
+#include <linux/psp-platform-access.h>
+#include <linux/set_memory.h>
+
+#include <asm/sev.h>
+
+#include "psp-dev.h"
+
+struct psp_sfs_device {
+	struct device *dev;
+	struct psp_device *psp;
+
+	struct sfs_command *command_hdr;
+
+	struct mutex ioctl_mutex;
+
+	struct miscdevice char_dev;
+
+	struct page *page;
+
+	/* used to abstract communication path */
+	u32	header_size;
+	u32	*payload_size;
+	u32	*result;
+	void	*payload;
+	void	*pkg_hdr;
+};
+
+struct sfs_command {
+	struct psp_ext_request ext_req;
+};
+
+void sfs_dev_destroy(struct psp_device *psp);
+int sfs_dev_init(struct psp_device *psp);
+void sfs_pci_init(void);
+
+#endif /* __SFS_H__ */
diff --git a/include/linux/psp-platform-access.h b/include/linux/psp-platform-access.h
index 1504fb012c05..540abf7de048 100644
--- a/include/linux/psp-platform-access.h
+++ b/include/linux/psp-platform-access.h
@@ -7,6 +7,8 @@
 
 enum psp_platform_access_msg {
 	PSP_CMD_NONE			= 0x0,
+	PSP_SFS_GET_FW_VERSIONS,
+	PSP_SFS_UPDATE,
 	PSP_CMD_HSTI_QUERY		= 0x14,
 	PSP_I2C_REQ_BUS_CMD		= 0x64,
 	PSP_DYNAMIC_BOOST_GET_NONCE,
diff --git a/include/uapi/linux/psp-sfs.h b/include/uapi/linux/psp-sfs.h
new file mode 100644
index 000000000000..e752fa041683
--- /dev/null
+++ b/include/uapi/linux/psp-sfs.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+/*
+ * Userspace interface for AMD Seamless Firmware Servicing (SFS)
+ *
+ * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
+ *
+ * Author: Ashish Kalra <ashish.kalra@amd.com>
+ */
+
+#ifndef __PSP_SFS_USER_H__
+#define __PSP_SFS_USER_H__
+
+#include <linux/types.h>
+
+/**
+ * SFS: AMD Seamless Firmware Support (SFS) interface
+ */
+
+#define PAYLOAD_NAME_SIZE		64
+#define TEE_EXT_CMD_BUFFER_SIZE	4096
+
+/**
+ * struct sfs_user_get_fw_versions - get current level of base firmware (output).
+ * @blob:                  current level of base firmware for ASP and patch levels (input/output).
+ * @sfs_status:            32-bit SFS status value (output).
+ * @sfs_extended_status:   32-bit SFS extended status value (output).
+ */
+struct sfs_user_get_fw_versions {
+	__u8	blob[TEE_EXT_CMD_BUFFER_SIZE];
+	__u32	sfs_status;
+	__u32	sfs_extended_status;
+} __packed;
+
+/**
+ * struct sfs_user_update_package - update SFS package (input).
+ * @payload_name:          name of SFS package to load, verify and execute (input).
+ * @sfs_status:            32-bit SFS status value (output).
+ * @sfs_extended_status:   32-bit SFS extended status value (output).
+ */
+struct sfs_user_update_package {
+	char	payload_name[PAYLOAD_NAME_SIZE];
+	__u32	sfs_status;
+	__u32	sfs_extended_status;
+} __packed;
+
+/**
+ * Seamless Firmware Support (SFS) IOC
+ *
+ * possible return codes for all SFS IOCTLs:
+ *  0:          success
+ *  -EINVAL:    invalid input
+ *  -E2BIG:     excess data passed
+ *  -EFAULT:    failed to copy to/from userspace
+ *  -EBUSY:     mailbox in recovery or in use
+ *  -ENODEV:    driver not bound with PSP device
+ *  -EACCES:    request isn't authorized
+ *  -EINVAL:    invalid parameter
+ *  -ETIMEDOUT: request timed out
+ *  -EAGAIN:    invalid request for state machine
+ *  -ENOENT:    not implemented
+ *  -ENFILE:    overflow
+ *  -EPERM:     invalid signature
+ *  -EIO:       unknown error
+ */
+#define SFS_IOC_TYPE	'S'
+
+/**
+ * SFSIOCFWVERS - returns blob containing FW versions
+ *                ASP provides the current level of Base Firmware for the ASP
+ *                and the other microprocessors as well as current patch
+ *                level(s).
+ */
+#define SFSIOCFWVERS	_IOWR(SFS_IOC_TYPE, 0x1, struct sfs_user_get_fw_versions)
+
+/**
+ * SFSIOCUPDATEPKG - updates package/payload
+ *                   ASP loads, verifies and executes the SFS package.
+ *                   By default, the SFS package/payload is loaded from
+ *                   /lib/firmware/amd, but alternative firmware loading
+ *                   path can be specified using kernel parameter
+ *                   firmware_class.path or the firmware loading path
+ *                   can be customized using sysfs file:
+ *                   /sys/module/firmware_class/parameters/path.
+ */
+#define SFSIOCUPDATEPKG	_IOWR(SFS_IOC_TYPE, 0x2, struct sfs_user_update_package)
+
+#endif /* __PSP_SFS_USER_H__ */
-- 
2.34.1

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

[Mario Limonciello]

On 7/24/25 4:16 PM, 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            | 316 ++++++++++++++++++++++++++++
>   drivers/crypto/ccp/sfs.h            |  53 +++++
>   include/linux/psp-platform-access.h |   2 +
>   include/uapi/linux/psp-sfs.h        |  87 ++++++++
>   7 files changed, 487 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
> 
> diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
> index 394484929dae..9b876bfb1289 100644
> --- a/drivers/crypto/ccp/Makefile
> +++ b/drivers/crypto/ccp/Makefile
> @@ -13,7 +13,8 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o \
>                                      tee-dev.o \
>                                      platform-access.o \
>                                      dbc.o \
> -                                   hsti.o
> +                                   hsti.o \
> +				   sfs.o
>   
>   obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
>   ccp-crypto-objs := ccp-crypto-main.o \
> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
> index 1c5a7189631e..8c4ad225ad67 100644
> --- a/drivers/crypto/ccp/psp-dev.c
> +++ b/drivers/crypto/ccp/psp-dev.c
> @@ -17,6 +17,7 @@
>   #include "psp-dev.h"
>   #include "sev-dev.h"
>   #include "tee-dev.h"
> +#include "sfs.h"
>   #include "platform-access.h"
>   #include "dbc.h"
>   #include "hsti.h"
> @@ -182,6 +183,17 @@ static int psp_check_tee_support(struct psp_device *psp)
>   	return 0;
>   }
>   
> +static int psp_check_sfs_support(struct psp_device *psp)
> +{
> +	/* Check if device supports SFS feature */
> +	if (!psp->capability.sev) {

Should this be psp->capability.sfs?

> +		dev_dbg(psp->dev, "psp does not support SFS\n");
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
>   static int psp_init(struct psp_device *psp)
>   {
>   	int ret;
> @@ -198,6 +210,12 @@ static int psp_init(struct psp_device *psp)
>   			return ret;
>   	}
>   
> +	if (!psp_check_sfs_support(psp)) {
> +		ret = sfs_dev_init(psp);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	if (psp->vdata->platform_access) {
>   		ret = platform_access_dev_init(psp);
>   		if (ret)
> @@ -302,6 +320,8 @@ void psp_dev_destroy(struct sp_device *sp)
>   
>   	tee_dev_destroy(psp);
>   
> +	sfs_dev_destroy(psp);
> +
>   	dbc_dev_destroy(psp);
>   
>   	platform_access_dev_destroy(psp);
> diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
> index e43ce87ede76..268c83f298cb 100644
> --- a/drivers/crypto/ccp/psp-dev.h
> +++ b/drivers/crypto/ccp/psp-dev.h
> @@ -32,7 +32,8 @@ union psp_cap_register {
>   		unsigned int sev			:1,
>   			     tee			:1,
>   			     dbc_thru_ext		:1,
> -			     rsvd1			:4,
> +			     sfs			:1,
> +			     rsvd1			:3,
>   			     security_reporting		:1,
>   			     fused_part			:1,
>   			     rsvd2			:1,
> @@ -68,6 +69,7 @@ struct psp_device {
>   	void *tee_data;
>   	void *platform_access_data;
>   	void *dbc_data;
> +	void *sfs_data;
>   
>   	union psp_cap_register capability;
>   };
> @@ -118,12 +120,16 @@ struct psp_ext_request {
>    * @PSP_SUB_CMD_DBC_SET_UID:		Set UID for DBC
>    * @PSP_SUB_CMD_DBC_GET_PARAMETER:	Get parameter from DBC
>    * @PSP_SUB_CMD_DBC_SET_PARAMETER:	Set parameter for DBC
> + * @PSP_SUB_CMD_SFS_GET_FW_VERS:	Get firmware versions for ASP and other MP
> + * @PSP_SUB_CMD_SFS_UPDATE:		Command to load, verify and execute SFS package
>    */
>   enum psp_sub_cmd {
>   	PSP_SUB_CMD_DBC_GET_NONCE	= PSP_DYNAMIC_BOOST_GET_NONCE,
>   	PSP_SUB_CMD_DBC_SET_UID		= PSP_DYNAMIC_BOOST_SET_UID,
>   	PSP_SUB_CMD_DBC_GET_PARAMETER	= PSP_DYNAMIC_BOOST_GET_PARAMETER,
>   	PSP_SUB_CMD_DBC_SET_PARAMETER	= PSP_DYNAMIC_BOOST_SET_PARAMETER,
> +	PSP_SUB_CMD_SFS_GET_FW_VERS	= PSP_SFS_GET_FW_VERSIONS,
> +	PSP_SUB_CMD_SFS_UPDATE		= PSP_SFS_UPDATE,
>   };
>   
>   int psp_extended_mailbox_cmd(struct psp_device *psp, unsigned int timeout_msecs,
> diff --git a/drivers/crypto/ccp/sfs.c b/drivers/crypto/ccp/sfs.c
> new file mode 100644
> index 000000000000..cbca01a884e1
> --- /dev/null
> +++ b/drivers/crypto/ccp/sfs.c
> @@ -0,0 +1,316 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AMD Secure Processor Seamless Firmware Servicing support.
> + *
> + * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
> + *
> + * Author: Ashish Kalra <ashish.kalra@amd.com>
> + */
> +
> +#include <linux/firmware.h>
> +
> +#include "sfs.h"
> +#include "sev-dev.h"
> +
> +#define SFS_DEFAULT_TIMEOUT		(10 * MSEC_PER_SEC)
> +#define SFS_MAX_PAYLOAD_SIZE		(2 * 1024 * 1024)
> +#define ORDER_2MB 9
> +
> +/* SFS Status values */
> +#define SFS_SUCCESS			0x00
> +#define SFS_INVALID_TOTAL_SIZE		0x02
> +#define SFS_INVALID_PKG_SIZE		0x04
> +#define SFS_DISABLED			0x05
> +#define SFS_INVALID_CUST_SIGN		0x06
> +#define SFS_INVALID_AMD_SIGN		0x07
> +#define SFS_INTERNAL_ERROR		0x08
> +#define SFS_CUST_SIGN_NOT_ALLOWED	0x09
> +#define SFS_INVALID_BASE_PATCH_LVL	0x0A
> +#define SFS_INVALID_CURR_PATCH_LVL	0x0B
> +#define SFS_INVALID_NEW_PATCH_LVL	0x0C
> +#define SFS_INVALID_SUBCOMMAND		0x0D
> +#define SFS_PROTECTION_FAIL		0x0E
> +#define SFS_BUSY			0x0F
> +#define SFS_FW_VERSION_MISMATCH	0x10
> +#define SFS_SYS_VERSION_MISMATCH	0x11
> +#define SFS_SEV_STILL_INITIALIZED	0x12
> +
> +static bool sfs_initialized;
> +
> +static int send_sfs_cmd(struct psp_sfs_device *sfs_dev, int msg)
> +{
> +	int ret;
> +
> +	*sfs_dev->result = 0;
> +	sfs_dev->command_hdr->ext_req.header.sub_cmd_id = msg;
> +
> +	ret = psp_extended_mailbox_cmd(sfs_dev->psp,
> +					SFS_DEFAULT_TIMEOUT,
> +					(struct psp_ext_request *)sfs_dev->command_hdr);
> +	if (ret == -EIO) {
> +		dev_dbg(sfs_dev->dev,
> +			 "msg 0x%x failed with PSP error: 0x%x\n",
> +			 msg, *sfs_dev->result);
> +		dev_dbg(sfs_dev->dev,
> +			 "msg 0x%x extended status: 0x%x\n",
> +			 msg, *(u32 *)sfs_dev->payload);
> +	}
> +
> +	return ret;
> +}
> +
> +static int send_sfs_get_fw_versions(struct psp_sfs_device *sfs_dev)
> +{
> +	int ret;
> +
> +	sfs_dev->payload_size = &sfs_dev->command_hdr->ext_req.header.payload_size;
> +	sfs_dev->result = &sfs_dev->command_hdr->ext_req.header.status;
> +	sfs_dev->payload = &sfs_dev->command_hdr->ext_req.buf;
> +	sfs_dev->pkg_hdr = (void *)sfs_dev->command_hdr + PAGE_SIZE;
> +	sfs_dev->header_size = sizeof(struct psp_ext_req_buffer_hdr);
> +
> +	/*
> +	 * SFS_GET_FW_VERSIONS command needs the output buffer to be
> +	 * initialized to 0xC7 in every byte.
> +	 */
> +	memset(sfs_dev->pkg_hdr, 0xc7, PAGE_SIZE);
> +	*sfs_dev->payload_size = 2 * PAGE_SIZE;
> +
> +	ret = send_sfs_cmd(sfs_dev, PSP_SFS_GET_FW_VERSIONS);
> +
> +	return ret;
> +}
> +
> +static int send_sfs_update_package(struct psp_sfs_device *sfs_dev, char *payload_name)
> +{
> +	char payload_path[PAYLOAD_NAME_SIZE];
> +	const struct firmware *firmware;
> +	unsigned long package_size;
> +	int ret;
> +
> +	sprintf(payload_path, "amd/%s", payload_name);
> +
> +	ret = firmware_request_nowarn(&firmware, payload_path, sfs_dev->dev);
> +	if (ret < 0) {
> +		dev_warn(sfs_dev->dev, "firmware request fail %d\n", ret);
> +		return -ENOENT;
> +	}
> +
> +	/* SFS Update Package should be 64KB aligned */
> +	package_size = ALIGN(firmware->size + PAGE_SIZE, 0x10000U);
> +
> +	/*
> +	 * SFS command buffer is a pre-allocated 2MB buffer, fail update package
> +	 * if SFS payload is larger than the pre-allocated command buffer.
> +	 */
> +	if (package_size > SFS_MAX_PAYLOAD_SIZE) {
> +		dev_warn(sfs_dev->dev,
> +			 "SFS payload size %ld larger than maximum supported payload size of 2MB\n",
> +			 package_size);
> +		return -ENOMEM;
> +	}
> +
> +	sfs_dev->payload_size = &sfs_dev->command_hdr->ext_req.header.payload_size;
> +	sfs_dev->result = &sfs_dev->command_hdr->ext_req.header.status;
> +	sfs_dev->payload = &sfs_dev->command_hdr->ext_req.buf;
> +	sfs_dev->pkg_hdr = (void *)sfs_dev->command_hdr + PAGE_SIZE;
> +	sfs_dev->header_size = sizeof(struct psp_ext_req_buffer_hdr);
> +
> +	/*
> +	 * Copy firmware data to a kernel allocated contiguous
> +	 * memory region.
> +	 */
> +	memcpy(sfs_dev->pkg_hdr, firmware->data, firmware->size);
> +	*sfs_dev->payload_size = package_size;
> +
> +	ret = send_sfs_cmd(sfs_dev, PSP_SFS_UPDATE);
> +
> +	release_firmware(firmware);
> +	return ret;
> +}
> +
> +static long sfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> +	struct psp_device *psp_master = psp_get_master_device();
> +	void __user *argp = (void __user *)arg;
> +	char payload_name[PAYLOAD_NAME_SIZE];
> +	struct psp_sfs_device *sfs_dev;
> +	int ret;
> +
> +	if (!psp_master || !psp_master->sfs_data)
> +		return -ENODEV;
> +	sfs_dev = psp_master->sfs_data;
> +
> +	if (!sfs_initialized)
> +		return -EINVAL;
> +
> +	mutex_lock(&sfs_dev->ioctl_mutex);

As it's new code, how about using guard(mutex) instead?  Then you don't 
need to have goto unlock in the failure cases and can just return the 
error directly.

> +
> +	switch (cmd) {
> +	case SFSIOCFWVERS:
> +		dev_dbg(sfs_dev->dev, "in SFSIOCFWVERS\n");
> +
> +		ret = send_sfs_get_fw_versions(sfs_dev);
> +		if (ret && ret != -EIO)
> +			goto unlock;
> +		/*
> +		 * return SFS status and extended status back to userspace
> +		 * if PSP status indicated command error.
> +		 */
> +		if (copy_to_user(argp, sfs_dev->pkg_hdr, PAGE_SIZE))
> +			ret = -EFAULT;
> +		if (copy_to_user(argp + PAGE_SIZE, sfs_dev->result, sizeof(u32)))
> +			ret = -EFAULT;
> +		if (copy_to_user(argp + PAGE_SIZE + sizeof(u32), sfs_dev->payload, sizeof(u32)))
> +			ret = -EFAULT;
> +		break;
> +	case SFSIOCUPDATEPKG:
> +		dev_dbg(sfs_dev->dev, "in SFSIOCUPDATEPKG\n");
> +
> +		if (copy_from_user(payload_name, argp, PAYLOAD_NAME_SIZE)) {
> +			ret = -EFAULT;
> +			goto unlock;
> +		}
> +
> +		ret = send_sfs_update_package(sfs_dev, payload_name);
> +		if (ret && ret != -EIO)
> +			goto unlock;
> +		/*
> +		 * return SFS status and extended status back to userspace
> +		 * if PSP status indicated command error.
> +		 */
> +		if (copy_to_user(argp + PAYLOAD_NAME_SIZE, sfs_dev->result, sizeof(u32)))
> +			ret = -EFAULT;
> +		if (copy_to_user(argp + PAYLOAD_NAME_SIZE + sizeof(u32), sfs_dev->payload,
> +				 sizeof(u32)))
> +			ret = -EFAULT;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +unlock:
> +	mutex_unlock(&sfs_dev->ioctl_mutex);
> +
> +	return ret;
> +}
> +
> +static const struct file_operations sfs_fops = {
> +	.owner	= THIS_MODULE,
> +	.unlocked_ioctl = sfs_ioctl,
> +};
> +
> +void sfs_dev_destroy(struct psp_device *psp)
> +{
> +	struct psp_sfs_device *sfs_dev = psp->sfs_data;
> +	int ret;
> +
> +	if (!sfs_dev)
> +		return;
> +
> +	/*
> +	 * TODO: free pre-allocated 2MB command buffer,
> +	 * if SEV-SNP is initialized the command buffer has
> +	 * been marked HV_Fixed and HV_Fixed pages remain
> +	 * in that state till system reset, they cannot be
> +	 * released back to the page allocator.
> +	 *
> +	 */
> +	snp_delete_hypervisor_fixed_pages_list(page_to_pfn(sfs_dev->page) << PAGE_SHIFT);
> +
> +	ret = set_memory_wb((unsigned long)page_address(sfs_dev->page), 512);
> +	if (ret)
> +		dev_dbg(sfs_dev->dev, "set memory wb failed\n");
> +
> +	sfs_initialized = false;
> +	misc_deregister(&sfs_dev->char_dev);
> +	mutex_destroy(&sfs_dev->ioctl_mutex);
> +	psp->sfs_data = NULL;
> +}
> +
> +int sfs_dev_init(struct psp_device *psp)
> +{
> +	struct device *dev = psp->dev;
> +	struct psp_sfs_device *sfs_dev;
> +	struct page *page;
> +	u64 cmd_buf_paddr;
> +	int ret;
> +
> +	/*
> +	 * SFS feature support can be detected on multiple devices but the SFS
> +	 * FW commands must be issued on the master. During probe, we do not
> +	 * know the master hence we create /dev/sfs on the first device probe.
> +	 */
> +	if (sfs_initialized)
> +		return 0;
> +
> +	sfs_dev = devm_kzalloc(dev, sizeof(*sfs_dev), GFP_KERNEL);
> +	if (!sfs_dev)
> +		return -ENOMEM;
> +
> +	BUILD_BUG_ON(sizeof(struct sfs_command) > PAGE_SIZE);
> +
> +	/*
> +	 * Pre-allocate static 2MB command buffer for all SFS commands.
> +	 */
> +	page = alloc_pages(GFP_KERNEL | __GFP_ZERO, ORDER_2MB);
> +	if (!page)
> +		return -ENOMEM;
> +	sfs_dev->page = page;
> +	sfs_dev->command_hdr = page_address(page);
> +	cmd_buf_paddr = page_to_pfn(sfs_dev->page) << PAGE_SHIFT;
> +
> +	/*
> +	 * If SEV-SNP is enabled the SFS command buffer needs to
> +	 * transitioned to HV_Fixed page state.
> +	 */
> +	dev_dbg(sfs_dev->dev, "cmdbuf page pa 0x%llx to be marked as HV_Fixed\n",
> +		cmd_buf_paddr);
> +
> +	ret = snp_insert_hypervisor_fixed_pages_list(cmd_buf_paddr, 512);
> +	if (ret) {
> +		dev_dbg(sfs_dev->dev, "cmdbuf page failed insertion in HV-Fixed page list\n");
> +		goto cleanup_cmd_buf;
> +	}
> +
> +	/*
> +	 * Buffers used for communication with different processors, x86 to ASP
> +	 * in this case must be mapped as non-cacheable.
> +	 */
> +	ret = set_memory_uc((unsigned long)page_address(page), 512);
> +	if (ret) {
> +		dev_dbg(sfs_dev->dev, "set memory uc failed\n");
> +		goto cleanup_cmd_buf_after_hv_fixed;
> +	}
> +
> +	dev_dbg(sfs_dev->dev, "cmdbuf page va 0x%lx marked as UnCacheable\n",
> +		(unsigned long)sfs_dev->command_hdr);
> +
> +	psp->sfs_data = sfs_dev;
> +	sfs_dev->dev = dev;
> +	sfs_dev->psp = psp;
> +
> +	dev_dbg(sfs_dev->dev, "seamless firmware serviving support is available\n");

servicing

> +
> +	sfs_dev->char_dev.minor = MISC_DYNAMIC_MINOR;
> +	sfs_dev->char_dev.name = "sfs";
> +	sfs_dev->char_dev.fops = &sfs_fops;
> +	sfs_dev->char_dev.mode = 0600;
> +	ret = misc_register(&sfs_dev->char_dev);
> +	if (ret)
> +		goto cleanup_cmd_buf_after_hv_fixed;
> +
> +	mutex_init(&sfs_dev->ioctl_mutex);
> +	sfs_initialized = true;
> +
> +	return 0;
> +
> +cleanup_cmd_buf_after_hv_fixed:
> +	snp_delete_hypervisor_fixed_pages_list(cmd_buf_paddr);
> +cleanup_cmd_buf:
> +	__free_pages(page, ORDER_2MB);
> +	psp->sfs_data = NULL;
> +	devm_kfree(dev, sfs_dev);
> +
> +	return ret;
> +}
> diff --git a/drivers/crypto/ccp/sfs.h b/drivers/crypto/ccp/sfs.h
> new file mode 100644
> index 000000000000..89b7792af076
> --- /dev/null
> +++ b/drivers/crypto/ccp/sfs.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * AMD Platform Security Processor (PSP) Seamless Firmware (SFS) Support.
> + *
> + * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
> + *
> + * Author: Ashish Kalra <ashish.kalra@amd.com>
> + */
> +
> +#ifndef __SFS_H__
> +#define __SFS_H__
> +
> +#include <uapi/linux/psp-sfs.h>
> +
> +#include <linux/device.h>
> +#include <linux/miscdevice.h>
> +#include <linux/psp-sev.h>
> +#include <linux/psp-platform-access.h>
> +#include <linux/set_memory.h>
> +
> +#include <asm/sev.h>
> +
> +#include "psp-dev.h"
> +
> +struct psp_sfs_device {
> +	struct device *dev;
> +	struct psp_device *psp;
> +
> +	struct sfs_command *command_hdr;
> +
> +	struct mutex ioctl_mutex;
> +
> +	struct miscdevice char_dev;
> +
> +	struct page *page;
> +
> +	/* used to abstract communication path */
> +	u32	header_size;
> +	u32	*payload_size;
> +	u32	*result;
> +	void	*payload;
> +	void	*pkg_hdr;
> +};
> +
> +struct sfs_command {
> +	struct psp_ext_request ext_req;
> +};
> +
> +void sfs_dev_destroy(struct psp_device *psp);
> +int sfs_dev_init(struct psp_device *psp);
> +void sfs_pci_init(void);
> +
> +#endif /* __SFS_H__ */
> diff --git a/include/linux/psp-platform-access.h b/include/linux/psp-platform-access.h
> index 1504fb012c05..540abf7de048 100644
> --- a/include/linux/psp-platform-access.h
> +++ b/include/linux/psp-platform-access.h
> @@ -7,6 +7,8 @@
>   
>   enum psp_platform_access_msg {
>   	PSP_CMD_NONE			= 0x0,
> +	PSP_SFS_GET_FW_VERSIONS,
> +	PSP_SFS_UPDATE,
>   	PSP_CMD_HSTI_QUERY		= 0x14,
>   	PSP_I2C_REQ_BUS_CMD		= 0x64,
>   	PSP_DYNAMIC_BOOST_GET_NONCE,
> diff --git a/include/uapi/linux/psp-sfs.h b/include/uapi/linux/psp-sfs.h
> new file mode 100644
> index 000000000000..e752fa041683
> --- /dev/null
> +++ b/include/uapi/linux/psp-sfs.h
> @@ -0,0 +1,87 @@
> +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> +/*
> + * Userspace interface for AMD Seamless Firmware Servicing (SFS)
> + *
> + * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
> + *
> + * Author: Ashish Kalra <ashish.kalra@amd.com>
> + */
> +
> +#ifndef __PSP_SFS_USER_H__
> +#define __PSP_SFS_USER_H__
> +
> +#include <linux/types.h>
> +
> +/**
> + * SFS: AMD Seamless Firmware Support (SFS) interface
> + */
> +
> +#define PAYLOAD_NAME_SIZE		64
> +#define TEE_EXT_CMD_BUFFER_SIZE	4096
> +
> +/**
> + * struct sfs_user_get_fw_versions - get current level of base firmware (output).
> + * @blob:                  current level of base firmware for ASP and patch levels (input/output).
> + * @sfs_status:            32-bit SFS status value (output).
> + * @sfs_extended_status:   32-bit SFS extended status value (output).
> + */
> +struct sfs_user_get_fw_versions {
> +	__u8	blob[TEE_EXT_CMD_BUFFER_SIZE];
> +	__u32	sfs_status;
> +	__u32	sfs_extended_status;
> +} __packed;
> +
> +/**
> + * struct sfs_user_update_package - update SFS package (input).
> + * @payload_name:          name of SFS package to load, verify and execute (input).
> + * @sfs_status:            32-bit SFS status value (output).
> + * @sfs_extended_status:   32-bit SFS extended status value (output).
> + */
> +struct sfs_user_update_package {
> +	char	payload_name[PAYLOAD_NAME_SIZE];
> +	__u32	sfs_status;
> +	__u32	sfs_extended_status;
> +} __packed;
> +
> +/**
> + * Seamless Firmware Support (SFS) IOC
> + *
> + * possible return codes for all SFS IOCTLs:
> + *  0:          success
> + *  -EINVAL:    invalid input
> + *  -E2BIG:     excess data passed
> + *  -EFAULT:    failed to copy to/from userspace
> + *  -EBUSY:     mailbox in recovery or in use
> + *  -ENODEV:    driver not bound with PSP device
> + *  -EACCES:    request isn't authorized
> + *  -EINVAL:    invalid parameter
> + *  -ETIMEDOUT: request timed out
> + *  -EAGAIN:    invalid request for state machine
> + *  -ENOENT:    not implemented
> + *  -ENFILE:    overflow
> + *  -EPERM:     invalid signature
> + *  -EIO:       unknown error
> + */
> +#define SFS_IOC_TYPE	'S'
> +
> +/**
> + * SFSIOCFWVERS - returns blob containing FW versions
> + *                ASP provides the current level of Base Firmware for the ASP
> + *                and the other microprocessors as well as current patch
> + *                level(s).
> + */
> +#define SFSIOCFWVERS	_IOWR(SFS_IOC_TYPE, 0x1, struct sfs_user_get_fw_versions)
> +
> +/**
> + * SFSIOCUPDATEPKG - updates package/payload
> + *                   ASP loads, verifies and executes the SFS package.
> + *                   By default, the SFS package/payload is loaded from
> + *                   /lib/firmware/amd, but alternative firmware loading
> + *                   path can be specified using kernel parameter
> + *                   firmware_class.path or the firmware loading path
> + *                   can be customized using sysfs file:
> + *                   /sys/module/firmware_class/parameters/path.
> + */
> +#define SFSIOCUPDATEPKG	_IOWR(SFS_IOC_TYPE, 0x2, struct sfs_user_update_package)
> +
> +#endif /* __PSP_SFS_USER_H__ */

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

[Kalra, Ashish]

On 7/24/2025 10:32 PM, Mario Limonciello wrote:
> 
> 
> On 7/24/25 4:16 PM, 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            | 316 ++++++++++++++++++++++++++++
>>   drivers/crypto/ccp/sfs.h            |  53 +++++
>>   include/linux/psp-platform-access.h |   2 +
>>   include/uapi/linux/psp-sfs.h        |  87 ++++++++
>>   7 files changed, 487 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
>>
>> diff --git a/drivers/crypto/ccp/Makefile b/drivers/crypto/ccp/Makefile
>> index 394484929dae..9b876bfb1289 100644
>> --- a/drivers/crypto/ccp/Makefile
>> +++ b/drivers/crypto/ccp/Makefile
>> @@ -13,7 +13,8 @@ ccp-$(CONFIG_CRYPTO_DEV_SP_PSP) += psp-dev.o \
>>                                      tee-dev.o \
>>                                      platform-access.o \
>>                                      dbc.o \
>> -                                   hsti.o
>> +                                   hsti.o \
>> +                   sfs.o
>>     obj-$(CONFIG_CRYPTO_DEV_CCP_CRYPTO) += ccp-crypto.o
>>   ccp-crypto-objs := ccp-crypto-main.o \
>> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
>> index 1c5a7189631e..8c4ad225ad67 100644
>> --- a/drivers/crypto/ccp/psp-dev.c
>> +++ b/drivers/crypto/ccp/psp-dev.c
>> @@ -17,6 +17,7 @@
>>   #include "psp-dev.h"
>>   #include "sev-dev.h"
>>   #include "tee-dev.h"
>> +#include "sfs.h"
>>   #include "platform-access.h"
>>   #include "dbc.h"
>>   #include "hsti.h"
>> @@ -182,6 +183,17 @@ static int psp_check_tee_support(struct psp_device *psp)
>>       return 0;
>>   }
>>   +static int psp_check_sfs_support(struct psp_device *psp)
>> +{
>> +    /* Check if device supports SFS feature */
>> +    if (!psp->capability.sev) {
> 
> Should this be psp->capability.sfs?

Yes, definately it should be psp->capability.sfs.

I don't know how did this got pushed, i have been testing with : 

static int psp_check_sfs_support(struct psp_device *psp)
{
        /* Check if device supports SFS feature */
        if (!psp->capability.sfs) {
                dev_dbg(psp->dev, "psp does not support SFS\n");
                return -ENODEV;
        }

        return 0;
}

I will push the correct function in the next version.

> 
>> +        dev_dbg(psp->dev, "psp does not support SFS\n");
>> +        return -ENODEV;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int psp_init(struct psp_device *psp)
>>   {
>>       int ret;
>> @@ -198,6 +210,12 @@ static int psp_init(struct psp_device *psp)
>>               return ret;
>>       }
>>   +    if (!psp_check_sfs_support(psp)) {
>> +        ret = sfs_dev_init(psp);
>> +        if (ret)
>> +            return ret;
>> +    }
>> +
>>       if (psp->vdata->platform_access) {
>>           ret = platform_access_dev_init(psp);
>>           if (ret)
>> @@ -302,6 +320,8 @@ void psp_dev_destroy(struct sp_device *sp)
>>         tee_dev_destroy(psp);
>>   +    sfs_dev_destroy(psp);
>> +
>>       dbc_dev_destroy(psp);
>>         platform_access_dev_destroy(psp);
>> diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
>> index e43ce87ede76..268c83f298cb 100644
>> --- a/drivers/crypto/ccp/psp-dev.h
>> +++ b/drivers/crypto/ccp/psp-dev.h
>> @@ -32,7 +32,8 @@ union psp_cap_register {
>>           unsigned int sev            :1,
>>                    tee            :1,
>>                    dbc_thru_ext        :1,
>> -                 rsvd1            :4,
>> +                 sfs            :1,
>> +                 rsvd1            :3,
>>                    security_reporting        :1,
>>                    fused_part            :1,
>>                    rsvd2            :1,
>> @@ -68,6 +69,7 @@ struct psp_device {
>>       void *tee_data;
>>       void *platform_access_data;
>>       void *dbc_data;
>> +    void *sfs_data;
>>         union psp_cap_register capability;
>>   };
>> @@ -118,12 +120,16 @@ struct psp_ext_request {
>>    * @PSP_SUB_CMD_DBC_SET_UID:        Set UID for DBC
>>    * @PSP_SUB_CMD_DBC_GET_PARAMETER:    Get parameter from DBC
>>    * @PSP_SUB_CMD_DBC_SET_PARAMETER:    Set parameter for DBC
>> + * @PSP_SUB_CMD_SFS_GET_FW_VERS:    Get firmware versions for ASP and other MP
>> + * @PSP_SUB_CMD_SFS_UPDATE:        Command to load, verify and execute SFS package
>>    */
>>   enum psp_sub_cmd {
>>       PSP_SUB_CMD_DBC_GET_NONCE    = PSP_DYNAMIC_BOOST_GET_NONCE,
>>       PSP_SUB_CMD_DBC_SET_UID        = PSP_DYNAMIC_BOOST_SET_UID,
>>       PSP_SUB_CMD_DBC_GET_PARAMETER    = PSP_DYNAMIC_BOOST_GET_PARAMETER,
>>       PSP_SUB_CMD_DBC_SET_PARAMETER    = PSP_DYNAMIC_BOOST_SET_PARAMETER,
>> +    PSP_SUB_CMD_SFS_GET_FW_VERS    = PSP_SFS_GET_FW_VERSIONS,
>> +    PSP_SUB_CMD_SFS_UPDATE        = PSP_SFS_UPDATE,
>>   };
>>     int psp_extended_mailbox_cmd(struct psp_device *psp, unsigned int timeout_msecs,
>> diff --git a/drivers/crypto/ccp/sfs.c b/drivers/crypto/ccp/sfs.c
>> new file mode 100644
>> index 000000000000..cbca01a884e1
>> --- /dev/null
>> +++ b/drivers/crypto/ccp/sfs.c
>> @@ -0,0 +1,316 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * AMD Secure Processor Seamless Firmware Servicing support.
>> + *
>> + * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
>> + *
>> + * Author: Ashish Kalra <ashish.kalra@amd.com>
>> + */
>> +
>> +#include <linux/firmware.h>
>> +
>> +#include "sfs.h"
>> +#include "sev-dev.h"
>> +
>> +#define SFS_DEFAULT_TIMEOUT        (10 * MSEC_PER_SEC)
>> +#define SFS_MAX_PAYLOAD_SIZE        (2 * 1024 * 1024)
>> +#define ORDER_2MB 9
>> +
>> +/* SFS Status values */
>> +#define SFS_SUCCESS            0x00
>> +#define SFS_INVALID_TOTAL_SIZE        0x02
>> +#define SFS_INVALID_PKG_SIZE        0x04
>> +#define SFS_DISABLED            0x05
>> +#define SFS_INVALID_CUST_SIGN        0x06
>> +#define SFS_INVALID_AMD_SIGN        0x07
>> +#define SFS_INTERNAL_ERROR        0x08
>> +#define SFS_CUST_SIGN_NOT_ALLOWED    0x09
>> +#define SFS_INVALID_BASE_PATCH_LVL    0x0A
>> +#define SFS_INVALID_CURR_PATCH_LVL    0x0B
>> +#define SFS_INVALID_NEW_PATCH_LVL    0x0C
>> +#define SFS_INVALID_SUBCOMMAND        0x0D
>> +#define SFS_PROTECTION_FAIL        0x0E
>> +#define SFS_BUSY            0x0F
>> +#define SFS_FW_VERSION_MISMATCH    0x10
>> +#define SFS_SYS_VERSION_MISMATCH    0x11
>> +#define SFS_SEV_STILL_INITIALIZED    0x12
>> +
>> +static bool sfs_initialized;
>> +
>> +static int send_sfs_cmd(struct psp_sfs_device *sfs_dev, int msg)
>> +{
>> +    int ret;
>> +
>> +    *sfs_dev->result = 0;
>> +    sfs_dev->command_hdr->ext_req.header.sub_cmd_id = msg;
>> +
>> +    ret = psp_extended_mailbox_cmd(sfs_dev->psp,
>> +                    SFS_DEFAULT_TIMEOUT,
>> +                    (struct psp_ext_request *)sfs_dev->command_hdr);
>> +    if (ret == -EIO) {
>> +        dev_dbg(sfs_dev->dev,
>> +             "msg 0x%x failed with PSP error: 0x%x\n",
>> +             msg, *sfs_dev->result);
>> +        dev_dbg(sfs_dev->dev,
>> +             "msg 0x%x extended status: 0x%x\n",
>> +             msg, *(u32 *)sfs_dev->payload);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static int send_sfs_get_fw_versions(struct psp_sfs_device *sfs_dev)
>> +{
>> +    int ret;
>> +
>> +    sfs_dev->payload_size = &sfs_dev->command_hdr->ext_req.header.payload_size;
>> +    sfs_dev->result = &sfs_dev->command_hdr->ext_req.header.status;
>> +    sfs_dev->payload = &sfs_dev->command_hdr->ext_req.buf;
>> +    sfs_dev->pkg_hdr = (void *)sfs_dev->command_hdr + PAGE_SIZE;
>> +    sfs_dev->header_size = sizeof(struct psp_ext_req_buffer_hdr);
>> +
>> +    /*
>> +     * SFS_GET_FW_VERSIONS command needs the output buffer to be
>> +     * initialized to 0xC7 in every byte.
>> +     */
>> +    memset(sfs_dev->pkg_hdr, 0xc7, PAGE_SIZE);
>> +    *sfs_dev->payload_size = 2 * PAGE_SIZE;
>> +
>> +    ret = send_sfs_cmd(sfs_dev, PSP_SFS_GET_FW_VERSIONS);
>> +
>> +    return ret;
>> +}
>> +
>> +static int send_sfs_update_package(struct psp_sfs_device *sfs_dev, char *payload_name)
>> +{
>> +    char payload_path[PAYLOAD_NAME_SIZE];
>> +    const struct firmware *firmware;
>> +    unsigned long package_size;
>> +    int ret;
>> +
>> +    sprintf(payload_path, "amd/%s", payload_name);
>> +
>> +    ret = firmware_request_nowarn(&firmware, payload_path, sfs_dev->dev);
>> +    if (ret < 0) {
>> +        dev_warn(sfs_dev->dev, "firmware request fail %d\n", ret);
>> +        return -ENOENT;
>> +    }
>> +
>> +    /* SFS Update Package should be 64KB aligned */
>> +    package_size = ALIGN(firmware->size + PAGE_SIZE, 0x10000U);
>> +
>> +    /*
>> +     * SFS command buffer is a pre-allocated 2MB buffer, fail update package
>> +     * if SFS payload is larger than the pre-allocated command buffer.
>> +     */
>> +    if (package_size > SFS_MAX_PAYLOAD_SIZE) {
>> +        dev_warn(sfs_dev->dev,
>> +             "SFS payload size %ld larger than maximum supported payload size of 2MB\n",
>> +             package_size);
>> +        return -ENOMEM;
>> +    }
>> +
>> +    sfs_dev->payload_size = &sfs_dev->command_hdr->ext_req.header.payload_size;
>> +    sfs_dev->result = &sfs_dev->command_hdr->ext_req.header.status;
>> +    sfs_dev->payload = &sfs_dev->command_hdr->ext_req.buf;
>> +    sfs_dev->pkg_hdr = (void *)sfs_dev->command_hdr + PAGE_SIZE;
>> +    sfs_dev->header_size = sizeof(struct psp_ext_req_buffer_hdr);
>> +
>> +    /*
>> +     * Copy firmware data to a kernel allocated contiguous
>> +     * memory region.
>> +     */
>> +    memcpy(sfs_dev->pkg_hdr, firmware->data, firmware->size);
>> +    *sfs_dev->payload_size = package_size;
>> +
>> +    ret = send_sfs_cmd(sfs_dev, PSP_SFS_UPDATE);
>> +
>> +    release_firmware(firmware);
>> +    return ret;
>> +}
>> +
>> +static long sfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>> +{
>> +    struct psp_device *psp_master = psp_get_master_device();
>> +    void __user *argp = (void __user *)arg;
>> +    char payload_name[PAYLOAD_NAME_SIZE];
>> +    struct psp_sfs_device *sfs_dev;
>> +    int ret;
>> +
>> +    if (!psp_master || !psp_master->sfs_data)
>> +        return -ENODEV;
>> +    sfs_dev = psp_master->sfs_data;
>> +
>> +    if (!sfs_initialized)
>> +        return -EINVAL;
>> +
>> +    mutex_lock(&sfs_dev->ioctl_mutex);
> 
> As it's new code, how about using guard(mutex) instead?  Then you don't need to have goto unlock in the failure cases and can just return the error directly.
> 

Sure, will do that.

Thanks,
Ashish

>> +
>> +    switch (cmd) {
>> +    case SFSIOCFWVERS:
>> +        dev_dbg(sfs_dev->dev, "in SFSIOCFWVERS\n");
>> +
>> +        ret = send_sfs_get_fw_versions(sfs_dev);
>> +        if (ret && ret != -EIO)
>> +            goto unlock;
>> +        /*
>> +         * return SFS status and extended status back to userspace
>> +         * if PSP status indicated command error.
>> +         */
>> +        if (copy_to_user(argp, sfs_dev->pkg_hdr, PAGE_SIZE))
>> +            ret = -EFAULT;
>> +        if (copy_to_user(argp + PAGE_SIZE, sfs_dev->result, sizeof(u32)))
>> +            ret = -EFAULT;
>> +        if (copy_to_user(argp + PAGE_SIZE + sizeof(u32), sfs_dev->payload, sizeof(u32)))
>> +            ret = -EFAULT;
>> +        break;
>> +    case SFSIOCUPDATEPKG:
>> +        dev_dbg(sfs_dev->dev, "in SFSIOCUPDATEPKG\n");
>> +
>> +        if (copy_from_user(payload_name, argp, PAYLOAD_NAME_SIZE)) {
>> +            ret = -EFAULT;
>> +            goto unlock;
>> +        }
>> +
>> +        ret = send_sfs_update_package(sfs_dev, payload_name);
>> +        if (ret && ret != -EIO)
>> +            goto unlock;
>> +        /*
>> +         * return SFS status and extended status back to userspace
>> +         * if PSP status indicated command error.
>> +         */
>> +        if (copy_to_user(argp + PAYLOAD_NAME_SIZE, sfs_dev->result, sizeof(u32)))
>> +            ret = -EFAULT;
>> +        if (copy_to_user(argp + PAYLOAD_NAME_SIZE + sizeof(u32), sfs_dev->payload,
>> +                 sizeof(u32)))
>> +            ret = -EFAULT;
>> +        break;
>> +    default:
>> +        ret = -EINVAL;
>> +    }
>> +
>> +unlock:
>> +    mutex_unlock(&sfs_dev->ioctl_mutex);
>> +
>> +    return ret;
>> +}
>> +
>> +static const struct file_operations sfs_fops = {
>> +    .owner    = THIS_MODULE,
>> +    .unlocked_ioctl = sfs_ioctl,
>> +};
>> +
>> +void sfs_dev_destroy(struct psp_device *psp)
>> +{
>> +    struct psp_sfs_device *sfs_dev = psp->sfs_data;
>> +    int ret;
>> +
>> +    if (!sfs_dev)
>> +        return;
>> +
>> +    /*
>> +     * TODO: free pre-allocated 2MB command buffer,
>> +     * if SEV-SNP is initialized the command buffer has
>> +     * been marked HV_Fixed and HV_Fixed pages remain
>> +     * in that state till system reset, they cannot be
>> +     * released back to the page allocator.
>> +     *
>> +     */
>> +    snp_delete_hypervisor_fixed_pages_list(page_to_pfn(sfs_dev->page) << PAGE_SHIFT);
>> +
>> +    ret = set_memory_wb((unsigned long)page_address(sfs_dev->page), 512);
>> +    if (ret)
>> +        dev_dbg(sfs_dev->dev, "set memory wb failed\n");
>> +
>> +    sfs_initialized = false;
>> +    misc_deregister(&sfs_dev->char_dev);
>> +    mutex_destroy(&sfs_dev->ioctl_mutex);
>> +    psp->sfs_data = NULL;
>> +}
>> +
>> +int sfs_dev_init(struct psp_device *psp)
>> +{
>> +    struct device *dev = psp->dev;
>> +    struct psp_sfs_device *sfs_dev;
>> +    struct page *page;
>> +    u64 cmd_buf_paddr;
>> +    int ret;
>> +
>> +    /*
>> +     * SFS feature support can be detected on multiple devices but the SFS
>> +     * FW commands must be issued on the master. During probe, we do not
>> +     * know the master hence we create /dev/sfs on the first device probe.
>> +     */
>> +    if (sfs_initialized)
>> +        return 0;
>> +
>> +    sfs_dev = devm_kzalloc(dev, sizeof(*sfs_dev), GFP_KERNEL);
>> +    if (!sfs_dev)
>> +        return -ENOMEM;
>> +
>> +    BUILD_BUG_ON(sizeof(struct sfs_command) > PAGE_SIZE);
>> +
>> +    /*
>> +     * Pre-allocate static 2MB command buffer for all SFS commands.
>> +     */
>> +    page = alloc_pages(GFP_KERNEL | __GFP_ZERO, ORDER_2MB);
>> +    if (!page)
>> +        return -ENOMEM;
>> +    sfs_dev->page = page;
>> +    sfs_dev->command_hdr = page_address(page);
>> +    cmd_buf_paddr = page_to_pfn(sfs_dev->page) << PAGE_SHIFT;
>> +
>> +    /*
>> +     * If SEV-SNP is enabled the SFS command buffer needs to
>> +     * transitioned to HV_Fixed page state.
>> +     */
>> +    dev_dbg(sfs_dev->dev, "cmdbuf page pa 0x%llx to be marked as HV_Fixed\n",
>> +        cmd_buf_paddr);
>> +
>> +    ret = snp_insert_hypervisor_fixed_pages_list(cmd_buf_paddr, 512);
>> +    if (ret) {
>> +        dev_dbg(sfs_dev->dev, "cmdbuf page failed insertion in HV-Fixed page list\n");
>> +        goto cleanup_cmd_buf;
>> +    }
>> +
>> +    /*
>> +     * Buffers used for communication with different processors, x86 to ASP
>> +     * in this case must be mapped as non-cacheable.
>> +     */
>> +    ret = set_memory_uc((unsigned long)page_address(page), 512);
>> +    if (ret) {
>> +        dev_dbg(sfs_dev->dev, "set memory uc failed\n");
>> +        goto cleanup_cmd_buf_after_hv_fixed;
>> +    }
>> +
>> +    dev_dbg(sfs_dev->dev, "cmdbuf page va 0x%lx marked as UnCacheable\n",
>> +        (unsigned long)sfs_dev->command_hdr);
>> +
>> +    psp->sfs_data = sfs_dev;
>> +    sfs_dev->dev = dev;
>> +    sfs_dev->psp = psp;
>> +
>> +    dev_dbg(sfs_dev->dev, "seamless firmware serviving support is available\n");
> 
> servicing
> 
>> +
>> +    sfs_dev->char_dev.minor = MISC_DYNAMIC_MINOR;
>> +    sfs_dev->char_dev.name = "sfs";
>> +    sfs_dev->char_dev.fops = &sfs_fops;
>> +    sfs_dev->char_dev.mode = 0600;
>> +    ret = misc_register(&sfs_dev->char_dev);
>> +    if (ret)
>> +        goto cleanup_cmd_buf_after_hv_fixed;
>> +
>> +    mutex_init(&sfs_dev->ioctl_mutex);
>> +    sfs_initialized = true;
>> +
>> +    return 0;
>> +
>> +cleanup_cmd_buf_after_hv_fixed:
>> +    snp_delete_hypervisor_fixed_pages_list(cmd_buf_paddr);
>> +cleanup_cmd_buf:
>> +    __free_pages(page, ORDER_2MB);
>> +    psp->sfs_data = NULL;
>> +    devm_kfree(dev, sfs_dev);
>> +
>> +    return ret;
>> +}
>> diff --git a/drivers/crypto/ccp/sfs.h b/drivers/crypto/ccp/sfs.h
>> new file mode 100644
>> index 000000000000..89b7792af076
>> --- /dev/null
>> +++ b/drivers/crypto/ccp/sfs.h
>> @@ -0,0 +1,53 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * AMD Platform Security Processor (PSP) Seamless Firmware (SFS) Support.
>> + *
>> + * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
>> + *
>> + * Author: Ashish Kalra <ashish.kalra@amd.com>
>> + */
>> +
>> +#ifndef __SFS_H__
>> +#define __SFS_H__
>> +
>> +#include <uapi/linux/psp-sfs.h>
>> +
>> +#include <linux/device.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/psp-sev.h>
>> +#include <linux/psp-platform-access.h>
>> +#include <linux/set_memory.h>
>> +
>> +#include <asm/sev.h>
>> +
>> +#include "psp-dev.h"
>> +
>> +struct psp_sfs_device {
>> +    struct device *dev;
>> +    struct psp_device *psp;
>> +
>> +    struct sfs_command *command_hdr;
>> +
>> +    struct mutex ioctl_mutex;
>> +
>> +    struct miscdevice char_dev;
>> +
>> +    struct page *page;
>> +
>> +    /* used to abstract communication path */
>> +    u32    header_size;
>> +    u32    *payload_size;
>> +    u32    *result;
>> +    void    *payload;
>> +    void    *pkg_hdr;
>> +};
>> +
>> +struct sfs_command {
>> +    struct psp_ext_request ext_req;
>> +};
>> +
>> +void sfs_dev_destroy(struct psp_device *psp);
>> +int sfs_dev_init(struct psp_device *psp);
>> +void sfs_pci_init(void);
>> +
>> +#endif /* __SFS_H__ */
>> diff --git a/include/linux/psp-platform-access.h b/include/linux/psp-platform-access.h
>> index 1504fb012c05..540abf7de048 100644
>> --- a/include/linux/psp-platform-access.h
>> +++ b/include/linux/psp-platform-access.h
>> @@ -7,6 +7,8 @@
>>     enum psp_platform_access_msg {
>>       PSP_CMD_NONE            = 0x0,
>> +    PSP_SFS_GET_FW_VERSIONS,
>> +    PSP_SFS_UPDATE,
>>       PSP_CMD_HSTI_QUERY        = 0x14,
>>       PSP_I2C_REQ_BUS_CMD        = 0x64,
>>       PSP_DYNAMIC_BOOST_GET_NONCE,
>> diff --git a/include/uapi/linux/psp-sfs.h b/include/uapi/linux/psp-sfs.h
>> new file mode 100644
>> index 000000000000..e752fa041683
>> --- /dev/null
>> +++ b/include/uapi/linux/psp-sfs.h
>> @@ -0,0 +1,87 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
>> +/*
>> + * Userspace interface for AMD Seamless Firmware Servicing (SFS)
>> + *
>> + * Copyright (C) 2023-2025 Advanced Micro Devices, Inc.
>> + *
>> + * Author: Ashish Kalra <ashish.kalra@amd.com>
>> + */
>> +
>> +#ifndef __PSP_SFS_USER_H__
>> +#define __PSP_SFS_USER_H__
>> +
>> +#include <linux/types.h>
>> +
>> +/**
>> + * SFS: AMD Seamless Firmware Support (SFS) interface
>> + */
>> +
>> +#define PAYLOAD_NAME_SIZE        64
>> +#define TEE_EXT_CMD_BUFFER_SIZE    4096
>> +
>> +/**
>> + * struct sfs_user_get_fw_versions - get current level of base firmware (output).
>> + * @blob:                  current level of base firmware for ASP and patch levels (input/output).
>> + * @sfs_status:            32-bit SFS status value (output).
>> + * @sfs_extended_status:   32-bit SFS extended status value (output).
>> + */
>> +struct sfs_user_get_fw_versions {
>> +    __u8    blob[TEE_EXT_CMD_BUFFER_SIZE];
>> +    __u32    sfs_status;
>> +    __u32    sfs_extended_status;
>> +} __packed;
>> +
>> +/**
>> + * struct sfs_user_update_package - update SFS package (input).
>> + * @payload_name:          name of SFS package to load, verify and execute (input).
>> + * @sfs_status:            32-bit SFS status value (output).
>> + * @sfs_extended_status:   32-bit SFS extended status value (output).
>> + */
>> +struct sfs_user_update_package {
>> +    char    payload_name[PAYLOAD_NAME_SIZE];
>> +    __u32    sfs_status;
>> +    __u32    sfs_extended_status;
>> +} __packed;
>> +
>> +/**
>> + * Seamless Firmware Support (SFS) IOC
>> + *
>> + * possible return codes for all SFS IOCTLs:
>> + *  0:          success
>> + *  -EINVAL:    invalid input
>> + *  -E2BIG:     excess data passed
>> + *  -EFAULT:    failed to copy to/from userspace
>> + *  -EBUSY:     mailbox in recovery or in use
>> + *  -ENODEV:    driver not bound with PSP device
>> + *  -EACCES:    request isn't authorized
>> + *  -EINVAL:    invalid parameter
>> + *  -ETIMEDOUT: request timed out
>> + *  -EAGAIN:    invalid request for state machine
>> + *  -ENOENT:    not implemented
>> + *  -ENFILE:    overflow
>> + *  -EPERM:     invalid signature
>> + *  -EIO:       unknown error
>> + */
>> +#define SFS_IOC_TYPE    'S'
>> +
>> +/**
>> + * SFSIOCFWVERS - returns blob containing FW versions
>> + *                ASP provides the current level of Base Firmware for the ASP
>> + *                and the other microprocessors as well as current patch
>> + *                level(s).
>> + */
>> +#define SFSIOCFWVERS    _IOWR(SFS_IOC_TYPE, 0x1, struct sfs_user_get_fw_versions)
>> +
>> +/**
>> + * SFSIOCUPDATEPKG - updates package/payload
>> + *                   ASP loads, verifies and executes the SFS package.
>> + *                   By default, the SFS package/payload is loaded from
>> + *                   /lib/firmware/amd, but alternative firmware loading
>> + *                   path can be specified using kernel parameter
>> + *                   firmware_class.path or the firmware loading path
>> + *                   can be customized using sysfs file:
>> + *                   /sys/module/firmware_class/parameters/path.
>> + */
>> +#define SFSIOCUPDATEPKG    _IOWR(SFS_IOC_TYPE, 0x2, struct sfs_user_update_package)
>> +
>> +#endif /* __PSP_SFS_USER_H__ */
> 

Re: [PATCH 1/2] crypto: ccp - Add new API for extending HV_Fixed Pages

[Tom Lendacky]

On 7/24/25 16:14, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Implement new API to add support for extending the HV_Fixed pages list
> passed to SNP_INIT_EX.
> 
> Adds a simple list based interface to extend the HV_Fixed pages list
> for PSP sub-devices such as the SFS driver.
> 
> Suggested-by: Thomas.Lendacky@amd.com <Thomas.Lendacky@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  drivers/crypto/ccp/sev-dev.c | 88 ++++++++++++++++++++++++++++++++++++
>  drivers/crypto/ccp/sev-dev.h |  3 ++
>  2 files changed, 91 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index e058ba027792..c3ff40cd7a96 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -82,6 +82,14 @@ MODULE_FIRMWARE("amd/amd_sev_fam19h_model1xh.sbin"); /* 4th gen EPYC */
>  static bool psp_dead;
>  static int psp_timeout;
>  
> +struct snp_hv_fixed_pages_entry {
> +	u64 base;
> +	int npages;
> +	struct list_head list;
> +};
> +static LIST_HEAD(snp_hv_fixed_pages);
> +static DEFINE_SPINLOCK(snp_hv_fixed_pages_lock);
> +
>  /* Trusted Memory Region (TMR):
>   *   The TMR is a 1MB area that must be 1MB aligned.  Use the page allocator
>   *   to allocate the memory, which will return aligned memory for the specified
> @@ -1073,6 +1081,76 @@ static void snp_set_hsave_pa(void *arg)
>  	wrmsrq(MSR_VM_HSAVE_PA, 0);
>  }
>  
> +int snp_insert_hypervisor_fixed_pages_list(u64 paddr, int npages)
> +{
> +	struct snp_hv_fixed_pages_entry *entry;
> +
> +	spin_lock(&snp_hv_fixed_pages_lock);

Please use guard() so that you don't have to issue spin_unlock() anywhere.

> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry) {
> +		spin_unlock(&snp_hv_fixed_pages_lock);
> +		return -ENOMEM;
> +	}
> +	entry->base = paddr;
> +	entry->npages = npages;
> +	list_add_tail(&entry->list, &snp_hv_fixed_pages);

You're creating this API that can now be called at any time. Either
restrict it to when SNP is not initialized or add support to issue
SNP_PAGE_SET_STATE.

I would suggest that you return an error for now if SNP is initialized.

> +
> +	spin_unlock(&snp_hv_fixed_pages_lock);
> +
> +	return 0;
> +}
> +
> +void snp_delete_hypervisor_fixed_pages_list(u64 paddr)
> +{

Not sure you can have this...  Once a page is marked HV_FIXED it can't be
changed unless SNP (SNPEn bit in SYS_CFG MSR) is disabled, which doesn't
happen until reboot.

So users of this interface will have to leak pages since they can't be
released back to the general allocation pool for chance they get used for
an SNP guest.

So this API should probably be deleted.

Or you change this to a driver HV_FIXED allocation/free setup where this
performs the allocation and adds the memory to the list and the free API
leaks the page.

> +	struct snp_hv_fixed_pages_entry *entry, *nentry;
> +
> +	spin_lock(&snp_hv_fixed_pages_lock);
> +	list_for_each_entry_safe(entry, nentry, &snp_hv_fixed_pages, list) {
> +		if (entry->base == paddr) {
> +			list_del(&entry->list);
> +			kfree(entry);
> +			break;
> +		}
> +	}
> +	spin_unlock(&snp_hv_fixed_pages_lock);
> +}
> +
> +static int snp_extend_hypervisor_fixed_pages(struct sev_data_range_list *range_list)
> +{
> +	struct sev_data_range *range = &range_list->ranges[range_list->num_elements];
> +	struct snp_hv_fixed_pages_entry *entry;
> +	int new_element_count, ret = 0;
> +
> +	spin_lock(&snp_hv_fixed_pages_lock);

guard()

> +	if (list_empty(&snp_hv_fixed_pages))
> +		goto out;
> +
> +	new_element_count = list_count_nodes(&snp_hv_fixed_pages) +
> +			    range_list->num_elements;
> +
> +	/*
> +	 * Ensure the list of HV_FIXED pages that will be passed to firmware
> +	 * do not exceed the page-sized argument buffer.
> +	 */
> +	if (new_element_count * sizeof(struct sev_data_range) +
> +	    sizeof(struct sev_data_range_list) > PAGE_SIZE) {
> +		ret = -E2BIG;
> +		goto out;
> +	}
> +
> +	list_for_each_entry(entry, &snp_hv_fixed_pages, list) {
> +		range->base = entry->base;
> +		range->page_count = entry->npages;

Will there be an issue if the size is not 2MB aligned? I think a PSMASH
will be done, but something to test if you are going to allow any page
alignment and page count.

> +		range++;
> +	}
> +	range_list->num_elements = new_element_count;
> +out:
> +	spin_unlock(&snp_hv_fixed_pages_lock);
> +
> +	return ret;
> +}
> +
>  static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
>  {
>  	struct sev_data_range_list *range_list = arg;
> @@ -1163,6 +1241,16 @@ static int __sev_snp_init_locked(int *error)
>  			return rc;
>  		}
>  
> +		/*
> +		 * Extend the HV_Fixed pages list with HV_Fixed pages added from other
> +		 * PSP sub-devices such as SFS. Warn if the list can't be extended
> +		 * but continue with SNP_INIT_EX.
> +		 */
> +		rc = snp_extend_hypervisor_fixed_pages(snp_range_list);
> +		if (rc)
> +			dev_warn(sev->dev,
> +				 "SEV: SNP_INIT_EX extend HV_Fixed pages failed rc = %d\n", rc);

If you aren't going to do anything with the error other than print a
warning, this should be moved to the snp_extend_hypervisor_fixed_pages()
function and have it be a void function.

I would assume we'll see a failure in the SFS component if this fails, though.

Thanks,
Tom

> +
>  		memset(&data, 0, sizeof(data));
>  		data.init_rmp = 1;
>  		data.list_paddr_en = 1;
> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
> index 3e4e5574e88a..444d7fffd801 100644
> --- a/drivers/crypto/ccp/sev-dev.h
> +++ b/drivers/crypto/ccp/sev-dev.h
> @@ -65,4 +65,7 @@ void sev_dev_destroy(struct psp_device *psp);
>  void sev_pci_init(void);
>  void sev_pci_exit(void);
>  
> +int snp_insert_hypervisor_fixed_pages_list(u64 paddr, int npages);
> +void snp_delete_hypervisor_fixed_pages_list(u64 paddr);
> +
>  #endif /* __SEV_DEV_H */

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

[Tom Lendacky]

On 7/24/25 16:16, 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

Based on your comments that this might not be the proper version, I'll
wait on reviewing this.

Thanks,
Tom

> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---

Re: [PATCH 1/2] crypto: ccp - Add new API for extending HV_Fixed Pages

[Kalra, Ashish]

Hello Tom,

On 7/25/2025 9:28 AM, Tom Lendacky wrote:
> On 7/24/25 16:14, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> Implement new API to add support for extending the HV_Fixed pages list
>> passed to SNP_INIT_EX.
>>
>> Adds a simple list based interface to extend the HV_Fixed pages list
>> for PSP sub-devices such as the SFS driver.
>>
>> Suggested-by: Thomas.Lendacky@amd.com <Thomas.Lendacky@amd.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>>  drivers/crypto/ccp/sev-dev.c | 88 ++++++++++++++++++++++++++++++++++++
>>  drivers/crypto/ccp/sev-dev.h |  3 ++
>>  2 files changed, 91 insertions(+)
>>
>> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
>> index e058ba027792..c3ff40cd7a96 100644
>> --- a/drivers/crypto/ccp/sev-dev.c
>> +++ b/drivers/crypto/ccp/sev-dev.c
>> @@ -82,6 +82,14 @@ MODULE_FIRMWARE("amd/amd_sev_fam19h_model1xh.sbin"); /* 4th gen EPYC */
>>  static bool psp_dead;
>>  static int psp_timeout;
>>  
>> +struct snp_hv_fixed_pages_entry {
>> +	u64 base;
>> +	int npages;
>> +	struct list_head list;
>> +};
>> +static LIST_HEAD(snp_hv_fixed_pages);
>> +static DEFINE_SPINLOCK(snp_hv_fixed_pages_lock);
>> +
>>  /* Trusted Memory Region (TMR):
>>   *   The TMR is a 1MB area that must be 1MB aligned.  Use the page allocator
>>   *   to allocate the memory, which will return aligned memory for the specified
>> @@ -1073,6 +1081,76 @@ static void snp_set_hsave_pa(void *arg)
>>  	wrmsrq(MSR_VM_HSAVE_PA, 0);
>>  }
>>  
>> +int snp_insert_hypervisor_fixed_pages_list(u64 paddr, int npages)
>> +{
>> +	struct snp_hv_fixed_pages_entry *entry;
>> +
>> +	spin_lock(&snp_hv_fixed_pages_lock);
> 
> Please use guard() so that you don't have to issue spin_unlock() anywhere.
> 
>> +
>> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>> +	if (!entry) {
>> +		spin_unlock(&snp_hv_fixed_pages_lock);
>> +		return -ENOMEM;
>> +	}
>> +	entry->base = paddr;
>> +	entry->npages = npages;
>> +	list_add_tail(&entry->list, &snp_hv_fixed_pages);
> 
> You're creating this API that can now be called at any time. Either
> restrict it to when SNP is not initialized or add support to issue
> SNP_PAGE_SET_STATE.
> 
> I would suggest that you return an error for now if SNP is initialized.
> 

Ok.

>> +
>> +	spin_unlock(&snp_hv_fixed_pages_lock);
>> +
>> +	return 0;
>> +}
>> +
>> +void snp_delete_hypervisor_fixed_pages_list(u64 paddr)
>> +{
> 
> Not sure you can have this...  Once a page is marked HV_FIXED it can't be
> changed unless SNP (SNPEn bit in SYS_CFG MSR) is disabled, which doesn't
> happen until reboot.
> 
> So users of this interface will have to leak pages since they can't be
> released back to the general allocation pool for chance they get used for
> an SNP guest.
> 
> So this API should probably be deleted.
> 
> Or you change this to a driver HV_FIXED allocation/free setup where this
> performs the allocation and adds the memory to the list and the free API
> leaks the page.
>

Again, as you mentioned above this API interface is restricted to use till SNP is initialized,
so i think we can still have this (to handle cases where a sub-device init failure path
needs to remove it's HV_Fixed page from the list). So probably i can have this with a
check for SNP being already initialized and returning an error if it is, allowing the
user to leak the page ? 
 
>> +	struct snp_hv_fixed_pages_entry *entry, *nentry;
>> +
>> +	spin_lock(&snp_hv_fixed_pages_lock);
>> +	list_for_each_entry_safe(entry, nentry, &snp_hv_fixed_pages, list) {
>> +		if (entry->base == paddr) {
>> +			list_del(&entry->list);
>> +			kfree(entry);
>> +			break;
>> +		}
>> +	}
>> +	spin_unlock(&snp_hv_fixed_pages_lock);
>> +}
>> +
>> +static int snp_extend_hypervisor_fixed_pages(struct sev_data_range_list *range_list)
>> +{
>> +	struct sev_data_range *range = &range_list->ranges[range_list->num_elements];
>> +	struct snp_hv_fixed_pages_entry *entry;
>> +	int new_element_count, ret = 0;
>> +
>> +	spin_lock(&snp_hv_fixed_pages_lock);
> 
> guard()
> 
>> +	if (list_empty(&snp_hv_fixed_pages))
>> +		goto out;
>> +
>> +	new_element_count = list_count_nodes(&snp_hv_fixed_pages) +
>> +			    range_list->num_elements;
>> +
>> +	/*
>> +	 * Ensure the list of HV_FIXED pages that will be passed to firmware
>> +	 * do not exceed the page-sized argument buffer.
>> +	 */
>> +	if (new_element_count * sizeof(struct sev_data_range) +
>> +	    sizeof(struct sev_data_range_list) > PAGE_SIZE) {
>> +		ret = -E2BIG;
>> +		goto out;
>> +	}
>> +
>> +	list_for_each_entry(entry, &snp_hv_fixed_pages, list) {
>> +		range->base = entry->base;
>> +		range->page_count = entry->npages;
> 
> Will there be an issue if the size is not 2MB aligned? I think a PSMASH
> will be done, but something to test if you are going to allow any page
> alignment and page count.
> 

I believe that SNP_INIT_EX can add HV_Fixed pages which are not 2MB size aligned.

Here is a sub list of HV_Fixed pages being passed to SNP_INIT_EX: 

[   25.940837] base 0x0, count 1
[   25.940838] base 0xa0000, count 96
[   25.940839] base 0x75b60000, count 75
[   25.940839] base 0x75c60000, count 928
[   25.940840] base 0x88965000, count 83
[   25.940841] base 0x8a40c000, count 1
[   25.940841] base 0x8e14d000, count 48187
[   25.940842] base 0x99d88000, count 235
[   25.940842] base 0x99e73000, count 1153
[   25.940843] base 0x9a2f4000, count 12043
[   25.940844] base 0x9fffa000, count 5
[   25.940844] base 0xa0000000, count 65536
[   25.940845] base 0xb4000000, count 1
[   25.940845] base 0xb5080000, count 1
[   25.940846] base 0xbe100000, count 1
[   25.940847] base 0xbf000000, count 1
[   25.940847] base 0xd0080000, count 1
[   25.940848] base 0xd1100000, count 1
[   25.940848] base 0xec400000, count 1
...
...

>> +		range++;
>> +	}
>> +	range_list->num_elements = new_element_count;
>> +out:
>> +	spin_unlock(&snp_hv_fixed_pages_lock);
>> +
>> +	return ret;
>> +}
>> +
>>  static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
>>  {
>>  	struct sev_data_range_list *range_list = arg;
>> @@ -1163,6 +1241,16 @@ static int __sev_snp_init_locked(int *error)
>>  			return rc;
>>  		}
>>  
>> +		/*
>> +		 * Extend the HV_Fixed pages list with HV_Fixed pages added from other
>> +		 * PSP sub-devices such as SFS. Warn if the list can't be extended
>> +		 * but continue with SNP_INIT_EX.
>> +		 */
>> +		rc = snp_extend_hypervisor_fixed_pages(snp_range_list);
>> +		if (rc)
>> +			dev_warn(sev->dev,
>> +				 "SEV: SNP_INIT_EX extend HV_Fixed pages failed rc = %d\n", rc);
> 
> If you aren't going to do anything with the error other than print a
> warning, this should be moved to the snp_extend_hypervisor_fixed_pages()
> function and have it be a void function.
>
Ok.
 
> I would assume we'll see a failure in the SFS component if this fails, though.

Yes, SFS or any other sub-device component will fail in this case, but i don't want to abort
SNP_INIT_EX in this case.

Thanks,
Ashish

> 
> Thanks,
> Tom
> 
>> +
>>  		memset(&data, 0, sizeof(data));
>>  		data.init_rmp = 1;
>>  		data.list_paddr_en = 1;
>> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
>> index 3e4e5574e88a..444d7fffd801 100644
>> --- a/drivers/crypto/ccp/sev-dev.h
>> +++ b/drivers/crypto/ccp/sev-dev.h
>> @@ -65,4 +65,7 @@ void sev_dev_destroy(struct psp_device *psp);
>>  void sev_pci_init(void);
>>  void sev_pci_exit(void);
>>  
>> +int snp_insert_hypervisor_fixed_pages_list(u64 paddr, int npages);
>> +void snp_delete_hypervisor_fixed_pages_list(u64 paddr);
>> +
>>  #endif /* __SEV_DEV_H */

Re: [PATCH 1/2] crypto: ccp - Add new API for extending HV_Fixed Pages

[Tom Lendacky]

On 7/25/25 10:16, Kalra, Ashish wrote:
> Hello Tom,
> 
> On 7/25/2025 9:28 AM, Tom Lendacky wrote:
>> On 7/24/25 16:14, Ashish Kalra wrote:
>>> From: Ashish Kalra <ashish.kalra@amd.com>

>>> +void snp_delete_hypervisor_fixed_pages_list(u64 paddr)
>>> +{
>>
>> Not sure you can have this...  Once a page is marked HV_FIXED it can't be
>> changed unless SNP (SNPEn bit in SYS_CFG MSR) is disabled, which doesn't
>> happen until reboot.
>>
>> So users of this interface will have to leak pages since they can't be
>> released back to the general allocation pool for chance they get used for
>> an SNP guest.
>>
>> So this API should probably be deleted.
>>
>> Or you change this to a driver HV_FIXED allocation/free setup where this
>> performs the allocation and adds the memory to the list and the free API
>> leaks the page.
>>
> 
> Again, as you mentioned above this API interface is restricted to use till SNP is initialized,
> so i think we can still have this (to handle cases where a sub-device init failure path
> needs to remove it's HV_Fixed page from the list). So probably i can have this with a
> check for SNP being already initialized and returning an error if it is, allowing the
> user to leak the page ? 

I'd prefer to have the decision to leak the page being done in a single
place. If this ends up being used by more than just SFS, then there's
another place that needs to know to do that.

>  

>>> +	list_for_each_entry(entry, &snp_hv_fixed_pages, list) {
>>> +		range->base = entry->base;
>>> +		range->page_count = entry->npages;
>>
>> Will there be an issue if the size is not 2MB aligned? I think a PSMASH
>> will be done, but something to test if you are going to allow any page
>> alignment and page count.
>>
> 
> I believe that SNP_INIT_EX can add HV_Fixed pages which are not 2MB size aligned.
> 
> Here is a sub list of HV_Fixed pages being passed to SNP_INIT_EX: 
> 
> [   25.940837] base 0x0, count 1
> [   25.940838] base 0xa0000, count 96
> [   25.940839] base 0x75b60000, count 75
> [   25.940839] base 0x75c60000, count 928
> [   25.940840] base 0x88965000, count 83
> [   25.940841] base 0x8a40c000, count 1
> [   25.940841] base 0x8e14d000, count 48187
> [   25.940842] base 0x99d88000, count 235
> [   25.940842] base 0x99e73000, count 1153
> [   25.940843] base 0x9a2f4000, count 12043
> [   25.940844] base 0x9fffa000, count 5
> [   25.940844] base 0xa0000000, count 65536
> [   25.940845] base 0xb4000000, count 1
> [   25.940845] base 0xb5080000, count 1
> [   25.940846] base 0xbe100000, count 1
> [   25.940847] base 0xbf000000, count 1
> [   25.940847] base 0xd0080000, count 1
> [   25.940848] base 0xd1100000, count 1
> [   25.940848] base 0xec400000, count 1

Right, but those are resource-based items that I think result in 4K direct
map entries around them being 4K. I just want you to verify that a 2M
mapping will be split automatically by the SNP code (which I believe it
will, but we should verify).

Thanks,
Tom

> ...