[PATCH v4 0/8] Basic SEV-SNP Selftests

[PATCH v4 0/8] 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 advertize SNP on incompatible firmware
Patch 2 - SNP test for KVM_SEV_INIT2
Patch 3 - Add VMGEXIT helper
Patch 4 - Introduce SEV+ VM type check
Patch 5 - SNP iotcl() plumbing for the SEV library
Patch 6 - Force set GUEST_MEMFD for SNP
Patch 7 - Cleanups of smoke test - Decouple policy from type
Patch 8 - SNP smoke test

v4:
1. Remove SNP FW API version check in the test and ensure the KVM
   capability advertizes the presence of the feature. Retain the minimum
   version definitions to exercise these API versions in the smoke test.
2. Retained only the SNP smoke test and SNP_INIT2 test
3. The SNP architectural defined merged with SNP_INIT2 test patch
4. SNP shutdown merged with SNP smoke test patch
5. Add SEV VM type check to abstract comparisons and reduce clutter
6. Define a SNP default policy which sets bits based on the presence of
   SMT
7. Decouple privatization and encryption for it to be SNP agnostic
8. Assert for only positive tests using vm_ioctl()
9. 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.

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

Any feedback/review is highly appreciated!

Pratik R. Sampat (8):
  KVM: SEV: Disable SEV-SNP on FW validation failure
  KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
  KVM: selftests: Add VMGEXIT helper
  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/kvm/svm/sev.c                        |  4 +-
 drivers/crypto/ccp/sev-dev.c                  |  6 ++
 include/linux/psp-sev.h                       |  3 +
 .../selftests/kvm/include/x86_64/processor.h  |  1 +
 .../selftests/kvm/include/x86_64/sev.h        | 55 ++++++++++-
 tools/testing/selftests/kvm/lib/kvm_util.c    |  7 +-
 .../selftests/kvm/lib/x86_64/processor.c      |  4 +-
 tools/testing/selftests/kvm/lib/x86_64/sev.c  | 98 ++++++++++++++++++-
 .../selftests/kvm/x86_64/sev_init2_tests.c    | 13 +++
 .../selftests/kvm/x86_64/sev_smoke_test.c     | 96 ++++++++++++++----
 10 files changed, 258 insertions(+), 29 deletions(-)

-- 
2.43.0

[PATCH v4 1/8] KVM: SEV: Disable SEV-SNP on FW validation failure

[Pratik R. Sampat]

On incompatible firmware versions, SEV-SNP support is pulled and the
setup is not performed. However, the platform and subsequently the KVM
capability may continue to advertize support for it. Disable support for
SEV-SNP if the FW version validation fails.

Fixes: 1dfe571c12cf ("KVM: SEV: Add initial SEV-SNP support")
Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
---
 arch/x86/kvm/svm/sev.c       | 4 +++-
 drivers/crypto/ccp/sev-dev.c | 6 ++++++
 include/linux/psp-sev.h      | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 72674b8825c4..5ced00e54f0e 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3053,7 +3053,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_fw_valid());
 
 out:
 	if (boot_cpu_has(X86_FEATURE_SEV))
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index af018afd9cd7..b45cd60c19b0 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -122,6 +122,12 @@ static inline bool sev_version_greater_or_equal(u8 maj, u8 min)
 	return false;
 }
 
