This patch series extends the sev_init2 and the sev_smoke test to exercise the SEV-SNP VM launch workflow.
Primarily, it introduces the architectural defines, its support in the SEV library and extends the tests to interact with the SEV-SNP ioctl() wrappers.
Patch 1 - Do not advertise SNP on initialization failure Patch 2 - SNP test for KVM_SEV_INIT2 Patch 3 - Add vmgexit helper Patch 4 - Add SMT control interface helper Patch 5 - Replace assert() with TEST_ASSERT_EQ() Patch 6 - Introduce SEV+ VM type check Patch 7 - SNP iotcl() plumbing for the SEV library Patch 8 - Force set GUEST_MEMFD for SNP Patch 9 - Cleanups of smoke test - Decouple policy from type Patch 10 - SNP smoke test
The series is based on git.kernel.org/pub/scm/virt/kvm/kvm.git next
v7..v8: * Dropped exporting the SNP initialized API from ccp to KVM. Instead call SNP_PLATFORM_STATUS within KVM to query the initialization. (Tom)
While it may be cheaper to query sev->snp_initialized from ccp, making the SNP platform call within KVM does away with any dependencies.
v6..v7: https://lore.kernel.org/kvm/20250221210200.244405-7-prsampat@amd.com/ Based on comments from Sean - * Replaced FW check with sev->snp_initialized * Dropped the patch which removes SEV+ KVM advertisement if INIT fails. This should be now be resolved by the combination of the patches [1,2] from Ashish. * Change vmgexit to an inline function * Export SMT control parsing interface to kvm_util Note: hyperv_cpuid KST only compile tested * Replace assert() with TEST_ASSERT_EQ() within SEV library * Define KVM_SEV_PAGE_TYPE_INVALID for SEV call of encrypt_region() * Parameterize encrypt_region() to include privatize_region() * Deduplication of sev test calls between SEV,SEV-ES and SNP * Removed FW version tests for SNP * Included testing of SNP_POLICY_DBG * Dropped most tags from patches that have been changed or indirectly affected
[1] https://lore.kernel.org/all/d6d08c6b-9602-4f3d-92c2-8db6d50a1b92@amd.com [2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226...
v5..v6: https://lore.kernel.org/kvm/ab433246-e97c-495b-ab67-b0cb1721fb99@amd.com/ * Rename is_sev_platform_init to sev_fw_initialized (Nikunj) * Rename KVM CPU feature X86_FEATURE_SNP to X86_FEATURE_SEV_SNP (Nikunj) * Collected Tags from Nikunj, Pankaj, Srikanth.
v4..v5: https://lore.kernel.org/kvm/8e7d8172-879e-4a28-8438-343b1c386ec9@amd.com/ * Introduced a check to disable advertising support for SEV, SEV-ES and SNP when platform initialization fails (Nikunj) * Remove the redundant SNP check within is_sev_vm() (Nikunj) * Cleanup of the encrypt_region flow for better readability (Nikunj) * Refactor paths to use the canonical $(ARCH) to rebase for kvm/next
v3..v4: https://lore.kernel.org/kvm/20241114234104.128532-1-pratikrajesh.sampat@amd.... * Remove SNP FW API version check in the test and ensure the KVM capability advertises the presence of the feature. Retain the minimum version definitions to exercise these API versions in the smoke test * Retained only the SNP smoke test and SNP_INIT2 test * The SNP architectural defined merged with SNP_INIT2 test patch * SNP shutdown merged with SNP smoke test patch * Add SEV VM type check to abstract comparisons and reduce clutter * Define a SNP default policy which sets bits based on the presence of SMT * Decouple privatization and encryption for it to be SNP agnostic * Assert for only positive tests using vm_ioctl() * Dropped tested-by tags
In summary - based on comments from Sean, I have primarily reduced the scope of this patch series to focus on breaking down the SNP smoke test patch (v3 - patch2) to first introduce SEV-SNP support and use this interface to extend the sev_init2 and the sev_smoke test.
The rest of the v3 patchset that introduces ioctl, pre fault, fallocate and negative tests, will be re-worked and re-introduced subsequently in future patch series post addressing the issues discussed.
v2..v3: https://lore.kernel.org/kvm/20240905124107.6954-1-pratikrajesh.sampat@amd.co... * Remove the assignments for the prefault and fallocate test type enums. * Fix error message for sev launch measure and finish. * Collect tested-by tags [Peter, Srikanth]
Pratik R. Sampat (10): KVM: SEV: Disable SEV-SNP support on initialization failure KVM: selftests: SEV-SNP test for KVM_SEV_INIT2 KVM: selftests: Add vmgexit helper KVM: selftests: Add SMT control state helper KVM: selftests: Replace assert() with TEST_ASSERT_EQ() KVM: selftests: Introduce SEV VM type check KVM: selftests: Add library support for interacting with SNP KVM: selftests: Force GUEST_MEMFD flag for SNP VM type KVM: selftests: Abstractions for SEV to decouple policy from type KVM: selftests: Add a basic SEV-SNP smoke test
arch/x86/include/uapi/asm/kvm.h | 1 + arch/x86/kvm/svm/sev.c | 30 +++++- tools/arch/x86/include/uapi/asm/kvm.h | 1 + .../testing/selftests/kvm/include/kvm_util.h | 35 +++++++ .../selftests/kvm/include/x86/processor.h | 1 + tools/testing/selftests/kvm/include/x86/sev.h | 42 ++++++++- tools/testing/selftests/kvm/lib/kvm_util.c | 7 +- .../testing/selftests/kvm/lib/x86/processor.c | 4 +- tools/testing/selftests/kvm/lib/x86/sev.c | 93 +++++++++++++++++-- .../testing/selftests/kvm/x86/hyperv_cpuid.c | 19 ---- .../selftests/kvm/x86/sev_init2_tests.c | 13 +++ .../selftests/kvm/x86/sev_smoke_test.c | 75 +++++++++------ 12 files changed, 261 insertions(+), 60 deletions(-)
During platform init, SNP initialization may fail for several reasons, such as firmware command failures and incompatible versions. However, the KVM capability may continue to advertise support for it. During setup, query the SNP platform status to obtain the initialization state and use it as an additional condition to determine support for SEV-SNP.
Fixes: 1dfe571c12cf ("KVM: SEV: Add initial SEV-SNP support") Suggested-by: Sean Christopherson seanjc@google.com Signed-off-by: Pratik R. Sampat prsampat@amd.com --- v7..v8: * Avoid exporting yet another API from CCP. Instead query SNP_PLATFORM_STATUS to get the current the initialization state within KVM (Tom) --- arch/x86/kvm/svm/sev.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 0dbb25442ec1..e21c3aa6f592 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2950,6 +2950,32 @@ void __init sev_set_cpu_caps(void) } }
+static bool snp_initialized(void) +{ + struct sev_user_data_snp_status *status; + struct sev_data_snp_addr buf; + bool initialized = false; + void *data; + int error; + + data = snp_alloc_firmware_page(GFP_KERNEL_ACCOUNT); + if (!data) + return initialized; + + buf.address = __psp_pa(data); + if (sev_do_cmd(SEV_CMD_SNP_PLATFORM_STATUS, &buf, &error)) + goto out; + + status = (struct sev_user_data_snp_status *)data; + if (status->state) + initialized = true; + +out: + snp_free_firmware_page(data); + + return initialized; +} + void __init sev_hardware_setup(void) { unsigned int eax, ebx, ecx, edx, sev_asid_count, sev_es_asid_count; @@ -3050,7 +3076,9 @@ void __init sev_hardware_setup(void) sev_es_asid_count = min_sev_asid - 1; WARN_ON_ONCE(misc_cg_set_capacity(MISC_CG_RES_SEV_ES, sev_es_asid_count)); sev_es_supported = true; - sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP); + sev_snp_supported = (sev_snp_enabled && + cc_platform_has(CC_ATTR_HOST_SEV_SNP) && + snp_initialized());
out: if (boot_cpu_has(X86_FEATURE_SEV))
Add the X86_FEATURE_SEV_SNP CPU feature to the architectural definition for the SEV-SNP VM type to exercise the KVM_SEV_INIT2 call. Ensure that the SNP test is skipped in scenarios where CPUID supports it but KVM does not, preventing reporting of failure in such cases.
Reviewed-by: Nikunj A Dadhania nikunj@amd.com Signed-off-by: Pratik R. Sampat prsampat@amd.com --- tools/testing/selftests/kvm/include/x86/processor.h | 1 + tools/testing/selftests/kvm/x86/sev_init2_tests.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)
diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h index d60da8966772..6f63fd10bbc6 100644 --- a/tools/testing/selftests/kvm/include/x86/processor.h +++ b/tools/testing/selftests/kvm/include/x86/processor.h @@ -199,6 +199,7 @@ struct kvm_x86_cpu_feature { #define X86_FEATURE_VGIF KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 16) #define X86_FEATURE_SEV KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1) #define X86_FEATURE_SEV_ES KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3) +#define X86_FEATURE_SEV_SNP KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 4)
/* * KVM defined paravirt features. diff --git a/tools/testing/selftests/kvm/x86/sev_init2_tests.c b/tools/testing/selftests/kvm/x86/sev_init2_tests.c index 3fb967f40c6a..ab3dd11ac163 100644 --- a/tools/testing/selftests/kvm/x86/sev_init2_tests.c +++ b/tools/testing/selftests/kvm/x86/sev_init2_tests.c @@ -28,6 +28,7 @@ int kvm_fd; u64 supported_vmsa_features; bool have_sev_es; +bool have_snp;
static int __sev_ioctl(int vm_fd, int cmd_id, void *data) { @@ -83,6 +84,9 @@ void test_vm_types(void) if (have_sev_es) test_init2(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
+ if (have_snp) + test_init2(KVM_X86_SNP_VM, &(struct kvm_sev_init){}); + test_init2_invalid(0, &(struct kvm_sev_init){}, "VM type is KVM_X86_DEFAULT_VM"); if (kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM)) @@ -138,15 +142,24 @@ int main(int argc, char *argv[]) "sev-es: KVM_CAP_VM_TYPES (%x) does not match cpuid (checking %x)", kvm_check_cap(KVM_CAP_VM_TYPES), 1 << KVM_X86_SEV_ES_VM);
+ have_snp = kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SNP_VM); + TEST_ASSERT(!have_snp || kvm_cpu_has(X86_FEATURE_SEV_SNP), + "sev-snp: KVM_CAP_VM_TYPES (%x) indicates SNP support (bit %d), but CPUID does not", + kvm_check_cap(KVM_CAP_VM_TYPES), KVM_X86_SNP_VM); + test_vm_types();
test_flags(KVM_X86_SEV_VM); if (have_sev_es) test_flags(KVM_X86_SEV_ES_VM); + if (have_snp) + test_flags(KVM_X86_SNP_VM);
test_features(KVM_X86_SEV_VM, 0); if (have_sev_es) test_features(KVM_X86_SEV_ES_VM, supported_vmsa_features); + if (have_snp) + test_features(KVM_X86_SNP_VM, supported_vmsa_features);
return 0; }
Abstract rep vmmcall coded into the vmgexit helper for the sev library.
No functional change intended.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- tools/testing/selftests/kvm/include/x86/sev.h | 5 +++++ tools/testing/selftests/kvm/x86/sev_smoke_test.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h index 82c11c81a956..3003dc837fb7 100644 --- a/tools/testing/selftests/kvm/include/x86/sev.h +++ b/tools/testing/selftests/kvm/include/x86/sev.h @@ -71,6 +71,11 @@ kvm_static_assert(SEV_RET_SUCCESS == 0); void sev_vm_init(struct kvm_vm *vm); void sev_es_vm_init(struct kvm_vm *vm);
+static inline void vmgexit(void) +{ + __asm__ __volatile__("rep; vmmcall"); +} + static inline void sev_register_encrypted_memory(struct kvm_vm *vm, struct userspace_mem_region *region) { diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c index a1a688e75266..6812b94bf5b6 100644 --- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c +++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c @@ -27,7 +27,7 @@ static void guest_sev_es_code(void) * force "termination" to signal "done" via the GHCB MSR protocol. */ wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ); - __asm__ __volatile__("rep; vmmcall"); + vmgexit(); }
static void guest_sev_code(void)
On 3/5/2025 11:59 PM, Pratik R. Sampat wrote:
Abstract rep vmmcall coded into the vmgexit helper for the sev library.
No functional change intended.
Signed-off-by: Pratik R. Sampat prsampat@amd.com
Reviewed-by: Pankaj Gupta pankaj.gupta@amd.com
tools/testing/selftests/kvm/include/x86/sev.h | 5 +++++ tools/testing/selftests/kvm/x86/sev_smoke_test.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h index 82c11c81a956..3003dc837fb7 100644 --- a/tools/testing/selftests/kvm/include/x86/sev.h +++ b/tools/testing/selftests/kvm/include/x86/sev.h @@ -71,6 +71,11 @@ kvm_static_assert(SEV_RET_SUCCESS == 0); void sev_vm_init(struct kvm_vm *vm); void sev_es_vm_init(struct kvm_vm *vm); +static inline void vmgexit(void) +{
- __asm__ __volatile__("rep; vmmcall");
+}
- static inline void sev_register_encrypted_memory(struct kvm_vm *vm, struct userspace_mem_region *region) {
diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c index a1a688e75266..6812b94bf5b6 100644 --- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c +++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c @@ -27,7 +27,7 @@ static void guest_sev_es_code(void) * force "termination" to signal "done" via the GHCB MSR protocol. */ wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
- __asm__ __volatile__("rep; vmmcall");
- vmgexit(); }
static void guest_sev_code(void)
Move the SMT control check out of the hyperv_cpuid selftest so that it is generally accessible all KVM selftests. Split the functionality into a helper that populates a buffer with SMT control value which other helpers can use to ascertain if SMT state is available and active.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- .../testing/selftests/kvm/include/kvm_util.h | 35 +++++++++++++++++++ .../testing/selftests/kvm/x86/hyperv_cpuid.c | 19 ---------- 2 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 4c4e5a847f67..446f04b2710f 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -542,6 +542,41 @@ static inline uint64_t vm_get_stat(struct kvm_vm *vm, const char *stat_name) return data; }
+static inline bool read_smt_control(char *buf, size_t buf_size) +{ + FILE *f = fopen("/sys/devices/system/cpu/smt/control", "r"); + bool ret; + + if (!f) + return false; + + ret = fread(buf, sizeof(*buf), buf_size, f) > 0; + fclose(f); + + return ret; +} + +static inline bool smt_possible(void) +{ + char buf[16]; + + if (read_smt_control(buf, sizeof(buf)) && + (!strncmp(buf, "forceoff", 8) || !strncmp(buf, "notsupported", 12))) + return false; + + return true; +} + +static inline bool smt_on(void) +{ + char buf[16]; + + if (read_smt_control(buf, sizeof(buf)) && !strncmp(buf, "on", 2)) + return true; + + return false; +} + void vm_create_irqchip(struct kvm_vm *vm);
static inline int __vm_create_guest_memfd(struct kvm_vm *vm, uint64_t size, diff --git a/tools/testing/selftests/kvm/x86/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86/hyperv_cpuid.c index 4e920705681a..1eb55d0b7297 100644 --- a/tools/testing/selftests/kvm/x86/hyperv_cpuid.c +++ b/tools/testing/selftests/kvm/x86/hyperv_cpuid.c @@ -22,25 +22,6 @@ static void guest_code(void) { }
-static bool smt_possible(void) -{ - char buf[16]; - FILE *f; - bool res = true; - - f = fopen("/sys/devices/system/cpu/smt/control", "r"); - if (f) { - if (fread(buf, sizeof(*buf), sizeof(buf), f) > 0) { - if (!strncmp(buf, "forceoff", 8) || - !strncmp(buf, "notsupported", 12)) - res = false; - } - fclose(f); - } - - return res; -} - static void test_hv_cpuid(struct kvm_vcpu *vcpu, bool evmcs_expected) { const bool has_irqchip = !vcpu || vcpu->vm->has_irqchip;
For SEV tests, assert() failures on VM type or fd do not provide sufficient error reporting. Replace assert() with TEST_ASSERT_EQ() to obtain more detailed information on the assertion condition failure, including the call stack.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- tools/testing/selftests/kvm/lib/x86/sev.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/x86/sev.c b/tools/testing/selftests/kvm/lib/x86/sev.c index e9535ee20b7f..60d7a03dc1c2 100644 --- a/tools/testing/selftests/kvm/lib/x86/sev.c +++ b/tools/testing/selftests/kvm/lib/x86/sev.c @@ -37,12 +37,12 @@ static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *regio void sev_vm_init(struct kvm_vm *vm) { if (vm->type == KVM_X86_DEFAULT_VM) { - assert(vm->arch.sev_fd == -1); + TEST_ASSERT_EQ(vm->arch.sev_fd, -1); vm->arch.sev_fd = open_sev_dev_path_or_exit(); vm_sev_ioctl(vm, KVM_SEV_INIT, NULL); } else { struct kvm_sev_init init = { 0 }; - assert(vm->type == KVM_X86_SEV_VM); + TEST_ASSERT_EQ(vm->type, KVM_X86_SEV_VM); vm_sev_ioctl(vm, KVM_SEV_INIT2, &init); } } @@ -50,12 +50,12 @@ void sev_vm_init(struct kvm_vm *vm) void sev_es_vm_init(struct kvm_vm *vm) { if (vm->type == KVM_X86_DEFAULT_VM) { - assert(vm->arch.sev_fd == -1); + TEST_ASSERT_EQ(vm->arch.sev_fd, -1); vm->arch.sev_fd = open_sev_dev_path_or_exit(); vm_sev_ioctl(vm, KVM_SEV_ES_INIT, NULL); } else { struct kvm_sev_init init = { 0 }; - assert(vm->type == KVM_X86_SEV_ES_VM); + TEST_ASSERT_EQ(vm->type, KVM_X86_SEV_ES_VM); vm_sev_ioctl(vm, KVM_SEV_INIT2, &init); } }
In preparation for SNP, declutter the vm type check by introducing a SEV-SNP VM type check as well as a transitive set of helper functions.
The SNP VM type is the subset of SEV-ES. Similarly, the SEV-ES and SNP types are subset of the SEV VM type check.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- tools/testing/selftests/kvm/include/x86/sev.h | 4 ++++ tools/testing/selftests/kvm/lib/x86/processor.c | 4 ++-- tools/testing/selftests/kvm/lib/x86/sev.c | 17 +++++++++++++++++ .../testing/selftests/kvm/x86/sev_smoke_test.c | 2 +- 4 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h index 3003dc837fb7..b112f7664534 100644 --- a/tools/testing/selftests/kvm/include/x86/sev.h +++ b/tools/testing/selftests/kvm/include/x86/sev.h @@ -27,6 +27,10 @@ enum sev_guest_state {
#define GHCB_MSR_TERM_REQ 0x100
+bool is_sev_vm(struct kvm_vm *vm); +bool is_sev_es_vm(struct kvm_vm *vm); +bool is_sev_snp_vm(struct kvm_vm *vm); + void sev_vm_launch(struct kvm_vm *vm, uint32_t policy); void sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement); void sev_vm_launch_finish(struct kvm_vm *vm); diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c index bd5a802fa7a5..a92dc1dad085 100644 --- a/tools/testing/selftests/kvm/lib/x86/processor.c +++ b/tools/testing/selftests/kvm/lib/x86/processor.c @@ -639,7 +639,7 @@ void kvm_arch_vm_post_create(struct kvm_vm *vm) sync_global_to_guest(vm, host_cpu_is_amd); sync_global_to_guest(vm, is_forced_emulation_enabled);
- if (vm->type == KVM_X86_SEV_VM || vm->type == KVM_X86_SEV_ES_VM) { + if (is_sev_vm(vm)) { struct kvm_sev_init init = { 0 };
vm_sev_ioctl(vm, KVM_SEV_INIT2, &init); @@ -1156,7 +1156,7 @@ void kvm_get_cpu_address_width(unsigned int *pa_bits, unsigned int *va_bits)
void kvm_init_vm_address_properties(struct kvm_vm *vm) { - if (vm->type == KVM_X86_SEV_VM || vm->type == KVM_X86_SEV_ES_VM) { + if (is_sev_vm(vm)) { vm->arch.sev_fd = open_sev_dev_path_or_exit(); vm->arch.c_bit = BIT_ULL(this_cpu_property(X86_PROPERTY_SEV_C_BIT)); vm->gpa_tag_mask = vm->arch.c_bit; diff --git a/tools/testing/selftests/kvm/lib/x86/sev.c b/tools/testing/selftests/kvm/lib/x86/sev.c index 60d7a03dc1c2..4587f2b6bc39 100644 --- a/tools/testing/selftests/kvm/lib/x86/sev.c +++ b/tools/testing/selftests/kvm/lib/x86/sev.c @@ -4,6 +4,23 @@
#include "sev.h"
+bool is_sev_snp_vm(struct kvm_vm *vm) +{ + return vm->type == KVM_X86_SNP_VM; +} + +/* A SNP VM is also a SEV-ES VM */ +bool is_sev_es_vm(struct kvm_vm *vm) +{ + return is_sev_snp_vm(vm) || vm->type == KVM_X86_SEV_ES_VM; +} + +/* A SEV-ES and SNP VM is also a SEV VM */ +bool is_sev_vm(struct kvm_vm *vm) +{ + return is_sev_es_vm(vm) || vm->type == KVM_X86_SEV_VM; +} + /* * sparsebit_next_clear() can return 0 if [x, 2**64-1] are all set, and the * -1 would then cause an underflow back to 2**64 - 1. This is expected and diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c index 6812b94bf5b6..a2de1e63c3cb 100644 --- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c +++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c @@ -123,7 +123,7 @@ static void test_sev(void *guest_code, uint64_t policy) for (;;) { vcpu_run(vcpu);
- if (policy & SEV_POLICY_ES) { + if (is_sev_es_vm(vm)) { TEST_ASSERT(vcpu->run->exit_reason == KVM_EXIT_SYSTEM_EVENT, "Wanted SYSTEM_EVENT, got %s", exit_reason_str(vcpu->run->exit_reason));
Extend the SEV library to include support for SNP ioctl() wrappers, which aid in launching and interacting with a SEV-SNP guest.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- arch/x86/include/uapi/asm/kvm.h | 1 + tools/arch/x86/include/uapi/asm/kvm.h | 1 + tools/testing/selftests/kvm/include/x86/sev.h | 33 ++++++++- tools/testing/selftests/kvm/lib/x86/sev.c | 68 +++++++++++++++++-- 4 files changed, 97 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index 9e75da97bce0..565e4d054627 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -841,6 +841,7 @@ struct kvm_sev_snp_launch_start { };
/* Kept in sync with firmware values for simplicity. */ +#define KVM_SEV_PAGE_TYPE_INVALID 0x0 #define KVM_SEV_SNP_PAGE_TYPE_NORMAL 0x1 #define KVM_SEV_SNP_PAGE_TYPE_ZERO 0x3 #define KVM_SEV_SNP_PAGE_TYPE_UNMEASURED 0x4 diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h index 88585c1de416..17e44fbdc2a7 100644 --- a/tools/arch/x86/include/uapi/asm/kvm.h +++ b/tools/arch/x86/include/uapi/asm/kvm.h @@ -841,6 +841,7 @@ struct kvm_sev_snp_launch_start { };
/* Kept in sync with firmware values for simplicity. */ +#define KVM_SEV_PAGE_TYPE_INVALID 0x0 #define KVM_SEV_SNP_PAGE_TYPE_NORMAL 0x1 #define KVM_SEV_SNP_PAGE_TYPE_ZERO 0x3 #define KVM_SEV_SNP_PAGE_TYPE_UNMEASURED 0x4 diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h index b112f7664534..c696d10f9332 100644 --- a/tools/testing/selftests/kvm/include/x86/sev.h +++ b/tools/testing/selftests/kvm/include/x86/sev.h @@ -25,6 +25,10 @@ enum sev_guest_state { #define SEV_POLICY_NO_DBG (1UL << 0) #define SEV_POLICY_ES (1UL << 2)
+#define SNP_POLICY_SMT (1ULL << 16) +#define SNP_POLICY_RSVD_MBO (1ULL << 17) +#define SNP_POLICY_DBG (1ULL << 19) + #define GHCB_MSR_TERM_REQ 0x100
bool is_sev_vm(struct kvm_vm *vm); @@ -34,13 +38,26 @@ bool is_sev_snp_vm(struct kvm_vm *vm); void sev_vm_launch(struct kvm_vm *vm, uint32_t policy); void sev_vm_launch_measure(struct kvm_vm *vm, uint8_t *measurement); void sev_vm_launch_finish(struct kvm_vm *vm); +void snp_vm_launch_start(struct kvm_vm *vm, uint64_t policy); +void snp_vm_launch_update(struct kvm_vm *vm); +void snp_vm_launch_finish(struct kvm_vm *vm);
struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t type, void *guest_code, struct kvm_vcpu **cpu); -void vm_sev_launch(struct kvm_vm *vm, uint32_t policy, uint8_t *measurement); +void vm_sev_launch(struct kvm_vm *vm, uint64_t policy, uint8_t *measurement);
kvm_static_assert(SEV_RET_SUCCESS == 0);
+/* + * A SEV-SNP VM requires the policy reserved bit to always be set. + * The SMT policy bit is also required to be set based on SMT being + * available and active on the system. + */ +static inline u64 snp_default_policy(void) +{ + return SNP_POLICY_RSVD_MBO | (smt_on() ? SNP_POLICY_SMT : 0); +} + /* * The KVM_MEMORY_ENCRYPT_OP uAPI is utter garbage and takes an "unsigned long" * instead of a proper struct. The size of the parameter is embedded in the @@ -74,6 +91,7 @@ kvm_static_assert(SEV_RET_SUCCESS == 0);
void sev_vm_init(struct kvm_vm *vm); void sev_es_vm_init(struct kvm_vm *vm); +void snp_vm_init(struct kvm_vm *vm);
static inline void vmgexit(void) { @@ -102,4 +120,17 @@ static inline void sev_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa, vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_DATA, &update_data); }
+static inline void snp_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa, + uint64_t hva, uint64_t size, uint8_t type) +{ + struct kvm_sev_snp_launch_update update_data = { + .uaddr = hva, + .gfn_start = gpa >> PAGE_SHIFT, + .len = size, + .type = type, + }; + + vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_UPDATE, &update_data); +} + #endif /* SELFTEST_KVM_SEV_H */ diff --git a/tools/testing/selftests/kvm/lib/x86/sev.c b/tools/testing/selftests/kvm/lib/x86/sev.c index 4587f2b6bc39..a56f5164b0a6 100644 --- a/tools/testing/selftests/kvm/lib/x86/sev.c +++ b/tools/testing/selftests/kvm/lib/x86/sev.c @@ -31,7 +31,8 @@ bool is_sev_vm(struct kvm_vm *vm) * and find the first range, but that's correct because the condition * expression would cause us to quit the loop. */ -static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *region) +static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *region, + uint8_t page_type, bool private) { const struct sparsebit *protected_phy_pages = region->protected_phy_pages; const vm_paddr_t gpa_base = region->region.guest_phys_addr; @@ -41,13 +42,23 @@ static void encrypt_region(struct kvm_vm *vm, struct userspace_mem_region *regio if (!sparsebit_any_set(protected_phy_pages)) return;
- sev_register_encrypted_memory(vm, region); + if (!is_sev_snp_vm(vm)) + sev_register_encrypted_memory(vm, region);
sparsebit_for_each_set_range(protected_phy_pages, i, j) { const uint64_t size = (j - i + 1) * vm->page_size; const uint64_t offset = (i - lowest_page_in_region) * vm->page_size;
- sev_launch_update_data(vm, gpa_base + offset, size); + if (private) + vm_mem_set_private(vm, gpa_base + offset, size); + + if (is_sev_snp_vm(vm)) + snp_launch_update_data(vm, gpa_base + offset, + (uint64_t)addr_gpa2hva(vm, gpa_base + offset), + size, page_type); + else + sev_launch_update_data(vm, gpa_base + offset, size); + } }
@@ -77,6 +88,14 @@ void sev_es_vm_init(struct kvm_vm *vm) } }
+void snp_vm_init(struct kvm_vm *vm) +{ + struct kvm_sev_init init = { 0 }; + + TEST_ASSERT_EQ(vm->type, KVM_X86_SNP_VM); + vm_sev_ioctl(vm, KVM_SEV_INIT2, &init); +} + void sev_vm_launch(struct kvm_vm *vm, uint32_t policy) { struct kvm_sev_launch_start launch_start = { @@ -93,7 +112,7 @@ void sev_vm_launch(struct kvm_vm *vm, uint32_t policy) TEST_ASSERT_EQ(status.state, SEV_GUEST_STATE_LAUNCH_UPDATE);
hash_for_each(vm->regions.slot_hash, ctr, region, slot_node) - encrypt_region(vm, region); + encrypt_region(vm, region, KVM_SEV_PAGE_TYPE_INVALID, false);
if (policy & SEV_POLICY_ES) vm_sev_ioctl(vm, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); @@ -129,6 +148,33 @@ void sev_vm_launch_finish(struct kvm_vm *vm) TEST_ASSERT_EQ(status.state, SEV_GUEST_STATE_RUNNING); }
+void snp_vm_launch_start(struct kvm_vm *vm, uint64_t policy) +{ + struct kvm_sev_snp_launch_start launch_start = { + .policy = policy, + }; + + vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_START, &launch_start); +} + +void snp_vm_launch_update(struct kvm_vm *vm) +{ + struct userspace_mem_region *region; + int ctr; + + hash_for_each(vm->regions.slot_hash, ctr, region, slot_node) + encrypt_region(vm, region, KVM_SEV_SNP_PAGE_TYPE_NORMAL, true); + + vm->arch.is_pt_protected = true; +} + +void snp_vm_launch_finish(struct kvm_vm *vm) +{ + struct kvm_sev_snp_launch_finish launch_finish = { 0 }; + + vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_FINISH, &launch_finish); +} + struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t type, void *guest_code, struct kvm_vcpu **cpu) { @@ -145,8 +191,20 @@ struct kvm_vm *vm_sev_create_with_one_vcpu(uint32_t type, void *guest_code, return vm; }
-void vm_sev_launch(struct kvm_vm *vm, uint32_t policy, uint8_t *measurement) +void vm_sev_launch(struct kvm_vm *vm, uint64_t policy, uint8_t *measurement) { + if (is_sev_snp_vm(vm)) { + vm_enable_cap(vm, KVM_CAP_EXIT_HYPERCALL, (1 << KVM_HC_MAP_GPA_RANGE)); + + snp_vm_launch_start(vm, policy); + + snp_vm_launch_update(vm); + + snp_vm_launch_finish(vm); + + return; + } + sev_vm_launch(vm, policy);
if (!measurement)
Force the SEV-SNP VM type to set the KVM_MEM_GUEST_MEMFD flag for the creation of private memslots.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- tools/testing/selftests/kvm/lib/kvm_util.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index 33fefeb3ca44..089488e2eaf6 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -413,14 +413,17 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus, nr_extra_pages); struct userspace_mem_region *slot0; struct kvm_vm *vm; - int i; + int i, flags = 0;
pr_debug("%s: mode='%s' type='%d', pages='%ld'\n", __func__, vm_guest_mode_string(shape.mode), shape.type, nr_pages);
vm = ____vm_create(shape);
- vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, 0); + if (shape.type == KVM_X86_SNP_VM) + flags |= KVM_MEM_GUEST_MEMFD; + + vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, flags); for (i = 0; i < NR_MEM_REGIONS; i++) vm->memslots[i] = 0;
In preparation for SNP, cleanup the smoke test to decouple deriving type from policy. This enables us to reuse existing interfaces as well as deduplicate the test calls that are called for SEV and SEV-ES.
No functional change intended.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- .../selftests/kvm/x86/sev_smoke_test.c | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c index a2de1e63c3cb..620aa7c41f7a 100644 --- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c +++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c @@ -61,7 +61,7 @@ static void compare_xsave(u8 *from_host, u8 *from_guest) abort(); }
-static void test_sync_vmsa(uint32_t policy) +static void test_sync_vmsa(uint32_t type, uint64_t policy) { struct kvm_vcpu *vcpu; struct kvm_vm *vm; @@ -71,7 +71,7 @@ static void test_sync_vmsa(uint32_t policy) double x87val = M_PI; struct kvm_xsave __attribute__((aligned(64))) xsave = { 0 };
- vm = vm_sev_create_with_one_vcpu(KVM_X86_SEV_ES_VM, guest_code_xsave, &vcpu); + vm = vm_sev_create_with_one_vcpu(type, guest_code_xsave, &vcpu); gva = vm_vaddr_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR, MEM_REGION_TEST_DATA); hva = addr_gva2hva(vm, gva); @@ -88,7 +88,7 @@ static void test_sync_vmsa(uint32_t policy) : "ymm4", "st", "st(1)", "st(2)", "st(3)", "st(4)", "st(5)", "st(6)", "st(7)"); vcpu_xsave_set(vcpu, &xsave);
- vm_sev_launch(vm, SEV_POLICY_ES | policy, NULL); + vm_sev_launch(vm, policy, NULL);
/* This page is shared, so make it decrypted. */ memset(hva, 0, 4096); @@ -107,14 +107,12 @@ static void test_sync_vmsa(uint32_t policy) kvm_vm_free(vm); }
-static void test_sev(void *guest_code, uint64_t policy) +static void test_sev(void *guest_code, uint32_t type, uint64_t policy) { struct kvm_vcpu *vcpu; struct kvm_vm *vm; struct ucall uc;
- uint32_t type = policy & SEV_POLICY_ES ? KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM; - vm = vm_sev_create_with_one_vcpu(type, guest_code, &vcpu);
/* TODO: Validate the measurement is as expected. */ @@ -160,16 +158,14 @@ static void guest_shutdown_code(void) __asm__ __volatile__("ud2"); }
-static void test_sev_es_shutdown(void) +static void test_sev_shutdown(uint32_t type, uint64_t policy) { struct kvm_vcpu *vcpu; struct kvm_vm *vm;
- uint32_t type = KVM_X86_SEV_ES_VM; - vm = vm_sev_create_with_one_vcpu(type, guest_shutdown_code, &vcpu);
- vm_sev_launch(vm, SEV_POLICY_ES, NULL); + vm_sev_launch(vm, policy, NULL);
vcpu_run(vcpu); TEST_ASSERT(vcpu->run->exit_reason == KVM_EXIT_SHUTDOWN, @@ -179,27 +175,33 @@ static void test_sev_es_shutdown(void) kvm_vm_free(vm); }
-int main(int argc, char *argv[]) +static void test_sev_smoke(void *guest, uint32_t type, uint64_t policy) { const u64 xf_mask = XFEATURE_MASK_X87_AVX;
- TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SEV)); - - test_sev(guest_sev_code, SEV_POLICY_NO_DBG); - test_sev(guest_sev_code, 0); + test_sev(guest, type, policy | SEV_POLICY_NO_DBG); + test_sev(guest, type, policy);
- if (kvm_cpu_has(X86_FEATURE_SEV_ES)) { - test_sev(guest_sev_es_code, SEV_POLICY_ES | SEV_POLICY_NO_DBG); - test_sev(guest_sev_es_code, SEV_POLICY_ES); + if (type == KVM_X86_SEV_VM) + return;
- test_sev_es_shutdown(); + test_sev_shutdown(type, policy);
- if (kvm_has_cap(KVM_CAP_XCRS) && - (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) { - test_sync_vmsa(0); - test_sync_vmsa(SEV_POLICY_NO_DBG); - } + if (kvm_has_cap(KVM_CAP_XCRS) && + (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) { + test_sync_vmsa(type, policy); + test_sync_vmsa(type, policy | SEV_POLICY_NO_DBG); } +} + +int main(int argc, char *argv[]) +{ + TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SEV)); + + test_sev_smoke(guest_sev_code, KVM_X86_SEV_VM, 0); + + if (kvm_cpu_has(X86_FEATURE_SEV_ES)) + test_sev_smoke(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
return 0; }
Extend sev_smoke_test to also run a minimal SEV-SNP smoke test that initializes and sets up private memory regions required to run a simple SEV-SNP guest.
Similar to its SEV-ES smoke test counterpart, this also does not support GHCB and ucall yet and uses the GHCB MSR protocol to trigger an exit of the type KVM_EXIT_SYSTEM_EVENT.
Signed-off-by: Pratik R. Sampat prsampat@amd.com --- .../selftests/kvm/x86/sev_smoke_test.c | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c index 620aa7c41f7a..0505cde77358 100644 --- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c +++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c @@ -16,6 +16,18 @@
#define XFEATURE_MASK_X87_AVX (XFEATURE_MASK_FP | XFEATURE_MASK_SSE | XFEATURE_MASK_YMM)
+static void guest_snp_code(void) +{ + uint64_t sev_msr = rdmsr(MSR_AMD64_SEV); + + GUEST_ASSERT(sev_msr & MSR_AMD64_SEV_ENABLED); + GUEST_ASSERT(sev_msr & MSR_AMD64_SEV_ES_ENABLED); + GUEST_ASSERT(sev_msr & MSR_AMD64_SEV_SNP_ENABLED); + + wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ); + vmgexit(); +} + static void guest_sev_es_code(void) { /* TODO: Check CPUID after GHCB-based hypercall support is added. */ @@ -179,7 +191,10 @@ static void test_sev_smoke(void *guest, uint32_t type, uint64_t policy) { const u64 xf_mask = XFEATURE_MASK_X87_AVX;
- test_sev(guest, type, policy | SEV_POLICY_NO_DBG); + if (type == KVM_X86_SNP_VM) + test_sev(guest, type, policy | SNP_POLICY_DBG); + else + test_sev(guest, type, policy | SEV_POLICY_NO_DBG); test_sev(guest, type, policy);
if (type == KVM_X86_SEV_VM) @@ -190,7 +205,10 @@ static void test_sev_smoke(void *guest, uint32_t type, uint64_t policy) if (kvm_has_cap(KVM_CAP_XCRS) && (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) { test_sync_vmsa(type, policy); - test_sync_vmsa(type, policy | SEV_POLICY_NO_DBG); + if (type == KVM_X86_SNP_VM) + test_sync_vmsa(type, policy | SNP_POLICY_DBG); + else + test_sync_vmsa(type, policy | SEV_POLICY_NO_DBG); } }
@@ -203,5 +221,8 @@ int main(int argc, char *argv[]) if (kvm_cpu_has(X86_FEATURE_SEV_ES)) test_sev_smoke(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
+ if (kvm_cpu_has(X86_FEATURE_SEV_SNP)) + test_sev_smoke(guest_snp_code, KVM_X86_SNP_VM, snp_default_policy()); + return 0; }
A very gentle ping on this series.
Thanks Pratik
On 3/5/25 4:59 PM, Pratik R. Sampat wrote:
This patch series extends the sev_init2 and the sev_smoke test to exercise the SEV-SNP VM launch workflow.
Primarily, it introduces the architectural defines, its support in the SEV library and extends the tests to interact with the SEV-SNP ioctl() wrappers.
Patch 1 - Do not advertise SNP on initialization failure Patch 2 - SNP test for KVM_SEV_INIT2 Patch 3 - Add vmgexit helper Patch 4 - Add SMT control interface helper Patch 5 - Replace assert() with TEST_ASSERT_EQ() Patch 6 - Introduce SEV+ VM type check Patch 7 - SNP iotcl() plumbing for the SEV library Patch 8 - Force set GUEST_MEMFD for SNP Patch 9 - Cleanups of smoke test - Decouple policy from type Patch 10 - SNP smoke test
The series is based on git.kernel.org/pub/scm/virt/kvm/kvm.git next
v7..v8:
- Dropped exporting the SNP initialized API from ccp to KVM. Instead call SNP_PLATFORM_STATUS within KVM to query the initialization. (Tom) While it may be cheaper to query sev->snp_initialized from ccp, making the SNP platform call within KVM does away with any dependencies.
v6..v7: https://lore.kernel.org/kvm/20250221210200.244405-7-prsampat@amd.com/ Based on comments from Sean -
- Replaced FW check with sev->snp_initialized
- Dropped the patch which removes SEV+ KVM advertisement if INIT fails. This should be now be resolved by the combination of the patches [1,2] from Ashish.
- Change vmgexit to an inline function
- Export SMT control parsing interface to kvm_util Note: hyperv_cpuid KST only compile tested
- Replace assert() with TEST_ASSERT_EQ() within SEV library
- Define KVM_SEV_PAGE_TYPE_INVALID for SEV call of encrypt_region()
- Parameterize encrypt_region() to include privatize_region()
- Deduplication of sev test calls between SEV,SEV-ES and SNP
- Removed FW version tests for SNP
- Included testing of SNP_POLICY_DBG
- Dropped most tags from patches that have been changed or indirectly affected
[1] https://lore.kernel.org/all/d6d08c6b-9602-4f3d-92c2-8db6d50a1b92@amd.com [2] https://lore.kernel.org/all/f78ddb64087df27e7bcb1ae0ab53f55aa0804fab.1739226...
v5..v6: https://lore.kernel.org/kvm/ab433246-e97c-495b-ab67-b0cb1721fb99@amd.com/
- Rename is_sev_platform_init to sev_fw_initialized (Nikunj)
- Rename KVM CPU feature X86_FEATURE_SNP to X86_FEATURE_SEV_SNP (Nikunj)
- Collected Tags from Nikunj, Pankaj, Srikanth.
v4..v5: https://lore.kernel.org/kvm/8e7d8172-879e-4a28-8438-343b1c386ec9@amd.com/
- Introduced a check to disable advertising support for SEV, SEV-ES and SNP when platform initialization fails (Nikunj)
- Remove the redundant SNP check within is_sev_vm() (Nikunj)
- Cleanup of the encrypt_region flow for better readability (Nikunj)
- Refactor paths to use the canonical $(ARCH) to rebase for kvm/next
v3..v4: https://lore.kernel.org/kvm/20241114234104.128532-1-pratikrajesh.sampat@amd....
- Remove SNP FW API version check in the test and ensure the KVM capability advertises the presence of the feature. Retain the minimum version definitions to exercise these API versions in the smoke test
- Retained only the SNP smoke test and SNP_INIT2 test
- The SNP architectural defined merged with SNP_INIT2 test patch
- SNP shutdown merged with SNP smoke test patch
- Add SEV VM type check to abstract comparisons and reduce clutter
- Define a SNP default policy which sets bits based on the presence of SMT
- Decouple privatization and encryption for it to be SNP agnostic
- Assert for only positive tests using vm_ioctl()
- Dropped tested-by tags
In summary - based on comments from Sean, I have primarily reduced the scope of this patch series to focus on breaking down the SNP smoke test patch (v3 - patch2) to first introduce SEV-SNP support and use this interface to extend the sev_init2 and the sev_smoke test.
The rest of the v3 patchset that introduces ioctl, pre fault, fallocate and negative tests, will be re-worked and re-introduced subsequently in future patch series post addressing the issues discussed.
v2..v3: https://lore.kernel.org/kvm/20240905124107.6954-1-pratikrajesh.sampat@amd.co...
- Remove the assignments for the prefault and fallocate test type enums.
- Fix error message for sev launch measure and finish.
- Collect tested-by tags [Peter, Srikanth]
Pratik R. Sampat (10): KVM: SEV: Disable SEV-SNP support on initialization failure KVM: selftests: SEV-SNP test for KVM_SEV_INIT2 KVM: selftests: Add vmgexit helper KVM: selftests: Add SMT control state helper KVM: selftests: Replace assert() with TEST_ASSERT_EQ() KVM: selftests: Introduce SEV VM type check KVM: selftests: Add library support for interacting with SNP KVM: selftests: Force GUEST_MEMFD flag for SNP VM type KVM: selftests: Abstractions for SEV to decouple policy from type KVM: selftests: Add a basic SEV-SNP smoke test
arch/x86/include/uapi/asm/kvm.h | 1 + arch/x86/kvm/svm/sev.c | 30 +++++- tools/arch/x86/include/uapi/asm/kvm.h | 1 + .../testing/selftests/kvm/include/kvm_util.h | 35 +++++++ .../selftests/kvm/include/x86/processor.h | 1 + tools/testing/selftests/kvm/include/x86/sev.h | 42 ++++++++- tools/testing/selftests/kvm/lib/kvm_util.c | 7 +- .../testing/selftests/kvm/lib/x86/processor.c | 4 +- tools/testing/selftests/kvm/lib/x86/sev.c | 93 +++++++++++++++++-- .../testing/selftests/kvm/x86/hyperv_cpuid.c | 19 ---- .../selftests/kvm/x86/sev_init2_tests.c | 13 +++ .../selftests/kvm/x86/sev_smoke_test.c | 75 +++++++++------ 12 files changed, 261 insertions(+), 60 deletions(-)
On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
This patch series extends the sev_init2 and the sev_smoke test to exercise the SEV-SNP VM launch workflow.
Primarily, it introduces the architectural defines, its support in the SEV library and extends the tests to interact with the SEV-SNP ioctl() wrappers.
[...]
Applied 2-9 to kvm-x86 selftests. AIUI, the KVM side of things should already be fixed. If KVM isn't fixed, I want to take that discussion/patch to a separate thread.
I made minor changes along the way (some details in the commits' []), please holler if you disagree with the end result.
[01/10] KVM: SEV: Disable SEV-SNP support on initialization failure (no commit info) [02/10] KVM: selftests: SEV-SNP test for KVM_SEV_INIT2 https://github.com/kvm-x86/linux/commit/68ed692e3954 [03/10] KVM: selftests: Add vmgexit helper https://github.com/kvm-x86/linux/commit/c4e1a848d721 [04/10] KVM: selftests: Add SMT control state helper https://github.com/kvm-x86/linux/commit/acf064345018 [05/10] KVM: selftests: Replace assert() with TEST_ASSERT_EQ() https://github.com/kvm-x86/linux/commit/f694f30e81c4 [06/10] KVM: selftests: Introduce SEV VM type check https://github.com/kvm-x86/linux/commit/4a4e1e8e92eb [07/10] KVM: selftests: Add library support for interacting with SNP https://github.com/kvm-x86/linux/commit/3bf3e0a52123 [08/10] KVM: selftests: Force GUEST_MEMFD flag for SNP VM type https://github.com/kvm-x86/linux/commit/b73a30cd9caa [09/10] KVM: selftests: Abstractions for SEV to decouple policy from type https://github.com/kvm-x86/linux/commit/a5d55f783fb7 [10/10] KVM: selftests: Add a basic SEV-SNP smoke test https://github.com/kvm-x86/linux/commit/ada014f5fc67
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.1742850... [2]: https://lore.kernel.org/kvm/d8de6de80c36721ea3eb92ecac81b211f401c3b2.1742850...
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
On Mon, May 05, 2025, Pratik R. Sampat wrote:
Hi Sean,
On 5/2/25 4:50 PM, Sean Christopherson wrote:
On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
This patch series extends the sev_init2 and the sev_smoke test to exercise the SEV-SNP VM launch workflow.
Primarily, it introduces the architectural defines, its support in the SEV library and extends the tests to interact with the SEV-SNP ioctl() wrappers.
[...]
Applied 2-9 to kvm-x86 selftests. AIUI, the KVM side of things should already be fixed. If KVM isn't fixed, I want to take that discussion/patch to a separate thread.
Thanks for pulling these patches in.
For 1 - Ashish's commit now returns failure for this case [1]. Although, it appears that the return code isn't checked within sev_platform_init()[2], so it shouldn't change existing behavior. In the kselftest case, if platform init fails, the selftest will also fail — just as it does currently too.
Argh, now I remember the issue. But _sev_platform_init_locked() returns '0' if psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked() will magically make it succeed the second time around.
So shouldn't the KVM code be this?
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index e0f446922a6e..dd04f979357d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void) sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP);
out: + if (sev_enabled) { + init_args.probe = true; + if (sev_platform_init(&init_args)) + sev_supported = sev_es_supported = sev_snp_supported = false; + else + sev_snp_supported &= sev_is_snp_initialized(); + } + if (boot_cpu_has(X86_FEATURE_SEV)) pr_info("SEV %s (ASIDs %u - %u)\n", sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" : @@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void)
if (!sev_enabled) return; - - /* - * Do both SNP and SEV initialization at KVM module load. - */ - init_args.probe = true; - sev_platform_init(&init_args); }
void sev_hardware_unsetup(void) --
Ashish, what am I missing?
Regardless of what we decide on what the right behavior is, fail vs skip (I don't mind the former) we can certainly do that over new patches rebased over the new series.
FAIL, for sure. Unless someone else pipes up with a good reason why they need to defer INIT_EX, that's Google's problem to solve.
Hello Sean,
On 5/5/2025 6:15 PM, Sean Christopherson wrote:
On Mon, May 05, 2025, Pratik R. Sampat wrote:
Hi Sean,
On 5/2/25 4:50 PM, Sean Christopherson wrote:
On Wed, 05 Mar 2025 16:59:50 -0600, Pratik R. Sampat wrote:
This patch series extends the sev_init2 and the sev_smoke test to exercise the SEV-SNP VM launch workflow.
Primarily, it introduces the architectural defines, its support in the SEV library and extends the tests to interact with the SEV-SNP ioctl() wrappers.
[...]
Applied 2-9 to kvm-x86 selftests. AIUI, the KVM side of things should already be fixed. If KVM isn't fixed, I want to take that discussion/patch to a separate thread.
Thanks for pulling these patches in.
For 1 - Ashish's commit now returns failure for this case [1]. Although, it appears that the return code isn't checked within sev_platform_init()[2], so it shouldn't change existing behavior. In the kselftest case, if platform init fails, the selftest will also fail — just as it does currently too.
Argh, now I remember the issue. But _sev_platform_init_locked() returns '0' if psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked() will magically make it succeed the second time around.
So shouldn't the KVM code be this?
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index e0f446922a6e..dd04f979357d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void) sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP); out:
if (sev_enabled) {init_args.probe = true;if (sev_platform_init(&init_args))sev_supported = sev_es_supported = sev_snp_supported = false;elsesev_snp_supported &= sev_is_snp_initialized();}if (boot_cpu_has(X86_FEATURE_SEV)) pr_info("SEV %s (ASIDs %u - %u)\n", sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" :@@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void) if (!sev_enabled) return;
/** Do both SNP and SEV initialization at KVM module load.*/init_args.probe = true;sev_platform_init(&init_args);} void sev_hardware_unsetup(void) --
Ashish, what am I missing?
As far as setting sev*_enabled is concerned, i believe they are more specific to SNP/SEV/SEV-ES being enabled in the system, which is separate from SEV_INIT/SNP_INIT (SNP_INIT success indicates that RMP been initialized, SNP has to be already enabled via MSR_SYSCFG before SNP_INIT is called), though SEV_INIT/SNP_INIT may fail but SEV/SNP support will still be enabled on the system.
Additionally as SEV_INIT/SNP_INIT during sev_platform_init() have failed, so any SEV/SEV-ES/SNP VM launch will fail as the firmware will return invalid platform state as INITs have failed.
From my understanding, sev*_enabled indicates the user support to enable/disable support for SEV/SEV-ES/SEV-SNP, as the sev*_enabled are the KVM module parameters, while sev*_supported indicates if platform has that support enabled.
And before the SEV/SNP init support was moved to KVM from CCP module, doing SEV/SNP INIT could fail but that still had KVM detecting SEV/SNP support enabled, so this moving SEV/SNP init stuff to KVM module from CCP driver is consistent with the previous behavior.
Thanks, Ashish
Regardless of what we decide on what the right behavior is, fail vs skip (I don't mind the former) we can certainly do that over new patches rebased over the new series.
FAIL, for sure. Unless someone else pipes up with a good reason why they need to defer INIT_EX, that's Google's problem to solve.
On Mon, May 05, 2025, Ashish Kalra wrote:
On 5/5/2025 6:15 PM, Sean Christopherson wrote:
@@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void) if (!sev_enabled) return;
/** Do both SNP and SEV initialization at KVM module load.*/init_args.probe = true;sev_platform_init(&init_args);} void sev_hardware_unsetup(void) --
Ashish, what am I missing?
As far as setting sev*_enabled is concerned, i believe they are more specific to SNP/SEV/SEV-ES being enabled in the system, which is separate from SEV_INIT/SNP_INIT (SNP_INIT success indicates that RMP been initialized, SNP has to be already enabled via MSR_SYSCFG before SNP_INIT is called), though SEV_INIT/SNP_INIT may fail but SEV/SNP support will still be enabled on the system.
No, if SNP_INIT fails and has zero chance of succeeding, then SNP is *NOT* supported *by KVM*. The platform may be configured to support SNP, but that matters not at all if KVM can't actually use the functionality.
Additionally as SEV_INIT/SNP_INIT during sev_platform_init() have failed, so any SEV/SEV-ES/SNP VM launch will fail as the firmware will return invalid platform state as INITs have failed.
Yeah, and that's *awful* behavior for KVM. Imagine if KVM did that for every feature, i.e. enumerated hardware support irrespective of KVM support.
The API is KVM_GET_SUPPORTED_CPUID, not KVM_GET_MOSTLY_SUPPORTED_CPUID.
From my understanding, sev*_enabled indicates the user support to enable/disable support for SEV/SEV-ES/SEV-SNP,
Yes, and they're also used to reflect and enumerate KVM support:
if (sev_enabled) { kvm_cpu_cap_set(X86_FEATURE_SEV); kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_VM); } if (sev_es_enabled) { kvm_cpu_cap_set(X86_FEATURE_SEV_ES); kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_ES_VM); } if (sev_snp_enabled) { kvm_cpu_cap_set(X86_FEATURE_SEV_SNP); kvm_caps.supported_vm_types |= BIT(KVM_X86_SNP_VM); }
as the sev*_enabled are the KVM module parameters, while sev*_supported indicates if platform has that support enabled.
sev*_supported are completely irrelevant. They are function local scratch variables that exist so that KVM doesn't clobber userspace's inputs while computing what is fully supported and enabled.
And before the SEV/SNP init support was moved to KVM from CCP module, doing SEV/SNP INIT could fail but that still had KVM detecting SEV/SNP support enabled, so this moving SEV/SNP init stuff to KVM module from CCP driver is consistent with the previous behavior.
And one of my driving motivations for getting the initialization into KVM was to fix that previous behavior.
Hello Sean,
On 5/5/2025 7:56 PM, Sean Christopherson wrote:
On Mon, May 05, 2025, Ashish Kalra wrote:
On 5/5/2025 6:15 PM, Sean Christopherson wrote:
@@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void) if (!sev_enabled) return;
/** Do both SNP and SEV initialization at KVM module load.*/init_args.probe = true;sev_platform_init(&init_args);} void sev_hardware_unsetup(void) --
Ashish, what am I missing?
As far as setting sev*_enabled is concerned, i believe they are more specific to SNP/SEV/SEV-ES being enabled in the system, which is separate from SEV_INIT/SNP_INIT (SNP_INIT success indicates that RMP been initialized, SNP has to be already enabled via MSR_SYSCFG before SNP_INIT is called), though SEV_INIT/SNP_INIT may fail but SEV/SNP support will still be enabled on the system.
No, if SNP_INIT fails and has zero chance of succeeding, then SNP is *NOT* supported *by KVM*. The platform may be configured to support SNP, but that matters not at all if KVM can't actually use the functionality.
Yes, i agree.
Additionally as SEV_INIT/SNP_INIT during sev_platform_init() have failed, so any SEV/SEV-ES/SNP VM launch will fail as the firmware will return invalid platform state as INITs have failed.
Yeah, and that's *awful* behavior for KVM. Imagine if KVM did that for every feature, i.e. enumerated hardware support irrespective of KVM support.
The API is KVM_GET_SUPPORTED_CPUID, not KVM_GET_MOSTLY_SUPPORTED_CPUID.
From my understanding, sev*_enabled indicates the user support to enable/disable support for SEV/SEV-ES/SEV-SNP,
Yes, and they're also used to reflect and enumerate KVM support:
if (sev_enabled) { kvm_cpu_cap_set(X86_FEATURE_SEV); kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_VM); } if (sev_es_enabled) { kvm_cpu_cap_set(X86_FEATURE_SEV_ES); kvm_caps.supported_vm_types |= BIT(KVM_X86_SEV_ES_VM); } if (sev_snp_enabled) { kvm_cpu_cap_set(X86_FEATURE_SEV_SNP); kvm_caps.supported_vm_types |= BIT(KVM_X86_SNP_VM); }
as the sev*_enabled are the KVM module parameters, while sev*_supported indicates if platform has that support enabled.
sev*_supported are completely irrelevant. They are function local scratch variables that exist so that KVM doesn't clobber userspace's inputs while computing what is fully supported and enabled.
Ok.
And before the SEV/SNP init support was moved to KVM from CCP module, doing SEV/SNP INIT could fail but that still had KVM detecting SEV/SNP support enabled, so this moving SEV/SNP init stuff to KVM module from CCP driver is consistent with the previous behavior.
And one of my driving motivations for getting the initialization into KVM was to fix that previous behavior.
Sure, i will test your patch and post it.
Thanks, Ashish
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;elsesev_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
On Mon, May 05, 2025, Pratik R. Sampat wrote:
On 5/5/2025 6:15 PM, Sean Christopherson wrote:
On Mon, May 05, 2025, Pratik R. Sampat wrote: Argh, now I remember the issue. But _sev_platform_init_locked() returns '0' if psp_init_on_probe is true, and I don't see how deferring __sev_snp_init_locked() will magically make it succeed the second time around.
So shouldn't the KVM code be this?
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index e0f446922a6e..dd04f979357d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -3038,6 +3038,14 @@ void __init sev_hardware_setup(void) sev_snp_supported = sev_snp_enabled && cc_platform_has(CC_ATTR_HOST_SEV_SNP); out:
if (sev_enabled) {init_args.probe = true;if (sev_platform_init(&init_args))sev_supported = sev_es_supported = sev_snp_supported = false;elsesev_snp_supported &= sev_is_snp_initialized();}if (boot_cpu_has(X86_FEATURE_SEV)) pr_info("SEV %s (ASIDs %u - %u)\n", sev_supported ? min_sev_asid <= max_sev_asid ? "enabled" :@@ -3067,12 +3075,6 @@ void __init sev_hardware_setup(void) if (!sev_enabled) return;
/** Do both SNP and SEV initialization at KVM module load.*/init_args.probe = true;sev_platform_init(&init_args);} void sev_hardware_unsetup(void) --
I agree with this approach. One thing maybe to consider further is to also call into SEV_platform_status() to check for init so that SEV/SEV-ES is not penalized and disabled for SNP's failures. Another approach could be to break up the SEV and SNP init setup so that we can spare a couple of platform calls in the process?
Nah, SNP initialization failure should be a rare occurence, I don't want to make the "normal" flow more complex just to handle something that should never happen in practice.
linux-kselftest-mirror@lists.linaro.org
-
Gupta, Pankaj -
Kalra, Ashish -
Pratik R. Sampat -
Sean Christopherson