[PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test

Sagi Shahar posted 21 patches 4 months, 2 weeks ago
There is a newer version of this series
[PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test
Posted by Sagi Shahar 4 months, 2 weeks ago
Adding a test to verify TDX lifecycle by creating a simple TD.

Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |  1 +
 .../selftests/kvm/include/x86/tdx/tdx_util.h  | 10 ++++++
 .../selftests/kvm/lib/x86/tdx/tdx_util.c      | 18 +++++++++++
 tools/testing/selftests/kvm/x86/tdx_vm_test.c | 31 +++++++++++++++++++
 4 files changed, 60 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/tdx_vm_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 1a73e08c8437..1a76e9fa45d6 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -155,6 +155,7 @@ TEST_GEN_PROGS_x86 += rseq_test
 TEST_GEN_PROGS_x86 += steal_time
 TEST_GEN_PROGS_x86 += system_counter_offset_test
 TEST_GEN_PROGS_x86 += pre_fault_memory_test
+TEST_GEN_PROGS_x86 += x86/tdx_vm_test
 
 # Compiled outputs used by test targets
 TEST_GEN_PROGS_EXTENDED_x86 += x86/nx_huge_pages_test
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
index 2467b6c35557..775ca249f74d 100644
--- a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
@@ -11,6 +11,14 @@ static inline bool is_tdx_vm(struct kvm_vm *vm)
 	return vm->type == KVM_X86_TDX_VM;
 }
 
+/*
+ * Verify that TDX is supported by KVM.
+ */
+static inline bool is_tdx_enabled(void)
+{
+	return !!(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_TDX_VM));
+}
+
 /*
  * TDX ioctls
  */
@@ -72,5 +80,7 @@ void vm_tdx_load_vcpu_boot_parameters(struct kvm_vm *vm, struct kvm_vcpu *vcpu);
 void vm_tdx_set_vcpu_entry_point(struct kvm_vcpu *vcpu, void *guest_code);
 
 void vm_tdx_finalize(struct kvm_vm *vm);
+struct kvm_vm *vm_tdx_create_with_one_vcpu(void *guest_code,
+					   struct kvm_vcpu **vcpu);
 
 #endif // SELFTESTS_TDX_TDX_UTIL_H
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
index 53cfadeff8de..714413e062fd 100644
--- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
@@ -328,3 +328,21 @@ void vm_tdx_finalize(struct kvm_vm *vm)
 	load_td_private_memory(vm);
 	vm_tdx_vm_ioctl(vm, KVM_TDX_FINALIZE_VM, 0, NULL);
 }
+
+struct kvm_vm *vm_tdx_create_with_one_vcpu(void *guest_code,
+					   struct kvm_vcpu **vcpu)
+{
+	struct vm_shape shape = {
+		.mode = VM_MODE_DEFAULT,
+		.type = KVM_X86_TDX_VM,
+	};
+	struct kvm_vm *vm;
+	struct kvm_vcpu *vcpus[1];
+
+	vm = __vm_create_with_vcpus(shape, 1, 0, guest_code, vcpus);
+	*vcpu = vcpus[0];
+
+	vm_tdx_finalize(vm);
+
+	return vm;
+}
diff --git a/tools/testing/selftests/kvm/x86/tdx_vm_test.c b/tools/testing/selftests/kvm/x86/tdx_vm_test.c
new file mode 100644
index 000000000000..a9ee489eea1a
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/tdx_vm_test.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "kvm_util.h"
+#include "tdx/tdx_util.h"
+#include "ucall_common.h"
+#include "kselftest_harness.h"
+
+static void guest_code_lifecycle(void)
+{
+	GUEST_DONE();
+}
+
+TEST(verify_td_lifecycle)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	struct ucall uc;
+
+	vm = vm_tdx_create_with_one_vcpu(guest_code_lifecycle, &vcpu);
+
+	vcpu_run(vcpu);
+	TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_DONE);
+
+	kvm_vm_free(vm);
+}
+
+int main(int argc, char **argv)
+{
+	TEST_REQUIRE(is_tdx_enabled());
+	return test_harness_run(argc, argv);
+}
-- 
2.51.0.536.g15c5d4f767-goog
Re: [PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test
Posted by Sean Christopherson 3 months, 2 weeks ago
On Thu, Sep 25, 2025, Sagi Shahar wrote:
> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> index 53cfadeff8de..714413e062fd 100644
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> @@ -328,3 +328,21 @@ void vm_tdx_finalize(struct kvm_vm *vm)
>  	load_td_private_memory(vm);
>  	vm_tdx_vm_ioctl(vm, KVM_TDX_FINALIZE_VM, 0, NULL);
>  }
> +
> +struct kvm_vm *vm_tdx_create_with_one_vcpu(void *guest_code,
> +					   struct kvm_vcpu **vcpu)
> +{
> +	struct vm_shape shape = {
> +		.mode = VM_MODE_DEFAULT,
> +		.type = KVM_X86_TDX_VM,
> +	};
> +	struct kvm_vm *vm;
> +	struct kvm_vcpu *vcpus[1];
> +
> +	vm = __vm_create_with_vcpus(shape, 1, 0, guest_code, vcpus);
> +	*vcpu = vcpus[0];
> +
> +	vm_tdx_finalize(vm);
> +
> +	return vm;
> +}