+bool snp_fw_valid(void)
+{
+	return sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
+}
+EXPORT_SYMBOL_GPL(snp_fw_valid);
+
 static void sev_irq_handler(int irq, void *data, unsigned int status)
 {
 	struct sev_device *sev = data;
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 903ddfea8585..e841a8fbbb15 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -945,6 +945,7 @@ int sev_do_cmd(int cmd, void *data, int *psp_ret);
 void *psp_copy_user_blob(u64 uaddr, u32 len);
 void *snp_alloc_firmware_page(gfp_t mask);
 void snp_free_firmware_page(void *addr);
+bool snp_fw_valid(void);
 
 #else	/* !CONFIG_CRYPTO_DEV_SP_PSP */
 
@@ -979,6 +980,8 @@ static inline void *snp_alloc_firmware_page(gfp_t mask)
 
 static inline void snp_free_firmware_page(void *addr) { }
 
+static inline bool snp_fw_valid(void) { return false; }
+
 #endif	/* CONFIG_CRYPTO_DEV_SP_PSP */
 
 #endif	/* __PSP_SEV_H__ */
-- 
2.43.0

[PATCH v4 2/8] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2

[Pratik R. Sampat]

Add the X86_FEATURE_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, so that a failure is not reported in such cases.

Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
---
 .../selftests/kvm/include/x86_64/processor.h        |  1 +
 .../testing/selftests/kvm/x86_64/sev_init2_tests.c  | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
index 645200e95f89..c18d2405798f 100644
--- a/tools/testing/selftests/kvm/include/x86_64/processor.h
+++ b/tools/testing/selftests/kvm/include/x86_64/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_SNP			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 4)
 
 /*
  * KVM defined paravirt features.
diff --git a/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c b/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
index 3fb967f40c6a..3f8fb2cc3431 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
+++ b/tools/testing/selftests/kvm/x86_64/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_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 v4 3/8] 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 <pratikrajesh.sampat@amd.com>
---
 tools/testing/selftests/kvm/include/x86_64/sev.h    | 2 ++
 tools/testing/selftests/kvm/x86_64/sev_smoke_test.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
index 82c11c81a956..e7df5d0987f6 100644
--- a/tools/testing/selftests/kvm/include/x86_64/sev.h
+++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
@@ -27,6 +27,8 @@ enum sev_guest_state {
 
 #define GHCB_MSR_TERM_REQ	0x100
 
+#define VMGEXIT()		{ __asm__ __volatile__("rep; vmmcall"); }
+
 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/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
index ae77698e6e97..97d9989c8011 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/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 v4 4/8] 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 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 <pratikrajesh.sampat@amd.com>
---
 .../testing/selftests/kvm/include/x86_64/sev.h  |  4 ++++
 .../selftests/kvm/lib/x86_64/processor.c        |  4 ++--
 tools/testing/selftests/kvm/lib/x86_64/sev.c    | 17 +++++++++++++++++
 .../selftests/kvm/x86_64/sev_smoke_test.c       |  2 +-
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
index e7df5d0987f6..faed91435963 100644
--- a/tools/testing/selftests/kvm/include/x86_64/sev.h
+++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
@@ -29,6 +29,10 @@ enum sev_guest_state {
 
 #define VMGEXIT()		{ __asm__ __volatile__("rep; vmmcall"); }
 
+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_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 636b29ba8985..13f060748fc2 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -641,7 +641,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);
@@ -1158,7 +1158,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_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
index e9535ee20b7f..d6e7a422b69d 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/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_snp_vm(vm) || 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_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
index 97d9989c8011..53bc0af62bad 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/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 v4 5/8] 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 <pratikrajesh.sampat@amd.com>
---
 .../selftests/kvm/include/x86_64/sev.h        | 49 ++++++++++-
 tools/testing/selftests/kvm/lib/x86_64/sev.c  | 81 ++++++++++++++++++-
 2 files changed, 125 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
index faed91435963..19454b0e10a6 100644
--- a/tools/testing/selftests/kvm/include/x86_64/sev.h
+++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
@@ -22,9 +22,20 @@ enum sev_guest_state {
 	SEV_GUEST_STATE_RUNNING,
 };
 
+/* Minimum firmware version required for the SEV-SNP support */
+#define SNP_MIN_API_MAJOR	1
+#define SNP_MIN_API_MINOR	51
+
 #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 SNP_FW_VER_MINOR(min)	((uint8_t)(min) << 0)
