linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v12 00/23] TDX KVM selftests
@ 2025-10-28 21:20 Sagi Shahar
  2025-10-28 21:20 ` [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types Sagi Shahar
                   ` (22 more replies)
  0 siblings, 23 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

This is v12 of the TDX selftests.

This series is based on v6.18-rc3

Changes from v11 [1]:
- Rebased on top of v6.18-rc3.
- Hook vm_tdx_finalize() into kvm_arch_vm_finalize_vcpus instead of
  calling it as part of vm_tdx_create_with_one_vcpu. See "KVM: selftests:
  Finalize TD memory as part of kvm_arch_vm_finalize_vcpus" which was
  added to this series.
- Replaced vm_tdx_create_with_one_vcpu with vm_create_shape_with_one_vcpu
  following Sean's patch to simplify creating VM shapes.

[1] https://lore.kernel.org/lkml/20250925172851.606193-1-sagis@google.com/

Ackerley Tng (2):
  KVM: selftests: Add helpers to init TDX memory and finalize VM
  KVM: selftests: Add ucall support for TDX

Erdem Aktas (2):
  KVM: selftests: Add TDX boot code
  KVM: selftests: Add support for TDX TDCALL from guest

Isaku Yamahata (2):
  KVM: selftests: Update kvm_init_vm_address_properties() for TDX
  KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs'
    attribute configuration

Sagi Shahar (16):
  KVM: selftests: Allocate pgd in virt_map() as necessary
  KVM: selftests: Expose functions to get default sregs values
  KVM: selftests: Expose function to allocate guest vCPU stack
  KVM: selftests: Expose segment definitons to assembly files
  KVM: selftests: Add kbuild definitons
  KVM: selftests: Define structs to pass parameters to TDX boot code
  KVM: selftests: Set up TDX boot code region
  KVM: selftests: Set up TDX boot parameters region
  KVM: selftests: Add helper to initialize TDX VM
  KVM: selftests: Call TDX init when creating a new TDX vm
  KVM: selftests: Setup memory regions for TDX on vm creation
  KVM: selftests: Call KVM_TDX_INIT_VCPU when creating a new TDX vcpu
  KVM: selftests: Set entry point for TDX guest code
  KVM: selftests: Finalize TD memory as part of
    kvm_arch_vm_finalize_vcpus
  KVM: selftests: Add wrapper for TDX MMIO from guest
  KVM: selftests: Add TDX lifecycle test

Sean Christopherson (1):
  KVM: selftests: Add macros so simplify creating VM shapes for
    non-default types

 tools/include/linux/kbuild.h                  |  18 +
 tools/testing/selftests/kvm/Makefile.kvm      |  32 ++
 .../testing/selftests/kvm/include/kvm_util.h  |  14 +
 .../selftests/kvm/include/ucall_common.h      |   1 +
 .../selftests/kvm/include/x86/processor.h     |  40 +++
 .../selftests/kvm/include/x86/processor_asm.h |  12 +
 tools/testing/selftests/kvm/include/x86/sev.h |   2 -
 .../selftests/kvm/include/x86/tdx/td_boot.h   |  74 ++++
 .../kvm/include/x86/tdx/td_boot_asm.h         |  16 +
 .../selftests/kvm/include/x86/tdx/tdcall.h    |  34 ++
 .../selftests/kvm/include/x86/tdx/tdx.h       |  14 +
 .../selftests/kvm/include/x86/tdx/tdx_util.h  |  84 +++++
 .../testing/selftests/kvm/include/x86/ucall.h |   6 -
 tools/testing/selftests/kvm/lib/kvm_util.c    |  10 +-
 .../testing/selftests/kvm/lib/x86/processor.c |  99 ++++--
 tools/testing/selftests/kvm/lib/x86/sev.c     |  16 -
 .../selftests/kvm/lib/x86/tdx/td_boot.S       |  60 ++++
 .../kvm/lib/x86/tdx/td_boot_offsets.c         |  21 ++
 .../selftests/kvm/lib/x86/tdx/tdcall.S        |  93 +++++
 .../kvm/lib/x86/tdx/tdcall_offsets.c          |  16 +
 tools/testing/selftests/kvm/lib/x86/tdx/tdx.c |  23 ++
 .../selftests/kvm/lib/x86/tdx/tdx_util.c      | 330 ++++++++++++++++++
 tools/testing/selftests/kvm/lib/x86/ucall.c   |  46 ++-
 .../selftests/kvm/x86/sev_smoke_test.c        |  40 +--
 tools/testing/selftests/kvm/x86/tdx_vm_test.c |  33 ++
 25 files changed, 1056 insertions(+), 78 deletions(-)
 create mode 100644 tools/include/linux/kbuild.h
 create mode 100644 tools/testing/selftests/kvm/include/x86/processor_asm.h
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/td_boot_asm.h
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/tdx.h
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/td_boot_offsets.c
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
 create mode 100644 tools/testing/selftests/kvm/x86/tdx_vm_test.c

-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply	[flat|nested] 69+ messages in thread

* [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29  1:13   ` Ira Weiny
                     ` (2 more replies)
  2025-10-28 21:20 ` [PATCH v12 02/23] KVM: selftests: Allocate pgd in virt_map() as necessary Sagi Shahar
                   ` (21 subsequent siblings)
  22 siblings, 3 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

From: Sean Christopherson <seanjc@google.com>

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>
Signed-off-by: Sagi Shahar <sagis@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 d3f3e455c031..310ec2b8afb7 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -209,6 +209,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;
 }
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 02/23] KVM: selftests: Allocate pgd in virt_map() as necessary
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
  2025-10-28 21:20 ` [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-31  3:51   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values Sagi Shahar
                   ` (20 subsequent siblings)
  22 siblings, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

If virt_map() is called before any call to ____vm_vaddr_alloc() it
will create the mapping using an invalid pgd.

Add call to virt_pgd_alloc() as part of virt_map() before creating the
mapping, similarly to ____vm_vaddr_alloc()

Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 1a93d6361671..0e6a487ca7a4 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -1569,6 +1569,7 @@ void virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
 	TEST_ASSERT(vaddr + size > vaddr, "Vaddr overflow");
 	TEST_ASSERT(paddr + size > paddr, "Paddr overflow");
 
+	virt_pgd_alloc(vm);
 	while (npages--) {
 		virt_pg_map(vm, vaddr, paddr);
 		sparsebit_set(vm->vpages_mapped, vaddr >> vm->page_shift);
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
  2025-10-28 21:20 ` [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types Sagi Shahar
  2025-10-28 21:20 ` [PATCH v12 02/23] KVM: selftests: Allocate pgd in virt_map() as necessary Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29  1:40   ` Ira Weiny
  2025-10-31  3:43   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack Sagi Shahar
                   ` (19 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

TDX can't set sregs values directly using KVM_SET_SREGS. Expose the
default values of certain sregs used by TDX VMs so they can be set
manually.

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/include/x86/processor.h     | 33 +++++++++++++++++++
 .../testing/selftests/kvm/lib/x86/processor.c | 12 +++----
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index dd21e11e1908..9caeb3de7df6 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -27,6 +27,10 @@ extern uint64_t guest_tsc_khz;
 #define MAX_NR_CPUID_ENTRIES 100
 #endif
 
+#ifndef NUM_INTERRUPTS
+#define NUM_INTERRUPTS 256
+#endif
+
 #define NONCANONICAL 0xaaaaaaaaaaaaaaaaull
 
 /* Forced emulation prefix, used to invoke the emulator unconditionally. */
@@ -1498,4 +1502,33 @@ void virt_map_level(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
 
 bool sys_clocksource_is_based_on_tsc(void);
 
+static inline uint16_t kvm_get_default_idt_limit(void)
+{
+	return NUM_INTERRUPTS * sizeof(struct idt_entry) - 1;
+}
+
+static inline uint16_t kvm_get_default_gdt_limit(void)
+{
+	return getpagesize() - 1;
+}
+
+static inline uint64_t kvm_get_default_cr0(void)
+{
+	return X86_CR0_PE | X86_CR0_NE | X86_CR0_PG;
+}
+
+static inline uint64_t kvm_get_default_cr4(void)
+{
+	uint64_t cr4 = X86_CR4_PAE | X86_CR4_OSFXSR;
+
+	if (kvm_cpu_has(X86_FEATURE_XSAVE))
+		cr4 |= X86_CR4_OSXSAVE;
+	return cr4;
+}
+
+static inline uint64_t kvm_get_default_efer(void)
+{
+	return EFER_LME | EFER_LMA | EFER_NX;
+}
+
 #endif /* SELFTEST_KVM_PROCESSOR_H */
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index b418502c5ecc..2d1544e8af6c 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -532,15 +532,13 @@ static void vcpu_init_sregs(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
 	vcpu_sregs_get(vcpu, &sregs);
 
 	sregs.idt.base = vm->arch.idt;
-	sregs.idt.limit = NUM_INTERRUPTS * sizeof(struct idt_entry) - 1;
+	sregs.idt.limit = kvm_get_default_idt_limit();
 	sregs.gdt.base = vm->arch.gdt;
-	sregs.gdt.limit = getpagesize() - 1;
+	sregs.gdt.limit = kvm_get_default_gdt_limit();
 
-	sregs.cr0 = X86_CR0_PE | X86_CR0_NE | X86_CR0_PG;
-	sregs.cr4 |= X86_CR4_PAE | X86_CR4_OSFXSR;
-	if (kvm_cpu_has(X86_FEATURE_XSAVE))
-		sregs.cr4 |= X86_CR4_OSXSAVE;
-	sregs.efer |= (EFER_LME | EFER_LMA | EFER_NX);
+	sregs.cr0 = kvm_get_default_cr0();
+	sregs.cr4 |= kvm_get_default_cr4();
+	sregs.efer |= kvm_get_default_efer();
 
 	kvm_seg_set_unusable(&sregs.ldt);
 	kvm_seg_set_kernel_code_64bit(&sregs.cs);
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (2 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29 13:24   ` Ira Weiny
  2025-10-31  3:52   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX Sagi Shahar
                   ` (18 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

TDX guests' registers cannot be initialized directly using
vcpu_regs_set(), hence the stack pointer needs to be initialized by
the guest itself, running boot code beginning at the reset vector.

Expose the function to allocate the guest stack so that TDX
initialization code can allocate it itself and skip the allocation in
vm_arch_vcpu_add() in that case.

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/include/x86/processor.h        |  2 ++
 tools/testing/selftests/kvm/lib/x86/processor.c  | 16 +++++++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index 9caeb3de7df6..dba2b3d558d1 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -1120,6 +1120,8 @@ static inline void vcpu_clear_cpuid_feature(struct kvm_vcpu *vcpu,
 	vcpu_set_or_clear_cpuid_feature(vcpu, feature, false);
 }
 
+vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm);
+
 uint64_t vcpu_get_msr(struct kvm_vcpu *vcpu, uint64_t msr_index);
 int _vcpu_set_msr(struct kvm_vcpu *vcpu, uint64_t msr_index, uint64_t msr_value);
 
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 2d1544e8af6c..2898fe4f6de4 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -693,12 +693,9 @@ void vcpu_arch_set_entry_point(struct kvm_vcpu *vcpu, void *guest_code)
 	vcpu_regs_set(vcpu, &regs);
 }
 
-struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
+vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm)
 {
-	struct kvm_mp_state mp_state;
-	struct kvm_regs regs;
 	vm_vaddr_t stack_vaddr;
-	struct kvm_vcpu *vcpu;
 
 	stack_vaddr = __vm_vaddr_alloc(vm, DEFAULT_STACK_PGS * getpagesize(),
 				       DEFAULT_GUEST_STACK_VADDR_MIN,
@@ -719,6 +716,15 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
 		    "__vm_vaddr_alloc() did not provide a page-aligned address");
 	stack_vaddr -= 8;
 
+	return stack_vaddr;
+}
+
+struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
+{
+	struct kvm_mp_state mp_state;
+	struct kvm_regs regs;
+	struct kvm_vcpu *vcpu;
+
 	vcpu = __vm_vcpu_add(vm, vcpu_id);
 	vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
 	vcpu_init_sregs(vm, vcpu);
@@ -727,7 +733,7 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
 	/* Setup guest general purpose registers */
 	vcpu_regs_get(vcpu, &regs);
 	regs.rflags = regs.rflags | 0x2;
-	regs.rsp = stack_vaddr;
+	regs.rsp = kvm_allocate_vcpu_stack(vm);
 	vcpu_regs_set(vcpu, &regs);
 
 	/* Setup the MP state */
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (3 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29 15:22   ` Ira Weiny
  2025-10-31  3:53   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 06/23] KVM: selftests: Expose segment definitons to assembly files Sagi Shahar
                   ` (17 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm, Adrian Hunter

From: Isaku Yamahata <isaku.yamahata@intel.com>

Let kvm_init_vm_address_properties() initialize vm->arch.{s_bit, tag_mask}
similar to SEV.

TDX sets the shared bit based on the guest physical address width and
currently supports 48 and 52 widths.

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Co-developed-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/include/x86/tdx/tdx_util.h       | 14 ++++++++++++++
 tools/testing/selftests/kvm/lib/x86/processor.c    | 12 ++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h

diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
new file mode 100644
index 000000000000..286d5e3c24b1
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTESTS_TDX_TDX_UTIL_H
+#define SELFTESTS_TDX_TDX_UTIL_H
+
+#include <stdbool.h>
+
+#include "kvm_util.h"
+
+static inline bool is_tdx_vm(struct kvm_vm *vm)
+{
+	return vm->type == KVM_X86_TDX_VM;
+}
+
+#endif // SELFTESTS_TDX_TDX_UTIL_H
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 2898fe4f6de4..519d60a3827c 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -9,6 +9,7 @@
 #include "pmu.h"
 #include "processor.h"
 #include "sev.h"
+#include "tdx/tdx_util.h"
 
 #ifndef NUM_INTERRUPTS
 #define NUM_INTERRUPTS 256
@@ -1195,12 +1196,19 @@ void kvm_get_cpu_address_width(unsigned int *pa_bits, unsigned int *va_bits)
 
 void kvm_init_vm_address_properties(struct kvm_vm *vm)
 {
+	uint32_t gpa_bits = kvm_cpu_property(X86_PROPERTY_GUEST_MAX_PHY_ADDR);
+
+	vm->arch.sev_fd = -1;
+
 	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;
-	} else {
-		vm->arch.sev_fd = -1;
+	} else if (is_tdx_vm(vm)) {
+		TEST_ASSERT(gpa_bits == 48 || gpa_bits == 52,
+			    "TDX: bad X86_PROPERTY_GUEST_MAX_PHY_ADDR value: %u", gpa_bits);
+		vm->arch.s_bit = BIT_ULL(gpa_bits - 1);
+		vm->gpa_tag_mask = vm->arch.s_bit;
 	}
 }
 
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 06/23] KVM: selftests: Expose segment definitons to assembly files
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (4 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-31  3:54   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 07/23] KVM: selftests: Add kbuild definitons Sagi Shahar
                   ` (16 subsequent siblings)
  22 siblings, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Move kernel segment definitions to a separate file which can be included
from assembly files.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/include/x86/processor_asm.h        | 12 ++++++++++++
 tools/testing/selftests/kvm/lib/x86/processor.c      |  5 +----
 2 files changed, 13 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/include/x86/processor_asm.h

diff --git a/tools/testing/selftests/kvm/include/x86/processor_asm.h b/tools/testing/selftests/kvm/include/x86/processor_asm.h
new file mode 100644
index 000000000000..7e5386a85ca8
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86/processor_asm.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Used for storing defines used by both processor.c and assembly code.
+ */
+#ifndef SELFTEST_KVM_PROCESSOR_ASM_H
+#define SELFTEST_KVM_PROCESSOR_ASM_H
+
+#define KERNEL_CS	0x8
+#define KERNEL_DS	0x10
+#define KERNEL_TSS	0x18
+
+#endif  // SELFTEST_KVM_PROCESSOR_ASM_H
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 519d60a3827c..5f75bd48623b 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -8,6 +8,7 @@
 #include "kvm_util.h"
 #include "pmu.h"
 #include "processor.h"
+#include "processor_asm.h"
 #include "sev.h"
 #include "tdx/tdx_util.h"
 
@@ -15,10 +16,6 @@
 #define NUM_INTERRUPTS 256
 #endif
 
-#define KERNEL_CS	0x8
-#define KERNEL_DS	0x10
-#define KERNEL_TSS	0x18
-
 vm_vaddr_t exception_handlers;
 bool host_cpu_is_amd;
 bool host_cpu_is_intel;
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 07/23] KVM: selftests: Add kbuild definitons
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (5 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 06/23] KVM: selftests: Expose segment definitons to assembly files Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29  7:43   ` Binbin Wu
                     ` (2 more replies)
  2025-10-28 21:20 ` [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code Sagi Shahar
                   ` (15 subsequent siblings)
  22 siblings, 3 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Add kbuild.h that can be used by files under tools/

Definitions are taken from the original definitions at
include/linux/kbuild.h

This is needed to expose values from c code to assembly code.

Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/include/linux/kbuild.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 tools/include/linux/kbuild.h

diff --git a/tools/include/linux/kbuild.h b/tools/include/linux/kbuild.h
new file mode 100644
index 000000000000..62e20ba9380e
--- /dev/null
+++ b/tools/include/linux/kbuild.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TOOLS_LINUX_KBUILD_H
+#define __TOOLS_LINUX_KBUILD_H
+
+#include <stddef.h>
+
+#define DEFINE(sym, val) \
+	asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))
+
+#define BLANK() asm volatile("\n.ascii \"->\"" : : )
+
+#define OFFSET(sym, str, mem) \
+	DEFINE(sym, offsetof(struct str, mem))
+
+#define COMMENT(x) \
+	asm volatile("\n.ascii \"->#" x "\"")
+
+#endif /* __TOOLS_LINUX_KBUILD_H */
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (6 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 07/23] KVM: selftests: Add kbuild definitons Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29 16:37   ` Reinette Chatre
  2025-10-31  4:01   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 09/23] KVM: selftests: Add " Sagi Shahar
                   ` (14 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

TDX registers are inaccessible to KVM. Therefore we need a different
mechanism to load boot parameters for TDX code. TDX boot code will read
the registers values from memory and set the registers manually.

This patch defines the data structures used to communicate between c
code and the TDX assembly boot code which will be added in a later
patch.

Use kbuild.h to expose the offsets into the structs from c code to
assembly code.

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Co-developed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      | 18 +++++
 .../selftests/kvm/include/x86/tdx/td_boot.h   | 69 +++++++++++++++++++
 .../kvm/lib/x86/tdx/td_boot_offsets.c         | 21 ++++++
 3 files changed, 108 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/td_boot_offsets.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 148d427ff24b..5e809064ff1c 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -19,6 +19,8 @@ LIBKVM += lib/userfaultfd_util.c
 
 LIBKVM_STRING += lib/string_override.c
 
+LIBKVM_ASM_DEFS += lib/x86/tdx/td_boot_offsets.c
+
 LIBKVM_x86 += lib/x86/apic.c
 LIBKVM_x86 += lib/x86/handlers.S
 LIBKVM_x86 += lib/x86/hyperv.c
@@ -239,6 +241,10 @@ OVERRIDE_TARGETS = 1
 include ../lib.mk
 include ../cgroup/lib/libcgroup.mk
 
+# Enable Kbuild tools.
+include $(top_srcdir)/scripts/Kbuild.include
+include $(top_srcdir)/scripts/Makefile.lib
+
 INSTALL_HDR_PATH = $(top_srcdir)/usr
 LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
 LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include
@@ -291,6 +297,7 @@ LIBKVM_S := $(filter %.S,$(LIBKVM))
 LIBKVM_C_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_C))
 LIBKVM_S_OBJ := $(patsubst %.S, $(OUTPUT)/%.o, $(LIBKVM_S))
 LIBKVM_STRING_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_STRING))
+LIBKVM_ASM_DEFS_OBJ += $(patsubst %.c, $(OUTPUT)/%.s, $(LIBKVM_ASM_DEFS))
 LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ) $(LIBKVM_STRING_OBJ) $(LIBCGROUP_O)
 SPLIT_TEST_GEN_PROGS := $(patsubst %, $(OUTPUT)/%, $(SPLIT_TESTS))
 SPLIT_TEST_GEN_OBJ := $(patsubst %, $(OUTPUT)/$(ARCH)/%.o, $(SPLIT_TESTS))
@@ -317,6 +324,7 @@ $(SPLIT_TEST_GEN_OBJ): $(OUTPUT)/$(ARCH)/%.o: $(ARCH)/%.c
 
 EXTRA_CLEAN += $(GEN_HDRS) \
 	       $(LIBKVM_OBJS) \
+	       $(LIBKVM_ASM_DEFS_OBJ) \
 	       $(SPLIT_TEST_GEN_OBJ) \
 	       $(TEST_DEP_FILES) \
 	       $(TEST_GEN_OBJ) \
@@ -328,18 +336,28 @@ $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c $(GEN_HDRS)
 $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S $(GEN_HDRS)
 	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
 
+$(LIBKVM_ASM_DEFS_OBJ): $(OUTPUT)/%.s: %.c FORCE
+	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -S $< -o $@
+
 # Compile the string overrides as freestanding to prevent the compiler from
 # generating self-referential code, e.g. without "freestanding" the compiler may
 # "optimize" memcmp() by invoking memcmp(), thus causing infinite recursion.
 $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c
 	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@
 
+$(OUTPUT)/include/x86/tdx/td_boot_offsets.h: $(OUTPUT)/lib/x86/tdx/td_boot_offsets.s FORCE
+	$(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)
+
+EXTRA_CLEAN += $(OUTPUT)/include/x86/tdx/td_boot_offsets.h
+
 $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS))))
 $(SPLIT_TEST_GEN_OBJ): $(GEN_HDRS)
 $(TEST_GEN_PROGS): $(LIBKVM_OBJS)
 $(TEST_GEN_PROGS_EXTENDED): $(LIBKVM_OBJS)
 $(TEST_GEN_OBJ): $(GEN_HDRS)
 
+FORCE:
+
 cscope: include_paths = $(LINUX_TOOL_INCLUDE) $(LINUX_HDR_PATH) include lib ..
 cscope:
 	$(RM) cscope.*
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h b/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
new file mode 100644
index 000000000000..32631645fe13
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTEST_TDX_TD_BOOT_H
+#define SELFTEST_TDX_TD_BOOT_H
+
+#include <stdint.h>
+
+#include <linux/compiler.h>
+#include <linux/sizes.h>
+
+/*
+ * Layout for boot section (not to scale)
+ *
+ *                                   GPA
+ * _________________________________ 0x1_0000_0000 (4GB)
+ * |   Boot code trampoline    |
+ * |___________________________|____ 0x0_ffff_fff0: Reset vector (16B below 4GB)
+ * |   Boot code               |
+ * |___________________________|____ td_boot will be copied here, so that the
+ * |                           |     jmp to td_boot is exactly at the reset vector
+ * |   Empty space             |
+ * |                           |
+ * |───────────────────────────|
+ * |                           |
+ * |                           |
+ * |   Boot parameters         |
+ * |                           |
+ * |                           |
+ * |___________________________|____ 0x0_ffff_0000: TD_BOOT_PARAMETERS_GPA
+ */
+#define FOUR_GIGABYTES_GPA (SZ_4G)
+
+/*
+ * The exact memory layout for LGDT or LIDT instructions.
+ */
+struct __packed td_boot_parameters_dtr {
+	uint16_t limit;
+	uint32_t base;
+};
+
+/*
+ * Allows each vCPU to be initialized with different rip and esp.
+ */
+struct td_per_vcpu_parameters {
+	uint32_t esp_gva;
+	uint64_t guest_code;
+};
+
+/*
+ * Boot parameters for the TD.
+ *
+ * Unlike a regular VM, KVM cannot set registers such as esp, eip, etc
+ * before boot, so to run selftests, these registers' values have to be
+ * initialized by the TD.
+ *
+ * This struct is loaded in TD private memory at TD_BOOT_PARAMETERS_GPA.
+ *
+ * The TD boot code will read off parameters from this struct and set up the
+ * vCPU for executing selftests.
+ */
+struct td_boot_parameters {
+	uint32_t cr0;
+	uint32_t cr3;
+	uint32_t cr4;
+	struct td_boot_parameters_dtr gdtr;
+	struct td_boot_parameters_dtr idtr;
+	struct td_per_vcpu_parameters per_vcpu[];
+};
+
+#endif /* SELFTEST_TDX_TD_BOOT_H */
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/td_boot_offsets.c b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot_offsets.c
new file mode 100644
index 000000000000..7f76a3585b99
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot_offsets.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+#define COMPILE_OFFSETS
+
+#include <linux/kbuild.h>
+
+#include "tdx/td_boot.h"
+
+static void __attribute__((used)) common(void)
+{
+	OFFSET(TD_BOOT_PARAMETERS_CR0, td_boot_parameters, cr0);
+	OFFSET(TD_BOOT_PARAMETERS_CR3, td_boot_parameters, cr3);
+	OFFSET(TD_BOOT_PARAMETERS_CR4, td_boot_parameters, cr4);
+	OFFSET(TD_BOOT_PARAMETERS_GDT, td_boot_parameters, gdtr);
+	OFFSET(TD_BOOT_PARAMETERS_IDT, td_boot_parameters, idtr);
+	OFFSET(TD_BOOT_PARAMETERS_PER_VCPU, td_boot_parameters, per_vcpu);
+	OFFSET(TD_PER_VCPU_PARAMETERS_ESP_GVA, td_per_vcpu_parameters, esp_gva);
+	OFFSET(TD_PER_VCPU_PARAMETERS_GUEST_CODE, td_per_vcpu_parameters,
+	       guest_code);
+	DEFINE(SIZEOF_TD_PER_VCPU_PARAMETERS,
+	       sizeof(struct td_per_vcpu_parameters));
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 09/23] KVM: selftests: Add TDX boot code
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (7 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-28 21:20 ` [PATCH v12 10/23] KVM: selftests: Set up TDX boot code region Sagi Shahar
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

From: Erdem Aktas <erdemaktas@google.com>

Add code to boot a TDX test VM. Since TDX registers are inaccessible to
KVM, the boot code loads the relevant values from memory into the
registers before jumping to the guest code.

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Co-developed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |  3 +
 .../selftests/kvm/include/x86/tdx/td_boot.h   |  5 ++
 .../kvm/include/x86/tdx/td_boot_asm.h         | 16 +++++
 .../selftests/kvm/lib/x86/tdx/td_boot.S       | 60 +++++++++++++++++++
 4 files changed, 84 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/td_boot_asm.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 5e809064ff1c..5c94e3afcd3a 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -31,6 +31,7 @@ LIBKVM_x86 += lib/x86/sev.c
 LIBKVM_x86 += lib/x86/svm.c
 LIBKVM_x86 += lib/x86/ucall.c
 LIBKVM_x86 += lib/x86/vmx.c
+LIBKVM_x86 += lib/x86/tdx/td_boot.S
 
 LIBKVM_arm64 += lib/arm64/gic.c
 LIBKVM_arm64 += lib/arm64/gic_v3.c
@@ -345,6 +346,8 @@ $(LIBKVM_ASM_DEFS_OBJ): $(OUTPUT)/%.s: %.c FORCE
 $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c
 	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@
 
+$(OUTPUT)/lib/x86/tdx/td_boot.o: $(OUTPUT)/include/x86/tdx/td_boot_offsets.h
+
 $(OUTPUT)/include/x86/tdx/td_boot_offsets.h: $(OUTPUT)/lib/x86/tdx/td_boot_offsets.s FORCE
 	$(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)
 
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h b/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
index 32631645fe13..a590516dd83c 100644
--- a/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
+++ b/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
@@ -66,4 +66,9 @@ struct td_boot_parameters {
 	struct td_per_vcpu_parameters per_vcpu[];
 };
 
+void td_boot(void);
+void td_boot_code_end(void);
+
+#define TD_BOOT_CODE_SIZE (td_boot_code_end - td_boot)
+
 #endif /* SELFTEST_TDX_TD_BOOT_H */
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/td_boot_asm.h b/tools/testing/selftests/kvm/include/x86/tdx/td_boot_asm.h
new file mode 100644
index 000000000000..10b4b527595c
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86/tdx/td_boot_asm.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTEST_TDX_TD_BOOT_ASM_H
+#define SELFTEST_TDX_TD_BOOT_ASM_H
+
+/*
+ * GPA where TD boot parameters will be loaded.
+ *
+ * TD_BOOT_PARAMETERS_GPA is arbitrarily chosen to
+ *
+ * + be within the 4GB address space
+ * + provide enough contiguous memory for the struct td_boot_parameters such
+ *   that there is one struct td_per_vcpu_parameters for KVM_MAX_VCPUS
+ */
+#define TD_BOOT_PARAMETERS_GPA 0xffff0000
+
+#endif  // SELFTEST_TDX_TD_BOOT_ASM_H
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S
new file mode 100644
index 000000000000..7aa33caa9a78
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include "tdx/td_boot_asm.h"
+#include "tdx/td_boot_offsets.h"
+#include "processor_asm.h"
+
+.code32
+
+.globl td_boot
+td_boot:
+	/* In this procedure, edi is used as a temporary register. */
+	cli
+
+	/* Paging is off. */
+
+	movl $TD_BOOT_PARAMETERS_GPA, %ebx
+
+	/*
+	 * Find the address of struct td_per_vcpu_parameters for this
+	 * vCPU based on esi (TDX spec: initialized with vCPU id). Put
+	 * struct address into register for indirect addressing.
+	 */
+	movl $SIZEOF_TD_PER_VCPU_PARAMETERS, %eax
+	mul %esi
+	leal TD_BOOT_PARAMETERS_PER_VCPU(%ebx), %edi
+	addl %edi, %eax
+
+	/* Setup stack. */
+	movl TD_PER_VCPU_PARAMETERS_ESP_GVA(%eax), %esp
+
+	/* Setup GDT. */
+	leal TD_BOOT_PARAMETERS_GDT(%ebx), %edi
+	lgdt (%edi)
+
+	/* Setup IDT. */
+	leal TD_BOOT_PARAMETERS_IDT(%ebx), %edi
+	lidt (%edi)
+
+	/*
+	 * Set up control registers (There are no instructions to mov from
+	 * memory to control registers, hence use edi as a scratch register).
+	 */
+	movl TD_BOOT_PARAMETERS_CR4(%ebx), %edi
+	movl %edi, %cr4
+	movl TD_BOOT_PARAMETERS_CR3(%ebx), %edi
+	movl %edi, %cr3
+	movl TD_BOOT_PARAMETERS_CR0(%ebx), %edi
+	movl %edi, %cr0
+
+	/* Switching to 64bit mode after ljmp and then jump to guest code */
+	ljmp $(KERNEL_CS),$1f
+1:
+	jmp *TD_PER_VCPU_PARAMETERS_GUEST_CODE(%eax)
+
+/* Leave marker so size of td_boot code can be computed. */
+.globl td_boot_code_end
+td_boot_code_end:
+
+/* Disable executable stack. */
+.section .note.GNU-stack,"",%progbits
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 10/23] KVM: selftests: Set up TDX boot code region
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (8 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 09/23] KVM: selftests: Add " Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-28 21:20 ` [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region Sagi Shahar
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Add memory for TDX boot code in a separate memslot.

Use virt_map() to get identity map in this memory region to allow for
seamless transition from paging disabled to paging enabled code.

Copy the boot code into the memory region and set up the reset vector
at this point. While it's possible to separate the memory allocation and
boot code initialization into separate functions, having all the
calculations for memory size and offsets in one place simplifies the
code and avoids duplications.

Handcode the reset vector as suggested by Sean Christopherson.

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |  1 +
 .../selftests/kvm/include/x86/tdx/tdx_util.h  |  2 +
 .../selftests/kvm/lib/x86/tdx/tdx_util.c      | 54 +++++++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 5c94e3afcd3a..86fe629f2e81 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -31,6 +31,7 @@ LIBKVM_x86 += lib/x86/sev.c
 LIBKVM_x86 += lib/x86/svm.c
 LIBKVM_x86 += lib/x86/ucall.c
 LIBKVM_x86 += lib/x86/vmx.c
+LIBKVM_x86 += lib/x86/tdx/tdx_util.c
 LIBKVM_x86 += lib/x86/tdx/td_boot.S
 
 LIBKVM_arm64 += lib/arm64/gic.c
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 286d5e3c24b1..ec05bcd59145 100644
--- a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
@@ -11,4 +11,6 @@ static inline bool is_tdx_vm(struct kvm_vm *vm)
 	return vm->type == KVM_X86_TDX_VM;
 }
 
+void vm_tdx_setup_boot_code_region(struct kvm_vm *vm);
+
 #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
new file mode 100644
index 000000000000..a1cf12de9d56
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <stdint.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "tdx/td_boot.h"
+#include "tdx/tdx_util.h"
+
+/* Arbitrarily selected to avoid overlaps with anything else */
+#define TD_BOOT_CODE_SLOT	20
+
+#define X86_RESET_VECTOR	0xfffffff0ul
+#define X86_RESET_VECTOR_SIZE	16
+
+void vm_tdx_setup_boot_code_region(struct kvm_vm *vm)
+{
+	size_t total_code_size = TD_BOOT_CODE_SIZE + X86_RESET_VECTOR_SIZE;
+	vm_paddr_t boot_code_gpa = X86_RESET_VECTOR - TD_BOOT_CODE_SIZE;
+	vm_paddr_t alloc_gpa = round_down(boot_code_gpa, PAGE_SIZE);
+	size_t nr_pages = DIV_ROUND_UP(total_code_size, PAGE_SIZE);
+	vm_paddr_t gpa;
+	uint8_t *hva;
+
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+				    alloc_gpa,
+				    TD_BOOT_CODE_SLOT, nr_pages,
+				    KVM_MEM_GUEST_MEMFD);
+
+	gpa = vm_phy_pages_alloc(vm, nr_pages, alloc_gpa, TD_BOOT_CODE_SLOT);
+	TEST_ASSERT(gpa == alloc_gpa, "Failed vm_phy_pages_alloc\n");
+
+	virt_map(vm, alloc_gpa, alloc_gpa, nr_pages);
+	hva = addr_gpa2hva(vm, boot_code_gpa);
+	memcpy(hva, td_boot, TD_BOOT_CODE_SIZE);
+
+	hva += TD_BOOT_CODE_SIZE;
+	TEST_ASSERT(hva == addr_gpa2hva(vm, X86_RESET_VECTOR),
+		    "Expected RESET vector at hva 0x%lx, got %lx",
+		    (unsigned long)addr_gpa2hva(vm, X86_RESET_VECTOR), (unsigned long)hva);
+
+	/*
+	 * Handcode "JMP rel8" at the RESET vector to jump back to the TD boot
+	 * code, as there are only 16 bytes at the RESET vector before RIP will
+	 * wrap back to zero.  Insert a trailing int3 so that the vCPU crashes
+	 * in case the JMP somehow falls through.  Note!  The target address is
+	 * relative to the end of the instruction!
+	 */
+	TEST_ASSERT(TD_BOOT_CODE_SIZE + 2 <= 128,
+		    "TD boot code not addressable by 'JMP rel8'");
+	hva[0] = 0xeb;
+	hva[1] = 256 - 2 - TD_BOOT_CODE_SIZE;
+	hva[2] = 0xcc;
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (9 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 10/23] KVM: selftests: Set up TDX boot code region Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29  8:52   ` Binbin Wu
  2025-10-29 21:01   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM Sagi Shahar
                   ` (11 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Allocate memory for TDX boot parameters and define the utility functions
necessary to fill this memory with the boot parameters.

Co-developed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>

---------------------------------------------

Changes from v10:
 * Removed code for setting up X86_CR4_OSXMMEXCPT bit. At least for now
   it is not needed and the test pass without it.
---
 .../selftests/kvm/include/x86/tdx/tdx_util.h  |  4 ++
 .../selftests/kvm/lib/x86/tdx/tdx_util.c      | 72 +++++++++++++++++++
 2 files changed, 76 insertions(+)

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 ec05bcd59145..dafdc7e46abe 100644
--- a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
@@ -12,5 +12,9 @@ static inline bool is_tdx_vm(struct kvm_vm *vm)
 }
 
 void vm_tdx_setup_boot_code_region(struct kvm_vm *vm);
+void vm_tdx_setup_boot_parameters_region(struct kvm_vm *vm, uint32_t nr_runnable_vcpus);
+void vm_tdx_load_common_boot_parameters(struct kvm_vm *vm);
+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);
 
 #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 a1cf12de9d56..f3b69923e928 100644
--- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
@@ -5,10 +5,12 @@
 #include "kvm_util.h"
 #include "processor.h"
 #include "tdx/td_boot.h"
+#include "tdx/td_boot_asm.h"
 #include "tdx/tdx_util.h"
 
 /* Arbitrarily selected to avoid overlaps with anything else */
 #define TD_BOOT_CODE_SLOT	20
+#define TD_BOOT_PARAMETERS_SLOT	21
 
 #define X86_RESET_VECTOR	0xfffffff0ul
 #define X86_RESET_VECTOR_SIZE	16
@@ -52,3 +54,73 @@ void vm_tdx_setup_boot_code_region(struct kvm_vm *vm)
 	hva[1] = 256 - 2 - TD_BOOT_CODE_SIZE;
 	hva[2] = 0xcc;
 }
