[PATCH v8 00/10] Basic SEV-SNP Selftests

[PATCH v8 00/10] Basic SEV-SNP Selftests

[Pratik R. Sampat]

This patch series extends the sev_init2 and the sev_smoke test to
exercise the SEV-SNP VM launch workflow.

Primarily, it introduces the architectural defines, its support in the
SEV library and extends the tests to interact with the SEV-SNP ioctl()
wrappers.

Patch 1  - Do not advertise SNP on initialization failure
Patch 2  - SNP test for KVM_SEV_INIT2
Patch 3  - Add vmgexit helper
Patch 4  - Add SMT control interface helper
Patch 5  - Replace assert() with TEST_ASSERT_EQ()
Patch 6  - Introduce SEV+ VM type check
Patch 7  - SNP iotcl() plumbing for the SEV library
Patch 8  - Force set GUEST_MEMFD for SNP
Patch 9  - Cleanups of smoke test - Decouple policy from type
Patch 10 - SNP smoke test

The series is based on
        git.kernel.org/pub/scm/virt/kvm/kvm.git next

v7..v8:
* Dropped exporting the SNP initialized API from ccp to KVM. Instead
  call SNP_PLATFORM_STATUS within KVM to query the initialization. (Tom)
  
  While it may be cheaper to query sev->snp_initialized from ccp, making
  the SNP platform call within KVM does away with any dependencies.

v6..v7:
https://lore.kernel.org/kvm/20250221210200.244405-7-prsampat@amd.com/
Based on comments from Sean -
* Replaced FW check with sev->snp_initialized
* Dropped the patch which removes SEV+ KVM advertisement if INIT fails.
  This should be now be resolved by the combination of the patches [1,2]
  from Ashish.
* Change vmgexit to an inline function
* Export SMT control parsing interface to kvm_util
  Note: hyperv_cpuid KST only compile tested
* Replace assert() with TEST_ASSERT_EQ() within SEV library
* Define KVM_SEV_PAGE_TYPE_INVALID for SEV call of encrypt_region()
* Parameterize encrypt_region() to include privatize_region()
* Deduplication of sev test calls between SEV,SEV-ES and SNP
* Removed FW version tests for SNP
* Included testing of SNP_POLICY_DBG
* Dropped most tags from patches that have been changed or indirectly
  affected

[1] https://lore.kernel.org/all/d6d08c6b-9602-4f3d-92c2-8db6d50a1b92@amd.com
[2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226950.git.ashish.kalra@amd.com

v5..v6:
https://lore.kernel.org/kvm/ab433246-e97c-495b-ab67-b0cb1721fb99@amd.com/
* Rename is_sev_platform_init to sev_fw_initialized (Nikunj)
* Rename KVM CPU feature X86_FEATURE_SNP to X86_FEATURE_SEV_SNP (Nikunj)
* Collected Tags from Nikunj, Pankaj, Srikanth.

v4..v5:
https://lore.kernel.org/kvm/8e7d8172-879e-4a28-8438-343b1c386ec9@amd.com/
* Introduced a check to disable advertising support for SEV, SEV-ES
  and SNP when platform initialization fails (Nikunj)
* Remove the redundant SNP check within is_sev_vm() (Nikunj)
* Cleanup of the encrypt_region flow for better readability (Nikunj)
* Refactor paths to use the canonical $(ARCH) to rebase for kvm/next

v3..v4:
https://lore.kernel.org/kvm/20241114234104.128532-1-pratikrajesh.sampat@amd.com/
* Remove SNP FW API version check in the test and ensure the KVM
  capability advertises the presence of the feature. Retain the minimum
  version definitions to exercise these API versions in the smoke test
* Retained only the SNP smoke test and SNP_INIT2 test
* The SNP architectural defined merged with SNP_INIT2 test patch
* SNP shutdown merged with SNP smoke test patch
* Add SEV VM type check to abstract comparisons and reduce clutter
* Define a SNP default policy which sets bits based on the presence of
  SMT
* Decouple privatization and encryption for it to be SNP agnostic
* Assert for only positive tests using vm_ioctl()
* Dropped tested-by tags

In summary - based on comments from Sean, I have primarily reduced the
scope of this patch series to focus on breaking down the SNP smoke test
patch (v3 - patch2) to first introduce SEV-SNP support and use this
interface to extend the sev_init2 and the sev_smoke test.

The rest of the v3 patchset that introduces ioctl, pre fault, fallocate
and negative tests, will be re-worked and re-introduced subsequently in
future patch series post addressing the issues discussed.

v2..v3:
https://lore.kernel.org/kvm/20240905124107.6954-1-pratikrajesh.sampat@amd.com/
* Remove the assignments for the prefault and fallocate test type
  enums.
* Fix error message for sev launch measure and finish.
* Collect tested-by tags [Peter, Srikanth]

Pratik R. Sampat (10):
  KVM: SEV: Disable SEV-SNP support on initialization failure
  KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
  KVM: selftests: Add vmgexit helper
  KVM: selftests: Add SMT control state helper
  KVM: selftests: Replace assert() with TEST_ASSERT_EQ()
  KVM: selftests: Introduce SEV VM type check
  KVM: selftests: Add library support for interacting with SNP
  KVM: selftests: Force GUEST_MEMFD flag for SNP VM type
  KVM: selftests: Abstractions for SEV to decouple policy from type
  KVM: selftests: Add a basic SEV-SNP smoke test

 arch/x86/include/uapi/asm/kvm.h               |  1 +
 arch/x86/kvm/svm/sev.c                        | 30 +++++-
 tools/arch/x86/include/uapi/asm/kvm.h         |  1 +
 .../testing/selftests/kvm/include/kvm_util.h  | 35 +++++++
 .../selftests/kvm/include/x86/processor.h     |  1 +
 tools/testing/selftests/kvm/include/x86/sev.h | 42 ++++++++-
 tools/testing/selftests/kvm/lib/kvm_util.c    |  7 +-
 .../testing/selftests/kvm/lib/x86/processor.c |  4 +-
 tools/testing/selftests/kvm/lib/x86/sev.c     | 93 +++++++++++++++++--
 .../testing/selftests/kvm/x86/hyperv_cpuid.c  | 19 ----
 .../selftests/kvm/x86/sev_init2_tests.c       | 13 +++
 .../selftests/kvm/x86/sev_smoke_test.c        | 75 +++++++++------
 12 files changed, 261 insertions(+), 60 deletions(-)

-- 
2.43.0

[PATCH v8 01/10] KVM: SEV: Disable SEV-SNP support on initialization failure

[Pratik R. Sampat]

During platform init, SNP initialization may fail for several reasons,
such as firmware command failures and incompatible versions. However,
the KVM capability may continue to advertise support for it. During
setup, query the SNP platform status to obtain the initialization state
and use it as an additional condition to determine support for SEV-SNP.

Fixes: 1dfe571c12cf ("KVM: SEV: Add initial SEV-SNP support")
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
v7..v8:
* Avoid exporting yet another API from CCP. Instead query
  SNP_PLATFORM_STATUS to get the current the initialization state
  within KVM (Tom)
---
 arch/x86/kvm/svm/sev.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 0dbb25442ec1..e21c3aa6f592 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2950,6 +2950,32 @@ void __init sev_set_cpu_caps(void)
 	}
 }
 
+static bool snp_initialized(void)
+{
+	struct sev_user_data_snp_status *status;
+	struct sev_data_snp_addr buf;
+	bool initialized = false;
+	void *data;
+	int error;
+
+	data = snp_alloc_firmware_page(GFP_KERNEL_ACCOUNT);
+	if (!data)
+		return initialized;
+
+	buf.address = __psp_pa(data);
+	if (sev_do_cmd(SEV_CMD_SNP_PLATFORM_STATUS, &buf, &error))
+		goto out;
+
+	status = (struct sev_user_data_snp_status *)data;
+	if (status->state)
+		initialized = true;
+
+out:
+	snp_free_firmware_page(data);
+
+	return initialized;
+}
+
 void __init sev_hardware_setup(void)
 {
 	unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count;
@@ -3050,7 +3076,9 @@ void __init sev_hardware_setup(void)
 	sev_es_asid_count = min_sev_asid - 1;
 	WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count));
 	sev_es_supported = true;
-	sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
+	sev_snp_supported = (sev_snp_enabled &&
+			    cc_platform_has(CC_ATTR_HOST_SEV_SNP) &&
+			    snp_initialized());
 
 out:
 	if (boot_cpu_has(X86_FEATURE_SEV))
-- 
2.43.0

[PATCH v8 02/10] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2

[Pratik R. Sampat]

