[PATCH v3 0/8] KVM: SVM: Support direct setting of VMSA for SEV-SNP guests

[PATCH v3 0/8] KVM: SVM: Support direct setting of VMSA for SEV-SNP guests

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Hi,

here is the updated version of KVM-AMD changes to allow user-space to
directly set the boot VMSA (direct-vmsa) for an SEV-SNP virtual
machine.

The direct-vmsa support is required for stable launch measurements
when loading the initial SEV-SNP guest image from an IGVM file.
Without this support KVM measures a VMSA at a fixed GPA for every
VCPU, which makes the launch measurement dependent on the number of
VCPUs. Besides that the current KVM VMSA-allocation behavior poses
challenges for live migration of SEV-SNP VMs. The current behavior is
fully preserved when direct-vmsa is not used.

For a longer justification have a look at this email from the v1
thread:
	
	https://lore.kernel.org/kvm/aiqwjn2DVOumfr3T@8bytes.org/

The user-space API remains unchanged since v2 of the series and
consists of three parts:

	- A new KVM_CAP_SNP_DIRECT_VMSA VM-level capability which can
	  be queried and enabled from user-space. When user-space
	  enabled the capability, KVM will no longer allocate VMSA
	  pages when creating VCPUs. This means that the capability
	  can only be enabled before any VCPUs are created.

	- Allow VMSA pages in SNP launch updates. This was already
	  present in the previous version, but has different semantics
	  now. In this version all it does is adding the VMSA pages to
	  the initial image and the launch measurement. 
	
	- A new per-VCPU MEM_ENC operation to get and set the VMSA and
	  GHCB addresses of a VCPU. User-space will use this operation
	  to set the GPA of the launch VMSA of the VCPUs, but can
	  also query and set the addresses during runtime. 

	  This operation is designed to support the requirements of
	  future live-migration support, which needs to be able to set
	  and get VMSA as well as GHCB addresses. Since this
	  functionality is a superset of the operation needed for
	  direct-vmsa support, it goes the extra mile and allows
	  import and export of all SEV-SNP specific VCPU state.

The changes are rebased to v7.4-rc1 and tested with lockdep enabled.

Please review.

-Joerg

Joerg Roedel (8):
  KVM: SEV: Document SNP direct VMSA userspace ABI
  KVM: SVM: Implement GET_AP_APIC_IDS NAE event
  KVM: SVM: Hold SRCU while reloading guest-owned VMSAs
  KVM: SEV: Add direct VMSA capability
  KVM: SEV: Allow VMSA pages in SNP launch updates
  KVM: SEV: Add SNP vCPU state get and set commands
  KVM: selftests: Test the SNP APIC-ID-list GHCB request
  KVM: selftests: Test SNP vCPU state and direct VMSA launch

 .../virt/kvm/x86/amd-memory-encryption.rst    |  66 ++-
 arch/x86/include/asm/kvm-x86-ops.h            |   1 +
 arch/x86/include/asm/kvm_host.h               |   1 +
 arch/x86/include/asm/sev-common.h             |   1 +
 arch/x86/include/uapi/asm/kvm.h               |  13 +
 arch/x86/include/uapi/asm/svm.h               |   1 +
 arch/x86/kvm/svm/sev.c                        | 290 +++++++++++-
 arch/x86/kvm/svm/svm.c                        |  23 +
 arch/x86/kvm/svm/svm.h                        |   2 +
 arch/x86/kvm/x86.c                            |   6 +
 include/uapi/linux/kvm.h                      |   2 +
 tools/testing/selftests/kvm/Makefile.kvm      |   2 +
 tools/testing/selftests/kvm/include/x86/sev.h |  49 ++
 .../selftests/kvm/x86/sev_snp_apic_id_test.c  | 180 ++++++++
 .../kvm/x86/sev_snp_direct_vmsa_test.c        | 421 ++++++++++++++++++
 15 files changed, 1042 insertions(+), 16 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c
 create mode 100644 tools/testing/selftests/kvm/x86/sev_snp_direct_vmsa_test.c

-- 
2.53.0

[PATCH v3 1/8] KVM: SEV: Document SNP direct VMSA userspace ABI

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Document the VMSA launch-update page type, the vCPU-scoped state get and
set commands, and the VM-scoped direct-VMSA capability in one place.

Describe the ordering and argument requirements for enabling direct-VMSA
mode, and contrast its VMSA allocation and measurement behavior with the
legacy launch flow.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 .../virt/kvm/x86/amd-memory-encryption.rst    | 66 ++++++++++++++++++-
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
index eeb0812cb10a..42391d9cebce 100644
--- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
@@ -542,6 +542,7 @@ Returns: 0 on success, < 0 on error, -EAGAIN if caller should retry
 where the allowed values for page_type are #define'd as::
 
         KVM_SEV_SNP_PAGE_TYPE_NORMAL
+        KVM_SEV_SNP_PAGE_TYPE_VMSA
         KVM_SEV_SNP_PAGE_TYPE_ZERO
         KVM_SEV_SNP_PAGE_TYPE_UNMEASURED
         KVM_SEV_SNP_PAGE_TYPE_SECRETS
@@ -550,7 +551,68 @@ where the allowed values for page_type are #define'd as::
 See the SEV-SNP spec [snp-fw-abi]_ for further details on how each page type is
 used/measured.
 
-20. KVM_SEV_SNP_LAUNCH_FINISH
+``KVM_SEV_SNP_PAGE_TYPE_VMSA`` creates VMSA pages as part of the measured
+initial image.  A request must contain exactly one 4 KiB VMSA page, but the
+command may be used multiple times.  Creating a VMSA page does not associate it
+with a vCPU; use ``KVM_SEV_SNP_SET_VCPU_STATE`` on the intended vCPU file
+descriptor before launch finish to make that association.  KVM treats the
+VMSA contents as guest-owned data, but requires VMPL 0 and a ``sev_features``
+value that matches the VM's configured VMSA features.
+
+20. KVM_SEV_SNP_GET_VCPU_STATE / KVM_SEV_SNP_SET_VCPU_STATE
+------------------------------------------------------------
+
+These commands get or set the VMSA and GHCB addresses for the vCPU on whose
+file descriptor the command is issued.  Unlike the other SEV commands,
+userspace must issue KVM_MEMORY_ENCRYPT_OP on a vCPU file descriptor.  The
+capability is reported as ``KVM_CAP_SNP_VCPU_STATE``.
+
+Parameters (in/out): struct kvm_sev_snp_vcpu_state
+
+Returns: 0 on success, -negative on error
+
+::
+
+        #define KVM_SEV_SNP_VCPU_STATE_VMSA_VALID        _BITULL(0)
+        #define KVM_SEV_SNP_VCPU_STATE_GHCB_VALID        _BITULL(1)
+
+        struct kvm_sev_snp_vcpu_state {
+                __u64 valid_fields;
+                __u64 vmsa_gpa;
+                __u64 ghcb_gpa;
+                __u64 pad[5];           /* Must be zero */
+        };
+
+``KVM_SEV_SNP_GET_VCPU_STATE`` returns the current addresses and sets the
+corresponding bit in ``valid_fields`` for each valid address.
+
+``KVM_SEV_SNP_SET_VCPU_STATE`` sets addresses whose validity bits are present
+and invalidates addresses whose bits are absent.  The command must be issued
+after launch start and before KVM_SEV_SNP_LAUNCH_FINISH, and the VM must have
+enabled ``KVM_CAP_SNP_DIRECT_VMSA``.  A valid VMSA GPA must be backed by
+guest_memfd and populated.  The GPA must be 4-KiB aligned.  A valid GHCB
+address is copied without inspecting its backing page.  Nonzero reserved
+fields or unknown validity bits are rejected.
+
+``KVM_CAP_SNP_DIRECT_VMSA`` is a VM-scoped capability that selects direct-VMSA
+mode.  Userspace enables it with ``KVM_ENABLE_CAP`` on an SNP VM before
+creating any vCPUs.  ``flags`` and all elements of ``args`` must be zero.
+Enabling the capability on a non-SNP VM or after creating a vCPU is rejected.
+
+In direct-VMSA mode, KVM does not allocate a KVM-owned VMSA when a vCPU is
+created and does not generate or measure one at launch finish.  All launch
+VMSAs are owned and supplied by userspace.  Valid VMSAs selected with
+``KVM_SEV_SNP_SET_VCPU_STATE`` are preserved, while vCPUs without a valid VMSA
+have no runnable VMSA until the guest uses SNP AP creation to supply one.  If
+the capability is not enabled, launch finish retains the legacy behavior of
+generating and measuring a KVM-owned VMSA for every vCPU.  VMSAs that were
+measured but not selected remain ordinary valid pages in the initial image.
+
+Direct VMSAs make the launch measurement independent of KVM's selected VMSA
+GPA and of the configured vCPU count.  This gives VMMs a stable launch
+measurement across hypervisors.
+
+21. KVM_SEV_SNP_LAUNCH_FINISH
 -----------------------------
 
 After completion of the SNP guest launch flow, the KVM_SEV_SNP_LAUNCH_FINISH
@@ -578,7 +640,7 @@ Returns: 0 on success, -negative on error
 See SNP_LAUNCH_FINISH in the SEV-SNP specification [snp-fw-abi]_ for further
 details on the input parameters in ``struct kvm_sev_snp_launch_finish``.
 
-21. KVM_SEV_SNP_ENABLE_REQ_CERTS
+22. KVM_SEV_SNP_ENABLE_REQ_CERTS
 --------------------------------
 
 The KVM_SEV_SNP_ENABLE_REQ_CERTS command will configure KVM to exit to
-- 
2.53.0

[PATCH v3 2/8] KVM: SVM: Implement GET_AP_APIC_IDS NAE event

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Advertise and handle the GHCB APIC ID list feature so that SNP guests can
discover the vCPUs for which they need to create VMSAs without relying on
INIT-SIPI.

