Need to add the selftest folks.

+ linux-kselftest@vger.kernel.org + Kees Cook kees@kernel.org + Shuah Khan shuah@kernel.org

Sagi Shahar wrote:

Redefinition of is_signed_type() causes compilation warning for tests which use kselftest_harness. Replace the definition with linux/overflow.h

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

Thanks! I've seen this as well and it fixes the warning for me as well. It might be worth picking up separate from this series depending on what the selftest folks say.

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

---

tools/testing/selftests/kselftest_harness.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index 2925e47db995..a580a0d33c65 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -56,6 +56,7 @@ #include <asm/types.h> #include <ctype.h> #include <errno.h> +#include <linux/overflow.h> #include <linux/unistd.h> #include <poll.h> #include <stdbool.h> @@ -751,8 +752,6 @@ for (; _metadata->trigger; _metadata->trigger = \ __bail(_assert, _metadata)) -#define is_signed_type(var) (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))

#define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \ /* Avoid multiple evaluation of the cases */ \ __typeof__(_expected) __exp = (_expected); \ -- 2.51.0.rc1.193.gad69d77794-goog

On 8/21/2025 12:28 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.

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

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

Reviewed-by: Binbin Wu binbin.wu@linux.intel.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 c3f5142b0a54..b4c8702ba4bd 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -1609,6 +1609,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);

On 8/21/2025 12:28 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.

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

.../selftests/kvm/include/x86/processor.h | 6 +++ .../testing/selftests/kvm/lib/x86/processor.c | 41 +++++++++++++++---- 2 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h index 2efb05c2f2fb..5c16507f9b2d 100644 --- a/tools/testing/selftests/kvm/include/x86/processor.h +++ b/tools/testing/selftests/kvm/include/x86/processor.h @@ -1026,6 +1026,12 @@ static inline struct kvm_cpuid2 *allocate_kvm_cpuid2(int nr_entries) void vcpu_init_cpuid(struct kvm_vcpu *vcpu, const struct kvm_cpuid2 *cpuid); +uint16_t kvm_get_default_idt_limit(void); +uint16_t kvm_get_default_gdt_limit(void); +uint64_t kvm_get_default_cr0(void); +uint64_t kvm_get_default_cr4(void); +uint64_t kvm_get_default_efer(void);

Can these be defined in the header file as static inline?

• static inline void vcpu_get_cpuid(struct kvm_vcpu *vcpu) { vcpu_ioctl(vcpu, KVM_GET_CPUID2, vcpu->cpuid);

diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c index d4c19ac885a9..b2a4b11ac8c0 100644 --- a/tools/testing/selftests/kvm/lib/x86/processor.c +++ b/tools/testing/selftests/kvm/lib/x86/processor.c @@ -488,6 +488,35 @@ static void kvm_seg_set_tss_64bit(vm_vaddr_t base, struct kvm_segment *segp) segp->present = 1; } +uint16_t kvm_get_default_idt_limit(void) +{

• return NUM_INTERRUPTS * sizeof(struct idt_entry) - 1;

+}

+uint16_t kvm_get_default_gdt_limit(void) +{

• return getpagesize() - 1;

+}

+uint64_t kvm_get_default_cr0(void) +{

• return X86_CR0_PE | X86_CR0_NE | X86_CR0_PG;

+}

+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;

+}

+uint64_t kvm_get_default_efer(void) +{

• return EFER_LME | EFER_LMA | EFER_NX;

+}

• static void vcpu_init_sregs(struct kvm_vm *vm, struct kvm_vcpu *vcpu) { struct kvm_sregs sregs;

@@ -498,15 +527,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);

On Thu, Aug 21, 2025 at 4:58 PM Ira Weiny ira.weiny@intel.com wrote:

Sagi Shahar wrote:

[snip]

...

diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c index b2a4b11ac8c0..1eae92957456 100644 --- a/tools/testing/selftests/kvm/lib/x86/processor.c +++ b/tools/testing/selftests/kvm/lib/x86/processor.c @@ -687,12 +687,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,
  

@@ -713,6 +710,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);
  

@@ -721,7 +727,8 @@ 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;
  

• if (vm->type != KVM_X86_TDX_VM)
  

•         regs.rsp = kvm_allocate_vcpu_stack(vm);
  

At this point in the series vm->type can't be KVM_X86_TDX_VM correct?

So that makes this safe during bisect?