Add the X86_FEATURE_SEV_SNP CPU feature to the architectural definition
for the SEV-SNP VM type to exercise the KVM_SEV_INIT2 call. Ensure that
the SNP test is skipped in scenarios where CPUID supports it but KVM
does not, preventing reporting of failure in such cases.

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 tools/testing/selftests/kvm/include/x86/processor.h |  1 +
 tools/testing/selftests/kvm/x86/sev_init2_tests.c   | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index d60da8966772..6f63fd10bbc6 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -199,6 +199,7 @@ struct kvm_x86_cpu_feature {
 #define	X86_FEATURE_VGIF		KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 16)
 #define X86_FEATURE_SEV			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1)
 #define X86_FEATURE_SEV_ES		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
+#define X86_FEATURE_SEV_SNP			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 4)
 
 /*
  * KVM defined paravirt features.
diff --git a/tools/testing/selftests/kvm/x86/sev_init2_tests.c b/tools/testing/selftests/kvm/x86/sev_init2_tests.c
index 3fb967f40c6a..ab3dd11ac163 100644
--- a/tools/testing/selftests/kvm/x86/sev_init2_tests.c
+++ b/tools/testing/selftests/kvm/x86/sev_init2_tests.c
@@ -28,6 +28,7 @@
 int kvm_fd;
 u64 supported_vmsa_features;
 bool have_sev_es;
+bool have_snp;
 
 static int __sev_ioctl(int vm_fd, int cmd_id, void *data)
 {
@@ -83,6 +84,9 @@ void test_vm_types(void)
 	if (have_sev_es)
 		test_init2(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
 
+	if (have_snp)
+		test_init2(KVM_X86_SNP_VM, &(struct kvm_sev_init){});
+
 	test_init2_invalid(0, &(struct kvm_sev_init){},
 			   "VM type is KVM_X86_DEFAULT_VM");
 	if (kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM))
@@ -138,15 +142,24 @@ int main(int argc, char *argv[])
 		    "sev-es: KVM_CAP_VM_TYPES (%x) does not match cpuid (checking %x)",
 		    kvm_check_cap(KVM_CAP_VM_TYPES), 1 << KVM_X86_SEV_ES_VM);
 
+	have_snp = kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SNP_VM);
+	TEST_ASSERT(!have_snp || kvm_cpu_has(X86_FEATURE_SEV_SNP),
+		    "sev-snp: KVM_CAP_VM_TYPES (%x) indicates SNP support (bit %d), but CPUID does not",
+		    kvm_check_cap(KVM_CAP_VM_TYPES), KVM_X86_SNP_VM);
+
 	test_vm_types();
 
 	test_flags(KVM_X86_SEV_VM);
 	if (have_sev_es)
 		test_flags(KVM_X86_SEV_ES_VM);
+	if (have_snp)
+		test_flags(KVM_X86_SNP_VM);
 
 	test_features(KVM_X86_SEV_VM, 0);
 	if (have_sev_es)
 		test_features(KVM_X86_SEV_ES_VM, supported_vmsa_features);
+	if (have_snp)
+		test_features(KVM_X86_SNP_VM, supported_vmsa_features);
 
 	return 0;
 }
-- 
2.43.0

[PATCH v8 03/10] KVM: selftests: Add vmgexit helper

[Pratik R. Sampat]

Abstract rep vmmcall coded into the vmgexit helper for the sev
library.

No functional change intended.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 tools/testing/selftests/kvm/include/x86/sev.h    | 5 +++++
 tools/testing/selftests/kvm/x86/sev_smoke_test.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h
index 82c11c81a956..3003dc837fb7 100644
--- a/tools/testing/selftests/kvm/include/x86/sev.h
+++ b/tools/testing/selftests/kvm/include/x86/sev.h
@@ -71,6 +71,11 @@ kvm_static_assert(SEV_RET_SUCCESS == 0);
 void sev_vm_init(struct kvm_vm *vm);
 void sev_es_vm_init(struct kvm_vm *vm);
 
+static inline void vmgexit(void)
+{
+	__asm__ __volatile__("rep; vmmcall");
+}
+
 static inline void sev_register_encrypted_memory(struct kvm_vm *vm,
 						 struct userspace_mem_region *region)
 {
diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
index a1a688e75266..6812b94bf5b6 100644
--- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
@@ -27,7 +27,7 @@ static void guest_sev_es_code(void)
 	 * force "termination" to signal "done" via the GHCB MSR protocol.
 	 */
 	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
-	__asm__ __volatile__("rep; vmmcall");
+	vmgexit();
 }
 
 static void guest_sev_code(void)
-- 
2.43.0

[PATCH v8 04/10] KVM: selftests: Add SMT control state helper

[Pratik R. Sampat]

Move the SMT control check out of the hyperv_cpuid selftest so that it
is generally accessible all KVM selftests. Split the functionality into
a helper that populates a buffer with SMT control value which other
helpers can use to ascertain if SMT state is available and active.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  | 35 +++++++++++++++++++
 .../testing/selftests/kvm/x86/hyperv_cpuid.c  | 19 ----------
 2 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 4c4e5a847f67..446f04b2710f 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -542,6 +542,41 @@ static inline uint64_t vm_get_stat(struct kvm_vm *vm, const char *stat_name)
 	return data;
 }
 
