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 <sa...@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
support for in-kernel I/O APIC (and PIC and PIT).
