Re: [PATCH v5 16/49] KVM: arm64: gic-v5: Request doorbells when VPEs enter WFI

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ABBA deadlock between `rq_lock` and `desc->lock` in `vgic_v5_put` via `irq_set_affinity`.
- [High] Stale `db_fired` state causes infinite VMEXIT loops upon subsequent WFIs.
- [High] Guest-reachable WARN_ON due to host-side `irq_set_affinity` failures.
--

--- Patch [16]: [PATCH v5 16/49] KVM: arm64: gic-v5: Request doorbells when VPEs enter WFI ---
commit c40948b426a64dc48dae28e60304e7f29600ab3c
Author: Sascha Bischoff <[email protected]>

KVM: arm64: gic-v5: Request doorbells when VPEs enter WFI

When a GICv5 VPE is made non-resident as part of the vcpu entering
WFI, request a VPE doorbell so that KVM can be notified when a
suitable SPI or LPI becomes pending for that VPE.

> diff --git a/arch/arm64/kvm/hyp/vgic-v5-sr.c b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> --- a/arch/arm64/kvm/hyp/vgic-v5-sr.c
> +++ b/arch/arm64/kvm/hyp/vgic-v5-sr.c
> @@ -22,6 +22,15 @@ void __vgic_v5_make_resident(struct vgic_v5_cpu_if *cpu_if)
>  
>  void __vgic_v5_make_non_resident(struct vgic_v5_cpu_if *cpu_if)
>  {
> +	/*
> +	 * Clear the db_fired state to ensure that we're ready for the next
> +	 * doorbell when it is requested. If a doorbell firing caused us to
> +	 * enter the guest, then we've already consumed that state at this
> +	 * point, so this is safe to clear. Use WRITE_ONCE() to ensure we're not
> +	 * racing with the doorbell firing and setting the state true again.
> +	 */
> +	WRITE_ONCE(cpu_if->gicv5_vpe.db_fired, false);

[Severity: High]
Will this state management regression leave the db_fired state stale for
back-to-back WFIs?

Since __vgic_v5_make_non_resident() is called from vgic_v5_put(), which is
invoked upon host preemption or user exit (but not standard guest re-entry
after kvm_vcpu_block), db_fired appears to remain true when the guest resumes.

If the guest executes WFI again, kvm_vcpu_check_block() checks
kvm_vgic_vcpu_pending_irq() in arch/arm64/kvm/vgic/vgic.c and observes
db_fired is still true, which causes it to instantly abort blocking. This
could result in the guest continuously spinning in VMEXITs.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
[ ... ]
> @@ -1099,6 +1099,46 @@ void vgic_v5_put(struct kvm_vcpu *vcpu)
[ ... ]
> +			/*
> +			 * Make the doorbell affine to this CPU, if it isn't
> +			 * already. Actively check the cpumask first as it is
> +			 * cheaper than changing the affinity every time.
> +			 */
> +			if (!cpumask_test_cpu(cpu, aff))
> +				WARN_ON(irq_set_affinity(db_irq, cpumask_of(cpu)));

[Severity: High]
Does this introduce an ABBA deadlock regression between rq_lock and desc->lock?

When a guest vCPU thread is preempted or enters WFI, the host scheduler's
__schedule() holds the runqueue lock (rq_lock) while calling
prepare_task_switch(). This triggers the sched_out preempt notifier, taking
us through kvm_sched_out() -> kvm_arch_vcpu_put() -> kvm_vgic_put() and
finally to vgic_v5_put().

Inside vgic_v5_put(), irq_set_affinity() acquires the hardware interrupt
descriptor lock (desc->lock). Taking desc->lock while already holding
rq_lock violates the locking hierarchy where desc->lock must be acquired
before rq_lock.

[Severity: High]
Can a guest trigger this WARN_ON regression to cause a host denial of service?

Calls to irq_set_affinity() can legitimately fail and return negative error
codes due to hardware constraints (such as ITS socket-crossing limitations)
or resource exhaustion.

Since the guest controls when it executes WFI, it can reliably trigger this
WARN_ON if the host scheduler places it on a restricted CPU. If panic_on_warn
is enabled, this allows the guest to crash the host.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16