Re: [PATCH v14 08/22] KVM: selftests: Add TDX boot code

[Peter Fang]

On Wed, Jul 22, 2026 at 11:13:13PM +0000, Lisa Wang wrote:
> From: Erdem Aktas <erdemaktas@xxxxxxxxxx>
> 
> Add code to boot a TDX test VM. Since TDX registers are inaccessible to
> KVM, the boot code loads the relevant values from memory into the
> registers before jumping to the guest code.
> 
> Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> Signed-off-by: Erdem Aktas <erdemaktas@xxxxxxxxxx>
> Co-developed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Co-developed-by: Sagi Shahar <sagis@xxxxxxxxxx>
> Signed-off-by: Sagi Shahar <sagis@xxxxxxxxxx>
> Signed-off-by: Lisa Wang <wyihan@xxxxxxxxxx>
> ---

[ ... ]

> diff --git a/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h b/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
> index bf2282931d49..89cf6c3485be 100644
> --- a/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
> +++ b/tools/testing/selftests/kvm/include/x86/tdx/td_boot.h
> @@ -2,9 +2,6 @@
>  #ifndef SELFTEST_TDX_TD_BOOT_H
>  #define SELFTEST_TDX_TD_BOOT_H
>  
> -#include <linux/compiler.h>
> -#include <linux/types.h>
> -
>  /*
>   * Layout for boot section (not to scale)
>   *
> @@ -24,7 +21,19 @@
>   * |                           |
>   * |                           |
>   * |___________________________|____ 0x0_ffff_0000: TD_BOOT_PARAMETERS_GPA
> + *
> + * TD_BOOT_PARAMETERS_GPA is arbitrarily chosen to
> + *
> + * + be within the 4GB address space
> + * + provide enough contiguous memory for the struct td_boot_parameters such
> + *   that there is one struct td_per_vcpu_parameters for KVM_MAX_VCPUS
>   */
> +#define TD_BOOT_PARAMETERS_GPA 0xffff0000
> +
> +#if !defined(__ASSEMBLY__) && !defined(__ASSEMBLER__)

Is the "__ASSEMBLY__" check really needed here? I don't think the
toolchain uses -D__ASSEMBLY__... Also, I think __ASSEMBLY__ is slowly
being removed from the tree [1][2].

[1] https://lore.kernel.org/all/20250310104256.123527-1-thuth@xxxxxxxxxx/
[2] https://lore.kernel.org/all/20251218182029.166993-1-thuth@xxxxxxxxxx/

> +
> +#include <linux/compiler.h>
> +#include <linux/types.h>
>  
>  /*
>   * The exact memory layout for LGDT or LIDT instructions.
> @@ -63,4 +72,11 @@ struct td_boot_parameters {
>  	struct td_per_vcpu_parameters per_vcpu[];
>  };
>  
> +void td_boot(void);
> +void td_boot_code_end(void);

This looks a bit off to me. td_boot_code_end is just a symbol and not a
function at all. Maybe something like this?

  extern u8 td_boot_code_start[], td_boot_code_end[];


> +
> +#define TD_BOOT_CODE_SIZE (td_boot_code_end - td_boot)
> +
> +#endif /* !defined(__ASSEMBLY__) && !defined(__ASSEMBLER__) */
> +
>  #endif /* SELFTEST_TDX_TD_BOOT_H */
> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S
> new file mode 100644
> index 000000000000..bca9a4c3d8f8
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/td_boot.S
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#include "tdx/td_boot.h"
> +#include "tdx/td_boot_offsets.h"
> +#include "processor_asm.h"
> +
> +.code32
> +
> +.globl td_boot
> +td_boot:
> +	/*
> +	 * In this procedure, edi is used as a temporary register.

        Hmm, "mul %esi" clobbers edx as well. Not sure if that needs to
        be called out. Perhaps documenting the registers that need to be
        preserved thoughout this procedure (eax, ebx) is more useful. A
        comment below already mentions that edi is a scratch register.

> +	 * Paging is turned off.
> +	 */
> +	cli
> +
> +	movl $TD_BOOT_PARAMETERS_GPA, %ebx
> +
> +	/*
> +	 * Find the address of struct td_per_vcpu_parameters for this
> +	 * vCPU based on esi (TDX spec: initialized with vCPU id). Put
> +	 * struct address into register for indirect addressing.
                               ^^^^^^^^ maybe just say eax?

> +	 */
> +	movl $SIZEOF_TD_PER_VCPU_PARAMETERS, %eax
> +	mul %esi
> +	leal TD_BOOT_PARAMETERS_PER_VCPU(%ebx), %edi
> +	addl %edi, %eax
> +