Re: [PATCH v15 22/23] KVM: selftests: Add support for TDX ucalls, via TDVMCALL_REPORT_FATAL_ERROR

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

On 10/2/2026 5:13 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] kvm: selftests: ucall global state poisoning breaks mixed-VM testing
> --
> 
> commit 742d058040571d70e8a67c18a52e7026f4abe945
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: selftests: Add support for TDX ucalls, via TDVMCALL_REPORT_FATAL_ERROR
> 
> This commit adds support for performing ucalls on TDX by utilizing the
> TDVMCALL_REPORT_FATAL_ERROR hypercall. It replaces the default ucall
> operations with TDX-specific handlers when a TDX VM is initialized.
> 
>> diff --git a/tools/testing/selftests/kvm/lib/x86/ucall.c b/tools/testing/selftests/kvm/lib/x86/ucall.c
>> index c003df3c7b8a8..c1cd32d684646 100644
>> --- a/tools/testing/selftests/kvm/lib/x86/ucall.c
>> +++ b/tools/testing/selftests/kvm/lib/x86/ucall.c
> 
> [ ... ]
> 
>> @@ -37,6 +57,11 @@ static struct {
>>  
>>  void ucall_arch_init(struct kvm_vm *vm, gpa_t mmio_gpa)
>>  {
>> +	if (is_tdx_vm(vm)) {
>> +		ucall_x86_ops.do_ucall = ucall_tdx_do_ucall;
>> +		ucall_x86_ops.get_ucall = ucall_tdx_get_ucall;
>> +	}
>> +
> 
> [Severity: Medium]
> Since ucall_x86_ops is a process-wide global struct, does modifying it here
> without an else branch to restore the default handlers break mixed-VM testing?
> 
> If a process creates a TDX VM and later creates a non-TDX VM, the non-TDX VM
> will inherit these TDX handlers. When sync_global_to_guest() is called, this
> modified state syncs into the non-TDX VM's memory, which might cause the
> guest to incorrectly issue TDCALL for ucalls, leading to an Invalid Opcode
> exception in the non-TDX guest.

Is it a possible scenario to test with TDX and non-TDX VMs in one process?

> 
> Additionally, host-side dispatch via ucall_arch_get_ucall() relies on this
> globally assigned pointer:
> 
> void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
> {
> 	return ucall_x86_ops.get_ucall(vcpu);
> }
> 
> Will this cause the host to dispatch based on the last initialized VM rather
> than dynamically checking is_tdx_vm(vcpu->vm)?
> 
>>  	sync_global_to_guest(vm, ucall_x86_ops);
>>  }
>