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

[Binbin Wu <binbin.wu@linux.intel.com>]

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@google.com>
>> ---
>>   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
>>
>