[V3 11/11] KVM: selftests: Add simple sev vm testing

Peter Gonda posted 11 patches 3 years, 8 months ago
There is a newer version of this series
[V3 11/11] KVM: selftests: Add simple sev vm testing
Posted by Peter Gonda 3 years, 8 months ago
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.

Signed-off-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Peter Gonda <pgonda@google.com>
---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/x86_64/sev.h        |   3 +
 tools/testing/selftests/kvm/lib/x86_64/sev.c  |   2 -
 .../selftests/kvm/x86_64/sev_all_boot_test.c  | 131 ++++++++++++++++++
 5 files changed, 136 insertions(+), 2 deletions(-)
 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 d625a3f83780..ca57969a0923 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -33,6 +33,7 @@
 /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
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index b247c4b595af..73b083f93b46 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -122,6 +122,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/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
index 2f7f7c741b12..b6552ea1c716 100644
--- a/tools/testing/selftests/kvm/include/x86_64/sev.h
+++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
@@ -22,6 +22,9 @@
 #define SEV_POLICY_NO_DBG	(1UL << 0)
 #define SEV_POLICY_ES		(1UL << 2)
 
+#define CPUID_MEM_ENC_LEAF 0x8000001f
+#define CPUID_EBX_CBIT_MASK 0x3f
+
 enum {
 	SEV_GSTATE_UNINIT = 0,
 	SEV_GSTATE_LUPDATE,
diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
index 3abcf50c0b5d..8f9f55c685a7 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
@@ -13,8 +13,6 @@
 #include "sev.h"
 
 #define PAGE_SHIFT		12
-#define CPUID_MEM_ENC_LEAF 0x8000001f
-#define CPUID_EBX_CBIT_MASK 0x3f
 
 struct sev_vm {
 	struct kvm_vm *vm;
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..b319d18bdb60
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Basic SEV boot tests.
+ *
+ * Copyright (C) 2021 Advanced Micro Devices
+ */
+#define _GNU_SOURCE /* for program_invocation_short_name */
+#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 VCPU_ID			2
+#define PAGE_STRIDE		32
+
+#define SHARED_PAGES		8192
+#define SHARED_VADDR_MIN	0x1000000
+
+#define PRIVATE_PAGES		2048
+#define PRIVATE_VADDR_MIN	(SHARED_VADDR_MIN + SHARED_PAGES * PAGE_SIZE)
+
+#define TOTAL_PAGES		(512 + SHARED_PAGES + PRIVATE_PAGES)
+
+#define NR_SYNCS 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:
+			TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx",
+				    (const char *)uc.args[0], __FILE__,
+				    uc.args[1], uc.args[2], uc.args[3]);
+		default:
+			TEST_ASSERT(
+				false, "Unexpected exit: %s",
+				exit_reason_str(vcpu->run->exit_reason));
+		}
+	}
+}
+
+static void __attribute__((__flatten__)) guest_sev_code(void)
+{
+	uint32_t eax, ebx, ecx, edx;
+	uint64_t sev_status;
+
+	GUEST_SYNC(1);
+
+	cpuid(CPUID_MEM_ENC_LEAF, &eax, &ebx, &ecx, &edx);
+	GUEST_ASSERT(eax & (1 << 1));
+
+	sev_status = rdmsr(MSR_AMD64_SEV);
+	GUEST_ASSERT((sev_status & 0x1) == 1);
+
+	GUEST_DONE();
+}
+
+static struct sev_vm *setup_test_common(void *guest_code, uint64_t policy,
+					struct kvm_vcpu **vcpu)
+{
+	uint8_t measurement[512];
+	struct sev_vm *sev;
+	struct kvm_vm *vm;
+	int i;
+
+	sev = sev_vm_create(policy, TOTAL_PAGES);
+	if (!sev)
+		return NULL;
+	vm = sev_get_vm(sev);
+
+	/* Set up VCPU and initial guest kernel. */
+	*vcpu = vm_vcpu_add(vm, VCPU_ID, guest_code);
+	kvm_vm_elf_load(vm, program_invocation_name);
+
+	/* Allocations/setup done. Encrypt initial guest payload. */
+	sev_vm_launch(sev);
+
+	/* Dump the initial measurement. A test to actually verify it would be nice. */
+	sev_vm_launch_measure(sev, measurement);
+	pr_info("guest measurement: ");
+	for (i = 0; i < 32; ++i)
+		pr_info("%02x", measurement[i]);
+	pr_info("\n");
+
+	sev_vm_launch_finish(sev);
+
+	return sev;
+}
+
+static void test_sev(void *guest_code, uint64_t policy)
+{
+	struct sev_vm *sev;
+	struct kvm_vcpu *vcpu;
+
+	sev = setup_test_common(guest_code, policy, &vcpu);
+	if (!sev)
+		return;
+
+	/* Guest is ready to run. Do the tests. */
+	guest_run_loop(vcpu);
+
+	pr_info("guest ran successfully\n");
+
+	sev_vm_free(sev);
+}
+
+int main(int argc, char *argv[])
+{
+	/* SEV tests */
+	test_sev(guest_sev_code, SEV_POLICY_NO_DBG);
+	test_sev(guest_sev_code, 0);
+
+	return 0;
+}
-- 
2.37.1.559.g78731f0fdb-goog
Re: [V3 11/11] KVM: selftests: Add simple sev vm testing
Posted by Sean Christopherson 3 years, 7 months ago
On Wed, Aug 10, 2022, 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.
> 
> Signed-off-by: Michael Roth <michael.roth@amd.com>

