[v4,5/5] KVM: SEV: Add SEV-SNP CipherTextHiding support

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+.
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 -1 then the ASID space is equally
divided between SEV-SNP and SEV-ES guests.

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

On 5/19/25 17:02, Ashish Kalra wrote:
> +	kvm-amd.ciphertext_hiding_nr_asids=
> +			[KVM,AMD] Enables SEV-SNP CipherTextHiding feature and
> +			controls show many ASIDs are available for SEV-SNP guests.
> +			The ASID space is basically split into legacy SEV and
> +			SEV-ES+. CipherTextHiding feature further splits the
> +			SEV-ES+ ASID space into SEV-ES and SEV-SNP.
> +			If the value is -1, then it is used as an auto flag
> +			and splits the ASID space equally between SEV-ES and
> +			SEV-SNP ASIDs.

This help text isn't great.

It doesn't come out and say what the connection between CipherTextHiding
and SEV-ES+. It's also impossible to choose a good number without
knowing how large the ASID space is in the first place.

Why use "-1"? Why not just take "auto" as a parameter?

It also needs to say what CipherTextHiding is in the first place and be
more clear about what the tradeoffs are from enabling this.

On 5/23/2025 12:29 PM, Dave Hansen wrote:
> On 5/19/25 17:02, Ashish Kalra wrote:
>> +	kvm-amd.ciphertext_hiding_nr_asids=
>> +			[KVM,AMD] Enables SEV-SNP CipherTextHiding feature and
>> +			controls show many ASIDs are available for SEV-SNP guests.
>> +			The ASID space is basically split into legacy SEV and
>> +			SEV-ES+. CipherTextHiding feature further splits the
>> +			SEV-ES+ ASID space into SEV-ES and SEV-SNP.
>> +			If the value is -1, then it is used as an auto flag
>> +			and splits the ASID space equally between SEV-ES and
>> +			SEV-SNP ASIDs.
> 
> This help text isn't great.
> 
> It doesn't come out and say what the connection between CipherTextHiding
> and SEV-ES+. It's also impossible to choose a good number without
> knowing how large the ASID space is in the first place.
> 
> Why use "-1"? Why not just take "auto" as a parameter?
> 
> It also needs to say what CipherTextHiding is in the first place and be
> more clear about what the tradeoffs are from enabling this.
> 
> 

Sure, i will work on improving the documentation.

Sean, looking forward to your review and feedback on these patch-series, especially the KVM side of patches.

Thanks,
Ashish

On 5/19/25 19:02, 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+.
> CipherTextHiding further partitions the SEV-ES+ ASID space into SEV-ES

s/CipherTextHiding/Ciphertext hiding/

> and SEV-SNP.
> 
> Add new module parameter to the KVM module to enable CipherTextHiding

Ditto.

> support and a user configurable system-wide maximum SNP ASID value. If
> the module parameter value is -1 then the ASID space is equally
> divided between SEV-SNP and SEV-ES guests.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 10 ++++++
>  arch/x86/kvm/svm/sev.c                        | 31 +++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1e5e76bba9da..2cddb2b5c59d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2891,6 +2891,16 @@
>  			(enabled). Disable by KVM if hardware lacks support
>  			for NPT.
>  
> +	kvm-amd.ciphertext_hiding_nr_asids=

s/ns_asids/asids/

I'm not sure that the "nr" adds anything here.

> +			[KVM,AMD] Enables SEV-SNP CipherTextHiding feature and
> +			controls show many ASIDs are available for SEV-SNP guests.
> +			The ASID space is basically split into legacy SEV and
> +			SEV-ES+. CipherTextHiding feature further splits the
> +			SEV-ES+ ASID space into SEV-ES and SEV-SNP.
> +			If the value is -1, then it is used as an auto flag
> +			and splits the ASID space equally between SEV-ES and
> +			SEV-SNP ASIDs.
> +

Ditto on Dave's comments.

