[PATCH v5 0/7] Add SEV-SNP CipherTextHiding feature support

[PATCH v5 0/7] Add SEV-SNP CipherTextHiding feature support

[Ashish Kalra]

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

Ciphertext hiding prevents host accesses from reading the ciphertext
of SNP guest private memory. Instead of reading ciphertext, the host
will see constant default values (0xff).

The SEV ASID space is basically split into legacy SEV and SEV-ES+.
CipherTextHiding further partitions the SEV-ES+ ASID space into SEV-ES
and SEV-SNP.

Add new module parameter to the KVM module to enable CipherTextHiding
support and a user configurable system-wide maximum SNP ASID value. If
the module parameter value is "max" then the complete SEV-ES+ ASID
space is allocated to SEV-SNP guests.

v5:
- Add pre-patch to cache SEV platform status and use this cached
information to set api_major/api_minor/build.
- Since the SEV platform status and SNP platform status differ, 
remove the state field from sev_device structure and instead track
SEV platform state from cached SEV platform status.
- If SNP is enabled then cached SNP platform status is used for 
api_major/api_minor/build.
- Fix using sev_do_cmd() instead of __sev_do_cmd_locked().
- Fix commit logs.
- Fix kernel-parameters documentation. 
- Modify KVM module parameter to enable CipherTextHiding to support
"max" option to allow complete SEV-ES+ ASID space to be allocated
to SEV-SNP guests.
- Do not enable ciphertext hiding if module parameter to specify
maximum SNP ASID is invalid.

v4:
- Fix buffer allocation for SNP_FEATURE_INFO command to correctly
handle page boundary check requirements.
- Return correct length for SNP_FEATURE_INFO command from
sev_cmd_buffer_len().
- Switch to using SNP platform status instead of SEV platform status if
SNP is enabled and cache SNP platform status and feature information.
Modify sev_get_api_version() accordingly.
- Fix commit logs.
- Expand the comments on why both the feature info and the platform
status fields have to be checked for CipherTextHiding feature 
detection and enablement.
- Add new preperation patch for CipherTextHiding feature which
introduces new {min,max}_{sev_es,snp}_asid variables along with
existing {min,max}_sev_asid variable to simplify partitioning of the
SEV and SEV-ES+ ASID space.
- Switch to single KVM module parameter to enable CipherTextHiding
feature and the maximum SNP ASID usable for SNP guests when 
CipherTextHiding feature is enabled.

v3:
- rebase to linux-next.
- rebase on top of support to move SEV-SNP initialization to
KVM module from CCP driver.
- Split CipherTextHiding support between CCP driver and KVM module
with KVM module calling into CCP driver to initialize SNP with
CipherTextHiding enabled and MAX ASID usable for SNP guest if
KVM is enabling CipherTextHiding feature.
- Move module parameters to enable CipherTextHiding feature and
MAX ASID usable for SNP guests from CCP driver to KVM module
which allows KVM to be responsible for enabling CipherTextHiding
feature if end-user requests it.

v2:
- Fix and add more description to commit logs.
- Rename sev_cache_snp_platform_status_and_discover_features() to 
snp_get_platform_data().
- Add check in snp_get_platform_data to guard against being called
after SNP_INIT_EX.
- Fix comments for new structure field definitions being added.
- Fix naming for new structure being added.
- Add new vm-type parameter to sev_asid_new().
- Fix identation.
- Rename CCP module parameters psp_cth_enabled to cipher_text_hiding and 
psp_max_snp_asid to max_snp_asid.
- Rename max_snp_asid to snp_max_snp_asid. 

Ashish Kalra (7):
  crypto: ccp - New bit-field definitions for SNP_PLATFORM_STATUS
    command
  crypto: ccp - Cache SEV platform status and platform state
  crypto: ccp - Add support for SNP_FEATURE_INFO command
  crypto: ccp - Introduce new API interface to indicate SEV-SNP
    Ciphertext hiding feature
  crypto: ccp - Add support to enable CipherTextHiding on SNP_INIT_EX
  KVM: SEV: Introduce new min,max sev_es and sev_snp asid variables
  KVM: SEV: Add SEV-SNP CipherTextHiding support

 .../admin-guide/kernel-parameters.txt         |  19 +++
 arch/x86/kvm/svm/sev.c                        |  94 +++++++++++--
 drivers/crypto/ccp/sev-dev.c                  | 127 ++++++++++++++++--
 drivers/crypto/ccp/sev-dev.h                  |   6 +-
 include/linux/psp-sev.h                       |  44 +++++-
 include/uapi/linux/psp-sev.h                  |  10 +-
 6 files changed, 273 insertions(+), 27 deletions(-)

-- 
2.34.1

[PATCH v5 1/7] crypto: ccp - New bit-field definitions for SNP_PLATFORM_STATUS command

[Ashish Kalra]

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

Define new bit-field definitions returned by SNP_PLATFORM_STATUS command
such as new capabilities like SNP_FEATURE_INFO command availability,
ciphertext hiding enabled and capability.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 include/uapi/linux/psp-sev.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h
index eeb20dfb1fda..c2fd324623c4 100644
--- a/include/uapi/linux/psp-sev.h
+++ b/include/uapi/linux/psp-sev.h
@@ -185,6 +185,10 @@ struct sev_user_data_get_id2 {
  * @mask_chip_id: whether chip id is present in attestation reports or not
  * @mask_chip_key: whether attestation reports are signed or not
  * @vlek_en: VLEK (Version Loaded Endorsement Key) hashstick is loaded
+ * @feature_info: whether SNP_FEATURE_INFO command is available
+ * @rapl_dis: whether RAPL is disabled
+ * @ciphertext_hiding_cap: whether platform has ciphertext hiding capability
+ * @ciphertext_hiding_en: whether ciphertext hiding is enabled
  * @rsvd1: reserved
  * @guest_count: the number of guest currently managed by the firmware
  * @current_tcb_version: current TCB version
@@ -200,7 +204,11 @@ struct sev_user_data_snp_status {
 	__u32 mask_chip_id:1;		/* Out */
 	__u32 mask_chip_key:1;		/* Out */
 	__u32 vlek_en:1;		/* Out */
-	__u32 rsvd1:29;
+	__u32 feature_info:1;		/* Out */
+	__u32 rapl_dis:1;		/* Out */
+	__u32 ciphertext_hiding_cap:1;	/* Out */
+	__u32 ciphertext_hiding_en:1;	/* Out */
+	__u32 rsvd1:25;
 	__u32 guest_count;		/* Out */
 	__u64 current_tcb_version;	/* Out */
 	__u64 reported_tcb_version;	/* Out */
-- 
2.34.1

[PATCH v5 2/7] crypto: ccp - Cache SEV platform status and platform state

[Ashish Kalra]

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

Cache the SEV platform status into sev_device structure and use this
cached SEV platform status for api_major/minor/build.

The platform state is unique between SEV and SNP and hence needs to be
tracked independently.

Remove the state field from sev_device structure and instead track SEV
state from the cached SEV platform status.

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 22 ++++++++++++----------
 drivers/crypto/ccp/sev-dev.h |  3 ++-
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 17edc6bf5622..5a2e1651d171 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1286,7 +1286,7 @@ static int __sev_platform_init_locked(int *error)
 
 	sev = psp_master->sev_data;
 
-	if (sev->state == SEV_STATE_INIT)
+	if (sev->sev_plat_status.state == SEV_STATE_INIT)
 		return 0;
 
 	__sev_platform_init_handle_tmr(sev);
@@ -1318,7 +1318,7 @@ static int __sev_platform_init_locked(int *error)
 		return rc;
 	}
 
-	sev->state = SEV_STATE_INIT;
+	sev->sev_plat_status.state = SEV_STATE_INIT;
 
 	/* Prepare for first SEV guest launch after INIT */
 	wbinvd_on_all_cpus();
@@ -1347,7 +1347,7 @@ static int _sev_platform_init_locked(struct sev_platform_init_args *args)
 
 	sev = psp_master->sev_data;
 
-	if (sev->state == SEV_STATE_INIT)
+	if (sev->sev_plat_status.state == SEV_STATE_INIT)
 		return 0;
 
 	rc = __sev_snp_init_locked(&args->error);
@@ -1384,7 +1384,7 @@ static int __sev_platform_shutdown_locked(int *error)
 
 	sev = psp->sev_data;
 
-	if (sev->state == SEV_STATE_UNINIT)
+	if (sev->sev_plat_status.state == SEV_STATE_UNINIT)
 		return 0;
 
 	ret = __sev_do_cmd_locked(SEV_CMD_SHUTDOWN, NULL, error);
@@ -1394,7 +1394,7 @@ static int __sev_platform_shutdown_locked(int *error)
 		return ret;
 	}
 
-	sev->state = SEV_STATE_UNINIT;
+	sev->sev_plat_status.state = SEV_STATE_UNINIT;
 	dev_dbg(sev->dev, "SEV firmware shutdown\n");
 
 	return ret;
