All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: "Jörg Rödel" <joro@8bytes.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	x86@kernel.org,  Tom Lendacky <thomas.lendacky@amd.com>,
	Michael Roth <michael.roth@amd.com>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org,
	coconut-svsm@lists.linux.dev,
	 Joerg Roedel <joerg.roedel@amd.com>
Subject: Re: [PATCH 4/4] kvm: svm: Support KVM_SEV_SNP_PAGE_TYPE_VMSA at SNP_LAUNCH_UPDATE
Date: Tue, 23 Jun 2026 14:29:01 -0700	[thread overview]
Message-ID: <ajr6nW11Ss9IP4gp@google.com> (raw)
In-Reply-To: <20260611123528.572255-5-joro@8bytes.org>

On Thu, Jun 11, 2026, Jörg Rödel wrote:
> From: Joerg Roedel <joerg.roedel@amd.com>
> 
> Support setting a VMSA in guest physical memory during the SEV-SNP
> launch process. Only one VMSA can be provided which will then be used
> for the BSP. All of the APs will not have a VMSA allocated or assigned
> when this feature is used.
> 
> This ensures stable launch measurements on SEV-SNP which are
> independent of the number of VCPUs the VM is launched with.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/include/uapi/asm/kvm.h |  1 +
>  arch/x86/kvm/svm/sev.c          | 44 ++++++++++++++++++++++++++++++++-
>  arch/x86/kvm/x86.c              |  1 +
>  include/uapi/linux/kvm.h        |  1 +
>  4 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 5f2b30d0405c..fc87a5ba295b 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -885,6 +885,7 @@ struct kvm_sev_snp_launch_start {
>  /* Kept in sync with firmware values for simplicity. */
>  #define KVM_SEV_PAGE_TYPE_INVALID		0x0
>  #define KVM_SEV_SNP_PAGE_TYPE_NORMAL		0x1
> +#define KVM_SEV_SNP_PAGE_TYPE_VMSA		0x2
>  #define KVM_SEV_SNP_PAGE_TYPE_ZERO		0x3
>  #define KVM_SEV_SNP_PAGE_TYPE_UNMEASURED	0x4
>  #define KVM_SEV_SNP_PAGE_TYPE_SECRETS		0x5
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 88db83b3ff8e..90399d5d0358 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2520,6 +2520,20 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	return rc;
>  }
>  
> +static bool snp_check_launch_vmsa(struct kvm_sev_info *sev,
> +				  struct sev_es_save_area *vmsa)
> +{
> +	/* VMSA sev_features must match VMs vmsa_features */
> +	if (vmsa->sev_features != sev->vmsa_features)
> +		return false;
> +
> +	/* Must always boot from VMPL0 */

Why?

> +	if (vmsa->vmpl != 0)
> +		return false;
> +
> +	return true;
> +}
> +
>  struct sev_gmem_populate_args {
>  	__u8 type;
>  	int sev_fd;
> @@ -2532,7 +2546,9 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>  	struct sev_gmem_populate_args *sev_populate_args = opaque;
>  	struct sev_data_snp_launch_update fw_args = {0};
>  	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> +	gpa_t gpa = gfn << PAGE_SHIFT;
>  	bool assigned = false;
> +	u64 sev_features = 0;
>  	int level;
>  	int ret;
>  
> @@ -2550,14 +2566,27 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>  	if (src_page) {
>  		void *src_vaddr = kmap_local_page(src_page);
>  		void *dst_vaddr = kmap_local_pfn(pfn);
> +		struct sev_es_save_area *vmsa = dst_vaddr;
> +		bool accept_page = true;
>  
>  		memcpy(dst_vaddr, src_vaddr, PAGE_SIZE);
>  
> +		if (sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_VMSA) {
> +			accept_page = snp_check_launch_vmsa(sev, vmsa);

Isn't this technically a breaking change?  AFAICT, nothing prevents userspace
from adding a VMSA page today.  Ah, never mind, snp_launch_update() does the
check.

Anyways, given that SVM_VMGEXIT_AP_CREATE doesn't check the actual memory, and
that control.allowed_sev_features is what provides the "real" protection, I would
rather either trust userspace to not screw up (first choice), or have userspace
provide the requested features as a param to the ioctl (second choice, because
it's purely a nice-to-have for userspace).

Then, assuming we don't *need* to enforce the VMPL0 thing, there's no need to
modify sev_gmem_post_populate().  I.e. decouple adding VMSA page(s) to the image
from associating a vCPU with a VMSA.

> +			if (accept_page)
> +				sev_features = vmsa->sev_features;
> +		}
> +
>  		kunmap_local(src_vaddr);
>  		kunmap_local(dst_vaddr);
> +
> +		if (!accept_page) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
>  	}
>  
> -	ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> +	ret = rmp_make_private(pfn, gpa, PG_LEVEL_4K,
>  			       sev_get_asid(kvm), true);
>  	if (ret)
>  		goto out;
> @@ -2593,6 +2622,9 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>  		kunmap_local(dst_vaddr);
>  	}
>  
> +	if (ret == 0 && sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_VMSA)
> +		sev->initial_vmsa_gpa = gpa;
> +
>  out:
>  	if (ret)
>  		pr_debug("%s: error updating GFN %llx, return code %d (fw_error %d)\n",
> @@ -2620,12 +2652,22 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  
>  	if (!params.len || !PAGE_ALIGNED(params.len) || params.flags ||
>  	    (params.type != KVM_SEV_SNP_PAGE_TYPE_NORMAL &&
> +	     params.type != KVM_SEV_SNP_PAGE_TYPE_VMSA &&
>  	     params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO &&
>  	     params.type != KVM_SEV_SNP_PAGE_TYPE_UNMEASURED &&
>  	     params.type != KVM_SEV_SNP_PAGE_TYPE_SECRETS &&
>  	     params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
>  		return -EINVAL;
>  
> +	if (params.type == KVM_SEV_SNP_PAGE_TYPE_VMSA) {
> +		/* VMSA page are allowed only once */

Why?  What would go wrong if a VM image has a pile of pre-baked VMSAs?

> +		if (sev->initial_vmsa_gpa != INVALID_PAGE)
> +			return -EBUSY;
> +		/* Can only deploy a single page as VMSA */
> +		if (params.len != PAGE_SIZE)
> +			return -EINVAL;
> +	}
> +
>  	src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
>  
>  	if (!PAGE_ALIGNED(src))
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0550359ed798..dc9abe62476e 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4870,6 +4870,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_MEMORY_FAULT_INFO:
>  	case KVM_CAP_X86_GUEST_MODE:
>  	case KVM_CAP_ONE_REG:
> +	case KVM_CAP_SNP_DIRECT_VMSA:
>  		r = 1;
>  		break;
>  	case KVM_CAP_PRE_FAULT_MEMORY:
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 6c8afa2047bf..bf034435f98c 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -996,6 +996,7 @@ struct kvm_enable_cap {
>  #define KVM_CAP_S390_USER_OPEREXEC 246
>  #define KVM_CAP_S390_KEYOP 247
>  #define KVM_CAP_S390_VSIE_ESAMODE 248
> +#define KVM_CAP_SNP_DIRECT_VMSA 249
>  
>  struct kvm_irq_routing_irqchip {
>  	__u32 irqchip;
> -- 
> 2.53.0
> 

      parent reply	other threads:[~2026-06-23 21:29 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-11 12:35 [PATCH 0/4] KVM: SEV: Support direct setting of VMSA for SEV-SNP guests Jörg Rödel
2026-06-11 12:35 ` [PATCH 1/4] kvm: svm: Streamline VMSA setting for VCPUs Jörg Rödel
2026-06-11 12:56   ` sashiko-bot
2026-06-11 14:13     ` Jörg Rödel
2026-06-16 20:52   ` Tom Lendacky
2026-06-23 10:55     ` Jörg Rödel
2026-06-23 20:18   ` Sean Christopherson
2026-06-11 12:35 ` [PATCH 2/4] kvm: svm: Defer VMSA allocation to LAUNCH_FINISH stage Jörg Rödel
2026-06-11 12:58   ` sashiko-bot
2026-06-11 14:29     ` Jörg Rödel
2026-06-16 21:33   ` Tom Lendacky
2026-06-23 11:26     ` Jörg Rödel
2026-06-11 12:35 ` [PATCH 3/4] kvm: svm: Support guest-provided VMSA for launching Jörg Rödel
2026-06-11 13:05   ` sashiko-bot
2026-06-11 14:43     ` Jörg Rödel
2026-06-16 21:48   ` Tom Lendacky
2026-06-23 11:36     ` Jörg Rödel
2026-06-23 21:07   ` Sean Christopherson
2026-06-11 12:35 ` [PATCH 4/4] kvm: svm: Support KVM_SEV_SNP_PAGE_TYPE_VMSA at SNP_LAUNCH_UPDATE Jörg Rödel
2026-06-11 12:43   ` Sean Christopherson
2026-06-11 13:23     ` Jörg Rödel
2026-06-16 17:55       ` Sean Christopherson
2026-06-17  6:45         ` Jörg Rödel
2026-06-17 13:00           ` Sean Christopherson
2026-06-17 13:25             ` Jörg Rödel
2026-06-17 13:37               ` Sean Christopherson
2026-06-17 14:44                 ` Jörg Rödel
2026-06-23 13:40                   ` Sean Christopherson
2026-06-23 14:44                     ` Jörg Rödel
2026-06-23 14:51                     ` [EXTERNAL] " Jon Lange
2026-06-23 20:23                       ` Sean Christopherson
2026-06-23 20:43                       ` Jethro Beekman
2026-06-23 21:43                         ` Sean Christopherson
2026-06-23 21:47                           ` Jethro Beekman
2026-06-23 22:02                             ` Sean Christopherson
2026-06-23 22:35                               ` Jethro Beekman
2026-06-23 22:55                                 ` Sean Christopherson
2026-06-23 23:08                                   ` Jethro Beekman
2026-06-23 23:43                                     ` Sean Christopherson
2026-06-17 13:18           ` James Bottomley
2026-06-17 13:28             ` Jörg Rödel
2026-06-17 13:45               ` James Bottomley
2026-06-17 14:53                 ` Jörg Rödel
2026-06-11 12:58   ` sashiko-bot
2026-06-11 15:23     ` Jörg Rödel
2026-06-16 22:11   ` Tom Lendacky
2026-06-23 11:48     ` Jörg Rödel
2026-06-23 21:29   ` Sean Christopherson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajr6nW11Ss9IP4gp@google.com \
    --to=seanjc@google.com \
    --cc=coconut-svsm@lists.linux.dev \
    --cc=joerg.roedel@amd.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=thomas.lendacky@amd.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.