Signed-off-by: Paolo Bonzini <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/include/kvm_util_base.h | 6 +-
.../selftests/kvm/set_memory_region_test.c | 8 +-
.../selftests/kvm/x86_64/sev_init2_tests.c | 146 ++++++++++++++++++
4 files changed, 153 insertions(+), 8 deletions(-)
create mode 100644 tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 492e937fab00..a81a89b1dc2a 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -116,6 +116,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/tsc_msrs_test
TEST_GEN_PROGS_x86_64 += x86_64/vmx_pmu_caps_test
TEST_GEN_PROGS_x86_64 += x86_64/xen_shinfo_test
TEST_GEN_PROGS_x86_64 += x86_64/xen_vmcall_test
+TEST_GEN_PROGS_x86_64 += x86_64/sev_init2_tests
TEST_GEN_PROGS_x86_64 += x86_64/sev_migrate_tests
TEST_GEN_PROGS_x86_64 += x86_64/amx_test
TEST_GEN_PROGS_x86_64 += x86_64/max_vcpuid_cap_test
diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 9e5afc472c14..44b6ea56a205 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -846,17 +846,15 @@ static inline struct kvm_vm *vm_create_barebones(void)
return ____vm_create(VM_SHAPE_DEFAULT);
}
-#ifdef __x86_64__
-static inline struct kvm_vm *vm_create_barebones_protected_vm(void)
+static inline struct kvm_vm *vm_create_barebones_type(unsigned long type)
{
const struct vm_shape shape = {
.mode = VM_MODE_DEFAULT,
- .type = KVM_X86_SW_PROTECTED_VM,
+ .type = type,
};
return ____vm_create(shape);
}
-#endif
static inline struct kvm_vm *vm_create(uint32_t nr_runnable_vcpus)
{
diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c
index 075b80dbe237..0ac6c21d7251 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -339,7 +339,7 @@ static void test_invalid_memory_region_flags(void)
#ifdef __x86_64__
if (kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM))
- vm = vm_create_barebones_protected_vm();
+ vm = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM);
else
#endif
vm = vm_create_barebones();
@@ -452,7 +452,7 @@ static void test_add_private_memory_region(void)
pr_info("Testing ADD of KVM_MEM_GUEST_MEMFD memory regions\n");
- vm = vm_create_barebones_protected_vm();
+ vm = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM);
test_invalid_guest_memfd(vm, vm->kvm_fd, 0, "KVM fd should fail");
test_invalid_guest_memfd(vm, vm->fd, 0, "VM's fd should fail");
@@ -461,7 +461,7 @@ static void test_add_private_memory_region(void)
test_invalid_guest_memfd(vm, memfd, 0, "Regular memfd() should fail");
close(memfd);
- vm2 = vm_create_barebones_protected_vm();
+ vm2 = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM);
memfd = vm_create_guest_memfd(vm2, MEM_REGION_SIZE, 0);
test_invalid_guest_memfd(vm, memfd, 0, "Other VM's guest_memfd() should fail");
@@ -489,7 +489,7 @@ static void test_add_overlapping_private_memory_regions(void)
pr_info("Testing ADD of overlapping KVM_MEM_GUEST_MEMFD memory regions\n");
- vm = vm_create_barebones_protected_vm();
+ vm = vm_create_barebones_type(KVM_X86_SW_PROTECTED_VM);
memfd = vm_create_guest_memfd(vm, MEM_REGION_SIZE * 4, 0);
diff --git a/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c b/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
new file mode 100644
index 000000000000..644fd5757041
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kvm.h>
+#include <linux/psp-sev.h>
+#include <stdio.h>
+#include <sys/ioctl.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <pthread.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "svm_util.h"
+#include "kselftest.h"
+
+#define SVM_SEV_FEAT_DEBUG_SWAP 32u
+
+/*
+ * Some features may have hidden dependencies, or may only work
+ * for certain VM types. Err on the side of safety and don't
+ * expect that all supported features can be passed one by one
+ * to KVM_SEV_INIT2.
+ *
+ * (Well, right now there's only one...)
+ */
+#define KNOWN_FEATURES SVM_SEV_FEAT_DEBUG_SWAP
+
+int kvm_fd;
+u64 supported_vmsa_features;
+bool have_sev_es;
+
+static int __sev_ioctl(int vm_fd, int cmd_id, void *data)
+{
+ struct kvm_sev_cmd cmd = {
+ .id = cmd_id,
+ .data = (uint64_t)data,
+ .sev_fd = open_sev_dev_path_or_exit(),
+ };
+ int ret;
+
+ ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd);
+ TEST_ASSERT(ret < 0 || cmd.error == SEV_RET_SUCCESS,
+ "%d failed: fw error: %d\n",
+ cmd_id, cmd.error);
+
+ return ret;
+}
+
+static void test_init2(unsigned long vm_type, struct kvm_sev_init *init)
+{
+ struct kvm_vm *vm;
+ int ret;
+
+ vm = vm_create_barebones_type(vm_type);
+ ret = __sev_ioctl(vm->fd, KVM_SEV_INIT2, init);
+ TEST_ASSERT(ret == 0,
+ "KVM_SEV_INIT2 return code is %d (expected 0), errno: %d",
+ ret, errno);
+ kvm_vm_free(vm);
+}
+
+static void test_init2_invalid(unsigned long vm_type, struct kvm_sev_init *init)
+{
+ struct kvm_vm *vm;
+ int ret;
+
+ vm = vm_create_barebones_type(vm_type);
+ ret = __sev_ioctl(vm->fd, KVM_SEV_INIT2, init);
+ TEST_ASSERT(ret == -1 && errno == EINVAL,
+ "KVM_SEV_INIT2 return code %d, errno: %d (expected EINVAL)",
+ ret, errno);
+ kvm_vm_free(vm);
+}
+
+void test_vm_types(void)
+{
+ test_init2(KVM_X86_SEV_VM, &(struct kvm_sev_init){});
+
+ if (have_sev_es)
+ test_init2(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
+ else
+ test_init2_invalid(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
+
+ test_init2_invalid(0, &(struct kvm_sev_init){});
+ if (kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM))
+ test_init2_invalid(KVM_X86_SW_PROTECTED_VM, &(struct kvm_sev_init){});
+}
+
+void test_flags(uint32_t vm_type)
+{
+ int i;
+
+ for (i = 0; i < 32; i++)
+ test_init2_invalid(vm_type, &(struct kvm_sev_init){
+ .flags = 1u << i,
+ });
+}
+
+void test_features(uint32_t vm_type, uint64_t supported_features)
+{
+ int i;
+
+ for (i = 0; i < 64; i++) {
+ if (!(supported_features & (1u << i)))
+ test_init2_invalid(vm_type, &(struct kvm_sev_init){
+ .vmsa_features = 1u << i,
+ });
+ else if (KNOWN_FEATURES & (1u << i))
+ test_init2(vm_type, &(struct kvm_sev_init){
+ .vmsa_features = 1u << i,
+ });
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ int kvm_fd = open_kvm_dev_path_or_exit();
+ bool have_sev;
+
+ TEST_REQUIRE(__kvm_has_device_attr(kvm_fd, 0, KVM_X86_SEV_VMSA_FEATURES) == 0);
+ kvm_device_attr_get(kvm_fd, 0, KVM_X86_SEV_VMSA_FEATURES, &supported_vmsa_features);
+
+ have_sev = kvm_cpu_has(X86_FEATURE_SEV);
+ TEST_ASSERT(have_sev == !!(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SEV_VM)),
+ "sev: KVM_CAP_VM_TYPES (%x) does not match cpuid (checking %x)",
+ kvm_check_cap(KVM_CAP_VM_TYPES), 1 << KVM_X86_SEV_VM);
+
+ TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SEV_VM));
+ have_sev_es = kvm_cpu_has(X86_FEATURE_SEV_ES);
+
+ TEST_ASSERT(have_sev_es == !!(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SEV_ES_VM)),
+ "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);
+
+ test_vm_types();
+
+ test_flags(KVM_X86_SEV_VM);
+ if (have_sev_es)
+ test_flags(KVM_X86_SEV_ES_VM);
+
+ test_features(KVM_X86_SEV_VM, 0);
+ if (have_sev_es)
+ test_features(KVM_X86_SEV_ES_VM, supported_vmsa_features);
+
+ return 0;
+}
--
2.39.1
On Fri, Feb 23, 2024, Paolo Bonzini wrote:
> diff --git a/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c b/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
> new file mode 100644
> index 000000000000..644fd5757041
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/kvm.h>
> +#include <linux/psp-sev.h>
> +#include <stdio.h>
> +#include <sys/ioctl.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <pthread.h>
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "svm_util.h"
> +#include "kselftest.h"
> +
> +#define SVM_SEV_FEAT_DEBUG_SWAP 32u
> +
> +/*
> + * Some features may have hidden dependencies, or may only work
> + * for certain VM types. Err on the side of safety and don't
> + * expect that all supported features can be passed one by one
> + * to KVM_SEV_INIT2.
> + *
> + * (Well, right now there's only one...)
> + */
> +#define KNOWN_FEATURES SVM_SEV_FEAT_DEBUG_SWAP
> +
> +int kvm_fd;
> +u64 supported_vmsa_features;
> +bool have_sev_es;
> +
> +static int __sev_ioctl(int vm_fd, int cmd_id, void *data)
> +{
> + struct kvm_sev_cmd cmd = {
> + .id = cmd_id,
> + .data = (uint64_t)data,
> + .sev_fd = open_sev_dev_path_or_exit(),
> + };
> + int ret;
> +
> + ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd);
> + TEST_ASSERT(ret < 0 || cmd.error == SEV_RET_SUCCESS,
> + "%d failed: fw error: %d\n",
> + cmd_id, cmd.error);
> +
> + return ret;
If you can hold off on v3 until next week, I'll get the SEV+SEV-ES smoke test
series into a branch and thus kvm-x86/next. Then this can take advantage of the
library files and functions that are added there. I don't know if it will save
much code, but it'll at least provide a better place to land some of the "library"
#define and helpers.
https://lore.kernel.org/all/[email protected]
> +}
> +
> +static void test_init2(unsigned long vm_type, struct kvm_sev_init *init)
> +{
> + struct kvm_vm *vm;
> + int ret;
> +
> + vm = vm_create_barebones_type(vm_type);
> + ret = __sev_ioctl(vm->fd, KVM_SEV_INIT2, init);
The SEV library provided vm_sev_ioctl() for this.
> + TEST_ASSERT(ret == 0,
> + "KVM_SEV_INIT2 return code is %d (expected 0), errno: %d",
> + ret, errno);
TEST
> + kvm_vm_free(vm);
> +}
> +
> +static void test_init2_invalid(unsigned long vm_type, struct kvm_sev_init *init)
> +{
> + struct kvm_vm *vm;
> + int ret;
> +
> + vm = vm_create_barebones_type(vm_type);
> + ret = __sev_ioctl(vm->fd, KVM_SEV_INIT2, init);
__vm_sev_ioctl() in the library.
> + TEST_ASSERT(ret == -1 && errno == EINVAL,
> + "KVM_SEV_INIT2 return code %d, errno: %d (expected EINVAL)",
> + ret, errno);
TEST_ASSERT() will spit out the errno and it's user-friendly name. I would prefer
the assert message to explain why failure was expected. That way readers of the
code don't need a comment, and runners of failed tests get more info.
Hrm, though that'd require assing in a "const char *msg", which would limit this
to constant strings and no formatting. I think that's still a net positive though.
TEST_ASSERT(ret == -1 && errno == EINVAL,
"KVM_SET_INIT2 should fail, %s.", msg);
> + kvm_vm_free(vm);
> +}
> +
> +void test_vm_types(void)
> +{
> + test_init2(KVM_X86_SEV_VM, &(struct kvm_sev_init){});
> +
> + if (have_sev_es)
> + test_init2(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
> + else
> + test_init2_invalid(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
E.g. this could be something like
test_init2_invalid(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){},
"SEV-ES unsupported);
Though shouldn't vm_create_barebones_type() fail on the unsupported VM type, not
KVM_SEV_INIT2?
> +
> + test_init2_invalid(0, &(struct kvm_sev_init){});
> + if (kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM))
> + test_init2_invalid(KVM_X86_SW_PROTECTED_VM, &(struct kvm_sev_init){});
> +}
> +
> +void test_flags(uint32_t vm_type)
> +{
> + int i;
> +
> + for (i = 0; i < 32; i++)
> + test_init2_invalid(vm_type, &(struct kvm_sev_init){
> + .flags = 1u << i,
BIT()
> + });
And I think I'd prefer to have the line run long?
test_init2_invalid(vm_type, &(struct kvm_sev_init) { .flags = BIT(i) });
> +}
> +
> +void test_features(uint32_t vm_type, uint64_t supported_features)
> +{
> + int i;
> +
> + for (i = 0; i < 64; i++) {
> + if (!(supported_features & (1u << i)))
> + test_init2_invalid(vm_type, &(struct kvm_sev_init){
> + .vmsa_features = 1u << i,
> + });
Rather than create &(struct kvm_sev_init) for each path, this?
struct kvm_sev_init init = {
.vmsa_features = BIT(i);
};
if (!(supported_features & BIT(i))
test_init2_invalid(vm_type, &init);
else if (KNOWN_FEATURES & (1u << i))
test_init2(vm_type, &init);
> + else if (KNOWN_FEATURES & (1u << i))
> + test_init2(vm_type, &(struct kvm_sev_init){
> + .vmsa_features = 1u << i,
> + });
> + }
> +}
On Fri, Feb 23, 2024 at 6:18 PM Sean Christopherson <[email protected]> wrote:
>
> On Fri, Feb 23, 2024, Paolo Bonzini wrote:
> > diff --git a/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c b/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
> > new file mode 100644
> > index 000000000000..644fd5757041
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/x86_64/sev_init2_tests.c
> > @@ -0,0 +1,146 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <linux/kvm.h>
> > +#include <linux/psp-sev.h>
> > +#include <stdio.h>
> > +#include <sys/ioctl.h>
> > +#include <stdlib.h>
> > +#include <errno.h>
> > +#include <pthread.h>
> > +
> > +#include "test_util.h"
> > +#include "kvm_util.h"
> > +#include "processor.h"
> > +#include "svm_util.h"
> > +#include "kselftest.h"
> > +
> > +#define SVM_SEV_FEAT_DEBUG_SWAP 32u
> > +
> > +/*
> > + * Some features may have hidden dependencies, or may only work
> > + * for certain VM types. Err on the side of safety and don't
> > + * expect that all supported features can be passed one by one
> > + * to KVM_SEV_INIT2.
> > + *
> > + * (Well, right now there's only one...)
> > + */
> > +#define KNOWN_FEATURES SVM_SEV_FEAT_DEBUG_SWAP
> > +
> > +int kvm_fd;
> > +u64 supported_vmsa_features;
> > +bool have_sev_es;
> > +
> > +static int __sev_ioctl(int vm_fd, int cmd_id, void *data)
> > +{
> > + struct kvm_sev_cmd cmd = {
> > + .id = cmd_id,
> > + .data = (uint64_t)data,
> > + .sev_fd = open_sev_dev_path_or_exit(),
> > + };
> > + int ret;
> > +
> > + ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd);
> > + TEST_ASSERT(ret < 0 || cmd.error == SEV_RET_SUCCESS,
> > + "%d failed: fw error: %d\n",
> > + cmd_id, cmd.error);
> > +
> > + return ret;
>
> If you can hold off on v3 until next week, I'll get the SEV+SEV-ES smoke test
> series into a branch and thus kvm-x86/next. Then this can take advantage of the
> library files and functions that are added there. I don't know if it will save
> much code, but it'll at least provide a better place to land some of the "library"
> #define and helpers.
>
> https://lore.kernel.org/all/[email protected]
I'll post v3 anyway, but hold on actually committing this until I've
taken a closer look at kvm-x86/next.
> > + TEST_ASSERT(ret == -1 && errno == EINVAL,
> > + "KVM_SEV_INIT2 return code %d, errno: %d (expected EINVAL)",
> > + ret, errno);
>
> TEST_ASSERT() will spit out the errno and it's user-friendly name. I would prefer
> the assert message to explain why failure was expected. That way readers of the
> code don't need a comment, and runners of failed tests get more info.
>
> Hrm, though that'd require assing in a "const char *msg", which would limit this
> to constant strings and no formatting. I think that's still a net positive though.
Ok, will do.
> TEST_ASSERT(ret == -1 && errno == EINVAL,
> "KVM_SET_INIT2 should fail, %s.", msg);
>
> > + kvm_vm_free(vm);
> > +}
> > +
> > +void test_vm_types(void)
> > +{
> > + test_init2(KVM_X86_SEV_VM, &(struct kvm_sev_init){});
> > +
> > + if (have_sev_es)
> > + test_init2(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
> > + else
> > + test_init2_invalid(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){});
>
> E.g. this could be something like
>
> test_init2_invalid(KVM_X86_SEV_ES_VM, &(struct kvm_sev_init){},
> "SEV-ES unsupported);
>
> Though shouldn't vm_create_barebones_type() fail on the unsupported VM type, not
> KVM_SEV_INIT2?
Yes, this test is broken. vm_create_barebones_type() errors out.
Paolo