Re: [PATCH 5/7] crypto: ccp - Allow SNP platform data to be queried after SNP INIT
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: 2026-09-10 20:09:49
Also in:
lkml
On 9/10/26 12:02, Pratik R. Sampat wrote:
quoted hunk ↗ jump to hunk
In preparation for refreshing the cached SNP platform status and feature information after a successful firmware live update from DOWNLOAD_FIRMWARE_EX, allow snp_get_platform_data() to be called while the SNP firmware is in the INIT state. Once SNP is initialized the firmware requires the output page of both commands to be firmware-owned. sev->snp_plat_status cannot satisfy that as it is embedded in struct sev_device, so use __sev_do_snp_platform_status(), which stages the output through a dedicated page, and mark/reclaim the SNP_FEATURE_INFO page around the command. Co-developed-by: Tycho Andersen (AMD) <tycho@kernel.org> Signed-off-by: Tycho Andersen (AMD) <tycho@kernel.org> Signed-off-by: Pratik R. Sampat <redacted> --- drivers/crypto/ccp/sev-dev.c | 44 ++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 10 deletions(-)diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index 1ed9e61a95cc..e891d6d1c6f0 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c@@ -131,6 +131,8 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic); static int snp_shutdown_on_panic(struct notifier_block *nb, unsigned long reason, void *arg); +static int __sev_do_snp_platform_status(struct sev_user_data_snp_status *status, + int *error); static struct notifier_block snp_panic_notifier = { .notifier_call = snp_shutdown_on_panic,@@ -1261,19 +1263,12 @@ static int snp_get_platform_data(struct sev_device *sev, int *error) { struct sev_data_snp_feature_info snp_feat_info; struct snp_feature_info *feat_info; - struct sev_data_snp_addr buf; struct page *page; int rc; - /* - * This function is expected to be called before SNP is - * initialized. - */ - if (sev->snp_initialized) - return -EINVAL; - - buf.address = __psp_pa(&sev->snp_plat_status); - rc = sev_do_cmd(SEV_CMD_SNP_PLATFORM_STATUS, &buf, error); + mutex_lock(&sev_cmd_mutex); + rc = __sev_do_snp_platform_status(&sev->snp_plat_status, error); + mutex_unlock(&sev_cmd_mutex); if (rc) { dev_err(sev->dev, "SNP PLATFORM_STATUS command failed, ret = %d, error = %#x\n", rc, *error);@@ -1302,17 +1297,46 @@ static int snp_get_platform_data(struct sev_device *sev, int *error) return -ENOMEM; feat_info = page_address(page); + + /* If SNP is initialized, transition to use a firmware-owned page */ + if (sev->snp_initialized) { + if (rmp_mark_pages_firmware(__pa(feat_info), 1, false)) { + *error = SEV_RET_NO_FW_CALL; + rc = -EFAULT; + goto free_page; + } + } +
You could change to use snp_alloc_firmware_page() which will allocate the page and place it in the proper state for you.
snp_feat_info.length = sizeof(snp_feat_info);
snp_feat_info.ecx_in = 0;
snp_feat_info.feature_info_paddr = __psp_pa(feat_info);
rc = sev_do_cmd(SEV_CMD_SNP_FEATURE_INFO, &snp_feat_info, error);
+
+ /*
+ * The feature_info page will be in reclaim state on success, or left
+ * in firmware state on failure. Transition the pages back to
+ * Hypervisor-owned state.
+ *
+ * snp_reclaim_pages() has already pinned the page via snp_leak_pages()
+ * if it could not do so, which keeps it away from the allocator. The
+ * reference taken here is dropped either way.
+ */
+ if (sev->snp_initialized) {
+ if (snp_reclaim_pages(__pa(feat_info), 1, false)) {
+ *error = SEV_RET_NO_FW_CALL;
+ rc = -EFAULT;
+ goto free_page;
+ }
+ }And then snp_free_firmware_page() here (after you copy the data - similar to what you did in snp_verify_mitigation()). (I think the same changes apply to __sev_do_snp_platform_status(), but don't worry about it for this series) Thanks, Tom
+ if (!rc) sev->snp_feat_info_0 = *feat_info; else dev_err(sev->dev, "SNP FEATURE_INFO command failed, ret = %d, error = %#x\n", rc, *error); +free_page: __free_page(page); return rc;