Re: [PATCH v3 09/10] KVM: Add IPI-aware directed-yield candidate selection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new candidate filter incorrectly excludes woken-up, runnable vCPUs from being yield targets by strictly requiring the `preempted` flag.
--

commit f2a5464aa09eda51a02ed6e5f04bf8ea0072c9ba
Author: Wanpeng Li <wanpengli@tencent.com>

KVM: Add IPI-aware directed-yield candidate selection

This commit introduces a priority-based candidate filter for directed yields
in kvm_vcpu_on_spin(). It prefers recent IPI receivers or vCPUs with pending
interrupts before falling back to the legacy preempted heuristic.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c

[ ... ]

> +static bool kvm_vcpu_is_good_yield_candidate(struct kvm_vcpu *me,
> +					     struct kvm_vcpu *vcpu,
> +					     bool yield_to_kernel_mode)
> +{
> +	/* Priority 1: confirmed recent IPI receiver. */
> +	if (kvm_vcpu_is_ipi_receiver(me, vcpu))
> +		return true;
> +
> +	/* Priority 2: arch-specific pending-interrupt hint. */
> +	if (kvm_arch_dy_has_pending_interrupt(vcpu))
> +		return true;
> +
> +	/* Priority 3: preempted, with optional in-kernel requirement. */
> +	if (!READ_ONCE(vcpu->preempted))
> +		return false;

[Severity: High]
Does this code inadvertently exclude runnable but non-preempted vCPUs
from being yield targets?

In the previous implementation, the preempted check was part of a compound
skip condition:

	if (READ_ONCE(vcpu->preempted) && yield_to_kernel_mode &&
	    !kvm_arch_dy_has_pending_interrupt(vcpu) &&
	    !kvm_arch_vcpu_preempted_in_kernel(vcpu))
		continue;

This meant that if a vCPU was not preempted (for instance, it was recently
woken up from a blocking state and is ready to run), it bypassed the skip
condition and was considered a valid target.

By explicitly returning false here when vcpu->preempted is false, this
new filter seems to drop those valid targets.

> +
> +	if (yield_to_kernel_mode && !kvm_arch_vcpu_preempted_in_kernel(vcpu))
> +		return false;
> +
> +	return true;
> +}

[ ... ]

>  		if (kvm_vcpu_is_blocking(vcpu) && !vcpu_dy_runnable(vcpu))
>  			continue;
>  
> -		/*
> -		 * Treat the target vCPU as being in-kernel if it has a pending
> -		 * interrupt, as the vCPU trying to yield may be spinning
> -		 * waiting on IPI delivery, i.e. the target vCPU is in-kernel
> -		 * for the purposes of directed yield.
> -		 */
> -		if (READ_ONCE(vcpu->preempted) && yield_to_kernel_mode &&
> -		    !kvm_arch_dy_has_pending_interrupt(vcpu) &&
> -		    !kvm_arch_vcpu_preempted_in_kernel(vcpu))
> -			continue;
> +		if (first_round) {
> +			/* Strict round: IPI-aware and legacy preempted filters. */
> +			if (!kvm_vcpu_is_good_yield_candidate(me, vcpu,
> +							      yield_to_kernel_mode))
> +				continue;
> +		} else {
> +			/*
> +			 * Relaxed round: only require preempted. This is the
> +			 * safety net for missed IPI tracking (e.g. APICv) or
> +			 * transient runnable-set changes since the strict
> +			 * scan.
> +			 */
> +			if (!READ_ONCE(vcpu->preempted))
> +				continue;

[Severity: High]
Similarly, does this relaxed round unconditionally skip non-preempted
vCPUs?

If the vCPU is simply runnable but not marked as preempted, this safety net
will completely bypass it.

>  		}
>  
>  		if (!kvm_vcpu_eligible_for_directed_yield(vcpu))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612013355.59231-1-kernellwp@gmail.com?part=9