I double checked and no one is creating VMs with KVM_X86_TDX_VM. The first test that sets KVM_X86_TDX_VM is the last patch in the series.

Ira

...

  vcpu_regs_set(vcpu, &regs);

  /* Setup the MP state */

-- 2.51.0.rc1.193.gad69d77794-goog

On Tue, Aug 26, 2025 at 12:39 AM Binbin Wu binbin.wu@linux.intel.com wrote:

On 8/21/2025 12:28 PM, 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.

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

.../selftests/kvm/include/x86/processor.h | 2 ++ tools/testing/selftests/kvm/lib/x86/processor.c | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h index 5c16507f9b2d..8fcc5118683e 100644 --- a/tools/testing/selftests/kvm/include/x86/processor.h +++ b/tools/testing/selftests/kvm/include/x86/processor.h @@ -1111,6 +1111,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 b2a4b11ac8c0..1eae92957456 100644 --- a/tools/testing/selftests/kvm/lib/x86/processor.c +++ b/tools/testing/selftests/kvm/lib/x86/processor.c @@ -687,12 +687,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,
  

@@ -713,6 +710,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);
  

@@ -721,7 +727,8 @@ 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;
  

• if (vm->type != KVM_X86_TDX_VM)
  

•         regs.rsp = kvm_allocate_vcpu_stack(vm);
  

I am wondering if this could be more generic. I.e, make vcpu_regs_get() return the error code. If vcpu_regs_get() failed (for TDX, since it's guest state is protected, the ioctl will return -EINVAL), the vcpu_regs_set(), including the allocation for the vcpu stack, could be skipped.

I'm dropping this check and only keeping the check from "KVM: selftests: Hook TDX support to vm and vcpu creation" which looks like this:

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); }

Since there are other differences specific to TDX I think this is cleaner than trying to handle kvm_allocate_vcpu_stack() individually.

...

  vcpu_regs_set(vcpu, &regs);

  /* Setup the MP state */

On Thu, Aug 21, 2025 at 5:03 PM Ira Weiny ira.weiny@intel.com wrote:

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.

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;
  

+}

NIT: Might have been better to define this in 4/19 and use it there. But looks like that logic is changed later on somewhere. So... meh.

We can skip the check for KVM_X86_TDX_VM entirely in 4/19 since like you said, it's replaced with something different later on.

Ira

[snip]

On Tue, Aug 26, 2025 at 12:51 AM Binbin Wu binbin.wu@linux.intel.com wrote:

On 8/21/2025 12:28 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.

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

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;
  

+}

If the branch "vm->type != KVM_X86_TDX_VM" in patch 04/19 is still needed, this helper could be added earlier and used instead of open code.

I'm dropping the check in 04/19. See my response to Ira.

...

+#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 1eae92957456..6dbf40cbbc2a 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 "processor.h" #include "sev.h" +#include "tdx/tdx_util.h"

#ifndef NUM_INTERRUPTS #define NUM_INTERRUPTS 256 @@ -1190,12 +1191,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 = 1ULL << (gpa_bits - 1);
  

Nit: Use BIT_ULL().

ACK, Will update in next version.

...

•         vm->gpa_tag_mask = vm->arch.s_bit;
  }
  

  }

NIT:

Sagi Shahar wrote:

Move kernel segment definitons to a separate file which can be included

^^^^^^^^^^ definitions

And in the subject.

Otherwise seems reasonable.

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

from assembly files.

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 6dbf40cbbc2a..4802fc81bea7 100644 --- a/tools/testing/selftests/kvm/lib/x86/processor.c +++ b/tools/testing/selftests/kvm/lib/x86/processor.c @@ -7,6 +7,7 @@ #include "test_util.h" #include "kvm_util.h" #include "processor.h" +#include "processor_asm.h" #include "sev.h" #include "tdx/tdx_util.h" @@ -14,10 +15,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.0.rc1.193.gad69d77794-goog

On Tue, Aug 26, 2025 at 3:36 AM Binbin Wu binbin.wu@linux.intel.com wrote:

On 8/21/2025 12:29 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

---

.../selftests/kvm/include/x86/tdx/tdx_util.h | 4 + .../selftests/kvm/lib/x86/tdx/tdx_util.c | 73 +++++++++++++++++++ 2 files changed, 77 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 15833b9eb5d5..52dc25e0cce4 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,74 @@ 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;
  

