Re: [PATCH v12 16/23] KVM: selftests: Setup memory regions for TDX on vm creation

[Binbin Wu]

On 10/29/2025 9:18 PM, Ira Weiny wrote:
> Sagi Shahar wrote:
> > Guest registers are inaccessible to kvm for TDX VMs. In order to set
> > register values for TDX we use a special boot code which loads the
> NIT: who is 'we'?
>
> > register values from memory and write them into the appropriate
> > registers.
> >
> > This patch sets up the memory regions used for the boot code and the
> > boot parameters for TDX.
> NIT: This is not needed.  Use imperative mood.
>
> > Signed-off-by: Sagi Shahar <sagis@xxxxxxxxxx>
> > ---
> >   tools/testing/selftests/kvm/lib/kvm_util.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index 0e6a487ca7a4..086e8a2a4d99 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"
> > @@ -435,7 +436,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);
> This caused me to dig a bit to understand why this hunk was needed given
> the commit message only discusses guest registers.  I did not recall any
> use of is_guest_memfd_required() in the vm_tdx_setup_*() calls so I was a
> bit confused.
>
> With this hunk considered the changelog should read something like:
>
> <commit message>
>
> Guest memfd is required for the primary memory region of TDX VMs.
>
> Furthermore, guest registers are inaccessible to kvm for TDX VMs.  TDX
> must use use special boot code which loads the register values from memory
> and writes them into the appropriate registers.
>
> Use guest_memfd for the primary memory regions and call the TDX boot code
> functions for TDX VMs.
>
> </commit message>
>
> This clearly explains why the change to is_guest_memfd_required() is
> needed.

+1

> In addition, the structure of this series is a bit odd to me.  I assume
> this patch exists after the setup calls were added to ensure
> bisect-ability?

I think it's better to switch the order of this patch and patch 15.
Patch 15 relies on the memory regions added by this patch for boot code and
parameters.


> Ira
>
> >   #else
> >   	return false;
> >   #endif
> > @@ -469,6 +470,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);
> >   
> >   	/*
> > --
> > 2.51.1.851.g4ebd6896fd-goog