+#define SNP_FW_VER_MAJOR(maj)	((uint8_t)(maj) << 8)
+
 #define GHCB_MSR_TERM_REQ	0x100
 
 #define VMGEXIT()		{ __asm__ __volatile__("rep; vmmcall"); }
@@ -36,13 +47,35 @@ 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 default 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)
+{
+	bool smt_active = false;
+	FILE *f;
+
+	f = fopen("/sys/devices/system/cpu/smt/active", "r");
+	if (f) {
+		smt_active = fgetc(f) - '0';
+		fclose(f);
+	}
+
+	return SNP_POLICY_RSVD_MBO | (smt_active ? 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
@@ -76,6 +109,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 sev_register_encrypted_memory(struct kvm_vm *vm,
 						 struct userspace_mem_region *region)
@@ -99,4 +133,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_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
index d6e7a422b69d..40b90d3a5769 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/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)
 {
 	const struct sparsebit *protected_phy_pages = region->protected_phy_pages;
 	const vm_paddr_t gpa_base = region->region.guest_phys_addr;
@@ -41,16 +42,39 @@ 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;
 
+		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);
+			continue;
+		}
+
 		sev_launch_update_data(vm, gpa_base + offset, size);
 	}
 }
 
+static void privatize_region(struct kvm_vm *vm, struct userspace_mem_region *region)
+{
+	const struct sparsebit *protected_phy_pages = region->protected_phy_pages;
+	const vm_paddr_t gpa_base = region->region.guest_phys_addr;
+	const sparsebit_idx_t lowest_page_in_region = gpa_base >> vm->page_shift;
+	sparsebit_idx_t i, j;
+
+	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;
+
+		vm_mem_set_private(vm, gpa_base + offset, size);
+	}
+}
+
 void sev_vm_init(struct kvm_vm *vm)
 {
 	if (vm->type == KVM_X86_DEFAULT_VM) {
@@ -77,6 +101,14 @@ void sev_es_vm_init(struct kvm_vm *vm)
 	}
 }
 
+void snp_vm_init(struct kvm_vm *vm)
+{
+	struct kvm_sev_init init = { 0 };
+
+	assert(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 +125,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, 0);
 
 	if (policy & SEV_POLICY_ES)
 		vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL);
@@ -129,6 +161,35 @@ 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) {
+		privatize_region(vm, region);
+		encrypt_region(vm, region, KVM_SEV_SNP_PAGE_TYPE_NORMAL);
+	}
+
+	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 +206,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 v4 6/8] 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 <pratikrajesh.sampat@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 480e3a40d197..26179fb2f0e7 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 v4 7/8] 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. Introduce, wrappers for SEV and SEV-ES types to
abstract the parametrized launch tests calls and reduce verbosity.

No functional change intended.

Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
---
 .../selftests/kvm/x86_64/sev_smoke_test.c     | 50 ++++++++++++-------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
index 53bc0af62bad..af1beabbbf8e 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/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. */
@@ -149,6 +147,21 @@ static void test_sev(void *guest_code, uint64_t policy)
 	kvm_vm_free(vm);
 }
 
+static void test_sev(uint64_t policy)
+{
+	__test_sev(guest_sev_code, KVM_X86_SEV_VM, policy);
+}
+
+static void test_sev_es(uint64_t policy)
+{
+	__test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, policy);
+}
+
+static void test_sync_vmsa_sev_es(uint64_t policy)
+{
+	__test_sync_vmsa(KVM_X86_SEV_ES_VM, policy);
+}
+
 static void guest_shutdown_code(void)
 {
 	struct desc_ptr idt;
@@ -160,16 +173,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,25 +190,30 @@ static void test_sev_es_shutdown(void)
 	kvm_vm_free(vm);
 }
 
