Re: [PATCH v14 11/22] KVM: selftests: Set up TDX boot parameters region

[Peter Fang]

On Wed, Jul 22, 2026 at 11:13:16PM +0000, Lisa Wang wrote:
> From: Sagi Shahar <sagis@xxxxxxxxxx>
> 
> Allocate memory for TDX boot parameters and define the utility functions
> necessary to fill this memory with the boot parameters.
> 
> Co-developed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Signed-off-by: Sagi Shahar <sagis@xxxxxxxxxx>
> Signed-off-by: Lisa Wang <wyihan@xxxxxxxxxx>
> ---
>  .../selftests/kvm/include/x86/tdx/tdx_util.h       |  2 +
>  tools/testing/selftests/kvm/lib/x86/processor.c    |  2 +
>  tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c | 51 ++++++++++++++++++++++
>  3 files changed, 55 insertions(+)
> 

[ ... ]

> +
> +void tdx_vm_load_common_boot_parameters(struct kvm_vm *vm)
> +{
> +	struct td_boot_parameters *params =
> +		addr_gpa2hva(vm, TD_BOOT_PARAMETERS_GPA);
> +	u32 cr4;
> +
> +	cr4 = kvm_get_default_cr4(vm->mmu.pgtable_levels);
> +
> +	/* TDX spec 11.6.2: CR4 bit MCE is fixed to 1 */
> +	cr4 |= X86_CR4_MCE;
> +
> +	/* TDX spec 11.6.2: CR4 bit VMXE and SMXE are fixed to 0 */

        This is now out of date (11.9.2 as of today)...
        I wonder if it's better to just say something like: "Per the TDX
        spec, ..."

> +	cr4 &= ~(X86_CR4_VMXE | X86_CR4_SMXE);
> +
> +	/* Set parameters! */
> +	params->cr0 = kvm_get_default_cr0();
> +	TEST_ASSERT(vm->mmu.pgd < (1ULL << 32),
> +		    "PGD must be within 32-bit address space for 32-bit boot code");
> +	params->cr3 = vm->mmu.pgd;
> +	params->cr4 = cr4;
> +	params->idtr.base = vm->arch.idt;
> +	params->idtr.limit = kvm_get_default_idt_limit();
> +	params->gdtr.base = vm->arch.gdt;

        vm->arch.{idt,gdt} must also be 32-bit addressable here, just
        like vm->mmu.pgd.

> +	params->gdtr.limit = kvm_get_default_gdt_limit();
> +
> +	TEST_ASSERT(params->cr0 != 0, "cr0 should not be 0");
> +	TEST_ASSERT(params->cr3 != 0, "cr3 should not be 0");
> +	TEST_ASSERT(params->cr4 != 0, "cr4 should not be 0");
> +	TEST_ASSERT(params->gdtr.base != 0, "gdt base address should not be 0");
> +	TEST_ASSERT(params->idtr.base != 0, "idt base address should not be 0");

        Making sure that cr0/cr4 are simply non-zero feels kind of odd
        to me. And cr3/gdtr/idtr all came from memory allocators with
        non-zero minimum addresses.

> +}
> +
>  static struct kvm_tdx_capabilities *tdx_read_capabilities(struct kvm_vm *vm)
>  {
>  	static struct kvm_tdx_capabilities *tdx_cap;
> 
> -- 
> 2.55.0.229.g6434b31f56-goog
> 
>