Return the required page count for undersized buffers.  For sufficiently
large requests, validate the GPA range and use KVM's SRCU-protected guest
write path to copy an exact snapshot of the online vCPU APIC IDs.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/sev-common.h |  1 +
 arch/x86/include/uapi/asm/svm.h   |  1 +
 arch/x86/kvm/svm/sev.c            | 67 ++++++++++++++++++++++++++++++-
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 01a6e4dbe423..c041f85dfd5c 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -136,6 +136,7 @@ enum psc_op {
 
 #define GHCB_HV_FT_SNP			BIT_ULL(0)
 #define GHCB_HV_FT_SNP_AP_CREATION	BIT_ULL(1)
+#define GHCB_HV_FT_APIC_ID_LIST		BIT_ULL(4)
 #define GHCB_HV_FT_SNP_MULTI_VMPL	BIT_ULL(5)
 
 /*
diff --git a/arch/x86/include/uapi/asm/svm.h b/arch/x86/include/uapi/asm/svm.h
index e80676185092..c9c85891971a 100644
--- a/arch/x86/include/uapi/asm/svm.h
+++ b/arch/x86/include/uapi/asm/svm.h
@@ -118,6 +118,7 @@
 #define SVM_VMGEXIT_AP_CREATE_ON_INIT		0
 #define SVM_VMGEXIT_AP_CREATE			1
 #define SVM_VMGEXIT_AP_DESTROY			2
+#define SVM_VMGEXIT_GET_APIC_IDS		0x80000017ull
 #define SVM_VMGEXIT_SNP_RUN_VMPL		0x80000018ull
 #define SVM_VMGEXIT_SAVIC			0x8000001aull
 #define SVM_VMGEXIT_SAVIC_REGISTER_GPA		0
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b90c5f14c1ac..f690edaf8b69 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -40,7 +40,9 @@
 #define GHCB_VERSION_MAX	2ULL
 #define GHCB_VERSION_MIN	1ULL
 
-#define GHCB_HV_FT_SUPPORTED	(GHCB_HV_FT_SNP | GHCB_HV_FT_SNP_AP_CREATION)
+#define GHCB_HV_FT_SUPPORTED	(GHCB_HV_FT_SNP | \
+				 GHCB_HV_FT_SNP_AP_CREATION | \
+				 GHCB_HV_FT_APIC_ID_LIST)
 
 /*
  * The GHCB spec essentially states that all non-zero error codes other than
@@ -3517,7 +3519,8 @@ static bool sev_es_are_required_ghcb_fields_valid(struct vcpu_svm *svm)
 	case SVM_VMGEXIT_AP_CREATION:
 		return kvm_ghcb_rax_is_valid(svm) ||
 		       lower_32_bits(control->exit_info_1) == SVM_VMGEXIT_AP_DESTROY;
-		break;
+	case SVM_VMGEXIT_GET_APIC_IDS:
+		return kvm_ghcb_rax_is_valid(svm);
 	case SVM_VMGEXIT_MMIO_READ:
 	case SVM_VMGEXIT_MMIO_WRITE:
 	case SVM_VMGEXIT_PSC:
@@ -4235,6 +4238,63 @@ static int sev_snp_ap_creation(struct vcpu_svm *svm)
 	return 0;
 }
 
+struct sev_apic_id_desc {
+	u32 num_entries;
+	u32 apic_ids[];
+};
+
+static int sev_snp_get_apic_ids(struct vcpu_svm *svm)
+{
+	struct kvm_vcpu *vcpu = &svm->vcpu;
+	struct kvm *kvm = vcpu->kvm;
+	struct sev_apic_id_desc *desc;
+	unsigned int nr_vcpus, size;
+	unsigned int i;
+	gpa_t gpa, end_gpa;
+	u64 pages;
+
+	nr_vcpus = atomic_read(&kvm->online_vcpus);
+	size = sizeof(*desc) + (nr_vcpus * sizeof(desc->apic_ids[0]));
+
+	pages = vcpu->arch.regs[VCPU_REGS_RAX];
+	if (pages < PFN_UP(size)) {
+		vcpu->arch.regs[VCPU_REGS_RAX] = PFN_UP(size);
+		return 1;
+	}
+
+	gpa = svm->vmcb->control.exit_info_1;
+	if (!PAGE_ALIGNED(gpa) ||
+	    check_add_overflow(gpa, size - 1, &end_gpa) ||
+	    !page_address_valid(vcpu, gpa) ||
+	    !page_address_valid(vcpu, end_gpa & PAGE_MASK))
+		goto invalid_buffer;
+
+	desc = kvzalloc(size, GFP_KERNEL_ACCOUNT);
+	if (!desc)
+		return -ENOMEM;
+
+	desc->num_entries = nr_vcpus;
+	for (i = 0; i < nr_vcpus; i++) {
+		struct kvm_vcpu *listed_vcpu = kvm_get_vcpu(kvm, i);
+
+		if (WARN_ON_ONCE(!listed_vcpu))
+			goto invalid_buffer_free;
+		desc->apic_ids[i] = listed_vcpu->vcpu_id;
+	}
+
+	if (kvm_write_guest(kvm, gpa, desc, size))
+		goto invalid_buffer_free;
+
+	kvfree(desc);
+	return 1;
+
+invalid_buffer_free:
+	kvfree(desc);
+invalid_buffer:
+	svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
+	return 1;
+}
+
 static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_gpa)
 {
 	struct sev_data_snp_guest_request data = {0};
@@ -4539,6 +4599,7 @@ static bool is_snp_only_vmgexit(u64 exit_code)
 {
 	switch (exit_code) {
 	case SVM_VMGEXIT_AP_CREATION:
+	case SVM_VMGEXIT_GET_APIC_IDS:
 	case SVM_VMGEXIT_GUEST_REQUEST:
 	case SVM_VMGEXIT_EXT_GUEST_REQUEST:
 	case SVM_VMGEXIT_PSC:
@@ -4710,6 +4771,8 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
 		if (sev_snp_ap_creation(svm))
 			svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
 		return 1;
+	case SVM_VMGEXIT_GET_APIC_IDS:
+		return sev_snp_get_apic_ids(svm);
 	case SVM_VMGEXIT_GUEST_REQUEST:
 	case SVM_VMGEXIT_EXT_GUEST_REQUEST:
 		if (!PAGE_ALIGNED(control->exit_info_1) ||
-- 
2.53.0

Re: [PATCH v3 2/8] KVM: SVM: Implement GET_AP_APIC_IDS NAE event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: SVM: Unconditional enablement of GHCB_HV_FT_APIC_ID_LIST feature
--

commit 3bffb8ab055298369df7622e8e3308488b5501b8
Author: Joerg Roedel <joerg.roedel@amd.com>

KVM: SVM: Implement GET_AP_APIC_IDS NAE event

This commit advertises GHCB_HV_FT_APIC_ID_LIST and implements the
SVM_VMGEXIT_GET_APIC_IDS NAE event so SEV-SNP guests can retrieve the list of
online vCPU APIC IDs without relying on INIT-SIPI.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index fad72cab76cf4..c0ae34871be90 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -40,7 +40,9 @@
>  #define GHCB_VERSION_MAX	2ULL
>  #define GHCB_VERSION_MIN	1ULL
>  
> -#define GHCB_HV_FT_SUPPORTED	(GHCB_HV_FT_SNP | GHCB_HV_FT_SNP_AP_CREATION)
> +#define GHCB_HV_FT_SUPPORTED	(GHCB_HV_FT_SNP | \
> +				 GHCB_HV_FT_SNP_AP_CREATION | \
> +				 GHCB_HV_FT_APIC_ID_LIST)

[ ... ]

> @@ -4708,6 +4769,8 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
>  		if (sev_snp_ap_creation(svm))
>  			svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
>  		return 1;
> +	case SVM_VMGEXIT_GET_APIC_IDS:
> +		return sev_snp_get_apic_ids(svm);

[Severity: High]
As reported by Sashiko on v2 [1], GHCB_HV_FT_APIC_ID_LIST is added
unconditionally to GHCB_HV_FT_SUPPORTED and sev_handle_vmgexit() dispatches
SVM_VMGEXIT_GET_APIC_IDS to sev_snp_get_apic_ids() for all SEV-SNP guests
without host userspace opting in.

When an SEV-SNP guest queries hypervisor features via GHCB_MSR_HV_FT_REQ in
sev_handle_vmgexit_msr_protocol() or SVM_VMGEXIT_HV_FEATURES in
sev_handle_vmgexit(), it will unconditionally observe GHCB_HV_FT_APIC_ID_LIST.
If a guest uses this feature and is migrated or restored onto a kernel without
SVM_VMGEXIT_GET_APIC_IDS support, guest execution will fail when invoking the
NAE event.

Should this new guest-visible feature default to off and be gated behind a
capability via KVM_CHECK_EXTENSION and KVM_ENABLE_CAP so userspace can manage
feature exposure and live migration compatibility?

[1] https://lore.kernel.org/r/20260908103338.427254-3-joro@8bytes.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102114137.1198982-1-joro@8bytes.org?part=2

Re: [PATCH v3 2/8] KVM: SVM: Implement GET_AP_APIC_IDS NAE event

[Jörg Rödel]

On Mon, Nov 02, 2026 at 11:52:44AM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> As reported by Sashiko on v2 [1], GHCB_HV_FT_APIC_ID_LIST is added
> unconditionally to GHCB_HV_FT_SUPPORTED and sev_handle_vmgexit() dispatches
> SVM_VMGEXIT_GET_APIC_IDS to sev_snp_get_apic_ids() for all SEV-SNP guests
> without host userspace opting in.
> 
> When an SEV-SNP guest queries hypervisor features via GHCB_MSR_HV_FT_REQ in
> sev_handle_vmgexit_msr_protocol() or SVM_VMGEXIT_HV_FEATURES in
> sev_handle_vmgexit(), it will unconditionally observe GHCB_HV_FT_APIC_ID_LIST.
> If a guest uses this feature and is migrated or restored onto a kernel without
> SVM_VMGEXIT_GET_APIC_IDS support, guest execution will fail when invoking the
> NAE event.
> 
> Should this new guest-visible feature default to off and be gated behind a
> capability via KVM_CHECK_EXTENSION and KVM_ENABLE_CAP so userspace can manage
> feature exposure and live migration compatibility?

No, this feature is for SEV-SNP guests only, which can not be live migrated.
Live migration for SEV-SNP guests will be built on-top of this feature (among
others).

[PATCH v3 3/8] KVM: SVM: Hold SRCU while reloading guest-owned VMSAs

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Take a dedicated KVM SRCU read lock around the guest-owned VMSA memslot
and guest_memfd lookup.  This makes the reload path self-contained and
prevents it from racing a userspace memslot replacement.

Keep the lock through VMCB installation, and release it on all lookup
failure paths.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm/sev.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f690edaf8b69..9681ff0aa5d3 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4059,6 +4059,7 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	gfn_t gfn = gpa_to_gfn(gpa);
 	unsigned long mmu_seq;
 	kvm_pfn_t pfn;
+	int idx;
 
 	lockdep_assert_held(&svm->sev_es.snp_vmsa_mutex);
 
@@ -4090,9 +4091,10 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	if (!VALID_PAGE(gpa))
 		return;
 
-	slot = gfn_to_memslot(vcpu->kvm, gfn);
+	idx = srcu_read_lock(&kvm->srcu);
+	slot = gfn_to_memslot(kvm, gfn);
 	if (!slot)
-		return;
+		goto out_unlock;
 
 	mmu_seq = kvm->mmu_invalidate_seq;
 	smp_rmb();
@@ -4101,8 +4103,8 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	 * The new VMSA will be private memory guest memory, so retrieve the
 	 * PFN from the gmem backend.
 	 */
-	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, NULL))
-		return;
+	if (kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, NULL))
+		goto out_unlock;
 
 	read_lock(&kvm->mmu_lock);
 	/*
@@ -4117,6 +4119,9 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	else
 		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
 	read_unlock(&kvm->mmu_lock);
+
+out_unlock:
+	srcu_read_unlock(&kvm->srcu, idx);
 }
 
 /*
-- 
2.53.0

Re: [PATCH v3 3/8] KVM: SVM: Hold SRCU while reloading guest-owned VMSAs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: SVM: redundant nested srcu_read_lock() in __sev_snp_reload_vmsa()
--

commit c628b3f34be1a24c7bbd7ced36d3f7d15af73940
Author: Joerg Roedel <joerg.roedel@amd.com>

KVM: SVM: Hold SRCU while reloading guest-owned VMSAs

This commit acquires a KVM SRCU read lock in __sev_snp_reload_vmsa() around
the memslot and guest_memfd lookup through VMCB installation.

>     KVM: SVM: Hold SRCU while reloading guest-owned VMSAs
>     
>     Take a dedicated KVM SRCU read lock around the guest-owned VMSA memslot
>     and guest_memfd lookup.  This makes the reload path self-contained and
>     prevents it from racing a userspace memslot replacement.
>     
>     Keep the lock through VMCB installation, and release it on all lookup
>     failure paths.
>     
>     Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c0ae34871be90..5bfe23790ba46 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4057,6 +4057,7 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	gfn_t gfn = gpa_to_gfn(gpa);
>  	unsigned long mmu_seq;
>  	kvm_pfn_t pfn;
> +	int idx;
>  
>  	lockdep_assert_held(&svm->sev_es.snp_vmsa_mutex);
>  
> @@ -4088,9 +4089,10 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	if (!VALID_PAGE(gpa))
>  		return;
>  
> -	slot = gfn_to_memslot(vcpu->kvm, gfn);
> +	idx = srcu_read_lock(&kvm->srcu);
> +	slot = gfn_to_memslot(kvm, gfn);
>  	if (!slot)
> -		return;
> +		goto out_unlock;
>  
>  	mmu_seq = kvm->mmu_invalidate_seq;
>  	smp_rmb();
> @@ -4099,8 +4101,8 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	 * The new VMSA will be private memory guest memory, so retrieve the
>  	 * PFN from the gmem backend.
>  	 */
> -	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, NULL))
> -		return;
> +	if (kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, NULL))
> +		goto out_unlock;
>  
>  	read_lock(&kvm->mmu_lock);
>  	/*
> @@ -4115,6 +4117,9 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	else
>  		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
>  	read_unlock(&kvm->mmu_lock);
> +
> +out_unlock:
> +	srcu_read_unlock(&kvm->srcu, idx);
>  }

[Severity: Medium]
Is taking a dedicated srcu_read_lock(&kvm->srcu) in __sev_snp_reload_vmsa()
necessary, and is the commit message accurate that this path previously raced
with userspace memslot replacement?

gfn_to_memslot() calls kvm_memslots() -> __kvm_memslots(), which already
requires kvm->srcu (or kvm->slots_lock) to be held:

include/linux/kvm_host.h:__kvm_memslots() {
	return srcu_dereference_check(kvm->memslots[as_id], &kvm->srcu,
			lockdep_is_held(&kvm->slots_lock) ||
			!refcount_read(&kvm->users_count));
}

Looking at all callers of __sev_snp_reload_vmsa(), kvm->srcu is already held
via kvm_vcpu_srcu_read_lock(vcpu):

1. sev_snp_reload_vmsa() is only called from vcpu_enter_guest() when
   servicing KVM_REQ_VMSA_PAGE_RELOAD:

arch/x86/kvm/x86.c:vcpu_enter_guest() {
		if (kvm_check_request(KVM_REQ_UPDATE_PROTECTED_GUEST_STATE, vcpu)) {
			kvm_vcpu_reset(vcpu, true);
			if (vcpu->arch.mp_state != KVM_MP_STATE_RUNNABLE) {
				r = 1;
				goto out;
			}
		}
		if (kvm_check_request(KVM_REQ_VMSA_PAGE_RELOAD, vcpu))
			kvm_x86_call(reload_vmsa)(vcpu);
}

2. sev_snp_init_protected_guest_state() is only called from
   sev_init_vmcb(svm, init_event) when init_event is true, which is reached
   via svm_vcpu_reset(vcpu, true) <- kvm_vcpu_reset(vcpu, true) from
   vcpu_enter_guest() (servicing KVM_REQ_UPDATE_PROTECTED_GUEST_STATE) and
   from kvm_apic_accept_events() (all callers of which hold kvm->srcu).

Even in the later commit 58d63c955c7c ("KVM: SEV: Add SNP vCPU state get and
set commands"), snp_set_vcpu_state() introduces its own helper,
sev_snp_install_guest_vmsa(), rather than calling __sev_snp_reload_vmsa().

Could this nested srcu_read_lock() and srcu_read_unlock() in
__sev_snp_reload_vmsa() be dropped, or replaced with an assertion that
kvm->srcu is held?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102114137.1198982-1-joro@8bytes.org?part=3

Re: [PATCH v3 3/8] KVM: SVM: Hold SRCU while reloading guest-owned VMSAs

[Jörg Rödel]

On Mon, Nov 02, 2026 at 11:53:36AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] KVM: SVM: redundant nested srcu_read_lock() in __sev_snp_reload_vmsa()

This is true, I will drop the patch, thanks.

[PATCH v3 4/8] KVM: SEV: Add direct VMSA capability

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Direct VMSA mode is currently selected implicitly when userspace installs
a guest-owned VMSA on any vCPU.  That makes the launch model a side effect
of per-vCPU state and applies it retroactively to every vCPU.

Add KVM_CAP_SNP_DIRECT_VMSA as a VM-scoped settable capability.  Require
userspace to enable it on an SNP VM before creating vCPUs, and reject
nonzero arguments.

When enabled, skip allocating KVM-owned VMSAs during vCPU creation and
skip generating and measuring KVM-owned VMSAs at launch finish.  Keep
legacy allocation and measurement behavior when it is not enabled.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm-x86-ops.h |  1 +
 arch/x86/include/asm/kvm_host.h    |  1 +
 arch/x86/kvm/svm/sev.c             | 28 ++++++++++++++++++++--------
 arch/x86/kvm/svm/svm.c             | 22 ++++++++++++++++++++++
 arch/x86/kvm/svm/svm.h             |  1 +
 arch/x86/kvm/x86.c                 |  5 +++++
 include/uapi/linux/kvm.h           |  1 +
 7 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 4dde4e3e608a..c853dc65fb53 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -23,6 +23,7 @@ KVM_X86_OP(vcpu_after_set_cpuid)
 KVM_X86_OP(vm_init)
 KVM_X86_OP_OPTIONAL(vm_destroy)
 KVM_X86_OP_OPTIONAL(vm_pre_destroy)
+KVM_X86_OP_OPTIONAL(enable_vm_cap)
 KVM_X86_OP_OPTIONAL_RET0(vcpu_precreate)
 KVM_X86_OP(vcpu_create)
 KVM_X86_OP(vcpu_free)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1ee4b127004d..be758e0d55aa 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1524,6 +1524,7 @@ struct kvm_x86_ops {
 	int (*vm_init)(struct kvm *kvm);
 	void (*vm_destroy)(struct kvm *kvm);
 	void (*vm_pre_destroy)(struct kvm *kvm);
+	int (*enable_vm_cap)(struct kvm *kvm, struct kvm_enable_cap *cap);
 
 	/* Create, but do not attach this VCPU */
 	int (*vcpu_precreate)(struct kvm *kvm);
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 9681ff0aa5d3..09e1c2da546d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2541,8 +2541,15 @@ 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);
-		u64 pfn = __pa(svm->sev_es.vmsa) >> PAGE_SHIFT;
+		u64 pfn;
+
+		if (sev->snp_direct_vmsa) {
+			if (!svm->sev_es.snp_has_guest_vmsa)
+				svm->vmcb->control.vmsa_pa = INVALID_PAGE;
+			goto protect_vcpu;
+		}
 