+static void test_sev_es_shutdown(uint64_t policy)
+{
+	__test_sev_shutdown(KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
+}
+
 int main(int argc, char *argv[])
 {
 	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(SEV_POLICY_NO_DBG);
+	test_sev(0);
 
 	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);
+		test_sev_es(SEV_POLICY_ES | SEV_POLICY_NO_DBG);
+		test_sev_es(SEV_POLICY_ES);
 
-		test_sev_es_shutdown();
+		test_sev_es_shutdown(SEV_POLICY_ES);
 
 		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);
+			test_sync_vmsa_sev_es(SEV_POLICY_ES);
+			test_sync_vmsa_sev_es(SEV_POLICY_NO_DBG | SEV_POLICY_ES);
 		}
 	}
 
-- 
2.43.0

[PATCH v4 8/8] 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 <pratikrajesh.sampat@amd.com>
---
 .../selftests/kvm/x86_64/sev_smoke_test.c     | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
index af1beabbbf8e..ff508d67377d 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/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. */
@@ -157,11 +169,21 @@ static void test_sev_es(uint64_t policy)
 	__test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, policy);
 }
 
+static void test_snp(uint64_t policy)
+{
+	__test_sev(guest_snp_code, KVM_X86_SNP_VM, policy);
+}
+
 static void test_sync_vmsa_sev_es(uint64_t policy)
 {
 	__test_sync_vmsa(KVM_X86_SEV_ES_VM, policy);
 }
 