>  	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 383db1da8699..68dcb13d98f2 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -59,6 +59,10 @@ 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 int ciphertext_hiding_nr_asids;
> +module_param(ciphertext_hiding_nr_asids, int, 0444);
> +MODULE_PARM_DESC(max_snp_asid, "  Number of ASIDs available for SEV-SNP guests when CipherTextHiding is enabled");
> +
>  #define AP_RESET_HOLD_NONE		0
>  #define AP_RESET_HOLD_NAE_EVENT		1
>  #define AP_RESET_HOLD_MSR_PROTO		2
> @@ -200,6 +204,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 CipherTextHiding is enabled, effectively disabling SEV-ES
> +	 * support.
>  	 */
>  
>  	if (min_asid > max_asid)
> @@ -2955,6 +2962,7 @@ 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_cipher_text_hiding = false;
>  	bool sev_snp_supported = false;
>  	bool sev_es_supported = false;
>  	bool sev_supported = false;
> @@ -3052,6 +3060,27 @@ void __init sev_hardware_setup(void)
>  	if (min_sev_asid == 1)
>  		goto out;
>  
> +	/*
> +	 * The ASID space is basically split into legacy SEV and SEV-ES+.
> +	 * CipherTextHiding feature further partitions the SEV-ES+ ASID space
> +	 * into ASIDs for SEV-ES and SEV-SNP guests.

I think it is already understood that the ASID space is split between SEV
and SEV-ES/SEV-SNP guests. So something like this maybe?

The ciphertext hiding feature partions the joint SEV-ES/SEV-SNP ASID range
into separate SEV-ES and SEV-SNP ASID ranges with teh SEV-SNP ASID range
starting at 1.

> +	 */
> +	if (ciphertext_hiding_nr_asids && sev_is_snp_ciphertext_hiding_supported()) {
> +		/* Do sanity checks on user-defined ciphertext_hiding_nr_asids */
> +		if (ciphertext_hiding_nr_asids != -1 &&
> +		    ciphertext_hiding_nr_asids >= min_sev_asid) {
> +			pr_info("ciphertext_hiding_nr_asids module parameter invalid, limiting SEV-SNP ASIDs to %d\n",
> +				 min_sev_asid);
> +			ciphertext_hiding_nr_asids = min_sev_asid - 1;

So specifying a number greater than min_sev_asid will result in enabling
ciphertext hiding and no SEV-ES guests allowed even though you report that
the number is invalid?

I think the message can be worded better to convey what happens.

"Requested ciphertext hiding ASIDs (%u) exceeds minimum SEV ASID (%u), setting ciphertext hiding ASID range to the maximum value (%u)\n"

Or something a little more concise.

> +		}
> +
> +		min_sev_es_asid = ciphertext_hiding_nr_asids == -1 ? (min_sev_asid - 1) / 2 :

Should this be (min_sev_asid - 1) / 2 + 1 ?

Take min_sev_asid = 3, that means min_sev_es_asid would be 2 and
max_snp_asid would be 1, right?

And if you set min_sev_es_asid before first you favor SEV-ES.

So should you do:

max_snp_asid = ciphertext_hiding_asids != -1 ? : (min_sev_asid - 1) / 2 + 1;
min_sev_es_asid = max_snp_asid + 1;


> +				  ciphertext_hiding_nr_asids + 1;
> +		max_snp_asid = min_sev_es_asid - 1;
> +		snp_cipher_text_hiding = true;
> +		pr_info("SEV-SNP CipherTextHiding feature support enabled\n");

"SEV-SNP ciphertext hiding enabled\n"

No need to use the CipherTextHiding nomenclature everywhere.

Thanks,
Tom

> +	}
> +
>  	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;
> @@ -3092,6 +3121,8 @@ void __init sev_hardware_setup(void)
>  	 * Do both SNP and SEV initialization at KVM module load.
>  	 */
>  	init_args.probe = true;
> +	if (snp_cipher_text_hiding)
> +		init_args.snp_max_snp_asid = max_snp_asid;
>  	sev_platform_init(&init_args);
>  }
>