+		pfn = __pa(svm->sev_es.vmsa) >> PAGE_SHIFT;
 		ret = sev_es_sync_vmsa(svm);
 		if (ret)
 			goto out;
@@ -2562,6 +2569,7 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
 			goto out;
 		}
 
+protect_vcpu:
 		svm->vcpu.arch.guest_state_protected = true;
 		/*
 		 * SEV-ES (and thus SNP) guest mandates LBR Virtualization to
@@ -3583,6 +3591,9 @@ void sev_free_vcpu(struct kvm_vcpu *vcpu)
 	 * a guest-owned page. Transition the page to hypervisor state before
 	 * releasing it back to the system.
 	 */
+	if (!svm->sev_es.vmsa)
+		goto skip_vmsa_free;
+
 	if (is_sev_snp_guest(vcpu)) {
 		u64 pfn = __pa(svm->sev_es.vmsa) >> PAGE_SHIFT;
 
@@ -4078,13 +4089,9 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	vmcb_mark_all_dirty(svm->vmcb);
 
 	/*
-	 * From this point forward, the VMSA will always be a guest-mapped page
-	 * rather than the initial one allocated by KVM in svm->sev_es.vmsa. In
-	 * theory, svm->sev_es.vmsa could be free'd and cleaned up here, but
-	 * that involves cleanups like flushing caches, which would ideally be
-	 * handled during teardown rather than guest boot.  Deferring that also
-	 * allows the existing logic for SEV-ES VMSAs to be re-used with
-	 * minimal SNP-specific changes.
+	 * From this point forward, the VMSA will always be a guest-mapped page.
+	 * If KVM allocated an initial VMSA, keep it until teardown to defer
+	 * cache flushing and other cleanup out of the guest boot path.
 	 */
 	svm->sev_es.snp_has_guest_vmsa = true;
 
@@ -4975,12 +4982,15 @@ 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);
+	struct kvm_sev_info *sev = to_kvm_sev_info(vcpu->kvm);
 	struct page *vmsa_page;
 
 	mutex_init(&svm->sev_es.snp_vmsa_mutex);
 
 	if (!is_sev_es_guest(vcpu))
 		return 0;
+	if (is_sev_snp_guest(vcpu) && sev->snp_direct_vmsa)
+		goto init_vmsa_state;
 
 	/*
 	 * SEV-ES guests require a separate (from the VMCB) VMSA page used to
@@ -4991,6 +5001,8 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
 		return -ENOMEM;
 
 	svm->sev_es.vmsa = page_address(vmsa_page);
+
+init_vmsa_state:
 	svm->sev_es.snp_pending_vmsa_gpa = INVALID_PAGE;
 	svm->sev_es.snp_guest_vmsa_gpa = INVALID_PAGE;
 
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 136069dfcd44..d84109394115 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5458,6 +5458,27 @@ static void *svm_alloc_apic_backing_page(struct kvm_vcpu *vcpu)
 	return page_address(page);
 }
 
+static int svm_enable_vm_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
+{
+	switch (cap->cap) {
+#ifdef CONFIG_KVM_AMD_SEV
+	case KVM_CAP_SNP_DIRECT_VMSA:
+		if (memchr_inv(cap->args, 0, sizeof(cap->args)) ||
+		    kvm->arch.vm_type != KVM_X86_SNP_VM)
+			return -EINVAL;
+
+		guard(mutex)(&kvm->lock);
+		if (kvm->created_vcpus)
+			return -EINVAL;
+
+		to_kvm_sev_info(kvm)->snp_direct_vmsa = true;
+		return 0;
+#endif
+	default:
+		return -EINVAL;
+	}
+}
+
 struct kvm_x86_ops svm_x86_ops __initdata = {
 	.name = KBUILD_MODNAME,
 
@@ -5478,6 +5499,7 @@ struct kvm_x86_ops svm_x86_ops __initdata = {
 	.vm_init = svm_vm_init,
 	.vm_pre_destroy = avic_vm_pre_destroy,
 	.vm_destroy = svm_vm_destroy,
+	.enable_vm_cap = svm_enable_vm_cap,
 
 	.prepare_switch_to_guest = svm_prepare_switch_to_guest,
 	.vcpu_load = svm_vcpu_load,
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 6d0242966dc8..1a94d0c1ef91 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -122,6 +122,7 @@ struct kvm_sev_info {
 	struct mutex guest_req_mutex; /* Must acquire before using bounce buffers */
 	cpumask_var_t have_run_cpus; /* CPUs that have done VMRUN for this VM. */
 	bool snp_certs_enabled;	/* SNP certificate-fetching support. */
+	bool snp_direct_vmsa;	/* Userspace provides and measures VMSA pages. */
 };
 #endif
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 32563a91fba9..0a96bdc7f869 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2415,6 +2415,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_VM_TYPES:
 		r = kvm_caps.supported_vm_types;
 		break;
+	case KVM_CAP_SNP_DIRECT_VMSA:
+		r = !!(kvm_caps.supported_vm_types & BIT(KVM_X86_SNP_VM));
+		break;
 	case KVM_CAP_READONLY_MEM:
 		r = kvm ? kvm_arch_has_readonly_mem(kvm) : 1;
 		break;
@@ -4254,6 +4257,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 	}
 	default:
 		r = -EINVAL;
+		if (kvm_x86_ops.enable_vm_cap)
+			r = kvm_x86_call(enable_vm_cap)(kvm, cap);
 		break;
 	}
 	return r;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 8dff2fc1972e..005c25263030 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1000,6 +1000,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_PPC_COMPAT_CAPS 250
 #define KVM_CAP_ARM_PMU_V3_STRICT 251
 #define KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES 252
+#define KVM_CAP_SNP_DIRECT_VMSA 253
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
-- 
2.53.0

Re: [PATCH v3 4/8] KVM: SEV: Add direct VMSA capability

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] KVM: SEV: snp_launch_update_vmsa() skips debug and state checks on direct VMSA
- [Low] KVM: SVM: unscoped guard(mutex) in switch case in svm_enable_vm_cap()
--

commit ab904a5d7e5b93c050ee3612b960971e0dec53a9
Author: Joerg Roedel <joerg.roedel@amd.com>

KVM: SEV: Add direct VMSA capability

This commit adds the VM-scoped KVM_CAP_SNP_DIRECT_VMSA capability so
userspace can explicitly opt into direct VMSA mode on an SNP VM before
creating vCPUs. When enabled, KVM skips allocating KVM-owned VMSAs during
vCPU creation and skips syncing and measuring KVM-owned VMSAs at launch
finish.

[ ... ]

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 5bfe23790ba46..5638786c1d03d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2541,8 +2541,15 @@ 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);
> -		u64 pfn = __pa(svm->sev_es.vmsa) >> PAGE_SHIFT;
> +		u64 pfn;
> +
> +		if (sev->snp_direct_vmsa) {
> +			if (!svm->sev_es.snp_has_guest_vmsa)
> +				svm->vmcb->control.vmsa_pa = INVALID_PAGE;
> +			goto protect_vcpu;
> +		}
>  
> +		pfn = __pa(svm->sev_es.vmsa) >> PAGE_SHIFT;
>  		ret = sev_es_sync_vmsa(svm);
>  		if (ret)
>  			goto out;

[Severity: Medium]
When sev->snp_direct_vmsa is set in snp_launch_update_vmsa(), jumping
directly to protect_vcpu skips sev_es_sync_vmsa(), which normally performs
these pre-launch checks:

arch/x86/kvm/svm/sev.c:sev_es_sync_vmsa() {
	if (vcpu->arch.guest_state_protected)
		return -EINVAL;

	/* Check some debug related fields before encrypting the VMSA */
	if (svm->vcpu.guest_debug || (svm->vmcb->save.dr7 & ~DR7_FIXED_1))
		return -EINVAL;
}

Because kvm_arch_vcpu_ioctl_set_guest_debug() and
kvm_vcpu_ioctl_x86_set_debugregs() only check
vcpu->arch.guest_state_protected, userspace can invoke KVM_SET_GUEST_DEBUG
or KVM_SET_DEBUGREGS on an SNP vCPU before calling KVM_SEV_SNP_LAUNCH_FINISH.

Could skipping these checks allow an SNP guest with KVM_CAP_SNP_DIRECT_VMSA
enabled to finalize launch with unsupported host guest_debug state (such as
BP_VECTOR intercepts, single-step, hardware breakpoint switching via
KVM_DEBUGREG_BP_ENABLED, or APICV_INHIBIT_REASON_BLOCKIRQ) locked in once
vcpu->arch.guest_state_protected is set, or allow snp_launch_update_vmsa() to
run again on an already-protected vCPU?

[ ... ]

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 136069dfcd44f..d841093941156 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -5458,6 +5458,27 @@ static void *svm_alloc_apic_backing_page(struct kvm_vcpu *vcpu)
>  	return page_address(page);
>  }
>  
> +static int svm_enable_vm_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
> +{
> +	switch (cap->cap) {
> +#ifdef CONFIG_KVM_AMD_SEV
> +	case KVM_CAP_SNP_DIRECT_VMSA:
> +		if (memchr_inv(cap->args, 0, sizeof(cap->args)) ||
> +		    kvm->arch.vm_type != KVM_X86_SNP_VM)
> +			return -EINVAL;
> +
> +		guard(mutex)(&kvm->lock);
> +		if (kvm->created_vcpus)
> +			return -EINVAL;
> +
> +		to_kvm_sev_info(kvm)->snp_direct_vmsa = true;
> +		return 0;
> +#endif
> +	default:
> +		return -EINVAL;
> +	}
> +}

[Severity: Low]
As reported by Sashiko on v2 [1], in svm_enable_vm_cap(), guard(mutex)
declares a cleanup-attributed variable inside the switch block without
enclosing braces around the KVM_CAP_SNP_DIRECT_VMSA case.

When cap->cap does not match KVM_CAP_SNP_DIRECT_VMSA, jumping to the default
label bypasses initialization of the guard variable while remaining in its
scope, causing a build failure with CONFIG_KVM_AMD_SEV enabled. Should the
case body be wrapped in braces or use scoped_guard()?

[1] https://lore.kernel.org/r/20260908103338.427254-5-joro@8bytes.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102114137.1198982-1-joro@8bytes.org?part=4

[PATCH v3 5/8] KVM: SEV: Allow VMSA pages in SNP launch updates

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Pass the firmware VMSA page type through KVM_SEV_SNP_LAUNCH_UPDATE.
Only accept VMSA pages when userspace has enabled
KVM_CAP_SNP_DIRECT_VMSA.
Require each request to describe exactly one 4-KiB VMSA page.  Allow
repeated requests and keep VMSA creation independent of association with a
vCPU.

The VMSA's VMPL and SEV features define its execution context.  They must
agree with KVM's VM-wide configuration.  Before passing a VMSA to firmware,
require VMPL 0.  Require sev_features to exactly match the VM's configured
VMSA features.  Treat the remaining contents as guest-owned data.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/uapi/asm/kvm.h |  1 +
 arch/x86/kvm/svm/sev.c          | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 00ce265391e1..f3c389d3c70e 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 09e1c2da546d..4c659c063be2 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2365,6 +2365,7 @@ struct sev_gmem_populate_args {
 	__u8 type;
 	int sev_fd;
 	int fw_error;
+	bool vmsa_invalid;
 };
 
 static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
@@ -2395,6 +2396,19 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
 		kunmap_local(src_vaddr);
 	}
 
+	if (sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_VMSA) {
+		struct sev_es_save_area *vmsa = kmap_local_pfn(pfn);
+
+		if (vmsa->vmpl || vmsa->sev_features != sev->vmsa_features) {
+			sev_populate_args->vmsa_invalid = true;
+			ret = -EINVAL;
+		}
+
+		kunmap_local(vmsa);
+		if (ret)
+			goto out;
+	}
+
 	ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
 			       sev_get_asid(kvm), true);
 	if (ret)
