Re: [PATCH 2/4] kvm: svm: Defer VMSA allocation to LAUNCH_FINISH stage

[Tom Lendacky <thomas.lendacky@amd.com>]

On 6/11/26 07:35, Jörg Rödel wrote:
> From: Joerg Roedel <joerg.roedel@amd.com>
> 
> Do not allocate a KVM-managed VMSA for all VCPUs on VCPU creation,
> defer it to the LAUNCH_FINISH stage of SEV-ES and SEV-SNP. At this
> stage the VMSAs get used for the first time.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/kvm/svm/sev.c | 40 +++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 9b1280222e20..350bb97c32c0 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1095,11 +1095,11 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
>  {
>  	struct kvm_vcpu *vcpu = &svm->vcpu;
>  	struct kvm_sev_info *sev = to_kvm_sev_info(vcpu->kvm);
> -	struct sev_es_save_area *save = sev_es_vmsa_ref(vcpu);
> +	struct sev_es_save_area *save;
>  	struct xregs_state *xsave;
>  	const u8 *s;
> +	int ret, i;
>  	u8 *d;
> -	int i;
>  
>  	lockdep_assert_held(&vcpu->mutex);
>  
> @@ -1110,6 +1110,12 @@ static int sev_es_sync_vmsa(struct vcpu_svm *svm)
>  	if (svm->vcpu.guest_debug || (svm->vmcb->save.dr7 & ~DR7_FIXED_1))
>  		return -EINVAL;
>  
> +	ret = sev_es_vcpu_alloc_vmsa(vcpu);
> +	if (ret)
> +		return ret;
> +
> +	save = sev_es_vmsa_ref(vcpu);
> +
>  	/*
>  	 * SEV-ES will use a VMSA that is pointed to by the VMCB, not
>  	 * the traditional VMSA that is part of the VMCB. Copy the
> @@ -1196,7 +1202,7 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
>  {
>  	struct sev_data_launch_update_vmsa vmsa;
>  	struct vcpu_svm *svm = to_svm(vcpu);
> -	void *vmsa_ref = sev_es_vmsa_ref(vcpu);
> +	void *vmsa_ref;
>  	int ret;
>  
>  	if (vcpu->guest_debug) {
> @@ -1209,6 +1215,8 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
>  	if (ret)
>  		return ret;
>  
> +	vmsa_ref = sev_es_vmsa_ref(vcpu);

Maybe just a comment above this that indicates if the sync was
successful then a VMSA has been allocated and only now can you get a
pointer to it ?
> +
>  	/*
>  	 * The LAUNCH_UPDATE_VMSA command will perform in-place encryption of
>  	 * the VMSA memory content (i.e it will write the same memory region
> @@ -1237,6 +1245,9 @@ static int __sev_launch_update_vmsa(struct kvm *kvm, struct kvm_vcpu *vcpu,
>  	fpstate_set_confidential(&vcpu->arch.guest_fpu);
>  	vcpu->arch.guest_state_protected = true;
>  
> +	/* VMSA encrypted - put it into the VMCB */
> +	svm->vmcb->control.vmsa_pa = sev_es_vmsa_pa(vcpu);
> +
>  	/*
>  	 * SEV-ES guest mandates LBR Virtualization to be _always_ ON. Enable it
>  	 * only after setting guest_state_protected because KVM_SET_MSRS allows
> @@ -2689,12 +2700,14 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  
>  	kvm_for_each_vcpu(i, vcpu, kvm) {
>  		struct vcpu_svm *svm = to_svm(vcpu);
> -		void *vmsa = sev_es_vmsa_ref(vcpu);
> +		void *vmsa;
>  
>  		ret = sev_es_sync_vmsa(svm);
>  		if (ret)
>  			goto out;
>  
> +		vmsa = sev_es_vmsa_ref(vcpu);
> +
>  		ret = sev_es_vcpu_vmsa_make_private(vcpu);
>  		if (ret)
>  			goto out;
> @@ -2710,6 +2723,10 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  		}
>  
>  		svm->vcpu.arch.guest_state_protected = true;
> +
> +		/* VMSA encrypted - put it into the VMCB */
> +		svm->vmcb->control.vmsa_pa = sev_es_vmsa_pa(vcpu);
> +
>  		/*
>  		 * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
>  		 * be _always_ ON. Enable it only after setting
> @@ -4914,22 +4931,11 @@ void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
>  int sev_vcpu_create(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
> -	int ret;
>  
>  	mutex_init(&svm->sev_es.snp_vmsa_mutex);
>  
> -	if (!is_sev_es_guest(vcpu))
> -		return 0;

If you leave this here, it might make it easier to add things in the
future should something be needed. Just tack them on at the bottom. Not
necessary, though.

Thanks,
Tom
> -
> -	/*
> -	 * SEV-ES guests require a separate (from the VMCB) VMSA page used to
> -	 * contain the encrypted register state of the guest.
> -	 */
> -	ret = sev_es_vcpu_alloc_vmsa(vcpu);
> -	if (ret)
> -		return ret;
> -
> -	vcpu->arch.guest_tsc_protected = snp_is_secure_tsc_enabled(vcpu->kvm);
> +	if (is_sev_es_guest(vcpu))
> +		vcpu->arch.guest_tsc_protected = snp_is_secure_tsc_enabled(vcpu->kvm);
>  
>  	return 0;
>  }