Hello Tom,

On 6/3/2025 11:26 AM, Tom Lendacky wrote:
> On 5/19/25 19:02, 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+.
>> CipherTextHiding further partitions the SEV-ES+ ASID space into SEV-ES
> 
> s/CipherTextHiding/Ciphertext hiding/
> 
>> and SEV-SNP.
>>
>> Add new module parameter to the KVM module to enable CipherTextHiding
> 
> Ditto.

Ok.

> 
>> support and a user configurable system-wide maximum SNP ASID value. If
>> the module parameter value is -1 then the ASID space is equally
>> divided between SEV-SNP and SEV-ES guests.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>>  .../admin-guide/kernel-parameters.txt         | 10 ++++++
>>  arch/x86/kvm/svm/sev.c                        | 31 +++++++++++++++++++
>>  2 files changed, 41 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 1e5e76bba9da..2cddb2b5c59d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2891,6 +2891,16 @@
>>  			(enabled). Disable by KVM if hardware lacks support
>>  			for NPT.
>>  
>> +	kvm-amd.ciphertext_hiding_nr_asids=
> 
> s/ns_asids/asids/
> 
> I'm not sure that the "nr" adds anything here.
>

Ok.
 
>> +			[KVM,AMD] Enables SEV-SNP CipherTextHiding feature and
>> +			controls show many ASIDs are available for SEV-SNP guests.
>> +			The ASID space is basically split into legacy SEV and
>> +			SEV-ES+. CipherTextHiding feature further splits the
>> +			SEV-ES+ ASID space into SEV-ES and SEV-SNP.
>> +			If the value is -1, then it is used as an auto flag
>> +			and splits the ASID space equally between SEV-ES and
>> +			SEV-SNP ASIDs.
>> +
> 
> Ditto on Dave's comments.
>

Ok.
 