+static inline bool read_smt_control(char *buf, size_t buf_size)
+{
+	FILE *f = fopen("/sys/devices/system/cpu/smt/control", "r");
+	bool ret;
+
+	if (!f)
+		return false;
+
+	ret = fread(buf, sizeof(*buf), buf_size, f) > 0;
+	fclose(f);
+
+	return ret;
+}
+
+static inline bool smt_possible(void)
+{
+	char buf[16];
+
+	if (read_smt_control(buf, sizeof(buf)) &&
+	    (!strncmp(buf, "forceoff", 8) || !strncmp(buf, "notsupported", 12)))
+		return false;
+
+	return true;
+}
+
+static inline bool smt_on(void)
+{
+	char buf[16];
+
+	if (read_smt_control(buf, sizeof(buf)) && !strncmp(buf, "on", 2))
+		return true;
+
+	return false;
+}
+
 void vm_create_irqchip(struct kvm_vm *vm);
 
 static inline int __vm_create_guest_memfd(struct kvm_vm *vm, uint64_t size,
diff --git a/tools/testing/selftests/kvm/x86/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86/hyperv_cpuid.c
index 4e920705681a..1eb55d0b7297 100644
--- a/tools/testing/selftests/kvm/x86/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86/hyperv_cpuid.c
@@ -22,25 +22,6 @@ static void guest_code(void)
 {
 }
 
-static bool smt_possible(void)
-{
-	char buf[16];
-	FILE *f;
-	bool res = true;
-
-	f = fopen("/sys/devices/system/cpu/smt/control", "r");
-	if (f) {
-		if (fread(buf, sizeof(*buf), sizeof(buf), f) > 0) {
-			if (!strncmp(buf, "forceoff", 8) ||
-			    !strncmp(buf, "notsupported", 12))
-				res = false;
-		}
-		fclose(f);
-	}
-
-	return res;
-}
-
 static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected)
 {
 	const bool has_irqchip = !vcpu || vcpu->vm->has_irqchip;
-- 
2.43.0

[PATCH v8 05/10] KVM: selftests: Replace assert() with TEST_ASSERT_EQ()

[Pratik R. Sampat]

For SEV tests, assert() failures on VM type or fd do not provide
sufficient error reporting. Replace assert() with TEST_ASSERT_EQ() to
obtain more detailed information on the assertion condition failure,
including the call stack.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 tools/testing/selftests/kvm/lib/x86/sev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/x86/sev.c b/tools/testing/selftests/kvm/lib/x86/sev.c
index e9535ee20b7f..60d7a03dc1c2 100644
--- a/tools/testing/selftests/kvm/lib/x86/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86/sev.c
@@ -37,12 +37,12 @@ static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *regio
 void sev_vm_init(struct kvm_vm *vm)
 {
 	if (vm->type == KVM_X86_DEFAULT_VM) {
-		assert(vm->arch.sev_fd == -1);
+		TEST_ASSERT_EQ(vm->arch.sev_fd, -1);
 		vm->arch.sev_fd = open_sev_dev_path_or_exit();
 		vm_sev_ioctl(vm, KVM_SEV_INIT, NULL);
 	} else {
 		struct kvm_sev_init init = { 0 };
-		assert(vm->type == KVM_X86_SEV_VM);
+		TEST_ASSERT_EQ(vm->type, KVM_X86_SEV_VM);
 		vm_sev_ioctl(vm, KVM_SEV_INIT2, &init);
 	}
 }
@@ -50,12 +50,12 @@ void sev_vm_init(struct kvm_vm *vm)
 void sev_es_vm_init(struct kvm_vm *vm)
 {
 	if (vm->type == KVM_X86_DEFAULT_VM) {
-		assert(vm->arch.sev_fd == -1);
+		TEST_ASSERT_EQ(vm->arch.sev_fd, -1);
 		vm->arch.sev_fd = open_sev_dev_path_or_exit();
 		vm_sev_ioctl(vm, KVM_SEV_ES_INIT, NULL);
 	} else {
 		struct kvm_sev_init init = { 0 };
-		assert(vm->type == KVM_X86_SEV_ES_VM);
+		TEST_ASSERT_EQ(vm->type, KVM_X86_SEV_ES_VM);
 		vm_sev_ioctl(vm, KVM_SEV_INIT2, &init);
 	}
 }
-- 
2.43.0

[PATCH v8 06/10] KVM: selftests: Introduce SEV VM type check

[Pratik R. Sampat]

In preparation for SNP, declutter the vm type check by introducing a
SEV-SNP VM type check as well as a transitive set of helper functions.

The SNP VM type is the subset of SEV-ES. Similarly, the SEV-ES and SNP
types are subset of the SEV VM type check.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 tools/testing/selftests/kvm/include/x86/sev.h   |  4 ++++
 tools/testing/selftests/kvm/lib/x86/processor.c |  4 ++--
 tools/testing/selftests/kvm/lib/x86/sev.c       | 17 +++++++++++++++++
 .../testing/selftests/kvm/x86/sev_smoke_test.c  |  2 +-
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h
index 3003dc837fb7..b112f7664534 100644
--- a/tools/testing/selftests/kvm/include/x86/sev.h
+++ b/tools/testing/selftests/kvm/include/x86/sev.h
@@ -27,6 +27,10 @@ enum sev_guest_state {
 
 #define GHCB_MSR_TERM_REQ	0x100
 
+bool is_sev_vm(struct kvm_vm *vm);
+bool is_sev_es_vm(struct kvm_vm *vm);
+bool is_sev_snp_vm(struct kvm_vm *vm);
+
 void sev_vm_launch(struct kvm_vm *vm, uint32_t policy);
 void sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement);
 void sev_vm_launch_finish(struct kvm_vm *vm);
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index bd5a802fa7a5..a92dc1dad085 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -639,7 +639,7 @@ void kvm_arch_vm_post_create(struct kvm_vm *vm)
 	sync_global_to_guest(vm, host_cpu_is_amd);
 	sync_global_to_guest(vm, is_forced_emulation_enabled);
 
-	if (vm->type == KVM_X86_SEV_VM || vm->type == KVM_X86_SEV_ES_VM) {
+	if (is_sev_vm(vm)) {
 		struct kvm_sev_init init = { 0 };
 
 		vm_sev_ioctl(vm, KVM_SEV_INIT2, &init);
@@ -1156,7 +1156,7 @@ void kvm_get_cpu_address_width(unsigned int *pa_bits, unsigned int *va_bits)
 
 void kvm_init_vm_address_properties(struct kvm_vm *vm)
 {
-	if (vm->type == KVM_X86_SEV_VM || vm->type == KVM_X86_SEV_ES_VM) {
+	if (is_sev_vm(vm)) {
 		vm->arch.sev_fd = open_sev_dev_path_or_exit();
 		vm->arch.c_bit = BIT_ULL(this_cpu_property(X86_PROPERTY_SEV_C_BIT));
 		vm->gpa_tag_mask = vm->arch.c_bit;
diff --git a/tools/testing/selftests/kvm/lib/x86/sev.c b/tools/testing/selftests/kvm/lib/x86/sev.c
index 60d7a03dc1c2..4587f2b6bc39 100644
--- a/tools/testing/selftests/kvm/lib/x86/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86/sev.c
@@ -4,6 +4,23 @@
 
 #include "sev.h"
 
+bool is_sev_snp_vm(struct kvm_vm *vm)
+{
+	return vm->type == KVM_X86_SNP_VM;
+}
+
+/* A SNP VM is also a SEV-ES VM */
+bool is_sev_es_vm(struct kvm_vm *vm)
+{
+	return is_sev_snp_vm(vm) || vm->type == KVM_X86_SEV_ES_VM;
+}
+
+/* A SEV-ES and SNP VM is also a SEV VM */
+bool is_sev_vm(struct kvm_vm *vm)
+{
+	return is_sev_es_vm(vm) || vm->type == KVM_X86_SEV_VM;
+}
+
 /*
  * sparsebit_next_clear() can return 0 if [x, 2**64-1] are all set, and the
  * -1 would then cause an underflow back to 2**64 - 1. This is expected and
diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
index 6812b94bf5b6..a2de1e63c3cb 100644
--- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
@@ -123,7 +123,7 @@ static void test_sev(void *guest_code, uint64_t policy)
 	for (;;) {
 		vcpu_run(vcpu);
 
-		if (policy & SEV_POLICY_ES) {
+		if (is_sev_es_vm(vm)) {
 			TEST_ASSERT(vcpu->run->exit_reason == KVM_EXIT_SYSTEM_EVENT,
 				    "Wanted SYSTEM_EVENT, got %s",
 				    exit_reason_str(vcpu->run->exit_reason));
-- 
2.43.0

[PATCH v8 07/10] KVM: selftests: Add library support for interacting with SNP

[Pratik R. Sampat]

Extend the SEV library to include support for SNP ioctl() wrappers,
which aid in launching and interacting with a SEV-SNP guest.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 arch/x86/include/uapi/asm/kvm.h               |  1 +
 tools/arch/x86/include/uapi/asm/kvm.h         |  1 +
 tools/testing/selftests/kvm/include/x86/sev.h | 33 ++++++++-
 tools/testing/selftests/kvm/lib/x86/sev.c     | 68 +++++++++++++++++--
 4 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 9e75da97bce0..565e4d054627 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -841,6 +841,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_ZERO		0x3
 #define KVM_SEV_SNP_PAGE_TYPE_UNMEASURED	0x4
diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
index 88585c1de416..17e44fbdc2a7 100644
--- a/tools/arch/x86/include/uapi/asm/kvm.h
+++ b/tools/arch/x86/include/uapi/asm/kvm.h
@@ -841,6 +841,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_ZERO		0x3
 #define KVM_SEV_SNP_PAGE_TYPE_UNMEASURED	0x4
diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h
index b112f7664534..c696d10f9332 100644
--- a/tools/testing/selftests/kvm/include/x86/sev.h
+++ b/tools/testing/selftests/kvm/include/x86/sev.h
@@ -25,6 +25,10 @@ enum sev_guest_state {
 #define SEV_POLICY_NO_DBG	(1UL << 0)
 #define SEV_POLICY_ES		(1UL << 2)
 
+#define SNP_POLICY_SMT		(1ULL << 16)
+#define SNP_POLICY_RSVD_MBO	(1ULL << 17)
+#define SNP_POLICY_DBG		(1ULL << 19)
+
 #define GHCB_MSR_TERM_REQ	0x100
 
 bool is_sev_vm(struct kvm_vm *vm);
@@ -34,13 +38,26 @@ bool is_sev_snp_vm(struct kvm_vm *vm);
 void sev_vm_launch(struct kvm_vm *vm, uint32_t policy);
 void sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement);
 void sev_vm_launch_finish(struct kvm_vm *vm);
+void snp_vm_launch_start(struct kvm_vm *vm, uint64_t policy);
+void snp_vm_launch_update(struct kvm_vm *vm);
+void snp_vm_launch_finish(struct kvm_vm *vm);
 
 struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t type, void *guest_code,
 					   struct kvm_vcpu **cpu);
-void vm_sev_launch(struct kvm_vm *vm, uint32_t policy, uint8_t *measurement);
+void vm_sev_launch(struct kvm_vm *vm, uint64_t policy, uint8_t *measurement);
 
 kvm_static_assert(SEV_RET_SUCCESS == 0);
 
+/*
+ * A SEV-SNP VM requires the policy reserved bit to always be set.
+ * The SMT policy bit is also required to be set based on SMT being
+ * available and active on the system.
+ */
+static inline u64 snp_default_policy(void)
+{
+	return SNP_POLICY_RSVD_MBO | (smt_on() ? SNP_POLICY_SMT : 0);
+}
+
 /*
  * The KVM_MEMORY_ENCRYPT_OP uAPI is utter garbage and takes an "unsigned long"
  * instead of a proper struct.  The size of the parameter is embedded in the
@@ -74,6 +91,7 @@ kvm_static_assert(SEV_RET_SUCCESS == 0);
 
 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);
 
 static inline void vmgexit(void)
 {
@@ -102,4 +120,17 @@ static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
 	vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_DATA, &update_data);
 }
 
+static inline void snp_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
+					  uint64_t hva, uint64_t size, uint8_t type)
+{
+	struct kvm_sev_snp_launch_update update_data = {
+		.uaddr = hva,
+		.gfn_start = gpa >> PAGE_SHIFT,
+		.len = size,
+		.type = type,
+	};
+
+	vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_UPDATE, &update_data);
+}
+
 #endif /* SELFTEST_KVM_SEV_H */
diff --git a/tools/testing/selftests/kvm/lib/x86/sev.c b/tools/testing/selftests/kvm/lib/x86/sev.c
index 4587f2b6bc39..a56f5164b0a6 100644
--- a/tools/testing/selftests/kvm/lib/x86/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86/sev.c
@@ -31,7 +31,8 @@ bool is_sev_vm(struct kvm_vm *vm)
  * and find the first range, but that's correct because the condition
  * expression would cause us to quit the loop.
  */
-static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *region)
+static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *region,
+			   uint8_t page_type, bool private)
 {
 	const struct sparsebit *protected_phy_pages = region->protected_phy_pages;
 	const vm_paddr_t gpa_base = region->region.guest_phys_addr;
@@ -41,13 +42,23 @@ static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *regio
 	if (!sparsebit_any_set(protected_phy_pages))
 		return;
 
-	sev_register_encrypted_memory(vm, region);
+	if (!is_sev_snp_vm(vm))
+		sev_register_encrypted_memory(vm, region);
 
 	sparsebit_for_each_set_range(protected_phy_pages, i, j) {
 		const uint64_t size = (j - i + 1) * vm->page_size;
 		const uint64_t offset = (i - lowest_page_in_region) * vm->page_size;
 
-		sev_launch_update_data(vm, gpa_base + offset, size);
+		if (private)
+			vm_mem_set_private(vm, gpa_base + offset, size);
+
+		if (is_sev_snp_vm(vm))
+			snp_launch_update_data(vm, gpa_base + offset,
+					       (uint64_t)addr_gpa2hva(vm, gpa_base + offset),
+					       size, page_type);
+		else
+			sev_launch_update_data(vm, gpa_base + offset, size);
+
 	}
 }
 
@@ -77,6 +88,14 @@ void sev_es_vm_init(struct kvm_vm *vm)
 	}
 }
 
+void snp_vm_init(struct kvm_vm *vm)
+{
+	struct kvm_sev_init init = { 0 };
+
+	TEST_ASSERT_EQ(vm->type, KVM_X86_SNP_VM);
+	vm_sev_ioctl(vm, KVM_SEV_INIT2, &init);
+}
+
 void sev_vm_launch(struct kvm_vm *vm, uint32_t policy)
 {
 	struct kvm_sev_launch_start launch_start = {
@@ -93,7 +112,7 @@ void sev_vm_launch(struct kvm_vm *vm, uint32_t policy)
 	TEST_ASSERT_EQ(status.state, SEV_GUEST_STATE_LAUNCH_UPDATE);
 
 	hash_for_each(vm->regions.slot_hash, ctr, region, slot_node)
-		encrypt_region(vm, region);
+		encrypt_region(vm, region, KVM_SEV_PAGE_TYPE_INVALID, false);
 
 	if (policy & SEV_POLICY_ES)
 		vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL);
@@ -129,6 +148,33 @@ void sev_vm_launch_finish(struct kvm_vm *vm)
 	TEST_ASSERT_EQ(status.state, SEV_GUEST_STATE_RUNNING);
 }
 
+void snp_vm_launch_start(struct kvm_vm *vm, uint64_t policy)
+{
+	struct kvm_sev_snp_launch_start launch_start = {
+		.policy = policy,
+	};
+
+	vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_START, &launch_start);
+}
+
+void snp_vm_launch_update(struct kvm_vm *vm)
+{
+	struct userspace_mem_region *region;
+	int ctr;
+
+	hash_for_each(vm->regions.slot_hash, ctr, region, slot_node)
+		encrypt_region(vm, region, KVM_SEV_SNP_PAGE_TYPE_NORMAL, true);
+
+	vm->arch.is_pt_protected = true;
+}
+
+void snp_vm_launch_finish(struct kvm_vm *vm)
+{
+	struct kvm_sev_snp_launch_finish launch_finish = { 0 };
+
+	vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_FINISH, &launch_finish);
+}
+
 struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t type, void *guest_code,
 					   struct kvm_vcpu **cpu)
 {
@@ -145,8 +191,20 @@ struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t type, void *guest_code,
 	return vm;
 }
 