Rather than add a full wrapper, and duplicate all of vm_sev_create_with_one_vcpu(),
we should just add macros to convert a type to a shape.

E.g. with this, you can simply add:

  #define VM_SHAPE_TDX	VM_TYPE(KVM_X86_TDX_VM)

And coupled with Ira's suggestion regarding vm_tdx_finalize(), there should be
no need for vm_tdx_create_with_one_vcpu().


--
From: Sean Christopherson <seanjc@google.com>
Date: Fri, 24 Oct 2025 09:14:43 -0700
Subject: [PATCH] KVM: selftests: Add macros so simplify creating VM shapes for
 non-default types

Add VM_TYPE() and __VM_TYPE() macros to create a vm_shape structure given
a type (and mode), and use the macros to define VM_SHAPE_{SEV,SEV_ES,SNP}
shapes for x86's SEV family of VM shapes.  Providing common infrastructure
will avoid having to copy+paste vm_sev_create_with_one_vcpu() for TDX.

Use the new SEV+ shapes and drop vm_sev_create_with_one_vcpu().

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  | 14 +++++++
 .../selftests/kvm/include/x86/processor.h     |  4 ++
 tools/testing/selftests/kvm/include/x86/sev.h |  2 -
 tools/testing/selftests/kvm/lib/x86/sev.c     | 16 --------
 .../selftests/kvm/x86/sev_smoke_test.c        | 40 +++++++++----------
 5 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index af52cd938b50..af0b53987c06 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -210,6 +210,20 @@ kvm_static_assert(sizeof(struct vm_shape) == sizeof(uint64_t));
 	shape;					\
 })
 
+#define __VM_TYPE(__mode, __type)		\
+({						\
+	struct vm_shape shape = {		\
+		.mode = (__mode),		\
+		.type = (__type)		\
+	};					\
+						\
+	shape;					\
+})
+
+#define VM_TYPE(__type)				\
+	__VM_TYPE(VM_MODE_DEFAULT, __type)
+
+
 #if defined(__aarch64__)
 
 extern enum vm_guest_mode vm_mode_default;
diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index 51cd84b9ca66..dd21e11e1908 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -362,6 +362,10 @@ static inline unsigned int x86_model(unsigned int eax)
 	return ((eax >> 12) & 0xf0) | ((eax >> 4) & 0x0f);
 }
 
+#define VM_SHAPE_SEV		VM_TYPE(KVM_X86_SEV_VM)
+#define VM_SHAPE_SEV_ES		VM_TYPE(KVM_X86_SEV_ES_VM)
+#define VM_SHAPE_SNP		VM_TYPE(KVM_X86_SNP_VM)
+
 /* Page table bitfield declarations */
 #define PTE_PRESENT_MASK        BIT_ULL(0)
 #define PTE_WRITABLE_MASK       BIT_ULL(1)
diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h
index 008b4169f5e2..3c3294599ba6 100644
--- a/tools/testing/selftests/kvm/include/x86/sev.h
+++ b/tools/testing/selftests/kvm/include/x86/sev.h
@@ -53,8 +53,6 @@ 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, uint64_t policy, uint8_t *measurement);
 
 kvm_static_assert(SEV_RET_SUCCESS == 0);
diff --git a/tools/testing/selftests/kvm/lib/x86/sev.c b/tools/testing/selftests/kvm/lib/x86/sev.c
index c3a9838f4806..1e3f6514c28d 100644
--- a/tools/testing/selftests/kvm/lib/x86/sev.c
+++ b/tools/testing/selftests/kvm/lib/x86/sev.c
@@ -158,22 +158,6 @@ void snp_vm_launch_finish(struct kvm_vm *vm)
 	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)
-{
-	struct vm_shape shape = {
-		.mode = VM_MODE_DEFAULT,
-		.type = type,
-	};
-	struct kvm_vm *vm;
-	struct kvm_vcpu *cpus[1];
-
-	vm = __vm_create_with_vcpus(shape, 1, 0, guest_code, cpus);
-	*cpu = cpus[0];
-
-	return vm;
-}
-
 void vm_sev_launch(struct kvm_vm *vm, uint64_t policy, uint8_t *measurement)
 {
 	if (is_sev_snp_vm(vm)) {
diff --git a/tools/testing/selftests/kvm/x86/sev_smoke_test.c b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
index 77256c89bb8d..3903793c6750 100644
--- a/tools/testing/selftests/kvm/x86/sev_smoke_test.c
+++ b/tools/testing/selftests/kvm/x86/sev_smoke_test.c
@@ -74,7 +74,7 @@ static void compare_xsave(u8 *from_host, u8 *from_guest)
 		abort();
 }
 
-static void test_sync_vmsa(uint32_t type, uint64_t policy)
+static void test_sync_vmsa(struct vm_shape shape, uint64_t policy)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
@@ -84,7 +84,7 @@ static void test_sync_vmsa(uint32_t type, uint64_t policy)
 	double x87val = M_PI;
 	struct kvm_xsave __attribute__((aligned(64))) xsave = { 0 };
 
-	vm = vm_sev_create_with_one_vcpu(type, guest_code_xsave, &vcpu);
+	vm = vm_create_shape_with_one_vcpu(shape, &vcpu, guest_code_xsave);
 	gva = vm_vaddr_alloc_shared(vm, PAGE_SIZE, KVM_UTIL_MIN_VADDR,
 				    MEM_REGION_TEST_DATA);
 	hva = addr_gva2hva(vm, gva);
@@ -120,13 +120,13 @@ static void test_sync_vmsa(uint32_t type, uint64_t policy)
 	kvm_vm_free(vm);
 }
 
-static void test_sev(void *guest_code, uint32_t type, uint64_t policy)
+static void test_sev(void *guest_code, struct vm_shape shape, uint64_t policy)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 	struct ucall uc;
 
-	vm = vm_sev_create_with_one_vcpu(type, guest_code, &vcpu);
+	vm = vm_create_shape_with_one_vcpu(shape, &vcpu, guest_code);
 
 	/* TODO: Validate the measurement is as expected. */
 	vm_sev_launch(vm, policy, NULL);
@@ -171,12 +171,12 @@ static void guest_shutdown_code(void)
 	__asm__ __volatile__("ud2");
 }
 
-static void test_sev_shutdown(uint32_t type, uint64_t policy)
+static void test_sev_shutdown(struct vm_shape shape, uint64_t policy)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 
-	vm = vm_sev_create_with_one_vcpu(type, guest_shutdown_code, &vcpu);
+	vm = vm_create_shape_with_one_vcpu(shape, &vcpu, guest_shutdown_code);
 
 	vm_sev_launch(vm, policy, NULL);
 
