[PATCH] KVM: arm64: Skip interrupts in LRs during EOIcount replay

[PATCH] KVM: arm64: Skip interrupts in LRs during EOIcount replay

[Valentine Burley]

Commit 05984ba67eb6 ("KVM: arm64: Invert ap_list sorting to push active
interrupts out") allowed active interrupts to be evicted from LRs to
make room for pending ones.

When an evicted interrupt is deactivated by the guest, the GIC
increments EOIcount. KVM replays this by finding an active interrupt
in the ap_list to deactivate. However, the replay logic may pick an
interrupt that is currently residing in an LR, leading to a spurious
deactivation and leaving the actually finished interrupt active.
This results in interrupt storms and boot failures (seen in Cuttlefish
guests on Qualcomm SC7180).

Fix this by skipping interrupts that are currently assigned to an LR
during EOIcount replay.

This allows booting Android VMs in Cuttlefish again.

Fixes: 05984ba67eb6 ("KVM: arm64: Invert ap_list sorting to push active interrupts out")
Cc: stable@vger.kernel.org
Signed-off-by: Valentine Burley <valentine.burley@collabora.com>
---
 arch/arm64/kvm/vgic/vgic-v2.c | 14 ++++++++++++++
 arch/arm64/kvm/vgic/vgic-v3.c | 19 +++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index 585491fbda80..821cf5bc30da 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -135,6 +135,20 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 			      irq->active))
 				continue;
 
+			bool was_in_lr = false;
+
+			for (int i = 0; i < cpuif->used_lrs; i++) {
+				u32 intid = cpuif->vgic_lr[i] & GICH_LR_VIRTUALID;
+
+				if (intid == irq->intid) {
+					was_in_lr = true;
+					break;
+				}
+			}
+
+			if (was_in_lr)
+				continue;
+
 			lr = vgic_v2_compute_lr(vcpu, irq) & ~GICH_LR_ACTIVE_BIT;
 		}
 
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 1d6dd1b545bd..00d9bc39bffb 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -179,6 +179,25 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 			      irq->active))
 				continue;
 
+			bool was_in_lr = false;
+
+			for (int i = 0; i < cpuif->used_lrs; i++) {
+				u32 intid;
+
+				if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
+					intid = cpuif->vgic_lr[i] & ICH_LR_VIRTUAL_ID_MASK;
+				else
+					intid = cpuif->vgic_lr[i] & GICH_LR_VIRTUALID;
+
+				if (intid == irq->intid) {
+					was_in_lr = true;
+					break;
+				}
+			}
+
+			if (was_in_lr)
+				continue;
+
 			lr = vgic_v3_compute_lr(vcpu, irq) & ~ICH_LR_ACTIVE_BIT;
 		}
 
-- 
2.51.0

Re: [PATCH] KVM: arm64: Skip interrupts in LRs during EOIcount replay

[Marc Zyngier]

Hi Valentine,

Thanks for this.