-void vm_sev_launch(struct kvm_vm *vm, uint32_t policy, uint8_t *measurement)
+void vm_sev_launch(struct kvm_vm *vm, uint64_t policy, uint8_t *measurement)
 {
+	if (is_sev_snp_vm(vm)) {
+		vm_enable_cap(vm, KVM_CAP_EXIT_HYPERCALL, (1 << KVM_HC_MAP_GPA_RANGE));
+
+		snp_vm_launch_start(vm, policy);
+
+		snp_vm_launch_update(vm);
+
+		snp_vm_launch_finish(vm);
+
+		return;
+	}
+
 	sev_vm_launch(vm, policy);
 
 	if (!measurement)
-- 
2.43.0

[PATCH v8 08/10] KVM: selftests: Force GUEST_MEMFD flag for SNP VM type

[Pratik R. Sampat]

Force the SEV-SNP VM type to set the KVM_MEM_GUEST_MEMFD flag for the
creation of private memslots.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 33fefeb3ca44..089488e2eaf6 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -413,14 +413,17 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
 						 nr_extra_pages);
 	struct userspace_mem_region *slot0;
 	struct kvm_vm *vm;
-	int i;
+	int i, flags = 0;
 
 	pr_debug("%s: mode='%s' type='%d', pages='%ld'\n", __func__,
 		 vm_guest_mode_string(shape.mode), shape.type, nr_pages);
 
 	vm = ____vm_create(shape);
 
-	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, 0);
+	if (shape.type == KVM_X86_SNP_VM)
+		flags |=  KVM_MEM_GUEST_MEMFD;
+
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, flags);
 	for (i = 0; i < NR_MEM_REGIONS; i++)
 		vm->memslots[i] = 0;
 
-- 
2.43.0

[PATCH v8 09/10] KVM: selftests: Abstractions for SEV to decouple policy from type

[Pratik R. Sampat]

In preparation for SNP, cleanup the smoke test to decouple deriving type
from policy. This enables us to reuse existing interfaces as well as
deduplicate the test calls that are called for SEV and SEV-ES.

No functional change intended.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 .../selftests/kvm/x86/sev_smoke_test.c        | 50 ++++++++++---------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
index a2de1e63c3cb..620aa7c41f7a 100644
--- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
@@ -61,7 +61,7 @@ static void compare_xsave(u8 *from_host, u8 *from_guest)
 		abort();
 }
 
-static void test_sync_vmsa(uint32_t policy)
+static void test_sync_vmsa(uint32_t type, uint64_t policy)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
@@ -71,7 +71,7 @@ static void test_sync_vmsa(uint32_t policy)
 	double x87val = M_PI;
 	struct kvm_xsave __attribute__((aligned(64))) xsave = { 0 };
 
-	vm = vm_sev_create_with_one_vcpu(KVM_X86_SEV_ES_VM, guest_code_xsave, &vcpu);
+	vm = vm_sev_create_with_one_vcpu(type, guest_code_xsave, &vcpu);
 	gva = vm_vaddr_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
 				    MEM_REGION_TEST_DATA);
 	hva = addr_gva2hva(vm, gva);
@@ -88,7 +88,7 @@ static void test_sync_vmsa(uint32_t policy)
 	    : "ymm4", "st", "st(1)", "st(2)", "st(3)", "st(4)", "st(5)", "st(6)", "st(7)");
 	vcpu_xsave_set(vcpu, &xsave);
 
-	vm_sev_launch(vm, SEV_POLICY_ES | policy, NULL);
+	vm_sev_launch(vm, policy, NULL);
 
 	/* This page is shared, so make it decrypted.  */
 	memset(hva, 0, 4096);
@@ -107,14 +107,12 @@ static void test_sync_vmsa(uint32_t policy)
 	kvm_vm_free(vm);
 }
 
-static void test_sev(void *guest_code, uint64_t policy)
+static void test_sev(void *guest_code, uint32_t type, uint64_t policy)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 	struct ucall uc;
 
-	uint32_t type = policy & SEV_POLICY_ES ? KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM;
-
 	vm = vm_sev_create_with_one_vcpu(type, guest_code, &vcpu);
 
 	/* TODO: Validate the measurement is as expected. */
@@ -160,16 +158,14 @@ static void guest_shutdown_code(void)
 	__asm__ __volatile__("ud2");
 }
 
-static void test_sev_es_shutdown(void)
+static void test_sev_shutdown(uint32_t type, uint64_t policy)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 
-	uint32_t type = KVM_X86_SEV_ES_VM;
-
 	vm = vm_sev_create_with_one_vcpu(type, guest_shutdown_code, &vcpu);
 
