[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cc8c6e32-f383-4ae9-8c49-5e61bfb0d86c@amd.com>
Date: Thu, 5 Jun 2025 16:32:13 +1000
From: Alexey Kardashevskiy <aik@....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, hpa@...or.com, herbert@...dor.apana.org.au,
akpm@...ux-foundation.org, paulmck@...nel.org, rostedt@...dmis.org
Cc: x86@...nel.org, thuth@...hat.com, ardb@...nel.org,
gregkh@...uxfoundation.org, john.allen@....com, davem@...emloft.net,
thomas.lendacky@....com, michael.roth@....com, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v4 5/5] KVM: SEV: Add SEV-SNP CipherTextHiding support
On 20/5/25 10:02, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@....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@...gle.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@....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);
> }
>
--
Alexey
Powered by blists - more mailing lists