@@ -2458,7 +2472,10 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 		 params.gfn_start, params.len, params.type, params.flags, params.uaddr);
 
 	if (!params.len || !PAGE_ALIGNED(params.len) || params.flags ||
+	    (params.type == KVM_SEV_SNP_PAGE_TYPE_VMSA &&
+	     (!sev->snp_direct_vmsa || params.len != PAGE_SIZE)) ||
 	    (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 &&
@@ -2507,6 +2524,9 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 				  params.type == KVM_SEV_SNP_PAGE_TYPE_CPUID,
 				  sev_gmem_post_populate, &sev_populate_args);
 	if (count < 0) {
+		if (sev_populate_args.vmsa_invalid)
+			return -EINVAL;
+
 		argp->error = sev_populate_args.fw_error;
 		pr_debug("%s: kvm_gmem_populate failed, ret %ld (fw_error %d)\n",
 			 __func__, count, argp->error);
-- 
2.53.0

[PATCH v3 6/8] KVM: SEV: Add SNP vCPU state get and set commands

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Add KVM_SEV_SNP_GET_VCPU_STATE and KVM_SEV_SNP_SET_VCPU_STATE as
vCPU-scoped KVM_MEMORY_ENCRYPT_OP commands and advertise
KVM_CAP_SNP_VCPU_STATE.  The shared, extensible argument reports the VMSA
and GHCB GPAs and has validity bits for both addresses.

For SET, validate the argument, launch ordering, VMSA alignment, and that a
valid VMSA is backed by guest_memfd.  Install or invalidate the VMSA and
GHCB as requested.  GET returns the currently tracked guest-owned VMSA GPA
and the GHCB address.

Require KVM_CAP_SNP_DIRECT_VMSA for SET.  The VM-scoped capability fixes
the launch model before vCPU creation; the per-vCPU command only selects
the guest-owned VMSA and GHCB addresses for that vCPU.

Route the commands through the locked vCPU ioctl hook.  The vCPU mutex
serializes the commands against launch finalization, which locks all vCPUs
before inspecting their state.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/uapi/asm/kvm.h |  12 +++
 arch/x86/kvm/svm/sev.c          | 162 ++++++++++++++++++++++++++++++++
 arch/x86/kvm/svm/svm.c          |   1 +
 arch/x86/kvm/svm/svm.h          |   1 +
 arch/x86/kvm/x86.c              |   1 +
 include/uapi/linux/kvm.h        |   1 +
 6 files changed, 178 insertions(+)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index f3c389d3c70e..f65d78b87f7e 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -746,6 +746,8 @@ enum sev_cmd_id {
 	KVM_SEV_SNP_LAUNCH_UPDATE,
 	KVM_SEV_SNP_LAUNCH_FINISH,
 	KVM_SEV_SNP_ENABLE_REQ_CERTS,
+	KVM_SEV_SNP_GET_VCPU_STATE,
+	KVM_SEV_SNP_SET_VCPU_STATE,
 
 	KVM_SEV_NR_MAX,
 };
@@ -902,6 +904,16 @@ struct kvm_sev_snp_launch_update {
 	__u64 pad2[4];
 };
 
+#define KVM_SEV_SNP_VCPU_STATE_VMSA_VALID	_BITULL(0)
+#define KVM_SEV_SNP_VCPU_STATE_GHCB_VALID	_BITULL(1)
+
+struct kvm_sev_snp_vcpu_state {
+	__u64 valid_fields;
+	__u64 vmsa_gpa;
+	__u64 ghcb_gpa;
+	__u64 pad[5];
+};
+
 #define KVM_SEV_SNP_ID_BLOCK_SIZE	96
 #define KVM_SEV_SNP_ID_AUTH_SIZE	4096
 #define KVM_SEV_SNP_FINISH_DATA_SIZE	32
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 4c659c063be2..dfda090eb793 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2544,6 +2544,8 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return 0;
 }
 
+static int sev_snp_install_guest_vmsa(struct vcpu_svm *svm, gpa_t gpa);
+
 static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
 {
 	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
@@ -2606,6 +2608,89 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int snp_get_vcpu_state(struct kvm_vcpu *vcpu,
+			      struct kvm_sev_cmd *argp)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	struct kvm *kvm = vcpu->kvm;
+	struct kvm_sev_snp_vcpu_state state = {};
+
+	if (!is_sev_snp_guest(vcpu))
+		return -ENOTTY;
+	if (!to_kvm_sev_info(kvm)->snp_context)
+		return -EINVAL;
+
+	guard(mutex)(&svm->sev_es.snp_vmsa_mutex);
+
+	if (VALID_PAGE(svm->sev_es.snp_guest_vmsa_gpa) &&
+	    VALID_PAGE(svm->vmcb->control.vmsa_pa)) {
+		state.vmsa_gpa = svm->sev_es.snp_guest_vmsa_gpa;
+		state.valid_fields |= KVM_SEV_SNP_VCPU_STATE_VMSA_VALID;
+	}
+
+	if (VALID_PAGE(svm->vmcb->control.ghcb_gpa)) {
+		state.ghcb_gpa = svm->vmcb->control.ghcb_gpa;
+		state.valid_fields |= KVM_SEV_SNP_VCPU_STATE_GHCB_VALID;
+	}
+
+	if (copy_to_user(u64_to_user_ptr(argp->data), &state, sizeof(state)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int snp_set_vcpu_state(struct kvm_vcpu *vcpu,
+			      struct kvm_sev_cmd *argp)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	struct kvm *kvm = vcpu->kvm;
+	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
+	struct kvm_sev_snp_vcpu_state state;
+	int ret;
+
+	if (!is_sev_snp_guest(vcpu))
+		return -ENOTTY;
+	if (!sev->snp_direct_vmsa)
+		return -EINVAL;
+	if (!sev->snp_context || kvm->arch.pre_fault_allowed)
+		return -EINVAL;
+
+	if (copy_from_user(&state, u64_to_user_ptr(argp->data), sizeof(state)))
+		return -EFAULT;
+
+	if (memchr_inv(state.pad, 0, sizeof(state.pad)) ||
+	    state.valid_fields & ~(KVM_SEV_SNP_VCPU_STATE_VMSA_VALID |
+				   KVM_SEV_SNP_VCPU_STATE_GHCB_VALID))
+		return -EINVAL;
+
+	if (state.valid_fields & KVM_SEV_SNP_VCPU_STATE_VMSA_VALID) {
+		if (!PAGE_ALIGNED(state.vmsa_gpa) ||
+		    !page_address_valid(vcpu, state.vmsa_gpa) ||
+		    IS_ALIGNED(state.vmsa_gpa, PMD_SIZE))
+			return -EINVAL;
+	}
+
+	guard(mutex)(&svm->sev_es.snp_vmsa_mutex);
+
+	if (state.valid_fields & KVM_SEV_SNP_VCPU_STATE_VMSA_VALID) {
+		ret = sev_snp_install_guest_vmsa(svm, state.vmsa_gpa);
+		if (ret)
+			return ret;
+	} else {
+		svm->sev_es.snp_has_guest_vmsa = true;
+		svm->sev_es.snp_guest_vmsa_gpa = INVALID_PAGE;
+		svm->vmcb->control.vmsa_pa = INVALID_PAGE;
+	}
+
+	if (state.valid_fields & KVM_SEV_SNP_VCPU_STATE_GHCB_VALID)
+		svm->vmcb->control.ghcb_gpa = state.ghcb_gpa;
+	else
+		svm->vmcb->control.ghcb_gpa = INVALID_PAGE;
+
+	vmcb_mark_all_dirty(svm->vmcb);
+	return 0;
+}
+
 static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
 {
 	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
@@ -2802,6 +2887,35 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
 	return r;
 }
 
+int sev_vcpu_mem_enc_ioctl(struct kvm_vcpu *vcpu, void __user *argp)
+{
+	struct kvm_sev_cmd sev_cmd;
+	int ret;
+
+	if (!sev_enabled)
+		return -ENOTTY;
+	if (!argp)
+		return -EINVAL;
+	if (copy_from_user(&sev_cmd, argp, sizeof(sev_cmd)))
+		return -EFAULT;
+
+	switch (sev_cmd.id) {
+	case KVM_SEV_SNP_GET_VCPU_STATE:
+		ret = snp_get_vcpu_state(vcpu, &sev_cmd);
+		break;
+	case KVM_SEV_SNP_SET_VCPU_STATE:
+		ret = snp_set_vcpu_state(vcpu, &sev_cmd);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (copy_to_user(argp, &sev_cmd, sizeof(sev_cmd)))
+		return -EFAULT;
+
+	return ret;
+}
+
 int sev_mem_enc_register_region(struct kvm *kvm,
 				struct kvm_enc_region *range)
 {
@@ -4082,6 +4196,54 @@ static int snp_begin_psc(struct vcpu_svm *svm)
 	return snp_do_psc(svm);
 }
 
+/*
+ * Install a guest-owned VMSA.  The caller must serialize against AP creation
+ * and destruction with snp_vmsa_mutex.
+ */
+static int sev_snp_install_guest_vmsa(struct vcpu_svm *svm, gpa_t gpa)
+{
+	struct kvm *kvm = svm->vcpu.kvm;
+	struct kvm_memory_slot *slot;
+	unsigned long mmu_seq;
+	kvm_pfn_t pfn;
+	gfn_t gfn;
+	int idx;
+	int ret;
+
+	lockdep_assert_held(&svm->sev_es.snp_vmsa_mutex);
+
+	gfn = gpa_to_gfn(gpa);
+	idx = srcu_read_lock(&kvm->srcu);
+	slot = gfn_to_memslot(kvm, gfn);
+	if (!slot) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	mmu_seq = kvm->mmu_invalidate_seq;
+	/* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */
+	smp_rmb();
+
+	/* Guest-owned VMSAs are backed by guest_memfd private memory. */
+	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, NULL);
+	if (ret)
+		goto out_unlock;
+
+	read_lock(&kvm->mmu_lock);
+	if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn)) {
+		ret = -EAGAIN;
+	} else {
+		svm->sev_es.snp_has_guest_vmsa = true;
+		WRITE_ONCE(svm->sev_es.snp_guest_vmsa_gpa, gpa);
+		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
+	}
+	read_unlock(&kvm->mmu_lock);
+
+out_unlock:
+	srcu_read_unlock(&kvm->srcu, idx);
+	return ret;
+}
+
 static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d84109394115..a16c3fd6bc5f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5601,6 +5601,7 @@ struct kvm_x86_ops svm_x86_ops __initdata = {
 	.vcpu_needs_initialization = sev_vcpu_needs_initialization,
 	.dev_get_attr = sev_dev_get_attr,
 	.mem_enc_ioctl = sev_mem_enc_ioctl,
+	.vcpu_mem_enc_ioctl = sev_vcpu_mem_enc_ioctl,
 	.mem_enc_register_region = sev_mem_enc_register_region,
 	.mem_enc_unregister_region = sev_mem_enc_unregister_region,
 	.guest_memory_reclaimed = sev_guest_memory_reclaimed,
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 1a94d0c1ef91..11b65a9d8ffc 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -994,6 +994,7 @@ void sev_es_unmap_ghcb(struct vcpu_svm *svm);
 #ifdef CONFIG_KVM_AMD_SEV
 bool sev_vcpu_needs_initialization(struct kvm_vcpu *vcpu);
 int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp);
+int sev_vcpu_mem_enc_ioctl(struct kvm_vcpu *vcpu, void __user *argp);
 int sev_mem_enc_register_region(struct kvm *kvm,
 				struct kvm_enc_region *range);
 int sev_mem_enc_unregister_region(struct kvm *kvm,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0a96bdc7f869..17ababa60844 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2415,6 +2415,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_VM_TYPES:
 		r = kvm_caps.supported_vm_types;
 		break;
+	case KVM_CAP_SNP_VCPU_STATE:
 	case KVM_CAP_SNP_DIRECT_VMSA:
 		r = !!(kvm_caps.supported_vm_types & BIT(KVM_X86_SNP_VM));
 		break;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 005c25263030..00259ec260ce 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1001,6 +1001,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_ARM_PMU_V3_STRICT 251
 #define KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES 252
 #define KVM_CAP_SNP_DIRECT_VMSA 253
+#define KVM_CAP_SNP_VCPU_STATE 254
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
-- 
2.53.0

Re: [PATCH v3 6/8] KVM: SEV: Add SNP vCPU state get and set commands

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] KVM: SEV: misuse of INVALID_PAGE for ghcb_gpa in snp_set/get_vcpu_state()
- [High] KVM: SEV: premature gmem fault-in and RMP update in snp_set_vcpu_state()
- [High] KVM: SEV: missing guest_state_protected check in snp_set_vcpu_state()
- [High] KVM: SEV: setting vmsa_pa in snp_set_vcpu_state() bypasses pre_sev_run()
- [Medium] KVM: SEV: unlocked sev->snp_context read in snp_get/set_vcpu_state()
- [Medium] KVM: SEV: stale VMSA state in snp_get_vcpu_state() on pending reload request
- [Medium] KVM: SEV: snp_get_vcpu_state() hides VMSA GPA when VMSA reload is pending
--

commit 58d63c955c7c0bdc5747b6eafd39cb3beae4ba9d
Author: Joerg Roedel <joerg.roedel@amd.com>

KVM: SEV: Add SNP vCPU state get and set commands

This commit adds KVM_SEV_SNP_GET_VCPU_STATE and KVM_SEV_SNP_SET_VCPU_STATE
as vCPU-scoped KVM_MEMORY_ENCRYPT_OP commands and advertises
KVM_CAP_SNP_VCPU_STATE so userspace can configure and query guest-owned VMSA
and GHCB GPAs per vCPU.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 974651a70a7dd..ff696e21fe41b 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c

[ ... ]

> @@ -2606,6 +2608,89 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	return ret;
>  }
>  
> +static int snp_get_vcpu_state(struct kvm_vcpu *vcpu,
> +			      struct kvm_sev_cmd *argp)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	struct kvm *kvm = vcpu->kvm;
> +	struct kvm_sev_snp_vcpu_state state = {};
> +
> +	if (!is_sev_snp_guest(vcpu))
> +		return -ENOTTY;
> +	if (!to_kvm_sev_info(kvm)->snp_context)
> +		return -EINVAL;