+static void test_sync_vmsa_snp(uint64_t policy)
+{
+	__test_sync_vmsa(KVM_X86_SNP_VM, policy);
+}
+
 static void guest_shutdown_code(void)
 {
 	struct desc_ptr idt;
@@ -195,6 +217,11 @@ static void test_sev_es_shutdown(uint64_t policy)
 	__test_sev_shutdown(KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
 }
 
+static void test_snp_shutdown(uint64_t policy)
+{
+	__test_sev_shutdown(KVM_X86_SNP_VM, policy);
+}
+
 int main(int argc, char *argv[])
 {
 	const u64 xf_mask = XFEATURE_MASK_X87_AVX;
@@ -217,5 +244,20 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	if (kvm_cpu_has(X86_FEATURE_SNP)) {
+		uint64_t snp_policy = snp_default_policy();
+
+		test_snp(snp_policy);
+		/* Test minimum firmware level */
+		test_snp(snp_policy | SNP_FW_VER_MAJOR(SNP_MIN_API_MAJOR) |
+			SNP_FW_VER_MINOR(SNP_MIN_API_MINOR));
+
+		test_snp_shutdown(snp_policy);
+
+		if (kvm_has_cap(KVM_CAP_XCRS) &&
+		    (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask)
+			test_sync_vmsa_snp(snp_policy);
+	}
+
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v4 0/8] Basic SEV-SNP Selftests

[Aithal, Srikanth]

On 11/15/2024 5:10 AM, 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 advertize SNP on incompatible firmware
> Patch 2 - SNP test for KVM_SEV_INIT2
> Patch 3 - Add VMGEXIT helper
> Patch 4 - Introduce SEV+ VM type check
> Patch 5 - SNP iotcl() plumbing for the SEV library
> Patch 6 - Force set GUEST_MEMFD for SNP
> Patch 7 - Cleanups of smoke test - Decouple policy from type
> Patch 8 - SNP smoke test
> 
> v4:
> 1. Remove SNP FW API version check in the test and ensure the KVM
>     capability advertizes the presence of the feature. Retain the minimum
>     version definitions to exercise these API versions in the smoke test.
> 2. Retained only the SNP smoke test and SNP_INIT2 test
> 3. The SNP architectural defined merged with SNP_INIT2 test patch
> 4. SNP shutdown merged with SNP smoke test patch
> 5. Add SEV VM type check to abstract comparisons and reduce clutter
> 6. Define a SNP default policy which sets bits based on the presence of
>     SMT
> 7. Decouple privatization and encryption for it to be SNP agnostic
> 8. Assert for only positive tests using vm_ioctl()
> 9. Dropped tested-by tags
> 
Tested-by: Srikanth Aithal <sraithal@amd.com>
> 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.
> 
> v3:
> https://lore.kernel.org/kvm/20240905124107.6954-1-pratikrajesh.sampat@amd.com/
> 1. Remove the assignments for the prefault and fallocate test type
>     enums.
> 2. Fix error message for sev launch measure and finish.
> 3. Collect tested-by tags [Peter, Srikanth]
> 
> Any feedback/review is highly appreciated!
> 
> Pratik R. Sampat (8):
>    KVM: SEV: Disable SEV-SNP on FW validation failure
>    KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
>    KVM: selftests: Add VMGEXIT helper
>    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/kvm/svm/sev.c                        |  4 +-
>   drivers/crypto/ccp/sev-dev.c                  |  6 ++
>   include/linux/psp-sev.h                       |  3 +
>   .../selftests/kvm/include/x86_64/processor.h  |  1 +
>   .../selftests/kvm/include/x86_64/sev.h        | 55 ++++++++++-
>   tools/testing/selftests/kvm/lib/kvm_util.c    |  7 +-
>   .../selftests/kvm/lib/x86_64/processor.c      |  4 +-
>   tools/testing/selftests/kvm/lib/x86_64/sev.c  | 98 ++++++++++++++++++-
>   .../selftests/kvm/x86_64/sev_init2_tests.c    | 13 +++
>   .../selftests/kvm/x86_64/sev_smoke_test.c     | 96 ++++++++++++++----
>   10 files changed, 258 insertions(+), 29 deletions(-)
> 

Re: [PATCH v4 0/8] Basic SEV-SNP Selftests

[Pratik Rajesh Sampat]

On 11/14/24 5:40 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 advertize SNP on incompatible firmware
> Patch 2 - SNP test for KVM_SEV_INIT2
> Patch 3 - Add VMGEXIT helper
> Patch 4 - Introduce SEV+ VM type check
> Patch 5 - SNP iotcl() plumbing for the SEV library
> Patch 6 - Force set GUEST_MEMFD for SNP
> Patch 7 - Cleanups of smoke test - Decouple policy from type
> Patch 8 - SNP smoke test
> 
> v4:
> 1. Remove SNP FW API version check in the test and ensure the KVM
>    capability advertizes the presence of the feature. Retain the minimum
>    version definitions to exercise these API versions in the smoke test.
> 2. Retained only the SNP smoke test and SNP_INIT2 test
> 3. The SNP architectural defined merged with SNP_INIT2 test patch
> 4. SNP shutdown merged with SNP smoke test patch
> 5. Add SEV VM type check to abstract comparisons and reduce clutter
> 6. Define a SNP default policy which sets bits based on the presence of
>    SMT
> 7. Decouple privatization and encryption for it to be SNP agnostic
> 8. Assert for only positive tests using vm_ioctl()
> 9. 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.
> 
> v3:
> https://lore.kernel.org/kvm/20240905124107.6954-1-pratikrajesh.sampat@amd.com/
> 1. Remove the assignments for the prefault and fallocate test type
>    enums.
> 2. Fix error message for sev launch measure and finish.
> 3. Collect tested-by tags [Peter, Srikanth]
> 
> Any feedback/review is highly appreciated!
> 
> Pratik R. Sampat (8):
>   KVM: SEV: Disable SEV-SNP on FW validation failure
>   KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
>   KVM: selftests: Add VMGEXIT helper
>   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/kvm/svm/sev.c                        |  4 +-
>  drivers/crypto/ccp/sev-dev.c                  |  6 ++
>  include/linux/psp-sev.h                       |  3 +
>  .../selftests/kvm/include/x86_64/processor.h  |  1 +
>  .../selftests/kvm/include/x86_64/sev.h        | 55 ++++++++++-
>  tools/testing/selftests/kvm/lib/kvm_util.c    |  7 +-
>  .../selftests/kvm/lib/x86_64/processor.c      |  4 +-
>  tools/testing/selftests/kvm/lib/x86_64/sev.c  | 98 ++++++++++++++++++-
>  .../selftests/kvm/x86_64/sev_init2_tests.c    | 13 +++
>  .../selftests/kvm/x86_64/sev_smoke_test.c     | 96 ++++++++++++++----
>  10 files changed, 258 insertions(+), 29 deletions(-)
> 

Hi,

A gentle ping on this patch series.

~Pratik Sampat

Re: [sos-linux-ext-patches] [PATCH v4 1/8] KVM: SEV: Disable SEV-SNP on FW validation failure

[Nikunj A. Dadhania]

On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
> On incompatible firmware versions, SEV-SNP support is pulled and the
> setup is not performed. However, the platform and subsequently the KVM
> capability may continue to advertize support for it. Disable support for
> SEV-SNP if the FW version validation fails.

Additionally, can we ensure that if sev_platform_init() fails, we do not
indicate SNP support?

Regards
Nikunj

Re: [sos-linux-ext-patches] [PATCH v4 2/8] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2

[Nikunj A. Dadhania]

On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
> Add the X86_FEATURE_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, so that a failure is not reported in such cases.
> 
> Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
> ---
>  .../selftests/kvm/include/x86_64/processor.h        |  1 +
>  .../testing/selftests/kvm/x86_64/sev_init2_tests.c  | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index 645200e95f89..c18d2405798f 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/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)

Unrelated to your patch, X86_FEATURE_SEV does not match the KVM API definitions,
1 is used for KVM_X86_SW_PROTECTED_VM 

  #define KVM_X86_DEFAULT_VM    0
  #define KVM_X86_SW_PROTECTED_VM       1
  #define KVM_X86_SEV_VM        2
  #define KVM_X86_SEV_ES_VM     3


>  #define X86_FEATURE_SEV_ES		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
> +#define X86_FEATURE_SNP			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 4)

Additionally, please add KVM_X86_SNP_VM to the Documentation/virt/kvm/api.rst

Regards
Nikunj

Re: [sos-linux-ext-patches] [PATCH v4 2/8] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2

[Nikunj A. Dadhania]

On 1/10/2025 11:22 AM, Nikunj A. Dadhania wrote:
> 
> 
> On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
>> Add the X86_FEATURE_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, so that a failure is not reported in such cases.
>>
>> Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
>> ---
>>  .../selftests/kvm/include/x86_64/processor.h        |  1 +
>>  .../testing/selftests/kvm/x86_64/sev_init2_tests.c  | 13 +++++++++++++
>>  2 files changed, 14 insertions(+)
>>
>> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
>> index 645200e95f89..c18d2405798f 100644
>> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
>> +++ b/tools/testing/selftests/kvm/include/x86_64/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)
> 
> Unrelated to your patch, X86_FEATURE_SEV does not match the KVM API definitions,
> 1 is used for KVM_X86_SW_PROTECTED_VM 