>>  	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 383db1da8699..68dcb13d98f2 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -59,6 +59,10 @@ 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 int ciphertext_hiding_nr_asids;
>> +module_param(ciphertext_hiding_nr_asids, int, 0444);
>> +MODULE_PARM_DESC(max_snp_asid, "  Number of ASIDs available for SEV-SNP guests when CipherTextHiding is enabled");
>> +
>>  #define AP_RESET_HOLD_NONE		0
>>  #define AP_RESET_HOLD_NAE_EVENT		1
>>  #define AP_RESET_HOLD_MSR_PROTO		2
>> @@ -200,6 +204,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 CipherTextHiding is enabled, effectively disabling SEV-ES
>> +	 * support.
>>  	 */
>>  
>>  	if (min_asid > max_asid)
>> @@ -2955,6 +2962,7 @@ 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_cipher_text_hiding = false;
>>  	bool sev_snp_supported = false;
>>  	bool sev_es_supported = false;
>>  	bool sev_supported = false;
>> @@ -3052,6 +3060,27 @@ void __init sev_hardware_setup(void)
>>  	if (min_sev_asid == 1)
>>  		goto out;
>>  
>> +	/*
>> +	 * The ASID space is basically split into legacy SEV and SEV-ES+.
>> +	 * CipherTextHiding feature further partitions the SEV-ES+ ASID space
>> +	 * into ASIDs for SEV-ES and SEV-SNP guests.
> 
> I think it is already understood that the ASID space is split between SEV
> and SEV-ES/SEV-SNP guests. So something like this maybe?
> 
> The ciphertext hiding feature partions the joint SEV-ES/SEV-SNP ASID range
> into separate SEV-ES and SEV-SNP ASID ranges with teh SEV-SNP ASID range
> starting at 1.
> 

Yes that sounds better.

>> +	 */
>> +	if (ciphertext_hiding_nr_asids && sev_is_snp_ciphertext_hiding_supported()) {
>> +		/* Do sanity checks on user-defined ciphertext_hiding_nr_asids */
>> +		if (ciphertext_hiding_nr_asids != -1 &&
>> +		    ciphertext_hiding_nr_asids >= min_sev_asid) {
>> +			pr_info("ciphertext_hiding_nr_asids module parameter invalid, limiting SEV-SNP ASIDs to %d\n",
>> +				 min_sev_asid);
>> +			ciphertext_hiding_nr_asids = min_sev_asid - 1;
> 
> So specifying a number greater than min_sev_asid will result in enabling
> ciphertext hiding and no SEV-ES guests allowed even though you report that
> the number is invalid?
>

Well, the user specified a non-zero ciphertext_hiding_nr_asids, so the intent is to enable ciphertext hiding and therefore 
sanitize the user specified parameter and enable ciphertext hiding.
 
> I think the message can be worded better to convey what happens.
> 
> "Requested ciphertext hiding ASIDs (%u) exceeds minimum SEV ASID (%u), setting ciphertext hiding ASID range to the maximum value (%u)\n"
> 
> Or something a little more concise.
> 

Ok.

>> +		}
>> +
>> +		min_sev_es_asid = ciphertext_hiding_nr_asids == -1 ? (min_sev_asid - 1) / 2 :
> 
> Should this be (min_sev_asid - 1) / 2 + 1 ?
> 
> Take min_sev_asid = 3, that means min_sev_es_asid would be 2 and
> max_snp_asid would be 1, right?

Yes.

> 
> And if you set min_sev_es_asid before first you favor SEV-ES.
>

Ok.
 
> So should you do:
> 
> max_snp_asid = ciphertext_hiding_asids != -1 ? : (min_sev_asid - 1) / 2 + 1;
> min_sev_es_asid = max_snp_asid + 1;
>

Considering the above example again, this will still be incorrect as with above change:

Taking, min_sev_asid == 3, max_snp_asid = 2 and min_sev_asid = 3.

So i believe it should be :

max_snp_asid = ciphertext_hiding_asids != -1 ? : (min_sev_asid - 1) / 2;
min_sev_es_asid = max_snp_asid + 1;

 
> 
>> +				  ciphertext_hiding_nr_asids + 1;
>> +		max_snp_asid = min_sev_es_asid - 1;
>> +		snp_cipher_text_hiding = true;
>> +		pr_info("SEV-SNP CipherTextHiding feature support enabled\n");
> 
> "SEV-SNP ciphertext hiding enabled\n"
> 
> No need to use the CipherTextHiding nomenclature everywhere.
> 

Ok.

Thanks,
Ashish

> Thanks,
> Tom
> 
>> +	}
>> +
>>  	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;
>> @@ -3092,6 +3121,8 @@ void __init sev_hardware_setup(void)
>>  	 * Do both SNP and SEV initialization at KVM module load.
>>  	 */
>>  	init_args.probe = true;
>> +	if (snp_cipher_text_hiding)
>> +		init_args.snp_max_snp_asid = max_snp_asid;
>>  	sev_platform_init(&init_args);
>>  }
>>

On 20/5/25 10:02, 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+.
> 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 -1 then the ASID space is equally
> divided between SEV-SNP and SEV-ES guests.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>   .../admin-guide/kernel-parameters.txt         | 10 ++++++
>   arch/x86/kvm/svm/sev.c                        | 31 +++++++++++++++++++
>   2 files changed, 41 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1e5e76bba9da..2cddb2b5c59d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2891,6 +2891,16 @@
>   			(enabled). Disable by KVM if hardware lacks support
>   			for NPT.
>   
> +	kvm-amd.ciphertext_hiding_nr_asids=
> +			[KVM,AMD] Enables SEV-SNP CipherTextHiding feature and
> +			controls show many ASIDs are available for SEV-SNP guests.
> +			The ASID space is basically split into legacy SEV and
> +			SEV-ES+. CipherTextHiding feature further splits the
> +			SEV-ES+ ASID space into SEV-ES and SEV-SNP.
> +			If the value is -1, then it is used as an auto flag
> +			and splits the ASID space equally between SEV-ES and
> +			SEV-SNP ASIDs.


