[PATCH v3 0/9] SEV Kernel Selftests
This series primarily introduces SEV-SNP test for the kernel selftest framework. It tests boot, ioctl, pre fault, and fallocate in various combinations to exercise both positive and negative launch flow paths. Patch 1 - Adds a wrapper for the ioctl calls that decouple ioctl and asserts, which enables the use of negative test cases. No functional change intended. Patch 2 - Extend the sev smoke tests to use the SNP specific ioctl calls and sets up memory to boot a SNP guest VM Patch 3 - Adds SNP to shutdown testing Patch 4, 5 - Tests the ioctl path for SEV, SEV-ES and SNP Patch 6 - Adds support for SNP in KVM_SEV_INIT2 tests Patch 7,8,9 - Enable Prefault tests for SEV, SEV-ES and SNP The patchset is rebased on top of kvm-x86/next branch. v3: 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] v2: https://lore.kernel.org/kvm/20240816192310.117456-1-pratikrajesh.sam...@amd.com/ 1. Add SMT parsing check to populate SNP policy flags 2. Extend Peter Gonda's shutdown test to include SNP 3. Introduce new tests for prefault which include exercising prefault, fallocate, hole-punch in various combinations. 4. Decouple ioctl patch reworked to introduce private variants of the the functions that call into the ioctl. Also reordered the patch for it to arrive first so that new APIs are not written right after their introduction. 5. General cleanups - adding comments, avoiding local booleans, better error message. Suggestions incorporated from Peter, Tom, and Sean. RFC: https://lore.kernel.org/kvm/20240710220540.188239-1-pratikrajesh.sam...@amd.com/ Any feedback/review is highly appreciated! Michael Roth (2): KVM: selftests: Add interface to manually flag protected/encrypted ranges KVM: selftests: Add a CoCo-specific test for KVM_PRE_FAULT_MEMORY Pratik R. Sampat (7): KVM: selftests: Decouple SEV ioctls from asserts KVM: selftests: Add a basic SNP smoke test KVM: selftests: Add SNP to shutdown testing KVM: selftests: SEV IOCTL test KVM: selftests: SNP IOCTL test KVM: selftests: SEV-SNP test for KVM_SEV_INIT2 KVM: selftests: Interleave fallocate for KVM_PRE_FAULT_MEMORY tools/testing/selftests/kvm/Makefile | 1 + .../testing/selftests/kvm/include/kvm_util.h | 13 + .../selftests/kvm/include/x86_64/processor.h | 1 + .../selftests/kvm/include/x86_64/sev.h| 76 +++- tools/testing/selftests/kvm/lib/kvm_util.c| 53 ++- .../selftests/kvm/lib/x86_64/processor.c | 6 +- tools/testing/selftests/kvm/lib/x86_64/sev.c | 190 +++- .../kvm/x86_64/coco_pre_fault_memory_test.c | 421 ++ .../selftests/kvm/x86_64/sev_init2_tests.c| 13 + .../selftests/kvm/x86_64/sev_smoke_test.c | 297 +++- 10 files changed, 1023 insertions(+), 48 deletions(-) create mode 100644 tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c -- 2.34.1
[PATCH v3 1/9] KVM: selftests: Decouple SEV ioctls from asserts
Add variants of sev, sev-es launch path that return the status of the
ioctl call instead of asserting for success. This enables both positive
and negative testing of the path.
No functional impact intended.
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
.../selftests/kvm/include/x86_64/sev.h| 22 +-
tools/testing/selftests/kvm/lib/x86_64/sev.c | 78 +++
2 files changed, 80 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h
b/tools/testing/selftests/kvm/include/x86_64/sev.h
index 82c11c81a956..3998152cc081 100644
--- a/tools/testing/selftests/kvm/include/x86_64/sev.h
+++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
@@ -27,6 +27,12 @@ enum sev_guest_state {
#define GHCB_MSR_TERM_REQ 0x100
+/* Variants of the SEV launch path that do not assert the ioctl status */
+int __sev_vm_launch_start(struct kvm_vm *vm, uint32_t policy);
+int __sev_vm_launch_update(struct kvm_vm *vm, uint32_t policy);
+int __sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement);
+int __sev_vm_launch_finish(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);
@@ -82,15 +88,23 @@ static inline void sev_register_encrypted_memory(struct
kvm_vm *vm,
vm_ioctl(vm, KVM_MEMORY_ENCRYPT_REG_REGION, &range);
}
-static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
- uint64_t size)
+static inline int __sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
+ uint64_t hva, uint64_t size)
{
struct kvm_sev_launch_update_data update_data = {
- .uaddr = (unsigned long)addr_gpa2hva(vm, gpa),
+ .uaddr = hva,
.len = size,
};
- vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_DATA, &update_data);
+ return __vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_DATA, &update_data);
+}
+
+static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
+ uint64_t hva, uint64_t size)
+{
+ int ret = __sev_launch_update_data(vm, gpa, hva, size);
+
+ TEST_ASSERT_VM_VCPU_IOCTL(!ret, KVM_SEV_LAUNCH_UPDATE_DATA, ret, vm);
}
#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 e9535ee20b7f..125a72246e09 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
@@ -14,15 +14,16 @@
* 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 int encrypt_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;
+ int ret;
if (!sparsebit_any_set(protected_phy_pages))
- return;
+ return 0;
sev_register_encrypted_memory(vm, region);
@@ -30,8 +31,15 @@ static void encrypt_region(struct kvm_vm *vm, struct
userspace_mem_region *regio
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);
+ ret = __sev_launch_update_data(vm, gpa_base + offset,
+ (uint64_t)addr_gpa2hva(vm,
gpa_base + offset),
+ size);
+ if (ret)
+ return ret;
+
}
+
+ return 0;
}
void sev_vm_init(struct kvm_vm *vm)
@@ -60,38 +68,74 @@ void sev_es_vm_init(struct kvm_vm *vm)
}
}
-void sev_vm_launch(struct kvm_vm *vm, uint32_t policy)
+int __sev_vm_launch_start(struct kvm_vm *vm, uint32_t policy)
{
struct kvm_sev_launch_start launch_start = {
.policy = policy,
};
+
+ return __vm_sev_ioctl(vm, KVM_SEV_LAUNCH_START, &launch_start);
+}
+
+int __sev_vm_launch_update(struct kvm_vm *vm, uint32_t policy)
+{
struct userspace_mem_region *region;
- struct kvm_sev_guest_status status;
int ctr;
- vm_sev_ioctl(vm, KVM_SEV_LAUNCH_START, &launch_start);
- vm_sev_ioctl(vm, KVM_SEV_GUEST_STATUS, &status);
+ hash_for_each(vm->regions.slot_hash, ctr, region, slot_node) {
+ int ret = encrypt_region(vm, re
[PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test
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.
Also, decouple policy and type and require functions to provide both
such that there is no assumption regarding the type using policy.
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
.../selftests/kvm/include/x86_64/processor.h | 1 +
.../selftests/kvm/include/x86_64/sev.h| 54 +++-
tools/testing/selftests/kvm/lib/kvm_util.c| 8 +-
.../selftests/kvm/lib/x86_64/processor.c | 6 +-
tools/testing/selftests/kvm/lib/x86_64/sev.c | 116 +-
.../selftests/kvm/x86_64/sev_smoke_test.c | 67 --
6 files changed, 230 insertions(+), 22 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h
b/tools/testing/selftests/kvm/include/x86_64/processor.h
index e247f99e0473..1dfa2c03b40f 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 {
#defineX86_FEATURE_VGIFKVM_X86_CPU_FEATURE(0x800A,
0, EDX, 16)
#define X86_FEATURE_SEVKVM_X86_CPU_FEATURE(0x801F,
0, EAX, 1)
#define X86_FEATURE_SEV_ES KVM_X86_CPU_FEATURE(0x801F, 0, EAX,
3)
+#define X86_FEATURE_SNPKVM_X86_CPU_FEATURE(0x801F, 0, EAX,
4)
/*
* KVM defined paravirt features.
diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h
b/tools/testing/selftests/kvm/include/x86_64/sev.h
index 3998152cc081..658c3cca208d 100644
--- a/tools/testing/selftests/kvm/include/x86_64/sev.h
+++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
@@ -22,8 +22,21 @@ enum sev_guest_state {
SEV_GUEST_STATE_RUNNING,
};
+/* Minimum firmware version required for the SEV-SNP support */
+#define SNP_FW_REQ_VER_MAJOR 1
+#define SNP_FW_REQ_VER_MINOR 51
+#define SNP_POLICY_MINOR_BIT 0
+#define SNP_POLICY_MAJOR_BIT 8
+
#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_POLICY (SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO)
+
+#define SNP_FW_VER_MAJOR(maj) ((uint8_t)(maj) << SNP_POLICY_MAJOR_BIT)
+#define SNP_FW_VER_MINOR(min) ((uint8_t)(min) << SNP_POLICY_MINOR_BIT)
#define GHCB_MSR_TERM_REQ 0x100
@@ -32,14 +45,22 @@ int __sev_vm_launch_start(struct kvm_vm *vm, uint32_t
policy);
int __sev_vm_launch_update(struct kvm_vm *vm, uint32_t policy);
int __sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement);
int __sev_vm_launch_finish(struct kvm_vm *vm);
+int __snp_vm_launch_start(struct kvm_vm *vm, uint64_t policy, uint8_t flags);
+int __snp_vm_launch_update(struct kvm_vm *vm, uint8_t page_type);
+int __snp_vm_launch_finish(struct kvm_vm *vm, uint16_t flags);
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);
+
+bool is_kvm_snp_supported(void);
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);
@@ -74,8 +95,18 @@ kvm_static_assert(SEV_RET_SUCCESS == 0);
__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, vm); \
})
+/* Ensure policy is within bounds for SEV, SEV-ES */
+#define ASSERT_SEV_POLICY(type, policy)\
+({ \
+ if (type == KVM_X86_SEV_VM || type == KVM_X86_SEV_ES_VM) { \
+ TEST_ASSERT(policy < ((uint32_t)~0U), \
+ "Policy beyond bounds for SEV");\
+ } \
+}) \
+
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,6 +130,
[PATCH v3 3/9] KVM: selftests: Add SNP to shutdown testing
Parameterize the shutdown test to include the SEV-SNP VM type
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
tools/testing/selftests/kvm/x86_64/sev_smoke_test.c | 10 +-
1 file changed, 5 insertions(+), 5 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 12d466915074..8e798f5a2a53 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
@@ -193,16 +193,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,
@@ -223,7 +221,7 @@ int main(int argc, char *argv[])
test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES |
SEV_POLICY_NO_DBG);
test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
- test_sev_es_shutdown();
+ test_sev_shutdown(KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
if (kvm_has_cap(KVM_CAP_XCRS) &&
(xgetbv(0) & XFEATURE_MASK_X87_AVX) ==
XFEATURE_MASK_X87_AVX) {
@@ -245,6 +243,8 @@ int main(int argc, char *argv[])
SNP_FW_VER_MAJOR(SNP_FW_REQ_VER_MAJOR) |
SNP_FW_VER_MINOR(SNP_FW_REQ_VER_MINOR));
+ test_sev_shutdown(KVM_X86_SNP_VM, snp_policy);
+
if (kvm_has_cap(KVM_CAP_XCRS) &&
(xgetbv(0) & XFEATURE_MASK_X87_AVX) ==
XFEATURE_MASK_X87_AVX) {
test_sync_vmsa(KVM_X86_SNP_VM, snp_policy);
--
2.34.1
[PATCH v3 4/9] KVM: selftests: SEV IOCTL test
Introduce tests for sev and sev-es ioctl that exercises the boot path
of launch, update and finish on an invalid policy.
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
.../selftests/kvm/x86_64/sev_smoke_test.c | 84 +++
1 file changed, 84 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 8e798f5a2a53..5fa4ee27609b 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
@@ -142,12 +142,96 @@ static void test_sync_vmsa(uint32_t type, uint64_t policy)
kvm_vm_free(vm);
}
+static void sev_guest_neg_status_assert(struct kvm_vm *vm, uint32_t type)
+{
+ struct kvm_sev_guest_status status;
+ int ret;
+
+ ret = __vm_sev_ioctl(vm, KVM_SEV_GUEST_STATUS, &status);
+ TEST_ASSERT(ret, "KVM_SEV_GUEST_STATUS should fail, invalid VM Type.");
+}
+
+static void vm_sev_es_launch_neg(struct kvm_vm *vm, uint32_t type, uint64_t
policy)
+{
+ int ret;
+
+ /* Launch start with policy SEV_POLICY_NO_DBG (0x0) */
+ ret = __sev_vm_launch_start(vm, 0);
+ TEST_ASSERT(ret,
+ "KVM_SEV_LAUNCH_START should fail due to type (%d) -
policy(0x0) mismatch",
+ type);
+
+ ret = __sev_vm_launch_update(vm, policy);
+ TEST_ASSERT(ret,
+ "KVM_SEV_LAUNCH_UPDATE should fail due to LAUNCH_START.
type: %d policy: 0x%lx",
+ type, policy);
+ sev_guest_neg_status_assert(vm, type);
+
+ ret = __sev_vm_launch_measure(vm, alloca(256));
+ TEST_ASSERT(ret,
+ "KVM_SEV_LAUNCH_UPDATE should fail due to LAUNCH_START.
type: %d policy: 0x%lx",
+ type, policy);
+ sev_guest_neg_status_assert(vm, type);
+
+ ret = __sev_vm_launch_finish(vm);
+ TEST_ASSERT(ret,
+ "KVM_SEV_LAUNCH_UPDATE should fail due to LAUNCH_START.
type: %d policy: 0x%lx",
+ type, policy);
+ sev_guest_neg_status_assert(vm, type);
+}
+
+/*
+ * Test for SEV ioctl launch path
+ * VMs of the type SEV and SEV-ES are created, however they are launched with
+ * an empty policy to observe the effect on the control flow of launching a VM.
+ *
+ * SEV - Expected to pass through the path of launch start, update, measure,
+ * and finish. vcpu_run expected to fail with error KVM_EXIT_IO.
+ *
+ * SEV-ES - Expected to fail the launch start as vm created with type
+ * KVM_X86_DEFAULT_VM but policy passed to launch start is KVM_X86_SEV_ES_VM.
+ * Post this, calls that pass the correct policy to update, measure, and finish
+ * are also expected to fail cascading.
+ */
+static void test_sev_launch(void *guest_code, uint32_t type, uint64_t policy)
+{
+ struct kvm_vcpu *vcpu;
+ int exp_exit_reason;
+ struct kvm_vm *vm;
+ struct ucall uc;
+
+ vm = vm_sev_create_with_one_vcpu(type, guest_code, &vcpu);
+
+ if (type == KVM_X86_SEV_VM) {
+ sev_vm_launch(vm, 0);
+ sev_vm_launch_measure(vm, alloca(256));
+ sev_vm_launch_finish(vm);
+ } else {
+ vm_sev_es_launch_neg(vm, type, policy);
+ }
+
+ vcpu_run(vcpu);
+ get_ucall(vcpu, &uc);
+ if (type == KVM_X86_SEV_VM)
+ exp_exit_reason = KVM_EXIT_IO;
+ else
+ exp_exit_reason = KVM_EXIT_FAIL_ENTRY;
+
+ TEST_ASSERT(vcpu->run->exit_reason == exp_exit_reason,
+ "vcpu_run failed exit expected: %d, got: %d",
+ exp_exit_reason, vcpu->run->exit_reason);
+
+ kvm_vm_free(vm);
+}
+
static void test_sev(void *guest_code, uint32_t type, uint64_t policy)
{
struct kvm_vcpu *vcpu;
struct kvm_vm *vm;
struct ucall uc;
+ test_sev_launch(guest_code, type, policy);
+
vm = vm_sev_create_with_one_vcpu(type, guest_code, &vcpu);
/* TODO: Validate the measurement is as expected. */
--
2.34.1
[PATCH v3 5/9] KVM: selftests: SNP IOCTL test
Introduce testing of SNP ioctl calls. Tests attributes such as flags,
page types, and policies in various combinations along the SNP launch
path.
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
.../testing/selftests/kvm/include/kvm_util.h | 11 ++
.../selftests/kvm/x86_64/sev_smoke_test.c | 140 +-
2 files changed, 150 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index bc7c242480d6..ab213708b551 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -912,6 +912,17 @@ static inline struct kvm_vm *vm_create(uint32_t
nr_runnable_vcpus)
return __vm_create(VM_SHAPE_DEFAULT, nr_runnable_vcpus, 0);
}
+static inline struct kvm_vm *vm_create_type(unsigned long type,
+ uint32_t nr_runnable_vcpus)
+{
+ const struct vm_shape shape = {
+ .mode = VM_MODE_DEFAULT,
+ .type = type,
+ };
+
+ return __vm_create(shape, nr_runnable_vcpus, 0);
+}
+
struct kvm_vm *__vm_create_with_vcpus(struct vm_shape shape, uint32_t nr_vcpus,
uint64_t extra_mem_pages,
void *guest_code, struct kvm_vcpu
*vcpus[]);
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 5fa4ee27609b..9a7efbe214ce 100644
--- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
@@ -224,13 +224,151 @@ static void test_sev_launch(void *guest_code, uint32_t
type, uint64_t policy)
kvm_vm_free(vm);
}
+static int __test_snp_launch_start(uint32_t type, uint64_t policy,
+ uint8_t flags, bool assert)
+{
+ struct kvm_vm *vm;
+ int ret = 0;
+
+ vm = vm_create_type(type, 1);
+ ret = __snp_vm_launch_start(vm, policy, flags);
+ if (assert)
+ TEST_ASSERT_VM_VCPU_IOCTL(!ret, KVM_SEV_SNP_LAUNCH_START, ret,
vm);
+ kvm_vm_free(vm);
+
+ return ret;
+}
+
+static void test_snp_launch_start(uint32_t type, uint64_t policy)
+{
+ uint8_t i;
+ int ret;
+
+ /* Flags must be zero for success */
+ __test_snp_launch_start(type, policy, 0, true);
+
+ for (i = 1; i < 8; i++) {
+ ret = __test_snp_launch_start(type, policy, BIT(i), false);
+ TEST_ASSERT(ret && errno == EINVAL,
+ "KVM_SEV_SNP_LAUNCH_START should fail, invalid
flag\n"
+ "(type: %d policy: 0x%lx, flag: 0x%lx)",
+ type, policy, BIT(i));
+ }
+
+ ret = __test_snp_launch_start(type, SNP_POLICY_SMT, 0, false);
+ TEST_ASSERT(ret && errno == EINVAL,
+ "KVM_SEV_SNP_LAUNCH_START should fail, SNP_POLICY_RSVD_MBO
policy bit not set\n"
+ "(type: %d policy: 0x%llx, flags: 0x0)",
+ type, SNP_POLICY_SMT);
+
+ ret = __test_snp_launch_start(type, SNP_POLICY_RSVD_MBO, 0, false);
+ if (unlikely(!is_smt_active())) {
+ TEST_ASSERT(!ret,
+ "KVM_SEV_SNP_LAUNCH_START should succeed,
SNP_POLICY_SMT not required on non-SMT systems\n"
+ "(type: %d policy: 0x%llx, flags: 0x0)",
+ type, SNP_POLICY_RSVD_MBO);
+ } else {
+ TEST_ASSERT(ret && errno == EINVAL,
+ "KVM_SEV_SNP_LAUNCH_START should fail,
SNP_POLICY_SMT is not set on a SMT system\n"
+ "(type: %d policy: 0x%llx, flags: 0x0)",
+ type, SNP_POLICY_RSVD_MBO);
+ }
+
+ ret = __test_snp_launch_start(type, SNP_POLICY |
+ SNP_FW_VER_MAJOR(UINT8_MAX) |
+ SNP_FW_VER_MINOR(UINT8_MAX), 0, false);
+ TEST_ASSERT(ret && errno == EIO,
+ "KVM_SEV_SNP_LAUNCH_START should fail, invalid version\n"
+ "expected: %d.%d got: %d.%d (type: %d policy: 0x%llx,
flags: 0x0)",
+ SNP_FW_REQ_VER_MAJOR, SNP_FW_REQ_VER_MINOR,
+ UINT8_MAX, UINT8_MAX, type,
+ SNP_POLICY | SNP_FW_VER_MAJOR(UINT8_MAX) |
SNP_FW_VER_MINOR(UINT8_MAX));
+}
+
+static void test_snp_launch_update(uint32_t type, uint64_t policy)
+{
+ struct kvm_vm *vm;
+ int ret;
+
+ for (int pgtype = 0; pgtype <= KVM_SEV_SNP_PAGE_TYPE_CPUID + 1;
pgtype++) {
+ vm = vm_create_type(type, 1);
+ snp_vm_launch_start(vm, policy);
+ ret = __snp_vm_launch_update(vm, pgtype);
+
+
[PATCH v3 6/9] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
Add SEV-SNP VM type to exercise the KVM_SEV_INIT2 call.
Also ensure that SNP case is skipped for scenarios where CPUID supports
it but KVM does not so that a failure is not reported for such cases.
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
.../testing/selftests/kvm/x86_64/sev_init2_tests.c | 13 +
1 file changed, 13 insertions(+)
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.34.1
[PATCH v3 7/9] KVM: selftests: Add interface to manually flag protected/encrypted ranges
From: Michael Roth
For SEV and SNP, currently __vm_phy_pages_alloc() handles setting the
region->protected_phy_pages bitmap to mark that the region needs to be
encrypted/measured into the initial guest state prior to
finalizing/starting the guest. It also marks what GPAs need to be mapped
as encrypted in the initial guest page table.
This works when using virtual/physical allocators to manage memory, but
if the test manages allocations/mapping directly then an alternative is
needed to set region->protected_phy_pages directly. Add an interface to
handle that.
Signed-off-by: Michael Roth
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
.../testing/selftests/kvm/include/kvm_util.h | 2 +
tools/testing/selftests/kvm/lib/kvm_util.c| 45 +--
2 files changed, 43 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h
b/tools/testing/selftests/kvm/include/kvm_util.h
index ab213708b551..642740fe1c59 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -394,6 +394,8 @@ static inline void vm_set_memory_attributes(struct kvm_vm
*vm, uint64_t gpa,
vm_ioctl(vm, KVM_SET_MEMORY_ATTRIBUTES, &attr);
}
+void vm_mem_set_protected(struct kvm_vm *vm, uint32_t memslot,
+ vm_paddr_t paddr, size_t num);
static inline void vm_mem_set_private(struct kvm_vm *vm, uint64_t gpa,
uint64_t size)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
b/tools/testing/selftests/kvm/lib/kvm_util.c
index bbf90ad224da..d44a37aebcec 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1991,6 +1991,43 @@ const char *exit_reason_str(unsigned int exit_reason)
return "Unknown";
}
+/*
+ * Set what guest GFNs need to be encrypted prior to finalizing a CoCo VM.
+ *
+ * Input Args:
+ * vm - Virtual Machine
+ * memslot - Memory region to allocate page from
+ * paddr - Start of physical address to mark as encrypted
+ * num - number of pages
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Generally __vm_phy_pages_alloc() will handle this automatically, but
+ * for cases where the test handles managing the physical allocation and
+ * mapping directly this interface should be used to mark physical pages
+ * that are intended to be encrypted as part of the initial guest state.
+ * This will also affect whether virt_map()/virt_pg_map() will map the
+ * page as encrypted or not in the initial guest page table.
+ *
+ * If the initial guest state has already been finalized, then setting
+ * it as encrypted will essentially be a noop since nothing more can be
+ * encrypted into the initial guest state at that point.
+ */
+void vm_mem_set_protected(struct kvm_vm *vm, uint32_t memslot,
+ vm_paddr_t paddr, size_t num)
+{
+ struct userspace_mem_region *region;
+ sparsebit_idx_t pg, base;
+
+ base = paddr >> vm->page_shift;
+ region = memslot2region(vm, memslot);
+
+ for (pg = base; pg < base + num; ++pg)
+ sparsebit_set(region->protected_phy_pages, pg);
+}
+
/*
* Physical Contiguous Page Allocator
*
@@ -2048,11 +2085,11 @@ vm_paddr_t __vm_phy_pages_alloc(struct kvm_vm *vm,
size_t num,
abort();
}
- for (pg = base; pg < base + num; ++pg) {
+ for (pg = base; pg < base + num; ++pg)
sparsebit_clear(region->unused_phy_pages, pg);
- if (protected)
- sparsebit_set(region->protected_phy_pages, pg);
- }
+
+ if (protected)
+ vm_mem_set_protected(vm, memslot, base << vm->page_shift, num);
return base * vm->page_size;
}
--
2.34.1
[PATCH v3 8/9] KVM: selftests: Add a CoCo-specific test for KVM_PRE_FAULT_MEMORY
From: Michael Roth
SEV, SEV-ES, and SNP have a few corner cases where there is potential
for KVM_PRE_FAULT_MEMORY to behave differently depending on when it is
issued during initial guest setup. Exercising these various paths
requires a bit more fine-grained control over when the
KVM_PRE_FAULT_MEMORY requests are issued while setting up the guests.
Since these CoCo-specific events are likely to be architecture-specific
KST helpers, take the existing generic test in pre_fault_memory_test.c
as a starting template, and then introduce an x86-specific version of
it with expanded coverage for SEV, SEV-ES, and SNP.
Since there's a reasonable chance that TDX could extend this for similar
testing of TDX, give it a "coco-" prefix rather than an SEV-specific
one.
Signed-off-by: Michael Roth
Co-developed-by: Pratik R. Sampat
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
tools/testing/selftests/kvm/Makefile | 1 +
.../kvm/x86_64/coco_pre_fault_memory_test.c | 314 ++
2 files changed, 315 insertions(+)
create mode 100644
tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
diff --git a/tools/testing/selftests/kvm/Makefile
b/tools/testing/selftests/kvm/Makefile
index 45cb70c048bb..7b97750a7d71 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -129,6 +129,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/amx_test
TEST_GEN_PROGS_x86_64 += x86_64/max_vcpuid_cap_test
TEST_GEN_PROGS_x86_64 += x86_64/triple_fault_event_test
TEST_GEN_PROGS_x86_64 += x86_64/recalc_apic_map_test
+TEST_GEN_PROGS_x86_64 += x86_64/coco_pre_fault_memory_test
TEST_GEN_PROGS_x86_64 += access_tracking_perf_test
TEST_GEN_PROGS_x86_64 += coalesced_io_test
TEST_GEN_PROGS_x86_64 += demand_paging_test
diff --git a/tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
b/tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
new file mode 100644
index ..c31a5f9e18f4
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0
+#include
+
+#include
+#include
+#include
+#include "sev.h"
+
+/* Arbitrarily chosen values */
+#define TEST_SIZE (SZ_2M + PAGE_SIZE)
+#define TEST_NPAGES(TEST_SIZE / PAGE_SIZE)
+#define TEST_SLOT 10
+#define TEST_GPA 0x1ul
+#define TEST_GVA 0x1ul
+
+enum prefault_snp_test_type {
+ /* Skip pre-faulting tests. */
+ NO_PREFAULT_TYPE,
+ /*
+* Issue KVM_PRE_FAULT_MEMORY for GFNs mapping non-private memory
+* before finalizing the initial guest contents (e.g. via
+* KVM_SEV_SNP_LAUNCH_FINISH for SNP guests).
+*
+* This should result in failure since KVM explicitly disallows
+* KVM_PRE_FAULT_MEMORY from being issued prior to finalizing the
+* initial guest contents.
+*/
+ PREFAULT_SHARED_BEFORE_FINALIZING,
+ /*
+* Issue KVM_PRE_FAULT_MEMORY for GFNs mapping private memory
+* before finalizing the initial guest contents (e.g. via
+* KVM_SEV_SNP_LAUNCH_FINISH for SNP guests).
+*
+* This should result in failure since KVM explicitly disallows
+* KVM_PRE_FAULT_MEMORY from being issued prior to finalizing the
+* initial guest contents.
+*/
+ PREFAULT_PRIVATE_BEFORE_FINALIZING,
+ /*
+* Issue KVM_PRE_FAULT_MEMORY for GFNs mapping shared/private
+* memory after finalizing the initial guest contents
+* (e.g. via * KVM_SEV_SNP_LAUNCH_FINISH for SNP guests).
+*
+* This should succeed since pre-faulting is supported for both
+* non-private/private memory once the guest contents are finalized.
+*/
+ PREFAULT_PRIVATE_SHARED_AFTER_FINALIZING
+};
+
+static void guest_code_sev(void)
+{
+ int i;
+
+ GUEST_ASSERT(rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ENABLED);
+
+ for (i = 0; i < TEST_NPAGES; i++) {
+ uint64_t *src = (uint64_t *)(TEST_GVA + i * PAGE_SIZE);
+ uint64_t val = *src;
+
+ /* Validate the data stored in the pages */
+ if ((i < TEST_NPAGES / 2 && val != i + 1) ||
+ (i >= TEST_NPAGES / 2 && val != 0)) {
+ GUEST_FAIL("Inconsistent view of memory values in
guest");
+ }
+ }
+
+ if (rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ES_ENABLED) {
+ wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
+ __asm__ __volatile__("rep; vmmcall");
+ GUEST_FAIL("This should be unreachable.");
+ }
+
+ GUEST_DONE();
+}
+
+static void __pre_fault_memory(struct kvm_vcpu *vcpu, u64 gpa, u64 size,
+ u64 left, bool
[PATCH v3 9/9] KVM: selftests: Interleave fallocate for KVM_PRE_FAULT_MEMORY
fallocate triggers gmem_prepare(), and KVM_PRE_FAULT_MEMORY can cause
guest page faults at unexpected points. Therefore, introduce several
test cases to interleave fallocate, hole punching through various
parts of the SNP launch lifecycle, and observe both positive and
negative vcpcu_run exit statuses.
Signed-off-by: Pratik R. Sampat
Tested-by: Peter Gonda
Tested-by: Srikanth Aithal
---
.../kvm/x86_64/coco_pre_fault_memory_test.c | 121 +-
1 file changed, 114 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
b/tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
index c31a5f9e18f4..e9757ba3234c 100644
--- a/tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
+++ b/tools/testing/selftests/kvm/x86_64/coco_pre_fault_memory_test.c
@@ -47,6 +47,31 @@ enum prefault_snp_test_type {
PREFAULT_PRIVATE_SHARED_AFTER_FINALIZING
};
+enum falloc_snp_test_type {
+ /* Skip alloc tests. */
+ NO_ALLOC_TYPE,
+ /*
+* Allocate and/or deallocate a region of guest memfd before
+* memory regions are updated to be protected and encrypted
+*
+* This should succeed since allocation and deallocation is
+* supported before the memory is finalized.
+*/
+ ALLOC_BEFORE_UPDATE,
+ ALLOC_AFTER_UPDATE,
+ DEALLOC_BEFORE_UPDATE,
+ ALLOC_DEALLOC_BEFORE_UPDATE,
+ /*
+* Allocate and/or deallocate a region of guest memfd after
+* memory regions are updated to be protected and encrypted
+*
+* This should fail since dealloc will nuke the pages that
+* contain the initial code that the guest will run.
+*/
+ DEALLOC_AFTER_UPDATE,
+ ALLOC_DEALLOC_AFTER_UPDATE
+};
+
static void guest_code_sev(void)
{
int i;
@@ -73,6 +98,29 @@ static void guest_code_sev(void)
GUEST_DONE();
}
+static void __falloc_region(struct kvm_vm *vm, bool punch_hole)
+{
+ int ctr, ret, flags = FALLOC_FL_KEEP_SIZE;
+ struct userspace_mem_region *region;
+
+ hash_for_each(vm->regions.slot_hash, ctr, region, slot_node) {
+ if (punch_hole)
+ flags |= FALLOC_FL_PUNCH_HOLE;
+ ret = fallocate(region->region.guest_memfd, flags, 0, PAGE_SIZE
* TEST_NPAGES);
+ TEST_ASSERT(!ret, "fallocate should succeed.");
+ }
+}
+
+static void gmemfd_alloc(struct kvm_vm *vm)
+{
+ __falloc_region(vm, false);
+}
+
+static void gmemfd_dealloc(struct kvm_vm *vm)
+{
+ __falloc_region(vm, true);
+}
+
static void __pre_fault_memory(struct kvm_vcpu *vcpu, u64 gpa, u64 size,
u64 left, bool expect_fail)
{
@@ -137,13 +185,34 @@ static void pre_fault_memory_negative(struct kvm_vcpu
*vcpu, u64 gpa,
}
static void pre_fault_memory_snp(struct kvm_vcpu *vcpu, struct kvm_vm *vm,
-bool private, enum prefault_snp_test_type
p_type)
+bool private, enum prefault_snp_test_type
p_type,
+enum falloc_snp_test_type f_type)
{
+ if (f_type == ALLOC_BEFORE_UPDATE ||
+ f_type == ALLOC_DEALLOC_BEFORE_UPDATE) {
+ gmemfd_alloc(vm);
+ }
+
+ if (f_type == DEALLOC_BEFORE_UPDATE ||
+ f_type == ALLOC_DEALLOC_BEFORE_UPDATE) {
+ gmemfd_dealloc(vm);
+ }
+
if (p_type == PREFAULT_SHARED_BEFORE_FINALIZING)
pre_fault_memory_negative(vcpu, TEST_GPA, SZ_2M, 0);
snp_vm_launch_start(vm, SNP_POLICY);
+ if (f_type == ALLOC_BEFORE_UPDATE ||
+ f_type == ALLOC_DEALLOC_BEFORE_UPDATE) {
+ gmemfd_alloc(vm);
+ }
+
+ if (f_type == DEALLOC_BEFORE_UPDATE ||
+ f_type == ALLOC_DEALLOC_BEFORE_UPDATE) {
+ gmemfd_dealloc(vm);
+ }
+
if (p_type == PREFAULT_SHARED_BEFORE_FINALIZING)
pre_fault_memory_negative(vcpu, TEST_GPA, SZ_2M, 0);
@@ -164,11 +233,36 @@ static void pre_fault_memory_snp(struct kvm_vcpu *vcpu,
struct kvm_vm *vm,
snp_vm_launch_update(vm);
+ if (f_type == ALLOC_AFTER_UPDATE ||
+ f_type == ALLOC_DEALLOC_AFTER_UPDATE) {
+ gmemfd_alloc(vm);
+ }
+
+ /*
+* Hole-punch after SNP LAUNCH UPDATE is not expected to fail
+* immediately, rather its affects are observed on vcpu_run()
+* as the pages that contain the initial code is nuked.
+*/
+ if (f_type == DEALLOC_AFTER_UPDATE ||
+ f_type == ALLOC_DEALLOC_AFTER_UPDATE) {
+ gmemfd_dealloc(vm);
+ }
+
if (p_type == PREFAULT_SHARED_BEFORE_FINALIZING)
pre_fault_memory_negative(vcpu, TEST_GPA, SZ_2M, 0);
snp_vm_launch_finish(vm);
+ if (f_type == ALLOC_AFTER_UPDATE ||
+ f_type == ALLOC_DEAL
Re: [PATCH v3 1/9] KVM: selftests: Decouple SEV ioctls from asserts
Hi Sean,
On 10/14/2024 5:18 PM, Sean Christopherson wrote:
> On Thu, Sep 05, 2024, Pratik R. Sampat wrote:
>> +static inline int __sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t
>> gpa,
>> + uint64_t hva, uint64_t size)
>> {
>> struct kvm_sev_launch_update_data update_data = {
>> -.uaddr = (unsigned long)addr_gpa2hva(vm, gpa),
>> +.uaddr = hva,
>> .len = size,
>> };
>>
>> -vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_DATA, &update_data);
>> +return __vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_DATA, &update_data);
>> +}
>> +
>> +static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
>> + uint64_t hva, uint64_t size)
>> +{
>> +int ret = __sev_launch_update_data(vm, gpa, hva, size);
>> +
>> +TEST_ASSERT_VM_VCPU_IOCTL(!ret, KVM_SEV_LAUNCH_UPDATE_DATA, ret, vm);
>> }
>>
>> #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 e9535ee20b7f..125a72246e09 100644
>> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> +++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> @@ -14,15 +14,16 @@
>> * 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 int encrypt_region(struct kvm_vm *vm, struct userspace_mem_region
>> *region)
>
> This is all kinds of wrong. encrypt_region() should never fail. And by
> allowing
> it to fail, any unexpected failure becomes harder to debug. It's also a lie,
> because sev_register_encrypted_memory() isn't allowed to fail, and I would bet
> that most readers would expect _that_ call to fail given the name.
>
> The granularity is also poor, and the complete lack of idempotency is going to
> be problematic. E.g. only the first region is actually tested, and if someone
> tries to do negative testing on multiple regions,
> sev_register_encrypted_memory()
> will fail due to trying to re-encrypt a region.
>
> __sev_vm_launch_update() has similar issues. encrypt_region() is allowed to
> fail, but its call to KVM_SEV_LAUNCH_UPDATE_VMSA is not.
>
> And peeking ahead, passing an @assert parameter to __test_snp_launch_start()
> (or
> any helper) is a non-starter. Readers should not have to dive into a helper's
> implementation to understand that this
>
> __test_snp_launch_start(type, policy, 0, true);
>
> is a happy path and this
>
> ret = __test_snp_launch_start(type, policy, BIT(i), false);
>
> is a sad path.
>
> And re-creating the VM every time is absurdly wasteful. While performance
> isn't
> a priority for selftests, there's no reason to make everything as slow as
> possible.
>
> Even just passing the page type to encrypt_region() is confusing. When the
> test
> is actually going to run the guest, applying ZERO and CPUID types to _all_
> pages
> is completely nonsensical.
>
> In general, I think trying to reuse the happy path's infrastructure is going
> to
> do more harm than good. This is what I was trying to get at in my feedback
> for
> the previous version.
>
> For negative tests, I would honestly say development them "from scratch", i.e.
> deliberately don't reuse the existing SEV-MEM/ES infrastructure. It'll
> require
> more copy+paste to get rolling, but I suspect that the end result will be less
> churn and far easier to read.
This makes sense. I was trying to be as minimal as possible without a
lot of replication while trying to introduce the negative tests. I see
that this has created several issues of granularity, even general
correctness and overall has created more problems than it solves.
I will try to develop the negative interface separately tailored for
this specific use-case rather than piggybacking on the happy path when I
send out the patchset #2.
Re: [PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test
Hi Sean,
Thank you for your comments.
...
>> .../selftests/kvm/include/x86_64/processor.h | 1 +
>> .../selftests/kvm/include/x86_64/sev.h| 54 +++-
>> tools/testing/selftests/kvm/lib/kvm_util.c| 8 +-
>> .../selftests/kvm/lib/x86_64/processor.c | 6 +-
>> tools/testing/selftests/kvm/lib/x86_64/sev.c | 116 +-
>> .../selftests/kvm/x86_64/sev_smoke_test.c | 67 --
>> 6 files changed, 230 insertions(+), 22 deletions(-)
>
> There is *way* too much going on in this one patch. There's at least 6+
> patches
> worth of stuff here:
>
> 1. Add x86 architectural defines
> 2. SNP ioctl() plumbing
> 3..N. Other misc plumbing, e.g. is_smt_active()
> N+1. __vm_create() change to force GUEST_MEMFD for SNP
> N+2. Whatever the ASSERT_SEV_POLICY() thing is doing, if it's actually
> necessary
> N+3..M. Refactorings to existing code to prep for SNP
> M+1. SNP support
>
> In general, if you feel the urge to start a changelog paragraph with "Also,"
> that's a sign you need to break up the patch.
Sure. I will split this into multiple patches which should
form the basis of the #1 patch series.
>
>> @@ -74,8 +95,18 @@ kvm_static_assert(SEV_RET_SUCCESS == 0);
>> __TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, vm); \
>> })
>>
>> +/* Ensure policy is within bounds for SEV, SEV-ES */
>> +#define ASSERT_SEV_POLICY(type, policy) \
>> +({ \
>> +if (type == KVM_X86_SEV_VM || type == KVM_X86_SEV_ES_VM) { \
>> +TEST_ASSERT(policy < ((uint32_t)~0U), \
>> +"Policy beyond bounds for SEV");\
>
> This is asinine. First, there's one user, i.e. I see no reason to have a
> funky
> macro to assert on the type. Second, _if_ this is a common macro, "type" can
> and
> should be incorporated into the assert. Third, I have no idea why selftests
> is
> validating its own inputs to KVM.
It wasn't strictly necessary to validate this. Since the function
vm_sev_launch() now took a u64 in policy (for SNP), I thought it maybe
useful to validate u32 for the rest, as library function can be called
by other selftests as well. However, I do see your point that it doesn't
make too much sense for selftests to try and validate it's own inputs.
I'm open to both - reducing the macro to a just a check within the
function or removing the macro altogether.
>
>> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c
>> b/tools/testing/selftests/kvm/lib/x86_64/processor.c
>> index 974bcd2df6d7..981f3c9fd1cf 100644
>> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
>> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
>> @@ -625,7 +625,8 @@ 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 (vm->type == KVM_X86_SEV_VM || vm->type == KVM_X86_SEV_ES_VM ||
>> +vm->type == KVM_X86_SNP_VM) {
>
> Probably time to add a helper, e.g. is_sev_vm() or something. If we follow
> KVM's
> lead, then we'd have is_sev_vm(), is_sev_es_vm(), and is_sev_snp_vm(), where
> an
> SNP VM returns true for all of them. Not sure I love that idea, just
> throwing it
> out there as one possibility.
Agreed. It will be cleaner to have helpers since similar checks are
being made in multiple places.
>
>> struct kvm_sev_init init = { 0 };
>>
>> vm_sev_ioctl(vm, KVM_SEV_INIT2, &init);
>> @@ -1134,7 +1135,8 @@ 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 (vm->type == KVM_X86_SEV_VM || vm->type == KVM_X86_SEV_ES_VM ||
>> +vm->type == KVM_X86_SNP_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 125a72246e09..ff3824564854 100644
>> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> +++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
>> @@ -14,7 +14,8 @@
>> * and find the first range, but that's correct because the condition
>> * expression would cause us to quit the loop.
>> */
>> -static int encrypt_region(struct kvm_vm *vm, struct userspace_mem_region
>> *region)
>> +static int 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;
>>
Re: [PATCH v3 0/9] SEV Kernel Selftests
Hi Sean, On 10/14/2024 5:23 PM, Sean Christopherson wrote: > On Thu, Sep 05, 2024, Pratik R. Sampat wrote: >> This series primarily introduces SEV-SNP test for the kernel selftest >> framework. It tests boot, ioctl, pre fault, and fallocate in various >> combinations to exercise both positive and negative launch flow paths. >> >> Patch 1 - Adds a wrapper for the ioctl calls that decouple ioctl and >> asserts, which enables the use of negative test cases. No functional >> change intended. >> Patch 2 - Extend the sev smoke tests to use the SNP specific ioctl >> calls and sets up memory to boot a SNP guest VM >> Patch 3 - Adds SNP to shutdown testing >> Patch 4, 5 - Tests the ioctl path for SEV, SEV-ES and SNP >> Patch 6 - Adds support for SNP in KVM_SEV_INIT2 tests >> Patch 7,8,9 - Enable Prefault tests for SEV, SEV-ES and SNP > > There are three separate series here: > > 1. Smoke test support for SNP > 2. Negative tests for SEV+ > 3. Prefault tests for SEV+ > > #3 likely has a dependency on #1, and probably on #2 as well (for style if > nothing > else). But that's really just an argument for focuing on #1 first, and the > moving > onto the others once that's ready to go. Based on your feedback on the rest of this patchset, this makes sense to me. I will first prep for the changes for patchset #1 and once we lock that down I can introduce patchset #2 and #3 based on that design. Thank you again for your feedback! Pratik
Re: [PATCH v3 7/9] KVM: selftests: Add interface to manually flag protected/encrypted ranges
Hi Sean,
On 10/14/2024 5:58 PM, Sean Christopherson wrote:
> On Thu, Sep 05, 2024, Pratik R. Sampat wrote:
>> From: Michael Roth
>>
>> For SEV and SNP, currently __vm_phy_pages_alloc() handles setting the
>> region->protected_phy_pages bitmap to mark that the region needs to be
>> encrypted/measured into the initial guest state prior to
>
> Nothing needs to be measured, no? (because there's no attestation)
>
Right.
>> finalizing/starting the guest. It also marks what GPAs need to be mapped
>> as encrypted in the initial guest page table.
>
> ...
>
>> static inline void vm_mem_set_private(struct kvm_vm *vm, uint64_t gpa,
>>uint64_t size)
>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c
>> b/tools/testing/selftests/kvm/lib/kvm_util.c
>> index bbf90ad224da..d44a37aebcec 100644
>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>> @@ -1991,6 +1991,43 @@ const char *exit_reason_str(unsigned int exit_reason)
>> return "Unknown";
>> }
>>
>> +/*
>> + * Set what guest GFNs need to be encrypted prior to finalizing a CoCo VM.
>> + *
>> + * Input Args:
>> + * vm - Virtual Machine
>> + * memslot - Memory region to allocate page from
>> + * paddr - Start of physical address to mark as encrypted
>> + * num - number of pages
>> + *
>> + * Output Args: None
>> + *
>> + * Return: None
>> + *
>> + * Generally __vm_phy_pages_alloc() will handle this automatically, but
>> + * for cases where the test handles managing the physical allocation and
>> + * mapping directly this interface should be used to mark physical pages
>> + * that are intended to be encrypted as part of the initial guest state.
>> + * This will also affect whether virt_map()/virt_pg_map() will map the
>> + * page as encrypted or not in the initial guest page table.
>> + *
>> + * If the initial guest state has already been finalized, then setting
>> + * it as encrypted will essentially be a noop since nothing more can be
>> + * encrypted into the initial guest state at that point.
>> + */
>> +void vm_mem_set_protected(struct kvm_vm *vm, uint32_t memslot,
>> + vm_paddr_t paddr, size_t num)
>> +{
>> +struct userspace_mem_region *region;
>> +sparsebit_idx_t pg, base;
>> +
>> +base = paddr >> vm->page_shift;
>> +region = memslot2region(vm, memslot);
>
> Please no, doing a memslot lookup in a helper like this is only going to
> encourage
> proliferation of bad code. vm_mem_add() really should be able to mark the
> region
> as protected.
>
> E.g. practically speaking, the only code that will be able to use this helper
> is
> code that is marking the entire memslot as protection. And ability to _clear_
> the protected_phy_pages bit is conspicuously missing.
>
>From my understanding, vm_mem_add() only allocates the protected
sparsebits but does not populate them. For that maybe a better path
would be to go through, vm_phy_pages_alloc() - something similar to how
the set_memory_region_test.c does? I think we avoided doing that because
vm_phys_pages_alloc() takes a paddr_min rather than guaranteeing the
specific paddr and even then would need the similar virt_map() logic to
stay. If this is a cleaner approach though, I'm happy to redo this code
that way.
Thanks!
Pratik
>> +
>> +for (pg = base; pg < base + num; ++pg)
>> +sparsebit_set(region->protected_phy_pages, pg);
>> +}
Re: [PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test
Hi Sean,
On 10/28/2024 12:55 PM, Sean Christopherson wrote:
> On Mon, Oct 21, 2024, Pratik R. Sampat wrote:
>>>> + test_sev(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
>>>>
>>>>test_sev_es_shutdown();
>>>>
>>>>if (kvm_has_cap(KVM_CAP_XCRS) &&
>>>>(xgetbv(0) & XFEATURE_MASK_X87_AVX) ==
>>>> XFEATURE_MASK_X87_AVX) {
>>>> - test_sync_vmsa(0);
>>>> - test_sync_vmsa(SEV_POLICY_NO_DBG);
>>>> + test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
>>>> + test_sync_vmsa(KVM_X86_SEV_ES_VM, SEV_POLICY_ES |
>>>> SEV_POLICY_NO_DBG);
>>>> + }
>>>> + }
>>>> +
>>>> + if (kvm_cpu_has(X86_FEATURE_SNP) && is_kvm_snp_supported()) {
>>>
>>> Why do we need both? KVM shouldn't advertise SNP if it's not supported.
>>
>> My rationale behind needing this was that the feature can be advertised
>> but it may not have the right API major or minor release which could be
>> updated post boot and could determine it's support during runtime.
>
> KVM will never determine support after KVM has been loaded. If *KVM* has a
> dependency on the API major.minor, then X86_FEATURE_SNP must be set if and
> only
> if the supported API version is available.
>
> If the API major.minor is purely a userspace thing, then
> is_kvm_snp_supported()
> is misnamed, because the check has nothing to do with KVM. E.g. something
> like
> is_snp_api_version_supported() would be more appropriate.
That's fair. It is related to the FW supplied to it from userspace and
naming it with kvm prefix is misleading. I'll change that.
>
>>>> + unsigned long snp_policy = SNP_POLICY;
>>>
>>> u64, no?
>>
>> Yes, sorry for the oversight. Will change it to u64.
>>
>>>
>>>> +
>>>> + if (unlikely(!is_smt_active()))
>>>> + snp_policy &= ~SNP_POLICY_SMT;
>>>
>>> Why does SNP_POLICY assume SMT? And what is RSVD_MBO? E.g. why not this?
>>>
>>> u64 policy = is_smt_active() ? SNP_POLICY_SMT : SNP_POLICY;
>>>
>>
>> I think most systems support SMT so I enabled the bit in by default and
>> only unset it when there isn't any support.
>
> That's confusing though, because you're mixing architectural defines with
> semi-
> arbitrary selftests behavior. RSVD_MBO on the other is apparently tightly
> coupled
> with SNP, i.e. SNP can't exist without that bit, so it makes sense that
> RSVD_MBO
> needs to be part of SNP_POLICY
>
> If you want to have a *software*-defined default policy, then make it obvious
> that
> it's software defined. E.g. name the #define SNP_DEFAULT_POLICY, not simply
> SNP_POLICY, because the latter is too easily misconstrued as the base SNP
> policy,
> which it is not. That said, IIUC, SMT *must* match the host configuration,
> i.e.
> whether or not SMT is set is non-negotiable. In that case, there's zero
> value in
> defining SNP_DEFAULT_POLICY, because it can't be a sane default for all
> systems.
>
Right, SMT should match the host configuration. Would a
SNP_DEFAULT_POLICY work if we made it check for SMT too in the macro?
Instead of,
#define SNP_POLICY (SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO)
Have something like this instead to make it generic and less ambiguous?
#define SNP_DEFAULT_POLICY() \
({ \
SNP_POLICY_RSVD_MBO | (is_smt_active() ? SNP_POLICY_SMT : 0); \
})
> Side topic, I assume one of SEV_POLICY_NO_DBG or SNP_POLICY_DBG *must* be
> specified,
> and that they are mutualy exclusive? E.g. what happens if the full policy is
> simply
> SNP_POLICY_RSVD_MBO?
SEV_POLICY_NO_DBG is mainly for the guest policy structure of SEV and
SEV-ES - pg 31, Table 2
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/55766_SEV-KM_API_Specification.pdf
and, SNP_POLICY_DBG is a bit in the guest policy structure of SNP - pg
27, Table 9
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56860.pdf
In the former, a SEV guest disables debugging if SEV_POLICY_NO_DBG is
set. Similarly, a SNP guest enables debugging if SNP_POLICY_DBG is set.
An SNP guest can certainly just have the policy SNP_POLICY_RSVD_MBO,
barring the case on a SMT system where that bit must be set too for a
successful launch.
Re: [PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test
On 11/4/2024 5:47 PM, Sean Christopherson wrote:
> On Mon, Nov 04, 2024, Pratik R. Sampat wrote:
>>
>>
>> On 10/31/2024 11:27 AM, Sean Christopherson wrote:
>>> On Thu, Oct 31, 2024, Pratik R. Sampat wrote:
>>>> Hi Sean,
>>>>
>>>> On 10/30/2024 12:57 PM, Sean Christopherson wrote:
>>>>> On Wed, Oct 30, 2024, Pratik R. Sampat wrote:
>>>>>> On 10/30/2024 8:46 AM, Sean Christopherson wrote:
>>>>>>> +/* Minimum firmware version required for the SEV-SNP support */
>>>>>>> +#define SNP_FW_REQ_VER_MAJOR 1
>>>>>>> +#define SNP_FW_REQ_VER_MINOR 51
>>>>>>>
>>>>>>> Side topic, why are these hardcoded? And where did they come from? If
>>>>>>> they're
>>>>>>> arbitrary KVM selftests values, make that super duper clear.
>>>>>>
>>>>>> Well, it's not entirely arbitrary. This was the version that SNP GA'd
>>>>>> with first so that kind of became the minimum required version needed.
>>>>>>
>>>>>> I think the only place we've documented this is here -
>>>>>> https://github.com/AMDESE/AMDSEV/tree/snp-latest?tab=readme-ov-file#upgrade-sev-firmware.
>>>>>>
>>>>>> Maybe, I can modify the comment above to say something like -
>>>>>> Minimum general availability release firmware required for SEV-SNP
>>>>>> support.
>>>>>
>>>>> Hmm, so if AMD says SNP is only supported for firmware version >= 1.51,
>>>>> why on
>>>>> earth is that not checked and enforced by the kernel? Relying on
>>>>> userspace to
>>>>> not crash the host (or worse) because of unsupported firmware is not a
>>>>> winning
>>>>> strategy.
>>>>
>>>> We do check against the firmware level 1.51 while setting things up
>>>> first (drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked()) and we bail
>>>> out if it's otherwise. From the userspace, calls to KVM_SEV_INIT2 or any
>>>> other corresponding SNP calls should fail cleanly without any adverse
>>>> effects to the host.
>>>
>>> And I'm saying, that's not good enough. If the platform doesn't support
>>> SNP,
>>> the KVM *must not* advertise support for SNP.
>>>
>>
>> Sure, fair to expect this. Currently, if the FW check fails, SNP is not
>> setup and there is nothing that indicates in the KVM capabilities (apart
>> from one dmesg error) that the support does not exist.
>>
>> One thing I could do (as an independent patch) is to introduce a CC API
>> that abstracts the FW version check made by the CCP module. Since sev
>> platform status can be gotten before INIT to extract the major and minor
>> version numbers, KVM can also call into this API and use that to decide
>> if the KVM capabilities for SNP must be set or not.
>
> Why is CC_ATTR_HOST_SEV_SNP set if hardware/firmware can't actually support
> SNP?
> KVM shouldn't have to care about some arbitrary firmware API version, the
> whole
> point of a driver is so that KVM doesn't have to deal with such details.
>
> I'm a-ok with a KVM selftest *asserting* that the kernel isn't broken, but KVM
> itself shouldn't need to manually check the firmware version.
Clearing CC_ATTR_HOST_SEV_SNP when the init fails is one approach to go
about it. Here we could clear it from here and eventually that would
prevent the the SNP feature being set in KVM capability.
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -1099,6 +1099,7 @@ static int __sev_snp_init_locked(int *error)
return 0;
if (!sev_version_greater_or_equal(SNP_MIN_API_MAJOR,
SNP_MIN_API_MINOR)) {
+ cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
A suggestion where we could more directly approach this could be by
exporting an explicit check from ccp instead?
--- 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 sev_snp_fw_available(void)
+{
+return sev_version_greater_or_equal(SNP_MIN_API_MAJOR, SNP_MIN_API_MINOR);
+}
+EXPORT_SYMBOL_GPL(sev_snp_fw_available);
which could be then called on will as follows:
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3050,7 +3050,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) && sev_snp_fw_available();
out:
if (boot_cpu_has(X86_FEATURE_SEV))
This would ensure that we could enable and disable the SNP capability
even in the case where maybe the firmware can get hotloaded using the
proposed download_firmware_ex[1] or in cases where the INIT could be
deferred; all while KVM wouldn't need to be bothered with the API
details.
[1]:
https://lore.kernel.org/lkml/20241029183907.3536683-1-dionnagl...@google.com/
Re: [PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test
On 10/31/2024 11:27 AM, Sean Christopherson wrote: > On Thu, Oct 31, 2024, Pratik R. Sampat wrote: >> Hi Sean, >> >> On 10/30/2024 12:57 PM, Sean Christopherson wrote: >>> On Wed, Oct 30, 2024, Pratik R. Sampat wrote: >>>> On 10/30/2024 8:46 AM, Sean Christopherson wrote: >>>>> +/* Minimum firmware version required for the SEV-SNP support */ >>>>> +#define SNP_FW_REQ_VER_MAJOR 1 >>>>> +#define SNP_FW_REQ_VER_MINOR 51 >>>>> >>>>> Side topic, why are these hardcoded? And where did they come from? If >>>>> they're >>>>> arbitrary KVM selftests values, make that super duper clear. >>>> >>>> Well, it's not entirely arbitrary. This was the version that SNP GA'd >>>> with first so that kind of became the minimum required version needed. >>>> >>>> I think the only place we've documented this is here - >>>> https://github.com/AMDESE/AMDSEV/tree/snp-latest?tab=readme-ov-file#upgrade-sev-firmware. >>>> >>>> Maybe, I can modify the comment above to say something like - >>>> Minimum general availability release firmware required for SEV-SNP support. >>> >>> Hmm, so if AMD says SNP is only supported for firmware version >= 1.51, why >>> on >>> earth is that not checked and enforced by the kernel? Relying on userspace >>> to >>> not crash the host (or worse) because of unsupported firmware is not a >>> winning >>> strategy. >> >> We do check against the firmware level 1.51 while setting things up >> first (drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked()) and we bail >> out if it's otherwise. From the userspace, calls to KVM_SEV_INIT2 or any >> other corresponding SNP calls should fail cleanly without any adverse >> effects to the host. > > And I'm saying, that's not good enough. If the platform doesn't support SNP, > the KVM *must not* advertise support for SNP. > Sure, fair to expect this. Currently, if the FW check fails, SNP is not setup and there is nothing that indicates in the KVM capabilities (apart from one dmesg error) that the support does not exist. One thing I could do (as an independent patch) is to introduce a CC API that abstracts the FW version check made by the CCP module. Since sev platform status can be gotten before INIT to extract the major and minor version numbers, KVM can also call into this API and use that to decide if the KVM capabilities for SNP must be set or not. Thanks! Pratik >> From the positive selftest perspective though, we want to make sure it's >> both supported and enabled, and skip the test if not. > > No, we want the test to assert that KVM reports SNP support if and only if SNP > is 100% supported. > >> I believe we can tell if it's supported by the platform using the MSR - >> MSR_AMD64_SEV_SNP_ENABLED or the X86_FEATURE_SEV_SNP from the KVM >> capabilities. However, to determine if it's enabled from the kernel, I >> made this check here. Having said that, I do agree that there should >> probably be a better way to expose this support to the userspace. >> >> Thanks >> Pratik
Re: [PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test
Hi Sean,
On 10/30/2024 8:46 AM, Sean Christopherson wrote:
> On Mon, Oct 28, 2024, Pratik R. Sampat wro4te:
>> On 10/28/2024 12:55 PM, Sean Christopherson wrote:
>>> On Mon, Oct 21, 2024, Pratik R. Sampat wrote:
>>>>>> +if (unlikely(!is_smt_active()))
>>>>>> +snp_policy &= ~SNP_POLICY_SMT;
>>>>>
>>>>> Why does SNP_POLICY assume SMT? And what is RSVD_MBO? E.g. why not this?
>>>>>
>>>>> u64 policy = is_smt_active() ? SNP_POLICY_SMT : SNP_POLICY;
>>>>>
>>>>
>>>> I think most systems support SMT so I enabled the bit in by default and
>>>> only unset it when there isn't any support.
>>>
>>> That's confusing though, because you're mixing architectural defines with
>>> semi-
>>> arbitrary selftests behavior. RSVD_MBO on the other is apparently tightly
>>> coupled
>>> with SNP, i.e. SNP can't exist without that bit, so it makes sense that
>>> RSVD_MBO
>>> needs to be part of SNP_POLICY
>>>
>>> If you want to have a *software*-defined default policy, then make it
>>> obvious that
>>> it's software defined. E.g. name the #define SNP_DEFAULT_POLICY, not simply
>>> SNP_POLICY, because the latter is too easily misconstrued as the base SNP
>>> policy,
>>> which it is not. That said, IIUC, SMT *must* match the host configuration,
>>> i.e.
>>> whether or not SMT is set is non-negotiable. In that case, there's zero
>>> value in
>>> defining SNP_DEFAULT_POLICY, because it can't be a sane default for all
>>> systems.
>>>
>>
>> Right, SMT should match the host configuration. Would a
>> SNP_DEFAULT_POLICY work if we made it check for SMT too in the macro?
>>
>> Instead of,
>> #define SNP_POLICY (SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO)
>>
>> Have something like this instead to make it generic and less ambiguous?
>> #define SNP_DEFAULT_POLICY()\
>> ({ \
>> SNP_POLICY_RSVD_MBO | (is_smt_active() ? SNP_POLICY_SMT : 0); \
>> })
>
> No, unless it's the least awful option, don't hide dynamic functionality in a
> macro
> that looks like it holds static data. The idea is totally fine, but put it
> in an
> actual helper, not a macro, _if_ there's actually a need for a default policy.
> If there's only ever one main path that creates SNP VMs, then I don't see the
> point
> in specifying a default policy.
>
Currently, there just seems to be one path of doing (later the prefault
tests exercise it) so I'm not too averse to just dropping it and having
what bits needs to be set during the main path.
I had only introduced it so that it would be easy to specify a minimal
working SNP policy as it was for SEV and SEV-ES without too much
ambiguity. But if it's causing more issues than resolving, I can
definitely get rid of it.
>>> Side topic, I assume one of SEV_POLICY_NO_DBG or SNP_POLICY_DBG *must* be
>>> specified,
>>> and that they are mutualy exclusive? E.g. what happens if the full policy
>>> is simply
>>> SNP_POLICY_RSVD_MBO?
>>
>> SEV_POLICY_NO_DBG is mainly for the guest policy structure of SEV and
>> SEV-ES - pg 31, Table 2
>> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/programmer-references/55766_SEV-KM_API_Specification.pdf
>>
>> and, SNP_POLICY_DBG is a bit in the guest policy structure of SNP - pg
>> 27, Table 9
>> https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56860.pdf
>>
>> In the former, a SEV guest disables debugging if SEV_POLICY_NO_DBG is
>> set. Similarly, a SNP guest enables debugging if SNP_POLICY_DBG is set.
>
> Ugh, one is SEV_xxx, the other is SNP_xxx. Argh! And IIUC, they are mutually
> exclusive (totally separate thigns?), because SNP guests use an 8-byte
> structure,
> whereas SEV/SEV-ES use a 4-byte structure, and with different layouts.
>
> That means this is _extremely_ confusing. Separate the SEV_xxx defines from
> the
> SNP_xxx defines, because other than a name, they have nothing in common.
>
Right. I see how that can be confusing. Sure I can make sure not to
bundle up these defines together.
> +/* Minimum firmware version required for the SEV-SNP support */
> +#define SNP_FW_REQ_VER_MAJOR 1
> +#define SNP_FW_REQ_VER_MINOR 51
>
> Side topic, why are these hardcoded? And whe
Re: [PATCH v3 2/9] KVM: selftests: Add a basic SNP smoke test
Hi Sean, On 10/30/2024 12:57 PM, Sean Christopherson wrote: > On Wed, Oct 30, 2024, Pratik R. Sampat wrote: >> On 10/30/2024 8:46 AM, Sean Christopherson wrote: >>> +/* Minimum firmware version required for the SEV-SNP support */ >>> +#define SNP_FW_REQ_VER_MAJOR 1 >>> +#define SNP_FW_REQ_VER_MINOR 51 >>> >>> Side topic, why are these hardcoded? And where did they come from? If >>> they're >>> arbitrary KVM selftests values, make that super duper clear. >> >> Well, it's not entirely arbitrary. This was the version that SNP GA'd >> with first so that kind of became the minimum required version needed. >> >> I think the only place we've documented this is here - >> https://github.com/AMDESE/AMDSEV/tree/snp-latest?tab=readme-ov-file#upgrade-sev-firmware. >> >> Maybe, I can modify the comment above to say something like - >> Minimum general availability release firmware required for SEV-SNP support. > > Hmm, so if AMD says SNP is only supported for firmware version >= 1.51, why on > earth is that not checked and enforced by the kernel? Relying on userspace to > not crash the host (or worse) because of unsupported firmware is not a winning > strategy. We do check against the firmware level 1.51 while setting things up first (drivers/crypto/ccp/sev-dev.c:__sev_snp_init_locked()) and we bail out if it's otherwise. From the userspace, calls to KVM_SEV_INIT2 or any other corresponding SNP calls should fail cleanly without any adverse effects to the host. >From the positive selftest perspective though, we want to make sure it's both supported and enabled, and skip the test if not. I believe we can tell if it's supported by the platform using the MSR - MSR_AMD64_SEV_SNP_ENABLED or the X86_FEATURE_SEV_SNP from the KVM capabilities. However, to determine if it's enabled from the kernel, I made this check here. Having said that, I do agree that there should probably be a better way to expose this support to the userspace. Thanks Pratik
[PATCH v4 4/8] KVM: selftests: Introduce SEV VM type check
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
---
.../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 3/8] KVM: selftests: Add VMGEXIT helper
Abstract rep vmmcall coded into the VMGEXIT helper for the sev
library.
No functional change intended.
Signed-off-by: Pratik R. Sampat
---
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 7/8] KVM: selftests: Abstractions for SEV to decouple policy from type
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
---
.../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 6/8] KVM: selftests: Force GUEST_MEMFD flag for SNP VM type
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
---
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 8/8] KVM: selftests: Add a basic SEV-SNP smoke test
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
---
.../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
[PATCH v4 1/8] KVM: SEV: Disable SEV-SNP on FW validation failure
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
---
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 5/8] KVM: selftests: Add library support for interacting with SNP
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
---
.../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,
[PATCH v4 0/8] Basic SEV-SNP Selftests
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.sam...@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 2/8] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
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
---
.../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 {
#defineX86_FEATURE_VGIFKVM_X86_CPU_FEATURE(0x800A,
0, EDX, 16)
#define X86_FEATURE_SEVKVM_X86_CPU_FEATURE(0x801F,
0, EAX, 1)
#define X86_FEATURE_SEV_ES KVM_X86_CPU_FEATURE(0x801F, 0, EAX,
3)
+#define X86_FEATURE_SNPKVM_X86_CPU_FEATURE(0x801F,
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
Re: [PATCH v8 00/10] Basic SEV-SNP Selftests
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
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.ka...@amd.com/ [2]: https://lore.kernel.org/kvm/d8de6de80c36721ea3eb92ecac81b211f401c3b2.1742850400.git.ashish.ka...@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
[PATCH v8 07/10] KVM: selftests: Add library support for interacting with SNP
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
---
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;
[PATCH v8 03/10] KVM: selftests: Add vmgexit helper
Abstract rep vmmcall coded into the vmgexit helper for the sev
library.
No functional change intended.
Signed-off-by: Pratik R. Sampat
---
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 06/10] KVM: selftests: Introduce SEV VM type check
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
---
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 08/10] KVM: selftests: Force GUEST_MEMFD flag for SNP VM type
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
---
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 05/10] KVM: selftests: Replace assert() with TEST_ASSERT_EQ()
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
---
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 00/10] Basic SEV-SNP Selftests
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-prsam...@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-8db6d50a1...@amd.com [2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226950.git.ashish.ka...@amd.com v5..v6: https://lore.kernel.org/kvm/ab433246-e97c-495b-ab67-b0cb1721f...@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-343b1c386...@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.sam...@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.sam...@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
[PATCH v8 02/10] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2
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
Signed-off-by: Pratik R. Sampat
---
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 {
#defineX86_FEATURE_VGIFKVM_X86_CPU_FEATURE(0x800A,
0, EDX, 16)
#define X86_FEATURE_SEVKVM_X86_CPU_FEATURE(0x801F,
0, EAX, 1)
#define X86_FEATURE_SEV_ES KVM_X86_CPU_FEATURE(0x801F, 0, EAX,
3)
+#define X86_FEATURE_SEV_SNPKVM_X86_CPU_FEATURE(0x801F,
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 01/10] KVM: SEV: Disable SEV-SNP support on initialization failure
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
Signed-off-by: Pratik R. Sampat
---
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 09/10] KVM: selftests: Abstractions for SEV to decouple policy from type
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
---
.../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 04/10] KVM: selftests: Add SMT control state helper
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
---
.../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 10/10] KVM: selftests: Add a basic SEV-SNP smoke test
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
---
.../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 00/10] Basic SEV-SNP Selftests
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-prsam...@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-8db6d50a1...@amd.com [2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226950.git.ashish.ka...@amd.com v5..v6: https://lore.kernel.org/kvm/ab433246-e97c-495b-ab67-b0cb1721f...@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-343b1c386...@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.sam...@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.sam...@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