Scratch that, I got confused between the KVM API definition and CPUID naming.

Regards
Nikunj

Re: [sos-linux-ext-patches] [PATCH v4 1/8] KVM: SEV: Disable SEV-SNP on FW validation failure

[Pratik Rajesh Sampat]

Hi Nikunj,

On 1/9/25 11:21 PM, Nikunj A. Dadhania wrote:
> 
> 
> On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
>> On incompatible firmware versions, SEV-SNP support is pulled and the
>> setup is not performed. However, the platform and subsequently the KVM
>> capability may continue to advertize support for it. Disable support for
>> SEV-SNP if the FW version validation fails.
> 
> Additionally, can we ensure that if sev_platform_init() fails, we do not
> indicate SNP support?

That sounds good to me. Although if the platform initialization fails,
I think we should not be advertising SEV, SEV-ES as well.

If that makes sense, we could do something similar to before by
exporting another function from ccp that returns whether the platform
is initialized. Then, within kvm's sev_hardware_setup(), we can check
this to ensure that none of the capabilities are set if the platform
initialization has failed?

Thanks!
Pratik

Re: [sos-linux-ext-patches] [PATCH v4 2/8] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2

[Pratik Rajesh Sampat]

On 1/9/25 11:52 PM, Nikunj A. Dadhania wrote:
> 
> 
> On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
>> Add the X86_FEATURE_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, so that a failure is not reported in such cases.
>>
>> Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
>> ---
>>  #define X86_FEATURE_SEV_ES		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
>> +#define X86_FEATURE_SNP			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 4)
> 
> Additionally, please add KVM_X86_SNP_VM to the Documentation/virt/kvm/api.rst
> 