[Severity: Medium]
Is it safe for snp_get_vcpu_state() and snp_set_vcpu_state() to read
sev->snp_context under vcpu->mutex without holding kvm->lock?

In snp_launch_start(), sev->snp_context is assigned under kvm->lock before
SEV_CMD_SNP_LAUNCH_START and snp_bind_asid() execute, and cleared via
snp_decommission_context() if either step fails:

arch/x86/kvm/svm/sev.c:snp_launch_start() {
	...
	sev->snp_context = snp_context_create(kvm, argp);
	if (!sev->snp_context)
		return -ENOTTY;
	...
e_free_context:
	snp_decommission_context(kvm);
	return rc;
}

If userspace invokes KVM_SEV_SNP_GET_VCPU_STATE or
KVM_SEV_SNP_SET_VCPU_STATE concurrently with KVM_SEV_SNP_LAUNCH_START, can
snp_set_vcpu_state() observe a transiently non-NULL sev->snp_context and
install state (including calling kvm_gmem_get_pfn() ->
sev_gmem_make_private()) before snp_bind_asid() has bound the asid or while
snp_launch_start() is failing and decommissioning the context?

> +
> +	guard(mutex)(&svm->sev_es.snp_vmsa_mutex);
> +
> +	if (VALID_PAGE(svm->sev_es.snp_guest_vmsa_gpa) &&
> +	    VALID_PAGE(svm->vmcb->control.vmsa_pa)) {
> +		state.vmsa_gpa = svm->sev_es.snp_guest_vmsa_gpa;
> +		state.valid_fields |= KVM_SEV_SNP_VCPU_STATE_VMSA_VALID;
> +	}

[Severity: Medium]
As reported by Sashiko on v2 [1], does checking
VALID_PAGE(svm->vmcb->control.vmsa_pa) here cause snp_get_vcpu_state() to
hide a valid snp_guest_vmsa_gpa whenever a deferred reload is pending?

Every path that clears or fails to install a guest-owned vmsa already sets
svm->sev_es.snp_guest_vmsa_gpa to INVALID_PAGE. The only case where
VALID_PAGE(svm->sev_es.snp_guest_vmsa_gpa) is true while
VALID_PAGE(svm->vmcb->control.vmsa_pa) is false is in
__sev_snp_reload_vmsa() when mmu_invalidate_retry_gfn() triggers a deferred
reload via KVM_REQ_VMSA_PAGE_RELOAD:

arch/x86/kvm/svm/sev.c:__sev_snp_reload_vmsa() {
	...
	svm->sev_es.snp_guest_vmsa_gpa = gpa;
	if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
		kvm_make_request(KVM_REQ_VMSA_PAGE_RELOAD, vcpu);
	else
		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
}

If the vCPU exits to userspace before vcpu_enter_guest() services
KVM_REQ_VMSA_PAGE_RELOAD, won't KVM_SEV_SNP_GET_VCPU_STATE omit
KVM_SEV_SNP_VCPU_STATE_VMSA_VALID and report vmsa_gpa as 0?

[Severity: Medium]
Conversely, what happens if the backing guest_memfd range or memslot is
invalidated while the vCPU is in userspace?

When sev_gmem_invalidate_range() runs, it reads snp_guest_vmsa_gpa via
READ_ONCE() and queues KVM_REQ_VMSA_PAGE_RELOAD without clearing
snp_guest_vmsa_gpa or vmsa_pa until the next KVM_RUN calls
sev_snp_reload_vmsa() in vcpu_enter_guest():

arch/x86/kvm/svm/sev.c:sev_gmem_invalidate_range() {
	...
	gpa_t gpa = READ_ONCE(to_svm(vcpu)->sev_es.snp_guest_vmsa_gpa);

	if (VALID_PAGE(gpa) &&
	    gpa_to_gfn(gpa) >= range->start &&
	    gpa_to_gfn(gpa) < range->end)
		kvm_make_request_and_kick(KVM_REQ_VMSA_PAGE_RELOAD, vcpu);
}

Because snp_get_vcpu_state() does not service pending
KVM_REQ_VMSA_PAGE_RELOAD (or KVM_REQ_UPDATE_PROTECTED_GUEST_STATE) requests,
won't KVM_SEV_SNP_GET_VCPU_STATE report stale vmsa state if called before
KVM_RUN?

Also, in snp_set_vcpu_state(), the else branch clears
svm->sev_es.snp_guest_vmsa_gpa = INVALID_PAGE outside mmu_lock without
WRITE_ONCE(), and neither branch clears a pending KVM_REQ_VMSA_PAGE_RELOAD
request. Should snp_set_vcpu_state() use WRITE_ONCE() and clear
KVM_REQ_VMSA_PAGE_RELOAD when updating or invalidating the vmsa?

> +
> +	if (VALID_PAGE(svm->vmcb->control.ghcb_gpa)) {
> +		state.ghcb_gpa = svm->vmcb->control.ghcb_gpa;
> +		state.valid_fields |= KVM_SEV_SNP_VCPU_STATE_GHCB_VALID;
> +	}
> +
> +	if (copy_to_user(u64_to_user_ptr(argp->data), &state, sizeof(state)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int snp_set_vcpu_state(struct kvm_vcpu *vcpu,
> +			      struct kvm_sev_cmd *argp)
> +{
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +	struct kvm *kvm = vcpu->kvm;
> +	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> +	struct kvm_sev_snp_vcpu_state state;
> +	int ret;
> +
> +	if (!is_sev_snp_guest(vcpu))
> +		return -ENOTTY;
> +	if (!sev->snp_direct_vmsa)
> +		return -EINVAL;
> +	if (!sev->snp_context || kvm->arch.pre_fault_allowed)
> +		return -EINVAL;

[Severity: High]
Does checking kvm->arch.pre_fault_allowed under vcpu->mutex actually
serialize snp_set_vcpu_state() against launch finalization?

In snp_launch_finish(), snp_launch_update_vmsa() locks all vCPUs, sets
vcpu->arch.guest_state_protected = true, and immediately unlocks all vCPUs
before snp_launch_finish() copies the user blobs, issues
SEV_CMD_SNP_LAUNCH_FINISH, and sets kvm->arch.pre_fault_allowed = true:

arch/x86/kvm/svm/sev.c:snp_launch_update_vmsa() {
	...
protect_vcpu:
		svm->vcpu.arch.guest_state_protected = true;
		svm_enable_lbrv(vcpu);
	}

out:
	kvm_unlock_all_vcpus(kvm);
	return ret;
}

Once kvm_unlock_all_vcpus() releases the vCPU mutexes (either while
SEV_CMD_SNP_LAUNCH_FINISH is in flight, or if snp_launch_finish() fails
before setting pre_fault_allowed), another thread calling
KVM_SEV_SNP_SET_VCPU_STATE can acquire vcpu->mutex, see
kvm->arch.pre_fault_allowed == false, and overwrite vmsa_pa,
snp_guest_vmsa_gpa, and ghcb_gpa on an already-protected vCPU.

Should snp_set_vcpu_state() check vcpu->arch.guest_state_protected instead?

> +
> +	if (copy_from_user(&state, u64_to_user_ptr(argp->data), sizeof(state)))
> +		return -EFAULT;
> +
> +	if (memchr_inv(state.pad, 0, sizeof(state.pad)) ||
> +	    state.valid_fields & ~(KVM_SEV_SNP_VCPU_STATE_VMSA_VALID |
> +				   KVM_SEV_SNP_VCPU_STATE_GHCB_VALID))
> +		return -EINVAL;
> +
> +	if (state.valid_fields & KVM_SEV_SNP_VCPU_STATE_VMSA_VALID) {
> +		if (!PAGE_ALIGNED(state.vmsa_gpa) ||
> +		    !page_address_valid(vcpu, state.vmsa_gpa) ||
> +		    IS_ALIGNED(state.vmsa_gpa, PMD_SIZE))
> +			return -EINVAL;
> +	}
> +
> +	guard(mutex)(&svm->sev_es.snp_vmsa_mutex);
> +
> +	if (state.valid_fields & KVM_SEV_SNP_VCPU_STATE_VMSA_VALID) {
> +		ret = sev_snp_install_guest_vmsa(svm, state.vmsa_gpa);
> +		if (ret)
> +			return ret;
> +	} else {
> +		svm->sev_es.snp_has_guest_vmsa = true;
> +		svm->sev_es.snp_guest_vmsa_gpa = INVALID_PAGE;
> +		svm->vmcb->control.vmsa_pa = INVALID_PAGE;
> +	}
> +
> +	if (state.valid_fields & KVM_SEV_SNP_VCPU_STATE_GHCB_VALID)
> +		svm->vmcb->control.ghcb_gpa = state.ghcb_gpa;
> +	else
> +		svm->vmcb->control.ghcb_gpa = INVALID_PAGE;

[Severity: High]
Does using INVALID_PAGE (~0ULL) for svm->vmcb->control.ghcb_gpa break GHCB
protocol handling and state reporting in snp_set_vcpu_state() and
snp_get_vcpu_state()?

svm->vmcb->control.ghcb_gpa backs MSR_AMD64_SEV_ES_GHCB, which is
initialized during vCPU reset in sev_es_init_vmcb() to the architectural
GHCB_MSR_SEV_INFO value:

arch/x86/kvm/svm/sev.c:sev_es_init_vmcb() {
	...
	if (!init_event)
		set_ghcb_msr(svm, GHCB_MSR_SEV_INFO((__u64)sev->ghcb_version,
						    GHCB_VERSION_MIN,
						    sev_enc_bit));
}

And sev_handle_vmgexit() inspects control->ghcb_gpa as follows:

arch/x86/kvm/svm/sev.c:sev_handle_vmgexit() {
	...
	ghcb_gpa = control->ghcb_gpa;
	if (ghcb_gpa & GHCB_MSR_INFO_MASK)
		return sev_handle_vmgexit_msr_protocol(svm);

	if (!ghcb_gpa) {
		...
	}

	if (is_sev_snp_guest(vcpu) &&
	    !ghcb_gpa_is_registered(svm, control->ghcb_gpa)) {
		...
		svm_vmgexit_bad_input(svm, GHCB_ERR_NOT_REGISTERED);
		return 1;
	}
}

This appears to cause three regressions:

1. When KVM_SEV_SNP_SET_VCPU_STATE is called without
KVM_SEV_SNP_VCPU_STATE_GHCB_VALID (for example, to set only the vmsa),
setting control.ghcb_gpa = INVALID_PAGE overwrites the GHCB_MSR_SEV_INFO
reset value with 0xffffffffffffffffULL, which has low 12 bits 0xfff and will
be misinterpreted by sev_handle_vmgexit() as a GHCB MSR protocol request.

2. When KVM_SEV_SNP_SET_VCPU_STATE is called with
KVM_SEV_SNP_VCPU_STATE_GHCB_VALID, snp_set_vcpu_state() writes
svm->vmcb->control.ghcb_gpa = state.ghcb_gpa without updating
svm->sev_es.ghcb_registered_gpa, so subsequent non-MSR VMGEXITs fail
ghcb_gpa_is_registered() with GHCB_ERR_NOT_REGISTERED.

3. In snp_get_vcpu_state(), VALID_PAGE(svm->vmcb->control.ghcb_gpa) only
tests != INVALID_PAGE, which evaluates to true for the initial
GHCB_MSR_SEV_INFO reset value, 0, and any GHCB MSR protocol value.

> +
> +	vmcb_mark_all_dirty(svm->vmcb);
> +	return 0;
> +}

[ ... ]

> @@ -4080,6 +4194,54 @@ static int snp_begin_psc(struct vcpu_svm *svm)
>  	return snp_do_psc(svm);
>  }
>  
> +/*
> + * Install a guest-owned VMSA.  The caller must serialize against AP creation
> + * and destruction with snp_vmsa_mutex.
> + */
> +static int sev_snp_install_guest_vmsa(struct vcpu_svm *svm, gpa_t gpa)
> +{
> +	struct kvm *kvm = svm->vcpu.kvm;
> +	struct kvm_memory_slot *slot;
> +	unsigned long mmu_seq;
> +	kvm_pfn_t pfn;
> +	gfn_t gfn;
> +	int idx;
> +	int ret;
> +
> +	lockdep_assert_held(&svm->sev_es.snp_vmsa_mutex);
> +
> +	gfn = gpa_to_gfn(gpa);
> +	idx = srcu_read_lock(&kvm->srcu);
> +	slot = gfn_to_memslot(kvm, gfn);
> +	if (!slot) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	mmu_seq = kvm->mmu_invalidate_seq;
> +	/* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */
> +	smp_rmb();
> +
> +	/* Guest-owned VMSAs are backed by guest_memfd private memory. */
> +	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, NULL);
> +	if (ret)
> +		goto out_unlock;

[Severity: High]
What happens if userspace calls KVM_SEV_SNP_SET_VCPU_STATE before populating
the vmsa page with KVM_SEV_SNP_LAUNCH_UPDATE?

