Re: [PATCH v15 10/23] KVM: selftests: Set up TDX boot parameters region

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] selftests: ineffective TEST_ASSERT due to prior 32-bit truncation
--

commit 818beb22776783ed2680d4f875f92a2f1dad9ff5
Author: Sagi Shahar <sagis@google.com>

KVM: selftests: Set up TDX boot parameters region

This commit allocates memory for TDX boot parameters and defines the utility
functions necessary to fill this memory with the boot parameters.

> diff --git a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> --- a/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
> +++ b/tools/testing/selftests/kvm/lib/x86/tdx/tdx_util.c
[ ... ]
> @@ -38,6 +39,53 @@ void tdx_vm_setup_boot_code_region(struct kvm_vm *vm)
[ ... ]
> +void tdx_vm_load_common_boot_parameters(struct kvm_vm *vm)
> +{
[ ... ]
> +	/* 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;
> +	params->gdtr.limit = kvm_get_default_gdt_limit();
> +
> +	TEST_ASSERT(params->idtr.base < (1ULL << 32),
> +		    "idt must be within 32-bit address space for 32-bit boot code");
> +	TEST_ASSERT(params->gdtr.base < (1ULL << 32),
> +		    "gdt must be within 32-bit address space for 32-bit boot code");

[Severity: Low]
Does this assignment silently truncate the 64-bit vm->arch.idt and
vm->arch.gdt addresses?

Since params->idtr.base and params->gdtr.base are 32-bit fields, the
subsequent TEST_ASSERT checks evaluate the already-truncated values. If a
test allocates an IDT or GDT address beyond 4GB, the assertions will
always be trivially true because the upper 32 bits are lost before the
check.

This could lead to confusing test failures during guest execution instead
of properly failing the assertion here.

Should these assertions check vm->arch.idt and vm->arch.gdt directly
before assigning them to the base fields?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001-tdx-selftests-v15-0-7c62a5d8a992@google.com?part=10