Ack, will add that.

Thanks for your review!
Pratik

Re: [sos-linux-ext-patches] [PATCH v4 1/8] KVM: SEV: Disable SEV-SNP on FW validation failure

[Nikunj A. Dadhania]

On 1/11/2025 3:20 AM, Pratik Rajesh Sampat wrote:
> Hi Nikunj,
> 
> On 1/9/25 11:21 PM, Nikunj A. Dadhania wrote:
>>
>>
>> On 11/15/2024 5:10 AM, Pratik R. Sampat wrote:
>>> On incompatible firmware versions, SEV-SNP support is pulled and the
>>> setup is not performed. However, the platform and subsequently the KVM
>>> capability may continue to advertize support for it. Disable support for
>>> SEV-SNP if the FW version validation fails.
>>
>> Additionally, can we ensure that if sev_platform_init() fails, we do not
>> indicate SNP support?
> 
> That sounds good to me. Although if the platform initialization fails,
> I think we should not be advertising SEV, SEV-ES as well.

Even better!

> 
> If that makes sense, we could do something similar to before by
> exporting another function from ccp that returns whether the platform
> is initialized. Then, within kvm's sev_hardware_setup(), we can check
> this to ensure that none of the capabilities are set if the platform
> initialization has failed?

Yes, that will ensure we do not advertise any of the SEV capabilities
if the ccp driver has failed loading the firmware or initializing the
platform.

Regards
Nikunj

Re: [PATCH v4 4/8] KVM: selftests: Introduce SEV VM type check

[Nikunj A. Dadhania]

On 11/15/2024 5:11 AM, Pratik R. Sampat wrote:
> In preparation for SNP, declutter the vm type check by introducing a
> SEV-SNP VM type check as well 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 <pratikrajesh.sampat@amd.com>
> ---
>  .../testing/selftests/kvm/include/x86_64/sev.h  |  4 ++++
>  .../selftests/kvm/lib/x86_64/processor.c        |  4 ++--
>  tools/testing/selftests/kvm/lib/x86_64/sev.c    | 17 +++++++++++++++++
>  .../selftests/kvm/x86_64/sev_smoke_test.c       |  2 +-
>  4 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
> index e7df5d0987f6..faed91435963 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/sev.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
> @@ -29,6 +29,10 @@ enum sev_guest_state {
>  
>  #define VMGEXIT()		{ __asm__ __volatile__("rep; vmmcall"); }
>  
> +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_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> index 636b29ba8985..13f060748fc2 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> @@ -641,7 +641,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);
> @@ -1158,7 +1158,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_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> index e9535ee20b7f..d6e7a422b69d 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/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_snp_vm(vm) || is_sev_es_vm(vm) || vm->type == KVM_X86_SEV_VM;

As is_sev_es_vm() already checks is_sev_snp_vm(), we can drop SNP VM check here, right ?

Regards
Nikunj

Re: [PATCH v4 5/8] KVM: selftests: Add library support for interacting with SNP

[Nikunj A. Dadhania]

