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.
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;
+ }
+
/*
* 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
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index 4802fc81bea7..5cf14f09c1b6 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -670,6 +670,11 @@ void kvm_arch_vm_post_create(struct kvm_vm *vm)
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;
@@ -680,9 +685,13 @@ void vcpu_arch_set_entry_point(struct kvm_vcpu *vcpu, void *guest_code)
{
struct kvm_regs regs;
- vcpu_regs_get(vcpu, ®s);
- regs.rip = (unsigned long) guest_code;
- vcpu_regs_set(vcpu, ®s);
+ if (is_tdx_vm(vcpu->vm))
+ vm_tdx_set_vcpu_entry_point(vcpu, guest_code);
+ else {
+ vcpu_regs_get(vcpu, ®s);
+ regs.rip = (unsigned long) guest_code;
+ vcpu_regs_set(vcpu, ®s);
+ }
}
vm_vaddr_t kvm_allocate_vcpu_stack(struct kvm_vm *vm)
@@ -711,6 +720,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;
@@ -718,16 +740,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, ®s);
- regs.rflags = regs.rflags | 0x2;
- if (vm->type != KVM_X86_TDX_VM)
+ 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, ®s);
+ regs.rflags = regs.rflags | 0x2;
regs.rsp = kvm_allocate_vcpu_stack(vm);
- vcpu_regs_set(vcpu, ®s);
+ vcpu_regs_set(vcpu, ®s);
+ }
/* Setup the MP state */
mp_state.mp_state = 0;
--
2.51.0.rc1.193.gad69d77794-googOn 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
support for in-kernel I/O APIC (and PIC and PIT).
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: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);
> 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 Tue, Aug 26, 2025, Sagi Shahar wrote: > 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: > > > > 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: Yep, exactly. > /* > * 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);
On Tue, Aug 26, 2025 at 4:31 PM Sean Christopherson <seanjc@google.com> wrote: > > On Tue, Aug 26, 2025, Sagi Shahar wrote: > > 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: > > > > > 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: > > Yep, exactly. > I took a stab at updating the KVM ABI and sent out a small patch [1] to fail KVM_CREATE_IRQCHIP with ENOTTY and the test passes without the special handling for SPLIT_IRQCHIP for TDX. [1] https://lore.kernel.org/lkml/20250826213455.2338722-1-sagis@google.com/ > > /* > > * 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);
On 8/21/2025 12:29 PM, Sagi Shahar wrote: > TDX require special handling for VM and VCPU initialization for various s/require/requires > reasons: > - Special ioctlss for creating VM and VCPU. s/ioctlss/ioctls > - TDX registers are inaccessible to KVM. > - TDX require special boot code trampoline for loading parameters. s/require/requires > - TDX only supports KVM_CAP_SPLIT_IRQCHIP. > > 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(-) >
On Tue, Aug 26, 2025 at 3:28 AM Chenyi Qiang <chenyi.qiang@intel.com> wrote: > > > > On 8/21/2025 12:29 PM, Sagi Shahar wrote: > > TDX require special handling for VM and VCPU initialization for various > > s/require/requires > > > reasons: > > - Special ioctlss for creating VM and VCPU. > > s/ioctlss/ioctls > > > - TDX registers are inaccessible to KVM. > > - TDX require special boot code trampoline for loading parameters. > > s/require/requires > ACK > > - TDX only supports KVM_CAP_SPLIT_IRQCHIP. > > > > 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(-) > > >
© 2016 - 2025 Red Hat, Inc.