@@ -188,28 +188,28 @@ static void test_sev_shutdown(uint32_t type, uint64_t policy)
 	kvm_vm_free(vm);
 }
 
-static void test_sev_smoke(void *guest, uint32_t type, uint64_t policy)
+static void test_sev_smoke(void *guest, struct vm_shape shape, uint64_t policy)
 {
 	const u64 xf_mask = XFEATURE_MASK_X87_AVX;
 
-	if (type == KVM_X86_SNP_VM)
-		test_sev(guest, type, policy | SNP_POLICY_DBG);
+	if (shape.type == KVM_X86_SNP_VM)
+		test_sev(guest, shape, policy | SNP_POLICY_DBG);
 	else
-		test_sev(guest, type, policy | SEV_POLICY_NO_DBG);
-	test_sev(guest, type, policy);
+		test_sev(guest, shape, policy | SEV_POLICY_NO_DBG);
+	test_sev(guest, shape, policy);
 
-	if (type == KVM_X86_SEV_VM)
+	if (shape.type == KVM_X86_SEV_VM)
 		return;
 
-	test_sev_shutdown(type, policy);
+	test_sev_shutdown(shape, policy);
 
 	if (kvm_has_cap(KVM_CAP_XCRS) &&
 	    (xgetbv(0) & kvm_cpu_supported_xcr0() & xf_mask) == xf_mask) {
-		test_sync_vmsa(type, policy);
-		if (type == KVM_X86_SNP_VM)
-			test_sync_vmsa(type, policy | SNP_POLICY_DBG);
+		test_sync_vmsa(shape, policy);
+		if (shape.type == KVM_X86_SNP_VM)
+			test_sync_vmsa(shape, policy | SNP_POLICY_DBG);
 		else
-			test_sync_vmsa(type, policy | SEV_POLICY_NO_DBG);
+			test_sync_vmsa(shape, policy | SEV_POLICY_NO_DBG);
 	}
 }
 
@@ -217,13 +217,13 @@ 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);
+	test_sev_smoke(guest_sev_code, VM_SHAPE_SEV, 0);
 
 	if (kvm_cpu_has(X86_FEATURE_SEV_ES))
-		test_sev_smoke(guest_sev_es_code, KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
+		test_sev_smoke(guest_sev_es_code, VM_SHAPE_SEV_ES, SEV_POLICY_ES);
 
 	if (kvm_cpu_has(X86_FEATURE_SEV_SNP))
-		test_sev_smoke(guest_snp_code, KVM_X86_SNP_VM, snp_default_policy());
+		test_sev_smoke(guest_snp_code, VM_SHAPE_SNP, snp_default_policy());
 
 	return 0;
 }

base-commit: 4cc167c50eb19d44ac7e204938724e685e3d8057
--
Re: [PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test
Posted by Ira Weiny 3 months, 2 weeks ago
Sean Christopherson wrote:
> On Thu, Sep 25, 2025, Sagi Shahar wrote:
> > diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > index 53cfadeff8de..714413e062fd 100644
> > --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> > @@ -328,3 +328,21 @@ void vm_tdx_finalize(struct kvm_vm *vm)
> >  	load_td_private_memory(vm);
> >  	vm_tdx_vm_ioctl(vm, KVM_TDX_FINALIZE_VM, 0, NULL);
> >  }
> > +
> > +struct kvm_vm *vm_tdx_create_with_one_vcpu(void *guest_code,
> > +					   struct kvm_vcpu **vcpu)
> > +{
> > +	struct vm_shape shape = {
> > +		.mode = VM_MODE_DEFAULT,
> > +		.type = KVM_X86_TDX_VM,
> > +	};
> > +	struct kvm_vm *vm;
> > +	struct kvm_vcpu *vcpus[1];
> > +
> > +	vm = __vm_create_with_vcpus(shape, 1, 0, guest_code, vcpus);
> > +	*vcpu = vcpus[0];
> > +
> > +	vm_tdx_finalize(vm);
> > +
> > +	return vm;
> > +}
> 
> Rather than add a full wrapper, and duplicate all of vm_sev_create_with_one_vcpu(),
> we should just add macros to convert a type to a shape.
> 
> E.g. with this, you can simply add:
> 
>   #define VM_SHAPE_TDX	VM_TYPE(KVM_X86_TDX_VM)
> 
> And coupled with Ira's suggestion regarding vm_tdx_finalize(), there should be
> no need for vm_tdx_create_with_one_vcpu().