In virt/kvm/guest_memfd.c, kvm_gmem_get_pfn() faults in an unpopulated folio
via FGP_CREAT and, if kvm_gmem_is_private_mem() is true, calls
kvm_arch_gmem_make_private() -> sev_gmem_make_private() ->
rmp_make_private(pfn, gpa, PG_LEVEL_4K, sev->asid, false):

virt/kvm/guest_memfd.c:kvm_gmem_get_pfn() {
	...
	folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
	...
	if (kvm_arch_has_gmem_convert() &&
	    kvm_gmem_is_private_mem(file_inode(file), index))
		r = kvm_arch_gmem_make_private(kvm, gfn, *pfn,
					       (kvm_pfn_t)1 << *max_order);
}

That transitions the unpopulated page's rmp entry to an unmeasured private
data page (Assigned=1, VMSA=0), causing a subsequent
KVM_SEV_SNP_LAUNCH_UPDATE on that gfn to fail in sev_gmem_post_populate()
with -EEXIST when snp_lookup_rmpentry() sees assigned == true.

In addition, kvm_gmem_get_pfn() returns -EFAULT if the memslot does not have
a guest_memfd (whereas !slot returns -EINVAL), and succeeds without error if
the gfn is currently marked shared in guest_memfd. Should
sev_snp_install_guest_vmsa() verify kvm_slot_has_gmem(slot),
kvm_mem_is_private(kvm, gfn), and that the page has already been populated as
a vmsa page rather than faulting it in via kvm_gmem_get_pfn()?

> +
> +	read_lock(&kvm->mmu_lock);
> +	if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn)) {
> +		ret = -EAGAIN;
> +	} else {
> +		svm->sev_es.snp_has_guest_vmsa = true;
> +		WRITE_ONCE(svm->sev_es.snp_guest_vmsa_gpa, gpa);
> +		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
> +	}

[Severity: High]
Does setting svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn) here allow
userspace to invoke KVM_RUN before KVM_SEV_SNP_LAUNCH_FINISH completes?

In direct-vmsa mode, KVM_SEV_SNP_LAUNCH_UPDATE with
KVM_SEV_SNP_PAGE_TYPE_VMSA already marks the page as a valid vmsa in the rmp.
Once sev_snp_install_guest_vmsa() sets svm->vmcb->control.vmsa_pa to a valid
address, neither sev_vcpu_needs_initialization() nor pre_sev_run() blocks
KVM_RUN before KVM_SEV_SNP_LAUNCH_FINISH:

arch/x86/kvm/svm/sev.c:sev_vcpu_needs_initialization() {
	return to_kvm_sev_info(vcpu->kvm)->need_init;
}

arch/x86/kvm/svm/sev.c:pre_sev_run() {
	...
	if (is_sev_es_guest(vcpu) && !VALID_PAGE(svm->vmcb->control.vmsa_pa))
		return -EINVAL;
}

Can userspace call KVM_RUN right after KVM_SEV_SNP_SET_VCPU_STATE and enter
the guest before snp_launch_finish() has set
vcpu->arch.guest_state_protected = true and enabled lbr virtualization?

[1] https://lore.kernel.org/r/20260908103338.427254-7-joro@8bytes.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102114137.1198982-1-joro@8bytes.org?part=6

[PATCH v3 7/8] KVM: selftests: Test the SNP APIC-ID-list GHCB request

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Exercise the GET_AP_APIC_IDS NAE event from an SNP guest using a test-local
GHCB encoder.  Verify the exact count and APIC IDs, missing RAX validity,
zero-sized requests, invalid GPAs, and the 1024-vCPU descriptor boundary
when supported.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../selftests/kvm/x86/sev_snp_apic_id_test.c  | 180 ++++++++++++++++++
 2 files changed, 181 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 17a4256e9081..9f2b15250a2a 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -154,6 +154,7 @@ TEST_GEN_PROGS_x86 += x86/xen_vmcall_test
 TEST_GEN_PROGS_x86 += x86/sev_dbg_test
 TEST_GEN_PROGS_x86 += x86/sev_init2_tests
 TEST_GEN_PROGS_x86 += x86/sev_migrate_tests
+TEST_GEN_PROGS_x86 += x86/sev_snp_apic_id_test
 TEST_GEN_PROGS_x86 += x86/sev_smoke_test
 TEST_GEN_PROGS_x86 += x86/amx_test
 TEST_GEN_PROGS_x86 += x86/max_vcpuid_cap_test
diff --git a/tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c b/tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c
new file mode 100644
index 000000000000..8ad9c15091c2
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <stddef.h>
+#include <stdint.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "sev.h"
+#include "svm_util.h"
+
+#define GHCB_SAVE_RAX_OFFSET		0x1f8
+#define GHCB_SAVE_SW_EXIT_CODE_OFFSET	0x390
+#define GHCB_SAVE_SW_EXIT_INFO_1_OFFSET	0x398
+#define GHCB_SAVE_SW_EXIT_INFO_2_OFFSET	0x3a0
+#define GHCB_SAVE_VALID_BITMAP_OFFSET	0x3f0
+
+#define GHCB_MSR_REG_GPA_REQ		0x012
+#define GHCB_MSR_REG_GPA_RESP		0x013
+#define GHCB_MSR_INFO_MASK		GENMASK_ULL(11, 0)
+
+#define GHCB_HV_RESP_MALFORMED_INPUT	2
+#define GHCB_ERR_MISSING_INPUT		4
+#define GHCB_ERR_INVALID_INPUT		5
+
+struct apic_id_desc {
+	u32 nr_entries;
+	u32 apic_ids[];
+};
+
+struct apic_id_results {
+	u64 missing_info1;
+	u64 missing_info2;
+	u64 zero_info1;
+	u64 zero_info2;
+	u64 zero_rax;
+	u64 invalid_info1;
+	u64 invalid_info2;
+	u64 valid_info1;
+	u64 valid_info2;
+	u32 nr_entries;
+	u32 first_apic_id;
+	u32 last_apic_id;
+};
+
+static void ghcb_set_field(void *ghcb, size_t offset, u64 value, bool valid)
+{
+	u8 *valid_bitmap = ghcb + GHCB_SAVE_VALID_BITMAP_OFFSET;
+
+	*(u64 *)(ghcb + offset) = value;
+	if (valid)
+		valid_bitmap[(offset / sizeof(u64)) / 8] |=
+			BIT((offset / sizeof(u64)) % 8);
+}
+
+static u64 ghcb_get_field(void *ghcb, size_t offset)
+{
+	return *(u64 *)(ghcb + offset);
+}
+
+static void do_get_apic_ids(void *ghcb, gpa_t buffer_gpa, u64 pages,
+			    bool rax_valid)
+{
+	memset(ghcb, 0, PAGE_SIZE);
+	ghcb_set_field(ghcb, GHCB_SAVE_SW_EXIT_CODE_OFFSET,
+		       SVM_VMGEXIT_GET_APIC_IDS, true);
+	ghcb_set_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_1_OFFSET, buffer_gpa, true);
+	ghcb_set_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_2_OFFSET, 0, true);
+	ghcb_set_field(ghcb, GHCB_SAVE_RAX_OFFSET, pages, rax_valid);
+	vmgexit();
+}
+
+static void guest_code(void *ghcb, gpa_t ghcb_gpa, void *list,
+		       gpa_t list_gpa, struct apic_id_results *results,
+		       u64 expected_vcpus)
+{
+	struct apic_id_desc *desc = list;
+	u64 msr;
+
+	wrmsr(MSR_AMD64_SEV_ES_GHCB,
+	      (ghcb_gpa >> PAGE_SHIFT) << PAGE_SHIFT | GHCB_MSR_REG_GPA_REQ);
+	vmgexit();
+	msr = rdmsr(MSR_AMD64_SEV_ES_GHCB);
+	if ((msr & GHCB_MSR_INFO_MASK) != GHCB_MSR_REG_GPA_RESP)
+		goto terminate;
+
+	wrmsr(MSR_AMD64_SEV_ES_GHCB, ghcb_gpa);
+
+	do_get_apic_ids(ghcb, list_gpa, 1, false);
+	results->missing_info1 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_1_OFFSET);
+	results->missing_info2 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_2_OFFSET);
+
+	do_get_apic_ids(ghcb, list_gpa, 0, true);
+	results->zero_info1 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_1_OFFSET);
+	results->zero_info2 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_2_OFFSET);
+	results->zero_rax = ghcb_get_field(ghcb, GHCB_SAVE_RAX_OFFSET);
+
+	do_get_apic_ids(ghcb, BIT_ULL(52), 2, true);
+	results->invalid_info1 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_1_OFFSET);
+	results->invalid_info2 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_2_OFFSET);
+
+	do_get_apic_ids(ghcb, list_gpa, 2, true);
+	results->valid_info1 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_1_OFFSET);
+	results->valid_info2 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_2_OFFSET);
+	results->nr_entries = desc->nr_entries;
+	results->first_apic_id = desc->apic_ids[0];
+	results->last_apic_id = desc->apic_ids[expected_vcpus - 1];
+
+terminate:
+	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
+	vmgexit();
+}
+
+static void run_apic_id_test(unsigned int nr_vcpus)
+{
+	struct apic_id_results *results;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	gva_t ghcb_gva, list_gva, results_gva;
+	gpa_t ghcb_gpa, list_gpa;
+	unsigned int i;
+
+	kvm_set_files_rlimit(nr_vcpus);
+	vm = vm_sev_create_with_one_vcpu(KVM_X86_SNP_VM, guest_code, &vcpu);
+	for (i = 1; i < nr_vcpus; i++)
+		__vm_vcpu_add(vm, i);
+
+	ghcb_gva = vm_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
+				   MEM_REGION_TEST_DATA);
+	list_gva = vm_alloc_shared(vm, 2 * PAGE_SIZE, KVM_UTIL_MIN_VADDR,
+				   MEM_REGION_TEST_DATA);
+	results_gva = vm_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
+				      MEM_REGION_TEST_DATA);
+	ghcb_gpa = addr_gva2gpa(vm, ghcb_gva);
+	list_gpa = addr_gva2gpa(vm, list_gva);
+	results = addr_gva2hva(vm, results_gva);
+
+	vcpu_args_set(vcpu, 6, ghcb_gva, ghcb_gpa, list_gva, list_gpa,
+		      results_gva, nr_vcpus);
+	memset(addr_gva2hva(vm, ghcb_gva), 0, PAGE_SIZE);
+	memset(addr_gva2hva(vm, list_gva), 0, 2 * PAGE_SIZE);
+	memset(results, 0, PAGE_SIZE);
+	vm_sev_launch(vm, snp_default_policy(), NULL);
+
+	vcpu_run(vcpu);
+	TEST_ASSERT_EQ(vcpu->run->exit_reason, KVM_EXIT_SYSTEM_EVENT);
+	TEST_ASSERT_EQ(vcpu->run->system_event.type, KVM_SYSTEM_EVENT_SEV_TERM);
+
+	TEST_ASSERT_EQ(results->missing_info1, GHCB_HV_RESP_MALFORMED_INPUT);
+	TEST_ASSERT_EQ(results->missing_info2, GHCB_ERR_MISSING_INPUT);
+	TEST_ASSERT_EQ(results->zero_info1, 0);
+	TEST_ASSERT_EQ(results->zero_info2, 0);
+	TEST_ASSERT_EQ(results->zero_rax,
+		       DIV_ROUND_UP(sizeof(struct apic_id_desc) + nr_vcpus * sizeof(u32),
+				    PAGE_SIZE));
+	TEST_ASSERT_EQ(results->invalid_info1, GHCB_HV_RESP_MALFORMED_INPUT);
+	TEST_ASSERT_EQ(results->invalid_info2, GHCB_ERR_INVALID_INPUT);
+	TEST_ASSERT_EQ(results->valid_info1, 0);
+	TEST_ASSERT_EQ(results->valid_info2, 0);
+	TEST_ASSERT_EQ(results->nr_entries, nr_vcpus);
+	TEST_ASSERT_EQ(results->first_apic_id, 0);
+	TEST_ASSERT_EQ(results->last_apic_id, nr_vcpus - 1);
+
+	kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+	unsigned int max_vcpus;
+
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SNP_VM));
+	run_apic_id_test(2);
+
+	/* 1024 IDs cross the one-page descriptor boundary. */
+	max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
+	if (max_vcpus >= 1024)
+		run_apic_id_test(1024);
+	else
+		pr_info("Skipping vCPU-count boundary test (max vCPUs: %u)\n", max_vcpus);
+
+	return 0;
+}
-- 
2.53.0

Re: [PATCH v3 7/8] KVM: selftests: Test the SNP APIC-ID-list GHCB request

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: selftests: 32-bit nr_vcpus passed to 64-bit va_arg in run_apic_id_test()
--

commit 3cc5f05e1197383f380887a6674ec843f8015520
Author: Joerg Roedel <joerg.roedel@amd.com>

KVM: selftests: Test the SNP APIC-ID-list GHCB request

This commit adds a KVM selftest exercising the GET_AP_APIC_IDS NAE event from
an SNP guest, testing valid requests, missing RAX validity, zero-sized
requests, invalid GPAs, and the 1024-vCPU descriptor boundary.

> diff --git a/tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c b/tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c
> new file mode 100644
> index 0000000000000..8ad9c15091c2d
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/sev_snp_apic_id_test.c

[ ... ]

