Re: [PATCH v2 8/9] KVM: SVM: Add support to initialize SEV/SNP functionality in KVM

[Alexey Kardashevskiy <aik@amd.com>]

On 17/12/24 10:59, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@amd.com>
> 
> Remove platform initialization of SEV/SNP from PSP driver probe time and
> move it to KVM module load time so that KVM can do SEV/SNP platform
> initialization explicitly if it actually wants to use SEV/SNP
> functionality.
> 
> With this patch, KVM will explicitly call into the PSP driver at load time
> to initialize SNP by default while SEV initialization is done on-demand
> when SEV/SEV-ES VMs are being launched.
> 
> Additionally do SEV platform shutdown when all SEV/SEV-ES VMs have been
> destroyed to support SEV firmware hotloading and do full SEV and SNP
> platform shutdown during KVM module unload time.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> ---
>   arch/x86/kvm/svm/sev.c | 33 +++++++++++++++++++++++++++++----
>   1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 72674b8825c4..d55e281ac798 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -86,6 +86,7 @@ unsigned int max_sev_asid;
>   static unsigned int min_sev_asid;
>   static unsigned long sev_me_mask;
>   static unsigned int nr_asids;
> +static unsigned int nr_sev_vms_active;
>   static unsigned long *sev_asid_bitmap;
>   static unsigned long *sev_reclaim_asid_bitmap;
>   
> @@ -444,10 +445,16 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
>   	if (ret)
>   		goto e_no_asid;
>   
> -	init_args.probe = false;
> -	ret = sev_platform_init(&init_args);
> -	if (ret)
> -		goto e_free;
> +	if ((vm_type == KVM_X86_SEV_VM) ||
> +	    (vm_type == KVM_X86_SEV_ES_VM)) {
> +		down_write(&sev_deactivate_lock);
> +		ret = sev_platform_init(&init_args);
> +		if (!ret)
> +			++nr_sev_vms_active;
> +		up_write(&sev_deactivate_lock);
> +		if (ret)
> +			goto e_free;
> +	}
>   
>   	/* This needs to happen after SEV/SNP firmware initialization. */
>   	if (vm_type == KVM_X86_SNP_VM) {
> @@ -2942,6 +2949,10 @@ void sev_vm_destroy(struct kvm *kvm)
>   			return;
>   	} else {
>   		sev_unbind_asid(kvm, sev->handle);
> +		down_write(&sev_deactivate_lock);
> +		if (--nr_sev_vms_active == 0)
> +			sev_platform_shutdown();
> +		up_write(&sev_deactivate_lock);
>   	}
>   
>   	sev_asid_free(sev);
> @@ -2966,6 +2977,7 @@ void __init sev_set_cpu_caps(void)
>   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};

{} (without '0') should do the trick too.

>   	bool sev_snp_supported = false;
>   	bool sev_es_supported = false;
>   	bool sev_supported = false;
> @@ -3082,6 +3094,16 @@ void __init sev_hardware_setup(void)
>   	sev_supported_vmsa_features = 0;
>   	if (sev_es_debug_swap_enabled)
>   		sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
> +
> +	if (!sev_enabled)
> +		return;
> +
> +	/*
> +	 * NOTE: Always do SNP INIT regardless of sev_snp_supported
> +	 * as SNP INIT has to be done to launch legacy SEV/SEV-ES
> +	 * VMs in case SNP is enabled system-wide.

Out of curiosity - is not SNP INIT what "enables SNP system-wide"? What 
is that thing which SNP INIT does to allow SEV VMs to run? Thanks,


> +	 */
> +	sev_snp_platform_init(&init_args);
>   }
>   
>   void sev_hardware_unsetup(void)
> @@ -3097,6 +3119,9 @@ void sev_hardware_unsetup(void)
>   
>   	misc_cg_set_capacity(MISC_CG_RES_SEV, 0);
>   	misc_cg_set_capacity(MISC_CG_RES_SEV_ES, 0);
> +
> +	/* Do SEV and SNP Shutdown */
> +	sev_snp_platform_shutdown();
>   }
>   
>   int sev_cpu_init(struct svm_cpu_data *sd)

-- 
Alexey