• /* Set this because UEFI also sets this up, to handle XMM exceptions */
  

• cr4 |= X86_CR4_OSXMMEXCPT;
  

• /* 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];
  

Nit: Better to align the style on whether wrap or not due to max characters per line

Thanks, going to wrap in both functions.

...

• 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;
  

+}

On Tue, Aug 26, 2025 at 4:22 AM Binbin Wu binbin.wu@linux.intel.com wrote:

On 8/21/2025 12:29 PM, Sagi Shahar wrote:

...

From: Isaku Yamahata isaku.yamahata@intel.com

This also exercises the KVM_TDX_CAPABILITIES ioctl.

That commit message should describe what the patch does instead of relying on the title/short log.

...

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/lib/x86/tdx/tdx_util.c | 17 +++++++++++++++++ 1 file changed, 17 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 3869756a5641..d8eab99d9333 100644 --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c @@ -232,6 +232,21 @@ 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);
  

• /* TDX spec: any bits 0 in supported_attrs must be 0 in attributes */
  

• TEST_ASSERT_EQ(attributes & ~tdx_cap->supported_attrs, 0);
  

• /* TDX spec: any bits 1 in attributes must be 1 in supported_attrs */
  

The comments are not accurate.

The descriptions in TDX spec are for ATTRIBUTES_ FIXED0 and ATTRIBUTES_ FIXED1. They are related to tdx_cap->supported_attrs returned by KVM, but they are not the same.

I actually think that one of the conditions is redundant. Here's my reasoning: If a bit is 0 in attributes then both conditions will be true regardless of the value of supported_attrs. If a bit is 1 in attributes then both conditions will be true iff the corresponding bit in supported_attrs is 1.

I'm going to keep only the second condition which is clearer and update the comment.

...

• 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;

@@ -251,6 +266,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);
  

On Wed, Aug 20, 2025 at 09:29:07PM -0700, 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 | 97 +++++++++++++++++++ 2 files changed, 99 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 d8eab99d9333..4024587ed3c2 100644 --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c @@ -274,3 +274,100 @@ 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 tdx_init_pages(struct kvm_vm *vm, void *hva, uint64_t gpa,

• 	   uint64_t size)
  

+{

• void *scratch_page = calloc(1, PAGE_SIZE);
• uint64_t nr_pages = size / PAGE_SIZE;
• int i;
• TEST_ASSERT(scratch_page,

•     "Could not allocate memory for loading memory region");
  

• for (i = 0; i < nr_pages; i++) {

• memcpy(scratch_page, hva, PAGE_SIZE);
  

• tdx_init_mem_region(vm, scratch_page, gpa, PAGE_SIZE);
  

• hva += PAGE_SIZE;
  

• gpa += PAGE_SIZE;
  

• }
• free(scratch_page);

+}

+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;
  

• sparsebit_idx_t j;
  

• if (!sparsebit_any_set(protected_pages))
  

• 	continue;
  

• 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_set_memory_attributes(vm, gpa, size_to_load,
  

• 				 KVM_MEMORY_ATTRIBUTE_PRIVATE);
  

• 	/*
  

• 	 * Here, memory is being loaded from hva to gpa. If the memory
  

• 	 * mapped to hva is also used to back gpa, then a copy has to be
  

• 	 * made just for loading, since KVM_TDX_INIT_MEM_REGION ioctl
  

• 	 * cannot encrypt memory in place.
  

• 	 *
  

• 	 * To determine if memory mapped to hva is also used to back
  

• 	 * gpa, use a heuristic:
  

• 	 *
  

• 	 * If this memslot has guest_memfd, then this memslot should
  

• 	 * have memory backed from two sources: hva for shared memory
  

• 	 * and gpa will be backed by guest_memfd.
  

• 	 */
  

• 	if (region->region.guest_memfd == -1)
  

Why to pass !guest_memfd region to tdx_init_mem_region()?

• 		tdx_init_pages(vm, (void *)hva, gpa, size_to_load);
  

• 	else
  

• 		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.0.rc1.193.gad69d77794-goog

On Mon, Aug 25, 2025 at 02:02:00PM -0500, Sagi Shahar wrote:

On Mon, Aug 25, 2025 at 3:41 AM Yan Zhao yan.y.zhao@intel.com wrote:

...