-	vm_sev_launch(vm, SEV_POLICY_ES, NULL);
+	vm_sev_launch(vm, policy, NULL);
 
 	vcpu_run(vcpu);
 	TEST_ASSERT(vcpu->run->exit_reason == KVM_EXIT_SHUTDOWN,
@@ -179,27 +175,33 @@ static void test_sev_es_shutdown(void)
 	kvm_vm_free(vm);
 }
 
-int main(int argc, char *argv[])
+static void test_sev_smoke(void *guest, uint32_t type, uint64_t policy)
 {
 	const u64 xf_mask = XFEATURE_MASK_X87_AVX;
 
-	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SEV));
-
-	test_sev(guest_sev_code, SEV_POLICY_NO_DBG);
-	test_sev(guest_sev_code, 0);
+	test_sev(guest, type, policy | SEV_POLICY_NO_DBG);
+	test_sev(guest, type, policy);
 
-	if (kvm_cpu_has(X86_FEATURE_SEV_ES)) {
-		test_sev(guest_sev_es_code, SEV_POLICY_ES | SEV_POLICY_NO_DBG);
-		test_sev(guest_sev_es_code, SEV_POLICY_ES);
+	if (type == KVM_X86_SEV_VM)
+		return;
 
-		test_sev_es_shutdown();
+	test_sev_shutdown(type, policy);
 
-		if (kvm_has_cap(KVM_CAP_XCRS) &&
-		    (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) {
-			test_sync_vmsa(0);
-			test_sync_vmsa(SEV_POLICY_NO_DBG);
-		}
+	if (kvm_has_cap(KVM_CAP_XCRS) &&
+	    (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) {
+		test_sync_vmsa(type, policy);
+		test_sync_vmsa(type, policy | SEV_POLICY_NO_DBG);
 	}
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SEV));
+
+	test_sev_smoke(guest_sev_code, KVM_X86_SEV_VM, 0);
+
+	if (kvm_cpu_has(X86_FEATURE_SEV_ES))
+		test_sev_smoke(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
 
 	return 0;
 }
-- 
2.43.0

[PATCH v8 10/10] KVM: selftests: Add a basic SEV-SNP smoke test

[Pratik R. Sampat]

Extend sev_smoke_test to also run a minimal SEV-SNP smoke test that
initializes and sets up private memory regions required to run a simple
SEV-SNP guest.

Similar to its SEV-ES smoke test counterpart, this also does not
support GHCB and ucall yet and uses the GHCB MSR protocol to trigger an
exit of the type KVM_EXIT_SYSTEM_EVENT.

Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
---
 .../selftests/kvm/x86/sev_smoke_test.c        | 25 +++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
index 620aa7c41f7a..0505cde77358 100644
--- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
@@ -16,6 +16,18 @@
 
 #define XFEATURE_MASK_X87_AVX (XFEATURE_MASK_FP | XFEATURE_MASK_SSE | XFEATURE_MASK_YMM)
 
+static void guest_snp_code(void)
+{
+	uint64_t sev_msr = rdmsr(MSR_AMD64_SEV);
+
+	GUEST_ASSERT(sev_msr & MSR_AMD64_SEV_ENABLED);
+	GUEST_ASSERT(sev_msr & MSR_AMD64_SEV_ES_ENABLED);
+	GUEST_ASSERT(sev_msr & MSR_AMD64_SEV_SNP_ENABLED);
+
+	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
+	vmgexit();
+}
+
 static void guest_sev_es_code(void)
 {
 	/* TODO: Check CPUID after GHCB-based hypercall support is added. */
@@ -179,7 +191,10 @@ static void test_sev_smoke(void *guest, uint32_t type, uint64_t policy)
 {
 	const u64 xf_mask = XFEATURE_MASK_X87_AVX;
 
-	test_sev(guest, type, policy | SEV_POLICY_NO_DBG);
+	if (type == KVM_X86_SNP_VM)
+		test_sev(guest, type, policy | SNP_POLICY_DBG);
+	else
+		test_sev(guest, type, policy | SEV_POLICY_NO_DBG);
 	test_sev(guest, type, policy);
 
 	if (type == KVM_X86_SEV_VM)
@@ -190,7 +205,10 @@ static void test_sev_smoke(void *guest, uint32_t type, uint64_t policy)
 	if (kvm_has_cap(KVM_CAP_XCRS) &&
 	    (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) {
 		test_sync_vmsa(type, policy);
-		test_sync_vmsa(type, policy | SEV_POLICY_NO_DBG);
+		if (type == KVM_X86_SNP_VM)
+			test_sync_vmsa(type, policy | SNP_POLICY_DBG);
+		else
+			test_sync_vmsa(type, policy | SEV_POLICY_NO_DBG);
 	}
 }
 
@@ -203,5 +221,8 @@ int main(int argc, char *argv[])
 	if (kvm_cpu_has(X86_FEATURE_SEV_ES))
 		test_sev_smoke(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
 
+	if (kvm_cpu_has(X86_FEATURE_SEV_SNP))
+		test_sev_smoke(guest_snp_code, KVM_X86_SNP_VM, snp_default_policy());
+
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v8 03/10] KVM: selftests: Add vmgexit helper

[Gupta, Pankaj]

On 3/5/2025 11:59 PM, Pratik R. Sampat wrote:
> Abstract rep vmmcall coded into the vmgexit helper for the sev
> library.
> 
> No functional change intended.
> 
> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>

Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>

> ---
>   tools/testing/selftests/kvm/include/x86/sev.h    | 5 +++++
>   tools/testing/selftests/kvm/x86/sev_smoke_test.c | 2 +-
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h
> index 82c11c81a956..3003dc837fb7 100644
> --- a/tools/testing/selftests/kvm/include/x86/sev.h
> +++ b/tools/testing/selftests/kvm/include/x86/sev.h
> @@ -71,6 +71,11 @@ kvm_static_assert(SEV_RET_SUCCESS == 0);
>   void sev_vm_init(struct kvm_vm *vm);
>   void sev_es_vm_init(struct kvm_vm *vm);
>   
> +static inline void vmgexit(void)
> +{
> +	__asm__ __volatile__("rep; vmmcall");
> +}
> +
>   static inline void sev_register_encrypted_memory(struct kvm_vm *vm,
>   						 struct userspace_mem_region *region)
>   {
> diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
> index a1a688e75266..6812b94bf5b6 100644
> --- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c
> +++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
> @@ -27,7 +27,7 @@ static void guest_sev_es_code(void)
>   	 * force "termination" to signal "done" via the GHCB MSR protocol.
>   	 */
>   	wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
> -	__asm__ __volatile__("rep; vmmcall");
> +	vmgexit();
>   }
>   
>   static void guest_sev_code(void)

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Pratik R. Sampat]

A very gentle ping on this series.

Thanks
Pratik

On 3/5/25 4:59 PM, Pratik R. Sampat wrote:
> This patch series extends the sev_init2 and the sev_smoke test to
> exercise the SEV-SNP VM launch workflow.
>
> Primarily, it introduces the architectural defines, its support in the
> SEV library and extends the tests to interact with the SEV-SNP ioctl()
> wrappers.
>
> Patch 1  - Do not advertise SNP on initialization failure
> Patch 2  - SNP test for KVM_SEV_INIT2
> Patch 3  - Add vmgexit helper
> Patch 4  - Add SMT control interface helper
> Patch 5  - Replace assert() with TEST_ASSERT_EQ()
> Patch 6  - Introduce SEV+ VM type check
> Patch 7  - SNP iotcl() plumbing for the SEV library
> Patch 8  - Force set GUEST_MEMFD for SNP
> Patch 9  - Cleanups of smoke test - Decouple policy from type
> Patch 10 - SNP smoke test
>
> The series is based on
>          git.kernel.org/pub/scm/virt/kvm/kvm.git next
>
> v7..v8:
> * Dropped exporting the SNP initialized API from ccp to KVM. Instead
>    call SNP_PLATFORM_STATUS within KVM to query the initialization. (Tom)
>    
>    While it may be cheaper to query sev->snp_initialized from ccp, making
>    the SNP platform call within KVM does away with any dependencies.
>
> v6..v7:
> https://lore.kernel.org/kvm/20250221210200.244405-7-prsampat@amd.com/
> Based on comments from Sean -
> * Replaced FW check with sev->snp_initialized
> * Dropped the patch which removes SEV+ KVM advertisement if INIT fails.
>    This should be now be resolved by the combination of the patches [1,2]
>    from Ashish.
> * Change vmgexit to an inline function
> * Export SMT control parsing interface to kvm_util
>    Note: hyperv_cpuid KST only compile tested
> * Replace assert() with TEST_ASSERT_EQ() within SEV library
> * Define KVM_SEV_PAGE_TYPE_INVALID for SEV call of encrypt_region()
> * Parameterize encrypt_region() to include privatize_region()
> * Deduplication of sev test calls between SEV,SEV-ES and SNP
> * Removed FW version tests for SNP
> * Included testing of SNP_POLICY_DBG
> * Dropped most tags from patches that have been changed or indirectly
>    affected
>
> [1] https://lore.kernel.org/all/d6d08c6b-9602-4f3d-92c2-8db6d50a1b92@amd.com
> [2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226950.git.ashish.kalra@amd.com
>
> v5..v6:
> https://lore.kernel.org/kvm/ab433246-e97c-495b-ab67-b0cb1721fb99@amd.com/
> * Rename is_sev_platform_init to sev_fw_initialized (Nikunj)
> * Rename KVM CPU feature X86_FEATURE_SNP to X86_FEATURE_SEV_SNP (Nikunj)
> * Collected Tags from Nikunj, Pankaj, Srikanth.
>
> v4..v5:
> https://lore.kernel.org/kvm/8e7d8172-879e-4a28-8438-343b1c386ec9@amd.com/
> * Introduced a check to disable advertising support for SEV, SEV-ES
>    and SNP when platform initialization fails (Nikunj)
> * Remove the redundant SNP check within is_sev_vm() (Nikunj)
> * Cleanup of the encrypt_region flow for better readability (Nikunj)
> * Refactor paths to use the canonical $(ARCH) to rebase for kvm/next
>
> v3..v4:
> https://lore.kernel.org/kvm/20241114234104.128532-1-pratikrajesh.sampat@amd.com/
> * Remove SNP FW API version check in the test and ensure the KVM
>    capability advertises the presence of the feature. Retain the minimum
>    version definitions to exercise these API versions in the smoke test
> * Retained only the SNP smoke test and SNP_INIT2 test
> * The SNP architectural defined merged with SNP_INIT2 test patch
> * SNP shutdown merged with SNP smoke test patch
> * Add SEV VM type check to abstract comparisons and reduce clutter
> * Define a SNP default policy which sets bits based on the presence of
>    SMT
> * Decouple privatization and encryption for it to be SNP agnostic
> * Assert for only positive tests using vm_ioctl()
> * Dropped tested-by tags
>
> In summary - based on comments from Sean, I have primarily reduced the
> scope of this patch series to focus on breaking down the SNP smoke test
> patch (v3 - patch2) to first introduce SEV-SNP support and use this
> interface to extend the sev_init2 and the sev_smoke test.
>
> The rest of the v3 patchset that introduces ioctl, pre fault, fallocate
> and negative tests, will be re-worked and re-introduced subsequently in
> future patch series post addressing the issues discussed.
>
> v2..v3:
> https://lore.kernel.org/kvm/20240905124107.6954-1-pratikrajesh.sampat@amd.com/
> * Remove the assignments for the prefault and fallocate test type
>    enums.
> * Fix error message for sev launch measure and finish.
> * Collect tested-by tags [Peter, Srikanth]
>
> Pratik R. Sampat (10):
>    KVM: SEV: Disable SEV-SNP support on initialization failure
>    KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
>    KVM: selftests: Add vmgexit helper
>    KVM: selftests: Add SMT control state helper
>    KVM: selftests: Replace assert() with TEST_ASSERT_EQ()
>    KVM: selftests: Introduce SEV VM type check
>    KVM: selftests: Add library support for interacting with SNP
>    KVM: selftests: Force GUEST_MEMFD flag for SNP VM type
>    KVM: selftests: Abstractions for SEV to decouple policy from type
>    KVM: selftests: Add a basic SEV-SNP smoke test
>
>   arch/x86/include/uapi/asm/kvm.h               |  1 +
>   arch/x86/kvm/svm/sev.c                        | 30 +++++-
>   tools/arch/x86/include/uapi/asm/kvm.h         |  1 +
>   .../testing/selftests/kvm/include/kvm_util.h  | 35 +++++++
>   .../selftests/kvm/include/x86/processor.h     |  1 +
>   tools/testing/selftests/kvm/include/x86/sev.h | 42 ++++++++-
>   tools/testing/selftests/kvm/lib/kvm_util.c    |  7 +-
>   .../testing/selftests/kvm/lib/x86/processor.c |  4 +-
>   tools/testing/selftests/kvm/lib/x86/sev.c     | 93 +++++++++++++++++--
>   .../testing/selftests/kvm/x86/hyperv_cpuid.c  | 19 ----
>   .../selftests/kvm/x86/sev_init2_tests.c       | 13 +++
>   .../selftests/kvm/x86/sev_smoke_test.c        | 75 +++++++++------
>   12 files changed, 261 insertions(+), 60 deletions(-)
>

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Sean Christopherson]

On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
> This patch series extends the sev_init2 and the sev_smoke test to
> exercise the SEV-SNP VM launch workflow.
> 
> Primarily, it introduces the architectural defines, its support in the
> SEV library and extends the tests to interact with the SEV-SNP ioctl()
> wrappers.
> 
> [...]

Applied 2-9 to kvm-x86 selftests.  AIUI, the KVM side of things should already
be fixed.  If KVM isn't fixed, I want to take that discussion/patch to a
separate thread.

I made minor changes along the way (some details in the commits' []), please
holler if you disagree with the end result.

[01/10] KVM: SEV: Disable SEV-SNP support on initialization failure
        (no commit info)
[02/10] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
        https://github.com/kvm-x86/linux/commit/68ed692e3954
[03/10] KVM: selftests: Add vmgexit helper
        https://github.com/kvm-x86/linux/commit/c4e1a848d721
[04/10] KVM: selftests: Add SMT control state helper
        https://github.com/kvm-x86/linux/commit/acf064345018
[05/10] KVM: selftests: Replace assert() with TEST_ASSERT_EQ()
        https://github.com/kvm-x86/linux/commit/f694f30e81c4
[06/10] KVM: selftests: Introduce SEV VM type check
        https://github.com/kvm-x86/linux/commit/4a4e1e8e92eb
[07/10] KVM: selftests: Add library support for interacting with SNP
        https://github.com/kvm-x86/linux/commit/3bf3e0a52123
[08/10] KVM: selftests: Force GUEST_MEMFD flag for SNP VM type
        https://github.com/kvm-x86/linux/commit/b73a30cd9caa
[09/10] KVM: selftests: Abstractions for SEV to decouple policy from type
        https://github.com/kvm-x86/linux/commit/a5d55f783fb7
[10/10] KVM: selftests: Add a basic SEV-SNP smoke test
        https://github.com/kvm-x86/linux/commit/ada014f5fc67

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Pratik R. Sampat]

Hi Sean,

On 5/2/25 4:50 PM, Sean Christopherson wrote:
> On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
>> This patch series extends the sev_init2 and the sev_smoke test to
>> exercise the SEV-SNP VM launch workflow.
>>
>> Primarily, it introduces the architectural defines, its support in the
>> SEV library and extends the tests to interact with the SEV-SNP ioctl()
>> wrappers.
>>
>> [...]
> 
> Applied 2-9 to kvm-x86 selftests.  AIUI, the KVM side of things should already
> be fixed.  If KVM isn't fixed, I want to take that discussion/patch to a
> separate thread.
> 

Thanks for pulling these patches in.

For 1 - Ashish's commit now returns failure for this case [1].
Although, it appears that the return code isn't checked within
sev_platform_init()[2], so it shouldn't change existing behavior. In the
kselftest case, if platform init fails, the selftest will also fail — just as
it does currently too.

Regardless of what we decide on what the right behavior is, fail vs skip (I
don't mind the former) we can certainly do that over new patches rebased over
the new series.

[1]: https://lore.kernel.org/kvm/ab9a028cf232663f9fc839f48cfcf97694846c13.1742850400.git.ashish.kalra@amd.com/
[2]: https://lore.kernel.org/kvm/d8de6de80c36721ea3eb92ecac81b211f401c3b2.1742850400.git.ashish.kalra@amd.com/

> I made minor changes along the way (some details in the commits' []), please
> holler if you disagree with the end result.

Thank you for cleaning these up!

Pratik

> 
> [01/10] KVM: SEV: Disable SEV-SNP support on initialization failure
>         (no commit info)
> [02/10] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
>         https://github.com/kvm-x86/linux/commit/68ed692e3954
> [03/10] KVM: selftests: Add vmgexit helper
>         https://github.com/kvm-x86/linux/commit/c4e1a848d721
> [04/10] KVM: selftests: Add SMT control state helper
>         https://github.com/kvm-x86/linux/commit/acf064345018
> [05/10] KVM: selftests: Replace assert() with TEST_ASSERT_EQ()
>         https://github.com/kvm-x86/linux/commit/f694f30e81c4
> [06/10] KVM: selftests: Introduce SEV VM type check
>         https://github.com/kvm-x86/linux/commit/4a4e1e8e92eb
> [07/10] KVM: selftests: Add library support for interacting with SNP
>         https://github.com/kvm-x86/linux/commit/3bf3e0a52123
> [08/10] KVM: selftests: Force GUEST_MEMFD flag for SNP VM type
>         https://github.com/kvm-x86/linux/commit/b73a30cd9caa
> [09/10] KVM: selftests: Abstractions for SEV to decouple policy from type
>         https://github.com/kvm-x86/linux/commit/a5d55f783fb7
> [10/10] KVM: selftests: Add a basic SEV-SNP smoke test
>         https://github.com/kvm-x86/linux/commit/ada014f5fc67
> 
> --
> https://github.com/kvm-x86/linux/tree/next

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Sean Christopherson]

On Mon, May 05, 2025, Pratik R. Sampat wrote:
> Hi Sean,
> 
> On 5/2/25 4:50 PM, Sean Christopherson wrote:
> > On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
> >> This patch series extends the sev_init2 and the sev_smoke test to
> >> exercise the SEV-SNP VM launch workflow.
> >>
> >> Primarily, it introduces the architectural defines, its support in the
> >> SEV library and extends the tests to interact with the SEV-SNP ioctl()
> >> wrappers.
> >>
> >> [...]
> > 
> > Applied 2-9 to kvm-x86 selftests.  AIUI, the KVM side of things should already
> > be fixed.  If KVM isn't fixed, I want to take that discussion/patch to a
> > separate thread.
> > 
> 
> Thanks for pulling these patches in.
> 
> For 1 - Ashish's commit now returns failure for this case [1].
> Although, it appears that the return code isn't checked within
> sev_platform_init()[2], so it shouldn't change existing behavior. In the
> kselftest case, if platform init fails, the selftest will also fail — just as
> it does currently too.

Argh, now I remember the issue.  But _sev_platform_init_locked() returns '0' if
psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked()
will magically make it succeed the second time around.

So shouldn't the KVM code be this?

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index e0f446922a6e..dd04f979357d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void)
        sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
 
 out:
+       if (sev_enabled) {
+               init_args.probe = true;
+               if (sev_platform_init(&init_args))
+                       sev_supported = sev_es_supported = sev_snp_supported = false;
+               else
+                       sev_snp_supported &= sev_is_snp_initialized();
+       }
+
        if (boot_cpu_has(X86_FEATURE_SEV))
                pr_info("SEV %s (ASIDs %u - %u)\n",
                        sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" :
@@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
 
        if (!sev_enabled)
                return;
-
-       /*
-        * Do both SNP and SEV initialization at KVM module load.
-        */
-       init_args.probe = true;
-       sev_platform_init(&init_args);
 }
 
 void sev_hardware_unsetup(void)
--

Ashish, what am I missing?

> Regardless of what we decide on what the right behavior is, fail vs skip (I
> don't mind the former) we can certainly do that over new patches rebased over
> the new series.

FAIL, for sure.  Unless someone else pipes up with a good reason why they need
to defer INIT_EX, that's Google's problem to solve.

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Kalra, Ashish]

Hello Sean,

On 5/5/2025 6:15 PM, Sean Christopherson wrote:
> On Mon, May 05, 2025, Pratik R. Sampat wrote:
>> Hi Sean,
>>
>> On 5/2/25 4:50 PM, Sean Christopherson wrote:
>>> On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
>>>> This patch series extends the sev_init2 and the sev_smoke test to
>>>> exercise the SEV-SNP VM launch workflow.
>>>>
>>>> Primarily, it introduces the architectural defines, its support in the
>>>> SEV library and extends the tests to interact with the SEV-SNP ioctl()
>>>> wrappers.
>>>>
>>>> [...]
>>>
>>> Applied 2-9 to kvm-x86 selftests.  AIUI, the KVM side of things should already
>>> be fixed.  If KVM isn't fixed, I want to take that discussion/patch to a
>>> separate thread.
>>>
>>
>> Thanks for pulling these patches in.
>>
>> For 1 - Ashish's commit now returns failure for this case [1].
>> Although, it appears that the return code isn't checked within
>> sev_platform_init()[2], so it shouldn't change existing behavior. In the
>> kselftest case, if platform init fails, the selftest will also fail — just as
>> it does currently too.
> 
> Argh, now I remember the issue.  But _sev_platform_init_locked() returns '0' if
> psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked()
> will magically make it succeed the second time around.
> 
> So shouldn't the KVM code be this?
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index e0f446922a6e..dd04f979357d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void)
>         sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>  
>  out:
> +       if (sev_enabled) {
> +               init_args.probe = true;
> +               if (sev_platform_init(&init_args))
> +                       sev_supported = sev_es_supported = sev_snp_supported = false;
> +               else
> +                       sev_snp_supported &= sev_is_snp_initialized();
> +       }
> +
>         if (boot_cpu_has(X86_FEATURE_SEV))
>                 pr_info("SEV %s (ASIDs %u - %u)\n",
>                         sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" :
> @@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
>  
>         if (!sev_enabled)
>                 return;
> -
> -       /*
> -        * Do both SNP and SEV initialization at KVM module load.
> -        */
> -       init_args.probe = true;
> -       sev_platform_init(&init_args);
>  }
>  
>  void sev_hardware_unsetup(void)
> --
> 
> Ashish, what am I missing?
> 

As far as setting sev*_enabled is concerned, i believe they are more specific to SNP/SEV/SEV-ES being enabled in the system,
which is separate from SEV_INIT/SNP_INIT (SNP_INIT success indicates that RMP been initialized, SNP has to be already enabled via 
MSR_SYSCFG before SNP_INIT is called), though SEV_INIT/SNP_INIT may fail but SEV/SNP support will still be enabled on the
system.

Additionally as SEV_INIT/SNP_INIT during sev_platform_init() have failed, so any SEV/SEV-ES/SNP VM launch will fail
as the firmware will return invalid platform state as INITs have failed.
 
From my understanding, sev*_enabled indicates the user support to enable/disable support for SEV/SEV-ES/SEV-SNP, 
as the sev*_enabled are the KVM module parameters, while sev*_supported indicates if platform has that support enabled.

And before the SEV/SNP init support was moved to KVM from CCP module, doing SEV/SNP INIT could fail but that still
had KVM detecting SEV/SNP support enabled, so this moving SEV/SNP init stuff to KVM module from CCP driver is
consistent with the previous behavior.
 
Thanks,
Ashish

>> Regardless of what we decide on what the right behavior is, fail vs skip (I
>> don't mind the former) we can certainly do that over new patches rebased over
>> the new series.
> 
> FAIL, for sure.  Unless someone else pipes up with a good reason why they need
> to defer INIT_EX, that's Google's problem to solve.

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Sean Christopherson]

On Mon, May 05, 2025, Ashish Kalra wrote:
> On 5/5/2025 6:15 PM, Sean Christopherson wrote:
> > @@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
> >  
> >         if (!sev_enabled)
> >                 return;
> > -
> > -       /*
> > -        * Do both SNP and SEV initialization at KVM module load.
> > -        */
> > -       init_args.probe = true;
> > -       sev_platform_init(&init_args);
> >  }
> >  
> >  void sev_hardware_unsetup(void)
> > --
> > 
> > Ashish, what am I missing?
> > 
> 
> As far as setting sev*_enabled is concerned, i believe they are more specific
> to SNP/SEV/SEV-ES being enabled in the system, which is separate from
> SEV_INIT/SNP_INIT (SNP_INIT success indicates that RMP been initialized, SNP
> has to be already enabled via MSR_SYSCFG before SNP_INIT is called), though
> SEV_INIT/SNP_INIT may fail but SEV/SNP support will still be enabled on the
> system.