@@ -1502,7 +1502,7 @@ static int sev_ioctl_do_pek_pdh_gen(int cmd, struct sev_issue_cmd *argp, bool wr
 	if (!writable)
 		return -EPERM;
 
-	if (sev->state == SEV_STATE_UNINIT) {
+	if (sev->sev_plat_status.state == SEV_STATE_UNINIT) {
 		rc = sev_move_to_init_state(argp, &shutdown_required);
 		if (rc)
 			return rc;
@@ -1551,7 +1551,7 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable)
 	data.len = input.length;
 
 cmd:
-	if (sev->state == SEV_STATE_UNINIT) {
+	if (sev->sev_plat_status.state == SEV_STATE_UNINIT) {
 		ret = sev_move_to_init_state(argp, &shutdown_required);
 		if (ret)
 			goto e_free_blob;
@@ -1606,10 +1606,12 @@ static int sev_get_api_version(void)
 		return 1;
 	}
 
+	/* Cache SEV platform status */
+	sev->sev_plat_status = status;
+
 	sev->api_major = status.api_major;
 	sev->api_minor = status.api_minor;
 	sev->build = status.build;
-	sev->state = status.state;
 
 	return 0;
 }
@@ -1837,7 +1839,7 @@ static int sev_ioctl_do_pek_import(struct sev_issue_cmd *argp, bool writable)
 	data.oca_cert_len = input.oca_cert_len;
 
 	/* If platform is not in INIT state then transition it to INIT */
-	if (sev->state != SEV_STATE_INIT) {
+	if (sev->sev_plat_status.state != SEV_STATE_INIT) {
 		ret = sev_move_to_init_state(argp, &shutdown_required);
 		if (ret)
 			goto e_free_oca;
@@ -2008,7 +2010,7 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable)
 
 cmd:
 	/* If platform is not in INIT state then transition it to INIT. */
-	if (sev->state != SEV_STATE_INIT) {
+	if (sev->sev_plat_status.state != SEV_STATE_INIT) {
 		if (!writable) {
 			ret = -EPERM;
 			goto e_free_cert;
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index 3e4e5574e88a..24dd8ff8afaa 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -42,7 +42,6 @@ struct sev_device {
 
 	struct sev_vdata *vdata;
 
-	int state;
 	unsigned int int_rcvd;
 	wait_queue_head_t int_queue;
 	struct sev_misc_dev *misc;
@@ -57,6 +56,8 @@ struct sev_device {
 	bool cmd_buf_backup_active;
 
 	bool snp_initialized;
+
+	struct sev_user_data_status sev_plat_status;
 };
 
 int sev_dev_init(struct psp_device *psp);
-- 
2.34.1

[PATCH v5 3/7] crypto: ccp - Add support for SNP_FEATURE_INFO command

[Ashish Kalra]

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

The FEATURE_INFO command provides hypervisors a programmatic means to
learn about the supported features of the currently loaded firmware.
FEATURE_INFO command leverages the same mechanism as the CPUID
instruction. Instead of using the CPUID instruction to retrieve
Fn8000_0024, software can use FEATURE_INFO.

Switch to using SNP platform status instead of SEV platform status if
SNP is enabled and cache SNP platform status and feature information
from CPUID 0x7fff_0024, sub-function 0, in the sev_device structure.

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 72 ++++++++++++++++++++++++++++++++++++
 drivers/crypto/ccp/sev-dev.h |  3 ++
 include/linux/psp-sev.h      | 29 +++++++++++++++
 3 files changed, 104 insertions(+)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 5a2e1651d171..d1517a91a27d 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -233,6 +233,7 @@ static int sev_cmd_buffer_len(int cmd)
 	case SEV_CMD_SNP_GUEST_REQUEST:		return sizeof(struct sev_data_snp_guest_request);
 	case SEV_CMD_SNP_CONFIG:		return sizeof(struct sev_user_data_snp_config);
 	case SEV_CMD_SNP_COMMIT:		return sizeof(struct sev_data_snp_commit);
+	case SEV_CMD_SNP_FEATURE_INFO:		return sizeof(struct sev_data_snp_feature_info);
 	default:				return 0;
 	}
 
@@ -1073,6 +1074,67 @@ static void snp_set_hsave_pa(void *arg)
 	wrmsrq(MSR_VM_HSAVE_PA, 0);
 }
 
+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 not
+	 * 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);
+	if (rc) {
+		dev_err(sev->dev, "SNP PLATFORM_STATUS command failed, ret = %d, error = %#x\n",
+			rc, *error);
+		return rc;
+	}
+
+	sev->api_major = sev->snp_plat_status.api_major;
+	sev->api_minor = sev->snp_plat_status.api_minor;
+	sev->build = sev->snp_plat_status.build_id;
+
+	/*
+	 * Do feature discovery of the currently loaded firmware,
+	 * and cache feature information from CPUID 0x8000_0024,
+	 * sub-function 0.
+	 */
+	if (!sev->snp_plat_status.feature_info)
+		return 0;
+
+	/*
+	 * Use dynamically allocated structure for the SNP_FEATURE_INFO
+	 * command to ensure structure is 8-byte aligned, and does not
+	 * cross a page boundary.
+	 */
+	page = alloc_page(GFP_KERNEL);
+	if (!page)
+		return -ENOMEM;
+
+	feat_info = page_address(page);
+	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);
+	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(page);
+
+	return rc;
+}
+
 static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
 {
 	struct sev_data_range_list *range_list = arg;
@@ -1599,6 +1661,16 @@ static int sev_get_api_version(void)
 	struct sev_user_data_status status;
 	int error = 0, ret;
 
+	/*
+	 * Cache SNP platform status and SNP feature information
+	 * if SNP is available.
+	 */
+	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP)) {
+		ret = snp_get_platform_data(sev, &error);
+		if (ret)
+			return 1;
+	}
+
 	ret = sev_platform_status(&status, &error);
 	if (ret) {
 		dev_err(sev->dev,
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index 24dd8ff8afaa..5aed2595c9ae 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -58,6 +58,9 @@ struct sev_device {
 	bool snp_initialized;
 
 	struct sev_user_data_status sev_plat_status;
+
+	struct sev_user_data_snp_status snp_plat_status;
+	struct snp_feature_info snp_feat_info_0;
 };
 
 int sev_dev_init(struct psp_device *psp);
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 0f5f94137f6d..935547c26985 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -107,6 +107,7 @@ enum sev_cmd {
 	SEV_CMD_SNP_DOWNLOAD_FIRMWARE_EX = 0x0CA,
 	SEV_CMD_SNP_COMMIT		= 0x0CB,
 	SEV_CMD_SNP_VLEK_LOAD		= 0x0CD,
+	SEV_CMD_SNP_FEATURE_INFO	= 0x0CE,
 
 	SEV_CMD_MAX,
 };
@@ -814,6 +815,34 @@ struct sev_data_snp_commit {
 	u32 len;
 } __packed;
 
+/**
+ * struct sev_data_snp_feature_info - SEV_SNP_FEATURE_INFO structure
+ *
+ * @length: len of the command buffer read by the PSP
+ * @ecx_in: subfunction index
+ * @feature_info_paddr : SPA of the FEATURE_INFO structure
+ */
+struct sev_data_snp_feature_info {
+	u32 length;
+	u32 ecx_in;
+	u64 feature_info_paddr;
+} __packed;
+
+/**
+ * struct feature_info - FEATURE_INFO structure
+ *
+ * @eax: output of SNP_FEATURE_INFO command
+ * @ebx: output of SNP_FEATURE_INFO command
+ * @ecx: output of SNP_FEATURE_INFO command
+ * #edx: output of SNP_FEATURE_INFO command
+ */
+struct snp_feature_info {
+	u32 eax;
+	u32 ebx;
+	u32 ecx;
+	u32 edx;
+} __packed;
+
 #ifdef CONFIG_CRYPTO_DEV_SP_PSP
 
 /**
-- 
2.34.1

[PATCH v5 4/7] crypto: ccp - Introduce new API interface to indicate SEV-SNP Ciphertext hiding feature

[Ashish Kalra]

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

Implement a new API interface that indicates both the support for the
SEV-SNP Ciphertext Hiding feature by the SEV firmware and whether this
feature is enabled in the platform BIOS.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 21 +++++++++++++++++++++
 include/linux/psp-sev.h      |  5 +++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index d1517a91a27d..3f2bbba93617 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1074,6 +1074,27 @@ static void snp_set_hsave_pa(void *arg)
 	wrmsrq(MSR_VM_HSAVE_PA, 0);
 }
 
+bool sev_is_snp_ciphertext_hiding_supported(void)
+{
+	struct psp_device *psp = psp_master;
+	struct sev_device *sev;
+
+	if (!psp || !psp->sev_data)
+		return false;
+
+	sev = psp->sev_data;
+
+	/*
+	 * Feature information indicates if CipherTextHiding feature is
+	 * supported by the SEV firmware and additionally platform status
+	 * indicates if CipherTextHiding feature is enabled in the
+	 * Platform BIOS.
+	 */
+	return ((sev->snp_feat_info_0.ecx & SNP_CIPHER_TEXT_HIDING_SUPPORTED) &&
+		 sev->snp_plat_status.ciphertext_hiding_cap);
+}
+EXPORT_SYMBOL_GPL(sev_is_snp_ciphertext_hiding_supported);
+
 static int snp_get_platform_data(struct sev_device *sev, int *error)
 {
 	struct sev_data_snp_feature_info snp_feat_info;
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 935547c26985..ca19fddfcd4d 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -843,6 +843,8 @@ struct snp_feature_info {
 	u32 edx;
 } __packed;
 
+#define SNP_CIPHER_TEXT_HIDING_SUPPORTED	BIT(3)
+
 #ifdef CONFIG_CRYPTO_DEV_SP_PSP
 
 /**
@@ -986,6 +988,7 @@ void *psp_copy_user_blob(u64 uaddr, u32 len);
 void *snp_alloc_firmware_page(gfp_t mask);
 void snp_free_firmware_page(void *addr);
 void sev_platform_shutdown(void);
+bool sev_is_snp_ciphertext_hiding_supported(void);
 
 #else	/* !CONFIG_CRYPTO_DEV_SP_PSP */
 
@@ -1022,6 +1025,8 @@ static inline void snp_free_firmware_page(void *addr) { }
 
 static inline void sev_platform_shutdown(void) { }
 
+static inline bool sev_is_snp_ciphertext_hiding_supported(void) { return false; }
+
 #endif	/* CONFIG_CRYPTO_DEV_SP_PSP */
 
 #endif	/* __PSP_SEV_H__ */
-- 
2.34.1

[PATCH v5 5/7] crypto: ccp - Add support to enable CipherTextHiding on SNP_INIT_EX

[Ashish Kalra]

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

Ciphertext hiding needs to be enabled on SNP_INIT_EX.

Enhance sev_platform_init_args by adding a new argument that enables the
KVM module to specify whether the Ciphertext Hiding feature should be
activated during SNP initialization. Additionally, this argument will
allow for the definition of the maximum ASID that can be used for an
SEV-SNP guest when Ciphertext Hiding is enabled.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 drivers/crypto/ccp/sev-dev.c | 12 +++++++++---
 include/linux/psp-sev.h      | 10 ++++++++--
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 3f2bbba93617..c883ccf8c3ff 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1186,7 +1186,7 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
 	return 0;
 }
 
-static int __sev_snp_init_locked(int *error)
+static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
 {
 	struct psp_device *psp = psp_master;
 	struct sev_data_snp_init_ex data;
@@ -1247,6 +1247,12 @@ static int __sev_snp_init_locked(int *error)
 		}
 
 		memset(&data, 0, sizeof(data));
+
+		if (max_snp_asid) {
+			data.ciphertext_hiding_en = 1;
+			data.max_snp_asid = max_snp_asid;
+		}
+
 		data.init_rmp = 1;
 		data.list_paddr_en = 1;
 		data.list_paddr = __psp_pa(snp_range_list);
@@ -1433,7 +1439,7 @@ static int _sev_platform_init_locked(struct sev_platform_init_args *args)
 	if (sev->sev_plat_status.state == SEV_STATE_INIT)
 		return 0;
 
-	rc = __sev_snp_init_locked(&args->error);
+	rc = __sev_snp_init_locked(&args->error, args->max_snp_asid);
 	if (rc && rc != -ENODEV)
 		return rc;
 
@@ -1516,7 +1522,7 @@ static int snp_move_to_init_state(struct sev_issue_cmd *argp, bool *shutdown_req
 {
 	int error, rc;
 
-	rc = __sev_snp_init_locked(&error);
+	rc = __sev_snp_init_locked(&error, 0);
 	if (rc) {
 		argp->error = SEV_RET_INVALID_PLATFORM_STATE;
 		return rc;
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index ca19fddfcd4d..b6eda9c560ee 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -748,10 +748,13 @@ struct sev_data_snp_guest_request {
 struct sev_data_snp_init_ex {
 	u32 init_rmp:1;
 	u32 list_paddr_en:1;
-	u32 rsvd:30;
+	u32 rapl_dis:1;
+	u32 ciphertext_hiding_en:1;
+	u32 rsvd:28;
 	u32 rsvd1;
 	u64 list_paddr;
-	u8  rsvd2[48];
+	u16 max_snp_asid;
+	u8  rsvd2[46];
 } __packed;
 
 /**
@@ -800,10 +803,13 @@ struct sev_data_snp_shutdown_ex {
  * @probe: True if this is being called as part of CCP module probe, which
  *  will defer SEV_INIT/SEV_INIT_EX firmware initialization until needed
  *  unless psp_init_on_probe module param is set
+ * @max_snp_asid: maximum ASID usable for SEV-SNP guest if
+ *  CipherTextHiding feature is to be enabled
  */
 struct sev_platform_init_args {
 	int error;
 	bool probe;
+	unsigned int max_snp_asid;
 };
 
 /**
-- 
2.34.1

[PATCH v5 6/7] KVM: SEV: Introduce new min,max sev_es and sev_snp asid variables

[Ashish Kalra]

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

Introduce new min, max sev_es_asid and sev_snp_asid variables.

The new {min,max}_{sev_es,snp}_asid variables along with existing
{min,max}_sev_asid variable simplifies partitioning of the
SEV and SEV-ES+ ASID space.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 arch/x86/kvm/svm/sev.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index fde328ed3f78..89ce9e298201 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -85,6 +85,10 @@ static DECLARE_RWSEM(sev_deactivate_lock);
 static DEFINE_MUTEX(sev_bitmap_lock);
 unsigned int max_sev_asid;
 static unsigned int min_sev_asid;
+static unsigned int max_sev_es_asid;
+static unsigned int min_sev_es_asid;
+static unsigned int max_snp_asid;
+static unsigned int min_snp_asid;
 static unsigned long sev_me_mask;
 static unsigned int nr_asids;
 static unsigned long *sev_asid_bitmap;
@@ -172,20 +176,31 @@ static void sev_misc_cg_uncharge(struct kvm_sev_info *sev)
 	misc_cg_uncharge(type, sev->misc_cg, 1);
 }
 
-static int sev_asid_new(struct kvm_sev_info *sev)
+static int sev_asid_new(struct kvm_sev_info *sev, unsigned long vm_type)
 {
 	/*
 	 * SEV-enabled guests must use asid from min_sev_asid to max_sev_asid.
 	 * SEV-ES-enabled guest can use from 1 to min_sev_asid - 1.
-	 * Note: min ASID can end up larger than the max if basic SEV support is
-	 * effectively disabled by disallowing use of ASIDs for SEV guests.
 	 */
-	unsigned int min_asid = sev->es_active ? 1 : min_sev_asid;
-	unsigned int max_asid = sev->es_active ? min_sev_asid - 1 : max_sev_asid;
-	unsigned int asid;
+	unsigned int min_asid, max_asid, asid;
 	bool retry = true;
 	int ret;
 
+	if (vm_type == KVM_X86_SNP_VM) {
+		min_asid = min_snp_asid;
+		max_asid = max_snp_asid;
+	} else if (sev->es_active) {
+		min_asid = min_sev_es_asid;
+		max_asid = max_sev_es_asid;
+	} else {
+		min_asid = min_sev_asid;
+		max_asid = max_sev_asid;
+	}
+
+	/*
+	 * The min ASID can end up larger than the max if basic SEV support is
+	 * effectively disabled by disallowing use of ASIDs for SEV guests.
+	 */
 	if (min_asid > max_asid)
 		return -ENOTTY;
 
@@ -439,7 +454,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
 	if (vm_type == KVM_X86_SNP_VM)
 		sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
 
-	ret = sev_asid_new(sev);
+	ret = sev_asid_new(sev, vm_type);
 	if (ret)
 		goto e_no_asid;
 
@@ -2996,6 +3011,9 @@ void __init sev_hardware_setup(void)
 	if (min_sev_asid == 1)
 		goto out;
 
+	min_sev_es_asid = min_snp_asid = 1;
+	max_sev_es_asid = max_snp_asid = min_sev_asid - 1;
+
 	sev_es_asid_count = min_sev_asid - 1;
 	WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count));
 	sev_es_supported = true;
@@ -3019,11 +3037,11 @@ void __init sev_hardware_setup(void)
 	if (boot_cpu_has(X86_FEATURE_SEV_ES))
 		pr_info("SEV-ES %s (ASIDs %u - %u)\n",
 			str_enabled_disabled(sev_es_supported),
-			min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1);
+			min_sev_es_asid, max_sev_es_asid);
 	if (boot_cpu_has(X86_FEATURE_SEV_SNP))
 		pr_info("SEV-SNP %s (ASIDs %u - %u)\n",
 			str_enabled_disabled(sev_snp_supported),
-			min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1);
+			min_snp_asid, max_snp_asid);
 
 	sev_enabled = sev_supported;
 	sev_es_enabled = sev_es_supported;
-- 
2.34.1

[PATCH v5 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support

[Ashish Kalra]

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

Ciphertext hiding prevents host accesses from reading the ciphertext of
SNP guest private memory. Instead of reading ciphertext, the host reads
will see constant default values (0xff).

The SEV ASID space is basically split into legacy SEV and SEV-ES+.
Ciphertext hiding further partitions the SEV-ES+ ASID space into SEV-ES
and SEV-SNP.

Add new module parameter to the KVM module to enable Ciphertext hiding
support and a user configurable system-wide maximum SNP ASID value. If
the module parameter value is "max" then the complete SEV-ES+ ASID
space is allocated to SEV-SNP guests.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
---
 .../admin-guide/kernel-parameters.txt         | 19 ++++++
 arch/x86/kvm/svm/sev.c                        | 58 ++++++++++++++++++-
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index ee0735c6b8e2..05e50c37969e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2942,6 +2942,25 @@
 			(enabled). Disable by KVM if hardware lacks support
 			for NPT.
 
+	kvm-amd.ciphertext_hiding_asids=
+			[KVM,AMD] Ciphertext hiding prevents host accesses from reading
+			the ciphertext of SNP guest private memory. Instead of reading
+			ciphertext, the host will see constant default values (0xff).
+			The SEV ASID space is split into legacy SEV and joint
+			SEV-ES and SEV-SNP ASID space. Ciphertext hiding further
+			partitions the joint SEV-ES/SEV-SNP ASID space into separate
+			SEV-ES and SEV-SNP ASID ranges with the SEV-SNP ASID range
+			starting at 1. For SEV-ES/SEV-SNP guests the maximum ASID
+			available is MIN_SEV_ASID - 1 and MIN_SEV_ASID value is
+			discovered by CPUID Fn8000_001F[EDX].
+
+			Format: { <unsigned int> | "max" }
+			A non-zero value enables SEV-SNP CipherTextHiding feature and sets
+			how many ASIDs are available for SEV-SNP guests.
+			A Value of "max" assigns all ASIDs available in the joint SEV-ES
+			and SEV-SNP ASID range to SNP guests and also effectively disables
+			SEV-ES.
+
 	kvm-arm.mode=
 			[KVM,ARM,EARLY] Select one of KVM/arm64's modes of
 			operation.
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 89ce9e298201..16723b8e0e37 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -59,6 +59,11 @@ static bool sev_es_debug_swap_enabled = true;
 module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444);
 static u64 sev_supported_vmsa_features;
 
+static char ciphertext_hiding_asids[16];
+module_param_string(ciphertext_hiding_asids, ciphertext_hiding_asids,
+		    sizeof(ciphertext_hiding_asids), 0444);
+MODULE_PARM_DESC(ciphertext_hiding_asids, "  Enable ciphertext hiding for SEV-SNP guests and set the number of ASIDs to use ('max' to use all available SEV-SNP ASIDs");
+
 #define AP_RESET_HOLD_NONE		0
 #define AP_RESET_HOLD_NAE_EVENT		1
 #define AP_RESET_HOLD_MSR_PROTO		2
@@ -200,6 +205,9 @@ static int sev_asid_new(struct kvm_sev_info *sev, unsigned long vm_type)
 	/*
 	 * The min ASID can end up larger than the max if basic SEV support is
 	 * effectively disabled by disallowing use of ASIDs for SEV guests.
+	 * Similarly for SEV-ES guests the min ASID can end up larger than the
+	 * max when ciphertext hiding is enabled, effectively disabling SEV-ES
+	 * support.
 	 */
 	if (min_asid > max_asid)
 		return -ENOTTY;
@@ -2913,10 +2921,46 @@ static bool is_sev_snp_initialized(void)
 	return initialized;
 }
 
+static bool check_and_enable_sev_snp_ciphertext_hiding(void)
+{
+	unsigned int ciphertext_hiding_asid_nr = 0;
+
+	if (!sev_is_snp_ciphertext_hiding_supported()) {
+		pr_warn("Module parameter ciphertext_hiding_asids specified but ciphertext hiding not supported or enabled\n");
+		return false;
+	}
+
+	if (isdigit(ciphertext_hiding_asids[0])) {
+		if (kstrtoint(ciphertext_hiding_asids, 10, &ciphertext_hiding_asid_nr)) {
+			pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
+				ciphertext_hiding_asids);
+			return false;
+		}
+		/* Do sanity checks on user-defined ciphertext_hiding_asids */
+		if (ciphertext_hiding_asid_nr >= min_sev_asid) {
+			pr_warn("Requested ciphertext hiding ASIDs (%u) exceeds or equals minimum SEV ASID (%u)\n",
+				ciphertext_hiding_asid_nr, min_sev_asid);
+			return false;
+		}
+	} else if (!strcmp(ciphertext_hiding_asids, "max")) {
+		ciphertext_hiding_asid_nr = min_sev_asid - 1;
+	} else {
+		pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
+			ciphertext_hiding_asids);
+		return false;
+	}
+
+	max_snp_asid = ciphertext_hiding_asid_nr;
+	min_sev_es_asid = max_snp_asid + 1;
+	pr_info("SEV-SNP ciphertext hiding enabled\n");
+	return true;
+}
+
 void __init sev_hardware_setup(void)
 {
 	unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count;
 	struct sev_platform_init_args init_args = {0};
+	bool snp_ciphertext_hiding_enabled = false;
 	bool sev_snp_supported = false;
 	bool sev_es_supported = false;
 	bool sev_supported = false;
@@ -3014,6 +3058,14 @@ void __init sev_hardware_setup(void)
 	min_sev_es_asid = min_snp_asid = 1;
 	max_sev_es_asid = max_snp_asid = min_sev_asid - 1;
 
+	/*
+	 * The ciphertext hiding feature partitions the joint SEV-ES/SEV-SNP
+	 * ASID range into separate SEV-ES and SEV-SNP ASID ranges with
+	 * the SEV-SNP ASID starting at 1.
+	 */
+	if (ciphertext_hiding_asids[0])
+		snp_ciphertext_hiding_enabled = check_and_enable_sev_snp_ciphertext_hiding();
+
 	sev_es_asid_count = min_sev_asid - 1;
 	WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count));
 	sev_es_supported = true;
