Re: [PATCH v8 02/13] xen/arm: gic-v2: Implement GIC suspend/resume functions

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

On Mon, May 11, 2026 at 9:41 AM Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
>
> Hi Mykola,
>
> >
> >>
> >>>
> >>> For GICC_APRn/GICC_NSAPRn, those registers describe active priority state 
> >>> for
> >>> interrupts already acknowledged by the CPU interface. The final suspend 
> >>> path is
> >>> not expected to run with an active physical interrupt context. If those
> >>> registers were non-zero there, restoring only APR/NSAPR would not make the
> >>> corresponding interrupt handling context valid after resume, and could 
> >>> instead
> >>> leave the CPU interface with stale active priority state.
> >>
> >> Ok I understand now, but if we are expecting here GICD_ISACTIVERn zeroed, 
> >> why are
> >> we saving/restoring it? Shouldn’t we instead have a runtime check that 
> >> it’s zero and in case
> >> it’s not bail out? And in the resume path we would only zero it.
> >>
> >> Am I missing something?
> >
> > Good questions.
> >
> > Yes, the distinction I should have made clearer is between CPU-interface
> > active-priority state and distributor active state.
> >
> > For GICC_APRn/GICC_NSAPRn, I expect the state to be quiesced at this point.
> > Those registers track active priorities in the CPU interface. Xen reaches
> > gic_suspend() with local interrupts disabled, and for the guest-routed
> > interrupt case that can leave a distributor active bit behind, Xen has
> > already performed the physical EOI, so the CPU-interface priority has been
> > dropped.
> > There is no CPU-interface active-priority context that we can meaningfully
> > replay after resume.
> >
> > That is different from GICD_ISACTIVERn. In EOImode==1, EOIR only drops the
> > priority. The interrupt remains active in the distributor until the separate
> > deactivation step. For a guest-routed interrupt Xen's GICv2 guest end path 
> > does
> > only the physical EOI; deactivation is completed later by the virtual 
> > GIC/GICV
> > path when the guest completes the interrupt.
> >
> > This is why APR/NSAPR and ISACTIVERn are treated differently. For example:
> >
> >  1. A physical IRQ routed to a guest is acknowledged by Xen.
> >  2. The GIC marks the interrupt active in the distributor.
> >  3. Xen EOIs it, which drops the physical priority.
> >  4. Xen queues/injects the interrupt to the vGIC.
> >  5. The guest has not yet run, or the virtual interrupt is not yet 
> > deliverable
> >     because of guest PMR/priority/local IRQ masking/vGIC state.
> >  6. Therefore the guest-side deactivate has not happened yet, and the 
> > physical
> >     distributor active bit remains set.
> >
> > There is also a late suspend window in the current Xen path: domains are
> > suspended and the scheduler is disabled before local IRQs are disabled.
> > A guest-routed IRQ can therefore be taken by Xen after the guest is already
> > suspended, but before gic_suspend(). Xen can EOI/priority-drop it and queue
> > it for the guest, while the guest cannot run and deactivate it before the
> > GIC state is saved.
> >
> > This is the same class of issue handled by Linux for GIC EOImode==1. Linux
> > saves/restores the active state because forwarded interrupts can remain 
> > active
> > while passed to a VM [1].
> >
> > So I don't think GICD_ISACTIVERn should be treated as "must be zero" unless 
> > we
> > also add an explicit suspend-abort/quiesce policy for in-flight guest
> > interrupts. That would be a different design: detect non-zero 
> > active/in-flight
> > state, unwind suspend, thaw domains, let the guest drain/deactivate the
> > interrupts, and retry later. This series does not implement that policy. 
> > Given
> > the current flow, preserving GICD_ISACTIVERn avoids losing architectural
> > interrupt-controller state across suspend/resume.
> >
> > I am not opposed to such a policy as a follow-up if we want stricter suspend
> > quiescence rules, but I think it should be designed explicitly rather than
> > inferred from the GIC save/restore code.
> >
> > Best regards,
> > Mykola
> >
> > [1] 
> > https://patchwork.kernel.org/project/linux-arm-kernel/patch/1447701208-18150-5-git-send-email-marc.zyngier@xxxxxxx/
>
> Right, yes I agree! I have another question though, since GICC_APRn state 
> should be
> quiesced in the suspend path (allimplemented active-priority bits should read 
> as zero),
> should we have a runtime check just after disabling the CPU interface?

Yes, I think a runtime check is appropriate here.

Best regards,
Mykola

>
> Cheers,
> Luca
>