No, if SNP_INIT fails and has zero chance of succeeding, then SNP is *NOT*
supported *by KVM*.  The platform may be configured to support SNP, but that
matters not at all if KVM can't actually use the functionality.

> Additionally as SEV_INIT/SNP_INIT during sev_platform_init() have failed, so
> any SEV/SEV-ES/SNP VM launch will fail as the firmware will return invalid
> platform state as INITs have failed.

Yeah, and that's *awful* behavior for KVM.  Imagine if KVM did that for every
feature, i.e. enumerated hardware support irrespective of KVM support.

The API is KVM_GET_SUPPORTED_CPUID, not KVM_GET_MOSTLY_SUPPORTED_CPUID.

> >From my understanding, sev*_enabled indicates the user support to
> >enable/disable support for SEV/SEV-ES/SEV-SNP, 

Yes, and they're also used to reflect and enumerate KVM support:

	if (sev_enabled) {
		kvm_cpu_cap_set(X86_FEATURE_SEV);
		kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_VM);
	}
	if (sev_es_enabled) {
		kvm_cpu_cap_set(X86_FEATURE_SEV_ES);
		kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_ES_VM);
	}
	if (sev_snp_enabled) {
		kvm_cpu_cap_set(X86_FEATURE_SEV_SNP);
		kvm_caps.supported_vm_types |= BIT(KVM_X86_SNP_VM);
	}

