[PATCH] KVM: arm64: vgic: Check the interrupt is still ours before migrating it
Hyunwoo Kim
imv4bel at gmail.com
Sun Jun 14 22:43:39 PDT 2026
On Sun, Jun 14, 2026 at 04:16:44PM +0100, Marc Zyngier wrote:
> 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?
I confirmed your diff fixes the issue. Could you submit this
patch? Feel free to add Tested-by: Hyunwoo Kim <imv4bel at gmail.com>
Best regards,
Hyunwoo Kim
More information about the linux-arm-kernel
mailing list