Re: [PATCH 1/4] kvm: svm: Streamline VMSA setting for VCPUs

[Sean Christopherson]

On Thu, Jun 11, 2026, Jörg Rödel wrote:
> From: Joerg Roedel <joerg.roedel@xxxxxxx>
> 
> Streamline the VMSA setting state of vcpus, where a VMSA can be either
> KVM-allocated or guest-provided. This consolidates the various
> tracking state around VMSAs.

This needs to be split up into distinct logic changes.  There are renames, there
are tracking changes, there is code movement, and who knows what else, all bundled
together.  And the changelog is little more than "consolidate stuff".

> 
> Signed-off-by: Joerg Roedel <joerg.roedel@xxxxxxx>
> ---
>  arch/x86/kvm/svm/sev.c | 301 ++++++++++++++++++++++++++++-------------
>  arch/x86/kvm/svm/svm.h |  31 ++++-
>  2 files changed, 237 insertions(+), 95 deletions(-)

+180 lines of code doesn't scream "streamline" to me.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 6c6a6d663e29..9b1280222e20 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -147,6 +147,9 @@ static bool sev_snp_guest(struct kvm *kvm)
>  }
>  
>  static int snp_decommission_context(struct kvm *kvm);
> +static int kvm_rmp_make_shared(struct kvm *kvm, u64 pfn, enum pg_level level);
> +static void sev_flush_encrypted_page(struct kvm_vcpu *vcpu, void *va);
> +static int snp_page_reclaim(struct kvm *kvm, u64 pfn);
>  
>  struct enc_region {
>  	struct list_head list;
> @@ -156,6 +159,173 @@ struct enc_region {
>  	unsigned long size;
>  };
>  
> +static void *sev_es_vmsa_ref(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	void *vmsa = NULL;
> +
> +	if (svm->sev_es.vmsa.vmsa_state == VMSA_SHARED) {

Unnecessary curly braces.

> +		vmsa = page_address(svm->sev_es.vmsa.vmsa_page);
> +	}
> +
> +	return vmsa;
> +}

...

> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 5137416be593..3d4799f09b23 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -240,9 +240,29 @@ struct svm_nested_state {
>  	bool force_msr_bitmap_recalc;
>  };
>  
> +enum vmsa_state {
> +	/* No VMSA set */
> +	VMSA_NONE,
> +	/* VMSA allocated by KVM - Shared in RMP (if applicable) */
> +	VMSA_SHARED,
> +	/* VMSA allocated by KVM - Guest-private in RMP (SEV-SNP only) */
> +	VMSA_PRIVATE,
> +	/* Guest-owned VMSA */
> +	VMSA_GUEST,
> +};

I am strongly against this tracking.  AFAICT, every case is completely redundant
with other information that *must* be tracked.