Why in halves? 0 or max would make sense and I'd think the user wants all SEV-ES+ VMs be hidden by default so I'd name the parameter as no_hiding_nr_asids and make the default value of zero mean "every SEV-ES+ is hidden".

Or there is a downside of hiding all VMs?


> +
>   	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 383db1da8699..68dcb13d98f2 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -59,6 +59,10 @@ 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 int ciphertext_hiding_nr_asids;
> +module_param(ciphertext_hiding_nr_asids, int, 0444);
> +MODULE_PARM_DESC(max_snp_asid, "  Number of ASIDs available for SEV-SNP guests when CipherTextHiding is enabled");
> +
>   #define AP_RESET_HOLD_NONE		0
>   #define AP_RESET_HOLD_NAE_EVENT		1
>   #define AP_RESET_HOLD_MSR_PROTO		2
> @@ -200,6 +204,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 CipherTextHiding is enabled, effectively disabling SEV-ES
> +	 * support.
>   	 */
>   
>   	if (min_asid > max_asid)
> @@ -2955,6 +2962,7 @@ 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_cipher_text_hiding = false;
>   	bool sev_snp_supported = false;
>   	bool sev_es_supported = false;
>   	bool sev_supported = false;
> @@ -3052,6 +3060,27 @@ void __init sev_hardware_setup(void)
>   	if (min_sev_asid == 1)
>   		goto out;
>   
> +	/*
> +	 * The ASID space is basically split into legacy SEV and SEV-ES+.
> +	 * CipherTextHiding feature further partitions the SEV-ES+ ASID space
> +	 * into ASIDs for SEV-ES and SEV-SNP guests.
> +	 */
> +	if (ciphertext_hiding_nr_asids && sev_is_snp_ciphertext_hiding_supported()) {
> +		/* Do sanity checks on user-defined ciphertext_hiding_nr_asids */
> +		if (ciphertext_hiding_nr_asids != -1 &&
> +		    ciphertext_hiding_nr_asids >= min_sev_asid) {
> +			pr_info("ciphertext_hiding_nr_asids module parameter invalid, limiting SEV-SNP ASIDs to %d\n",
> +				 min_sev_asid);
> +			ciphertext_hiding_nr_asids = min_sev_asid - 1;
> +		}
> +
> +		min_sev_es_asid = ciphertext_hiding_nr_asids == -1 ? (min_sev_asid - 1) / 2 :
> +				  ciphertext_hiding_nr_asids + 1;
> +		max_snp_asid = min_sev_es_asid - 1;
> +		snp_cipher_text_hiding = true;
> +		pr_info("SEV-SNP CipherTextHiding feature support enabled\n");


Can do "init_args.snp_max_snp_asid = max_snp_asid;" here (as max_snp_asid seems to not change between here and next hunk) and drop snp_cipher_text_hiding. Thanks,

> +	}
> +
>   	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;
> @@ -3092,6 +3121,8 @@ void __init sev_hardware_setup(void)
>   	 * Do both SNP and SEV initialization at KVM module load.
>   	 */
>   	init_args.probe = true;
> +	if (snp_cipher_text_hiding)
> +		init_args.snp_max_snp_asid = max_snp_asid;
>   	sev_platform_init(&init_args);
>   }
>

On 6/4/25 19:17, Kalra, Ashish wrote:
> Hello Tom,
> 
> On 6/3/2025 11:26 AM, Tom Lendacky wrote:
>> On 5/19/25 19:02, Ashish Kalra wrote:
>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>

>>
>> The ciphertext hiding feature partions the joint SEV-ES/SEV-SNP ASID range
>> into separate SEV-ES and SEV-SNP ASID ranges with teh SEV-SNP ASID range
>> starting at 1.
>>
> 
> Yes that sounds better.

Just fix my spelling errors :)

