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

Binbin Wu
2025-10-30T06:01:13Z

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
>>
>