If you're including Mike's SOB, then he should either be credited as the author
or with Co-developed-by.  If the code has been significantly rewritten, i.e.
carrying Mike's SOB would be disingenuous, then do a less "official"

  Originally-by: Michael Roth <michael.roth@amd.com>

or 

  Suggested-by: Michael Roth <michael.roth@amd.com>
Re: [V3 11/11] KVM: selftests: Add simple sev vm testing
Posted by Sean Christopherson 3 years, 7 months ago
/sev_vm_launch_measurOn Wed, Aug 10, 2022, Peter Gonda wrote:
> diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
> index 2f7f7c741b12..b6552ea1c716 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/sev.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
> @@ -22,6 +22,9 @@
>  #define SEV_POLICY_NO_DBG	(1UL << 0)
>  #define SEV_POLICY_ES		(1UL << 2)
>  
> +#define CPUID_MEM_ENC_LEAF 0x8000001f
> +#define CPUID_EBX_CBIT_MASK 0x3f

Ha!  I was going to say "put these in processor.h", but I have an even better idea.
I'll try to a series posted tomorrow (compile tested only at this point), but what
I'm hoping to do is to allow automagic retrieval of multi-bit CPUID properties, a la
the existing this_cpu_has() stuff.

E.g.

	#define X86_PROPERTY_CBIT_LOCATION		KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 0, 5)

and then

	sev->enc_bit = this_cpu_property(X86_PROPERTY_CBIT_LOCATION);

LOL, now I see that the defines in sev.c were introduced back in patch 08.  That's
probably fine for your submission so as not to take a dependency on the "property"
idea.  This patch doesn't need to move the CPUID_* defines because it can use
this_cpu_has(X86_FEATURE_SEV) and avoid referencing CPUID_MEM_ENC_LEAF.