> as the sev*_enabled are the KVM module parameters, while sev*_supported
> indicates if platform has that support enabled.

sev*_supported are completely irrelevant.  They are function local scratch variables
that exist so that KVM doesn't clobber userspace's inputs while computing what is
fully supported and enabled.

> And before the SEV/SNP init support was moved to KVM from CCP module, doing
> SEV/SNP INIT could fail but that still had KVM detecting SEV/SNP support
> enabled, so this moving SEV/SNP init stuff to KVM module from CCP driver is
> consistent with the previous behavior.

And one of my driving motivations for getting the initialization into KVM was to
fix that previous behavior.

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Pratik R. Sampat]

On 5/5/2025 6:15 PM, Sean Christopherson wrote:
> On Mon, May 05, 2025, Pratik R. Sampat wrote:
>> Hi Sean,
>>
>> On 5/2/25 4:50 PM, Sean Christopherson wrote:
>>> On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
>>>> This patch series extends the sev_init2 and the sev_smoke test to
>>>> exercise the SEV-SNP VM launch workflow.
>>>>
>>>> Primarily, it introduces the architectural defines, its support in the
>>>> SEV library and extends the tests to interact with the SEV-SNP ioctl()
>>>> wrappers.
>>>>
>>>> [...]
>>>
>>> Applied 2-9 to kvm-x86 selftests.  AIUI, the KVM side of things should already
>>> be fixed.  If KVM isn't fixed, I want to take that discussion/patch to a
>>> separate thread.
>>>
>>
>> Thanks for pulling these patches in.
>>
>> For 1 - Ashish's commit now returns failure for this case [1].
>> Although, it appears that the return code isn't checked within
>> sev_platform_init()[2], so it shouldn't change existing behavior. In the
>> kselftest case, if platform init fails, the selftest will also fail — just as
>> it does currently too.
> 
> Argh, now I remember the issue.  But _sev_platform_init_locked() returns '0' if
> psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked()
> will magically make it succeed the second time around.
> 
> So shouldn't the KVM code be this?
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index e0f446922a6e..dd04f979357d 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void)
>         sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
>  
>  out:
> +       if (sev_enabled) {
> +               init_args.probe = true;
> +               if (sev_platform_init(&init_args))
> +                       sev_supported = sev_es_supported = sev_snp_supported = false;
> +               else
> +                       sev_snp_supported &= sev_is_snp_initialized();
> +       }
> +
>         if (boot_cpu_has(X86_FEATURE_SEV))
>                 pr_info("SEV %s (ASIDs %u - %u)\n",
>                         sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" :
> @@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
>  
>         if (!sev_enabled)
>                 return;
> -
> -       /*
> -        * Do both SNP and SEV initialization at KVM module load.
> -        */
> -       init_args.probe = true;
> -       sev_platform_init(&init_args);
>  }
>  
>  void sev_hardware_unsetup(void)
> --
> 

