Re: [PATCH v16 18/20] KVM: selftests: Add support for per-VM ucall ops on x86

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: selftests: ucall_x86_ops is a host global instead of per-VM state
--

commit 220d73be4cec0aafb094f8711c312a5a8ae361c9
Author: Sean Christopherson <seanjc@google.com>

KVM: selftests: Add support for per-VM ucall ops on x86

This commit introduces an ops structure for x86 ucall handling so that
different VM types, such as TDX VMs, can wire up their own do_ucall and
get_ucall callbacks.

> diff --git a/tools/testing/selftests/kvm/lib/x86/ucall.c b/tools/testing/selftests/kvm/lib/x86/ucall.c
> index 38050c60a0670..3aec4dd46d1b2 100644
> --- a/tools/testing/selftests/kvm/lib/x86/ucall.c
> +++ b/tools/testing/selftests/kvm/lib/x86/ucall.c
[ ... ]
> @@ -26,3 +26,26 @@ void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
>  	}
>  	return NULL;
>  }
> +
> +static struct {
> +	void (*do_ucall)(gva_t uc);
> +	void *(*get_ucall)(struct kvm_vcpu *vcpu);
> +} ucall_x86_ops = {
> +	.do_ucall = ucall_x86_do_ucall,
> +	.get_ucall = ucall_x86_get_ucall,
> +};
> +
> +void ucall_arch_init(struct kvm_vm *vm, gpa_t mmio_gpa)
> +{
> +	sync_global_to_guest(vm, ucall_x86_ops);
> +}
> +
> +void ucall_arch_do_ucall(gva_t uc)
> +{
> +	ucall_x86_ops.do_ucall(uc);
> +}
> +
> +void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
> +{
> +	return ucall_x86_ops.get_ucall(vcpu);
> +}

[Severity: Medium]
As reported by Sashiko on v15 [1], ucall_x86_ops in
tools/testing/selftests/kvm/lib/x86/ucall.c is defined as a single
process-wide static variable on the host rather than per-VM state.

While ucall_arch_init() uses sync_global_to_guest(vm, ucall_x86_ops) to copy
the structure into each guest VM's private address space for guest-side
ucall_arch_do_ucall() execution, ucall_arch_get_ucall() executes on the host
and dispatches through the single host-side ucall_x86_ops.get_ucall pointer
instead of per-VM state associated with vcpu->vm.

In the following patch ("KVM: selftests: Add support for TDX ucalls, via
TDVMCALL_REPORT_FATAL_ERROR"), ucall_arch_init() mutates the host
ucall_x86_ops in place when is_tdx_vm(vm) is true without restoring the
default x86 callbacks when is_tdx_vm(vm) is false. If a selftest process
creates a TDX VM via __vm_create() -> ucall_init() -> ucall_arch_init() and
concurrently or subsequently creates and runs a non-TDX VM, won't the
non-TDX VM inherit the TDX ucall callbacks both in guest memory and on the
host in get_ucall() -> ucall_arch_get_ucall()?

[1] https://lore.kernel.org/r/20261001-tdx-selftests-v15-21-7c62a5d8a992@google.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-tdx-selftests-v16-0-2024e8fa517a@google.com?part=18