>  enum {
>  	SEV_GSTATE_UNINIT = 0,
>  	SEV_GSTATE_LUPDATE,
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> index 3abcf50c0b5d..8f9f55c685a7 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> @@ -13,8 +13,6 @@
>  #include "sev.h"
>  
>  #define PAGE_SHIFT		12

Already defined in processor.h

> -#define CPUID_MEM_ENC_LEAF 0x8000001f
> -#define CPUID_EBX_CBIT_MASK 0x3f
>  
>  struct sev_vm {
>  	struct kvm_vm *vm;
> 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..b319d18bdb60
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c
> @@ -0,0 +1,131 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Basic SEV boot tests.
> + *
> + * Copyright (C) 2021 Advanced Micro Devices
> + */
> +#define _GNU_SOURCE /* for program_invocation_short_name */
> +#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 VCPU_ID			2

Nooooooo.  Unless there is a really, REALLY good reason this needs to be '2', just
pass '0' as a literal to vm_vcpu_add() and delete VCPU_ID.

> +#define PAGE_STRIDE		32
> +
> +#define SHARED_PAGES		8192
> +#define SHARED_VADDR_MIN	0x1000000
> +
> +#define PRIVATE_PAGES		2048
> +#define PRIVATE_VADDR_MIN	(SHARED_VADDR_MIN + SHARED_PAGES * PAGE_SIZE)
> +
> +#define TOTAL_PAGES		(512 + SHARED_PAGES + PRIVATE_PAGES)
> +
> +#define NR_SYNCS 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:
> +			TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx",
> +				    (const char *)uc.args[0], __FILE__,
> +				    uc.args[1], uc.args[2], uc.args[3]);
> +		default:
> +			TEST_ASSERT(
> +				false, "Unexpected exit: %s",
> +				exit_reason_str(vcpu->run->exit_reason));
> +		}
> +	}
> +}
> +
> +static void __attribute__((__flatten__)) guest_sev_code(void)

Is __flatten__ strictly necessary?  I don't see this being copied over anything
that would require it to be a contiguous chunk.

> +{
> +	uint32_t eax, ebx, ecx, edx;
> +	uint64_t sev_status;
> +
> +	GUEST_SYNC(1);
> +
> +	cpuid(CPUID_MEM_ENC_LEAF, &eax, &ebx, &ecx, &edx);
> +	GUEST_ASSERT(eax & (1 << 1));

	GUEST_ASSERT(this_cpu_has(X86_FEATURE_SEV));
> +
> +	sev_status = rdmsr(MSR_AMD64_SEV);
> +	GUEST_ASSERT((sev_status & 0x1) == 1);
> +
> +	GUEST_DONE();
> +}
> +
> +static struct sev_vm *setup_test_common(void *guest_code, uint64_t policy,
> +					struct kvm_vcpu **vcpu)
> +{
> +	uint8_t measurement[512];
> +	struct sev_vm *sev;
> +	struct kvm_vm *vm;
> +	int i;
> +
> +	sev = sev_vm_create(policy, TOTAL_PAGES);

	TEST_ASSERT(sev, ...) so that this doesn't silently "pass"?

> +	if (!sev)
> +		return NULL;
> +	vm = sev_get_vm(sev);
> +
> +	/* Set up VCPU and initial guest kernel. */
> +	*vcpu = vm_vcpu_add(vm, VCPU_ID, guest_code);
> +	kvm_vm_elf_load(vm, program_invocation_name);
> +
> +	/* Allocations/setup done. Encrypt initial guest payload. */
> +	sev_vm_launch(sev);
> +
> +	/* Dump the initial measurement. A test to actually verify it would be nice. */
> +	sev_vm_launch_measure(sev, measurement);
> +	pr_info("guest measurement: ");
> +	for (i = 0; i < 32; ++i)
> +		pr_info("%02x", measurement[i]);
> +	pr_info("\n");
> +
> +	sev_vm_launch_finish(sev);
> +
> +	return sev;
> +}
> +
> +static void test_sev(void *guest_code, uint64_t policy)
> +{
> +	struct sev_vm *sev;
> +	struct kvm_vcpu *vcpu;
> +
> +	sev = setup_test_common(guest_code, policy, &vcpu);
> +	if (!sev)
> +		return;

And with an assert above, this return goes away.  Or better yet, fold setup_test_common()
into test_sev(), there's only the one user of the so called "common" function.

> +
> +	/* Guest is ready to run. Do the tests. */
> +	guest_run_loop(vcpu);
> +
> +	pr_info("guest ran successfully\n");
> +
> +	sev_vm_free(sev);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	/* SEV tests */
> +	test_sev(guest_sev_code, SEV_POLICY_NO_DBG);
> +	test_sev(guest_sev_code, 0);
> +
> +	return 0;
> +}
> -- 
> 2.37.1.559.g78731f0fdb-goog
>
Re: [V3 11/11] KVM: selftests: Add simple sev vm testing
Posted by Peter Gonda 3 years, 7 months ago
On Wed, Aug 17, 2022 at 6:22 PM Sean Christopherson <seanjc@google.com> wrote:
>
> /sev_vm_launch_measurOn Wed, Aug 10, 2022, Peter Gonda wrote:
> > diff --git a/tools/testing/selftests/kvm/include/x86_64/sev.h b/tools/testing/selftests/kvm/include/x86_64/sev.h
> > index 2f7f7c741b12..b6552ea1c716 100644
> > --- a/tools/testing/selftests/kvm/include/x86_64/sev.h
> > +++ b/tools/testing/selftests/kvm/include/x86_64/sev.h
> > @@ -22,6 +22,9 @@
> >  #define SEV_POLICY_NO_DBG    (1UL << 0)
> >  #define SEV_POLICY_ES                (1UL << 2)
> >
> > +#define CPUID_MEM_ENC_LEAF 0x8000001f
> > +#define CPUID_EBX_CBIT_MASK 0x3f
>
> Ha!  I was going to say "put these in processor.h", but I have an even better idea.
> I'll try to a series posted tomorrow (compile tested only at this point), but what
> I'm hoping to do is to allow automagic retrieval of multi-bit CPUID properties, a la
> the existing this_cpu_has() stuff.
>
> E.g.
>
>         #define X86_PROPERTY_CBIT_LOCATION              KVM_X86_CPU_PROPERTY(0x8000001F, 0, EBX, 0, 5)
>
> and then
>
>         sev->enc_bit = this_cpu_property(X86_PROPERTY_CBIT_LOCATION);
>
> LOL, now I see that the defines in sev.c were introduced back in patch 08.  That's
> probably fine for your submission so as not to take a dependency on the "property"
> idea.  This patch doesn't need to move the CPUID_* defines because it can use
> this_cpu_has(X86_FEATURE_SEV) and avoid referencing CPUID_MEM_ENC_LEAF.

