linux-kernel - Re: [PATCH v9 15/19] KVM: selftests: Hook TDX support to vm and vcpu creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <68ae1604a387c_300e8f2947e@iweiny-mobl.notmuch>
Date: Tue, 26 Aug 2025 15:16:04 -0500
From: Ira Weiny <ira.weiny@...el.com>
To: Sean Christopherson <seanjc@...gle.com>, Sagi Shahar <sagis@...gle.com>
CC: <linux-kselftest@...r.kernel.org>, Paolo Bonzini <pbonzini@...hat.com>,
	Shuah Khan <shuah@...nel.org>, Ackerley Tng <ackerleytng@...gle.com>, "Ryan
 Afranji" <afranji@...gle.com>, Andrew Jones <ajones@...tanamicro.com>, "Isaku
 Yamahata" <isaku.yamahata@...el.com>, Erdem Aktas <erdemaktas@...gle.com>,
	Rick Edgecombe <rick.p.edgecombe@...el.com>, Roger Wang
	<runanwang@...gle.com>, Binbin Wu <binbin.wu@...ux.intel.com>, Oliver Upton
	<oliver.upton@...ux.dev>, "Pratik R. Sampat" <pratikrajesh.sampat@....com>,
	Reinette Chatre <reinette.chatre@...el.com>, Ira Weiny <ira.weiny@...el.com>,
	Chao Gao <chao.gao@...el.com>, Chenyi Qiang <chenyi.qiang@...el.com>,
	<linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>
Subject: Re: [PATCH v9 15/19] KVM: selftests: Hook TDX support to vm and vcpu
 creation

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@...gle.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).