On 11/15/2024 5:11 AM, Pratik R. Sampat wrote:
> 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 <pratikrajesh.sampat@amd.com>
> ---
>  .../selftests/kvm/include/x86_64/sev.h        | 49 ++++++++++-
>  tools/testing/selftests/kvm/lib/x86_64/sev.c  | 81 ++++++++++++++++++-
>  2 files changed, 125 insertions(+), 5 deletions(-)


> diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> index d6e7a422b69d..40b90d3a5769 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/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)
>  {
>  	const struct sparsebit *protected_phy_pages = region->protected_phy_pages;
>  	const vm_paddr_t gpa_base = region->region.guest_phys_addr;
> @@ -41,16 +42,39 @@ 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;
>  
> +		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);
> +			continue;
> +		}
> +

Instead of using continue, if/else is the better code flow:

	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);

Right?

Regards,
Nikunj

Re: [PATCH v4 5/8] KVM: selftests: Add library support for interacting with SNP

[Pratik Rajesh Sampat]

On 1/13/25 2:32 AM, Nikunj A. Dadhania wrote:
> 
> 
> On 11/15/2024 5:11 AM, Pratik R. Sampat wrote:
>> 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 <pratikrajesh.sampat@amd.com>
>> ---
>>  .../selftests/kvm/include/x86_64/sev.h        | 49 ++++++++++-
>>  tools/testing/selftests/kvm/lib/x86_64/sev.c  | 81 ++++++++++++++++++-
>>  2 files changed, 125 insertions(+), 5 deletions(-)
> 
> 
>> diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> index d6e7a422b69d..40b90d3a5769 100644
>> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> +++ b/tools/testing/selftests/kvm/lib/x86_64/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)
>>  {
>>  	const struct sparsebit *protected_phy_pages = region->protected_phy_pages;
>>  	const vm_paddr_t gpa_base = region->region.guest_phys_addr;
>> @@ -41,16 +42,39 @@ 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;
>>  
>> +		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);
>> +			continue;
>> +		}
>> +
> 
> Instead of using continue, if/else is the better code flow:
> 
> 	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);
> 
> Right?

Sure, I can change that. That's definitely cleaner.

Thanks!
Pratik

Re: [PATCH v4 4/8] KVM: selftests: Introduce SEV VM type check

[Pratik Rajesh Sampat]

On 1/13/25 2:18 AM, Nikunj A. Dadhania wrote:
> 
> 
> On 11/15/2024 5:11 AM, Pratik R. Sampat wrote:
>> In preparation for SNP, declutter the vm type check by introducing a
>> SEV-SNP VM type check as well 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 <pratikrajesh.sampat@amd.com>
>> ---
>>  .../testing/selftests/kvm/include/x86_64/sev.h  |  4 ++++
>>  .../selftests/kvm/lib/x86_64/processor.c        |  4 ++--
>>  tools/testing/selftests/kvm/lib/x86_64/sev.c    | 17 +++++++++++++++++
>>  .../selftests/kvm/x86_64/sev_smoke_test.c       |  2 +-
>>  4 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
>> index e7df5d0987f6..faed91435963 100644
>> --- a/tools/testing/selftests/kvm/include/x86_64/sev.h
>> +++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
>> @@ -29,6 +29,10 @@ enum sev_guest_state {
>>  
>>  #define VMGEXIT()		{ __asm__ __volatile__("rep; vmmcall"); }
>>  
>> +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_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
>> index 636b29ba8985..13f060748fc2 100644
>> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
>> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
>> @@ -641,7 +641,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);
>> @@ -1158,7 +1158,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_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> index e9535ee20b7f..d6e7a422b69d 100644
>> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> +++ b/tools/testing/selftests/kvm/lib/x86_64/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_snp_vm(vm) || is_sev_es_vm(vm) || vm->type == KVM_X86_SEV_VM;
> 
> As is_sev_es_vm() already checks is_sev_snp_vm(), we can drop SNP VM check here, right ?
> 

That's right, thanks for catching that. I'll get rid of the
is_sev_es_vm() check here.

Pratik