> +static void guest_code(void *ghcb, gpa_t ghcb_gpa, void *list,
> +		       gpa_t list_gpa, struct apic_id_results *results,
> +		       u64 expected_vcpus)
> +{

[ ... ]

> +	do_get_apic_ids(ghcb, list_gpa, 2, true);
> +	results->valid_info1 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_1_OFFSET);
> +	results->valid_info2 = ghcb_get_field(ghcb, GHCB_SAVE_SW_EXIT_INFO_2_OFFSET);
> +	results->nr_entries = desc->nr_entries;
> +	results->first_apic_id = desc->apic_ids[0];
> +	results->last_apic_id = desc->apic_ids[expected_vcpus - 1];
> +
> +terminate:
> +	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
> +	vmgexit();
> +}
> +
> +static void run_apic_id_test(unsigned int nr_vcpus)
> +{
> +	struct apic_id_results *results;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	gva_t ghcb_gva, list_gva, results_gva;
> +	gpa_t ghcb_gpa, list_gpa;
> +	unsigned int i;
> +
> +	kvm_set_files_rlimit(nr_vcpus);
> +	vm = vm_sev_create_with_one_vcpu(KVM_X86_SNP_VM, guest_code, &vcpu);
> +	for (i = 1; i < nr_vcpus; i++)
> +		__vm_vcpu_add(vm, i);
> +
> +	ghcb_gva = vm_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
> +				   MEM_REGION_TEST_DATA);
> +	list_gva = vm_alloc_shared(vm, 2 * PAGE_SIZE, KVM_UTIL_MIN_VADDR,
> +				   MEM_REGION_TEST_DATA);
> +	results_gva = vm_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
> +				      MEM_REGION_TEST_DATA);
> +	ghcb_gpa = addr_gva2gpa(vm, ghcb_gva);
> +	list_gpa = addr_gva2gpa(vm, list_gva);
> +	results = addr_gva2hva(vm, results_gva);
> +
> +	vcpu_args_set(vcpu, 6, ghcb_gva, ghcb_gpa, list_gva, list_gpa,
> +		      results_gva, nr_vcpus);

[Severity: Medium]
In run_apic_id_test(), nr_vcpus is declared as unsigned int (32-bit) and
passed without a cast as the 6th variadic argument to vcpu_args_set().

In C, default argument promotions do not widen unsigned int to 64 bits, while
tools/testing/selftests/kvm/lib/x86/processor.c:vcpu_args_set() extracts all
variadic arguments as 64-bit u64 values:

    if (num >= 6)
        regs.r9 = va_arg(ap, u64);

Because the first six arguments to vcpu_args_set() consume all six integer
argument registers on x86-64, nr_vcpus is passed on the stack. A 32-bit store
to that stack slot leaves the upper 32 bits uninitialized, and va_arg(ap, u64)
loads all 64 bits into regs.r9, which guest_code() receives as u64
expected_vcpus and uses to index desc->apic_ids[expected_vcpus - 1].

Can this result in reading uninitialized upper bits in expected_vcpus and
indexing out of bounds in desc->apic_ids[]? Should nr_vcpus be cast to u64 (or
declared as u64) when passed to vcpu_args_set()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102114137.1198982-1-joro@8bytes.org?part=7

[PATCH v3 8/8] KVM: selftests: Test SNP vCPU state and direct VMSA launch

[Jörg Rödel]

From: Joerg Roedel <joerg.roedel@amd.com>

Add vCPU-scoped SEV ioctl helpers and exercise the SNP vCPU state get and
set commands.  Enable KVM_CAP_SNP_DIRECT_VMSA before creating vCPUs and
verify that invalid capability arguments and late enable attempts fail.

Boot through a handcrafted BSP VMSA, verify VMSA and GHCB address round
trips and validity bits, rebind to the selected GPA, and verify an extra
unassociated AP remains uninitialized.

Reject multi-page launch updates and VMSAs with nonzero VMPL or mismatched
SEV features.  Also verify the legacy launch path and reject non-SNP use,
bad or reserved arguments, unsafe alignment, absent pages, wrong ordering,
and post-launch SET calls.  Verify that SET accepts both shared and private
guestmemfd-backed VMSA GPAs without consulting memory attributes or the
RMP.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 tools/testing/selftests/kvm/include/x86/sev.h |  49 ++
 .../kvm/x86/sev_snp_direct_vmsa_test.c        | 421 ++++++++++++++++++
 3 files changed, 471 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/sev_snp_direct_vmsa_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 9f2b15250a2a..1131fb0f630d 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -155,6 +155,7 @@ TEST_GEN_PROGS_x86 += x86/sev_dbg_test
 TEST_GEN_PROGS_x86 += x86/sev_init2_tests
 TEST_GEN_PROGS_x86 += x86/sev_migrate_tests
 TEST_GEN_PROGS_x86 += x86/sev_snp_apic_id_test
+TEST_GEN_PROGS_x86 += x86/sev_snp_direct_vmsa_test
 TEST_GEN_PROGS_x86 += x86/sev_smoke_test
 TEST_GEN_PROGS_x86 += x86/amx_test
 TEST_GEN_PROGS_x86 += x86/max_vcpuid_cap_test
diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h
index dec383e59a47..0f6c92b802ee 100644
--- a/tools/testing/selftests/kvm/include/x86/sev.h
+++ b/tools/testing/selftests/kvm/include/x86/sev.h
@@ -100,6 +100,30 @@ static inline u64 snp_default_policy(void)
 	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd,	ret, vm);		\
 })
 
+static inline int __vcpu_sev_ioctl(struct kvm_vcpu *vcpu, u32 cmd, void *arg)
+{
+	union {
+		struct kvm_sev_cmd c;
+		unsigned long raw;
+	} sev_cmd = { .c = {
+		.id = cmd,
+		.data = (u64)arg,
+		.sev_fd = vcpu->vm->arch.sev_fd,
+	} };
+	int ret;
+
+	ret = __vcpu_ioctl(vcpu, KVM_MEMORY_ENCRYPT_OP, &sev_cmd.raw);
+	return ret ?: sev_cmd.c.error;
+}
+
+#define vcpu_sev_ioctl(vcpu, cmd, arg)				\
+({								\
+	struct kvm_vcpu *__vcpu = (vcpu);				\
+	int ret = __vcpu_sev_ioctl(__vcpu, cmd, arg);		\
+								\
+	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, __vcpu->vm);	\
+})
+
 void sev_vm_init(struct kvm_vm *vm);
 void sev_es_vm_init(struct kvm_vm *vm);
 void snp_vm_init(struct kvm_vm *vm);
@@ -144,6 +168,31 @@ static inline void snp_launch_update_data(struct kvm_vm *vm, gpa_t gpa,
 	vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_UPDATE, &update_data);
 }
 
