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@xxxxxxx>
> 
> 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@xxxxxxxxxx>
> Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx>
> ---
>  .../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",