Re: [PATCH v3 05/10] KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hoisting the vCPU ID check in `kvm_xen_hcall_vcpu_op()` incorrectly rejects all cross-vCPU `__HYPERVISOR_vcpu_op` hypercalls, breaking SMP initialization for Xen guests.
--

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

KVM: x86/xen: Consolidate checks on Xen vCPU ID for singleshot timer hypercalls

This commit hoists the Xen vCPU ID checks for set_singleshot_timer and
stop_singleshot_timer out of their switch cases in kvm_xen_hcall_vcpu_op()
into the common path.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1614,13 +1614,13 @@ static bool kvm_xen_hcall_vcpu_op(struct kvm_vcpu *vcpu, bool longmode, int cmd,
>  	if (vcpu->arch.xen.vcpu_id == XEN_VCPU_ID_INVALID)
>  		return false;
>  
> +	if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> +		*r = -EINVAL;
> +		return true;
> +	}
> +

[Severity: High]
Does hoisting this check unintentionally reject cross-vCPU commands that should
be delegated to userspace?

If a guest issues a cross-vCPU hypercall like VCPUOP_up or VCPUOP_initialise,
this check will now evaluate to true and return -EINVAL directly from the
kernel.

Previously, these non-timer hypercalls would bypass the switch statement and
fall through to return false, properly delegating the operation to userspace.

Could this cause regressions in SMP initialization for Xen guests by
intercepting and rejecting commands intended for secondary vCPUs?

>  	switch (cmd) {
>  	case VCPUOP_set_singleshot_timer:
> -		if (vcpu->arch.xen.vcpu_id != vcpu_id) {
> -			*r = -EINVAL;
> -			return true;
> -		}
> -

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625223623.3376478-1-seanjc@google.com?part=5