2023-01-10 18:11:28

by Peter Gonda

[permalink] [raw]
Subject: [PATCH V6 7/7] KVM: selftests: Add simple sev vm testing

A very simple of booting SEV guests that checks related CPUID bits. This
is a stripped down version of "[PATCH v2 08/13] KVM: selftests: add SEV
boot tests" from Michael but much simpler.

Cc: Paolo Bonzini <[email protected]>
Cc: Sean Christopherson <[email protected]>
Cc: Vishal Annapurve <[email protected]>
Cc: Ackerley Tng <[email protected]>
cc: Andrew Jones <[email protected]>
Suggested-by: Michael Roth <[email protected]>
Signed-off-by: Peter Gonda <[email protected]>
---
tools/testing/selftests/kvm/.gitignore | 84 +++++++++++++++++++
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/x86_64/sev_all_boot_test.c | 84 +++++++++++++++++++
3 files changed, 169 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 6d9381d60172..6d826957c6ae 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -1,7 +1,91 @@
# SPDX-License-Identifier: GPL-2.0-only
+<<<<<<< HEAD
*
!/**/
!*.c
!*.h
!*.S
!*.sh
+=======
+/aarch64/aarch32_id_regs
+/aarch64/arch_timer
+/aarch64/debug-exceptions
+/aarch64/get-reg-list
+/aarch64/hypercalls
+/aarch64/psci_test
+/aarch64/vcpu_width_config
+/aarch64/vgic_init
+/aarch64/vgic_irq
+/s390x/memop
+/s390x/resets
+/s390x/sync_regs_test
+/s390x/tprot
+/x86_64/amx_test
+/x86_64/cpuid_test
+/x86_64/cr4_cpuid_sync_test
+/x86_64/debug_regs
+/x86_64/evmcs_test
+/x86_64/emulator_error_test
+/x86_64/fix_hypercall_test
+/x86_64/get_msr_index_features
+/x86_64/kvm_clock_test
+/x86_64/kvm_pv_test
+/x86_64/hyperv_clock
+/x86_64/hyperv_cpuid
+/x86_64/hyperv_features
+/x86_64/hyperv_svm_test
+/x86_64/max_vcpuid_cap_test
+/x86_64/mmio_warning_test
+/x86_64/monitor_mwait_test
+/x86_64/nested_exceptions_test
+/x86_64/nx_huge_pages_test
+/x86_64/platform_info_test
+/x86_64/pmu_event_filter_test
+/x86_64/set_boot_cpu_id
+/x86_64/set_sregs_test
+/x86_64/sev_all_boot_test
+/x86_64/sev_migrate_tests
+/x86_64/smm_test
+/x86_64/state_test
+/x86_64/svm_vmcall_test
+/x86_64/svm_int_ctl_test
+/x86_64/svm_nested_soft_inject_test
+/x86_64/sync_regs_test
+/x86_64/tsc_msrs_test
+/x86_64/tsc_scaling_sync
+/x86_64/ucna_injection_test
+/x86_64/userspace_io_test
+/x86_64/userspace_msr_exit_test
+/x86_64/vmx_apic_access_test
+/x86_64/vmx_close_while_nested_test
+/x86_64/vmx_dirty_log_test
+/x86_64/vmx_exception_with_invalid_guest_state
+/x86_64/vmx_invalid_nested_guest_state
+/x86_64/vmx_msrs_test
+/x86_64/vmx_preemption_timer_test
+/x86_64/vmx_set_nested_state_test
+/x86_64/vmx_tsc_adjust_test
+/x86_64/vmx_nested_tsc_scaling_test
+/x86_64/xapic_ipi_test
+/x86_64/xapic_state_test
+/x86_64/xen_shinfo_test
+/x86_64/xen_vmcall_test
+/x86_64/xss_msr_test
+/x86_64/vmx_pmu_caps_test
+/x86_64/triple_fault_event_test
+/access_tracking_perf_test
+/demand_paging_test
+/dirty_log_test
+/dirty_log_perf_test
+/hardware_disable_test
+/kvm_create_max_vcpus
+/kvm_page_table_test
+/max_guest_memory_test
+/memslot_modification_stress_test
+/memslot_perf_test
+/rseq_test
+/set_memory_region_test
+/steal_time
+/kvm_binary_stats_test
+/system_counter_offset_test
+>>>>>>> KVM: selftests: Add simple sev vm testing
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index b7cfb15712d1..66d7ab3da990 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -111,6 +111,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_all_boot_test
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/x86_64/sev_all_boot_test.c b/tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c
new file mode 100644
index 000000000000..e9e4d7305bc1
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Basic SEV boot tests.
+ *
+ */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "svm_util.h"
+#include "linux/psp-sev.h"
+#include "sev.h"
+
+#define NR_SYNCS 1
+
+#define MSR_AMD64_SEV_BIT 1
+
+static void guest_run_loop(struct kvm_vcpu *vcpu)
+{
+ struct ucall uc;
+ int i;
+
+ for (i = 0; i <= NR_SYNCS; ++i) {
+ vcpu_run(vcpu);
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_SYNC:
+ continue;
+ case UCALL_DONE:
+ return;
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(uc);
+ default:
+ TEST_FAIL("Unexpected exit: %s",
+ exit_reason_str(vcpu->run->exit_reason));
+ }
+ }
+}
+
+static void is_sev_enabled(void)
+{
+ uint64_t sev_status;
+
+ GUEST_ASSERT(this_cpu_has(X86_FEATURE_SEV));
+
+ sev_status = rdmsr(MSR_AMD64_SEV);
+ GUEST_ASSERT(sev_status & 0x1);
+}
+
+static void guest_sev_code(void)
+{
+ GUEST_SYNC(1);
+
+ is_sev_enabled();
+
+ GUEST_DONE();
+}
+
+static void test_sev(void *guest_code, uint64_t policy)
+{
+ struct kvm_vm *vm;
+ struct kvm_vcpu *vcpu;
+
+ vm = vm_sev_create_with_one_vcpu(policy, guest_code, &vcpu);
+ TEST_ASSERT(vm, "vm_sev_create_with_one_vcpu() failed to create VM\n");
+
+ guest_run_loop(vcpu);
+
+ kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+ TEST_REQUIRE(is_kvm_sev_supported());
+
+ test_sev(guest_sev_code, SEV_POLICY_NO_DBG);
+ test_sev(guest_sev_code, 0);
+
+ return 0;
+}
--
2.39.0.314.g84b9a713c41-goog