I agree with this approach. One thing maybe to consider further is to also call
into SEV_platform_status() to check for init so that SEV/SEV-ES is not
penalized and disabled for SNP's failures. Another approach could be to break
up the SEV and SNP init setup so that we can spare a couple of platform calls
in the process?

> Ashish, what am I missing?
> 
>> Regardless of what we decide on what the right behavior is, fail vs skip (I
>> don't mind the former) we can certainly do that over new patches rebased over
>> the new series.
> 
> FAIL, for sure.  Unless someone else pipes up with a good reason why they need
> to defer INIT_EX, that's Google's problem to solve.
Ack!

Pratik

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Sean Christopherson]

On Mon, May 05, 2025, Pratik R. Sampat wrote:
> On 5/5/2025 6:15 PM, Sean Christopherson wrote:
> > On Mon, May 05, 2025, Pratik R. Sampat wrote:
> > Argh, now I remember the issue.  But _sev_platform_init_locked() returns '0' if
> > psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked()
> > will magically make it succeed the second time around.
> > 
> > So shouldn't the KVM code be this?
> > 
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index e0f446922a6e..dd04f979357d 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void)
> >         sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
> >  
> >  out:
> > +       if (sev_enabled) {
> > +               init_args.probe = true;
> > +               if (sev_platform_init(&init_args))
> > +                       sev_supported = sev_es_supported = sev_snp_supported = false;
> > +               else
> > +                       sev_snp_supported &= sev_is_snp_initialized();
> > +       }
> > +
> >         if (boot_cpu_has(X86_FEATURE_SEV))
> >                 pr_info("SEV %s (ASIDs %u - %u)\n",
> >                         sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" :
> > @@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
> >  
> >         if (!sev_enabled)
> >                 return;
> > -
> > -       /*
> > -        * Do both SNP and SEV initialization at KVM module load.
> > -        */
> > -       init_args.probe = true;
> > -       sev_platform_init(&init_args);
> >  }
> >  
> >  void sev_hardware_unsetup(void)
> > --
> > 
> 
> I agree with this approach. One thing maybe to consider further is to also call
> into SEV_platform_status() to check for init so that SEV/SEV-ES is not
> penalized and disabled for SNP's failures. Another approach could be to break
> up the SEV and SNP init setup so that we can spare a couple of platform calls
> in the process?

Nah, SNP initialization failure should be a rare occurence, I don't want to make
the "normal" flow more complex just to handle something that should never happen
in practice.

Re: [PATCH v8 00/10] Basic SEV-SNP Selftests

[Kalra, Ashish]

Hello Sean,

On 5/5/2025 7:56 PM, Sean Christopherson wrote:
> On Mon, May 05, 2025, Ashish Kalra wrote:
>> On 5/5/2025 6:15 PM, Sean Christopherson wrote:
>>> @@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
>>>  
>>>         if (!sev_enabled)
>>>                 return;
>>> -
>>> -       /*
>>> -        * Do both SNP and SEV initialization at KVM module load.
>>> -        */
>>> -       init_args.probe = true;
>>> -       sev_platform_init(&init_args);
>>>  }
>>>  
>>>  void sev_hardware_unsetup(void)
>>> --
>>>
>>> Ashish, what am I missing?
>>>
>>
>> As far as setting sev*_enabled is concerned, i believe they are more specific
>> to SNP/SEV/SEV-ES being enabled in the system, which is separate from
>> SEV_INIT/SNP_INIT (SNP_INIT success indicates that RMP been initialized, SNP
>> has to be already enabled via MSR_SYSCFG before SNP_INIT is called), though
>> SEV_INIT/SNP_INIT may fail but SEV/SNP support will still be enabled on the
>> system.
> 
> No, if SNP_INIT fails and has zero chance of succeeding, then SNP is *NOT*
> supported *by KVM*.  The platform may be configured to support SNP, but that
> matters not at all if KVM can't actually use the functionality.
>

Yes, i agree.
 
>> Additionally as SEV_INIT/SNP_INIT during sev_platform_init() have failed, so
>> any SEV/SEV-ES/SNP VM launch will fail as the firmware will return invalid
>> platform state as INITs have failed.
> 
> Yeah, and that's *awful* behavior for KVM.  Imagine if KVM did that for every
> feature, i.e. enumerated hardware support irrespective of KVM support.
> 
> The API is KVM_GET_SUPPORTED_CPUID, not KVM_GET_MOSTLY_SUPPORTED_CPUID.
> 
>> >From my understanding, sev*_enabled indicates the user support to
>>> enable/disable support for SEV/SEV-ES/SEV-SNP, 
> 
> Yes, and they're also used to reflect and enumerate KVM support:
> 
> 	if (sev_enabled) {
> 		kvm_cpu_cap_set(X86_FEATURE_SEV);
> 		kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_VM);
> 	}
> 	if (sev_es_enabled) {
> 		kvm_cpu_cap_set(X86_FEATURE_SEV_ES);
> 		kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_ES_VM);
> 	}
> 	if (sev_snp_enabled) {
> 		kvm_cpu_cap_set(X86_FEATURE_SEV_SNP);
> 		kvm_caps.supported_vm_types |= BIT(KVM_X86_SNP_VM);
> 	}
> 
>> as the sev*_enabled are the KVM module parameters, while sev*_supported
>> indicates if platform has that support enabled.
> 
> sev*_supported are completely irrelevant.  They are function local scratch variables
> that exist so that KVM doesn't clobber userspace's inputs while computing what is
> fully supported and enabled.
> 

Ok.

>> And before the SEV/SNP init support was moved to KVM from CCP module, doing
>> SEV/SNP INIT could fail but that still had KVM detecting SEV/SNP support
>> enabled, so this moving SEV/SNP init stuff to KVM module from CCP driver is
>> consistent with the previous behavior.
> 
> And one of my driving motivations for getting the initialization into KVM was to
> fix that previous behavior.

Sure, i will test your patch and post it.  

Thanks,
Ashish