On Wed, Aug 20, 2025 at 09:29:07PM -0700, 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 | 97 +++++++++++++++++++ 2 files changed, 99 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 d8eab99d9333..4024587ed3c2 100644 --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c @@ -274,3 +274,100 @@ 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 tdx_init_pages(struct kvm_vm *vm, void *hva, uint64_t gpa,

•                    uint64_t size)
  

+{

• void *scratch_page = calloc(1, PAGE_SIZE);
  

• uint64_t nr_pages = size / PAGE_SIZE;
  

• int i;
  

• TEST_ASSERT(scratch_page,
  

•             "Could not allocate memory for loading memory region");
  

• for (i = 0; i < nr_pages; i++) {
  

•         memcpy(scratch_page, hva, PAGE_SIZE);
  

•         tdx_init_mem_region(vm, scratch_page, gpa, PAGE_SIZE);
  

•         hva += PAGE_SIZE;
  

•         gpa += PAGE_SIZE;
  

• }
  

• free(scratch_page);
  

+}

+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;
  

•         sparsebit_idx_t j;
  

•         if (!sparsebit_any_set(protected_pages))
  

•                 continue;
  

•         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_set_memory_attributes(vm, gpa, size_to_load,
  

•                                          KVM_MEMORY_ATTRIBUTE_PRIVATE);
  

•                 /*
  

•                  * Here, memory is being loaded from hva to gpa. If the memory
  

•                  * mapped to hva is also used to back gpa, then a copy has to be
  

•                  * made just for loading, since KVM_TDX_INIT_MEM_REGION ioctl
  

•                  * cannot encrypt memory in place.
  

•                  *
  

•                  * To determine if memory mapped to hva is also used to back
  

•                  * gpa, use a heuristic:
  

•                  *
  

•                  * If this memslot has guest_memfd, then this memslot should
  

•                  * have memory backed from two sources: hva for shared memory
  

•                  * and gpa will be backed by guest_memfd.
  

•                  */
  

•                 if (region->region.guest_memfd == -1)
  

Why to pass !guest_memfd region to tdx_init_mem_region()?

Not sure I understand your comment.

From the implementation of tdx_init_pages(), it also invokes tdx_init_mem_region(), which further invokes ioctl KVM_TDX_INIT_MEM_REGION.

However, if the region is with guest_memfd == -1, the ioctl KVM_TDX_INIT_MEM_REGION should fail as kvm_gmem_populate() won't succeed.

So, I'm wondering why there's a need to for the case of "region->region.guest_memfd == -1".

Or anything I missed?

...

...

•                         tdx_init_pages(vm, (void *)hva, gpa, size_to_load);
  

•                 else
  

•                         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.0.rc1.193.gad69d77794-goog

On Wed, Aug 27, 2025 at 10:24:03AM +0800, Binbin Wu wrote:

On 8/26/2025 9:07 AM, Yan Zhao wrote:

...

On Mon, Aug 25, 2025 at 02:02:00PM -0500, Sagi Shahar wrote:

...

On Mon, Aug 25, 2025 at 3:41 AM Yan Zhao yan.y.zhao@intel.com wrote:

...

On Wed, Aug 20, 2025 at 09:29:07PM -0700, 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 | 97 +++++++++++++++++++ 2 files changed, 99 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 d8eab99d9333..4024587ed3c2 100644 --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c @@ -274,3 +274,100 @@ 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 tdx_init_pages(struct kvm_vm *vm, void *hva, uint64_t gpa,

•                    uint64_t size)
  

+{

• void *scratch_page = calloc(1, PAGE_SIZE);
  

• uint64_t nr_pages = size / PAGE_SIZE;
  

• int i;
  

• TEST_ASSERT(scratch_page,
  

•             "Could not allocate memory for loading memory region");
  

• for (i = 0; i < nr_pages; i++) {
  

•         memcpy(scratch_page, hva, PAGE_SIZE);
  

•         tdx_init_mem_region(vm, scratch_page, gpa, PAGE_SIZE);
  

•         hva += PAGE_SIZE;
  

•         gpa += PAGE_SIZE;
  

• }
  

• free(scratch_page);
  

+}

+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;
  

•         sparsebit_idx_t j;
  

•         if (!sparsebit_any_set(protected_pages))
  

•                 continue;
  

•         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_set_memory_attributes(vm, gpa, size_to_load,
  

•                                          KVM_MEMORY_ATTRIBUTE_PRIVATE);
  