> 
>>> +	 */
>>> +	if (ciphertext_hiding_nr_asids && sev_is_snp_ciphertext_hiding_supported()) {
>>> +		/* Do sanity checks on user-defined ciphertext_hiding_nr_asids */
>>> +		if (ciphertext_hiding_nr_asids != -1 &&
>>> +		    ciphertext_hiding_nr_asids >= min_sev_asid) {
>>> +			pr_info("ciphertext_hiding_nr_asids module parameter invalid, limiting SEV-SNP ASIDs to %d\n",
>>> +				 min_sev_asid);
>>> +			ciphertext_hiding_nr_asids = min_sev_asid - 1;
>>
>> So specifying a number greater than min_sev_asid will result in enabling
>> ciphertext hiding and no SEV-ES guests allowed even though you report that
>> the number is invalid?
>>
> 
> Well, the user specified a non-zero ciphertext_hiding_nr_asids, so the intent is to enable ciphertext hiding and therefore 
> sanitize the user specified parameter and enable ciphertext hiding.

Should you support something like ciphertext_hiding_asids=max (similar to
the 'auto' comment that Dave had) and report an error if a value is
specified that exceeds min_sev_asid and not enable ciphertext hiding?

Thanks,
Tom

>

On 6/5/2025 1:32 AM, Alexey Kardashevskiy wrote:
> On 20/5/25 10:02, 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+.
>> 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 -1 then the ASID space is equally
>> divided between SEV-SNP and SEV-ES guests.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>>   .../admin-guide/kernel-parameters.txt         | 10 ++++++
>>   arch/x86/kvm/svm/sev.c                        | 31 +++++++++++++++++++
>>   2 files changed, 41 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 1e5e76bba9da..2cddb2b5c59d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2891,6 +2891,16 @@
>>               (enabled). Disable by KVM if hardware lacks support
>>               for NPT.
>>   +    kvm-amd.ciphertext_hiding_nr_asids=
>> +            [KVM,AMD] Enables SEV-SNP CipherTextHiding feature and
>> +            controls show many ASIDs are available for SEV-SNP guests.
>> +            The ASID space is basically split into legacy SEV and
>> +            SEV-ES+. CipherTextHiding feature further splits the
>> +            SEV-ES+ ASID space into SEV-ES and SEV-SNP.
>> +            If the value is -1, then it is used as an auto flag
>> +            and splits the ASID space equally between SEV-ES and
>> +            SEV-SNP ASIDs.
> 
> 
> Why in halves? 0 or max would make sense and I'd think the user wants all SEV-ES+ VMs be hidden by default so I'd name the parameter as no_hiding_nr_asids and make the default value of zero mean "every SEV-ES+ is hidden".
>

Actually 'max' sounds nice, but we still want to name it ciphertext_hiding_asids or something and keep the value relative to it, so probably it can be 
a number or 'max'.

Thanks,
Ashish
 
> Or there is a downside of hiding all VMs?
> 
> 
>> +
>>       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 383db1da8699..68dcb13d98f2 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -59,6 +59,10 @@ 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 int ciphertext_hiding_nr_asids;
>> +module_param(ciphertext_hiding_nr_asids, int, 0444);
>> +MODULE_PARM_DESC(max_snp_asid, "  Number of ASIDs available for SEV-SNP guests when CipherTextHiding is enabled");
>> +
>>   #define AP_RESET_HOLD_NONE        0
>>   #define AP_RESET_HOLD_NAE_EVENT        1
>>   #define AP_RESET_HOLD_MSR_PROTO        2
>> @@ -200,6 +204,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 CipherTextHiding is enabled, effectively disabling SEV-ES
>> +     * support.
>>        */
>>         if (min_asid > max_asid)
>> @@ -2955,6 +2962,7 @@ 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_cipher_text_hiding = false;
>>       bool sev_snp_supported = false;
>>       bool sev_es_supported = false;
>>       bool sev_supported = false;
>> @@ -3052,6 +3060,27 @@ void __init sev_hardware_setup(void)
>>       if (min_sev_asid == 1)
>>           goto out;
>>   +    /*
>> +     * The ASID space is basically split into legacy SEV and SEV-ES+.
>> +     * CipherTextHiding feature further partitions the SEV-ES+ ASID space
>> +     * into ASIDs for SEV-ES and SEV-SNP guests.
>> +     */
>> +    if (ciphertext_hiding_nr_asids && sev_is_snp_ciphertext_hiding_supported()) {
>> +        /* Do sanity checks on user-defined ciphertext_hiding_nr_asids */
>> +        if (ciphertext_hiding_nr_asids != -1 &&
>> +            ciphertext_hiding_nr_asids >= min_sev_asid) {
>> +            pr_info("ciphertext_hiding_nr_asids module parameter invalid, limiting SEV-SNP ASIDs to %d\n",
>> +                 min_sev_asid);
>> +            ciphertext_hiding_nr_asids = min_sev_asid - 1;
>> +        }
>> +
>> +        min_sev_es_asid = ciphertext_hiding_nr_asids == -1 ? (min_sev_asid - 1) / 2 :
>> +                  ciphertext_hiding_nr_asids + 1;
>> +        max_snp_asid = min_sev_es_asid - 1;
>> +        snp_cipher_text_hiding = true;
>> +        pr_info("SEV-SNP CipherTextHiding feature support enabled\n");
> 
> 
> Can do "init_args.snp_max_snp_asid = max_snp_asid;" here (as max_snp_asid seems to not change between here and next hunk) and drop snp_cipher_text_hiding. Thanks,
> 
>> +    }
>> +
>>       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;
>> @@ -3092,6 +3121,8 @@ void __init sev_hardware_setup(void)
>>        * Do both SNP and SEV initialization at KVM module load.
>>        */
>>       init_args.probe = true;
>> +    if (snp_cipher_text_hiding)
>> +        init_args.snp_max_snp_asid = max_snp_asid;
>>       sev_platform_init(&init_args);
>>   }
>>   
>

On 6/5/2025 11:23 AM, Tom Lendacky wrote:
> On 6/4/25 19:17, Kalra, Ashish wrote:
>> Hello Tom,
>>
>> On 6/3/2025 11:26 AM, Tom Lendacky wrote:
>>> On 5/19/25 19:02, Ashish Kalra wrote:
>>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>>
> 
>>>
>>> The ciphertext hiding feature partions the joint SEV-ES/SEV-SNP ASID range
>>> into separate SEV-ES and SEV-SNP ASID ranges with teh SEV-SNP ASID range
>>> starting at 1.
>>>
>>
>> Yes that sounds better.
> 
> Just fix my spelling errors :)
>