On Sat, 07 Mar 2026 11:59:50 +0000,
Valentine Burley <valentine.burley@collabora.com> wrote:
> 
> Commit 05984ba67eb6 ("KVM: arm64: Invert ap_list sorting to push active
> interrupts out") allowed active interrupts to be evicted from LRs to
> make room for pending ones.
> 
> When an evicted interrupt is deactivated by the guest, the GIC
> increments EOIcount. KVM replays this by finding an active interrupt
> in the ap_list to deactivate. However, the replay logic may pick an
> interrupt that is currently residing in an LR, leading to a spurious
> deactivation and leaving the actually finished interrupt active.

Well, the interrupt is not in an LR anymore. It has been sync'd
("folded") back into the vgic_irq structure, and the lr copy is now
stale.

But the observation is key: if in EOImode==0, and if we don't trap
ICC_EOIRx_EL1 with ICH_HCR_EL2.TC, we can race against the maintenance
interrupt that signals a non-zero EOIcount, leaving the guest a chance
to Ack more interrupts. Since we don't have the interrupt number, we
pick the first active interrupt in the AP list, with prejudice. Boo.

I can't reproduce it locally, but in a crap integration, where the GIC
is clocked at a few dozen MHz, this is far more likely to happen. I
should dig that Lazor out of the bin and put it back in the test rig.

In retrospect, it is obvious. I just couldn't see it until then. Many
thanks for going the extra mile and pointing out the core issue.

[skip v2 stuff]

> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index 1d6dd1b545bd..00d9bc39bffb 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -179,6 +179,25 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
>  			      irq->active))
>  				continue;
>  
> +			bool was_in_lr = false;
> +
> +			for (int i = 0; i < cpuif->used_lrs; i++) {
> +				u32 intid;
> +
> +				if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3)
> +					intid = cpuif->vgic_lr[i] & ICH_LR_VIRTUAL_ID_MASK;
> +				else
> +					intid = cpuif->vgic_lr[i] & GICH_LR_VIRTUALID;
> +
> +				if (intid == irq->intid) {
> +					was_in_lr = true;
> +					break;
> +				}
> +			}
> +
> +			if (was_in_lr)
> +				continue;
> +

I'm afraid we can't afford this sort of quadratic behaviour, and I
really don't want to go back rummaging in what was in the LRs -- this
data is notionally stale. But we can rely on some properties of the AP
list, of which the interrupts in the LRs are guaranteed to be a strict
prefix.

We just need to keep track of what point we have reached in the AP
list at flush time, and use that as the starting point for the
EOIcount search. It's a tiny bit of extra state, but I'd rather have
that than what you are suggesting here.

Could you please give the hack below a go on your setup? It seems to
work for me, but given that I never observed the issue the first
place...

Thanks again,

	M.

diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index 585491fbda807..e5b49274a6683 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -115,7 +115,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_v2_cpu_if *cpuif = &vgic_cpu->vgic_v2;
 	u32 eoicount = FIELD_GET(GICH_HCR_EOICOUNT, cpuif->vgic_hcr);
-	struct vgic_irq *irq;
+	struct vgic_irq *irq = vgic_cpu->last_lr_irq;
 
 	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
 
@@ -123,7 +123,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 		vgic_v2_fold_lr(vcpu, cpuif->vgic_lr[lr]);
 
 	/* See the GICv3 equivalent for the EOIcount handling rationale */
