[PATCH] KVM: arm64: vgic: Check the interrupt is still ours before migrating it
Marc Zyngier
maz at kernel.org
Sun Jun 14 08:16:44 PDT 2026
On Fri, 12 Jun 2026 03:22:35 +0100,
Hyunwoo Kim <imv4bel at gmail.com> wrote:
>
> On Wed, Jun 10, 2026 at 05:00:25PM +0100, Marc Zyngier wrote:
> > It's rather unclear to me what the semantics of this are.
> >
> > If vcpu-a decides to nuke the LPIs of vcpu-b and the LPI had in the
> > meantime been migrated to vcpu-c, but obviously not observed by vcpu-c
> > yet as the LPI is still on vcpu-b's AP-list, then I don't see the
> > point in keeping this state.
> >
> > Am I missing something obvious?
>
> I looked a bit more into Oliver's review, the one suggesting that pending
> be cleared only for resident LPIs while the ones being migrated are left
> in place.
>
> What the leave preserves is the pending edge of a single LPI whose target
> is already vcpu-c but which is still on vcpu-b's ap_list. This edge is
> always lost when we just clear it, but for a device that fires again a
> later INT reaches vcpu-c through the oracle, so it is mostly harmless.
Not completely harmless. When the guest writes EnableLPIs==0, it
accepts the lost of any pending bit that could be stored. These won't
be regenerated, unless the device signals a new event that maps to the
same LPIs. But again, this is the guest's own decision, and I don't
see a reason to prevent it from shooting itself in the foot.
> The
> exception is a software LPI that never fires again(irq->hw == false):
> that edge is then lost with no way to recover it, because
> its_sync_lpi_pending_table only re-syncs the LPIs whose target_vcpu matches,
> and the disable path does no pending writeback. I am not entirely sure about
> this part, though.
I don't think this is a problem, as the architecture doesn't guarantee
the state of the pending table after turning EnableLPIs off. There's
even a note recommending to move the interrupts to another RD before
doing that.
>
> Since this does not look like the common case, if it does not need to be
> covered I will send v2 keeping only the pending clear and the ref hold in
> vgic_prune_ap_list(). What do you think?
So that it is entirely unambiguous, my suggestion is to have this:
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 5a4768d8cd4f3..70a161383e5a6 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -203,6 +203,7 @@ void vgic_flush_pending_lpis(struct kvm_vcpu *vcpu)
list_for_each_entry_safe(irq, tmp, &vgic_cpu->ap_list_head, ap_list) {
if (irq_is_lpi(vcpu->kvm, irq->intid)) {
raw_spin_lock(&irq->irq_lock);
+ irq->pending_latch = false;
list_del(&irq->ap_list);
irq->vcpu = NULL;
raw_spin_unlock(&irq->irq_lock);
@@ -792,7 +793,11 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
continue;
}
- /* This interrupt looks like it has to be migrated. */
+ /*
+ * This interrupt looks like it has to be migrated,
+ * make sure it is kept alive while locks are dropped.
+ */
+ vgic_get_irq_ref(irq);
raw_spin_unlock(&irq->irq_lock);
raw_spin_unlock(&vgic_cpu->ap_list_lock);
@@ -836,6 +841,8 @@ static void vgic_prune_ap_list(struct kvm_vcpu *vcpu)
raw_spin_unlock(&vcpuB->arch.vgic_cpu.ap_list_lock);
raw_spin_unlock(&vcpuA->arch.vgic_cpu.ap_list_lock);
+ deleted_lpis |= vgic_put_irq_norelease(vcpu->kvm, irq);
+
if (target_vcpu_needs_kick) {
kvm_make_request(KVM_REQ_IRQ_PENDING, target_vcpu);
kvm_vcpu_kick(target_vcpu);
Could you please give it a go with whatever reproducer you have?
Thanks,
M.
--
Jazz isn't dead. It just smells funny.
More information about the linux-arm-kernel
mailing list