@@ -3022,6 +3074,8 @@ void __init sev_hardware_setup(void)
 out:
 	if (sev_enabled) {
 		init_args.probe = true;
+		if (snp_ciphertext_hiding_enabled)
+			init_args.max_snp_asid = max_snp_asid;
 		if (sev_platform_init(&init_args))
 			sev_supported = sev_es_supported = sev_snp_supported = false;
 		else if (sev_snp_supported)
@@ -3036,7 +3090,9 @@ void __init sev_hardware_setup(void)
 			min_sev_asid, max_sev_asid);
 	if (boot_cpu_has(X86_FEATURE_SEV_ES))
 		pr_info("SEV-ES %s (ASIDs %u - %u)\n",
-			str_enabled_disabled(sev_es_supported),
+			sev_es_supported ? min_sev_es_asid < min_sev_asid ? "enabled" :
+									    "unusable" :
+									    "disabled",
 			min_sev_es_asid, max_sev_es_asid);
 	if (boot_cpu_has(X86_FEATURE_SEV_SNP))
 		pr_info("SEV-SNP %s (ASIDs %u - %u)\n",
-- 
2.34.1

Re: [PATCH v5 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support

[Kim Phillips]

Hi Ashish,

I can confirm that this v5 series fixes v4's __sev_do_cmd_locked
assertion failure problem, thanks.  More comments inline:

On 7/1/25 3:16 PM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>

Extra From: line not necessary.

> @@ -2913,10 +2921,46 @@ static bool is_sev_snp_initialized(void)
>   	return initialized;
>   }
>   
> +static bool check_and_enable_sev_snp_ciphertext_hiding(void)
> +{
> +	unsigned int ciphertext_hiding_asid_nr = 0;
> +
> +	if (!sev_is_snp_ciphertext_hiding_supported()) {
> +		pr_warn("Module parameter ciphertext_hiding_asids specified but ciphertext hiding not supported or enabled\n");
> +		return false;
> +	}
> +
> +	if (isdigit(ciphertext_hiding_asids[0])) {
> +		if (kstrtoint(ciphertext_hiding_asids, 10, &ciphertext_hiding_asid_nr)) {
> +			pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
> +				ciphertext_hiding_asids);
> +			return false;
> +		}
> +		/* Do sanity checks on user-defined ciphertext_hiding_asids */
> +		if (ciphertext_hiding_asid_nr >= min_sev_asid) {
> +			pr_warn("Requested ciphertext hiding ASIDs (%u) exceeds or equals minimum SEV ASID (%u)\n",
> +				ciphertext_hiding_asid_nr, min_sev_asid);
> +			return false;
> +		}
> +	} else if (!strcmp(ciphertext_hiding_asids, "max")) {
> +		ciphertext_hiding_asid_nr = min_sev_asid - 1;
> +	} else {
> +		pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
> +			ciphertext_hiding_asids);
> +		return false;
> +	}