2023-03-24 18:26:16

by Sean Christopherson

[permalink] [raw]
Subject: Re: [PATCH V6 7/7] KVM: selftests: Add simple sev vm testing

On Tue, Jan 10, 2023, Peter Gonda wrote:
> A very simple of booting SEV guests that checks related CPUID bits. This
> is a stripped down version of "[PATCH v2 08/13] KVM: selftests: add SEV
> boot tests" from Michael but much simpler.
>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Sean Christopherson <[email protected]>
> Cc: Vishal Annapurve <[email protected]>
> Cc: Ackerley Tng <[email protected]>
> cc: Andrew Jones <[email protected]>
> Suggested-by: Michael Roth <[email protected]>
> Signed-off-by: Peter Gonda <[email protected]>
> ---
> tools/testing/selftests/kvm/.gitignore | 84 +++++++++++++++++++
> tools/testing/selftests/kvm/Makefile | 1 +
> .../selftests/kvm/x86_64/sev_all_boot_test.c | 84 +++++++++++++++++++
> 3 files changed, 169 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 6d9381d60172..6d826957c6ae 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -1,7 +1,91 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +<<<<<<< HEAD
> *
> !/**/
> !*.c
> !*.h
> !*.S
> !*.sh
> +=======

For the love of all that is holy, please take the time to actually look at what
you're posting.

> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index b7cfb15712d1..66d7ab3da990 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -111,6 +111,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_all_boot_test
> 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/x86_64/sev_all_boot_test.c b/tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c
> new file mode 100644
> index 000000000000..e9e4d7305bc1
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c

"all_boot" is kinda odd. Maybe just sev_basic_test?

> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Basic SEV boot tests.
> + *
> + */

Meh, I'd leave off the comment, the testname should be pretty self-explanatory.

> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +
> +#include "test_util.h"
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "svm_util.h"
> +#include "linux/psp-sev.h"
> +#include "sev.h"
> +
> +#define NR_SYNCS 1
> +
> +#define MSR_AMD64_SEV_BIT 1

msr-index.h exists in tools, use the defines from there.

> +
> +static void guest_run_loop(struct kvm_vcpu *vcpu)

Another pointless helper with one caller. _If_ more subtests come along _and_
need this separate loop, then by all means. But right now, it's just noise.

> +{
> + struct ucall uc;
> + int i;
> +
> + for (i = 0; i <= NR_SYNCS; ++i) {

There's zero reason to cap the number of loops, and it can lead to false passes,
e.g. if the guest gets stuck on GUEST_SYNC() then this will pass. Just do

for (;;) {

> + vcpu_run(vcpu);
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_SYNC:
> + continue;
> + case UCALL_DONE:
> + return;
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT(uc);
> + default:
> + TEST_FAIL("Unexpected exit: %s",
> + exit_reason_str(vcpu->run->exit_reason));
> + }
> + }
> +}
> +
> +static void is_sev_enabled(void)
> +{
> + uint64_t sev_status;
> +
> + GUEST_ASSERT(this_cpu_has(X86_FEATURE_SEV));
> +
> + sev_status = rdmsr(MSR_AMD64_SEV);
> + GUEST_ASSERT(sev_status & 0x1);
> +}
> +
> +static void guest_sev_code(void)
> +{
> + GUEST_SYNC(1);
> +
> + is_sev_enabled();

Again, completely unnecessary helper. And is_blah_enabled() _strongly_ suggests
a pure function that returns true/false.

All in all, this can be slimmed down to:

// SPDX-License-Identifier: GPL-2.0-only
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>

#include "test_util.h"
#include "kvm_util.h"
#include "processor.h"
#include "svm_util.h"
#include "linux/psp-sev.h"
#include "sev.h"

static void guest_sev_code(void)
{
GUEST_ASSERT(this_cpu_has(X86_FEATURE_SEV));
GUEST_ASSERT(rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SEV_ENABLED);

GUEST_DONE();
}

static void test_sev(void *guest_code, uint64_t policy)
{
struct kvm_vcpu *vcpu;
struct kvm_vm *vm;
struct ucall uc;
int i;

vm = vm_sev_create_with_one_vcpu(policy, guest_code, &vcpu);

for (;;) {
vcpu_run(vcpu);

switch (get_ucall(vcpu, &uc)) {
case UCALL_SYNC:
continue;
case UCALL_DONE:
return;
case UCALL_ABORT:
REPORT_GUEST_ASSERT(uc);
default:
TEST_FAIL("Unexpected exit: %s",
exit_reason_str(vcpu->run->exit_reason));
}
}

kvm_vm_free(vm);
}

int main(int argc, char *argv[])
{
TEST_REQUIRE(is_kvm_sev_supported());

test_sev(guest_sev_code, SEV_POLICY_NO_DBG);
test_sev(guest_sev_code, 0);

return 0;
}