linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Sagi Shahar <sagis@google.com>
Cc: linux-kselftest@vger.kernel.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	 Shuah Khan <shuah@kernel.org>,
	Ackerley Tng <ackerleytng@google.com>,
	 Ryan Afranji <afranji@google.com>,
	Andrew Jones <ajones@ventanamicro.com>,
	 Isaku Yamahata <isaku.yamahata@intel.com>,
	Erdem Aktas <erdemaktas@google.com>,
	 Rick Edgecombe <rick.p.edgecombe@intel.com>,
	Roger Wang <runanwang@google.com>,
	 Binbin Wu <binbin.wu@linux.intel.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	 "Pratik R. Sampat" <pratikrajesh.sampat@amd.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	 Ira Weiny <ira.weiny@intel.com>, Chao Gao <chao.gao@intel.com>,
	 Chenyi Qiang <chenyi.qiang@intel.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH v9 15/19] KVM: selftests: Hook TDX support to vm and vcpu creation
Date: Tue, 26 Aug 2025 10:31:19 -0700	[thread overview]
Message-ID: <aK3vZ5HuKKeFuuM4@google.com> (raw)
In-Reply-To: <20250821042915.3712925-16-sagis@google.com>

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

  parent reply	other threads:[~2025-08-26 17:31 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-21  4:28 [PATCH v9 00/19] TDX KVM selftests Sagi Shahar
2025-08-21  4:28 ` [PATCH v9 01/19] KVM: selftests: Include overflow.h instead of redefining is_signed_type() Sagi Shahar
2025-08-21 14:16   ` Sean Christopherson
2025-08-21 14:37   ` Ira Weiny
2025-08-21 14:42     ` Sean Christopherson
2025-08-21  4:28 ` [PATCH v9 02/19] KVM: selftests: Allocate pgd in virt_map() as necessary Sagi Shahar
2025-08-21 14:45   ` Ira Weiny
2025-08-26  2:36   ` Binbin Wu
2025-08-21  4:28 ` [PATCH v9 03/19] KVM: selftests: Expose functions to get default sregs values Sagi Shahar
2025-08-26  2:36   ` Binbin Wu
2025-08-26 14:05     ` Sean Christopherson
2025-08-21  4:28 ` [PATCH v9 04/19] KVM: selftests: Expose function to allocate guest vCPU stack Sagi Shahar
2025-08-21 22:00   ` Ira Weiny
2025-08-21 22:24     ` Sagi Shahar
2025-08-26  5:39   ` Binbin Wu
2025-08-26 16:00     ` Sagi Shahar
2025-08-26 17:16     ` Sean Christopherson
2025-08-21  4:28 ` [PATCH v9 05/19] KVM: selftests: Update kvm_init_vm_address_properties() for TDX Sagi Shahar
2025-08-21 22:05   ` Ira Weiny
2025-08-21 22:30     ` Sagi Shahar
2025-08-26  5:51   ` Binbin Wu
2025-08-26 16:04     ` Sagi Shahar
2025-08-21  4:28 ` [PATCH v9 06/19] KVM: selftests: Expose segment definitons to assembly files Sagi Shahar
2025-08-21 20:04   ` Ira Weiny
2025-08-21  4:29 ` [PATCH v9 07/19] KVM: selftests: Add kbuild definitons Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 08/19] KVM: selftests: Define structs to pass parameters to TDX boot code Sagi Shahar
2025-08-26  6:52   ` Binbin Wu
2025-08-26 16:10     ` Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 09/19] KVM: selftests: Add " Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 10/19] KVM: selftests: Set up TDX boot code region Sagi Shahar
2025-08-25  5:32   ` Yan Zhao
2025-08-26 16:38     ` Sean Christopherson
2025-08-27  1:36       ` Yan Zhao
2025-08-21  4:29 ` [PATCH v9 11/19] KVM: selftests: Set up TDX boot parameters region Sagi Shahar
2025-08-26  8:36   ` Binbin Wu
2025-08-26 16:17     ` Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 12/19] KVM: selftests: Add helper to initialize TDX VM Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 13/19] KVM: selftests: TDX: Use KVM_TDX_CAPABILITIES to validate TDs' attribute configuration Sagi Shahar
2025-08-26  9:22   ` Binbin Wu
2025-09-04  3:57     ` Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 14/19] KVM: selftests: Add helpers to init TDX memory and finalize VM Sagi Shahar
2025-08-25  8:40   ` Yan Zhao
2025-08-25 19:02     ` Sagi Shahar
2025-08-26  1:07       ` Yan Zhao
2025-08-27  2:24         ` Binbin Wu
2025-08-27  2:44           ` Yan Zhao
2025-08-27  3:52             ` Yan Zhao
2025-08-21  4:29 ` [PATCH v9 15/19] KVM: selftests: Hook TDX support to vm and vcpu creation Sagi Shahar
2025-08-26  8:28   ` Chenyi Qiang
2025-08-26 16:12     ` Sagi Shahar
2025-08-26 17:31   ` Sean Christopherson [this message]
2025-08-26 20:16     ` Ira Weiny
2025-08-26 20:29       ` Sagi Shahar
2025-08-26 20:30         ` Sagi Shahar
2025-08-26 21:31           ` Sean Christopherson
2025-08-26 21:38             ` Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 16/19] KVM: selftests: Add support for TDX TDCALL from guest Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 17/19] KVM: selftests: Add wrapper for TDX MMIO " Sagi Shahar
2025-08-21  4:29 ` [PATCH v9 18/19] KVM: selftests: Add ucall support for TDX Sagi Shahar
2025-08-27  7:18   ` Binbin Wu
2025-09-02 15:45     ` Sean Christopherson
2025-09-03  1:07       ` Binbin Wu
2025-08-21  4:29 ` [PATCH v9 19/19] KVM: selftests: Add TDX lifecycle test Sagi Shahar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aK3vZ5HuKKeFuuM4@google.com \
    --to=seanjc@google.com \
    --cc=ackerleytng@google.com \
    --cc=afranji@google.com \
    --cc=ajones@ventanamicro.com \
    --cc=binbin.wu@linux.intel.com \
    --cc=chao.gao@intel.com \
    --cc=chenyi.qiang@intel.com \
    --cc=erdemaktas@google.com \
    --cc=ira.weiny@intel.com \
    --cc=isaku.yamahata@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=pbonzini@redhat.com \
    --cc=pratikrajesh.sampat@amd.com \
    --cc=reinette.chatre@intel.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=runanwang@google.com \
    --cc=sagis@google.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).