This code can be made much simpler if all the invalid
cases were combined to emit a single pr_warn().

> @@ -3036,7 +3090,9 @@ void __init sev_hardware_setup(void)
>   			min_sev_asid, max_sev_asid);
>   	if (boot_cpu_has(X86_FEATURE_SEV_ES))
>   		pr_info("SEV-ES %s (ASIDs %u - %u)\n",
> -			str_enabled_disabled(sev_es_supported),
> +			sev_es_supported ? min_sev_es_asid < min_sev_asid ? "enabled" :
> +									    "unusable" :
> +									    "disabled",
>   			min_sev_es_asid, max_sev_es_asid);
>   	if (boot_cpu_has(X86_FEATURE_SEV_SNP))
>   		pr_info("SEV-SNP %s (ASIDs %u - %u)\n",

If I set ciphertext_hiding_asids=99, I get the new 'unusable':

kvm_amd: SEV-SNP ciphertext hiding enabled
...
kvm_amd: SEV enabled (ASIDs 100 - 1006)
kvm_amd: SEV-ES unusable (ASIDs 100 - 99)
kvm_amd: SEV-SNP enabled (ASIDs 1 - 99)

Ok.

Now, if I set ciphertext_hiding_asids=0, I get:

kvm_amd: SEV-SNP ciphertext hiding enabled
...
kvm_amd: SEV enabled (ASIDs 100 - 1006)
kvm_amd: SEV-ES enabled (ASIDs 1 - 99)
kvm_amd: SEV-SNP enabled (ASIDs 1 - 0)

..where SNP is unusable this time, yet it's not flagged as such.

If there's no difference between "unusable" and not enabled, then
I think it's better to keep the not enabled messaging behaviour
and just not emit the line at all:  It's confusing to see the
invalid "100 - 99" and "1 - 0" ranges.

Thanks,

Kim

Re: [PATCH v5 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support

[Kalra, Ashish]

Hello Kim,

On 7/2/2025 4:46 PM, Kim Phillips wrote:
> Hi Ashish,
> 
> I can confirm that this v5 series fixes v4's __sev_do_cmd_locked
> assertion failure problem, thanks.  More comments inline:
> 
> On 7/1/25 3:16 PM, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Extra From: line not necessary.
> 
>> @@ -2913,10 +2921,46 @@ static bool is_sev_snp_initialized(void)
>>       return initialized;
>>   }
>>   +static bool check_and_enable_sev_snp_ciphertext_hiding(void)
>> +{
>> +    unsigned int ciphertext_hiding_asid_nr = 0;
>> +
>> +    if (!sev_is_snp_ciphertext_hiding_supported()) {
>> +        pr_warn("Module parameter ciphertext_hiding_asids specified but ciphertext hiding not supported or enabled\n");
>> +        return false;
>> +    }
>> +
>> +    if (isdigit(ciphertext_hiding_asids[0])) {
>> +        if (kstrtoint(ciphertext_hiding_asids, 10, &ciphertext_hiding_asid_nr)) {
>> +            pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
>> +                ciphertext_hiding_asids);
>> +            return false;
>> +        }
>> +        /* Do sanity checks on user-defined ciphertext_hiding_asids */
>> +        if (ciphertext_hiding_asid_nr >= min_sev_asid) {
>> +            pr_warn("Requested ciphertext hiding ASIDs (%u) exceeds or equals minimum SEV ASID (%u)\n",
>> +                ciphertext_hiding_asid_nr, min_sev_asid);
>> +            return false;
>> +        }
>> +    } else if (!strcmp(ciphertext_hiding_asids, "max")) {
>> +        ciphertext_hiding_asid_nr = min_sev_asid - 1;
>> +    } else {
>> +        pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
>> +            ciphertext_hiding_asids);
>> +        return false;
>> +    }
> 
> This code can be made much simpler if all the invalid
> cases were combined to emit a single pr_warn().
> 

There definitely has to be a different pr_warn() for the sanity check case and invalid parameter cases and sanity check has to be done if the
specified parameter is an unsigned int, so the check needs to be done separately.

I can definitely add a branch just for the invalid cases.