+
+void vm_tdx_setup_boot_parameters_region(struct kvm_vm *vm, uint32_t nr_runnable_vcpus)
+{
+	size_t boot_params_size =
+		sizeof(struct td_boot_parameters) +
+		nr_runnable_vcpus * sizeof(struct td_per_vcpu_parameters);
+	int npages = DIV_ROUND_UP(boot_params_size, PAGE_SIZE);
+	vm_paddr_t gpa;
+
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+				    TD_BOOT_PARAMETERS_GPA,
+				    TD_BOOT_PARAMETERS_SLOT, npages,
+				    KVM_MEM_GUEST_MEMFD);
+	gpa = vm_phy_pages_alloc(vm, npages, TD_BOOT_PARAMETERS_GPA, TD_BOOT_PARAMETERS_SLOT);
+	TEST_ASSERT(gpa == TD_BOOT_PARAMETERS_GPA, "Failed vm_phy_pages_alloc\n");
+
+	virt_map(vm, TD_BOOT_PARAMETERS_GPA, TD_BOOT_PARAMETERS_GPA, npages);
+}
+
+void vm_tdx_load_common_boot_parameters(struct kvm_vm *vm)
+{
+	struct td_boot_parameters *params =
+		addr_gpa2hva(vm, TD_BOOT_PARAMETERS_GPA);
+	uint32_t cr4;
+
+	TEST_ASSERT_EQ(vm->mode, VM_MODE_PXXV48_4K);
+
+	cr4 = kvm_get_default_cr4();
+
+	/* TDX spec 11.6.2: CR4 bit MCE is fixed to 1 */
+	cr4 |= X86_CR4_MCE;
+
+	/* TDX spec 11.6.2: CR4 bit VMXE and SMXE are fixed to 0 */
+	cr4 &= ~(X86_CR4_VMXE | X86_CR4_SMXE);
+
+	/* Set parameters! */
+	params->cr0 = kvm_get_default_cr0();
+	params->cr3 = vm->pgd;
+	params->cr4 = cr4;
+	params->idtr.base = vm->arch.idt;
+	params->idtr.limit = kvm_get_default_idt_limit();
+	params->gdtr.base = vm->arch.gdt;
+	params->gdtr.limit = kvm_get_default_gdt_limit();
+
+	TEST_ASSERT(params->cr0 != 0, "cr0 should not be 0");
+	TEST_ASSERT(params->cr3 != 0, "cr3 should not be 0");
+	TEST_ASSERT(params->cr4 != 0, "cr4 should not be 0");
+	TEST_ASSERT(params->gdtr.base != 0, "gdt base address should not be 0");
+	TEST_ASSERT(params->idtr.base != 0, "idt base address should not be 0");
+}
+
+void vm_tdx_load_vcpu_boot_parameters(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
+{
+	struct td_boot_parameters *params =
+		addr_gpa2hva(vm, TD_BOOT_PARAMETERS_GPA);
+	struct td_per_vcpu_parameters *vcpu_params =
+		&params->per_vcpu[vcpu->id];
+
+	vcpu_params->esp_gva = kvm_allocate_vcpu_stack(vm);
+}
+
+void vm_tdx_set_vcpu_entry_point(struct kvm_vcpu *vcpu, void *guest_code)
+{
+	struct td_boot_parameters *params =
+		addr_gpa2hva(vcpu->vm, TD_BOOT_PARAMETERS_GPA);
+	struct td_per_vcpu_parameters *vcpu_params =
+		&params->per_vcpu[vcpu->id];
+
+	vcpu_params->guest_code = (uint64_t)guest_code;
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (10 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29 21:16   ` Ira Weiny
                     ` (2 more replies)
  2025-10-28 21:20 ` [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration Sagi Shahar
                   ` (10 subsequent siblings)
  22 siblings, 3 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

KVM_TDX_INIT_VM needs to be called after KVM_CREATE_VM and before
creating any VCPUs, thus before KVM_SET_CPUID2. KVM_TDX_INIT_VM accepts
the CPUID values directly.

Since KVM_GET_CPUID2 can't be used at this point, calculate the CPUID
values manually by using kvm_get_supported_cpuid() and filter the
returned CPUIDs against the supported CPUID values read from the TDX
module.

Co-developed-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/include/x86/tdx/tdx_util.h  |  54 +++++++
 .../selftests/kvm/lib/x86/tdx/tdx_util.c      | 132 ++++++++++++++++++
 2 files changed, 186 insertions(+)

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 dafdc7e46abe..a2509959c7ce 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,60 @@ static inline bool is_tdx_vm(struct kvm_vm *vm)
 	return vm->type == KVM_X86_TDX_VM;
 }
 
+/*
+ * TDX ioctls
+ */
+
+#define __vm_tdx_vm_ioctl(vm, cmd, metadata, arg)			\
+({									\
+	int r;								\
+									\
+	union {								\
+		struct kvm_tdx_cmd c;					\
+		unsigned long raw;					\
+	} tdx_cmd = { .c = {						\
+		.id = (cmd),						\
+		.flags = (uint32_t)(metadata),				\
+		.data = (uint64_t)(arg),				\
+	} };								\
+									\
+	r = __vm_ioctl(vm, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);	\
+	r ?: tdx_cmd.c.hw_error;					\
+})
+
+#define vm_tdx_vm_ioctl(vm, cmd, flags, arg)				\
+({									\
+	int ret = __vm_tdx_vm_ioctl(vm, cmd, flags, arg);		\
+									\
+	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd,	ret, vm);		\
+})
+
+#define __vm_tdx_vcpu_ioctl(vcpu, cmd, metadata, arg)			\
+({									\
+	int r;								\
+									\
+	union {								\
+		struct kvm_tdx_cmd c;					\
+		unsigned long raw;					\
+	} tdx_cmd = { .c = {						\
+		.id = (cmd),						\
+		.flags = (uint32_t)(metadata),				\
+		.data = (uint64_t)(arg),				\
+	} };								\
+									\
+	r = __vcpu_ioctl(vcpu, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);	\
+	r ?: tdx_cmd.c.hw_error;					\
+})
+
+#define vm_tdx_vcpu_ioctl(vcpu, cmd, flags, arg)			\
+({									\
+	int ret = __vm_tdx_vcpu_ioctl(vcpu, cmd, flags, arg);		\
+									\
+	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, (vcpu)->vm);	\
+})
+
+void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes);
+
 void vm_tdx_setup_boot_code_region(struct kvm_vm *vm);
 void vm_tdx_setup_boot_parameters_region(struct kvm_vm *vm, uint32_t nr_runnable_vcpus);
 void vm_tdx_load_common_boot_parameters(struct kvm_vm *vm);
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 f3b69923e928..7a622b4810b1 100644
--- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
@@ -124,3 +124,135 @@ void vm_tdx_set_vcpu_entry_point(struct kvm_vcpu *vcpu, void *guest_code)
 
 	vcpu_params->guest_code = (uint64_t)guest_code;
 }
