From: Mykola Kvach <mykola_kvach@epam.com>

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@epam.com>
---
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;
+
         rc = gicv3_lpi_allocate_pendtable(cpu);
         if ( rc )
             printk(XENLOG_ERR "Unable to allocate the pendtable for CPU%lu\n",
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index bc07f97c16..dc5e58066d 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1067,12 +1067,12 @@ out:
     return res;
 }
 
-static void gicv3_hyp_disable(void)
+static void gicv3_hyp_enable(bool enable)
 {
     register_t hcr;
 
     hcr = READ_SYSREG(ICH_HCR_EL2);
-    hcr &= ~GICH_HCR_EN;
+    hcr = enable ? (hcr | GICH_HCR_EN) : (hcr & ~GICH_HCR_EN);
     WRITE_SYSREG(hcr, ICH_HCR_EL2);
     isb();
 }
@@ -1179,7 +1179,7 @@ static void gicv3_disable_interface(void)
     spin_lock(&gicv3.lock);
 
     gicv3_cpu_disable();
-    gicv3_hyp_disable();
+    gicv3_hyp_enable(false);
 
     spin_unlock(&gicv3.lock);
 }
@@ -1915,6 +1915,311 @@ static bool gic_dist_supports_lpis(void)
     return (readl_relaxed(GICD + GICD_TYPER) & GICD_TYPE_LPIS);
 }
 