All sounds reasonable to me but some questions/comments below.


[snip]

> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index af52cd938b50..af0b53987c06 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -210,6 +210,20 @@ kvm_static_assert(sizeof(struct vm_shape) == sizeof(uint64_t));
>  	shape;					\
>  })
>  
> +#define __VM_TYPE(__mode, __type)		\
> +({						\
> +	struct vm_shape shape = {		\
> +		.mode = (__mode),		\
> +		.type = (__type)		\
> +	};					\
> +						\
> +	shape;					\
> +})
> +
> +#define VM_TYPE(__type)				\
> +	__VM_TYPE(VM_MODE_DEFAULT, __type)

We already have VM_SHAPE()?  Why do we need this as well?

> +
> +
>  #if defined(__aarch64__)
>  
>  extern enum vm_guest_mode vm_mode_default;
> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index 51cd84b9ca66..dd21e11e1908 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> @@ -362,6 +362,10 @@ static inline unsigned int x86_model(unsigned int eax)
>  	return ((eax >> 12) & 0xf0) | ((eax >> 4) & 0x0f);
>  }
>  
> +#define VM_SHAPE_SEV		VM_TYPE(KVM_X86_SEV_VM)
> +#define VM_SHAPE_SEV_ES		VM_TYPE(KVM_X86_SEV_ES_VM)
> +#define VM_SHAPE_SNP		VM_TYPE(KVM_X86_SNP_VM)

FWIW I think the SEV bits should be pulled apart from the TDX bits and the
TDX bits squashed back into this series with the SEV as a per-cursor patch.

Ira

[snip]
Re: [PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test
Posted by Sean Christopherson 3 months, 2 weeks ago
On Mon, Oct 27, 2025, Ira Weiny wrote:
> Sean Christopherson wrote:
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index af52cd938b50..af0b53987c06 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -210,6 +210,20 @@ kvm_static_assert(sizeof(struct vm_shape) == sizeof(uint64_t));
> >  	shape;					\
> >  })
> >  
> > +#define __VM_TYPE(__mode, __type)		\
> > +({						\
> > +	struct vm_shape shape = {		\
> > +		.mode = (__mode),		\
> > +		.type = (__type)		\
> > +	};					\
> > +						\
> > +	shape;					\
> > +})
> > +
> > +#define VM_TYPE(__type)				\
> > +	__VM_TYPE(VM_MODE_DEFAULT, __type)
> 
> We already have VM_SHAPE()?  Why do we need this as well?

VM_SHAPE() takes the "mode", and assumes a default type.  The alternative would
be something like __VM_SHAPE(__type, __mode), but that's annoying, especially on
x86 which only has one mode.

And __VM_SHAPE(__type) + ____VM_SHAPE(__type, __mode) feels even more weird.

I'm definitely open to more ideas, VM_TYPE() isn't great either, just the least
awful option I came up with.

> >  #if defined(__aarch64__)
> >  
> >  extern enum vm_guest_mode vm_mode_default;
> > diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> > index 51cd84b9ca66..dd21e11e1908 100644
> > --- a/tools/testing/selftests/kvm/include/x86/processor.h
> > +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> > @@ -362,6 +362,10 @@ static inline unsigned int x86_model(unsigned int eax)
> >  	return ((eax >> 12) & 0xf0) | ((eax >> 4) & 0x0f);
> >  }
> >  
> > +#define VM_SHAPE_SEV		VM_TYPE(KVM_X86_SEV_VM)
> > +#define VM_SHAPE_SEV_ES		VM_TYPE(KVM_X86_SEV_ES_VM)
> > +#define VM_SHAPE_SNP		VM_TYPE(KVM_X86_SNP_VM)
> 
> FWIW I think the SEV bits should be pulled apart from the TDX bits and the
> TDX bits squashed back into this series with the SEV as a per-cursor patch.

