Re: [PATCH v3 27/40] KVM: arm64: gic-v5: Track SPI state for in-flight SPIs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] O(N^2) list iteration and hypercalls under a global lock causes severe lock contention.
- [High] Level-triggered SPIs will permanently deadlock because the ack notifier is gated behind `!irq_is_pending(irq)`.
- [High] Unconditional acquisition of the global `vgic_v5_spi_ap_list_lock` in the vCPU fast-path creates severe lock contention.
--

commit 28a2c4a894df246c922d7f15a80a0ec24111b271
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Track SPI state for in-flight SPIs

This commit adds tracking for GICv5 SPI state to trigger deactivation
notifiers when SPIs are consumed by the guest. It synchronizes guest
PPI and SPI states back to KVM's shadow state during vCPU exits.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index ffb0500a4c9a7..20ae7daf4c451 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c

[ ... ]

> @@ -1056,6 +1056,63 @@ void vgic_v5_flush_ppi_state(struct kvm_vcpu *vcpu)
> +void vgic_v5_fold_irq_state(struct kvm_vcpu *vcpu)
> +{
> +	struct vgic_dist *vgic_dist = &vcpu->kvm->arch.vgic;
> +	struct vgic_irq *irq;
> +
> +	/* Sync back the guest PPI state to the KVM shadow state */
> +	vgic_v5_fold_ppi_state(vcpu);
> +
> +	/*
> +	 * For SPIs, which are on the global AP list, we synchronise their state
> +	 * with the hardware state. If they have been deactivated, immediately
> +	 * pop them off the list. The notifier is called without the SPI AP list
> +	 * lock held to avoid deadlocks.
> +	 */
> +retry:
> +	raw_spin_lock(&vgic_dist->vgic_v5_spi_ap_list_lock);

[Severity: High]
The commit message states "When no SPIs are in flight, no SPI state is
queried", but does this code unconditionally acquire the global
vgic_v5_spi_ap_list_lock on every vCPU exit even if the list is empty?

Could this introduce cacheline bouncing and global lock contention
across all vCPUs during normal VM operation?

> +	list_for_each_entry(irq, &vgic_dist->vgic_v5_spi_ap_list_head, ap_list) {
> +		bool pending;
> +		u32 intid;
> +		u64 icsr;
> +
> +		raw_spin_lock(&irq->irq_lock);
> +
> +		icsr = kvm_call_hyp_ret(__vgic_v5_vdrcfg, irq->intid);
> +
> +		irq->active = !!FIELD_GET(ICC_ICSR_EL1_Active, icsr);
> +		pending = !!FIELD_GET(ICC_ICSR_EL1_Pending, icsr);
> +
> +		if (irq->config == VGIC_CONFIG_EDGE)
> +			irq->pending_latch = pending;
> +
> +		if (irq->config == VGIC_CONFIG_LEVEL && !(pending || irq->active))
> +			irq->pending_latch = false;
> +
> +		/* Deactivated? */
> +		if (!irq->active && !pending && !irq_is_pending(irq)) {

[Severity: High]
Will this condition permanently block level-triggered SPIs from firing the
ack notifier?

For a level-triggered irqfd, irq_is_pending(irq) checks irq->line_level,
which stays high until the resampler runs. If kvm_notify_acked_irq() is
gated by this check, the resampler will never run to clear the software
line level, locking up the interrupt after its first injection.

> +			/* Use raw SPI index without type for the GSI */
> +			intid = FIELD_GET(GICV5_HWIRQ_ID, irq->intid);
> +
> +			/* And we're done with this SPI */
> +			list_del(&irq->ap_list);
> +			irq->vcpu = NULL;
> +
> +			raw_spin_unlock(&irq->irq_lock);
> +			raw_spin_unlock(&vgic_dist->vgic_v5_spi_ap_list_lock);
> +
> +			kvm_notify_acked_irq(vcpu->kvm, 0, intid);
> +			vgic_put_irq(vcpu->kvm, irq);
> +
> +			goto retry;

[Severity: High]
Does this goto retry result in an O(N^2) loop?

Whenever an SPI is deleted, the loop restarts from the head of
vgic_v5_spi_ap_list_head. This forces the code to re-evaluate all
previously visited SPIs using the __vgic_v5_vdrcfg hypercall
while holding the global list lock, which might heavily penalize 
performance when multiple SPIs deactivate simultaneously.

> +		}
> +
> +		raw_spin_unlock(&irq->irq_lock);
> +	}
> +	raw_spin_unlock(&vgic_dist->vgic_v5_spi_ap_list_lock);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703154811.3355680-1-sascha.bischoff@arm.com?part=27