•                 /*
  

•                  * Here, memory is being loaded from hva to gpa. If the memory
  

•                  * mapped to hva is also used to back gpa, then a copy has to be
  

•                  * made just for loading, since KVM_TDX_INIT_MEM_REGION ioctl
  

•                  * cannot encrypt memory in place.
  

•                  *
  

•                  * To determine if memory mapped to hva is also used to back
  

•                  * gpa, use a heuristic:
  

•                  *
  

•                  * If this memslot has guest_memfd, then this memslot should
  

•                  * have memory backed from two sources: hva for shared memory
  

•                  * and gpa will be backed by guest_memfd.
  

•                  */
  

•                 if (region->region.guest_memfd == -1)
  

Why to pass !guest_memfd region to tdx_init_mem_region()?

Not sure I understand your comment.

From the implementation of tdx_init_pages(), it also invokes tdx_init_mem_region(), which further invokes ioctl KVM_TDX_INIT_MEM_REGION.

However, if the region is with guest_memfd == -1, the ioctl KVM_TDX_INIT_MEM_REGION should fail as kvm_gmem_populate() won't succeed.

So, I'm wondering why there's a need to for the case of "region->region.guest_memfd == -1".

Or anything I missed?

I had the same question in v8 https://lore.kernel.org/lkml/4b7e7099-79da-4178-8f16-6780d8137ae1@linux.inte...

I agree with you. In patch "KVM: selftests: TDX: Test LOG_DIRTY_PAGES flag to a non-GUEST_MEMFD memslot", virt_map_shared() prevents the non-guest_memfd region from being searched by the load_td_private_memory(). Otherwise, the tdx_init_mem_region() would fail on this region.

I guess the code path for non-guest_memfd is due to some old versions of TDX KVM code before upstream. Currently, KVM doesn't support private memory from non-guest_memfd backed memory.

I guess so. Maybe just drop this case and assert?

TEST_ASSERT(region->region.guest_memfd != -1, "TDX private memory only supports guest_memfd backend\n");

...

...

...

...

•                         tdx_init_pages(vm, (void *)hva, gpa, size_to_load);
  

•                 else
  

•                         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.0.rc1.193.gad69d77794-goog

Sean Christopherson wrote:

On Wed, Aug 20, 2025, Sagi Shahar wrote:

...

TDX require special handling for VM and VCPU initialization for various reasons:

• Special ioctlss for creating VM and VCPU.
• TDX registers are inaccessible to KVM.
• TDX require special boot code trampoline for loading parameters.
• TDX only supports KVM_CAP_SPLIT_IRQCHIP.

Please split this up and elaborate at least a little bit on why each flow needs special handling for TDX. Even for someone like me who is fairly familiar with TDX, there's too much "Trust me bro" and not enough explanation of why selftests really need all of these special paths for TDX.

At least four patches, one for each of your bullet points. Probably 5 or 6, as I think the CPUID handling warrants its own patch.

...

Hook this special handling into __vm_create() and vm_arch_vcpu_add() using the utility functions added in previous patches.

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

tools/testing/selftests/kvm/lib/kvm_util.c | 24 ++++++++- .../testing/selftests/kvm/lib/x86/processor.c | 49 ++++++++++++++----- 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index b4c8702ba4bd..d9f0ff97770d 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" @@ -465,7 +466,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 @@ -499,6 +500,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);

