Re: [PATCH v7 03/12] xen/arm: gic-v3: Implement GICv3 suspend/resume functions

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Julien,

Thank you for the review.

On Fri, Dec 26, 2025 at 2:39 PM Julien Grall <julien@xxxxxxx> wrote:
>
> Hi Mykola,
>
> On 11/12/2025 18:43, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> >
> > System suspend may lead to a state where GIC would be powered down.
> > Therefore, Xen should save/restore the context of GIC on suspend/resume.
> >
> > Note that the context consists of states of registers which are
> > controlled by the hypervisor. Other GIC registers which are accessible
> > by guests are saved/restored on context switch.
> >
> > Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> > ---
> > Changes in V7:
> > - restore LPI regs on resume
> > - add timeout during redist disabling
> > - squash with suspend/resume handling for GICv3 eSPI registers
> > - drop ITS guard paths so suspend/resume always runs; switch missing ctx
> >    allocation to panic
> > - trim TODO comments; narrow redistributor storage to PPI icfgr
> > - keep distributor context allocation even without ITS; adjust resume
> >    to use GENMASK(31, 0) for clearing enables
> > - drop storage of the SGI configuration register, as SGIs are always
> >    edge-triggered
> > ---
> >   xen/arch/arm/gic-v3-lpi.c              |   3 +
> >   xen/arch/arm/gic-v3.c                  | 319 ++++++++++++++++++++++++-
> >   xen/arch/arm/include/asm/gic_v3_defs.h |   1 +
> >   3 files changed, 320 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> > index de5052e5cf..61a6e18303 100644
> > --- a/xen/arch/arm/gic-v3-lpi.c
> > +++ b/xen/arch/arm/gic-v3-lpi.c
> > @@ -391,6 +391,9 @@ static int cpu_callback(struct notifier_block *nfb, 
> > unsigned long action,
> >       switch ( action )
> >       {
> >       case CPU_UP_PREPARE:
> > +        if ( system_state == SYS_STATE_resume )
> > +            break;
>
> Do we need this check because we don't free the LPI pending table when
> the CPU is turned off?

Yes. In the system suspend/resume case we intentionally keep the LPI
pending table in RAM and reuse it after wake-up/CPU bring-up. The state
is still stored in memory, so we do not need to save/restore it elsewhere;
we just need to avoid allocating a new table on resume.

>
> If so, don't we already have a problem for CPU hotplug because the
> percpu area will be freed but not the pending table?

System suspend/resume is a special case in Xen: system_state is
transitioned during suspend, and the generic percpu allocator does not
free/allocate percpu areas in that state (see [1] and [2]). As a result,
the percpu storage (including the pointer to the pending table) remains
intact across the suspend/resume cycle, and reusing the existing table
is safe.

CPU hotplug is different: system_state remains in the normal running
state, so percpu areas can be freed on CPU offline. In that scenario
we should not rely on the “reuse on resume” logic; the pending table
needs to be explicitly freed as part of the CPU teardown (otherwise it
would indeed be leaked once the percpu pointer is gone). The check in
this change is intended for suspend/resume, not for hotplug semantics.

[1] https://elixir.bootlin.com/xen/v4.21.0/source/xen/common/percpu.c#L36
[2] https://elixir.bootlin.com/xen/v4.21.0/source/xen/common/percpu.c#L88

Thanks,
Mykola

>
> Cheers,
>
> --
> Julien Grall
>