>> @@ -3036,7 +3090,9 @@ void __init sev_hardware_setup(void)
>>               min_sev_asid, max_sev_asid);
>>       if (boot_cpu_has(X86_FEATURE_SEV_ES))
>>           pr_info("SEV-ES %s (ASIDs %u - %u)\n",
>> -            str_enabled_disabled(sev_es_supported),
>> +            sev_es_supported ? min_sev_es_asid < min_sev_asid ? "enabled" :
>> +                                        "unusable" :
>> +                                        "disabled",
>>               min_sev_es_asid, max_sev_es_asid);
>>       if (boot_cpu_has(X86_FEATURE_SEV_SNP))
>>           pr_info("SEV-SNP %s (ASIDs %u - %u)\n",
> 
> If I set ciphertext_hiding_asids=99, I get the new 'unusable':
> 
> kvm_amd: SEV-SNP ciphertext hiding enabled
> ...
> kvm_amd: SEV enabled (ASIDs 100 - 1006)
> kvm_amd: SEV-ES unusable (ASIDs 100 - 99)
> kvm_amd: SEV-SNP enabled (ASIDs 1 - 99)
> 
> Ok.

Which is correct. 

This is similar to the SEV case where min_sev_asid can be greater than max_sev_asid and that also emits similarly : 
SEV unusable (ASIDs 1007 - 1006) (this is an example of that case).

> 
> Now, if I set ciphertext_hiding_asids=0, I get:
> 
> kvm_amd: SEV-SNP ciphertext hiding enabled
> ...
> kvm_amd: SEV enabled (ASIDs 100 - 1006)
> kvm_amd: SEV-ES enabled (ASIDs 1 - 99)
> kvm_amd: SEV-SNP enabled (ASIDs 1 - 0)
> 
> ..where SNP is unusable this time, yet it's not flagged as such.
> 

Actually SNP still needs to be usable/enabled in this case, as specifying ciphertext_hiding_asids=0 is same as specifying that ciphertext hiding feature should
not be enabled, so code-wise this is behaving correctly, but messaging needs to be fixed, which i will fix.

Thanks,
Ashish

> If there's no difference between "unusable" and not enabled, then
> I think it's better to keep the not enabled messaging behaviour
> and just not emit the line at all:  It's confusing to see the
> invalid "100 - 99" and "1 - 0" ranges.
> 
> Thanks,
> 
> Kim

Re: [PATCH v5 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support

[Kalra, Ashish]

On 7/2/2025 5:43 PM, Kalra, Ashish wrote:
> Hello Kim,
> 
> On 7/2/2025 4:46 PM, Kim Phillips wrote:
>> Hi Ashish,
>>
>> I can confirm that this v5 series fixes v4's __sev_do_cmd_locked
>> assertion failure problem, thanks.  More comments inline:
>>
>> On 7/1/25 3:16 PM, Ashish Kalra wrote:
>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> Extra From: line not necessary.
>>
>>> @@ -2913,10 +2921,46 @@ static bool is_sev_snp_initialized(void)
>>>       return initialized;
>>>   }
>>>   +static bool check_and_enable_sev_snp_ciphertext_hiding(void)
>>> +{
>>> +    unsigned int ciphertext_hiding_asid_nr = 0;
>>> +
>>> +    if (!sev_is_snp_ciphertext_hiding_supported()) {
>>> +        pr_warn("Module parameter ciphertext_hiding_asids specified but ciphertext hiding not supported or enabled\n");
>>> +        return false;
>>> +    }
>>> +
>>> +    if (isdigit(ciphertext_hiding_asids[0])) {
>>> +        if (kstrtoint(ciphertext_hiding_asids, 10, &ciphertext_hiding_asid_nr)) {
>>> +            pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
>>> +                ciphertext_hiding_asids);
>>> +            return false;
>>> +        }
>>> +        /* Do sanity checks on user-defined ciphertext_hiding_asids */
>>> +        if (ciphertext_hiding_asid_nr >= min_sev_asid) {
>>> +            pr_warn("Requested ciphertext hiding ASIDs (%u) exceeds or equals minimum SEV ASID (%u)\n",
>>> +                ciphertext_hiding_asid_nr, min_sev_asid);
>>> +            return false;
>>> +        }
>>> +    } else if (!strcmp(ciphertext_hiding_asids, "max")) {
>>> +        ciphertext_hiding_asid_nr = min_sev_asid - 1;
>>> +    } else {
>>> +        pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
>>> +            ciphertext_hiding_asids);
>>> +        return false;
>>> +    }
>>
>> This code can be made much simpler if all the invalid
>> cases were combined to emit a single pr_warn().
>>
> 
> There definitely has to be a different pr_warn() for the sanity check case and invalid parameter cases and sanity check has to be done if the
> specified parameter is an unsigned int, so the check needs to be done separately.
> 
> I can definitely add a branch just for the invalid cases.
> 
>>> @@ -3036,7 +3090,9 @@ void __init sev_hardware_setup(void)
>>>               min_sev_asid, max_sev_asid);
>>>       if (boot_cpu_has(X86_FEATURE_SEV_ES))
>>>           pr_info("SEV-ES %s (ASIDs %u - %u)\n",
>>> -            str_enabled_disabled(sev_es_supported),
>>> +            sev_es_supported ? min_sev_es_asid < min_sev_asid ? "enabled" :
>>> +                                        "unusable" :
>>> +                                        "disabled",
>>>               min_sev_es_asid, max_sev_es_asid);
>>>       if (boot_cpu_has(X86_FEATURE_SEV_SNP))
>>>           pr_info("SEV-SNP %s (ASIDs %u - %u)\n",
>>
>> If I set ciphertext_hiding_asids=99, I get the new 'unusable':
>>
>> kvm_amd: SEV-SNP ciphertext hiding enabled
>> ...
>> kvm_amd: SEV enabled (ASIDs 100 - 1006)
>> kvm_amd: SEV-ES unusable (ASIDs 100 - 99)
>> kvm_amd: SEV-SNP enabled (ASIDs 1 - 99)
>>
>> Ok.
> 
> Which is correct. 
> 
> This is similar to the SEV case where min_sev_asid can be greater than max_sev_asid and that also emits similarly : 
> SEV unusable (ASIDs 1007 - 1006) (this is an example of that case).
> 

Also do note that the message above is printing the exact values of min_sev_es_asid and max_sev_es_asid, as they have been computed.

And it adds that SEV-ES is now unusable as now min_sev_es_asid > max_sev_es_asid.

>>
>> Now, if I set ciphertext_hiding_asids=0, I get:
>>
>> kvm_amd: SEV-SNP ciphertext hiding enabled
>> ...
>> kvm_amd: SEV enabled (ASIDs 100 - 1006)
>> kvm_amd: SEV-ES enabled (ASIDs 1 - 99)
>> kvm_amd: SEV-SNP enabled (ASIDs 1 - 0)
>>
>> ..where SNP is unusable this time, yet it's not flagged as such.
>>
> 
> Actually SNP still needs to be usable/enabled in this case, as specifying ciphertext_hiding_asids=0 is same as specifying that ciphertext hiding feature should
> not be enabled, so code-wise this is behaving correctly, but messaging needs to be fixed, which i will fix.
> 

And i do need to fix this case for ciphertext_hiding_asids==0, i.e., ciphertext hiding feature is not enabled, as the above is not functioning correctly.

Thanks,
Ashish

> 
>> If there's no difference between "unusable" and not enabled, then
>> I think it's better to keep the not enabled messaging behaviour
>> and just not emit the line at all:  It's confusing to see the
>> invalid "100 - 99" and "1 - 0" ranges.
>>
>> Thanks,
>>
>> Kim
> 

Re: [PATCH v5 2/7] crypto: ccp - Cache SEV platform status and platform state

[Tom Lendacky]

On 7/1/25 15:15, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Cache the SEV platform status into sev_device structure and use this
> cached SEV platform status for api_major/minor/build.
> 
> The platform state is unique between SEV and SNP and hence needs to be
> tracked independently.
> 
> Remove the state field from sev_device structure and instead track SEV
> state from the cached SEV platform status.
> 
> Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  drivers/crypto/ccp/sev-dev.c | 22 ++++++++++++----------
>  drivers/crypto/ccp/sev-dev.h |  3 ++-
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 17edc6bf5622..5a2e1651d171 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1286,7 +1286,7 @@ static int __sev_platform_init_locked(int *error)
>  
>  	sev = psp_master->sev_data;
>  
> -	if (sev->state == SEV_STATE_INIT)
> +	if (sev->sev_plat_status.state == SEV_STATE_INIT)
>  		return 0;
>  
>  	__sev_platform_init_handle_tmr(sev);
> @@ -1318,7 +1318,7 @@ static int __sev_platform_init_locked(int *error)
>  		return rc;
>  	}
>  
> -	sev->state = SEV_STATE_INIT;
> +	sev->sev_plat_status.state = SEV_STATE_INIT;
>  
>  	/* Prepare for first SEV guest launch after INIT */
>  	wbinvd_on_all_cpus();
> @@ -1347,7 +1347,7 @@ static int _sev_platform_init_locked(struct sev_platform_init_args *args)
>  
>  	sev = psp_master->sev_data;
>  
> -	if (sev->state == SEV_STATE_INIT)
> +	if (sev->sev_plat_status.state == SEV_STATE_INIT)
>  		return 0;
>  
>  	rc = __sev_snp_init_locked(&args->error);
> @@ -1384,7 +1384,7 @@ static int __sev_platform_shutdown_locked(int *error)
>  
>  	sev = psp->sev_data;
>  
> -	if (sev->state == SEV_STATE_UNINIT)
> +	if (sev->sev_plat_status.state == SEV_STATE_UNINIT)
>  		return 0;
>  
>  	ret = __sev_do_cmd_locked(SEV_CMD_SHUTDOWN, NULL, error);
> @@ -1394,7 +1394,7 @@ static int __sev_platform_shutdown_locked(int *error)
>  		return ret;
>  	}
>  
> -	sev->state = SEV_STATE_UNINIT;
> +	sev->sev_plat_status.state = SEV_STATE_UNINIT;
>  	dev_dbg(sev->dev, "SEV firmware shutdown\n");
>  
>  	return ret;
> @@ -1502,7 +1502,7 @@ static int sev_ioctl_do_pek_pdh_gen(int cmd, struct sev_issue_cmd *argp, bool wr
>  	if (!writable)
>  		return -EPERM;
>  
> -	if (sev->state == SEV_STATE_UNINIT) {
> +	if (sev->sev_plat_status.state == SEV_STATE_UNINIT) {
>  		rc = sev_move_to_init_state(argp, &shutdown_required);
>  		if (rc)
>  			return rc;
> @@ -1551,7 +1551,7 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable)
>  	data.len = input.length;
>  
>  cmd:
> -	if (sev->state == SEV_STATE_UNINIT) {
> +	if (sev->sev_plat_status.state == SEV_STATE_UNINIT) {
>  		ret = sev_move_to_init_state(argp, &shutdown_required);
>  		if (ret)
>  			goto e_free_blob;
> @@ -1606,10 +1606,12 @@ static int sev_get_api_version(void)
>  		return 1;
>  	}
>  
> +	/* Cache SEV platform status */
> +	sev->sev_plat_status = status;
> +
>  	sev->api_major = status.api_major;
>  	sev->api_minor = status.api_minor;
>  	sev->build = status.build;
> -	sev->state = status.state;
>  
>  	return 0;
>  }
> @@ -1837,7 +1839,7 @@ static int sev_ioctl_do_pek_import(struct sev_issue_cmd *argp, bool writable)
>  	data.oca_cert_len = input.oca_cert_len;
>  
>  	/* If platform is not in INIT state then transition it to INIT */
> -	if (sev->state != SEV_STATE_INIT) {
> +	if (sev->sev_plat_status.state != SEV_STATE_INIT) {
>  		ret = sev_move_to_init_state(argp, &shutdown_required);
>  		if (ret)
>  			goto e_free_oca;
> @@ -2008,7 +2010,7 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable)
>  
>  cmd:
>  	/* If platform is not in INIT state then transition it to INIT. */
> -	if (sev->state != SEV_STATE_INIT) {
> +	if (sev->sev_plat_status.state != SEV_STATE_INIT) {
>  		if (!writable) {
>  			ret = -EPERM;
>  			goto e_free_cert;
> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
> index 3e4e5574e88a..24dd8ff8afaa 100644
> --- a/drivers/crypto/ccp/sev-dev.h
> +++ b/drivers/crypto/ccp/sev-dev.h
> @@ -42,7 +42,6 @@ struct sev_device {
>  
>  	struct sev_vdata *vdata;
>  
> -	int state;
>  	unsigned int int_rcvd;
>  	wait_queue_head_t int_queue;
>  	struct sev_misc_dev *misc;
> @@ -57,6 +56,8 @@ struct sev_device {
>  	bool cmd_buf_backup_active;
>  
>  	bool snp_initialized;
> +
> +	struct sev_user_data_status sev_plat_status;
>  };
>  
>  int sev_dev_init(struct psp_device *psp);

Re: [PATCH v5 3/7] crypto: ccp - Add support for SNP_FEATURE_INFO command

[Tom Lendacky]

On 7/1/25 15:15, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> The FEATURE_INFO command provides hypervisors a programmatic means to
> learn about the supported features of the currently loaded firmware.
> FEATURE_INFO command leverages the same mechanism as the CPUID

s/mechanism/output format/

> instruction. Instead of using the CPUID instruction to retrieve
> Fn8000_0024, software can use FEATURE_INFO.

This makes it sound like you have the option of which to use, but that
isn't the case. Fn8000_0024 can only be retrieved via the FEATURE_INFO
command.

> 
> Switch to using SNP platform status instead of SEV platform status if

For what and how? SEV platform status is still called afterward, so what
does the switch consist of?

> SNP is enabled and cache SNP platform status and feature information
> from CPUID 0x7fff_0024, sub-function 0, in the sev_device structure.

7fff_0024 ?

> 
> Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  drivers/crypto/ccp/sev-dev.c | 72 ++++++++++++++++++++++++++++++++++++
>  drivers/crypto/ccp/sev-dev.h |  3 ++
>  include/linux/psp-sev.h      | 29 +++++++++++++++
>  3 files changed, 104 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 5a2e1651d171..d1517a91a27d 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -233,6 +233,7 @@ static int sev_cmd_buffer_len(int cmd)
>  	case SEV_CMD_SNP_GUEST_REQUEST:		return sizeof(struct sev_data_snp_guest_request);
>  	case SEV_CMD_SNP_CONFIG:		return sizeof(struct sev_user_data_snp_config);
>  	case SEV_CMD_SNP_COMMIT:		return sizeof(struct sev_data_snp_commit);
> +	case SEV_CMD_SNP_FEATURE_INFO:		return sizeof(struct sev_data_snp_feature_info);
>  	default:				return 0;
>  	}
>  
> @@ -1073,6 +1074,67 @@ static void snp_set_hsave_pa(void *arg)
>  	wrmsrq(MSR_VM_HSAVE_PA, 0);
>  }
>  
> +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 not
> +	 * 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);
> +	if (rc) {
> +		dev_err(sev->dev, "SNP PLATFORM_STATUS command failed, ret = %d, error = %#x\n",
> +			rc, *error);
> +		return rc;
> +	}
> +
> +	sev->api_major = sev->snp_plat_status.api_major;
> +	sev->api_minor = sev->snp_plat_status.api_minor;
> +	sev->build = sev->snp_plat_status.build_id;
> +
> +	/*
> +	 * Do feature discovery of the currently loaded firmware,
> +	 * and cache feature information from CPUID 0x8000_0024,
> +	 * sub-function 0.
> +	 */
> +	if (!sev->snp_plat_status.feature_info)
> +		return 0;
> +
> +	/*
> +	 * Use dynamically allocated structure for the SNP_FEATURE_INFO
> +	 * command to ensure structure is 8-byte aligned, and does not
> +	 * cross a page boundary.
> +	 */
> +	page = alloc_page(GFP_KERNEL);
> +	if (!page)
> +		return -ENOMEM;
> +
> +	feat_info = page_address(page);
> +	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);
> +	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(page);
> +
> +	return rc;
> +}
> +
>  static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
>  {
>  	struct sev_data_range_list *range_list = arg;
> @@ -1599,6 +1661,16 @@ static int sev_get_api_version(void)
>  	struct sev_user_data_status status;
>  	int error = 0, ret;
>  
> +	/*
> +	 * Cache SNP platform status and SNP feature information
> +	 * if SNP is available.
> +	 */
> +	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP)) {
> +		ret = snp_get_platform_data(sev, &error);
> +		if (ret)
> +			return 1;
> +	}
> +
>  	ret = sev_platform_status(&status, &error);
>  	if (ret) {
>  		dev_err(sev->dev,
> diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
> index 24dd8ff8afaa..5aed2595c9ae 100644
> --- a/drivers/crypto/ccp/sev-dev.h
> +++ b/drivers/crypto/ccp/sev-dev.h
> @@ -58,6 +58,9 @@ struct sev_device {
>  	bool snp_initialized;
>  
>  	struct sev_user_data_status sev_plat_status;
> +
> +	struct sev_user_data_snp_status snp_plat_status;
> +	struct snp_feature_info snp_feat_info_0;
>  };
>  
>  int sev_dev_init(struct psp_device *psp);
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 0f5f94137f6d..935547c26985 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -107,6 +107,7 @@ enum sev_cmd {
>  	SEV_CMD_SNP_DOWNLOAD_FIRMWARE_EX = 0x0CA,
>  	SEV_CMD_SNP_COMMIT		= 0x0CB,
>  	SEV_CMD_SNP_VLEK_LOAD		= 0x0CD,
> +	SEV_CMD_SNP_FEATURE_INFO	= 0x0CE,
>  
>  	SEV_CMD_MAX,
>  };
> @@ -814,6 +815,34 @@ struct sev_data_snp_commit {
>  	u32 len;
>  } __packed;
>  
> +/**
> + * struct sev_data_snp_feature_info - SEV_SNP_FEATURE_INFO structure
> + *
> + * @length: len of the command buffer read by the PSP
> + * @ecx_in: subfunction index
> + * @feature_info_paddr : SPA of the FEATURE_INFO structure

s/SPA/system physical address/  to match use everywhere else in the file.

Thanks,
Tom

> + */
> +struct sev_data_snp_feature_info {
> +	u32 length;
> +	u32 ecx_in;
> +	u64 feature_info_paddr;
> +} __packed;
> +
> +/**
> + * struct feature_info - FEATURE_INFO structure
> + *
> + * @eax: output of SNP_FEATURE_INFO command
> + * @ebx: output of SNP_FEATURE_INFO command
> + * @ecx: output of SNP_FEATURE_INFO command
> + * #edx: output of SNP_FEATURE_INFO command
> + */
> +struct snp_feature_info {
> +	u32 eax;
> +	u32 ebx;
> +	u32 ecx;
> +	u32 edx;
> +} __packed;
> +
>  #ifdef CONFIG_CRYPTO_DEV_SP_PSP
>  
>  /**

Re: [PATCH v5 4/7] crypto: ccp - Introduce new API interface to indicate SEV-SNP Ciphertext hiding feature

[Tom Lendacky]

On 7/1/25 15:15, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Implement a new API interface that indicates both the support for the
> SEV-SNP Ciphertext Hiding feature by the SEV firmware and whether this
> feature is enabled in the platform BIOS.

The API is a single result about support, so how about something like:

  Implement an API that checks overall feature support for SEV-SNP
  ciphertext hiding.

  The API verifies both the SEV firmware's support for the feature and
  its enablement in the platform's BIOS.

Thanks,
Tom

> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  drivers/crypto/ccp/sev-dev.c | 21 +++++++++++++++++++++
>  include/linux/psp-sev.h      |  5 +++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index d1517a91a27d..3f2bbba93617 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1074,6 +1074,27 @@ static void snp_set_hsave_pa(void *arg)
>  	wrmsrq(MSR_VM_HSAVE_PA, 0);
>  }
>  
> +bool sev_is_snp_ciphertext_hiding_supported(void)
> +{
> +	struct psp_device *psp = psp_master;
> +	struct sev_device *sev;
> +
> +	if (!psp || !psp->sev_data)
> +		return false;
> +
> +	sev = psp->sev_data;
> +
> +	/*
> +	 * Feature information indicates if CipherTextHiding feature is
> +	 * supported by the SEV firmware and additionally platform status
> +	 * indicates if CipherTextHiding feature is enabled in the
> +	 * Platform BIOS.
> +	 */
> +	return ((sev->snp_feat_info_0.ecx & SNP_CIPHER_TEXT_HIDING_SUPPORTED) &&
> +		 sev->snp_plat_status.ciphertext_hiding_cap);
> +}
> +EXPORT_SYMBOL_GPL(sev_is_snp_ciphertext_hiding_supported);
> +
>  static int snp_get_platform_data(struct sev_device *sev, int *error)
>  {
>  	struct sev_data_snp_feature_info snp_feat_info;
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 935547c26985..ca19fddfcd4d 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -843,6 +843,8 @@ struct snp_feature_info {
>  	u32 edx;
>  } __packed;
>  
> +#define SNP_CIPHER_TEXT_HIDING_SUPPORTED	BIT(3)
> +
>  #ifdef CONFIG_CRYPTO_DEV_SP_PSP
>  
>  /**
> @@ -986,6 +988,7 @@ void *psp_copy_user_blob(u64 uaddr, u32 len);
>  void *snp_alloc_firmware_page(gfp_t mask);
>  void snp_free_firmware_page(void *addr);
>  void sev_platform_shutdown(void);
> +bool sev_is_snp_ciphertext_hiding_supported(void);
>  
>  #else	/* !CONFIG_CRYPTO_DEV_SP_PSP */
>  
> @@ -1022,6 +1025,8 @@ static inline void snp_free_firmware_page(void *addr) { }
>  
>  static inline void sev_platform_shutdown(void) { }
>  
> +static inline bool sev_is_snp_ciphertext_hiding_supported(void) { return false; }
> +
>  #endif	/* CONFIG_CRYPTO_DEV_SP_PSP */
>  
>  #endif	/* __PSP_SEV_H__ */

Re: [PATCH v5 5/7] crypto: ccp - Add support to enable CipherTextHiding on SNP_INIT_EX

[Tom Lendacky]

On 7/1/25 15:15, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Ciphertext hiding needs to be enabled on SNP_INIT_EX.

How about:

In order for ciphertext hiding to be enabled, it must be specified on
the SNP_INIT_EX command as part of SNP initialization.

> 
> Enhance sev_platform_init_args by adding a new argument that enables the
> KVM module to specify whether the Ciphertext Hiding feature should be
> activated during SNP initialization. Additionally, this argument will
> allow for the definition of the maximum ASID that can be used for an
> SEV-SNP guest when Ciphertext Hiding is enabled.

Modify the sev_platform_init_args structure used as input to
sev_platform_init(), to add a field that, when non-zero, indicates
ciphertext hiding should be enabled and indicates the maximum ASID that
can be used for an SEV-SNP guest.

I think that reads a bit cleaner and clearer.

> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  drivers/crypto/ccp/sev-dev.c | 12 +++++++++---
>  include/linux/psp-sev.h      | 10 ++++++++--
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 3f2bbba93617..c883ccf8c3ff 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -1186,7 +1186,7 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
>  	return 0;
>  }
>  
> -static int __sev_snp_init_locked(int *error)
> +static int __sev_snp_init_locked(int *error, unsigned int max_snp_asid)
>  {
>  	struct psp_device *psp = psp_master;
>  	struct sev_data_snp_init_ex data;
> @@ -1247,6 +1247,12 @@ static int __sev_snp_init_locked(int *error)
>  		}
>  
>  		memset(&data, 0, sizeof(data));
> +
> +		if (max_snp_asid) {
> +			data.ciphertext_hiding_en = 1;
> +			data.max_snp_asid = max_snp_asid;
> +		}
> +
>  		data.init_rmp = 1;
>  		data.list_paddr_en = 1;
>  		data.list_paddr = __psp_pa(snp_range_list);
> @@ -1433,7 +1439,7 @@ static int _sev_platform_init_locked(struct sev_platform_init_args *args)
>  	if (sev->sev_plat_status.state == SEV_STATE_INIT)
>  		return 0;
>  
> -	rc = __sev_snp_init_locked(&args->error);
> +	rc = __sev_snp_init_locked(&args->error, args->max_snp_asid);
>  	if (rc && rc != -ENODEV)
>  		return rc;
>  
> @@ -1516,7 +1522,7 @@ static int snp_move_to_init_state(struct sev_issue_cmd *argp, bool *shutdown_req
>  {
>  	int error, rc;
>  
> -	rc = __sev_snp_init_locked(&error);
> +	rc = __sev_snp_init_locked(&error, 0);
>  	if (rc) {
>  		argp->error = SEV_RET_INVALID_PLATFORM_STATE;
>  		return rc;
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index ca19fddfcd4d..b6eda9c560ee 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -748,10 +748,13 @@ struct sev_data_snp_guest_request {
>  struct sev_data_snp_init_ex {
>  	u32 init_rmp:1;
>  	u32 list_paddr_en:1;
> -	u32 rsvd:30;
> +	u32 rapl_dis:1;
> +	u32 ciphertext_hiding_en:1;
> +	u32 rsvd:28;
>  	u32 rsvd1;
>  	u64 list_paddr;
> -	u8  rsvd2[48];
> +	u16 max_snp_asid;
> +	u8  rsvd2[46];
>  } __packed;
>  
>  /**
> @@ -800,10 +803,13 @@ struct sev_data_snp_shutdown_ex {
>   * @probe: True if this is being called as part of CCP module probe, which
>   *  will defer SEV_INIT/SEV_INIT_EX firmware initialization until needed
>   *  unless psp_init_on_probe module param is set
> + * @max_snp_asid: maximum ASID usable for SEV-SNP guest if
> + *  CipherTextHiding feature is to be enabled

when non-zero, enable ciphertext hiding and specify the maximum ASID
that can be used for an SEV-SNP guest.

Thanks,
Tom

>   */
>  struct sev_platform_init_args {
>  	int error;
>  	bool probe;
> +	unsigned int max_snp_asid;
>  };
>  
>  /**

Re: [PATCH v5 6/7] KVM: SEV: Introduce new min,max sev_es and sev_snp asid variables

[Tom Lendacky]

On 7/1/25 15:16, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Introduce new min, max sev_es_asid and sev_snp_asid variables.
> 
> The new {min,max}_{sev_es,snp}_asid variables along with existing
> {min,max}_sev_asid variable simplifies partitioning of the
> SEV and SEV-ES+ ASID space.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  arch/x86/kvm/svm/sev.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 

Re: [PATCH v5 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support

[Tom Lendacky]

On 7/1/25 15:16, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Ciphertext hiding prevents host accesses from reading the ciphertext of
> SNP guest private memory. Instead of reading ciphertext, the host reads
> will see constant default values (0xff).
> 
> The SEV ASID space is basically split into legacy SEV and SEV-ES+.

s/basically//
s/legacy//

s|SEV-ES+.|SEV-ES/SEV-SNP ASID ranges.|

> Ciphertext hiding further partitions the SEV-ES+ ASID space into SEV-ES
> and SEV-SNP.

Enabling ciphertext hiding further splits the SEV-ES/SEV-SNP ASID space
into separate ASID ranges for SEV-ES and SEV-SNP guests.

> 
> Add new module parameter to the KVM module to enable Ciphertext hiding

s/Ciphertext/ciphertext/

> support and a user configurable system-wide maximum SNP ASID value. If
> the module parameter value is "max" then the complete SEV-ES+ ASID

s|SEV-ES+|SEV-ES/SEV-SNP|

> space is allocated to SEV-SNP guests.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 19 ++++++
>  arch/x86/kvm/svm/sev.c                        | 58 ++++++++++++++++++-
>  2 files changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index ee0735c6b8e2..05e50c37969e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2942,6 +2942,25 @@
>  			(enabled). Disable by KVM if hardware lacks support
>  			for NPT.
>  
> +	kvm-amd.ciphertext_hiding_asids=
> +			[KVM,AMD] Ciphertext hiding prevents host accesses from reading
> +			the ciphertext of SNP guest private memory. Instead of reading
> +			ciphertext, the host will see constant default values (0xff).
> +			The SEV ASID space is split into legacy SEV and joint

s/legacy//

> +			SEV-ES and SEV-SNP ASID space. Ciphertext hiding further
> +			partitions the joint SEV-ES/SEV-SNP ASID space into separate
> +			SEV-ES and SEV-SNP ASID ranges with the SEV-SNP ASID range
> +			starting at 1. For SEV-ES/SEV-SNP guests the maximum ASID
> +			available is MIN_SEV_ASID - 1 and MIN_SEV_ASID value is

s/and/where/

> +			discovered by CPUID Fn8000_001F[EDX].
> +
> +			Format: { <unsigned int> | "max" }
> +			A non-zero value enables SEV-SNP CipherTextHiding feature and sets

s/CipherTextHiding feature/ciphertext hiding/

> +			how many ASIDs are available for SEV-SNP guests.

the ASID range available for SEV-SNP guests.

> +			A Value of "max" assigns all ASIDs available in the joint SEV-ES
> +			and SEV-SNP ASID range to SNP guests and also effectively disables

s/and also effectively disables/, effectively disabling/

> +			SEV-ES.
> +
>  	kvm-arm.mode=
>  			[KVM,ARM,EARLY] Select one of KVM/arm64's modes of
>  			operation.
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 89ce9e298201..16723b8e0e37 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -59,6 +59,11 @@ static bool sev_es_debug_swap_enabled = true;
>  module_param_named(debug_swap, sev_es_debug_swap_enabled, bool, 0444);
>  static u64 sev_supported_vmsa_features;
>  
> +static char ciphertext_hiding_asids[16];
> +module_param_string(ciphertext_hiding_asids, ciphertext_hiding_asids,
> +		    sizeof(ciphertext_hiding_asids), 0444);
> +MODULE_PARM_DESC(ciphertext_hiding_asids, "  Enable ciphertext hiding for SEV-SNP guests and set the number of ASIDs to use ('max' to use all available SEV-SNP ASIDs");

This reads a bit awkward, but I don't really have a suggestion for a
short, concise message.

> +
>  #define AP_RESET_HOLD_NONE		0
>  #define AP_RESET_HOLD_NAE_EVENT		1
>  #define AP_RESET_HOLD_MSR_PROTO		2
> @@ -200,6 +205,9 @@ static int sev_asid_new(struct kvm_sev_info *sev, unsigned long vm_type)
>  	/*
>  	 * The min ASID can end up larger than the max if basic SEV support is
>  	 * effectively disabled by disallowing use of ASIDs for SEV guests.
> +	 * Similarly for SEV-ES guests the min ASID can end up larger than the
> +	 * max when ciphertext hiding is enabled, effectively disabling SEV-ES
> +	 * support.
>  	 */
>  	if (min_asid > max_asid)
>  		return -ENOTTY;
> @@ -2913,10 +2921,46 @@ static bool is_sev_snp_initialized(void)
>  	return initialized;
>  }
>  
> +static bool check_and_enable_sev_snp_ciphertext_hiding(void)
> +{
> +	unsigned int ciphertext_hiding_asid_nr = 0;
> +
> +	if (!sev_is_snp_ciphertext_hiding_supported()) {
> +		pr_warn("Module parameter ciphertext_hiding_asids specified but ciphertext hiding not supported or enabled\n");

s/or enabled//

> +		return false;
> +	}
> +
> +	if (isdigit(ciphertext_hiding_asids[0])) {
> +		if (kstrtoint(ciphertext_hiding_asids, 10, &ciphertext_hiding_asid_nr)) {
> +			pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
> +				ciphertext_hiding_asids);
> +			return false;
> +		}

Add a blank line.

> +		/* Do sanity checks on user-defined ciphertext_hiding_asids */

s/checks/check/

> +		if (ciphertext_hiding_asid_nr >= min_sev_asid) {
> +			pr_warn("Requested ciphertext hiding ASIDs (%u) exceeds or equals minimum SEV ASID (%u)\n",

Module parameter ciphertext_hiding_asids (%u) ...

> +				ciphertext_hiding_asid_nr, min_sev_asid);
> +			return false;
> +		}
> +	} else if (!strcmp(ciphertext_hiding_asids, "max")) {


> +		ciphertext_hiding_asid_nr = min_sev_asid - 1;
> +	} else {
> +		pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
> +			ciphertext_hiding_asids);
> +		return false;
> +	}
> +
> +	max_snp_asid = ciphertext_hiding_asid_nr;
> +	min_sev_es_asid = max_snp_asid + 1;
> +	pr_info("SEV-SNP ciphertext hiding enabled\n");

Add a blank line.

Thanks,
Tom

> +	return true;
> +}
> +
>  void __init sev_hardware_setup(void)
>  {
>  	unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count;
>  	struct sev_platform_init_args init_args = {0};
> +	bool snp_ciphertext_hiding_enabled = false;
>  	bool sev_snp_supported = false;
>  	bool sev_es_supported = false;
>  	bool sev_supported = false;
> @@ -3014,6 +3058,14 @@ void __init sev_hardware_setup(void)
>  	min_sev_es_asid = min_snp_asid = 1;
>  	max_sev_es_asid = max_snp_asid = min_sev_asid - 1;
>  
> +	/*
> +	 * The ciphertext hiding feature partitions the joint SEV-ES/SEV-SNP
> +	 * ASID range into separate SEV-ES and SEV-SNP ASID ranges with
> +	 * the SEV-SNP ASID starting at 1.
> +	 */
> +	if (ciphertext_hiding_asids[0])
> +		snp_ciphertext_hiding_enabled = check_and_enable_sev_snp_ciphertext_hiding();
> +
>  	sev_es_asid_count = min_sev_asid - 1;
>  	WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count));
>  	sev_es_supported = true;
> @@ -3022,6 +3074,8 @@ void __init sev_hardware_setup(void)
>  out:
>  	if (sev_enabled) {
>  		init_args.probe = true;
> +		if (snp_ciphertext_hiding_enabled)
> +			init_args.max_snp_asid = max_snp_asid;
>  		if (sev_platform_init(&init_args))
>  			sev_supported = sev_es_supported = sev_snp_supported = false;
>  		else if (sev_snp_supported)
> @@ -3036,7 +3090,9 @@ void __init sev_hardware_setup(void)
>  			min_sev_asid, max_sev_asid);
>  	if (boot_cpu_has(X86_FEATURE_SEV_ES))
>  		pr_info("SEV-ES %s (ASIDs %u - %u)\n",
> -			str_enabled_disabled(sev_es_supported),
> +			sev_es_supported ? min_sev_es_asid < min_sev_asid ? "enabled" :
> +									    "unusable" :
> +									    "disabled",
>  			min_sev_es_asid, max_sev_es_asid);
>  	if (boot_cpu_has(X86_FEATURE_SEV_SNP))
>  		pr_info("SEV-SNP %s (ASIDs %u - %u)\n",

Re: [PATCH v5 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support

[Kim Phillips]

On 7/7/25 1:16 AM, Kalra, Ashish wrote:
> 
> On 7/2/2025 5:43 PM, Kalra, Ashish wrote:
>> Hello Kim,
>>
>> On 7/2/2025 4:46 PM, Kim Phillips wrote:
>>> Hi Ashish,
>>>
>>> I can confirm that this v5 series fixes v4's __sev_do_cmd_locked
>>> assertion failure problem, thanks.  More comments inline:
>>>
>>> On 7/1/25 3:16 PM, Ashish Kalra wrote:
>>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>
>>> Extra From: line not necessary.
>>>
>>>> @@ -2913,10 +2921,46 @@ static bool is_sev_snp_initialized(void)
>>>>        return initialized;
>>>>    }
>>>>    +static bool check_and_enable_sev_snp_ciphertext_hiding(void)
>>>> +{
>>>> +    unsigned int ciphertext_hiding_asid_nr = 0;
>>>> +
>>>> +    if (!sev_is_snp_ciphertext_hiding_supported()) {
>>>> +        pr_warn("Module parameter ciphertext_hiding_asids specified but ciphertext hiding not supported or enabled\n");
>>>> +        return false;
>>>> +    }
>>>> +
>>>> +    if (isdigit(ciphertext_hiding_asids[0])) {
>>>> +        if (kstrtoint(ciphertext_hiding_asids, 10, &ciphertext_hiding_asid_nr)) {
>>>> +            pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
>>>> +                ciphertext_hiding_asids);
>>>> +            return false;
>>>> +        }
>>>> +        /* Do sanity checks on user-defined ciphertext_hiding_asids */
>>>> +        if (ciphertext_hiding_asid_nr >= min_sev_asid) {
>>>> +            pr_warn("Requested ciphertext hiding ASIDs (%u) exceeds or equals minimum SEV ASID (%u)\n",
>>>> +                ciphertext_hiding_asid_nr, min_sev_asid);
>>>> +            return false;
>>>> +        }
>>>> +    } else if (!strcmp(ciphertext_hiding_asids, "max")) {
>>>> +        ciphertext_hiding_asid_nr = min_sev_asid - 1;
>>>> +    } else {
>>>> +        pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n",
>>>> +            ciphertext_hiding_asids);
>>>> +        return false;
>>>> +    }
>>>
>>> This code can be made much simpler if all the invalid
>>> cases were combined to emit a single pr_warn().
>>>
>>
>> There definitely has to be a different pr_warn() for the sanity check case and invalid parameter cases and sanity check has to be done if the
>> specified parameter is an unsigned int, so the check needs to be done separately.
>>
>> I can definitely add a branch just for the invalid cases.
>>
>>>> @@ -3036,7 +3090,9 @@ void __init sev_hardware_setup(void)
>>>>                min_sev_asid, max_sev_asid);
>>>>        if (boot_cpu_has(X86_FEATURE_SEV_ES))
>>>>            pr_info("SEV-ES %s (ASIDs %u - %u)\n",
>>>> -            str_enabled_disabled(sev_es_supported),
>>>> +            sev_es_supported ? min_sev_es_asid < min_sev_asid ? "enabled" :
>>>> +                                        "unusable" :
>>>> +                                        "disabled",
>>>>                min_sev_es_asid, max_sev_es_asid);
>>>>        if (boot_cpu_has(X86_FEATURE_SEV_SNP))
>>>>            pr_info("SEV-SNP %s (ASIDs %u - %u)\n",
>>>
>>> If I set ciphertext_hiding_asids=99, I get the new 'unusable':
>>>
>>> kvm_amd: SEV-SNP ciphertext hiding enabled
>>> ...
>>> kvm_amd: SEV enabled (ASIDs 100 - 1006)
>>> kvm_amd: SEV-ES unusable (ASIDs 100 - 99)
>>> kvm_amd: SEV-SNP enabled (ASIDs 1 - 99)
>>>
>>> Ok.
>>
>> Which is correct.
>>
>> This is similar to the SEV case where min_sev_asid can be greater than max_sev_asid and that also emits similarly :
>> SEV unusable (ASIDs 1007 - 1006) (this is an example of that case).
>>
> 
> Also do note that the message above is printing the exact values of min_sev_es_asid and max_sev_es_asid, as they have been computed.
> 
> And it adds that SEV-ES is now unusable as now min_sev_es_asid > max_sev_es_asid.

Right, it'd be nice if that were made clearer to the user, too:

min_sev_es_asid 100 > max_sev_es_asid 99

>>> Now, if I set ciphertext_hiding_asids=0, I get:
>>>
>>> kvm_amd: SEV-SNP ciphertext hiding enabled
>>> ...
>>> kvm_amd: SEV enabled (ASIDs 100 - 1006)
>>> kvm_amd: SEV-ES enabled (ASIDs 1 - 99)
>>> kvm_amd: SEV-SNP enabled (ASIDs 1 - 0)
>>>
>>> ..where SNP is unusable this time, yet it's not flagged as such.
>>>
>>
>> Actually SNP still needs to be usable/enabled in this case, as specifying ciphertext_hiding_asids=0 is same as specifying that ciphertext hiding feature should
>> not be enabled, so code-wise this is behaving correctly, but messaging needs to be fixed, which i will fix.
>>
> 
> And i do need to fix this case for ciphertext_hiding_asids==0, i.e., ciphertext hiding feature is not enabled, as the above is not functioning correctly.

Right, it's not just messaging, SNP should still be enabled when ciphertext_hiding_asids==0,
whereas this is not the case with this patchset.


> 
> Thanks,
> Ashish
> 
>>
>>> If there's no difference between "unusable" and not enabled, then
>>> I think it's better to keep the not enabled messaging behaviour
>>> and just not emit the line at all:  It's confusing to see the
>>> invalid "100 - 99" and "1 - 0" ranges.

Please also consider this.

Thanks,

Kim