+
+static struct kvm_tdx_capabilities *tdx_read_capabilities(struct kvm_vm *vm)
+{
+	struct kvm_tdx_capabilities *tdx_cap = NULL;
+	int nr_cpuid_configs = 4;
+	int rc = -1;
+	int i;
+
+	do {
+		nr_cpuid_configs *= 2;
+
+		tdx_cap = realloc(tdx_cap, sizeof(*tdx_cap) +
+					   sizeof(tdx_cap->cpuid) +
+					   (sizeof(struct kvm_cpuid_entry2) * nr_cpuid_configs));
+		TEST_ASSERT(tdx_cap,
+			    "Could not allocate memory for tdx capability nr_cpuid_configs %d\n",
+			    nr_cpuid_configs);
+
+		tdx_cap->cpuid.nent = nr_cpuid_configs;
+		rc = __vm_tdx_vm_ioctl(vm, KVM_TDX_CAPABILITIES, 0, tdx_cap);
+	} while (rc < 0 && errno == E2BIG);
+
+	TEST_ASSERT(rc == 0, "KVM_TDX_CAPABILITIES failed: %d %d",
+		    rc, errno);
+
+	pr_debug("tdx_cap: supported_attrs: 0x%016llx\n"
+		 "tdx_cap: supported_xfam 0x%016llx\n",
+		 tdx_cap->supported_attrs, tdx_cap->supported_xfam);
+
+	for (i = 0; i < tdx_cap->cpuid.nent; i++) {
+		const struct kvm_cpuid_entry2 *config = &tdx_cap->cpuid.entries[i];
+
+		pr_debug("cpuid config[%d]: leaf 0x%x sub_leaf 0x%x eax 0x%08x ebx 0x%08x ecx 0x%08x edx 0x%08x\n",
+			 i, config->function, config->index,
+			 config->eax, config->ebx, config->ecx, config->edx);
+	}
+
+	return tdx_cap;
+}
+
+static struct kvm_cpuid_entry2 *tdx_find_cpuid_config(struct kvm_tdx_capabilities *cap,
+						      uint32_t leaf, uint32_t sub_leaf)
+{
+	struct kvm_cpuid_entry2 *config;
+	uint32_t i;
+
+	for (i = 0; i < cap->cpuid.nent; i++) {
+		config = &cap->cpuid.entries[i];
+
+		if (config->function == leaf && config->index == sub_leaf)
+			return config;
+	}
+
+	return NULL;
+}
+
+/*
+ * Filter CPUID based on TDX supported capabilities
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   cpuid_data - CPUID fileds to filter
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * For each CPUID leaf, filter out non-supported bits based on the capabilities reported
+ * by the TDX module
+ */
+static void vm_tdx_filter_cpuid(struct kvm_vm *vm,
+				struct kvm_cpuid2 *cpuid_data)
+{
+	struct kvm_tdx_capabilities *tdx_cap;
+	struct kvm_cpuid_entry2 *config;
+	struct kvm_cpuid_entry2 *e;
+	int i;
+
+	tdx_cap = tdx_read_capabilities(vm);
+
+	i = 0;
+	while (i < cpuid_data->nent) {
+		e = cpuid_data->entries + i;
+		config = tdx_find_cpuid_config(tdx_cap, e->function, e->index);
+
+		if (!config) {
+			int left = cpuid_data->nent - i - 1;
+
+			if (left > 0)
+				memmove(cpuid_data->entries + i,
+					cpuid_data->entries + i + 1,
+					sizeof(*cpuid_data->entries) * left);
+			cpuid_data->nent--;
+			continue;
+		}
+
+		e->eax &= config->eax;
+		e->ebx &= config->ebx;
+		e->ecx &= config->ecx;
+		e->edx &= config->edx;
+
+		i++;
+	}
+
+	free(tdx_cap);
+}
+
+void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
+{
+	struct kvm_tdx_init_vm *init_vm;
+	const struct kvm_cpuid2 *tmp;
+	struct kvm_cpuid2 *cpuid;
+
+	tmp = kvm_get_supported_cpuid();
+
+	cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);
+	memcpy(cpuid, tmp, kvm_cpuid2_size(tmp->nent));
+	vm_tdx_filter_cpuid(vm, cpuid);
+
+	init_vm = calloc(1, sizeof(*init_vm) +
+			 sizeof(init_vm->cpuid.entries[0]) * cpuid->nent);
+	TEST_ASSERT(init_vm, "init_vm allocation failed");
+
+	memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
+	free(cpuid);
+
+	init_vm->attributes = attributes;
+
+	vm_tdx_vm_ioctl(vm, KVM_TDX_INIT_VM, 0, init_vm);
+
+	free(init_vm);
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (11 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29 21:19   ` Ira Weiny
  2025-10-30  1:35   ` Binbin Wu
  2025-10-28 21:20 ` [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM Sagi Shahar
                   ` (9 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

From: Isaku Yamahata <isaku.yamahata@intel.com>

Make sure that all the attributes enabled by the test are reported as
supported by the TDX module.

This also exercises the KVM_TDX_CAPABILITIES ioctl.

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

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 7a622b4810b1..2551b3eac8f8 100644
--- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
@@ -231,6 +231,18 @@ static void vm_tdx_filter_cpuid(struct kvm_vm *vm,
 	free(tdx_cap);
 }
 
+static void tdx_check_attributes(struct kvm_vm *vm, uint64_t attributes)
+{
+	struct kvm_tdx_capabilities *tdx_cap;
+
+	tdx_cap = tdx_read_capabilities(vm);
+
+	/* Make sure all the attributes are reported as supported */
+	TEST_ASSERT_EQ(attributes & tdx_cap->supported_attrs, attributes);
+
+	free(tdx_cap);
+}
+
 void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
 {
 	struct kvm_tdx_init_vm *init_vm;
@@ -250,6 +262,8 @@ void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
 	memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
 	free(cpuid);
 
+	tdx_check_attributes(vm, attributes);
+
 	init_vm->attributes = attributes;
 
 	vm_tdx_vm_ioctl(vm, KVM_TDX_INIT_VM, 0, init_vm);
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (12 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29 21:27   ` Ira Weiny
                     ` (2 more replies)
  2025-10-28 21:20 ` [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm Sagi Shahar
                   ` (8 subsequent siblings)
  22 siblings, 3 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

From: Ackerley Tng <ackerleytng@google.com>

TDX protected memory needs to be measured and encrypted before it can be
used by the guest. Traverse the VM's memory regions and initialize all
the protected ranges by calling KVM_TDX_INIT_MEM_REGION.

Once all the memory is initialized, the VM can be finalized by calling
KVM_TDX_FINALIZE_VM.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../selftests/kvm/include/x86/tdx/tdx_util.h  |  2 +
 .../selftests/kvm/lib/x86/tdx/tdx_util.c      | 58 +++++++++++++++++++
 2 files changed, 60 insertions(+)

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 a2509959c7ce..2467b6c35557 100644
--- a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
@@ -71,4 +71,6 @@ void vm_tdx_load_common_boot_parameters(struct kvm_vm *vm);
 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);
+
 #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 2551b3eac8f8..53cfadeff8de 100644
--- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
@@ -270,3 +270,61 @@ void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
 
 	free(init_vm);
 }
+
+static void tdx_init_mem_region(struct kvm_vm *vm, void *source_pages,
+				uint64_t gpa, uint64_t size)
+{
+	uint32_t metadata = KVM_TDX_MEASURE_MEMORY_REGION;
+	struct kvm_tdx_init_mem_region mem_region = {
+		.source_addr = (uint64_t)source_pages,
+		.gpa = gpa,
+		.nr_pages = size / PAGE_SIZE,
+	};
+	struct kvm_vcpu *vcpu;
+
+	vcpu = list_first_entry_or_null(&vm->vcpus, struct kvm_vcpu, list);
+
+	TEST_ASSERT((mem_region.nr_pages > 0) &&
+		    ((mem_region.nr_pages * PAGE_SIZE) == size),
+		    "Cannot add partial pages to the guest memory.\n");
+	TEST_ASSERT(((uint64_t)source_pages & (PAGE_SIZE - 1)) == 0,
+		    "Source memory buffer is not page aligned\n");
+	vm_tdx_vcpu_ioctl(vcpu, KVM_TDX_INIT_MEM_REGION, metadata, &mem_region);
+}
+
+static void load_td_private_memory(struct kvm_vm *vm)
+{
+	struct userspace_mem_region *region;
+	int ctr;
+
+	hash_for_each(vm->regions.slot_hash, ctr, region, slot_node) {
+		const struct sparsebit *protected_pages = region->protected_phy_pages;
+		const vm_paddr_t gpa_base = region->region.guest_phys_addr;
+		const uint64_t hva_base = region->region.userspace_addr;
+		const sparsebit_idx_t lowest_page_in_region = gpa_base >> vm->page_shift;
+		sparsebit_idx_t i, j;
+
+		if (!sparsebit_any_set(protected_pages))
+			continue;
+
+		TEST_ASSERT(region->region.guest_memfd != -1,
+			    "TD private memory must be backed by guest_memfd");
+
+		sparsebit_for_each_set_range(protected_pages, i, j) {
+			const uint64_t size_to_load = (j - i + 1) * vm->page_size;
+			const uint64_t offset =
+				(i - lowest_page_in_region) * vm->page_size;
+			const uint64_t hva = hva_base + offset;
+			const uint64_t gpa = gpa_base + offset;
+
+			vm_mem_set_private(vm, gpa, size_to_load);
+			tdx_init_mem_region(vm, (void *)hva, gpa, size_to_load);
+		}
+	}
+}
+
+void vm_tdx_finalize(struct kvm_vm *vm)
+{
+	load_td_private_memory(vm);
+	vm_tdx_vm_ioctl(vm, KVM_TDX_FINALIZE_VM, 0, NULL);
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (13 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-30 22:20   ` Ira Weiny
  2025-10-31 16:03   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation Sagi Shahar
                   ` (7 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

TDX VMs need to issue the KVM_TDX_INIT_VM ioctl after VM creation to
initialize the TD. This ioctl also sets the cpuids and attributes for
the VM.

At this point we can also set the common boot parameters such as CR3,
CR4, etc. These parameters will get copied to the relevant registers by
the TD boot code trampoline.

Signed-off-by: Sagi Shahar <sagis@google.com>

---------------------------------------------

Changes from v10:
 * The call to vm_tdx_load_common_boot_parameters() was accidently
   dropped as part of the refactor from v9 to v10. I re-added it here.
---
 tools/testing/selftests/kvm/lib/x86/processor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 5f75bd48623b..990f2769c5d8 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -676,6 +676,11 @@ void kvm_arch_vm_post_create(struct kvm_vm *vm, unsigned int nr_vcpus)
 		vm_sev_ioctl(vm, KVM_SEV_INIT2, &init);
 	}
 
+	if (is_tdx_vm(vm)) {
+		vm_tdx_init_vm(vm, 0);
+		vm_tdx_load_common_boot_parameters(vm);
+	}
+
 	r = __vm_ioctl(vm, KVM_GET_TSC_KHZ, NULL);
 	TEST_ASSERT(r > 0, "KVM_GET_TSC_KHZ did not provide a valid TSC frequency.");
 	guest_tsc_khz = r;
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (14 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-29 13:18   ` Ira Weiny
  2025-10-28 21:20 ` [PATCH v12 17/23] KVM: selftests: Call KVM_TDX_INIT_VCPU when creating a new TDX vcpu Sagi Shahar
                   ` (6 subsequent siblings)
  22 siblings, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Guest registers are inaccessible to kvm for TDX VMs. In order to set
register values for TDX we use a special boot code which loads the
register values from memory and write them into the appropriate
registers.

This patch sets up the memory regions used for the boot code and the
boot parameters for TDX.

Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 0e6a487ca7a4..086e8a2a4d99 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -4,6 +4,7 @@
  *
  * Copyright (C) 2018, Google LLC.
  */
+#include "tdx/tdx_util.h"
 #include "test_util.h"
 #include "kvm_util.h"
 #include "processor.h"
@@ -435,7 +436,7 @@ void kvm_set_files_rlimit(uint32_t nr_vcpus)
 static bool is_guest_memfd_required(struct vm_shape shape)
 {
 #ifdef __x86_64__
-	return shape.type == KVM_X86_SNP_VM;
+	return (shape.type == KVM_X86_SNP_VM || shape.type == KVM_X86_TDX_VM);
 #else
 	return false;
 #endif
@@ -469,6 +470,12 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
 	for (i = 0; i < NR_MEM_REGIONS; i++)
 		vm->memslots[i] = 0;
 
+	if (is_tdx_vm(vm)) {
+		/* Setup additional mem regions for TDX. */
+		vm_tdx_setup_boot_code_region(vm);
+		vm_tdx_setup_boot_parameters_region(vm, nr_runnable_vcpus);
+	}
+
 	kvm_vm_elf_load(vm, program_invocation_name);
 
 	/*
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 17/23] KVM: selftests: Call KVM_TDX_INIT_VCPU when creating a new TDX vcpu
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (15 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-30  6:15   ` Binbin Wu
  2025-10-28 21:20 ` [PATCH v12 18/23] KVM: selftests: Set entry point for TDX guest code Sagi Shahar
                   ` (5 subsequent siblings)
  22 siblings, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

TDX VMs need to issue the KVM_TDX_INIT_VCPU ioctl for each vcpu after
vcpu creation.

Since the cpuids for TD are managed by the TDX module, read the values
virtualized for the TD using KVM_TDX_GET_CPUID and set them in kvm using
KVM_SET_CPUID2 so that kvm has an accurate view of the VM cpuid values.

Signed-off-by: Sagi Shahar <sagis@google.com>
---
 .../testing/selftests/kvm/lib/x86/processor.c | 35 ++++++++++++++-----
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 990f2769c5d8..036875fe140f 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -722,6 +722,19 @@ vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm)
 	return stack_vaddr;
 }
 
+static void vm_tdx_vcpu_add(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid2 *cpuid;
+
+	cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);
+	vm_tdx_vcpu_ioctl(vcpu, KVM_TDX_GET_CPUID, 0, cpuid);
+	vcpu_init_cpuid(vcpu, cpuid);
+	free(cpuid);
+	vm_tdx_vcpu_ioctl(vcpu, KVM_TDX_INIT_VCPU, 0, NULL);
+
+	vm_tdx_load_vcpu_boot_parameters(vm, vcpu);
+}
+
 struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
 {
 	struct kvm_mp_state mp_state;
@@ -729,15 +742,21 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
 	struct kvm_vcpu *vcpu;
 
 	vcpu = __vm_vcpu_add(vm, vcpu_id);
-	vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
-	vcpu_init_sregs(vm, vcpu);
-	vcpu_init_xcrs(vm, vcpu);
 
-	/* Setup guest general purpose registers */
-	vcpu_regs_get(vcpu, &regs);
-	regs.rflags = regs.rflags | 0x2;
-	regs.rsp = kvm_allocate_vcpu_stack(vm);
-	vcpu_regs_set(vcpu, &regs);
+	if (is_tdx_vm(vm)) {
+		vm_tdx_vcpu_add(vm, vcpu);
+	} else {
+		vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
+
+		vcpu_init_sregs(vm, vcpu);
+		vcpu_init_xcrs(vm, vcpu);
+
+		/* Setup guest general purpose registers */
+		vcpu_regs_get(vcpu, &regs);
+		regs.rflags = regs.rflags | 0x2;
+		regs.rsp = kvm_allocate_vcpu_stack(vm);
+		vcpu_regs_set(vcpu, &regs);
+	}
 
 	/* Setup the MP state */
 	mp_state.mp_state = 0;
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 18/23] KVM: selftests: Set entry point for TDX guest code
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (16 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 17/23] KVM: selftests: Call KVM_TDX_INIT_VCPU when creating a new TDX vcpu Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-31 16:03   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus Sagi Shahar
                   ` (4 subsequent siblings)
  22 siblings, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Since the rip register is inaccessible for TDX VMs, we need a different
way to set the guest entry point for TDX VMs. This is done by writing
the guest code address to a predefined location in the guest memory and
loading it into rip as part of the TDX boot code.

Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/lib/x86/processor.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 036875fe140f..17f5a381fe43 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -691,9 +691,13 @@ void vcpu_arch_set_entry_point(struct kvm_vcpu *vcpu, void *guest_code)
 {
 	struct kvm_regs regs;
 
-	vcpu_regs_get(vcpu, &regs);
-	regs.rip = (unsigned long) guest_code;
-	vcpu_regs_set(vcpu, &regs);
+	if (is_tdx_vm(vcpu->vm))
+		vm_tdx_set_vcpu_entry_point(vcpu, guest_code);
+	else {
+		vcpu_regs_get(vcpu, &regs);
+		regs.rip = (unsigned long) guest_code;
+		vcpu_regs_set(vcpu, &regs);
+	}
 }
 
 vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm)
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (17 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 18/23] KVM: selftests: Set entry point for TDX guest code Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-31 13:10   ` Ira Weiny
  2025-10-31 16:05   ` Reinette Chatre
  2025-10-28 21:20 ` [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest Sagi Shahar
                   ` (3 subsequent siblings)
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Call vm_tdx_finalize() as part of kvm_arch_vm_finalize_vcpus if this is
a TDX vm

Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/lib/x86/processor.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 17f5a381fe43..09cc75ae8d26 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -1360,3 +1360,9 @@ bool kvm_arch_has_default_irqchip(void)
 {
 	return true;
 }
+
+void kvm_arch_vm_finalize_vcpus(struct kvm_vm *vm)
+{
+	if (is_tdx_vm(vm))
+		vm_tdx_finalize(vm);
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (18 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-31 14:11   ` Ira Weiny
  2025-10-28 21:20 ` [PATCH v12 21/23] KVM: selftests: Add wrapper for TDX MMIO " Sagi Shahar
                   ` (2 subsequent siblings)
  22 siblings, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

From: Erdem Aktas <erdemaktas@google.com>

Add support for TDX guests to issue TDCALLs to the TDX module.

Signed-off-by: Erdem Aktas <erdemaktas@google.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |  8 ++
 .../selftests/kvm/include/x86/tdx/tdcall.h    | 34 +++++++
 .../selftests/kvm/lib/x86/tdx/tdcall.S        | 93 +++++++++++++++++++
 .../kvm/lib/x86/tdx/tdcall_offsets.c          | 16 ++++
 4 files changed, 151 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 86fe629f2e81..969338b66592 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -20,6 +20,7 @@ LIBKVM += lib/userfaultfd_util.c
 LIBKVM_STRING += lib/string_override.c
 
 LIBKVM_ASM_DEFS += lib/x86/tdx/td_boot_offsets.c
+LIBKVM_ASM_DEFS += lib/x86/tdx/tdcall_offsets.c
 
 LIBKVM_x86 += lib/x86/apic.c
 LIBKVM_x86 += lib/x86/handlers.S
@@ -33,6 +34,7 @@ LIBKVM_x86 += lib/x86/ucall.c
 LIBKVM_x86 += lib/x86/vmx.c
 LIBKVM_x86 += lib/x86/tdx/tdx_util.c
 LIBKVM_x86 += lib/x86/tdx/td_boot.S
+LIBKVM_x86 += lib/x86/tdx/tdcall.S
 
 LIBKVM_arm64 += lib/arm64/gic.c
 LIBKVM_arm64 += lib/arm64/gic_v3.c
@@ -352,7 +354,13 @@ $(OUTPUT)/lib/x86/tdx/td_boot.o: $(OUTPUT)/include/x86/tdx/td_boot_offsets.h
 $(OUTPUT)/include/x86/tdx/td_boot_offsets.h: $(OUTPUT)/lib/x86/tdx/td_boot_offsets.s FORCE
 	$(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)
 
+$(OUTPUT)/lib/x86/tdx/tdcall.o: $(OUTPUT)/include/x86/tdx/tdcall_offsets.h
+
+$(OUTPUT)/include/x86/tdx/tdcall_offsets.h: $(OUTPUT)/lib/x86/tdx/tdcall_offsets.s FORCE
+	$(call filechk,offsets,__TDCALL__OFFSETS_H__)
+
 EXTRA_CLEAN += $(OUTPUT)/include/x86/tdx/td_boot_offsets.h
+EXTRA_CLEAN += $(OUTPUT)/include/x86/tdx/tdcall_offsets.h
 
 $(shell mkdir -p $(sort $(dir $(TEST_GEN_PROGS))))
 $(SPLIT_TEST_GEN_OBJ): $(GEN_HDRS)
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
new file mode 100644
index 000000000000..60c70646f876
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Adapted from arch/x86/include/asm/shared/tdx.h */
+
+#ifndef SELFTESTS_TDX_TDCALL_H
+#define SELFTESTS_TDX_TDCALL_H
+
+#include <linux/bits.h>
+
+#define TDX_TDCALL_HAS_OUTPUT BIT(0)
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/*
+ * Used in __tdx_tdcall() to pass down and get back registers' values of
+ * the TDCALL instruction when requesting services from the VMM.
+ *
+ * This is a software only structure and not part of the TDX module/VMM ABI.
+ */
+struct tdx_tdcall_args {
+	u64 r10;
+	u64 r11;
+	u64 r12;
+	u64 r13;
+	u64 r14;
+	u64 r15;
+};
+
+/* Used to request services from the VMM */
+u64 __tdx_tdcall(struct tdx_tdcall_args *args, unsigned long flags);
+
+#endif // __ASSEMBLY__
+#endif // SELFTESTS_TDX_TDCALL_H
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
new file mode 100644
index 000000000000..05869e86b9d8
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall.S
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Adapted from arch/x86/virt/vmx/tdx/tdxcall.S */
+
+#ifndef __ASSEMBLY__
+#define __ASSEMBLY__
+#endif
+
+#include <linux/bits.h>
+#include "tdx/tdcall.h"
+#include "tdx/tdcall_offsets.h"
+
+/*
+ * TDCALL is supported in Binutils >= 2.36, add it for older version.
+ */
+#define tdcall		.byte 0x66,0x0f,0x01,0xcc
+
+/*
+ * Bitmasks of exposed registers (with VMM).
+ */
+#define TDX_R10		BIT(10)
+#define TDX_R11		BIT(11)
+#define TDX_R12		BIT(12)
+#define TDX_R13		BIT(13)
+#define TDX_R14		BIT(14)
+#define TDX_R15		BIT(15)
+
+/*
+ * These registers are clobbered to hold arguments for each
+ * TDVMCALL. They are safe to expose to the VMM.
+ * Each bit in this mask represents a register ID. Bit field
+ * details can be found in TDX GHCI specification, section
+ * titled "TDCALL [TDG.VP.VMCALL] leaf".
+ */
+#define TDVMCALL_EXPOSE_REGS_MASK    \
+         (TDX_R10 | TDX_R11 | TDX_R12 | TDX_R13 | TDX_R14 | TDX_R15)
+
+.code64
+.section .text
+
+.globl __tdx_tdcall
+.type __tdx_tdcall, @function
+__tdx_tdcall:
+	/* Set up stack frame */
+	push %rbp
+	movq %rsp, %rbp
+
+	/* Save callee-saved GPRs as mandated by the x86_64 ABI */
+	push %r15
+	push %r14
+	push %r13
+	push %r12
+
+	/* Mangle function call ABI into TDCALL ABI: */
+	/* Set TDCALL leaf ID (TDVMCALL (0)) in RAX */
+	xor %eax, %eax
+
+	/* Copy tdcall registers from arg struct: */
+	movq TDX_TDCALL_R10(%rdi), %r10
+	movq TDX_TDCALL_R11(%rdi), %r11
+	movq TDX_TDCALL_R12(%rdi), %r12
+	movq TDX_TDCALL_R13(%rdi), %r13
+	movq TDX_TDCALL_R14(%rdi), %r14
+	movq TDX_TDCALL_R15(%rdi), %r15
+
+	movl $TDVMCALL_EXPOSE_REGS_MASK, %ecx
+
+	tdcall
+
+	/* TDVMCALL leaf return code is in R10 */
+	movq %r10, %rax
+
+	/* Copy tdcall result registers to arg struct if needed */
+	testq $TDX_TDCALL_HAS_OUTPUT, %rsi
+	jz .Lout
+
+	movq %r10, TDX_TDCALL_R10(%rdi)
+	movq %r11, TDX_TDCALL_R11(%rdi)
+	movq %r12, TDX_TDCALL_R12(%rdi)
+	movq %r13, TDX_TDCALL_R13(%rdi)
+	movq %r14, TDX_TDCALL_R14(%rdi)
+	movq %r15, TDX_TDCALL_R15(%rdi)
+.Lout:
+	/* Restore callee-saved GPRs as mandated by the x86_64 ABI */
+	pop %r12
+	pop %r13
+	pop %r14
+	pop %r15
+
+	pop %rbp
+	ret
+
+/* Disable executable stack */
+.section .note.GNU-stack,"",%progbits
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c
new file mode 100644
index 000000000000..dcd4457be6e5
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdcall_offsets.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+#define COMPILE_OFFSETS
+
+#include <linux/kbuild.h>
+
+#include "tdx/tdcall.h"
+
+static void __attribute__((used)) common(void)
+{
+	OFFSET(TDX_TDCALL_R10, tdx_tdcall_args, r10);
+	OFFSET(TDX_TDCALL_R11, tdx_tdcall_args, r11);
+	OFFSET(TDX_TDCALL_R12, tdx_tdcall_args, r12);
+	OFFSET(TDX_TDCALL_R13, tdx_tdcall_args, r13);
+	OFFSET(TDX_TDCALL_R14, tdx_tdcall_args, r14);
+	OFFSET(TDX_TDCALL_R15, tdx_tdcall_args, r15);
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 21/23] KVM: selftests: Add wrapper for TDX MMIO from guest
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (19 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-31 14:21   ` Ira Weiny
  2025-10-28 21:20 ` [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX Sagi Shahar
  2025-10-28 21:20 ` [PATCH v12 23/23] KVM: selftests: Add TDX lifecycle test Sagi Shahar
  22 siblings, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Add utility function to issue MMIO TDCALL from TDX guests.

Signed-off-by: Sagi Shahar <sagis@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |  1 +
 .../selftests/kvm/include/x86/tdx/tdx.h       | 14 +++++++++++
 tools/testing/selftests/kvm/lib/x86/tdx/tdx.c | 23 +++++++++++++++++++
 3 files changed, 38 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/include/x86/tdx/tdx.h
 create mode 100644 tools/testing/selftests/kvm/lib/x86/tdx/tdx.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 969338b66592..b7a518d62098 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -35,6 +35,7 @@ LIBKVM_x86 += lib/x86/vmx.c
 LIBKVM_x86 += lib/x86/tdx/tdx_util.c
 LIBKVM_x86 += lib/x86/tdx/td_boot.S
 LIBKVM_x86 += lib/x86/tdx/tdcall.S
+LIBKVM_x86 += lib/x86/tdx/tdx.c
 
 LIBKVM_arm64 += lib/arm64/gic.c
 LIBKVM_arm64 += lib/arm64/gic_v3.c
diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx.h b/tools/testing/selftests/kvm/include/x86/tdx/tdx.h
new file mode 100644
index 000000000000..22b096402998
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTESTS_TDX_TDX_H
+#define SELFTESTS_TDX_TDX_H
+
+#include <stdint.h>
+
+/* MMIO direction */
+#define MMIO_READ	0
+#define MMIO_WRITE	1
+
+uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
+					     uint64_t data_in);
+
+#endif // SELFTESTS_TDX_TDX_H
diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
new file mode 100644
index 000000000000..f9c1acd5b30c
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "tdx/tdcall.h"
+#include "tdx/tdx.h"
+
+#define TDG_VP_VMCALL 0
+
+#define TDG_VP_VMCALL_VE_REQUEST_MMIO	48
+
+uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
+					     uint64_t data_in)
+{
+	struct tdx_tdcall_args args = {
+		.r10 = TDG_VP_VMCALL,
+		.r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
+		.r12 = size,
+		.r13 = MMIO_WRITE,
+		.r14 = address,
+		.r15 = data_in,
+	};
+
+	return __tdx_tdcall(&args, 0);
+}
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (20 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 21/23] KVM: selftests: Add wrapper for TDX MMIO " Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  2025-10-31 14:38   ` Ira Weiny
  2025-10-31 15:55   ` Sean Christopherson
  2025-10-28 21:20 ` [PATCH v12 23/23] KVM: selftests: Add TDX lifecycle test Sagi Shahar
  22 siblings, 2 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

From: Ackerley Tng <ackerleytng@google.com>

ucalls for non-Coco VMs work by having the guest write to the rdi
register, then perform an io instruction to exit to the host. The host
then reads rdi using kvm_get_regs().

CPU registers can't be read using kvm_get_regs() for TDX, so TDX
guests use MMIO to pass the struct ucall's hva to the host. MMIO was
chosen because it is one of the simplest (hence unlikely to fail)
mechanisms that support passing 8 bytes from guest to host.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>

----------------------------------------------

Changes from v10:
 * Removed ucall_arch_init() decleration from ucall.h.
 * Replace vm_type type check with is_tdx_vm().
 * Move mmio info initialization under is_tdx_vm() case.
---
 .../selftests/kvm/include/ucall_common.h      |  1 +
 .../testing/selftests/kvm/include/x86/ucall.h |  6 ---
 tools/testing/selftests/kvm/lib/x86/ucall.c   | 46 +++++++++++++++++--
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index d9d6581b8d4f..f5eebf690033 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -4,6 +4,7 @@
  */
 #ifndef SELFTEST_KVM_UCALL_COMMON_H
 #define SELFTEST_KVM_UCALL_COMMON_H
+#include "kvm_util.h"
 #include "test_util.h"
 #include "ucall.h"
 
diff --git a/tools/testing/selftests/kvm/include/x86/ucall.h b/tools/testing/selftests/kvm/include/x86/ucall.h
index d3825dcc3cd9..7e54ec2c1a45 100644
--- a/tools/testing/selftests/kvm/include/x86/ucall.h
+++ b/tools/testing/selftests/kvm/include/x86/ucall.h
@@ -2,12 +2,6 @@
 #ifndef SELFTEST_KVM_UCALL_H
 #define SELFTEST_KVM_UCALL_H
 
-#include "kvm_util.h"
-
 #define UCALL_EXIT_REASON       KVM_EXIT_IO
 
-static inline void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
-{
-}
-
 #endif
diff --git a/tools/testing/selftests/kvm/lib/x86/ucall.c b/tools/testing/selftests/kvm/lib/x86/ucall.c
index 1265cecc7dd1..fae6f37b0bcd 100644
--- a/tools/testing/selftests/kvm/lib/x86/ucall.c
+++ b/tools/testing/selftests/kvm/lib/x86/ucall.c
@@ -5,11 +5,35 @@
  * Copyright (C) 2018, Red Hat, Inc.
  */
 #include "kvm_util.h"
+#include "tdx/tdx.h"
+#include "tdx/tdx_util.h"
 
 #define UCALL_PIO_PORT ((uint16_t)0x1000)
 
+static uint8_t vm_type;
+static vm_paddr_t host_ucall_mmio_gpa;
+static vm_paddr_t ucall_mmio_gpa;
+
+void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
+{
+	vm_type = vm->type;
+	sync_global_to_guest(vm, vm_type);
+
+	if (is_tdx_vm(vm)) {
+		host_ucall_mmio_gpa = ucall_mmio_gpa = mmio_gpa;
+		ucall_mmio_gpa |= vm->arch.s_bit;
+	}
+
+	sync_global_to_guest(vm, ucall_mmio_gpa);
+}
+
 void ucall_arch_do_ucall(vm_vaddr_t uc)
 {
+	if (vm_type == KVM_X86_TDX_VM) {
+		tdg_vp_vmcall_ve_request_mmio_write(ucall_mmio_gpa, 8, uc);
+		return;
+	}
+
 	/*
 	 * FIXME: Revert this hack (the entire commit that added it) once nVMX
 	 * preserves L2 GPRs across a nested VM-Exit.  If a ucall from L2, e.g.
@@ -46,11 +70,23 @@ void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
 {
 	struct kvm_run *run = vcpu->run;
 
-	if (run->exit_reason == KVM_EXIT_IO && run->io.port == UCALL_PIO_PORT) {
-		struct kvm_regs regs;
+	switch (vm_type) {
+	case KVM_X86_TDX_VM:
+		if (vcpu->run->exit_reason == KVM_EXIT_MMIO &&
+		    vcpu->run->mmio.phys_addr == host_ucall_mmio_gpa &&
+		    vcpu->run->mmio.len == 8 && vcpu->run->mmio.is_write) {
+			uint64_t data = *(uint64_t *)vcpu->run->mmio.data;
+
+			return (void *)data;
+		}
+		return NULL;
+	default:
+		if (run->exit_reason == KVM_EXIT_IO && run->io.port == UCALL_PIO_PORT) {
+			struct kvm_regs regs;
 
-		vcpu_regs_get(vcpu, &regs);
-		return (void *)regs.rdi;
+			vcpu_regs_get(vcpu, &regs);
+			return (void *)regs.rdi;
+		}
+		return NULL;
 	}
-	return NULL;
 }
-- 
2.51.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* [PATCH v12 23/23] KVM: selftests: Add TDX lifecycle test
  2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
                   ` (21 preceding siblings ...)
  2025-10-28 21:20 ` [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX Sagi Shahar
@ 2025-10-28 21:20 ` Sagi Shahar
  22 siblings, 0 replies; 69+ messages in thread
From: Sagi Shahar @ 2025-10-28 21:20 UTC (permalink / raw)
  To: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Sagi Shahar, Roger Wang, Binbin Wu,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Adding a test to verify TDX lifecycle by creating a simple TD.

Signed-off-by: Sagi Shahar <sagis@google.com>

---------------------------------------------

Changes from v11:
 * Removed vm_tdx_create_with_one_vcpu and replaced the call site with
   vm_create_shape_with_one_vcpu
---
 tools/testing/selftests/kvm/Makefile.kvm      |  1 +
 .../selftests/kvm/include/x86/processor.h     |  1 +
 .../selftests/kvm/include/x86/tdx/tdx_util.h  |  8 +++++
 tools/testing/selftests/kvm/x86/tdx_vm_test.c | 33 +++++++++++++++++++
 4 files changed, 43 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 b7a518d62098..2f49c8965df9 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -156,6 +156,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/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index dba2b3d558d1..7cd70ff15412 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -369,6 +369,7 @@ static inline unsigned int x86_model(unsigned int eax)
 #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)
+#define VM_SHAPE_TDX		VM_TYPE(KVM_X86_TDX_VM)
 
 /* Page table bitfield declarations */
 #define PTE_PRESENT_MASK        BIT_ULL(0)
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..f8e1c4d92a7a 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
  */
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..a37ab0fb2a97
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/tdx_vm_test.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "processor.h"
+#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_create_shape_with_one_vcpu(VM_SHAPE_TDX, &vcpu,
+					   guest_code_lifecycle);
+
+	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.1.851.g4ebd6896fd-goog


^ permalink raw reply related	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types
  2025-10-28 21:20 ` [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types Sagi Shahar
@ 2025-10-29  1:13   ` Ira Weiny
  2025-10-29  6:57   ` Binbin Wu
  2025-10-31  3:42   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29  1:13 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> 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>
> Signed-off-by: Sagi Shahar <sagis@google.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values
  2025-10-28 21:20 ` [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values Sagi Shahar
@ 2025-10-29  1:40   ` Ira Weiny
  2025-10-31  3:43   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29  1:40 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> TDX can't set sregs values directly using KVM_SET_SREGS. Expose the
> default values of certain sregs used by TDX VMs so they can be set
> manually.
> 
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types
  2025-10-28 21:20 ` [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types Sagi Shahar
  2025-10-29  1:13   ` Ira Weiny
@ 2025-10-29  6:57   ` Binbin Wu
  2025-10-31  3:42   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Binbin Wu @ 2025-10-29  6:57 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm



On 10/29/2025 5:20 AM, Sagi Shahar wrote:
> From: Sean Christopherson <seanjc@google.com>
>
> 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>
> Signed-off-by: Sagi Shahar <sagis@google.com>

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 07/23] KVM: selftests: Add kbuild definitons
  2025-10-28 21:20 ` [PATCH v12 07/23] KVM: selftests: Add kbuild definitons Sagi Shahar
@ 2025-10-29  7:43   ` Binbin Wu
  2025-10-29 15:46     ` Ira Weiny
  2025-10-29 15:55   ` Ira Weiny
  2025-10-31  3:56   ` Reinette Chatre
  2 siblings, 1 reply; 69+ messages in thread
From: Binbin Wu @ 2025-10-29  7:43 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm



On 10/29/2025 5:20 AM, Sagi Shahar wrote:
> Add kbuild.h that can be used by files under tools/
>
> Definitions are taken from the original definitions at
> include/linux/kbuild.h
>
> This is needed to expose values from c code to assembly code.
>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>   tools/include/linux/kbuild.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>   create mode 100644 tools/include/linux/kbuild.h
>
> diff --git a/tools/include/linux/kbuild.h b/tools/include/linux/kbuild.h
> new file mode 100644
> index 000000000000..62e20ba9380e
> --- /dev/null
> +++ b/tools/include/linux/kbuild.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TOOLS_LINUX_KBUILD_H
> +#define __TOOLS_LINUX_KBUILD_H
> +
> +#include <stddef.h>

This is not in kernel's version.
Instead, consumers of kbuild.h include the necessary header.

Maybe it can follow kernel's style?

> +
> +#define DEFINE(sym, val) \
> +	asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))
> +
> +#define BLANK() asm volatile("\n.ascii \"->\"" : : )
> +
> +#define OFFSET(sym, str, mem) \
> +	DEFINE(sym, offsetof(struct str, mem))
> +
> +#define COMMENT(x) \
> +	asm volatile("\n.ascii \"->#" x "\"")
> +
> +#endif /* __TOOLS_LINUX_KBUILD_H */


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region
  2025-10-28 21:20 ` [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region Sagi Shahar
@ 2025-10-29  8:52   ` Binbin Wu
  2025-10-29 21:01   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Binbin Wu @ 2025-10-29  8:52 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm



On 10/29/2025 5:20 AM, Sagi Shahar wrote:

[...]
> +
> +void vm_tdx_load_common_boot_parameters(struct kvm_vm *vm)
> +{
> +	struct td_boot_parameters *params =
> +		addr_gpa2hva(vm, TD_BOOT_PARAMETERS_GPA);
> +	uint32_t cr4;
> +
> +	TEST_ASSERT_EQ(vm->mode, VM_MODE_PXXV48_4K);
> +
> +	cr4 = kvm_get_default_cr4();
> +
> +	/* TDX spec 11.6.2: CR4 bit MCE is fixed to 1 */
> +	cr4 |= X86_CR4_MCE;
> +
> +	/* TDX spec 11.6.2: CR4 bit VMXE and SMXE are fixed to 0 */
> +	cr4 &= ~(X86_CR4_VMXE | X86_CR4_SMXE);
> +
> +	/* Set parameters! */
> +	params->cr0 = kvm_get_default_cr0();
> +	params->cr3 = vm->pgd;
Since TDX guest code starts from 32-bit, is it better to check that vm->pgd is
not beyond 32-bit?


> +	params->cr4 = cr4;
> +	params->idtr.base = vm->arch.idt;
> +	params->idtr.limit = kvm_get_default_idt_limit();
> +	params->gdtr.base = vm->arch.gdt;
> +	params->gdtr.limit = kvm_get_default_gdt_limit();
> +
> +	TEST_ASSERT(params->cr0 != 0, "cr0 should not be 0");
> +	TEST_ASSERT(params->cr3 != 0, "cr3 should not be 0");
> +	TEST_ASSERT(params->cr4 != 0, "cr4 should not be 0");
> +	TEST_ASSERT(params->gdtr.base != 0, "gdt base address should not be 0");
> +	TEST_ASSERT(params->idtr.base != 0, "idt base address should not be 0");
> +}
> +
[...]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation
  2025-10-28 21:20 ` [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation Sagi Shahar
@ 2025-10-29 13:18   ` Ira Weiny
  2025-10-30  6:01     ` Binbin Wu
  0 siblings, 1 reply; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 13:18 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> Guest registers are inaccessible to kvm for TDX VMs. In order to set
> register values for TDX we use a special boot code which loads the

NIT: who is 'we'?

> register values from memory and write them into the appropriate
> registers.
> 
> This patch sets up the memory regions used for the boot code and the
> boot parameters for TDX.

NIT: This is not needed.  Use imperative mood.

> 
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 0e6a487ca7a4..086e8a2a4d99 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -4,6 +4,7 @@
>   *
>   * Copyright (C) 2018, Google LLC.
>   */
> +#include "tdx/tdx_util.h"
>  #include "test_util.h"
>  #include "kvm_util.h"
>  #include "processor.h"
> @@ -435,7 +436,7 @@ void kvm_set_files_rlimit(uint32_t nr_vcpus)
>  static bool is_guest_memfd_required(struct vm_shape shape)
>  {
>  #ifdef __x86_64__
> -	return shape.type == KVM_X86_SNP_VM;
> +	return (shape.type == KVM_X86_SNP_VM || shape.type == KVM_X86_TDX_VM);

This caused me to dig a bit to understand why this hunk was needed given
the commit message only discusses guest registers.  I did not recall any
use of is_guest_memfd_required() in the vm_tdx_setup_*() calls so I was a
bit confused.

With this hunk considered the changelog should read something like:

<commit message>

Guest memfd is required for the primary memory region of TDX VMs.

Furthermore, guest registers are inaccessible to kvm for TDX VMs.  TDX
must use use special boot code which loads the register values from memory
and writes them into the appropriate registers.

Use guest_memfd for the primary memory regions and call the TDX boot code
functions for TDX VMs.

</commit message>

This clearly explains why the change to is_guest_memfd_required() is
needed.

In addition, the structure of this series is a bit odd to me.  I assume
this patch exists after the setup calls were added to ensure
bisect-ability?

Ira

>  #else
>  	return false;
>  #endif
> @@ -469,6 +470,12 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
>  	for (i = 0; i < NR_MEM_REGIONS; i++)
>  		vm->memslots[i] = 0;
>  
> +	if (is_tdx_vm(vm)) {
> +		/* Setup additional mem regions for TDX. */
> +		vm_tdx_setup_boot_code_region(vm);
> +		vm_tdx_setup_boot_parameters_region(vm, nr_runnable_vcpus);
> +	}
> +
>  	kvm_vm_elf_load(vm, program_invocation_name);
>  
>  	/*
> -- 
> 2.51.1.851.g4ebd6896fd-goog
> 



^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack
  2025-10-28 21:20 ` [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack Sagi Shahar
@ 2025-10-29 13:24   ` Ira Weiny
  2025-10-31  3:52   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 13:24 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> TDX guests' registers cannot be initialized directly using
> vcpu_regs_set(), hence the stack pointer needs to be initialized by
> the guest itself, running boot code beginning at the reset vector.
> 
> Expose the function to allocate the guest stack so that TDX
> initialization code can allocate it itself and skip the allocation in
> vm_arch_vcpu_add() in that case.
> 
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX
  2025-10-28 21:20 ` [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX Sagi Shahar
@ 2025-10-29 15:22   ` Ira Weiny
  2025-10-31  3:53   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 15:22 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm, Adrian Hunter

Sagi Shahar wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Let kvm_init_vm_address_properties() initialize vm->arch.{s_bit, tag_mask}
> similar to SEV.
> 
> TDX sets the shared bit based on the guest physical address width and
> currently supports 48 and 52 widths.
> 
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>


Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 07/23] KVM: selftests: Add kbuild definitons
  2025-10-29  7:43   ` Binbin Wu
@ 2025-10-29 15:46     ` Ira Weiny
  0 siblings, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 15:46 UTC (permalink / raw)
  To: Binbin Wu, Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm

Binbin Wu wrote:
> 
> 
> On 10/29/2025 5:20 AM, Sagi Shahar wrote:
> > Add kbuild.h that can be used by files under tools/
> >
> > Definitions are taken from the original definitions at
> > include/linux/kbuild.h
> >
> > This is needed to expose values from c code to assembly code.
> >
> > Signed-off-by: Sagi Shahar <sagis@google.com>
> > ---
> >   tools/include/linux/kbuild.h | 18 ++++++++++++++++++
> >   1 file changed, 18 insertions(+)
> >   create mode 100644 tools/include/linux/kbuild.h
> >
> > diff --git a/tools/include/linux/kbuild.h b/tools/include/linux/kbuild.h
> > new file mode 100644
> > index 000000000000..62e20ba9380e
> > --- /dev/null
> > +++ b/tools/include/linux/kbuild.h
> > @@ -0,0 +1,18 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __TOOLS_LINUX_KBUILD_H
> > +#define __TOOLS_LINUX_KBUILD_H
> > +
> > +#include <stddef.h>
> 
> This is not in kernel's version.
> Instead, consumers of kbuild.h include the necessary header.
> 
> Maybe it can follow kernel's style?

Is there a need to not include what is needed?  Generally that is a good
idea unless the header file dependencies cause some build slowdowns.  I
don't think that is the case here.

Ira

> 
> > +
> > +#define DEFINE(sym, val) \
> > +	asm volatile("\n.ascii \"->" #sym " %0 " #val "\"" : : "i" (val))
> > +
> > +#define BLANK() asm volatile("\n.ascii \"->\"" : : )
> > +
> > +#define OFFSET(sym, str, mem) \
> > +	DEFINE(sym, offsetof(struct str, mem))
> > +
> > +#define COMMENT(x) \
> > +	asm volatile("\n.ascii \"->#" x "\"")
> > +
> > +#endif /* __TOOLS_LINUX_KBUILD_H */
> 



^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 07/23] KVM: selftests: Add kbuild definitons
  2025-10-28 21:20 ` [PATCH v12 07/23] KVM: selftests: Add kbuild definitons Sagi Shahar
  2025-10-29  7:43   ` Binbin Wu
@ 2025-10-29 15:55   ` Ira Weiny
  2025-10-31  3:56   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 15:55 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> Add kbuild.h that can be used by files under tools/
> 
> Definitions are taken from the original definitions at
> include/linux/kbuild.h
> 
> This is needed to expose values from c code to assembly code.
> 
> Signed-off-by: Sagi Shahar <sagis@google.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code
  2025-10-28 21:20 ` [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code Sagi Shahar
@ 2025-10-29 16:37   ` Reinette Chatre
  2025-10-30 14:20     ` Sean Christopherson
  2025-10-31  4:01   ` Reinette Chatre
  1 sibling, 1 reply; 69+ messages in thread
From: Reinette Chatre @ 2025-10-29 16:37 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> TDX registers are inaccessible to KVM. Therefore we need a different
> mechanism to load boot parameters for TDX code. TDX boot code will read
> the registers values from memory and set the registers manually.
> 
> This patch defines the data structures used to communicate between c
> code and the TDX assembly boot code which will be added in a later
> patch.
> 

(sidenote: I do not know what the bar for this work is so I'll defer
comments related to local customs like using "we" and "this patch" in
changelog)

> Use kbuild.h to expose the offsets into the structs from c code to
> assembly code.
> 


> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> index 148d427ff24b..5e809064ff1c 100644
> --- a/tools/testing/selftests/kvm/Makefile.kvm
> +++ b/tools/testing/selftests/kvm/Makefile.kvm

...

> @@ -328,18 +336,28 @@ $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c $(GEN_HDRS)
>  $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S $(GEN_HDRS)
>  	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
>  
> +$(LIBKVM_ASM_DEFS_OBJ): $(OUTPUT)/%.s: %.c FORCE
> +	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -S $< -o $@
> +
>  # Compile the string overrides as freestanding to prevent the compiler from
>  # generating self-referential code, e.g. without "freestanding" the compiler may
>  # "optimize" memcmp() by invoking memcmp(), thus causing infinite recursion.
>  $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c
>  	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@
>  
> +$(OUTPUT)/include/x86/tdx/td_boot_offsets.h: $(OUTPUT)/lib/x86/tdx/td_boot_offsets.s FORCE
> +	$(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)
> +

Some folks prefer to keep build output separate and may build tests using a command
line like:
	make O=<output dir> TARGETS=kvm -C tools/testing/selftests

This is a valid usage and will result in td_boot_offsets.h placed in <output dir> that
is not covered by current include path. A build with above command line thus fails:

lib/x86/tdx/td_boot.S:4:10: fatal error: tdx/td_boot_offsets.h: No such file or directory
    4 | #include "tdx/td_boot_offsets.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.


Something like below may be needed to add the output directory to the include path:

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 2f49c8965df9..98bc40a7f069 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -262,7 +262,7 @@ CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
 	-fno-stack-protector -fno-PIE -fno-strict-aliasing \
 	-I$(LINUX_TOOL_INCLUDE) -I$(LINUX_TOOL_ARCH_INCLUDE) \
 	-I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(ARCH) \
-	-I ../rseq -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
+	-I ../rseq -I.. -I$(OUTPUT)/include/$(ARCH) $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
 ifeq ($(ARCH),s390)
 	CFLAGS += -march=z10
 endif

Reinette

^ permalink raw reply related	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region
  2025-10-28 21:20 ` [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region Sagi Shahar
  2025-10-29  8:52   ` Binbin Wu
@ 2025-10-29 21:01   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-29 21:01 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> Allocate memory for TDX boot parameters and define the utility functions
> necessary to fill this memory with the boot parameters.
> 
> Co-developed-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> 
> ---------------------------------------------

A couple of patches in this series have this separator above that is not
recognized (the familiar "---"). This results in the separator self and the
changes below that follows it (everything until valid separator "---") to be
included as changelog when the patch is applied.


> 
> Changes from v10:
>  * Removed code for setting up X86_CR4_OSXMMEXCPT bit. At least for now
>    it is not needed and the test pass without it.
> ---

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM
  2025-10-28 21:20 ` [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM Sagi Shahar
@ 2025-10-29 21:16   ` Ira Weiny
  2025-10-29 23:01     ` Reinette Chatre
  2025-10-30  1:25   ` Binbin Wu
  2025-10-31  4:06   ` Reinette Chatre
  2 siblings, 1 reply; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 21:16 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:

[snip]

> 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 dafdc7e46abe..a2509959c7ce 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,60 @@ static inline bool is_tdx_vm(struct kvm_vm *vm)
>  	return vm->type == KVM_X86_TDX_VM;
>  }
>  
> +/*
> + * TDX ioctls
> + */
> +
> +#define __vm_tdx_vm_ioctl(vm, cmd, metadata, arg)			\

NIT: Why not call 'metadata' -> 'flags'?

> +({									\
> +	int r;								\
> +									\
> +	union {								\
> +		struct kvm_tdx_cmd c;					\
> +		unsigned long raw;					\
> +	} tdx_cmd = { .c = {						\
> +		.id = (cmd),						\
> +		.flags = (uint32_t)(metadata),				\
> +		.data = (uint64_t)(arg),				\
> +	} };								\
> +									\
> +	r = __vm_ioctl(vm, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);	\
> +	r ?: tdx_cmd.c.hw_error;					\
> +})

I see this is a common pattern for kvm selftests but I'm struggling to
figure out why this is a macro and not a function call?

> +
> +#define vm_tdx_vm_ioctl(vm, cmd, flags, arg)				\
> +({									\
> +	int ret = __vm_tdx_vm_ioctl(vm, cmd, flags, arg);		\
> +									\
> +	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd,	ret, vm);		\
> +})
> +
> +#define __vm_tdx_vcpu_ioctl(vcpu, cmd, metadata, arg)			\

NIT: Why not just call 'metadata', 'flags'?

> +({									\
> +	int r;								\
> +									\
> +	union {								\
> +		struct kvm_tdx_cmd c;					\
> +		unsigned long raw;					\
> +	} tdx_cmd = { .c = {						\
> +		.id = (cmd),						\
> +		.flags = (uint32_t)(metadata),				\
> +		.data = (uint64_t)(arg),				\
> +	} };								\
> +									\
> +	r = __vcpu_ioctl(vcpu, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);	\
> +	r ?: tdx_cmd.c.hw_error;					\
> +})
> +

[snip]

> +
> +static struct kvm_tdx_capabilities *tdx_read_capabilities(struct kvm_vm *vm)
> +{
> +	struct kvm_tdx_capabilities *tdx_cap = NULL;
> +	int nr_cpuid_configs = 4;
> +	int rc = -1;
> +	int i;
> +
> +	do {
> +		nr_cpuid_configs *= 2;
> +
> +		tdx_cap = realloc(tdx_cap, sizeof(*tdx_cap) +
> +					   sizeof(tdx_cap->cpuid) +
> +					   (sizeof(struct kvm_cpuid_entry2) * nr_cpuid_configs));
> +		TEST_ASSERT(tdx_cap,
> +			    "Could not allocate memory for tdx capability nr_cpuid_configs %d\n",
> +			    nr_cpuid_configs);
> +
> +		tdx_cap->cpuid.nent = nr_cpuid_configs;
> +		rc = __vm_tdx_vm_ioctl(vm, KVM_TDX_CAPABILITIES, 0, tdx_cap);

Why not use vm_tdx_vm_ioctl()?

Generally though it is good.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration
  2025-10-28 21:20 ` [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration Sagi Shahar
@ 2025-10-29 21:19   ` Ira Weiny
  2025-10-30  1:35   ` Binbin Wu
  1 sibling, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 21:19 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Make sure that all the attributes enabled by the test are reported as
> supported by the TDX module.
> 
> This also exercises the KVM_TDX_CAPABILITIES ioctl.

NIT: I'd strike this as tdx_read_capabilities() was already exercised in
the init path (last patch).

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM
  2025-10-28 21:20 ` [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM Sagi Shahar
@ 2025-10-29 21:27   ` Ira Weiny
  2025-10-30  2:32   ` Binbin Wu
  2025-10-31 15:58   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-29 21:27 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> TDX protected memory needs to be measured and encrypted before it can be
> used by the guest. Traverse the VM's memory regions and initialize all
> the protected ranges by calling KVM_TDX_INIT_MEM_REGION.
> 
> Once all the memory is initialized, the VM can be finalized by calling
> KVM_TDX_FINALIZE_VM.
> 
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Co-developed-by: Erdem Aktas <erdemaktas@google.com>
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Co-developed-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  .../selftests/kvm/include/x86/tdx/tdx_util.h  |  2 +
>  .../selftests/kvm/lib/x86/tdx/tdx_util.c      | 58 +++++++++++++++++++
>  2 files changed, 60 insertions(+)
> 
> 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 a2509959c7ce..2467b6c35557 100644
> --- a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
> +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
> @@ -71,4 +71,6 @@ void vm_tdx_load_common_boot_parameters(struct kvm_vm *vm);
>  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);

FWIW this is not what I was expecting to see based on the previous
discussion.  Knowing that this call is needed later I'm inclined to let it
go but generally it would have been better to separate out this call
when/if the follow on tests require it; rather than defining this call
here without context.

That said:

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM
  2025-10-29 21:16   ` Ira Weiny
@ 2025-10-29 23:01     ` Reinette Chatre
  0 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-29 23:01 UTC (permalink / raw)
  To: Ira Weiny, Sagi Shahar, linux-kselftest, Paolo Bonzini,
	Shuah Khan, Sean Christopherson, Ackerley Tng, Ryan Afranji,
	Andrew Jones, Isaku Yamahata, Erdem Aktas, Rick Edgecombe,
	Roger Wang, Binbin Wu, Oliver Upton, Pratik R. Sampat, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm



On 10/29/25 2:16 PM, Ira Weiny wrote:
> 
>> +
>> +#define vm_tdx_vm_ioctl(vm, cmd, flags, arg)				\
>> +({									\
>> +	int ret = __vm_tdx_vm_ioctl(vm, cmd, flags, arg);		\
>> +									\
>> +	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd,	ret, vm);		\
>> +})
>> +
>> +#define __vm_tdx_vcpu_ioctl(vcpu, cmd, metadata, arg)			\
> 
> NIT: Why not just call 'metadata', 'flags'?
> 

Making this change would make the code easier to read by being consistent
with caller here as well as with kernel terms. If making this change please
consider its callers also, for example the "metadata" local variable of
tdx_init_mem_region() introduced in patch #14. Naming it flags would then also
be consistent with this change as well as how the flag is used in the kernel.

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM
  2025-10-28 21:20 ` [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM Sagi Shahar
  2025-10-29 21:16   ` Ira Weiny
@ 2025-10-30  1:25   ` Binbin Wu
  2025-10-31  4:06   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Binbin Wu @ 2025-10-30  1:25 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kernel, kvm, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Oliver Upton, Pratik R. Sampat, Reinette Chatre, Ira Weiny,
	Chao Gao, Chenyi Qiang



On 10/29/2025 5:20 AM, Sagi Shahar wrote:
> KVM_TDX_INIT_VM needs to be called after KVM_CREATE_VM and before
> creating any VCPUs, thus before KVM_SET_CPUID2. KVM_TDX_INIT_VM accepts
> the CPUID values directly.
This sentence seems not accurate.
KVM_TDX_INIT_VM, i.e. the seamcall TDH.MNG.INIT, allows only directly
configurable CPUID bits to be 1.

>
> Since KVM_GET_CPUID2 can't be used at this point,

I don't think this is relevant.

As mentioned above, only directly configurable CPUID bits can be 1, so the
CPUIDs input for KVM_TDX_INIT_VM should be filtered against the supported
directly configurable CPUID bits.

>   calculate the CPUID
> values manually by using kvm_get_supported_cpuid() and filter the
> returned CPUIDs against the supported CPUID values read from the TDX

supported CPUID -> supported configurable CPUID

> module.
>
>
[...]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration
  2025-10-28 21:20 ` [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration Sagi Shahar
  2025-10-29 21:19   ` Ira Weiny
@ 2025-10-30  1:35   ` Binbin Wu
  1 sibling, 0 replies; 69+ messages in thread
From: Binbin Wu @ 2025-10-30  1:35 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm



On 10/29/2025 5:20 AM, Sagi Shahar wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Make sure that all the attributes enabled by the test are reported as
> supported by the TDX module.
More accurately, supported by both the TDX module and KVM.
KVM filters out the attributes not supported by itself.

Otherwise,
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

>
> This also exercises the KVM_TDX_CAPABILITIES ioctl.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Co-developed-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>   tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> 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 7a622b4810b1..2551b3eac8f8 100644
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> @@ -231,6 +231,18 @@ static void vm_tdx_filter_cpuid(struct kvm_vm *vm,
>   	free(tdx_cap);
>   }
>   
> +static void tdx_check_attributes(struct kvm_vm *vm, uint64_t attributes)
> +{
> +	struct kvm_tdx_capabilities *tdx_cap;
> +
> +	tdx_cap = tdx_read_capabilities(vm);
> +
> +	/* Make sure all the attributes are reported as supported */
> +	TEST_ASSERT_EQ(attributes & tdx_cap->supported_attrs, attributes);
> +
> +	free(tdx_cap);
> +}
> +
>   void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
>   {
>   	struct kvm_tdx_init_vm *init_vm;
> @@ -250,6 +262,8 @@ void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
>   	memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
>   	free(cpuid);
>   
> +	tdx_check_attributes(vm, attributes);
> +
>   	init_vm->attributes = attributes;
>   
>   	vm_tdx_vm_ioctl(vm, KVM_TDX_INIT_VM, 0, init_vm);


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM
  2025-10-28 21:20 ` [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM Sagi Shahar
  2025-10-29 21:27   ` Ira Weiny
@ 2025-10-30  2:32   ` Binbin Wu
  2025-10-31 15:58   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Binbin Wu @ 2025-10-30  2:32 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm



On 10/29/2025 5:20 AM, Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@google.com>
>
> TDX protected memory needs to be measured and encrypted before it can be
> used by the guest. Traverse the VM's memory regions and initialize all
> the protected ranges by calling KVM_TDX_INIT_MEM_REGION.
>
> Once all the memory is initialized, the VM can be finalized by calling
> KVM_TDX_FINALIZE_VM.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Co-developed-by: Erdem Aktas <erdemaktas@google.com>
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Co-developed-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>



^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation
  2025-10-29 13:18   ` Ira Weiny
@ 2025-10-30  6:01     ` Binbin Wu
  0 siblings, 0 replies; 69+ messages in thread
From: Binbin Wu @ 2025-10-30  6:01 UTC (permalink / raw)
  To: Ira Weiny, Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Chao Gao, Chenyi Qiang,
	linux-kernel, kvm



On 10/29/2025 9:18 PM, Ira Weiny wrote:
> Sagi Shahar wrote:
>> Guest registers are inaccessible to kvm for TDX VMs. In order to set
>> register values for TDX we use a special boot code which loads the
> NIT: who is 'we'?
>
>> register values from memory and write them into the appropriate
>> registers.
>>
>> This patch sets up the memory regions used for the boot code and the
>> boot parameters for TDX.
> NIT: This is not needed.  Use imperative mood.
>
>> Signed-off-by: Sagi Shahar <sagis@google.com>
>> ---
>>   tools/testing/selftests/kvm/lib/kvm_util.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
>> index 0e6a487ca7a4..086e8a2a4d99 100644
>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>> @@ -4,6 +4,7 @@
>>    *
>>    * Copyright (C) 2018, Google LLC.
>>    */
>> +#include "tdx/tdx_util.h"
>>   #include "test_util.h"
>>   #include "kvm_util.h"
>>   #include "processor.h"
>> @@ -435,7 +436,7 @@ void kvm_set_files_rlimit(uint32_t nr_vcpus)
>>   static bool is_guest_memfd_required(struct vm_shape shape)
>>   {
>>   #ifdef __x86_64__
>> -	return shape.type == KVM_X86_SNP_VM;
>> +	return (shape.type == KVM_X86_SNP_VM || shape.type == KVM_X86_TDX_VM);
> This caused me to dig a bit to understand why this hunk was needed given
> the commit message only discusses guest registers.  I did not recall any
> use of is_guest_memfd_required() in the vm_tdx_setup_*() calls so I was a
> bit confused.
>
> With this hunk considered the changelog should read something like:
>
> <commit message>
>
> Guest memfd is required for the primary memory region of TDX VMs.
>
> Furthermore, guest registers are inaccessible to kvm for TDX VMs.  TDX
> must use use special boot code which loads the register values from memory
> and writes them into the appropriate registers.
>
> Use guest_memfd for the primary memory regions and call the TDX boot code
> functions for TDX VMs.
>
> </commit message>
>
> This clearly explains why the change to is_guest_memfd_required() is
> needed.

+1

>
> In addition, the structure of this series is a bit odd to me.  I assume
> this patch exists after the setup calls were added to ensure
> bisect-ability?

I think it's better to switch the order of this patch and patch 15.
Patch 15 relies on the memory regions added by this patch for boot code and
parameters.


>
> Ira
>
>>   #else
>>   	return false;
>>   #endif
>> @@ -469,6 +470,12 @@ struct kvm_vm *__vm_create(struct vm_shape shape, uint32_t nr_runnable_vcpus,
>>   	for (i = 0; i < NR_MEM_REGIONS; i++)
>>   		vm->memslots[i] = 0;
>>   
>> +	if (is_tdx_vm(vm)) {
>> +		/* Setup additional mem regions for TDX. */
>> +		vm_tdx_setup_boot_code_region(vm);
>> +		vm_tdx_setup_boot_parameters_region(vm, nr_runnable_vcpus);
>> +	}
>> +
>>   	kvm_vm_elf_load(vm, program_invocation_name);
>>   
>>   	/*
>> -- 
>> 2.51.1.851.g4ebd6896fd-goog
>>
>


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 17/23] KVM: selftests: Call KVM_TDX_INIT_VCPU when creating a new TDX vcpu
  2025-10-28 21:20 ` [PATCH v12 17/23] KVM: selftests: Call KVM_TDX_INIT_VCPU when creating a new TDX vcpu Sagi Shahar
@ 2025-10-30  6:15   ` Binbin Wu
  0 siblings, 0 replies; 69+ messages in thread
From: Binbin Wu @ 2025-10-30  6:15 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Sean Christopherson,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm



On 10/29/2025 5:20 AM, Sagi Shahar wrote:
> TDX VMs need to issue the KVM_TDX_INIT_VCPU ioctl for each vcpu after
> vcpu creation.
>
> Since the cpuids for TD are managed by the TDX module, read the values
> virtualized for the TD using KVM_TDX_GET_CPUID and set them in kvm using
> KVM_SET_CPUID2 so that kvm has an accurate view of the VM cpuid values.
>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>   .../testing/selftests/kvm/lib/x86/processor.c | 35 ++++++++++++++-----
>   1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
> index 990f2769c5d8..036875fe140f 100644
> --- a/tools/testing/selftests/kvm/lib/x86/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86/processor.c
> @@ -722,6 +722,19 @@ vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm)
>   	return stack_vaddr;
>   }
>   
> +static void vm_tdx_vcpu_add(struct kvm_vm *vm, struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpuid2 *cpuid;
> +
> +	cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);
> +	vm_tdx_vcpu_ioctl(vcpu, KVM_TDX_GET_CPUID, 0, cpuid);
> +	vcpu_init_cpuid(vcpu, cpuid);
> +	free(cpuid);
> +	vm_tdx_vcpu_ioctl(vcpu, KVM_TDX_INIT_VCPU, 0, NULL);
> +
> +	vm_tdx_load_vcpu_boot_parameters(vm, vcpu);
> +}
> +
>   struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
>   {
>   	struct kvm_mp_state mp_state;
> @@ -729,15 +742,21 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
>   	struct kvm_vcpu *vcpu;
>   
>   	vcpu = __vm_vcpu_add(vm, vcpu_id);
> -	vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
> -	vcpu_init_sregs(vm, vcpu);
> -	vcpu_init_xcrs(vm, vcpu);
>   
> -	/* Setup guest general purpose registers */
> -	vcpu_regs_get(vcpu, &regs);
> -	regs.rflags = regs.rflags | 0x2;
> -	regs.rsp = kvm_allocate_vcpu_stack(vm);
> -	vcpu_regs_set(vcpu, &regs);
> +	if (is_tdx_vm(vm)) {
> +		vm_tdx_vcpu_add(vm, vcpu);
Nit:
Since vcpu is added by  __vm_vcpu_add() above, using 'init' instead of 'add' in
the function name makes it more clear.

> +	} else {
> +		vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
> +
> +		vcpu_init_sregs(vm, vcpu);
> +		vcpu_init_xcrs(vm, vcpu);
> +
> +		/* Setup guest general purpose registers */
> +		vcpu_regs_get(vcpu, &regs);
> +		regs.rflags = regs.rflags | 0x2;
> +		regs.rsp = kvm_allocate_vcpu_stack(vm);
> +		vcpu_regs_set(vcpu, &regs);
> +	}
>   
>   	/* Setup the MP state */
>   	mp_state.mp_state = 0;


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code
  2025-10-29 16:37   ` Reinette Chatre
@ 2025-10-30 14:20     ` Sean Christopherson
  0 siblings, 0 replies; 69+ messages in thread
From: Sean Christopherson @ 2025-10-30 14:20 UTC (permalink / raw)
  To: Reinette Chatre
  Cc: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Binbin Wu, Oliver Upton,
	Pratik R. Sampat, Ira Weiny, Chao Gao, Chenyi Qiang, linux-kernel,
	kvm

On Wed, Oct 29, 2025, Reinette Chatre wrote:
> Hi Sagi,
> 
> On 10/28/25 2:20 PM, Sagi Shahar wrote:
> > TDX registers are inaccessible to KVM. Therefore we need a different
> > mechanism to load boot parameters for TDX code. TDX boot code will read
> > the registers values from memory and set the registers manually.
> > 
> > This patch defines the data structures used to communicate between c
> > code and the TDX assembly boot code which will be added in a later
> > patch.
> > 
> 
> (sidenote: I do not know what the bar for this work is so I'll defer
> comments related to local customs like using "we" and "this patch" in
> changelog)

The same as KVM x86, which follows the same rules as the tip tree, with a few
intentional differences.  By all means, call out those things, it'll save me the
effort :-)

Documentation/process/maintainer-kvm-x86.rst

> 
> > Use kbuild.h to expose the offsets into the structs from c code to
> > assembly code.
> > 
> 
> 
> > diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> > index 148d427ff24b..5e809064ff1c 100644
> > --- a/tools/testing/selftests/kvm/Makefile.kvm
> > +++ b/tools/testing/selftests/kvm/Makefile.kvm
> 
> ...
> 
> > @@ -328,18 +336,28 @@ $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c $(GEN_HDRS)
> >  $(LIBKVM_S_OBJ): $(OUTPUT)/%.o: %.S $(GEN_HDRS)
> >  	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
> >  
> > +$(LIBKVM_ASM_DEFS_OBJ): $(OUTPUT)/%.s: %.c FORCE
> > +	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -S $< -o $@
> > +
> >  # Compile the string overrides as freestanding to prevent the compiler from
> >  # generating self-referential code, e.g. without "freestanding" the compiler may
> >  # "optimize" memcmp() by invoking memcmp(), thus causing infinite recursion.
> >  $(LIBKVM_STRING_OBJ): $(OUTPUT)/%.o: %.c
> >  	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c -ffreestanding $< -o $@
> >  
> > +$(OUTPUT)/include/x86/tdx/td_boot_offsets.h: $(OUTPUT)/lib/x86/tdx/td_boot_offsets.s FORCE
> > +	$(call filechk,offsets,__TDX_BOOT_OFFSETS_H__)

Presumably this needs to be guarded so that it's x86-only.  I can't tell for sure
as there are other problems in this series of a similar nature that prevent me from
getting far enough to see.  Please build test on at least one other architecture
before sending the next version.

lib/kvm_util.c:7:10: fatal error: tdx/tdx_util.h: No such file or directory
    7 | #include "tdx/tdx_util.h"
      |          ^~~~~~~~~~~~~~~~
compilation terminated.


If possible, I would also really like to see these programatically defined, e.g.
something like (I have no idea if this is remotely valid syntax):

  $(OUTPUT)/$(TEST_GEN_HEADERS): $(OUTPUT)/%.s FORCE
     $(call filechk,offsets,__%_h__)


> Some folks prefer to keep build output separate and may build tests using a command
> line like:
> 	make O=<output dir> TARGETS=kvm -C tools/testing/selftests

Ya, I exclusively build that way.

> This is a valid usage and will result in td_boot_offsets.h placed in <output dir> that
> is not covered by current include path. A build with above command line thus fails:
> 
> lib/x86/tdx/td_boot.S:4:10: fatal error: tdx/td_boot_offsets.h: No such file or directory
>     4 | #include "tdx/td_boot_offsets.h"
>       |          ^~~~~~~~~~~~~~~~~~~~~~~
> compilation terminated.
> 
> 
> Something like below may be needed to add the output directory to the include path:
> 
> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> index 2f49c8965df9..98bc40a7f069 100644
> --- a/tools/testing/selftests/kvm/Makefile.kvm
> +++ b/tools/testing/selftests/kvm/Makefile.kvm
> @@ -262,7 +262,7 @@ CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \
>  	-fno-stack-protector -fno-PIE -fno-strict-aliasing \
>  	-I$(LINUX_TOOL_INCLUDE) -I$(LINUX_TOOL_ARCH_INCLUDE) \
>  	-I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(ARCH) \
> -	-I ../rseq -I.. $(EXTRA_CFLAGS) $(KHDR_INCLUDES)
> +	-I ../rseq -I.. -I$(OUTPUT)/include/$(ARCH) $(EXTRA_CFLAGS) $(KHDR_INCLUDES)

Hrm, ya, though I assume we want to define e.g. KVM_GEN_HDRS so that they can be
added to the csope.  Note, ARM already has some generated header stuff, but the
generated code comes from outside of KVM selftests, so we'll want to make sure to
avoid a collision with GEN_HDRS, thus the KVM_ prefix.

	ifeq ($(ARCH),arm64)
	tools_dir := $(top_srcdir)/tools
	arm64_tools_dir := $(tools_dir)/arch/arm64/tools/

	ifneq ($(abs_objdir),)
	arm64_hdr_outdir := $(abs_objdir)/tools/
	else
	arm64_hdr_outdir := $(tools_dir)/
	endif

	GEN_HDRS := $(arm64_hdr_outdir)arch/arm64/include/generated/
	CFLAGS += -I$(GEN_HDRS)

	$(GEN_HDRS): $(wildcard $(arm64_tools_dir)/*)
		$(MAKE) -C $(arm64_tools_dir) OUTPUT=$(arm64_hdr_outdir)
	endif

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm
  2025-10-28 21:20 ` [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm Sagi Shahar
@ 2025-10-30 22:20   ` Ira Weiny
  2025-10-31 16:03   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-30 22:20 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> TDX VMs need to issue the KVM_TDX_INIT_VM ioctl after VM creation to
> initialize the TD. This ioctl also sets the cpuids and attributes for
> the VM.
> 
> At this point we can also set the common boot parameters such as CR3,

Avoid 'we'.

> CR4, etc. These parameters will get copied to the relevant registers by
> the TD boot code trampoline.
> 
> Signed-off-by: Sagi Shahar <sagis@google.com>
> 
> ---------------------------------------------
> 
> Changes from v10:
>  * The call to vm_tdx_load_common_boot_parameters() was accidently
>    dropped as part of the refactor from v9 to v10. I re-added it here.

This can be dropped as well.

Ira

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types
  2025-10-28 21:20 ` [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types Sagi Shahar
  2025-10-29  1:13   ` Ira Weiny
  2025-10-29  6:57   ` Binbin Wu
@ 2025-10-31  3:42   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  3:42 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

Typo in subject: "so simplify" -> "to simplify"?

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index d3f3e455c031..310ec2b8afb7 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -209,6 +209,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)
> +
> +

Avoid multiple blank lines.

Reinette


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values
  2025-10-28 21:20 ` [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values Sagi Shahar
  2025-10-29  1:40   ` Ira Weiny
@ 2025-10-31  3:43   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  3:43 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> TDX can't set sregs values directly using KVM_SET_SREGS. Expose the
> default values of certain sregs used by TDX VMs so they can be set
> manually.
> 
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  .../selftests/kvm/include/x86/processor.h     | 33 +++++++++++++++++++
>  .../testing/selftests/kvm/lib/x86/processor.c | 12 +++----
>  2 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index dd21e11e1908..9caeb3de7df6 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> @@ -27,6 +27,10 @@ extern uint64_t guest_tsc_khz;
>  #define MAX_NR_CPUID_ENTRIES 100
>  #endif
>  
> +#ifndef NUM_INTERRUPTS
> +#define NUM_INTERRUPTS 256
> +#endif
> +

Can this duplicate snippet now be dropped from
tools/testing/selftests/kvm/lib/x86/processor.c ?

Reinette


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 02/23] KVM: selftests: Allocate pgd in virt_map() as necessary
  2025-10-28 21:20 ` [PATCH v12 02/23] KVM: selftests: Allocate pgd in virt_map() as necessary Sagi Shahar
@ 2025-10-31  3:51   ` Reinette Chatre
  0 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  3:51 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> If virt_map() is called before any call to ____vm_vaddr_alloc() it
> will create the mapping using an invalid pgd.

If virt_map() is currently called before any call to ____vm_vaddr_alloc()
then this patch would be a bugfix ... but it is not a bugfix so this
"if" does not seem relevant to existing code but a future change for which
this changelog does not provide information.
> Add call to virt_pgd_alloc() as part of virt_map() before creating the
> mapping, similarly to ____vm_vaddr_alloc()

The changelog is missing "why" this change is needed. Yes, it mentions the
hypothetical scenario that it aims to address but it would be helpful to
be specific about what this is a preparatory change for. What scenario would
require virt_map() to be called before ____vm_vaddr_alloc()?

Alternatively, would this patch not be unnecessary if
vm_tdx_setup_boot_code_region() is moved to be after kvm_vm_elf_load()?
As it is written (looking ahead at patch #16) it looks like the TDX boot region
creation and initialization is between slot 0's creation and initialization.

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack
  2025-10-28 21:20 ` [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack Sagi Shahar
  2025-10-29 13:24   ` Ira Weiny
@ 2025-10-31  3:52   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  3:52 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> TDX guests' registers cannot be initialized directly using

Previous patch used the term "TDX VMs". It will make the changelogs easier to
read if the same terms are used consistently.

> vcpu_regs_set(), hence the stack pointer needs to be initialized by
> the guest itself, running boot code beginning at the reset vector.

Sean highlighted in https://lore.kernel.org/lkml/aQN0Qg24tMQ9ckUT@google.com/
that the changelog requirements for selftests should follow
Documentation/process/maintainer-kvm-x86.rst. This means that the changelogs
should start with a short description of the change followed by the context
and problem description (if needed).

> 
> Expose the function to allocate the guest stack so that TDX
> initialization code can allocate it itself and skip the allocation in
> vm_arch_vcpu_add() in that case.

TDX still allocates the stack in vm_arch_vcpu_add() though, no?

Perhaps something like below (caveat is that KVM style is new to me
also so consider this a draft):

	Introduce kvm_allocate_vcpu_stack() to allocate a vCPU's stack
	in preparation for TDX to allocate a vCPU's stack and initialize
	its stack pointer.

	TDX VMs' registers are protected state and cannot be initialized
	using the KVM_SET_REGS ioctl() that is used for normal VMs. A TDX
	vCPU's stack address will be a property of the TDX specific boot code
	that initializes the vCPUs' stack pointers at boot. 

> 
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  .../selftests/kvm/include/x86/processor.h        |  2 ++
>  tools/testing/selftests/kvm/lib/x86/processor.c  | 16 +++++++++++-----
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
> index 9caeb3de7df6..dba2b3d558d1 100644
> --- a/tools/testing/selftests/kvm/include/x86/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86/processor.h
> @@ -1120,6 +1120,8 @@ static inline void vcpu_clear_cpuid_feature(struct kvm_vcpu *vcpu,
>  	vcpu_set_or_clear_cpuid_feature(vcpu, feature, false);
>  }
>  
> +vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm);
> +
>  uint64_t vcpu_get_msr(struct kvm_vcpu *vcpu, uint64_t msr_index);
>  int _vcpu_set_msr(struct kvm_vcpu *vcpu, uint64_t msr_index, uint64_t msr_value);
>  
> diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
> index 2d1544e8af6c..2898fe4f6de4 100644
> --- a/tools/testing/selftests/kvm/lib/x86/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86/processor.c
> @@ -693,12 +693,9 @@ void vcpu_arch_set_entry_point(struct kvm_vcpu *vcpu, void *guest_code)
>  	vcpu_regs_set(vcpu, &regs);
>  }
>  
> -struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
> +vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm)
>  {
> -	struct kvm_mp_state mp_state;
> -	struct kvm_regs regs;
>  	vm_vaddr_t stack_vaddr;
> -	struct kvm_vcpu *vcpu;
>  
>  	stack_vaddr = __vm_vaddr_alloc(vm, DEFAULT_STACK_PGS * getpagesize(),
>  				       DEFAULT_GUEST_STACK_VADDR_MIN,
> @@ -719,6 +716,15 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
>  		    "__vm_vaddr_alloc() did not provide a page-aligned address");
>  	stack_vaddr -= 8;
>  
> +	return stack_vaddr;
> +}
> +
> +struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
> +{
> +	struct kvm_mp_state mp_state;
> +	struct kvm_regs regs;
> +	struct kvm_vcpu *vcpu;

Even though the original code did not do so I'd propose these declarations be in
reverse fir order.

> +
>  	vcpu = __vm_vcpu_add(vm, vcpu_id);
>  	vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
>  	vcpu_init_sregs(vm, vcpu);
> @@ -727,7 +733,7 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id)
>  	/* Setup guest general purpose registers */
>  	vcpu_regs_get(vcpu, &regs);
>  	regs.rflags = regs.rflags | 0x2;
> -	regs.rsp = stack_vaddr;
> +	regs.rsp = kvm_allocate_vcpu_stack(vm);
>  	vcpu_regs_set(vcpu, &regs);
>  
>  	/* Setup the MP state */

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX
  2025-10-28 21:20 ` [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX Sagi Shahar
  2025-10-29 15:22   ` Ira Weiny
@ 2025-10-31  3:53   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  3:53 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm, Adrian Hunter

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Let kvm_init_vm_address_properties() initialize vm->arch.{s_bit, tag_mask}
> similar to SEV.

To me this seems like a verbatim description of what can be seen from the
patch self. How about something like:
	Initialize which GPA bit a TDX VM uses to tag shared memory in guest page
	tables.

> 
> TDX sets the shared bit based on the guest physical address width and
> currently supports 48 and 52 widths.
> 

...

> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
> new file mode 100644
> index 000000000000..286d5e3c24b1
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdx_util.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTESTS_TDX_TDX_UTIL_H
> +#define SELFTESTS_TDX_TDX_UTIL_H
> +
> +#include <stdbool.h>
> +
> +#include "kvm_util.h"
> +
> +static inline bool is_tdx_vm(struct kvm_vm *vm)
> +{
> +	return vm->type == KVM_X86_TDX_VM;
> +}
> +
> +#endif // SELFTESTS_TDX_TDX_UTIL_H

I recommend this work sticks to using /* ... */ for single line comments.
For reference you can compare the custom of all the other header files in this
area.

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 06/23] KVM: selftests: Expose segment definitons to assembly files
  2025-10-28 21:20 ` [PATCH v12 06/23] KVM: selftests: Expose segment definitons to assembly files Sagi Shahar
@ 2025-10-31  3:54   ` Reinette Chatre
  0 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  3:54 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

typo in subject: definitons -> definitions

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> Move kernel segment definitions to a separate file which can be included
> from assembly files.
> 
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  .../selftests/kvm/include/x86/processor_asm.h        | 12 ++++++++++++
>  tools/testing/selftests/kvm/lib/x86/processor.c      |  5 +----
>  2 files changed, 13 insertions(+), 4 deletions(-)
>  create mode 100644 tools/testing/selftests/kvm/include/x86/processor_asm.h
> 
> diff --git a/tools/testing/selftests/kvm/include/x86/processor_asm.h b/tools/testing/selftests/kvm/include/x86/processor_asm.h
> new file mode 100644
> index 000000000000..7e5386a85ca8
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86/processor_asm.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Used for storing defines used by both processor.c and assembly code.

I recommend replacing "processor.c" with "c" ... having filenames in
headers become stale quite fast. "Used for storing" could also be dropped
to keep it brief.

> + */
> +#ifndef SELFTEST_KVM_PROCESSOR_ASM_H
> +#define SELFTEST_KVM_PROCESSOR_ASM_H
> +
> +#define KERNEL_CS	0x8
> +#define KERNEL_DS	0x10
> +#define KERNEL_TSS	0x18
> +
> +#endif  // SELFTEST_KVM_PROCESSOR_ASM_H

(use /* ... */ ... please consider for whole series.)

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 07/23] KVM: selftests: Add kbuild definitons
  2025-10-28 21:20 ` [PATCH v12 07/23] KVM: selftests: Add kbuild definitons Sagi Shahar
  2025-10-29  7:43   ` Binbin Wu
  2025-10-29 15:55   ` Ira Weiny
@ 2025-10-31  3:56   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  3:56 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

typo in subject: definitons -> definitions

This is not actually a KVM selftest change though but an addition to
core tools. I do not know if such an addition can flow via the KVM tree but I
really do not think that it should be disguised as a KVM change as the
subject implies.

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> Add kbuild.h that can be used by files under tools/

Similar to earlier feedback this is obvious from the patch self.

> 
> Definitions are taken from the original definitions at
> include/linux/kbuild.h

Always write in imperative mood.

> 
> This is needed to expose values from c code to assembly code.

I do not think this description is a strong motivation.
Another draft for consideration:

	Add the kbuild definitions to enable a tool to use
	the kbuild filechk_offset script to generate C header files
	containing structure member offset information.
	
	Tools depending on assembly code that operates on structures
	need to hardcode the offsets of structure members. The kernel's
	kbuild scripts can instead generate C header files with offset
	information for inclusion into assembly code. 

Reinette


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code
  2025-10-28 21:20 ` [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code Sagi Shahar
  2025-10-29 16:37   ` Reinette Chatre
@ 2025-10-31  4:01   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  4:01 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

subject nit: Define -> Declare

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> TDX registers are inaccessible to KVM. Therefore we need a different

Avoid impersonating code by using "we".

> mechanism to load boot parameters for TDX code. TDX boot code will read
> the registers values from memory and set the registers manually.

Above seems to be a mix of context and summary of change. It looks like
the changelogs of this series need to be reworked to meet the KVM requirements
documented in "Changelog" section of  Documentation/process/maintainer-kvm-x86.rst
highlighted by Sean.

> 
> This patch defines the data structures used to communicate between c

Avoid using "this patch" - it is redundant.

> code and the TDX assembly boot code which will be added in a later
> patch.
> 
> Use kbuild.h to expose the offsets into the structs from c code to
> assembly code.

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM
  2025-10-28 21:20 ` [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM Sagi Shahar
  2025-10-29 21:16   ` Ira Weiny
  2025-10-30  1:25   ` Binbin Wu
@ 2025-10-31  4:06   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31  4:06 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:

> 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 dafdc7e46abe..a2509959c7ce 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,60 @@ static inline bool is_tdx_vm(struct kvm_vm *vm)
>  	return vm->type == KVM_X86_TDX_VM;
>  }
>  
> +/*
> + * TDX ioctls
> + */
> +
> +#define __vm_tdx_vm_ioctl(vm, cmd, metadata, arg)			\
> +({									\
> +	int r;								\
> +									\
> +	union {								\
> +		struct kvm_tdx_cmd c;					\
> +		unsigned long raw;					\
> +	} tdx_cmd = { .c = {						\
> +		.id = (cmd),						\
> +		.flags = (uint32_t)(metadata),				\
> +		.data = (uint64_t)(arg),				\
> +	} };								\
> +									\
> +	r = __vm_ioctl(vm, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);	\
> +	r ?: tdx_cmd.c.hw_error;					\
> +})
> +
> +#define vm_tdx_vm_ioctl(vm, cmd, flags, arg)				\
> +({									\
> +	int ret = __vm_tdx_vm_ioctl(vm, cmd, flags, arg);		\
> +									\
> +	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd,	ret, vm);		\
> +})
> +
> +#define __vm_tdx_vcpu_ioctl(vcpu, cmd, metadata, arg)			\
> +({									\
> +	int r;								\
> +									\
> +	union {								\
> +		struct kvm_tdx_cmd c;					\
> +		unsigned long raw;					\
> +	} tdx_cmd = { .c = {						\
> +		.id = (cmd),						\
> +		.flags = (uint32_t)(metadata),				\
> +		.data = (uint64_t)(arg),				\
> +	} };								\
> +									\
> +	r = __vcpu_ioctl(vcpu, KVM_MEMORY_ENCRYPT_OP, &tdx_cmd.raw);	\
> +	r ?: tdx_cmd.c.hw_error;					\
> +})
> +
> +#define vm_tdx_vcpu_ioctl(vcpu, cmd, flags, arg)			\
> +({									\
> +	int ret = __vm_tdx_vcpu_ioctl(vcpu, cmd, flags, arg);		\
> +									\
> +	__TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, (vcpu)->vm);	\
> +})
> +
> +void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes);
> +
>  void vm_tdx_setup_boot_code_region(struct kvm_vm *vm);
>  void vm_tdx_setup_boot_parameters_region(struct kvm_vm *vm, uint32_t nr_runnable_vcpus);
>  void vm_tdx_load_common_boot_parameters(struct kvm_vm *vm);

For completeness to help with discussion below other patches add:
  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);


When considering the TDX functions in tdx_util.h visible above the namespace of
TDX related functions is not clear to me. I believe an intuitive namespace
makes the code easier to understand and build upon.

Almost all tdx_util.h functions appear to have the "vm_tdx" prefix even when they just operate on a vCPU scope,
for example:
	void vm_tdx_set_vcpu_entry_point(struct kvm_vcpu *vcpu, void *guest_code);
	and
	vm_tdx_vcpu_ioctl()

Also, when operating on a VM there may be an extra "vm" added to create a function like
vm_tdx_vm_ioctl() with two "vm" in its name.

Compare with similar functions for normal VMs:

	vm_ioctl()	->	vm_tdx_vm_ioctl()
	vcpu_ioctl()	->	vm_tdx_vcpu_ioctl()

Could it not perhaps instead be:

	vm_ioctl()	->	tdx_vm_ioctl()
	vcpu_ioctl()	->	tdx_vcpu_ioctl()


The functions could still have "vm"/"vcpu" in their name to designate the scope, for example:
	void tdx_vm_setup_boot_code_region(struct kvm_vm *vm);
	void tdx_vm_setup_boot_parameters_region(struct kvm_vm *vm, uint32_t nr_runnable_vcpus);
	void tdx_vm_load_common_boot_parameters(struct kvm_vm *vm);
	void tdx_vcpu_load_boot_parameters(struct kvm_vm *vm, struct kvm_vcpu *vcpu);
	void tdx_vcpu_set_entry_point(struct kvm_vcpu *vcpu, void *guest_code);
	void tdx_vm_finalize(struct kvm_vm *vm);

With a namespace like above it is clear that (a) it is a TDX call and (b) what the scope of the
call is. This helps to understand what the code does while reading it and makes clear how to
name new functions when adding new features.

...

> +/*
> + * Filter CPUID based on TDX supported capabilities
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   cpuid_data - CPUID fileds to filter

fileds -> fields?

> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * For each CPUID leaf, filter out non-supported bits based on the capabilities reported
> + * by the TDX module
> + */
> +static void vm_tdx_filter_cpuid(struct kvm_vm *vm,
> +				struct kvm_cpuid2 *cpuid_data)
> +{
> +	struct kvm_tdx_capabilities *tdx_cap;
> +	struct kvm_cpuid_entry2 *config;
> +	struct kvm_cpuid_entry2 *e;
> +	int i;
> +
> +	tdx_cap = tdx_read_capabilities(vm);
> +
> +	i = 0;
> +	while (i < cpuid_data->nent) {
> +		e = cpuid_data->entries + i;
> +		config = tdx_find_cpuid_config(tdx_cap, e->function, e->index);
> +
> +		if (!config) {
> +			int left = cpuid_data->nent - i - 1;
> +
> +			if (left > 0)
> +				memmove(cpuid_data->entries + i,
> +					cpuid_data->entries + i + 1,
> +					sizeof(*cpuid_data->entries) * left);
> +			cpuid_data->nent--;
> +			continue;
> +		}
> +
> +		e->eax &= config->eax;
> +		e->ebx &= config->ebx;
> +		e->ecx &= config->ecx;
> +		e->edx &= config->edx;
> +
> +		i++;
> +	}
> +
> +	free(tdx_cap);
> +}
> +
> +void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
> +{
> +	struct kvm_tdx_init_vm *init_vm;
> +	const struct kvm_cpuid2 *tmp;
> +	struct kvm_cpuid2 *cpuid;
> +
> +	tmp = kvm_get_supported_cpuid();
> +
> +	cpuid = allocate_kvm_cpuid2(MAX_NR_CPUID_ENTRIES);

Could this allocation be limited to tmp->nent?

> +	memcpy(cpuid, tmp, kvm_cpuid2_size(tmp->nent));
> +	vm_tdx_filter_cpuid(vm, cpuid);
> +
> +	init_vm = calloc(1, sizeof(*init_vm) +
> +			 sizeof(init_vm->cpuid.entries[0]) * cpuid->nent);
> +	TEST_ASSERT(init_vm, "init_vm allocation failed");
> +
> +	memcpy(&init_vm->cpuid, cpuid, kvm_cpuid2_size(cpuid->nent));
> +	free(cpuid);
> +
> +	init_vm->attributes = attributes;
> +
> +	vm_tdx_vm_ioctl(vm, KVM_TDX_INIT_VM, 0, init_vm);
> +
> +	free(init_vm);
> +}

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus
  2025-10-28 21:20 ` [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus Sagi Shahar
@ 2025-10-31 13:10   ` Ira Weiny
  2025-10-31 16:05   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-31 13:10 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> Call vm_tdx_finalize() as part of kvm_arch_vm_finalize_vcpus if this is
> a TDX vm
> 
> Signed-off-by: Sagi Shahar <sagis@google.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest
  2025-10-28 21:20 ` [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest Sagi Shahar
@ 2025-10-31 14:11   ` Ira Weiny
  2025-10-31 15:15     ` Sean Christopherson
  0 siblings, 1 reply; 69+ messages in thread
From: Ira Weiny @ 2025-10-31 14:11 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> From: Erdem Aktas <erdemaktas@google.com>
> 
> Add support for TDX guests to issue TDCALLs to the TDX module.

Generally it is nice to have more details.  As someone new to TDX I
have to remind myself what a TDCALL is.  And any random kernel developer
reading this in the future will likely have even less clue than me.

Paraphrased from the spec:

TDCALL is the instruction used by the guest TD software (in TDX non-root
mode) to invoke guest-side TDX functions.  TDG.VP.VMCALL helps invoke
services from the host VMM.

Add support for TDX guests to invoke services from the host VMM.

> 
> Signed-off-by: Erdem Aktas <erdemaktas@google.com>
> Co-developed-by: Sagi Shahar <sagis@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---

[snip]

> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h
> new file mode 100644
> index 000000000000..60c70646f876
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/x86/tdx/tdcall.h

[snip]

> +
> +/*
> + * Used in __tdx_tdcall() to pass down and get back registers' values of
> + * the TDCALL instruction when requesting services from the VMM.
> + *
> + * This is a software only structure and not part of the TDX module/VMM ABI.

This is a good comment.

> + */
> +struct tdx_tdcall_args {
> +	u64 r10;
> +	u64 r11;
> +	u64 r12;
> +	u64 r13;
> +	u64 r14;
> +	u64 r15;
> +};
> +

[snip]

> +
> +/*
> + * Bitmasks of exposed registers (with VMM).
> + */
> +#define TDX_R10		BIT(10)
> +#define TDX_R11		BIT(11)
> +#define TDX_R12		BIT(12)
> +#define TDX_R13		BIT(13)
> +#define TDX_R14		BIT(14)
> +#define TDX_R15		BIT(15)
> +
> +/*
> + * These registers are clobbered to hold arguments for each
> + * TDVMCALL. They are safe to expose to the VMM.

I'm not sure what this comment means by being 'safe to expose to the VMM'?
They are all overwritten per the data specified correct?

> + * Each bit in this mask represents a register ID. Bit field
> + * details can be found in TDX GHCI specification, section
> + * titled "TDCALL [TDG.VP.VMCALL] leaf".

TDX GHCI specification v1.5, March 2023
2.4.1 TDCALL [TDG.VP.VMCALL] leaf

This nails down any issues which may arise if the module/spec changes.

Ira

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 21/23] KVM: selftests: Add wrapper for TDX MMIO from guest
  2025-10-28 21:20 ` [PATCH v12 21/23] KVM: selftests: Add wrapper for TDX MMIO " Sagi Shahar
@ 2025-10-31 14:21   ` Ira Weiny
  0 siblings, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-31 14:21 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> Add utility function to issue MMIO TDCALL from TDX guests.

Some detail from the spec would be nice here.

[snip]

> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
> new file mode 100644
> index 000000000000..f9c1acd5b30c
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "tdx/tdcall.h"
> +#include "tdx/tdx.h"
> +
> +#define TDG_VP_VMCALL 0
> +
> +#define TDG_VP_VMCALL_VE_REQUEST_MMIO	48
> +
> +uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,

Size is limited to specific values.  An enum would help here to self
document/limit issues.

enum mmio_write_size {
	MMIO_SIZE_1B,
	MMIO_SIZE_2B,
	MMIO_SIZE_4B,
	MMIO_SIZE_8B
};

Ira

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX
  2025-10-28 21:20 ` [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX Sagi Shahar
@ 2025-10-31 14:38   ` Ira Weiny
  2025-10-31 15:55   ` Sean Christopherson
  1 sibling, 0 replies; 69+ messages in thread
From: Ira Weiny @ 2025-10-31 14:38 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Reinette Chatre,
	Ira Weiny, Chao Gao, Chenyi Qiang
  Cc: linux-kernel, kvm

Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> ucalls for non-Coco VMs work by having the guest write to the rdi
> register, then perform an io instruction to exit to the host. The host
> then reads rdi using kvm_get_regs().
> 
> CPU registers can't be read using kvm_get_regs() for TDX, so TDX
> guests use MMIO to pass the struct ucall's hva to the host. MMIO was
> chosen because it is one of the simplest (hence unlikely to fail)
> mechanisms that support passing 8 bytes from guest to host.
> 

[snip]

> index 1265cecc7dd1..fae6f37b0bcd 100644
> --- a/tools/testing/selftests/kvm/lib/x86/ucall.c
> +++ b/tools/testing/selftests/kvm/lib/x86/ucall.c
> @@ -5,11 +5,35 @@
>   * Copyright (C) 2018, Red Hat, Inc.
>   */
>  #include "kvm_util.h"
> +#include "tdx/tdx.h"
> +#include "tdx/tdx_util.h"
>  
>  #define UCALL_PIO_PORT ((uint16_t)0x1000)
>  
> +static uint8_t vm_type;
> +static vm_paddr_t host_ucall_mmio_gpa;
> +static vm_paddr_t ucall_mmio_gpa;
> +
> +void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
> +{
> +	vm_type = vm->type;
> +	sync_global_to_guest(vm, vm_type);
> +
> +	if (is_tdx_vm(vm)) {
> +		host_ucall_mmio_gpa = ucall_mmio_gpa = mmio_gpa;
> +		ucall_mmio_gpa |= vm->arch.s_bit;
> +	}
> +
> +	sync_global_to_guest(vm, ucall_mmio_gpa);

Is this needed for non-tdx VMs?

Ira

[snip]

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest
  2025-10-31 14:11   ` Ira Weiny
@ 2025-10-31 15:15     ` Sean Christopherson
  2025-10-31 15:58       ` Sagi Shahar
  2025-10-31 16:01       ` Sean Christopherson
  0 siblings, 2 replies; 69+ messages in thread
From: Sean Christopherson @ 2025-10-31 15:15 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Binbin Wu, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Chao Gao, Chenyi Qiang,
	linux-kernel, kvm

On Fri, Oct 31, 2025, Ira Weiny wrote:
> Sagi Shahar wrote:
> > From: Erdem Aktas <erdemaktas@google.com>
> > 
> > Add support for TDX guests to issue TDCALLs to the TDX module.
> 
> Generally it is nice to have more details.  As someone new to TDX I
> have to remind myself what a TDCALL is.  And any random kernel developer
> reading this in the future will likely have even less clue than me.
> 
> Paraphrased from the spec:
> 
> TDCALL is the instruction used by the guest TD software (in TDX non-root
> mode) to invoke guest-side TDX functions.  TDG.VP.VMCALL helps invoke
> services from the host VMM.
> 
> Add support for TDX guests to invoke services from the host VMM.

Eh, at some point a baseline amount of knowledge is required.  I highly doubt
regurgitating the spec is going to make a huge difference

I also dislike the above wording, because it doesn't help understand _why_ KVM
selftests need to support TDCALL, or _how_ the functionality will be utilized.
E.g. strictly speaking, we could write KVM selftests without ever doing a single
TDG.VP.VMCALL, because we control both sides (guest and VMM).  And I have a hard
time belive name-dropping TDG.VP.VMCALL is going to connect the dots between
TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation
of "legacy" functionality".

What I would like to know is why selftests are copy-pasting the kernel's scheme
for marshalling data to/from the registers used by TDCALL, how selftests are
expected to utilize TDCALL, etc.  I'm confident that if someone actually took the
time to write a changelog explaining those details, then what TDCALL "is" will
be fairly clear, even if the reader doesn't know exactly what it is.

E.g. IMO this is ugly and lazy on multiple fronts:

uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
                                            uint64_t data_in)
{
       struct tdx_tdcall_args args = {
               .r10 = TDG_VP_VMCALL,
               .r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
               .r12 = size,
               .r13 = MMIO_WRITE,
               .r14 = address,
               .r15 = data_in,
       };

       return __tdx_tdcall(&args, 0);
}

First, these are KVM selftests, there's no need to provide a super fancy namespace
because we are "competing" with thousands upon thousands of lines of code from
other components and subsystems.

Similarly, tdg_vp_vmcall_ve_request_mmio_write() is absurdly verbose.  Referencing
#VE in any way is also flat out wrong.

It's also far too specific to TDX, which is going to be problematic when full
support for SEV-ES+ selftests comes along.  I.e. calling this from common code
is going to be a pain in the rear, bordering on unworkable.

And related to your comment about having enums for the sizes, there's absolutely
zero reason the caller should have to specify the size.

In short, don't simply copy what was done for the kernel.  The kernel is operating
under constraints that do not and should not ever apply to KVM selftests.  Except
for tests like set_memory_region_test.c that delete memslots while a vCPU is running
and thus _may_ generate MMIO accesses, our selftests should never, ever take a #VE
(or #VC) and then request MMIO in the handler.  If a test wants to do MMIO, then
do MMIO.

So, I want to see GUEST_MMIO_WRITE() and GUEST_MMIO_READ(), or probably even just
MMIO_WRITE() and MMIO_READ().  And then under the hood, wire up kvm_arch_mmio_write()
and kvm_arch_mmio_read() in kvm_util_arch.h.  And from there have x86 globally track
if it's TDX, SEV-ES+, or "normal".  That'd also give us a good reason+way to assert
on s390 if a test attempts MMIO, as s390 doesn't support emulated MMIO.

One potential hiccup is if/when KVM selftests get access to actual MMIO, i.e. don't
want to trigger emulation, e.g. for VFIO related selftests when accessing BARs.
Though the answer there is probably to just use WRITE/READ_ONCE() and call it good.

E.g.

#define MMIO_WRITE(addr, val)					\
	kvm_arch_mmio_write(addr, val);

#define kvm_arch_mmio_write(addr, val)				\
({								\
	if (guest_needs_tdvmcall)				\
		tdx_mmio_write(addr, val, sizeof(val));		\
	else if (guest_needs_vmgexit)				\
		sev_mmio_write(addr, val, sizeof(val));		\
	else							\
		WRITE_ONCE(addr, val);				\
})

#define MMIO_READ(addr, val)					\
	kvm_arch_mmio_read(addr, val);

#define kvm_arch_mmio_read(addr, val)				\
({								\
	if (guest_needs_tdvmcall)				\
		tdx_mmio_read(addr, &(val), sizeof(val));	\
	else if (guest_needs_vmgexit)				\
		sev_mmio_write(addr, &(val), sizeof(val));	\
	else							\
		(val) = READ_ONCE(addr);			\
})


^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX
  2025-10-28 21:20 ` [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX Sagi Shahar
  2025-10-31 14:38   ` Ira Weiny
@ 2025-10-31 15:55   ` Sean Christopherson
  1 sibling, 0 replies; 69+ messages in thread
From: Sean Christopherson @ 2025-10-31 15:55 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: linux-kselftest, Paolo Bonzini, Shuah Khan, Ackerley Tng,
	Ryan Afranji, Andrew Jones, Isaku Yamahata, Erdem Aktas,
	Rick Edgecombe, Roger Wang, Binbin Wu, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Ira Weiny, Chao Gao,
	Chenyi Qiang, linux-kernel, kvm

On Tue, Oct 28, 2025, Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> ucalls for non-Coco VMs work by having the guest write to the rdi
> register, then perform an io instruction to exit to the host. The host
> then reads rdi using kvm_get_regs().
> 
> CPU registers can't be read using kvm_get_regs() for TDX, so TDX
> guests use MMIO to pass the struct ucall's hva to the host. MMIO was
> chosen because it is one of the simplest (hence unlikely to fail)
> mechanisms that support passing 8 bytes from guest to host.

Uh, I beg to differ.  Stop following the GHCI verbatim.  The protocols defined by
the GHCB and GHCI specs are horrific, but necessary, evils.  They exist to define
guest<=>host ABIs+contracts so that guests can communicate with hypervisors,
without massive fragmentation in the ecosystem.  But as mentioned in an ealier
mail, KVM selftests don't care so much about ABIs and contracts because we control
both the guest and the host.

The GHCI matters only for the guest<=>KVM contract, it matters not at all for
guest<=>VMM communication for KVM selfetsts.

Simply set RCX (the mask of GPRs to preserve) to the maximal value for _all_
TDG.VP.VMCALL invocations.  There's zero reason for KVM selftests guests to hide
state from the host.  Then TDX guests can do port I/O and pass the address of the
ucall structure in RDX, just as regular guests do.  I.e. there's zero need to
change ucall_arch_get_ucall(), at all.  We'll want to modify ucall_arch_do_ucall()
so that we don't need to wire up a #VE handler for every test, but that's easy
enough to do.

Side topic, that's also one of the easiest TDX selftests that can be written:
verify the TDX-Module and KVM honor the spec and preserve registers according to
RCX, e.g. that KVM doesn't clobber registers just because they're not defined to
some magic purpose in the GHCI.

SEV-ES+ will need to come up with a slightly different approach because there's
no way to automagically expose RDI to the host, but that's an SEV-ES+ problem.

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest
  2025-10-31 15:15     ` Sean Christopherson
@ 2025-10-31 15:58       ` Sagi Shahar
  2025-10-31 16:12         ` Sean Christopherson
  2025-10-31 16:01       ` Sean Christopherson
  1 sibling, 1 reply; 69+ messages in thread
From: Sagi Shahar @ 2025-10-31 15:58 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Ira Weiny, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Binbin Wu, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Chao Gao, Chenyi Qiang,
	linux-kernel, kvm

On Fri, Oct 31, 2025 at 10:15 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Oct 31, 2025, Ira Weiny wrote:
> > Sagi Shahar wrote:
> > > From: Erdem Aktas <erdemaktas@google.com>
> > >
> > > Add support for TDX guests to issue TDCALLs to the TDX module.
> >
> > Generally it is nice to have more details.  As someone new to TDX I
> > have to remind myself what a TDCALL is.  And any random kernel developer
> > reading this in the future will likely have even less clue than me.
> >
> > Paraphrased from the spec:
> >
> > TDCALL is the instruction used by the guest TD software (in TDX non-root
> > mode) to invoke guest-side TDX functions.  TDG.VP.VMCALL helps invoke
> > services from the host VMM.
> >
> > Add support for TDX guests to invoke services from the host VMM.
>
> Eh, at some point a baseline amount of knowledge is required.  I highly doubt
> regurgitating the spec is going to make a huge difference
>
> I also dislike the above wording, because it doesn't help understand _why_ KVM
> selftests need to support TDCALL, or _how_ the functionality will be utilized.
> E.g. strictly speaking, we could write KVM selftests without ever doing a single
> TDG.VP.VMCALL, because we control both sides (guest and VMM).  And I have a hard
> time belive name-dropping TDG.VP.VMCALL is going to connect the dots between
> TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation
> of "legacy" functionality".
>
> What I would like to know is why selftests are copy-pasting the kernel's scheme
> for marshalling data to/from the registers used by TDCALL, how selftests are
> expected to utilize TDCALL, etc.  I'm confident that if someone actually took the
> time to write a changelog explaining those details, then what TDCALL "is" will
> be fairly clear, even if the reader doesn't know exactly what it is.
>
> E.g. IMO this is ugly and lazy on multiple fronts:

To give some context to why this was done this way: Part of the reason
for the selftests is to test the GHCI protocol itself. Some of the
selftests will issue calls with purposely invalid arguments to ensure
KVM handles these cases properly. For example, issuing a port IO calls
with sizes other than 1,2 or 4 and ensure we get an error on the guest
side.

The code was intentionally written to be specific to TDX so we can
test the TDX GHCI spec itself.

As I understand it, you want the selftests to operate at a higher
level and abstract away the specific GHCI details so that the code can
be shared between TDX and SEV. I can refactor the code to abstract
away implementation details. However, tests that want to exercise the
API at a fine-grained level to test different arguments will need to
define these TDCALLs themselves.

These calls were placed in a header that can be included in the guest
code. I can add higher level wrappers that can be used for common
code.

>
> uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
>                                             uint64_t data_in)
> {
>        struct tdx_tdcall_args args = {
>                .r10 = TDG_VP_VMCALL,
>                .r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
>                .r12 = size,
>                .r13 = MMIO_WRITE,
>                .r14 = address,
>                .r15 = data_in,
>        };
>
>        return __tdx_tdcall(&args, 0);
> }
>
> First, these are KVM selftests, there's no need to provide a super fancy namespace
> because we are "competing" with thousands upon thousands of lines of code from
> other components and subsystems.
>
> Similarly, tdg_vp_vmcall_ve_request_mmio_write() is absurdly verbose.  Referencing
> #VE in any way is also flat out wrong.

This name was taken from the GHCI spec: TDG.VP.VMCALL<#VE.RequestMMIO>
("Intel TDX Guest-Hypervisor Communication Interface v1.5" section 3.7)

>
> It's also far too specific to TDX, which is going to be problematic when full
> support for SEV-ES+ selftests comes along.  I.e. calling this from common code
> is going to be a pain in the rear, bordering on unworkable.
>
> And related to your comment about having enums for the sizes, there's absolutely
> zero reason the caller should have to specify the size.
>
> In short, don't simply copy what was done for the kernel.  The kernel is operating
> under constraints that do not and should not ever apply to KVM selftests.  Except
> for tests like set_memory_region_test.c that delete memslots while a vCPU is running
> and thus _may_ generate MMIO accesses, our selftests should never, ever take a #VE
> (or #VC) and then request MMIO in the handler.  If a test wants to do MMIO, then
> do MMIO.
>
> So, I want to see GUEST_MMIO_WRITE() and GUEST_MMIO_READ(), or probably even just
> MMIO_WRITE() and MMIO_READ().  And then under the hood, wire up kvm_arch_mmio_write()
> and kvm_arch_mmio_read() in kvm_util_arch.h.  And from there have x86 globally track
> if it's TDX, SEV-ES+, or "normal".  That'd also give us a good reason+way to assert
> on s390 if a test attempts MMIO, as s390 doesn't support emulated MMIO.
>
> One potential hiccup is if/when KVM selftests get access to actual MMIO, i.e. don't
> want to trigger emulation, e.g. for VFIO related selftests when accessing BARs.
> Though the answer there is probably to just use WRITE/READ_ONCE() and call it good.
>
> E.g.
>
> #define MMIO_WRITE(addr, val)                                   \
>         kvm_arch_mmio_write(addr, val);
>
> #define kvm_arch_mmio_write(addr, val)                          \
> ({                                                              \
>         if (guest_needs_tdvmcall)                               \
>                 tdx_mmio_write(addr, val, sizeof(val));         \
>         else if (guest_needs_vmgexit)                           \
>                 sev_mmio_write(addr, val, sizeof(val));         \
>         else                                                    \
>                 WRITE_ONCE(addr, val);                          \
> })
>
> #define MMIO_READ(addr, val)                                    \
>         kvm_arch_mmio_read(addr, val);
>
> #define kvm_arch_mmio_read(addr, val)                           \
> ({                                                              \
>         if (guest_needs_tdvmcall)                               \
>                 tdx_mmio_read(addr, &(val), sizeof(val));       \
>         else if (guest_needs_vmgexit)                           \
>                 sev_mmio_write(addr, &(val), sizeof(val));      \
>         else                                                    \
>                 (val) = READ_ONCE(addr);                        \
> })
>

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM
  2025-10-28 21:20 ` [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM Sagi Shahar
  2025-10-29 21:27   ` Ira Weiny
  2025-10-30  2:32   ` Binbin Wu
@ 2025-10-31 15:58   ` Reinette Chatre
  2 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31 15:58 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, 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 2551b3eac8f8..53cfadeff8de 100644
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> @@ -270,3 +270,61 @@ void vm_tdx_init_vm(struct kvm_vm *vm, uint64_t attributes)
>  
>  	free(init_vm);
>  }
> +
> +static void tdx_init_mem_region(struct kvm_vm *vm, void *source_pages,
> +				uint64_t gpa, uint64_t size)
> +{
> +	uint32_t metadata = KVM_TDX_MEASURE_MEMORY_REGION;
> +	struct kvm_tdx_init_mem_region mem_region = {
> +		.source_addr = (uint64_t)source_pages,
> +		.gpa = gpa,
> +		.nr_pages = size / PAGE_SIZE,
> +	};
> +	struct kvm_vcpu *vcpu;
> +
> +	vcpu = list_first_entry_or_null(&vm->vcpus, struct kvm_vcpu, list);
> +
> +	TEST_ASSERT((mem_region.nr_pages > 0) &&
> +		    ((mem_region.nr_pages * PAGE_SIZE) == size),
> +		    "Cannot add partial pages to the guest memory.\n");
> +	TEST_ASSERT(((uint64_t)source_pages & (PAGE_SIZE - 1)) == 0,
> +		    "Source memory buffer is not page aligned\n");

IS_ALIGNED() helper may be used here?

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest
  2025-10-31 15:15     ` Sean Christopherson
  2025-10-31 15:58       ` Sagi Shahar
@ 2025-10-31 16:01       ` Sean Christopherson
  1 sibling, 0 replies; 69+ messages in thread
From: Sean Christopherson @ 2025-10-31 16:01 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Binbin Wu, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Chao Gao, Chenyi Qiang,
	linux-kernel, kvm

On Fri, Oct 31, 2025, Sean Christopherson wrote:
> On Fri, Oct 31, 2025, Ira Weiny wrote:
> > Sagi Shahar wrote:
> > > From: Erdem Aktas <erdemaktas@google.com>
> > > 
> > > Add support for TDX guests to issue TDCALLs to the TDX module.
> > 
> > Generally it is nice to have more details.  As someone new to TDX I
> > have to remind myself what a TDCALL is.  And any random kernel developer
> > reading this in the future will likely have even less clue than me.
> > 
> > Paraphrased from the spec:
> > 
> > TDCALL is the instruction used by the guest TD software (in TDX non-root
> > mode) to invoke guest-side TDX functions.  TDG.VP.VMCALL helps invoke
> > services from the host VMM.
> > 
> > Add support for TDX guests to invoke services from the host VMM.
> 
> Eh, at some point a baseline amount of knowledge is required.  I highly doubt
> regurgitating the spec is going to make a huge difference
> 
> I also dislike the above wording, because it doesn't help understand _why_ KVM
> selftests need to support TDCALL, or _how_ the functionality will be utilized.
> E.g. strictly speaking, we could write KVM selftests without ever doing a single
> TDG.VP.VMCALL, because we control both sides (guest and VMM).  And I have a hard
> time belive name-dropping TDG.VP.VMCALL is going to connect the dots between
> TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation
> of "legacy" functionality".
> 
> What I would like to know is why selftests are copy-pasting the kernel's scheme
> for marshalling data to/from the registers used by TDCALL, 

I almost forgot.  I detest the "throw everything into a structure" approach,
which the kernel used largely so that it could share code between SEAMCALLs and
TDCALLs.  Unless there's a good reason no to, I would much rather have prototypes
like

  uint64_t __tdvmcall(<all the args>)
  uint64_t tdvmcall_1(uint64_t arg1);
  uint64_t tdvmcall_2(uint64_t arg1, uint64_t arg2);
  uint64_t tdvmcall_3(...);
  uint65_t tdvmcall_4(...);
  uint64_t tdvmcall_5(...);
  uint64_t tdvmcall_6(...);

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm
  2025-10-28 21:20 ` [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm Sagi Shahar
  2025-10-30 22:20   ` Ira Weiny
@ 2025-10-31 16:03   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31 16:03 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> TDX VMs need to issue the KVM_TDX_INIT_VM ioctl after VM creation to
> initialize the TD. This ioctl also sets the cpuids and attributes for
> the VM.

"TDX VMs need to issue the KVM_TDX_INIT_VM ioctl" ... take care with the language
here since it is not the VM that issues the ioctl() (same in patch #17 changelog).

"after VM creation to initialize the TD" ... is the switching between terms
("VM" and "TD") necessary? Always referring to the same "thing" using the same
term really helps to make the text easier to read.

Finally, please do stick to imperative tone. For example,
	Initialize the VM with the TDX specific parameters, such as guest CPUIDs
	emulated by the TDX module, that the VM can support.

Reinette



^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 18/23] KVM: selftests: Set entry point for TDX guest code
  2025-10-28 21:20 ` [PATCH v12 18/23] KVM: selftests: Set entry point for TDX guest code Sagi Shahar
@ 2025-10-31 16:03   ` Reinette Chatre
  0 siblings, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31 16:03 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> Since the rip register is inaccessible for TDX VMs, we need a different
> way to set the guest entry point for TDX VMs. This is done by writing
> the guest code address to a predefined location in the guest memory and
> loading it into rip as part of the TDX boot code.

Check the changelog for code impersonation, imperative tone, and matching the
KVM requirements.

> 
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  tools/testing/selftests/kvm/lib/x86/processor.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
> index 036875fe140f..17f5a381fe43 100644
> --- a/tools/testing/selftests/kvm/lib/x86/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86/processor.c
> @@ -691,9 +691,13 @@ void vcpu_arch_set_entry_point(struct kvm_vcpu *vcpu, void *guest_code)
>  {
>  	struct kvm_regs regs;
>  
> -	vcpu_regs_get(vcpu, &regs);
> -	regs.rip = (unsigned long) guest_code;
> -	vcpu_regs_set(vcpu, &regs);
> +	if (is_tdx_vm(vcpu->vm))
> +		vm_tdx_set_vcpu_entry_point(vcpu, guest_code);

Please use braces around both branches. (for reference
"Placing Braces and Spaces" in Documentation/process/coding-style.rst)

> +	else {
> +		vcpu_regs_get(vcpu, &regs);
> +		regs.rip = (unsigned long) guest_code;

You can drop the space after the cast above.

> +		vcpu_regs_set(vcpu, &regs);
> +	}
>  }
>  
>  vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm)

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus
  2025-10-28 21:20 ` [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus Sagi Shahar
  2025-10-31 13:10   ` Ira Weiny
@ 2025-10-31 16:05   ` Reinette Chatre
  1 sibling, 0 replies; 69+ messages in thread
From: Reinette Chatre @ 2025-10-31 16:05 UTC (permalink / raw)
  To: Sagi Shahar, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Sean Christopherson, Ackerley Tng, Ryan Afranji, Andrew Jones,
	Isaku Yamahata, Erdem Aktas, Rick Edgecombe, Roger Wang,
	Binbin Wu, Oliver Upton, Pratik R. Sampat, Ira Weiny, Chao Gao,
	Chenyi Qiang
  Cc: linux-kernel, kvm

Hi Sagi,

In subject, use () to indicate a function name: kvm_arch_vm_finalize_vcpus().
Even so, I think the subject can be improved to describe what the patch does
instead of describing what function is changed. For example,
	"Finalize TDX VM after creation to make it runnable"

On 10/28/25 2:20 PM, Sagi Shahar wrote:
> Call vm_tdx_finalize() as part of kvm_arch_vm_finalize_vcpus if this is
> a TDX vm

This needs a proper changelog. Above just writes out in words exactly what can be
seen from the patch.

> 
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  tools/testing/selftests/kvm/lib/x86/processor.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
> index 17f5a381fe43..09cc75ae8d26 100644
> --- a/tools/testing/selftests/kvm/lib/x86/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86/processor.c
> @@ -1360,3 +1360,9 @@ bool kvm_arch_has_default_irqchip(void)
>  {
>  	return true;
>  }
> +
> +void kvm_arch_vm_finalize_vcpus(struct kvm_vm *vm)
> +{
> +	if (is_tdx_vm(vm))
> +		vm_tdx_finalize(vm);
> +}

Reinette

^ permalink raw reply	[flat|nested] 69+ messages in thread

* Re: [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest
  2025-10-31 15:58       ` Sagi Shahar
@ 2025-10-31 16:12         ` Sean Christopherson
  0 siblings, 0 replies; 69+ messages in thread
From: Sean Christopherson @ 2025-10-31 16:12 UTC (permalink / raw)
  To: Sagi Shahar
  Cc: Ira Weiny, linux-kselftest, Paolo Bonzini, Shuah Khan,
	Ackerley Tng, Ryan Afranji, Andrew Jones, Isaku Yamahata,
	Erdem Aktas, Rick Edgecombe, Roger Wang, Binbin Wu, Oliver Upton,
	Pratik R. Sampat, Reinette Chatre, Chao Gao, Chenyi Qiang,
	linux-kernel, kvm

On Fri, Oct 31, 2025, Sagi Shahar wrote:
> On Fri, Oct 31, 2025 at 10:15 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, Oct 31, 2025, Ira Weiny wrote:
> > > Sagi Shahar wrote:
> > > > From: Erdem Aktas <erdemaktas@google.com>
> > > >
> > > > Add support for TDX guests to issue TDCALLs to the TDX module.
> > >
> > > Generally it is nice to have more details.  As someone new to TDX I
> > > have to remind myself what a TDCALL is.  And any random kernel developer
> > > reading this in the future will likely have even less clue than me.
> > >
> > > Paraphrased from the spec:
> > >
> > > TDCALL is the instruction used by the guest TD software (in TDX non-root
> > > mode) to invoke guest-side TDX functions.  TDG.VP.VMCALL helps invoke
> > > services from the host VMM.
> > >
> > > Add support for TDX guests to invoke services from the host VMM.
> >
> > Eh, at some point a baseline amount of knowledge is required.  I highly doubt
> > regurgitating the spec is going to make a huge difference
> >
> > I also dislike the above wording, because it doesn't help understand _why_ KVM
> > selftests need to support TDCALL, or _how_ the functionality will be utilized.
> > E.g. strictly speaking, we could write KVM selftests without ever doing a single
> > TDG.VP.VMCALL, because we control both sides (guest and VMM).  And I have a hard
> > time belive name-dropping TDG.VP.VMCALL is going to connect the dots between
> > TDCALL and the "tunneling" scheme defined by the GHCI for requesting emulation
> > of "legacy" functionality".
> >
> > What I would like to know is why selftests are copy-pasting the kernel's scheme
> > for marshalling data to/from the registers used by TDCALL, how selftests are
> > expected to utilize TDCALL, etc.  I'm confident that if someone actually took the
> > time to write a changelog explaining those details, then what TDCALL "is" will
> > be fairly clear, even if the reader doesn't know exactly what it is.
> >
> > E.g. IMO this is ugly and lazy on multiple fronts:
> 
> To give some context to why this was done this way: Part of the reason
> for the selftests is to test the GHCI protocol itself.

The only part of the protocol that we're actually testing is the guest<=>KVM
interaction.  Testing the guest<=>VMM interaction is self-referential, i.e. we're
validating that we implemented the guest and VMM sides correctly, which is all
kinds of silly.

> Some of the selftests will issue calls with purposely invalid arguments to
> ensure KVM handles these cases properly. For example, issuing a port IO calls
> with sizes other than 1,2 or 4 and ensure we get an error on the guest side.

That's fine, great in fact, but that's completely orthogonal to how selftests
implement the literal guest or VMM code.

> The code was intentionally written to be specific to TDX so we can
> test the TDX GHCI spec itself.

That's 100% possible without copy+pasting the kernel, and also 100% possible
while also providing sane, common interaces.

> As I understand it, you want the selftests to operate at a higher
> level and abstract away the specific GHCI details so that the code can
> be shared between TDX and SEV. 

No, I want us to think critically about what we're actually doing, and I want us
to write code that is maintainable and as easy to follow as possible.

> I can refactor the code to abstract away implementation details. However,
> tests that want to exercise the API at a fine-grained level to test different
> arguments will need to define these TDCALLs themselves.

It's not so much about abstracting details as it is about making it easy to write
tests.  Yes, some TDX-specific tests will need to know the gory details.  But in
the grand scheme, those will be a very tiny percentage of all KVM selftests.

E.g. in all likelihood there should be literally _one_ test to validate that KVM
and the TDX-Module honor the guest<=>KVM GHCI contract.  Or maybe one test per
instruction/operation.  Everything else, even tests that are TDX specific, should
not care one whit about the GHCI.

> These calls were placed in a header that can be included in the guest
> code. I can add higher level wrappers that can be used for common
> code.
> 
> >
> > uint64_t tdg_vp_vmcall_ve_request_mmio_write(uint64_t address, uint64_t size,
> >                                             uint64_t data_in)
> > {
> >        struct tdx_tdcall_args args = {
> >                .r10 = TDG_VP_VMCALL,
> >                .r11 = TDG_VP_VMCALL_VE_REQUEST_MMIO,
> >                .r12 = size,
> >                .r13 = MMIO_WRITE,
> >                .r14 = address,
> >                .r15 = data_in,
> >        };
> >
> >        return __tdx_tdcall(&args, 0);
> > }
> >
> > First, these are KVM selftests, there's no need to provide a super fancy namespace
> > because we are "competing" with thousands upon thousands of lines of code from
> > other components and subsystems.
> >
> > Similarly, tdg_vp_vmcall_ve_request_mmio_write() is absurdly verbose.  Referencing
> > #VE in any way is also flat out wrong.
> 
> This name was taken from the GHCI spec: TDG.VP.VMCALL<#VE.RequestMMIO>
> ("Intel TDX Guest-Hypervisor Communication Interface v1.5" section 3.7)

I know, and I'm saying throw away the GHCI except for when it's absolutely necessary
to care what the GHCI says.

^ permalink raw reply	[flat|nested] 69+ messages in thread

end of thread, other threads:[~2025-10-31 16:12 UTC | newest]

Thread overview: 69+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-28 21:20 [PATCH v12 00/23] TDX KVM selftests Sagi Shahar
2025-10-28 21:20 ` [PATCH v12 01/23] KVM: selftests: Add macros so simplify creating VM shapes for non-default types Sagi Shahar
2025-10-29  1:13   ` Ira Weiny
2025-10-29  6:57   ` Binbin Wu
2025-10-31  3:42   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 02/23] KVM: selftests: Allocate pgd in virt_map() as necessary Sagi Shahar
2025-10-31  3:51   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 03/23] KVM: selftests: Expose functions to get default sregs values Sagi Shahar
2025-10-29  1:40   ` Ira Weiny
2025-10-31  3:43   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 04/23] KVM: selftests: Expose function to allocate guest vCPU stack Sagi Shahar
2025-10-29 13:24   ` Ira Weiny
2025-10-31  3:52   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 05/23] KVM: selftests: Update kvm_init_vm_address_properties() for TDX Sagi Shahar
2025-10-29 15:22   ` Ira Weiny
2025-10-31  3:53   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 06/23] KVM: selftests: Expose segment definitons to assembly files Sagi Shahar
2025-10-31  3:54   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 07/23] KVM: selftests: Add kbuild definitons Sagi Shahar
2025-10-29  7:43   ` Binbin Wu
2025-10-29 15:46     ` Ira Weiny
2025-10-29 15:55   ` Ira Weiny
2025-10-31  3:56   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 08/23] KVM: selftests: Define structs to pass parameters to TDX boot code Sagi Shahar
2025-10-29 16:37   ` Reinette Chatre
2025-10-30 14:20     ` Sean Christopherson
2025-10-31  4:01   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 09/23] KVM: selftests: Add " Sagi Shahar
2025-10-28 21:20 ` [PATCH v12 10/23] KVM: selftests: Set up TDX boot code region Sagi Shahar
2025-10-28 21:20 ` [PATCH v12 11/23] KVM: selftests: Set up TDX boot parameters region Sagi Shahar
2025-10-29  8:52   ` Binbin Wu
2025-10-29 21:01   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 12/23] KVM: selftests: Add helper to initialize TDX VM Sagi Shahar
2025-10-29 21:16   ` Ira Weiny
2025-10-29 23:01     ` Reinette Chatre
2025-10-30  1:25   ` Binbin Wu
2025-10-31  4:06   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 13/23] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration Sagi Shahar
2025-10-29 21:19   ` Ira Weiny
2025-10-30  1:35   ` Binbin Wu
2025-10-28 21:20 ` [PATCH v12 14/23] KVM: selftests: Add helpers to init TDX memory and finalize VM Sagi Shahar
2025-10-29 21:27   ` Ira Weiny
2025-10-30  2:32   ` Binbin Wu
2025-10-31 15:58   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 15/23] KVM: selftests: Call TDX init when creating a new TDX vm Sagi Shahar
2025-10-30 22:20   ` Ira Weiny
2025-10-31 16:03   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation Sagi Shahar
2025-10-29 13:18   ` Ira Weiny
2025-10-30  6:01     ` Binbin Wu
2025-10-28 21:20 ` [PATCH v12 17/23] KVM: selftests: Call KVM_TDX_INIT_VCPU when creating a new TDX vcpu Sagi Shahar
2025-10-30  6:15   ` Binbin Wu
2025-10-28 21:20 ` [PATCH v12 18/23] KVM: selftests: Set entry point for TDX guest code Sagi Shahar
2025-10-31 16:03   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 19/23] KVM: selftests: Finalize TD memory as part of kvm_arch_vm_finalize_vcpus Sagi Shahar
2025-10-31 13:10   ` Ira Weiny
2025-10-31 16:05   ` Reinette Chatre
2025-10-28 21:20 ` [PATCH v12 20/23] KVM: selftests: Add support for TDX TDCALL from guest Sagi Shahar
2025-10-31 14:11   ` Ira Weiny
2025-10-31 15:15     ` Sean Christopherson
2025-10-31 15:58       ` Sagi Shahar
2025-10-31 16:12         ` Sean Christopherson
2025-10-31 16:01       ` Sean Christopherson
2025-10-28 21:20 ` [PATCH v12 21/23] KVM: selftests: Add wrapper for TDX MMIO " Sagi Shahar
2025-10-31 14:21   ` Ira Weiny
2025-10-28 21:20 ` [PATCH v12 22/23] KVM: selftests: Add ucall support for TDX Sagi Shahar
2025-10-31 14:38   ` Ira Weiny
2025-10-31 15:55   ` Sean Christopherson
2025-10-28 21:20 ` [PATCH v12 23/23] KVM: selftests: Add TDX lifecycle test Sagi Shahar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).