+#ifdef CONFIG_SYSTEM_SUSPEND
+
+/* This struct represent block of 32 IRQs */
+struct dist_irq_block {
+    uint32_t icfgr[2];
+    uint32_t ipriorityr[8];
+    uint64_t irouter[32];
+    uint32_t isactiver;
+    uint32_t isenabler;
+};
+
+struct redist_ctx {
+    uint32_t ctlr;
+    uint32_t icfgr; /* only PPIs stored */
+    uint32_t igroupr;
+    uint32_t ipriorityr[8];
+    uint32_t isactiver;
+    uint32_t isenabler;
+
+    uint64_t pendbase;
+    uint64_t propbase;
+};
+
+/* GICv3 registers to be saved/restored on system suspend/resume */
+struct gicv3_ctx {
+    struct dist_ctx {
+        uint32_t ctlr;
+        struct dist_irq_block *irqs, *espi_irqs;
+    } dist;
+
+    /* have only one rdist structure for last running CPU during suspend */
+    struct redist_ctx rdist;
+
+    struct cpu_ctx {
+        uint32_t ctlr;
+        uint32_t pmr;
+        uint32_t bpr;
+        uint32_t sre_el2;
+        uint32_t grpen;
+    } cpu;
+};
+
+static struct gicv3_ctx gicv3_ctx;
+
+static void __init gicv3_alloc_context(void)
+{
+    uint32_t blocks = DIV_ROUND_UP(gicv3_info.nr_lines, 32);
+
+    /* The spec allows for systems without any SPIs */
+    if ( blocks > 1 )
+    {
+        gicv3_ctx.dist.irqs = xzalloc_array(struct dist_irq_block, blocks - 1);
+        if ( !gicv3_ctx.dist.irqs )
+            panic("Failed to allocate memory for GICv3 suspend context\n");
+    }
+
+#ifdef CONFIG_GICV3_ESPI
+    if ( !gic_number_espis() )
+        return;
+
+    blocks = gic_number_espis() / 32;
+    gicv3_ctx.dist.espi_irqs = xzalloc_array(struct dist_irq_block, blocks);
+    if ( !gicv3_ctx.dist.espi_irqs )
+        panic("Failed to allocate memory for GICv3 eSPI suspend context\n");
+#endif
+}
+
+static int gicv3_disable_redist(void)
+{
+    void __iomem* waker = GICD_RDIST_BASE + GICR_WAKER;
+    s_time_t deadline;
+
+    /*
+     * Avoid infinite loop if Non-secure does not have access to GICR_WAKER.
+     * See Arm IHI 0069H.b, 12.11.42 GICR_WAKER:
+     *     When GICD_CTLR.DS == 0 and an access is Non-secure accesses to this
+     *     register are RAZ/WI.
+     */
+    if ( !(readl_relaxed(GICD + GICD_CTLR) & GICD_CTLR_DS) )
+        return 0;
+
+    deadline = NOW() + MILLISECS(1000);
+
+    writel_relaxed(readl_relaxed(waker) | GICR_WAKER_ProcessorSleep, waker);
+    while ( (readl_relaxed(waker) & GICR_WAKER_ChildrenAsleep) == 0 )
+    {
+        if ( NOW() > deadline )
+        {
+            printk("GICv3: Timeout waiting for redistributor to sleep\n");
+            return -ETIMEDOUT;
+        }
+        cpu_relax();
+        udelay(10);
+    }
+
+    return 0;
+}
+
+#define GET_SPI_REG_OFFSET(name, is_espi) \
+    ((is_espi) ? GICD_##name##nE : GICD_##name)
+
+static void gicv3_store_spi_irq_block(struct dist_irq_block *irqs,
+                                      unsigned int i, bool is_espi)
+{
+    void __iomem *base;
+    unsigned int irq;
+
+    base = GICD + GET_SPI_REG_OFFSET(ICFGR, is_espi) + i * sizeof(irqs->icfgr);
+    irqs->icfgr[0] = readl_relaxed(base);
+    irqs->icfgr[1] = readl_relaxed(base + 4);
+
+    base = GICD + GET_SPI_REG_OFFSET(IPRIORITYR, is_espi);
+    base += i * sizeof(irqs->ipriorityr);
+    for ( irq = 0; irq < ARRAY_SIZE(irqs->ipriorityr); irq++ )
+        irqs->ipriorityr[irq] = readl_relaxed(base + 4 * irq);
+
+    base = GICD + GET_SPI_REG_OFFSET(IROUTER, is_espi);
+    base += i * sizeof(irqs->irouter);
+    for ( irq = 0; irq < ARRAY_SIZE(irqs->irouter); irq++ )
+        irqs->irouter[irq] = readq_relaxed_non_atomic(base + 8 * irq);
+
+    base = GICD + GET_SPI_REG_OFFSET(ISACTIVER, is_espi);
+    base += i * sizeof(irqs->isactiver);
+    irqs->isactiver = readl_relaxed(base);
+
+    base = GICD + GET_SPI_REG_OFFSET(ISENABLER, is_espi);
+    base += i * sizeof(irqs->isenabler);
+    irqs->isenabler = readl_relaxed(base);
+}
+
+static void gicv3_restore_spi_irq_block(struct dist_irq_block *irqs,
+                                        unsigned int i, bool is_espi)
+{
+    void __iomem *base;
+    unsigned int irq;
+
+    base = GICD + GET_SPI_REG_OFFSET(ICFGR, is_espi) + i * sizeof(irqs->icfgr);
+    writel_relaxed(irqs->icfgr[0], base);
+    writel_relaxed(irqs->icfgr[1], base + 4);
+
+    base = GICD + GET_SPI_REG_OFFSET(IPRIORITYR, is_espi);
+    base += i * sizeof(irqs->ipriorityr);
+    for ( irq = 0; irq < ARRAY_SIZE(irqs->ipriorityr); irq++ )
+        writel_relaxed(irqs->ipriorityr[irq], base + 4 * irq);
+
+    base = GICD + GET_SPI_REG_OFFSET(IROUTER, is_espi);
+    base += i * sizeof(irqs->irouter);
+    for ( irq = 0; irq < ARRAY_SIZE(irqs->irouter); irq++ )
+        writeq_relaxed_non_atomic(irqs->irouter[irq], base + 8 * irq);
+
+    base = GICD + GET_SPI_REG_OFFSET(ICENABLER, is_espi) + i * 4;
+    writel_relaxed(GENMASK(31, 0), base);
+
+    base = GICD + GET_SPI_REG_OFFSET(ISENABLER, is_espi);
+    base += i * sizeof(irqs->isenabler);
+    writel_relaxed(irqs->isenabler, base);
+
+    base = GICD + GET_SPI_REG_OFFSET(ICACTIVER, is_espi) + i * 4;
+    writel_relaxed(GENMASK(31, 0), base);
+
+    base = GICD + GET_SPI_REG_OFFSET(ISACTIVER, is_espi);
+    base += i * sizeof(irqs->isactiver);
+    writel_relaxed(irqs->isactiver, base);
+}
+
+static int gicv3_suspend(void)
+{
+    unsigned int i;
+    void __iomem *base;
+    int ret;
+    struct redist_ctx *rdist = &gicv3_ctx.rdist;
+
+    /* Save GICC configuration */
+    gicv3_ctx.cpu.ctlr     = READ_SYSREG(ICC_CTLR_EL1);
+    gicv3_ctx.cpu.pmr      = READ_SYSREG(ICC_PMR_EL1);
+    gicv3_ctx.cpu.bpr      = READ_SYSREG(ICC_BPR1_EL1);
+    gicv3_ctx.cpu.sre_el2  = READ_SYSREG(ICC_SRE_EL2);
+    gicv3_ctx.cpu.grpen    = READ_SYSREG(ICC_IGRPEN1_EL1);
+
+    gicv3_disable_interface();
+
+    ret = gicv3_disable_redist();
+    if ( ret )
+        return out_enable_iface;
+
+    /* Save GICR configuration */
+    gicv3_redist_wait_for_rwp();
+
+    base = GICD_RDIST_SGI_BASE;
+
+    rdist->ctlr = readl_relaxed(base + GICR_CTLR);
+
+    /* Save priority on PPI and SGI interrupts */
+    for ( i = 0; i < NR_GIC_LOCAL_IRQS / 4; i++ )
+        rdist->ipriorityr[i] = readl_relaxed(base + GICR_IPRIORITYR0 + 4 * i);
+
+    rdist->isactiver = readl_relaxed(base + GICR_ISACTIVER0);
+    rdist->isenabler = readl_relaxed(base + GICR_ISENABLER0);
+    rdist->igroupr   = readl_relaxed(base + GICR_IGROUPR0);
+    rdist->icfgr     = readl_relaxed(base + GICR_ICFGR1);
+
+    rdist->propbase = readq_relaxed(base + GICR_PROPBASER);
+    rdist->pendbase = readq_relaxed(base + GICR_PENDBASER);
+
+    /* Save GICD configuration */
+    gicv3_dist_wait_for_rwp();
+    gicv3_ctx.dist.ctlr = readl_relaxed(GICD + GICD_CTLR);
+
+    for ( i = 1; i < DIV_ROUND_UP(gicv3_info.nr_lines, 32); i++ )
+        gicv3_store_spi_irq_block(gicv3_ctx.dist.irqs + i - 1, i, false);
+
+#ifdef CONFIG_GICV3_ESPI
+    for ( i = 0; i < gic_number_espis() / 32; i++ )
+        gicv3_store_spi_irq_block(gicv3_ctx.dist.espi_irqs + i, i, true);
+#endif
+
+    return 0;
+
+ out_enable_iface:
+    gicv3_hyp_enable(true);
+    WRITE_SYSREG(gicv3_ctx.cpu.ctlr, ICC_CTLR_EL1);
+    isb();
+
+    return ret;
+}
+
+static void gicv3_resume(void)
+{
+    int ret;
+    unsigned int i;
+    void __iomem *base;
+    struct redist_ctx *rdist = &gicv3_ctx.rdist;
+
+    writel_relaxed(0, GICD + GICD_CTLR);
+
+    for ( i = NR_GIC_LOCAL_IRQS; i < gicv3_info.nr_lines; i += 32 )
+        writel_relaxed(GENMASK(31, 0), GICD + GICD_IGROUPR + (i / 32) * 4);
+
+    for ( i = 1; i < DIV_ROUND_UP(gicv3_info.nr_lines, 32); i++ )
+        gicv3_restore_spi_irq_block(gicv3_ctx.dist.irqs + i - 1, i, false);
+
+#ifdef CONFIG_GICV3_ESPI
+    for ( i = 0; i < gic_number_espis() / 32; i++ )
+        gicv3_restore_spi_irq_block(gicv3_ctx.dist.espi_irqs + i, i, true);
+#endif
+
+    writel_relaxed(gicv3_ctx.dist.ctlr, GICD + GICD_CTLR);
+    gicv3_dist_wait_for_rwp();
+
+    ret = gicv3_lpi_init_rdist(GICD_RDIST_BASE);
+    /*
+     * If LPIs are already enabled, assume firmware or the still-powered
+     * redistributor has valid PROPBASER/PENDBASER and skip reprogramming.
+     * Return -EBUSY so callers can ignore this case.
+     */
+    if ( ret && ret != -ENODEV && ret != -EBUSY )
+        panic("GICv3: Failed to re-initialize LPIs during resume\n");
+    else if ( ret == -EBUSY ) /* extra checks, just to be sure */
+    {
+        base = GICD_RDIST_BASE;
+        if ( readq_relaxed(base + GICR_PROPBASER) != rdist->propbase ||
+             readq_relaxed(base + GICR_PENDBASER) != rdist->pendbase )
+        {
+            panic("GICv3: LPIs already enabled with unexpected PROPBASER/PENDBASER during resume\n");
+        }
+    }
+
+    /* Restore GICR (Redistributor) configuration */
+    if ( gicv3_enable_redist() )
+        panic("GICv3: Failed to re-enable redistributor during resume\n");
+
+    base = GICD_RDIST_SGI_BASE;
+
+    writel_relaxed(GENMASK(31, 0), base + GICR_ICENABLER0);
+    gicv3_redist_wait_for_rwp();
+
+    for ( i = 0; i < NR_GIC_LOCAL_IRQS / 4; i++ )
+        writel_relaxed(rdist->ipriorityr[i], base + GICR_IPRIORITYR0 + i * 4);
+
+    writel_relaxed(rdist->isactiver, base + GICR_ISACTIVER0);
+    writel_relaxed(rdist->igroupr,   base + GICR_IGROUPR0);
+    writel_relaxed(rdist->icfgr,     base + GICR_ICFGR1);
+
+    gicv3_redist_wait_for_rwp();
+
+    writel_relaxed(rdist->isenabler, base + GICR_ISENABLER0);
+    writel_relaxed(rdist->ctlr, GICD_RDIST_BASE + GICR_CTLR);
+
+    gicv3_redist_wait_for_rwp();
+
+    WRITE_SYSREG(gicv3_ctx.cpu.sre_el2, ICC_SRE_EL2);
+    isb();
+
+    /* Restore CPU interface (System registers) */
+    WRITE_SYSREG(gicv3_ctx.cpu.pmr,   ICC_PMR_EL1);
+    WRITE_SYSREG(gicv3_ctx.cpu.bpr,   ICC_BPR1_EL1);
+    WRITE_SYSREG(gicv3_ctx.cpu.ctlr,  ICC_CTLR_EL1);
+    WRITE_SYSREG(gicv3_ctx.cpu.grpen, ICC_IGRPEN1_EL1);
+    isb();
+
+    gicv3_hyp_init();
+}
+
+#endif /* CONFIG_SYSTEM_SUSPEND */
+
 /* Set up the GIC */
 static int __init gicv3_init(void)
 {
@@ -1989,6 +2294,10 @@ static int __init gicv3_init(void)
 
     gicv3_hyp_init();
 
+#ifdef CONFIG_SYSTEM_SUSPEND
+    gicv3_alloc_context();
+#endif
+
 out:
     spin_unlock(&gicv3.lock);
 
@@ -2028,6 +2337,10 @@ static const struct gic_hw_operations gicv3_ops = {
 #endif
     .iomem_deny_access   = gicv3_iomem_deny_access,
     .do_LPI              = gicv3_do_LPI,
+#ifdef CONFIG_SYSTEM_SUSPEND
+    .suspend             = gicv3_suspend,
+    .resume              = gicv3_resume,
+#endif
 };
 
 static int __init gicv3_dt_preinit(struct dt_device_node *node, const void *data)
diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
index c373b94d19..992c8f9c2f 100644
--- a/xen/arch/arm/include/asm/gic_v3_defs.h
+++ b/xen/arch/arm/include/asm/gic_v3_defs.h
@@ -94,6 +94,7 @@
 #define GICD_TYPE_LPIS               (1U << 17)
 
 #define GICD_CTLR_RWP                (1UL << 31)
+#define GICD_CTLR_DS                 (1U << 6)
 #define GICD_CTLR_ARE_NS             (1U << 4)
 #define GICD_CTLR_ENABLE_G1A         (1U << 1)
 #define GICD_CTLR_ENABLE_G1          (1U << 0)
-- 
2.43.0

Hi Mykola,

On 11/12/2025 18:43, Mykola Kvach wrote:
> From: Mykola Kvach <mykola_kvach@epam.com>
> 
> 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@epam.com>
> ---
> 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?

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

Cheers,

-- 
Julien Grall

Hi Julien,

Thank you for the review.

On Fri, Dec 26, 2025 at 2:39 PM Julien Grall <julien@xen.org> wrote:
>
> Hi Mykola,
>
> On 11/12/2025 18:43, Mykola Kvach wrote:
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > 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@epam.com>
> > ---
> > 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
>

Hi,

On 26/12/2025 12:39, Julien Grall wrote:
> Hi Mykola,
> 
> On 11/12/2025 18:43, Mykola Kvach wrote:
>> From: Mykola Kvach <mykola_kvach@epam.com>
>>
>> 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@epam.com>
>> ---
>> 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?
> 
> If so, don't we already have a problem for CPU hotplug because the 
> percpu area will be freed but not the pending table?

Looking at [1], we don't seem to support CPU OFF when the GICv3 ITS is 
enabled. So this change could be delayed. But CC Mykyta for awareness.

Cheers,

[1] 
https://lore.kernel.org/48bafdb8e6269a3d958065c6a1062ce2736632a0.1762939773.git.mykyta_poturai@epam.com

> 
> Cheers,
> 

-- 
Julien Grall

On Wed, Dec 31, 2025 at 5:35 PM Julien Grall <julien@xen.org> wrote:
>
> Hi,
>
> On 26/12/2025 12:39, Julien Grall wrote:
> > Hi Mykola,
> >
> > On 11/12/2025 18:43, Mykola Kvach wrote:
> >> From: Mykola Kvach <mykola_kvach@epam.com>
> >>
> >> 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@epam.com>
> >> ---
> >> 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?
> >
> > If so, don't we already have a problem for CPU hotplug because the
> > percpu area will be freed but not the pending table?
>
> Looking at [1], we don't seem to support CPU OFF when the GICv3 ITS is
> enabled. So this change could be delayed. But CC Mykyta for awareness.

Probably you are right. Another option is to move this
`system_state == SYS_STATE_resume` handling out of this change and into
the follow-up commit where ITS suspend/resume support is introduced, so
the rationale and behavior stay co-located with the suspend
implementation.

If you think the better approach is to avoid these changes for now,
I’m fine with that as well and can drop them in the next revision of
the series (and reintroduce them together with the ITS suspend).

Thanks,
Mykola

>
> Cheers,
>
> [1]
> https://lore.kernel.org/48bafdb8e6269a3d958065c6a1062ce2736632a0.1762939773.git.mykyta_poturai@epam.com
>
> >
> > Cheers,
> >
>
> --
> Julien Grall
>