Re: [PATCH v7 7/7] KVM: SEV: Add SEV-SNP CipherTextHiding support
From: Tom Lendacky <thomas.lendacky@amd.com>
Date: 2025-07-25 18:28:19
Also in:
kvm, linux-doc, lkml
On 7/25/25 12:58, Kim Phillips wrote:
Hi Ashish, For patches 1 through 6 in this series: Reviewed-by: Kim Phillips <redacted> For this 7/7 patch, consider making the simplification changes I've supplied in the diff at the bottom of this email: it cuts the number of lines for check_and_enable_sev_snp_ciphertext_hiding() in half.
Not sure that change works completely... see below.
Thanks, Kim On 7/21/25 9:14 AM, Ashish Kalra wrote:quoted
From: Ashish Kalra <ashish.kalra@amd.com>
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 7ac0f0f25e68..bd0947360e18 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c@@ -59,7 +59,7 @@ 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]; +static char ciphertext_hiding_asids[10]; 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 specify the number of ASIDs to use ('max' to utilize all available SEV-SNP ASIDs");@@ -2970,42 +2970,22 @@ static bool is_sev_snp_initialized(void)static bool check_and_enable_sev_snp_ciphertext_hiding(void) { - unsigned int ciphertext_hiding_asid_nr = 0; - - if (!ciphertext_hiding_asids[0]) - return false;
If the parameter was never specified
-
- if (!sev_is_snp_ciphertext_hiding_supported()) {
- pr_warn("Module parameter ciphertext_hiding_asids specified but
ciphertext hiding not supported\n");
- return false;
- }Removing this block will create an issue below.
-
- if (isdigit(ciphertext_hiding_asids[0])) {
- if (kstrtoint(ciphertext_hiding_asids, 10,
&ciphertext_hiding_asid_nr))
- goto invalid_parameter;
-
- /* Do sanity check on user-defined ciphertext_hiding_asids */
- if (ciphertext_hiding_asid_nr >= min_sev_asid) {
- pr_warn("Module parameter 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;
+ if (!strcmp(ciphertext_hiding_asids, "max")) {
+ max_snp_asid = min_sev_asid - 1;
+ return true;
}
- if (ciphertext_hiding_asid_nr) {
- 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;
+ /* Do sanity check on user-defined ciphertext_hiding_asids */
+ if (kstrtoint(ciphertext_hiding_asids,
sizeof(ciphertext_hiding_asids), &max_snp_asid) ||The second parameter is supposed to be the base, this gets lucky because you changed the size of the ciphertext_hiding_asids to 10.
quoted hunk ↗ jump to hunk
+ max_snp_asid >= min_sev_asid || + !sev_is_snp_ciphertext_hiding_supported()) { + pr_warn("ciphertext_hiding not supported, or invalid ciphertext_hiding_asids \"%s\", or !(0 < %u < minimum SEV ASID %u)\n", + ciphertext_hiding_asids, max_snp_asid, min_sev_asid); + max_snp_asid = min_sev_asid - 1; + return false; } -invalid_parameter: - pr_warn("Module parameter ciphertext_hiding_asids (%s) invalid\n", - ciphertext_hiding_asids); - return false; + return true; } void __init sev_hardware_setup(void)@@ -3122,8 +3102,11 @@ void __init sev_hardware_setup(void)* ASID range into separate SEV-ES and SEV-SNP ASID ranges with * the SEV-SNP ASID starting at 1. */ - if (check_and_enable_sev_snp_ciphertext_hiding()) + if (check_and_enable_sev_snp_ciphertext_hiding()) { + pr_info("SEV-SNP ciphertext hiding enabled\n"); init_args.max_snp_asid = max_snp_asid; + min_sev_es_asid = max_snp_asid + 1;
If "max" was specified, but ciphertext hiding isn't enabled, you've now changed min_sev_es_asid to an incorrect value and will be trying to enable ciphertext hiding during initialization. Thanks, Tom
+ } if (sev_platform_init(&init_args)) sev_supported = sev_es_supported = sev_snp_supported = false; else if (sev_snp_supported)