+static inline void snp_launch_update_vmsa(struct kvm_vm *vm, gpa_t gpa,
+					  void *vmsa)
+{
+	vm_mem_set_private(vm, gpa, PAGE_SIZE);
+	snp_launch_update_data(vm, gpa, (u64)vmsa, PAGE_SIZE,
+			       KVM_SEV_SNP_PAGE_TYPE_VMSA);
+}
+
+static inline void snp_get_vcpu_state(struct kvm_vcpu *vcpu,
+				      struct kvm_sev_snp_vcpu_state *state)
+{
+	vcpu_sev_ioctl(vcpu, KVM_SEV_SNP_GET_VCPU_STATE, state);
+}
+
+static inline void snp_set_vcpu_state(struct kvm_vcpu *vcpu, gpa_t vmsa_gpa)
+{
+	struct kvm_sev_snp_vcpu_state state = {
+		.vmsa_gpa = vmsa_gpa,
+		.valid_fields = KVM_SEV_SNP_VCPU_STATE_VMSA_VALID |
+				KVM_SEV_SNP_VCPU_STATE_GHCB_VALID,
+	};
+
+	vcpu_sev_ioctl(vcpu, KVM_SEV_SNP_SET_VCPU_STATE, &state);
+}
+
 static inline void sev_dbg_crypt_memory(struct kvm_vm *vm, unsigned int cmd,
 					void *dst, void *src, unsigned int len)
 {
diff --git a/tools/testing/selftests/kvm/x86/sev_snp_direct_vmsa_test.c b/tools/testing/selftests/kvm/x86/sev_snp_direct_vmsa_test.c
new file mode 100644
index 000000000000..63b01d563dc0
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/sev_snp_direct_vmsa_test.c
@@ -0,0 +1,421 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <errno.h>
+#include <stdlib.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "sev.h"
+#include "svm_util.h"
+
+#define DIRECT_MARKER	0x444952454354564dULL
+#define LEGACY_MARKER	0x4c4547414359564dULL
+#define SNP_ACTIVE_SEV_FEATURE	BIT_ULL(0)
+#define VMSA_PMD_SIZE	BIT_ULL(21)
+#define VMSA_MIN_GPA	(VMSA_PMD_SIZE + PAGE_SIZE)
+
+struct test_vmsa {
+	struct vmcb_seg es, cs, ss, ds, fs, gs;
+	struct vmcb_seg gdtr, ldtr, idtr, tr;
+	u64 pl0_ssp, pl1_ssp, pl2_ssp, pl3_ssp;
+	u64 u_cet;
+	u8 reserved_0xc8[2];
+	u8 vmpl;
+	u8 cpl;
+	u8 reserved_0xcc[4];
+	u64 efer;
+	u8 reserved_0xd8[104];
+	u64 xss;
+	u64 cr4, cr3, cr0, dr7, dr6, rflags, rip;
+	u64 dr0, dr1, dr2, dr3;
+	u64 dr0_addr_mask, dr1_addr_mask, dr2_addr_mask, dr3_addr_mask;
+	u8 reserved_0x1c0[24];
+	u64 rsp, s_cet, ssp, isst_addr, rax;
+	u64 star, lstar, cstar, sfmask, kernel_gs_base;
+	u64 sysenter_cs, sysenter_esp, sysenter_eip, cr2;
+	u8 reserved_0x248[32];
+	u64 g_pat, dbgctl, br_from, br_to, last_excp_from, last_excp_to;
+	u8 reserved_0x298[80];
+	u32 pkru, tsc_aux;
+	u64 tsc_scale, tsc_offset;
+	u8 reserved_0x300[8];
+	u64 rcx, rdx, rbx, reserved_0x320, rbp, rsi, rdi;
+	u64 r8, r9, r10, r11, r12, r13, r14, r15;
+	u8 reserved_0x380[16];
+	u64 guest_exit_info_1, guest_exit_info_2, guest_exit_int_info, guest_nrip;
+	u64 sev_features, vintr_ctrl, guest_exit_code, virtual_tom, tlb_id, pcpu_id;
+	u64 event_inj, xcr0;
+	u8 reserved_0x3f0[16];
+	u64 x87_dp;
+	u32 mxcsr;
+	u16 x87_ftw, x87_fsw, x87_fcw, x87_fop, x87_ds, x87_cs;
+	u64 x87_rip;
+	u8 fpreg_x87[80];
+	u8 fpreg_xmm[256];
+	u8 fpreg_ymm[256];
+} __packed;
+
+static_assert(offsetof(struct test_vmsa, vmpl) == 0xca);
+static_assert(offsetof(struct test_vmsa, rip) == 0x178);
+static_assert(offsetof(struct test_vmsa, sev_features) == 0x3b0);
+static_assert(offsetof(struct test_vmsa, xcr0) == 0x3e8);
+
+static void guest_direct_entry(u64 *marker)
+{
+	*marker = DIRECT_MARKER;
+	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
+	vmgexit();
+}
+
+static void guest_legacy_entry(u64 *marker)
+{
+	*marker = LEGACY_MARKER;
+	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
+	vmgexit();
+}
+
+static void copy_segment(struct vmcb_seg *dst, const struct kvm_segment *src)
+{
+	dst->selector = src->selector;
+	dst->base = src->base;
+	dst->limit = src->limit;
+	dst->attrib = src->type |
+		(src->s << SVM_SELECTOR_S_SHIFT) |
+		(src->dpl << SVM_SELECTOR_DPL_SHIFT) |
+		((src->present && !src->unusable) << SVM_SELECTOR_P_SHIFT) |
+		(src->avl << SVM_SELECTOR_AVL_SHIFT) |
+		(src->l << SVM_SELECTOR_L_SHIFT) |
+		(src->db << SVM_SELECTOR_DB_SHIFT) |
+		(src->g << SVM_SELECTOR_G_SHIFT);
+}
+
+static void copy_dtable(struct vmcb_seg *dst, const struct kvm_dtable *src)
+{
+	dst->base = src->base;
+	dst->limit = src->limit;
+}
+
+static void prepare_vmsa(struct kvm_vcpu *vcpu, struct test_vmsa *vmsa,
+			 void *entry)
+{
+	struct kvm_sregs sregs;
+	struct kvm_regs regs;
+
+	memset(vmsa, 0, PAGE_SIZE);
+	vcpu_sregs_get(vcpu, &sregs);
+	vcpu_regs_get(vcpu, &regs);
+
+	copy_segment(&vmsa->es, &sregs.es);
+	copy_segment(&vmsa->cs, &sregs.cs);
+	copy_segment(&vmsa->ss, &sregs.ss);
+	copy_segment(&vmsa->ds, &sregs.ds);
+	copy_segment(&vmsa->fs, &sregs.fs);
+	copy_segment(&vmsa->gs, &sregs.gs);
+	copy_dtable(&vmsa->gdtr, &sregs.gdt);
+	copy_segment(&vmsa->ldtr, &sregs.ldt);
+	copy_dtable(&vmsa->idtr, &sregs.idt);
+	copy_segment(&vmsa->tr, &sregs.tr);
+
+	vmsa->cpl = sregs.cs.dpl;
+	/*
+	 * KVM_GET_SREGS exposes the guest-visible EFER and therefore omits
+	 * SVME, which KVM normally adds to the hardware VMSA itself.
+	 */
+	vmsa->efer = sregs.efer | EFER_SVME;
+	vmsa->cr4 = sregs.cr4;
+	vmsa->cr3 = sregs.cr3;
+	vmsa->cr0 = sregs.cr0;
+	vmsa->dr7 = 0x400;
+	vmsa->dr6 = 0xffff0ff0;
+	vmsa->rflags = regs.rflags;
+	vmsa->rip = (u64)entry;
+	vmsa->rsp = regs.rsp;
+	vmsa->rax = regs.rax;
+	vmsa->rcx = regs.rcx;
+	vmsa->rdx = regs.rdx;
+	vmsa->rbx = regs.rbx;
+	vmsa->rbp = regs.rbp;
+	vmsa->rsi = regs.rsi;
+	vmsa->rdi = regs.rdi;
+	vmsa->r8 = regs.r8;
+	vmsa->r9 = regs.r9;
+	vmsa->r10 = regs.r10;
+	vmsa->r11 = regs.r11;
+	vmsa->r12 = regs.r12;
+	vmsa->r13 = regs.r13;
+	vmsa->r14 = regs.r14;
+	vmsa->r15 = regs.r15;
+	vmsa->g_pat = 0x0007040600070406ULL;
+	vmsa->sev_features = SNP_ACTIVE_SEV_FEATURE;
+	vmsa->xcr0 = 1;
+	vmsa->mxcsr = 0x1f80;
+	vmsa->x87_fcw = 0x37f;
+}
+
+static void expect_launch_update_vmsa_error(struct kvm_vm *vm, gpa_t gpa,
+					    void *vmsa, u64 size)
+{
+	struct kvm_sev_snp_launch_update update = {
+		.gfn_start = gpa >> PAGE_SHIFT,
+		.uaddr = (u64)vmsa,
+		.len = size,
+		.type = KVM_SEV_SNP_PAGE_TYPE_VMSA,
+	};
+
+	errno = 0;
+	TEST_ASSERT_EQ(__vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_UPDATE, &update), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+}
+
+static void exclude_from_normal_launch(struct kvm_vm *vm, gpa_t gpa,
+				       unsigned int npages)
+{
+	struct userspace_mem_region *region;
+
+	region = memslot2region(vm, vm->memslots[MEM_REGION_TEST_DATA]);
+	sparsebit_clear_num(region->protected_phy_pages, gpa >> PAGE_SHIFT, npages);
+}
+
+static void assert_vcpu_terminated(struct kvm_vcpu *vcpu)
+{
+	vcpu_run(vcpu);
+	TEST_ASSERT_EQ(vcpu->run->exit_reason, KVM_EXIT_SYSTEM_EVENT);
+	TEST_ASSERT_EQ(vcpu->run->system_event.type, KVM_SYSTEM_EVENT_SEV_TERM);
+}
+
+static struct kvm_vm *create_direct_vmsa_vm(unsigned int nr_vcpus,
+					    void *guest_code,
+					    struct kvm_vcpu **vcpus)
+{
+	struct vm_shape shape = {
+		.mode = VM_MODE_DEFAULT,
+		.type = KVM_X86_SNP_VM,
+	};
+	struct kvm_vm *vm;
+	unsigned int i;
+
+	vm = __vm_create(shape, nr_vcpus, 0);
+	vm_enable_cap(vm, KVM_CAP_SNP_DIRECT_VMSA, 0);
+	for (i = 0; i < nr_vcpus; i++)
+		vcpus[i] = vm_vcpu_add(vm, i, guest_code);
+	kvm_arch_vm_finalize_vcpus(vm);
+
+	return vm;
+}
+
+static void test_direct_vmsa(void)
+{
+	struct test_vmsa *vmsas, *selected_vmsa;
+	struct kvm_sev_snp_vcpu_state state = {};
+	struct kvm_mp_state mp_state = {
+		.mp_state = KVM_MP_STATE_UNINITIALIZED,
+	};
+	struct kvm_vcpu *vcpus[2];
+	struct kvm_vcpu *vcpu, *ap;
+	struct kvm_vm *vm;
+	gva_t marker_gva;
+	gpa_t vmsa_gpa;
+	u64 *marker;
+
+	vm = create_direct_vmsa_vm(ARRAY_SIZE(vcpus), guest_legacy_entry, vcpus);
+	vcpu = vcpus[0];
+	ap = vcpus[1];
+	/* Restore normal AP state after vm_vcpu_add() makes it runnable. */
+	vcpu_mp_state_set(ap, &mp_state);
+	marker_gva = vm_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
+				     MEM_REGION_TEST_DATA);
+	marker = addr_gva2hva(vm, marker_gva);
+	vcpu_args_set(vcpu, 1, marker_gva);
+
+	vmsa_gpa = ____vm_phy_pages_alloc(vm, 2, VMSA_MIN_GPA,
+					vm->memslots[MEM_REGION_TEST_DATA], true, false);
+	TEST_ASSERT(vmsa_gpa & (VMSA_PMD_SIZE - 1), "unsafe VMSA GPA");
+	vmsas = aligned_alloc(PAGE_SIZE, 2 * PAGE_SIZE);
+	TEST_ASSERT(vmsas, "Failed to allocate VMSA source pages");
+	selected_vmsa = (void *)vmsas + PAGE_SIZE;
+	prepare_vmsa(vcpu, &vmsas[0], guest_legacy_entry);
+	prepare_vmsa(vcpu, selected_vmsa, guest_direct_entry);
+
+	snp_vm_launch_start(vm, snp_default_policy());
+	vm_mem_set_private(vm, vmsa_gpa, 2 * PAGE_SIZE);
+	expect_launch_update_vmsa_error(vm, vmsa_gpa, vmsas, 2 * PAGE_SIZE);
+
+	vmsas[0].vmpl = 1;
+	expect_launch_update_vmsa_error(vm, vmsa_gpa, vmsas, PAGE_SIZE);
+	vmsas[0].vmpl = 0;
+	vmsas[0].sev_features = 0;
+	expect_launch_update_vmsa_error(vm, vmsa_gpa, vmsas, PAGE_SIZE);
+	vmsas[0].sev_features = SNP_ACTIVE_SEV_FEATURE;
+
+	snp_launch_update_vmsa(vm, vmsa_gpa, vmsas);
+	snp_launch_update_vmsa(vm, vmsa_gpa + PAGE_SIZE, selected_vmsa);
+	exclude_from_normal_launch(vm, vmsa_gpa, 2);
+	snp_vm_launch_update(vm);
+
+	/* Rebinding is allowed; the second, selected VMSA must win. */
+	snp_set_vcpu_state(vcpu, vmsa_gpa);
+	snp_set_vcpu_state(vcpu, vmsa_gpa + PAGE_SIZE);
+	snp_get_vcpu_state(vcpu, &state);
+	TEST_ASSERT_EQ(state.valid_fields,
+		       KVM_SEV_SNP_VCPU_STATE_VMSA_VALID |
+		       KVM_SEV_SNP_VCPU_STATE_GHCB_VALID);
+	TEST_ASSERT_EQ(state.vmsa_gpa, vmsa_gpa + PAGE_SIZE);
+	TEST_ASSERT_EQ(state.ghcb_gpa, 0);
+	snp_vm_launch_finish(vm);
+
+	vcpu_mp_state_get(ap, &mp_state);
+	TEST_ASSERT_EQ(mp_state.mp_state, KVM_MP_STATE_UNINITIALIZED);
+	*marker = 0;
+	assert_vcpu_terminated(vcpu);
+	TEST_ASSERT_EQ(*marker, DIRECT_MARKER);
+
+	state = (struct kvm_sev_snp_vcpu_state) {
+		.vmsa_gpa = vmsa_gpa,
+		.valid_fields = KVM_SEV_SNP_VCPU_STATE_VMSA_VALID,
+	};
+	errno = 0;
+	TEST_ASSERT_EQ(__vcpu_sev_ioctl(vcpu, KVM_SEV_SNP_SET_VCPU_STATE,
+					&state), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+
+	free(vmsas);
+	kvm_vm_free(vm);
+}
+
+static void expect_set_vcpu_state_error(struct kvm_vcpu *vcpu,
+					struct kvm_sev_snp_vcpu_state *state,
+					int expected_errno)
+{
+	errno = 0;
+	TEST_ASSERT_EQ(__vcpu_sev_ioctl(vcpu, KVM_SEV_SNP_SET_VCPU_STATE, state), -1);
+	TEST_ASSERT_EQ(errno, expected_errno);
+}
+
+static void test_invalid_requests(void)
+{
+	struct kvm_sev_snp_vcpu_state state = {
+		.valid_fields = KVM_SEV_SNP_VCPU_STATE_VMSA_VALID,
+	};
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	gva_t shared_gva;
+	gpa_t private_gpa;
+
+	vm = vm_create_with_one_vcpu(&vcpu, guest_legacy_entry);
+	expect_set_vcpu_state_error(vcpu, &state, ENOTTY);
+	kvm_vm_free(vm);
+
+	vm = vm_sev_create_with_one_vcpu(KVM_X86_SNP_VM, guest_legacy_entry, &vcpu);
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+	TEST_ASSERT_EQ(__vm_enable_cap(vm, KVM_CAP_SNP_DIRECT_VMSA, 0), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+	snp_vm_launch_start(vm, snp_default_policy());
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+	kvm_vm_free(vm);
+
+	vm = create_direct_vmsa_vm(1, guest_legacy_entry, &vcpu);
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+	snp_vm_launch_start(vm, snp_default_policy());
+
+	state.pad[4] = 1;
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+	state.pad[4] = 0;
+	state.valid_fields |= BIT_ULL(2);
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+	state.valid_fields &= ~BIT_ULL(2);
+	state.vmsa_gpa = PAGE_SIZE + 1;
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+	state.vmsa_gpa = VMSA_PMD_SIZE;
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+	state.vmsa_gpa = BIT_ULL(40) + PAGE_SIZE;
+	expect_set_vcpu_state_error(vcpu, &state, EINVAL);
+
+	shared_gva = vm_alloc_shared(vm, 2 * PAGE_SIZE, KVM_UTIL_MIN_VADDR,
+				     MEM_REGION_TEST_DATA);
+	state.vmsa_gpa = addr_gva2gpa(vm, shared_gva);
+	if (!(state.vmsa_gpa & (VMSA_PMD_SIZE - 1)))
+		state.vmsa_gpa += PAGE_SIZE;
+	state.ghcb_gpa = BIT_ULL(40);
+	state.valid_fields |= KVM_SEV_SNP_VCPU_STATE_GHCB_VALID;
+	vcpu_sev_ioctl(vcpu, KVM_SEV_SNP_SET_VCPU_STATE, &state);
+
+	private_gpa = ____vm_phy_pages_alloc(vm, 1, VMSA_MIN_GPA,
+					   vm->memslots[MEM_REGION_TEST_DATA], true, false);
+	vm_mem_set_private(vm, private_gpa, PAGE_SIZE);
+	state.vmsa_gpa = private_gpa;
+	vcpu_sev_ioctl(vcpu, KVM_SEV_SNP_SET_VCPU_STATE, &state);
+
+	memset(&state, 0, sizeof(state));
+	snp_get_vcpu_state(vcpu, &state);
+	TEST_ASSERT_EQ(state.valid_fields,
+		       KVM_SEV_SNP_VCPU_STATE_VMSA_VALID |
+		       KVM_SEV_SNP_VCPU_STATE_GHCB_VALID);
+	TEST_ASSERT_EQ(state.vmsa_gpa, private_gpa);
+	TEST_ASSERT_EQ(state.ghcb_gpa, BIT_ULL(40));
+
+	kvm_vm_free(vm);
+}
+
+static void test_direct_vmsa_capability(void)
+{
+	struct kvm_enable_cap cap = {
+		.cap = KVM_CAP_SNP_DIRECT_VMSA,
+	};
+	struct vm_shape shape = {
+		.mode = VM_MODE_DEFAULT,
+		.type = KVM_X86_SNP_VM,
+	};
+	struct kvm_vm *vm;
+
+	vm = vm_create_barebones();
+	TEST_ASSERT_EQ(__vm_enable_cap(vm, KVM_CAP_SNP_DIRECT_VMSA, 0), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+	kvm_vm_free(vm);
+
+	vm = __vm_create(shape, 1, 0);
+	TEST_ASSERT_EQ(__vm_enable_cap(vm, KVM_CAP_SNP_DIRECT_VMSA, 1), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+	cap.args[3] = 1;
+	TEST_ASSERT_EQ(__vm_ioctl(vm, KVM_ENABLE_CAP, &cap), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+	cap.args[3] = 0;
+	cap.flags = 1;
+	TEST_ASSERT_EQ(__vm_ioctl(vm, KVM_ENABLE_CAP, &cap), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+	vm_enable_cap(vm, KVM_CAP_SNP_DIRECT_VMSA, 0);
+	vm_vcpu_add(vm, 0, guest_legacy_entry);
+	TEST_ASSERT_EQ(__vm_enable_cap(vm, KVM_CAP_SNP_DIRECT_VMSA, 0), -1);
+	TEST_ASSERT_EQ(errno, EINVAL);
+	kvm_vm_free(vm);
+}
+
+static void test_legacy_launch(void)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	gva_t marker_gva;
+	u64 *marker;
+
+	vm = vm_sev_create_with_one_vcpu(KVM_X86_SNP_VM, guest_legacy_entry, &vcpu);
+	marker_gva = vm_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
+				     MEM_REGION_TEST_DATA);
+	marker = addr_gva2hva(vm, marker_gva);
+	vcpu_args_set(vcpu, 1, marker_gva);
+	vm_sev_launch(vm, snp_default_policy(), NULL);
+	*marker = 0;
+	assert_vcpu_terminated(vcpu);
+	TEST_ASSERT_EQ(*marker, LEGACY_MARKER);
+	kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_SNP_VCPU_STATE));
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_SNP_DIRECT_VMSA));
+	TEST_ASSERT(sizeof(struct test_vmsa) <= PAGE_SIZE, "VMSA structure is too large");
+
+	test_direct_vmsa_capability();
+	test_invalid_requests();
+	test_direct_vmsa();
+	test_legacy_launch();
+	return 0;
+}
-- 
2.53.0