Sure.
 
>>
>>>> +	 */
>>>> +	if (ciphertext_hiding_nr_asids && sev_is_snp_ciphertext_hiding_supported()) {
>>>> +		/* Do sanity checks on user-defined ciphertext_hiding_nr_asids */
>>>> +		if (ciphertext_hiding_nr_asids != -1 &&
>>>> +		    ciphertext_hiding_nr_asids >= min_sev_asid) {
>>>> +			pr_info("ciphertext_hiding_nr_asids module parameter invalid, limiting SEV-SNP ASIDs to %d\n",
>>>> +				 min_sev_asid);
>>>> +			ciphertext_hiding_nr_asids = min_sev_asid - 1;
>>>
>>> So specifying a number greater than min_sev_asid will result in enabling
>>> ciphertext hiding and no SEV-ES guests allowed even though you report that
>>> the number is invalid?
>>>
>>
>> Well, the user specified a non-zero ciphertext_hiding_nr_asids, so the intent is to enable ciphertext hiding and therefore 
>> sanitize the user specified parameter and enable ciphertext hiding.
> 
> Should you support something like ciphertext_hiding_asids=max (similar to
> the 'auto' comment that Dave had) and report an error if a value is
> specified that exceeds min_sev_asid and not enable ciphertext hiding?
>

Ok, that makes sense.

Thanks,
Ashish
 
> Thanks,
> Tom
> 
>>  
>