Ya, that's my intent, "officially" post and land this SEV+ change, then have the
TDX series build on top.  Or did you mean something else?
Re: [PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test
Posted by Ira Weiny 3 months, 1 week ago
Sean Christopherson wrote:
> On Mon, Oct 27, 2025, Ira Weiny wrote:
> > Sean Christopherson wrote:
> > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > > index af52cd938b50..af0b53987c06 100644
> > > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > > @@ -210,6 +210,20 @@ kvm_static_assert(sizeof(struct vm_shape) == sizeof(uint64_t));
> > >  	shape;					\
> > >  })
> > >  
> > > +#define __VM_TYPE(__mode, __type)		\
> > > +({						\
> > > +	struct vm_shape shape = {		\
> > > +		.mode = (__mode),		\
> > > +		.type = (__type)		\
> > > +	};					\
> > > +						\
> > > +	shape;					\
> > > +})
> > > +
> > > +#define VM_TYPE(__type)				\
> > > +	__VM_TYPE(VM_MODE_DEFAULT, __type)
> > 
> > We already have VM_SHAPE()?  Why do we need this as well?
> 
> VM_SHAPE() takes the "mode", and assumes a default type.

Ah yea.

> The alternative would
> be something like __VM_SHAPE(__type, __mode), but that's annoying, especially on
> x86 which only has one mode.
> 
> And __VM_SHAPE(__type) + ____VM_SHAPE(__type, __mode) feels even more weird.
> 
> I'm definitely open to more ideas, VM_TYPE() isn't great either, just the least
> awful option I came up with.

No this name is fine then.  I got dyslexic with mode vs type, apologies.

> 
> > >  #if defined(__aarch64__)
> > >  
> > >  extern enum vm_guest_mode vm_mode_default;
> > > diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> > > index 51cd84b9ca66..dd21e11e1908 100644
> > > --- a/tools/testing/selftests/kvm/include/x86/processor.h
> > > +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> > > @@ -362,6 +362,10 @@ static inline unsigned int x86_model(unsigned int eax)
> > >  	return ((eax >> 12) & 0xf0) | ((eax >> 4) & 0x0f);
> > >  }
> > >  
> > > +#define VM_SHAPE_SEV		VM_TYPE(KVM_X86_SEV_VM)
> > > +#define VM_SHAPE_SEV_ES		VM_TYPE(KVM_X86_SEV_ES_VM)
> > > +#define VM_SHAPE_SNP		VM_TYPE(KVM_X86_SNP_VM)
> > 
> > FWIW I think the SEV bits should be pulled apart from the TDX bits and the
> > TDX bits squashed back into this series with the SEV as a per-cursor patch.
> 
> Ya, that's my intent, "officially" post and land this SEV+ change, then have the
> TDX series build on top.

Sounds good.

>
> Or did you mean something else?

No.

