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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <790fd770-de75-4bdf-a1dd-492f880b5fd6@amd.com>
Date: Wed, 2 Jul 2025 16:46:34 -0500
From: Kim Phillips <kim.phillips@....com>
To: Ashish Kalra <Ashish.Kalra@....com>, corbet@....net, seanjc@...gle.com,
 pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
 thomas.lendacky@....com, john.allen@....com, herbert@...dor.apana.org.au,
 davem@...emloft.net, akpm@...ux-foundation.org, rostedt@...dmis.org,
 paulmck@...nel.org
Cc: nikunj@....com, Neeraj.Upadhyay@....com, aik@....com, ardb@...nel.org,
 michael.roth@....com, arnd@...db.de, linux-doc@...r.kernel.org,
 linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
 kvm@...r.kernel.org
Subject: Re: [PATCH v5 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support

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@....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