OK I've put these into sev.c instead of the header. I can clean up
after you property patch series goes in.

>
> >  enum {
> >       SEV_GSTATE_UNINIT = 0,
> >       SEV_GSTATE_LUPDATE,
> > diff --git a/tools/testing/selftests/kvm/lib/x86_64/sev.c b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> > index 3abcf50c0b5d..8f9f55c685a7 100644
> > --- a/tools/testing/selftests/kvm/lib/x86_64/sev.c
> > +++ b/tools/testing/selftests/kvm/lib/x86_64/sev.c
> > @@ -13,8 +13,6 @@
> >  #include "sev.h"
> >
> >  #define PAGE_SHIFT           12
>
> Already defined in processor.h

Removed

>
> > -#define CPUID_MEM_ENC_LEAF 0x8000001f
> > -#define CPUID_EBX_CBIT_MASK 0x3f
> >
> >  struct sev_vm {
> >       struct kvm_vm *vm;
> > 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..b319d18bdb60
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/x86_64/sev_all_boot_test.c
> > @@ -0,0 +1,131 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Basic SEV boot tests.
> > + *
> > + * Copyright (C) 2021 Advanced Micro Devices
> > + */
> > +#define _GNU_SOURCE /* for program_invocation_short_name */
> > +#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 VCPU_ID                      2
>
> Nooooooo.  Unless there is a really, REALLY good reason this needs to be '2', just
> pass '0' as a literal to vm_vcpu_add() and delete VCPU_ID.

Not needed, removed.

>
> > +#define PAGE_STRIDE          32
> > +
> > +#define SHARED_PAGES         8192
> > +#define SHARED_VADDR_MIN     0x1000000
> > +
> > +#define PRIVATE_PAGES                2048
> > +#define PRIVATE_VADDR_MIN    (SHARED_VADDR_MIN + SHARED_PAGES * PAGE_SIZE)
> > +
> > +#define TOTAL_PAGES          (512 + SHARED_PAGES + PRIVATE_PAGES)
> > +
> > +#define NR_SYNCS 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:
> > +                     TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx",
> > +                                 (const char *)uc.args[0], __FILE__,
> > +                                 uc.args[1], uc.args[2], uc.args[3]);
> > +             default:
> > +                     TEST_ASSERT(
> > +                             false, "Unexpected exit: %s",
> > +                             exit_reason_str(vcpu->run->exit_reason));
> > +             }
> > +     }
> > +}
> > +
> > +static void __attribute__((__flatten__)) guest_sev_code(void)
>
> Is __flatten__ strictly necessary?  I don't see this being copied over anything
> that would require it to be a contiguous chunk.