Ira
Re: [PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test
Posted by Sagi Shahar 3 months, 2 weeks ago
On Mon, Oct 27, 2025 at 6:42 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Oct 27, 2025, Ira Weiny wrote:
> > Sean Christopherson wrote:
> > > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > > index af52cd938b50..af0b53987c06 100644
> > > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > > @@ -210,6 +210,20 @@ kvm_static_assert(sizeof(struct vm_shape) == sizeof(uint64_t));
> > >     shape;                                  \
> > >  })
> > >
> > > +#define __VM_TYPE(__mode, __type)          \
> > > +({                                         \
> > > +   struct vm_shape shape = {               \
> > > +           .mode = (__mode),               \
> > > +           .type = (__type)                \
> > > +   };                                      \
> > > +                                           \
> > > +   shape;                                  \
> > > +})
> > > +
> > > +#define VM_TYPE(__type)                            \
> > > +   __VM_TYPE(VM_MODE_DEFAULT, __type)
> >
> > We already have VM_SHAPE()?  Why do we need this as well?
>
> VM_SHAPE() takes the "mode", and assumes a default type.  The alternative would
> be something like __VM_SHAPE(__type, __mode), but that's annoying, especially on
> x86 which only has one mode.
>
> And __VM_SHAPE(__type) + ____VM_SHAPE(__type, __mode) feels even more weird.
>
> I'm definitely open to more ideas, VM_TYPE() isn't great either, just the least
> awful option I came up with.
>
> > >  #if defined(__aarch64__)
> > >
> > >  extern enum vm_guest_mode vm_mode_default;
> > > diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> > > index 51cd84b9ca66..dd21e11e1908 100644
> > > --- a/tools/testing/selftests/kvm/include/x86/processor.h
> > > +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> > > @@ -362,6 +362,10 @@ static inline unsigned int x86_model(unsigned int eax)
> > >     return ((eax >> 12) & 0xf0) | ((eax >> 4) & 0x0f);
> > >  }
> > >
> > > +#define VM_SHAPE_SEV               VM_TYPE(KVM_X86_SEV_VM)
> > > +#define VM_SHAPE_SEV_ES            VM_TYPE(KVM_X86_SEV_ES_VM)
> > > +#define VM_SHAPE_SNP               VM_TYPE(KVM_X86_SNP_VM)
> >
> > FWIW I think the SEV bits should be pulled apart from the TDX bits and the
> > TDX bits squashed back into this series with the SEV as a per-cursor patch.
>
> Ya, that's my intent, "officially" post and land this SEV+ change, then have the
> TDX series build on top.  Or did you mean something else?

I've got v12 mostly ready to be sent for review. I was thinking of
incorporating this change as part of that series. Do you prefer that I
wait until this patch lands before I post v12?
Re: [PATCH v11 21/21] KVM: selftests: Add TDX lifecycle test
Posted by Ira Weiny 3 months, 1 week ago
Sagi Shahar wrote:
> On Mon, Oct 27, 2025 at 6:42 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Oct 27, 2025, Ira Weiny wrote:
> > > Sean Christopherson wrote:

[snip]

> > > > diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> > > > index 51cd84b9ca66..dd21e11e1908 100644
> > > > --- a/tools/testing/selftests/kvm/include/x86/processor.h
> > > > +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> > > > @@ -362,6 +362,10 @@ static inline unsigned int x86_model(unsigned int eax)
> > > >     return ((eax >> 12) & 0xf0) | ((eax >> 4) & 0x0f);
> > > >  }
> > > >
> > > > +#define VM_SHAPE_SEV               VM_TYPE(KVM_X86_SEV_VM)
> > > > +#define VM_SHAPE_SEV_ES            VM_TYPE(KVM_X86_SEV_ES_VM)
> > > > +#define VM_SHAPE_SNP               VM_TYPE(KVM_X86_SNP_VM)
> > >
> > > FWIW I think the SEV bits should be pulled apart from the TDX bits and the
> > > TDX bits squashed back into this series with the SEV as a per-cursor patch.
> >
> > Ya, that's my intent, "officially" post and land this SEV+ change, then have the
> > TDX series build on top.  Or did you mean something else?
> 
> I've got v12 mostly ready to be sent for review. I was thinking of
> incorporating this change as part of that series. Do you prefer that I
> wait until this patch lands before I post v12?

I can't speak for Sean directly.  Buy my interpretation was you should
make this SEV change a lead in patch of v12.  IOW v12 patch 1/X is this
SEV change which sets up the new VM_TYPE() macro to be used subsequently
by the TDX code.

Then if there are further issues with the TDX stuff Sean can pick this off
as a clean up.  But my hope is that this series is pretty close to being
acceptable and it will land.  Then we can focus on building the tests on top of
this new architecture.

Ira