-	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
+	list_for_each_entry_continue(irq, &vgic_cpu->ap_list_head, ap_list) {
 		u32 lr;
 
 		if (!eoicount) {
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 386ddf69a9c51..457fb4cd3fc63 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -148,7 +148,7 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 	struct vgic_v3_cpu_if *cpuif = &vgic_cpu->vgic_v3;
 	u32 eoicount = FIELD_GET(ICH_HCR_EL2_EOIcount, cpuif->vgic_hcr);
-	struct vgic_irq *irq;
+	struct vgic_irq *irq = vgic_cpu->last_lr_irq;
 
 	DEBUG_SPINLOCK_BUG_ON(!irqs_disabled());
 
@@ -158,12 +158,12 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 	/*
 	 * EOIMode=0: use EOIcount to emulate deactivation. We are
 	 * guaranteed to deactivate in reverse order of the activation, so
-	 * just pick one active interrupt after the other in the ap_list,
-	 * and replay the deactivation as if the CPU was doing it. We also
-	 * rely on priority drop to have taken place, and the list to be
-	 * sorted by priority.
+	 * just pick one active interrupt after the other in the tail part
+	 * of the ap_list, past the LRs, and replay the deactivation as if
+	 * the CPU was doing it. We also rely on priority drop to have taken
+	 * place, and the list to be sorted by priority.
 	 */
-	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
+	list_for_each_entry_continue(irq, &vgic_cpu->ap_list_head, ap_list) {
 		u64 lr;
 
 		/*
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 430aa98888fda..4a641a12d026b 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -814,6 +814,9 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
 
 static inline void vgic_fold_lr_state(struct kvm_vcpu *vcpu)
 {
+	if (!vcpu->arch.vgic_cpu.last_lr_irq)
+		return;
+
 	if (kvm_vgic_global_state.type == VGIC_V2)
 		vgic_v2_fold_lr_state(vcpu);
 	else
@@ -960,10 +963,13 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 	if (irqs_outside_lrs(&als))
 		vgic_sort_ap_list(vcpu);
 
+	vgic_cpu->last_lr_irq = NULL;
+
 	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
 		scoped_guard(raw_spinlock,  &irq->irq_lock) {
 			if (likely(vgic_target_oracle(irq) == vcpu)) {
 				vgic_populate_lr(vcpu, irq, count++);
+				vgic_cpu->last_lr_irq = irq;
 			}
 		}
 
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index f2eafc65bbf4c..61040a14cb388 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -359,6 +359,9 @@ struct vgic_cpu {
 	 */
 	struct list_head ap_list_head;
 
+	/* Last vgic_irq part of the AP list recorded in an LR */
+	struct vgic_irq *last_lr_irq;
+
 	/*
 	 * Members below are used with GICv3 emulation only and represent
 	 * parts of the redistributor.

-- 
Jazz isn't dead. It just smells funny.

Re: [PATCH] KVM: arm64: Skip interrupts in LRs during EOIcount replay

[Valentine Burley]

Hi Marc,

Thanks a lot for your reply.

On Sat, 07 Mar 2026 17:33:08 +0100  Marc Zyngier <maz@kernel.org> wrote
 > I can't reproduce it locally, but in a crap integration, where the GIC
 > is clocked at a few dozen MHz, this is far more likely to happen. I
 > should dig that Lazor out of the bin and put it back in the test rig.
 > 
 > In retrospect, it is obvious. I just couldn't see it until then. Many
 > thanks for going the extra mile and pointing out the core issue.

Appreciate the clarification!

We also have a few Lazor boards in our CI, and the Trogdors are indeed
legendary for hitting all kinds of edge cases.

<snip>

 > Could you please give the hack below a go on your setup? It seems to
 > work for me, but given that I never observed the issue the first
 > place...

I've tested this on my sc7180 Trogdor setup, and it completely 
resolves the issue. The Cuttlefish VM now boots reliably.

Tested-by: Valentine Burley <valentine.burley@collabora.com>

Many thanks for the quick solution!

Best regards,
Valentine

Re: [PATCH] KVM: arm64: Skip interrupts in LRs during EOIcount replay

[Marc Zyngier]

On Sat, 07 Mar 2026 18:29:05 +0000,
Valentine Burley <valentine.burley@collabora.com> wrote:
> 
> Hi Marc,
> 
> Thanks a lot for your reply.
> 
> On Sat, 07 Mar 2026 17:33:08 +0100  Marc Zyngier <maz@kernel.org> wrote
>  > I can't reproduce it locally, but in a crap integration, where the GIC
>  > is clocked at a few dozen MHz, this is far more likely to happen. I
>  > should dig that Lazor out of the bin and put it back in the test rig.
>  > 
>  > In retrospect, it is obvious. I just couldn't see it until then. Many
>  > thanks for going the extra mile and pointing out the core issue.
> 
> Appreciate the clarification!
> 
> We also have a few Lazor boards in our CI, and the Trogdors are indeed
> legendary for hitting all kinds of edge cases.

Right. Time to have a look at SBoyd's u-boot patches from last year!

> 
> <snip>
> 
>  > Could you please give the hack below a go on your setup? It seems to
>  > work for me, but given that I never observed the issue the first
>  > place...
> 
> I've tested this on my sc7180 Trogdor setup, and it completely 
> resolves the issue. The Cuttlefish VM now boots reliably.
>
> Tested-by: Valentine Burley <valentine.burley@collabora.com>

Awesome. I have since tweaked the patch a tiny bit to keep the restart
pointer per CPU instead of per vcpu, as this doesn't live beyond the
load/put sequence. I'll post it formally for you to confirm that the
updated version is still correct.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.