/* @@ -1728,11 +1735,26 @@ void *addr_gpa2alias(struct kvm_vm *vm, vm_paddr_t gpa) return (void *) ((uintptr_t) region->host_alias + offset); } +static bool is_split_irqchip_required(struct kvm_vm *vm) +{ +#ifdef __x86_64__

• return is_tdx_vm(vm);

+#else

• return false;

+#endif +}

/* Create an interrupt controller chip for the specified VM. */ void vm_create_irqchip(struct kvm_vm *vm) { int r;

• if (is_split_irqchip_required(vm)) {

• vm_enable_cap(vm, KVM_CAP_SPLIT_IRQCHIP, 24);
  

• vm->has_irqchip = true;
  

• return;
  

• }

Ugh. IMO, this is a KVM bug. Allowing KVM_CREATE_IRQCHIP for a TDX VM is simply wrong. It _can't_ work. Waiting until KVM_CREATE_VCPU to fail setup is terrible ABI.

If we stretch the meaning of ENOTTY a bit and return that when trying to create a fully in-kernel IRQCHIP for a TDX VM, then the selftests code Just Works thanks to the code below, which handles the scenario where KVM was be built without

^^^^^^^^^^

I'm not following. Was there supposed to be a patch attached?

Ira

support for in-kernel I/O APIC (and PIC and PIT).

On Tue, Aug 26, 2025 at 3:29 PM Sagi Shahar sagis@google.com wrote:

On Tue, Aug 26, 2025 at 3:14 PM Ira Weiny ira.weiny@intel.com wrote:

...

Sean Christopherson wrote:

...

On Wed, Aug 20, 2025, Sagi Shahar wrote:

...

TDX require special handling for VM and VCPU initialization for various reasons:

• Special ioctlss for creating VM and VCPU.
• TDX registers are inaccessible to KVM.
• TDX require special boot code trampoline for loading parameters.
• TDX only supports KVM_CAP_SPLIT_IRQCHIP.

Please split this up and elaborate at least a little bit on why each flow needs special handling for TDX. Even for someone like me who is fairly familiar with TDX, there's too much "Trust me bro" and not enough explanation of why selftests really need all of these special paths for TDX.

At least four patches, one for each of your bullet points. Probably 5 or 6, as I think the CPUID handling warrants its own patch.

...

Hook this special handling into __vm_create() and vm_arch_vcpu_add() using the utility functions added in previous patches.

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

tools/testing/selftests/kvm/lib/kvm_util.c | 24 ++++++++- .../testing/selftests/kvm/lib/x86/processor.c | 49 ++++++++++++++----- 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index b4c8702ba4bd..d9f0ff97770d 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" @@ -465,7 +466,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 @@ -499,6 +500,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);

  /*

@@ -1728,11 +1735,26 @@ void *addr_gpa2alias(struct kvm_vm *vm, vm_paddr_t gpa) return (void *) ((uintptr_t) region->host_alias + offset); }

+static bool is_split_irqchip_required(struct kvm_vm *vm) +{ +#ifdef __x86_64__

• return is_tdx_vm(vm);

+#else

• return false;

+#endif +}

/* Create an interrupt controller chip for the specified VM. */ void vm_create_irqchip(struct kvm_vm *vm) { int r;

• if (is_split_irqchip_required(vm)) {

•       vm_enable_cap(vm, KVM_CAP_SPLIT_IRQCHIP, 24);
  

•       vm->has_irqchip = true;
  

•       return;
  

• }

Ugh. IMO, this is a KVM bug. Allowing KVM_CREATE_IRQCHIP for a TDX VM is simply wrong. It _can't_ work. Waiting until KVM_CREATE_VCPU to fail setup is terrible ABI.

If we stretch the meaning of ENOTTY a bit and return that when trying to create a fully in-kernel IRQCHIP for a TDX VM, then the selftests code Just Works thanks to the code below, which handles the scenario where KVM was be built without

     ^^^^^^^^^^

I'm not following. Was there supposed to be a patch attached?

I think Sean refers to the original implementation which was out of the scope for the git diff so it was left out of the patch:

/*

• Allocate a fully in-kernel IRQ chip by default, but fall back to a
• split model (x86 only) if that fails (KVM x86 allows compiling out
• support for KVM_CREATE_IRQCHIP).

*/ r = __vm_ioctl(vm, KVM_CREATE_IRQCHIP, NULL); if (r && errno == ENOTTY && kvm_has_cap(KVM_CAP_SPLIT_IRQCHIP)) vm_enable_cap(vm, KVM_CAP_SPLIT_IRQCHIP, 24); else TEST_ASSERT_VM_VCPU_IOCTL(!r, KVM_CREATE_IRQCHIP, r, vm);

/*

• Allocate a fully in-kernel IRQ chip by default, but fall back to a
• split model (x86 only) if that fails (KVM x86 allows compiling out
• support for KVM_CREATE_IRQCHIP).

*/ r = __vm_ioctl(vm, KVM_CREATE_IRQCHIP, NULL); if (r && errno == ENOTTY && kvm_has_cap(KVM_CAP_SPLIT_IRQCHIP)) vm_enable_cap(vm, KVM_CAP_SPLIT_IRQCHIP, 24); else TEST_ASSERT_VM_VCPU_IOCTL(!r, KVM_CREATE_IRQCHIP, r, vm); /*

• Allocate a fully in-kernel IRQ chip by default, but fall back to a
• split model (x86 only) if that fails (KVM x86 allows compiling out
• support for KVM_CREATE_IRQCHIP).

*/ r = __vm_ioctl(vm, KVM_CREATE_IRQCHIP, NULL); if (r && errno == ENOTTY && kvm_has_cap(KVM_CAP_SPLIT_IRQCHIP)) vm_enable_cap(vm, KVM_CAP_SPLIT_IRQCHIP, 24); else TEST_ASSERT_VM_VCPU_IOCTL(!r, KVM_CREATE_IRQCHIP, r, vm);

Sorry, I messed up the paste somehow:

/* * Allocate a fully in-kernel IRQ chip by default, but fall back to a * split model (x86 only) if that fails (KVM x86 allows compiling out * support for KVM_CREATE_IRQCHIP). */ r = __vm_ioctl(vm, KVM_CREATE_IRQCHIP, NULL); if (r && errno == ENOTTY && kvm_has_cap(KVM_CAP_SPLIT_IRQCHIP)) vm_enable_cap(vm, KVM_CAP_SPLIT_IRQCHIP, 24); else TEST_ASSERT_VM_VCPU_IOCTL(!r, KVM_CREATE_IRQCHIP, r, vm);

...

Ira

...

support for in-kernel I/O APIC (and PIC and PIT).

On 8/21/2025 12:29 PM, 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.

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

---

.../testing/selftests/kvm/include/x86/ucall.h | 4 +- tools/testing/selftests/kvm/lib/x86/ucall.c | 45 ++++++++++++++++--- 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/ucall.h b/tools/testing/selftests/kvm/include/x86/ucall.h index d3825dcc3cd9..0494a4a21557 100644 --- a/tools/testing/selftests/kvm/include/x86/ucall.h +++ b/tools/testing/selftests/kvm/include/x86/ucall.h @@ -6,8 +6,6 @@ #define UCALL_EXIT_REASON KVM_EXIT_IO -static inline void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa) -{ -} +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..0ad24baaa3c4 100644 --- a/tools/testing/selftests/kvm/lib/x86/ucall.c +++ b/tools/testing/selftests/kvm/lib/x86/ucall.c @@ -5,11 +5,34 @@

• Copyright (C) 2018, Red Hat, Inc.

*/ #include "kvm_util.h" +#include "tdx/tdx.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);
• host_ucall_mmio_gpa = ucall_mmio_gpa = mmio_gpa;
• if (vm_type == KVM_X86_TDX_VM)

• 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 +69,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;
  

My first thought was how did SEV_ES or SNP work for this since they are not able to get RDI neither. Then I had a check in sev_smoke_test.c, both guest_sev_es_code() and guest_snp_code() call GUEST_ASSERT(), which finally calls ucall_assert(), but in test_sev(), the code doesn't handle ucall for SEV_ES or SNP. Does it mean GUEST_ASSERT() is currently not working and ignored for SEV_ES and SNP? Or did I miss anything?

• 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; }

On 9/2/2025 11:45 PM, Sean Christopherson wrote:

On Wed, Aug 27, 2025, Binbin Wu wrote:

...

On 8/21/2025 12:29 PM, Sagi Shahar wrote:

...

@@ -46,11 +69,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;
  

My first thought was how did SEV_ES or SNP work for this since they are not able to get RDI neither. Then I had a check in sev_smoke_test.c, both guest_sev_es_code() and guest_snp_code() call GUEST_ASSERT(), which finally calls ucall_assert(), but in test_sev(), the code doesn't handle ucall for SEV_ES or SNP. Does it mean GUEST_ASSERT() is currently not working and ignored for SEV_ES and SNP? Or did I miss anything?

GUEST_ASSERT() "works" for -ES and -SNP in the sense that it generates as test failure due to the #VC not being handled (leads to SHUTDOWN). But you're correct that ucall isn't functional yet. x86/sev_smoke_test.c fudges around lack of ucall by using the GHCB MSR protocol to signal "done".

     /*
      * TODO: Add GHCB and ucall support for SEV-ES guests.  For now, simply
      * force "termination" to signal "done" via the GHCB MSR protocol.
      */
     wrmsr(MSR_AMD64_SEV_ES_GHCB, GHCB_MSR_TERM_REQ);
     vmgexit();

OK, thanks for the explanation!