Nope, removed.

>
> > +{
> > +     uint32_t eax, ebx, ecx, edx;
> > +     uint64_t sev_status;
> > +
> > +     GUEST_SYNC(1);
> > +
> > +     cpuid(CPUID_MEM_ENC_LEAF, &eax, &ebx, &ecx, &edx);
> > +     GUEST_ASSERT(eax & (1 << 1));
>
>         GUEST_ASSERT(this_cpu_has(X86_FEATURE_SEV));

Done.

> > +
> > +     sev_status = rdmsr(MSR_AMD64_SEV);
> > +     GUEST_ASSERT((sev_status & 0x1) == 1);
> > +
> > +     GUEST_DONE();
> > +}
> > +
> > +static struct sev_vm *setup_test_common(void *guest_code, uint64_t policy,
> > +                                     struct kvm_vcpu **vcpu)
> > +{
> > +     uint8_t measurement[512];
> > +     struct sev_vm *sev;
> > +     struct kvm_vm *vm;
> > +     int i;
> > +
> > +     sev = sev_vm_create(policy, TOTAL_PAGES);
>
>         TEST_ASSERT(sev, ...) so that this doesn't silently "pass"?

Done.

>
> > +     if (!sev)
> > +             return NULL;
> > +     vm = sev_get_vm(sev);
> > +
> > +     /* Set up VCPU and initial guest kernel. */
> > +     *vcpu = vm_vcpu_add(vm, VCPU_ID, guest_code);
> > +     kvm_vm_elf_load(vm, program_invocation_name);
> > +
> > +     /* Allocations/setup done. Encrypt initial guest payload. */
> > +     sev_vm_launch(sev);
> > +
> > +     /* Dump the initial measurement. A test to actually verify it would be nice. */
> > +     sev_vm_launch_measure(sev, measurement);
> > +     pr_info("guest measurement: ");
> > +     for (i = 0; i < 32; ++i)
> > +             pr_info("%02x", measurement[i]);
> > +     pr_info("\n");
> > +
> > +     sev_vm_launch_finish(sev);
> > +
> > +     return sev;
> > +}
> > +
> > +static void test_sev(void *guest_code, uint64_t policy)
> > +{
> > +     struct sev_vm *sev;
> > +     struct kvm_vcpu *vcpu;
> > +
> > +     sev = setup_test_common(guest_code, policy, &vcpu);
> > +     if (!sev)
> > +             return;
>
> And with an assert above, this return goes away.  Or better yet, fold setup_test_common()
> into test_sev(), there's only the one user of the so called "common" function.

Done.

>
> > +
> > +     /* Guest is ready to run. Do the tests. */
> > +     guest_run_loop(vcpu);
> > +
> > +     pr_info("guest ran successfully\n");
> > +
> > +     sev_vm_free(sev);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +     /* SEV tests */
> > +     test_sev(guest_sev_code, SEV_POLICY_NO_DBG);
> > +     test_sev(guest_sev_code, 0);
> > +
> > +     return 0;
> > +}
> > --
> > 2.37.1.559.g78731f0fdb-goog
> >