Re: [PATCH v12 04/13] xen/arm: gic-v3: Implement GICv3 suspend/resume functions

[Bertrand Marquis <Bertrand.Marquis@arm.com>]

Hi Mykola,

> On 25 Sep 2026, at 02:03, Mykola Kvach <xakep.amatop@gmail.com> wrote:
> 
> Hi Bertrand,
> 
> Thank you for the review.
> 
> On Wed, Sep 23, 2026 at 6:58 PM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
>> 
>> Hi Mykola,
>> 
>>> On 27 Aug 2026, at 16:31, Mykola Kvach <mykola_kvach@epam.com> wrote:
>>> 
>>> 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.
>>> 
>>> Before continuing suspend, also verify that the physical CPU interface
>>> has no Group 1 active-priority state left. Use ICC_CTLR_EL1.PRIbits to
>>> decide which ICC_AP1R<n>_EL1 registers are implemented, so Xen does not
>>> read an unimplemented AP1R register.
>>> 
>>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
>>> ---
>>> Changes in V10:
>>> - abort suspend when the physical Group 1 active-priority state is still
>>> present, deriving accessible ICC_AP1R<n>_EL1 registers from
>>> ICC_CTLR_EL1.PRIbits;
>>> - re-enable the redistributor before restoring CPU and virtual interface
>>> state on the suspend abort path;
>>> - panic if the redistributor cannot be re-enabled on the suspend abort path;
>>> - avoid saving/restoring reserved GICD_IPRIORITYR and GICD_IROUTER entries
>>> for a partially populated last SPI block;
>>> - disable Distributor group forwarding while preserving affinity routing
>>> state before restoring Distributor configuration;
>>> - disable SPI/eSPI forwarding and wait for RWP before restoring
>>> GICD_ICFGR<n>.Int_config.
>>> 
>>> Changes in V9:
>>> - fix the suspend-context comment typo and split dist_ctx declarations;
>>> - restore ICC_IGRPEN1_EL1 on the suspend error path;
>>> - re-initialize GICD_IGROUPRnE during resume;
>>> - restore GICD_IROUTER only after re-enabling ARE_NS during resume.
>>> 
>>> Changes in V8:
>>> - use right rdist base for prop/pend baser and ctrl
>>> 
>>> 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                    | 458 ++++++++++++++++++++++-
>>> xen/arch/arm/include/asm/arm64/sysregs.h |   5 +
>>> xen/arch/arm/include/asm/gic_v3_defs.h   |   3 +
>>> 4 files changed, 466 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
>>> index 847da26ff7..a63c8c4979 100644
>>> --- a/xen/arch/arm/gic-v3-lpi.c
>>> +++ b/xen/arch/arm/gic-v3-lpi.c
>>> @@ -467,6 +467,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 b16888ad84..038bf41142 100644
>>> --- a/xen/arch/arm/gic-v3.c
>>> +++ b/xen/arch/arm/gic-v3.c
>>> @@ -1078,12 +1078,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();
>>> }
>>> @@ -1190,7 +1190,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);
>>> }
>>> @@ -1926,6 +1926,450 @@ static bool gic_dist_supports_lpis(void)
>>>    return (readl_relaxed(GICD + GICD_TYPER) & GICD_TYPE_LPIS);
>>> }
>>> 
>>> +#ifdef CONFIG_SYSTEM_SUSPEND
>>> +
>>> +/* This struct represents a 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 */
>> 
>> Can you capitalize first comment letter ?
>> s/only/Only/
>> 
>>> +    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;
>>> +        struct dist_irq_block *espi_irqs;
>>> +    } dist;
>>> +
>>> +    /* have only one rdist structure for last running CPU during suspend */
>> 
>> Same here
>> s/have/Have/
>> 
>>> +    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, unsigned int nr_irqs,
>>> +                                      bool is_espi)
>>> +{
>>> +    void __iomem *base;
>>> +    unsigned int irq, nr_priority_regs;
>>> +
>>> +    ASSERT(nr_irqs && nr_irqs <= 32);
>>> +    nr_priority_regs = DIV_ROUND_UP(nr_irqs, 4);
>>> +
>>> +    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 < nr_priority_regs; 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 < nr_irqs; 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_config(struct dist_irq_block *irqs,
>>> +                                         unsigned int i, unsigned int nr_irqs,
>>> +                                         bool is_espi)
>>> +{
>>> +    void __iomem *base;
>>> +    unsigned int irq, nr_priority_regs;
>>> +
>>> +    ASSERT(nr_irqs && nr_irqs <= 32);
>>> +    nr_priority_regs = DIV_ROUND_UP(nr_irqs, 4);
>>> +
>>> +    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 < nr_priority_regs; irq++ )
>>> +        writel_relaxed(irqs->ipriorityr[irq], base + 4 * irq);
>>> +}
>>> +
>>> +static void gicv3_restore_spi_irq_routing(struct dist_irq_block *irqs,
>>> +                                          unsigned int i, unsigned int nr_irqs,
>>> +                                          bool is_espi)
>>> +{
>>> +    void __iomem *base;
>>> +    unsigned int irq;
>>> +
>>> +    ASSERT(nr_irqs && nr_irqs <= 32);
>>> +
>>> +    base = GICD + GET_SPI_REG_OFFSET(IROUTER, is_espi);
>>> +    base += i * sizeof(irqs->irouter);
>>> +    for ( irq = 0; irq < nr_irqs; irq++ )
>>> +        writeq_relaxed_non_atomic(irqs->irouter[irq], base + 8 * irq);
>>> +}
>>> +
>>> +static void gicv3_disable_spi_irq_block(unsigned int i, bool is_espi)
>>> +{
>>> +    void __iomem *base;
>>> +
>>> +    base = GICD + GET_SPI_REG_OFFSET(ICENABLER, is_espi) + i * 4;
>>> +    writel_relaxed(GENMASK(31, 0), base);
>>> +}
>>> +
>>> +static void gicv3_restore_spi_irq_state(struct dist_irq_block *irqs,
>>> +                                        unsigned int i, bool is_espi)
>>> +{
>>> +    void __iomem *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_check_ap1r(unsigned int n, register_t apr)
>>> +{
>>> +    if ( !apr )
>>> +        return 0;
>>> +
>>> +    printk(XENLOG_ERR "GICv3: suspend aborted: ICC_AP1R%u_EL1=%#"
>>> +           PRIregister"\n", n, apr);
>>> +
>>> +    return -EBUSY;
>>> +}
>>> +
>>> +static int gicv3_check_active_priorities(register_t ctlr)
>>> +{
>>> +    unsigned int pribits = MASK_EXTR(ctlr, ICC_CTLR_EL1_PRIBITS_MASK) + 1;
>>> +    int ret;
>>> +
>>> +    /*
>>> +     * Xen enables physical Group 1 interrupts through ICC_IGRPEN1_EL1,
>>> +     * so only the physical Group 1 active-priority registers are relevant
>>> +     * here. Use ICC_CTLR_EL1.PRIbits for the physical CPU interface, not
>>> +     * ICH_VTR_EL2, which describes the virtual interface. ICC_AP1R1_EL1 is
>>> +     * only implemented with at least 6 physical priority bits, and
>>> +     * ICC_AP1R2_EL1/ICC_AP1R3_EL1 with at least 7.
>>> +     */
>>> +    switch ( pribits )
>>> +    {
>>> +    case 8:
>>> +    case 7:
>>> +        ret = gicv3_check_ap1r(3, READ_SYSREG(ICC_AP1R3_EL1));
>>> +        if ( ret )
>>> +            return ret;
>>> +        ret = gicv3_check_ap1r(2, READ_SYSREG(ICC_AP1R2_EL1));
>>> +        if ( ret )
>>> +            return ret;
>>> +        /* Fall through */
>>> +    case 6:
>>> +        ret = gicv3_check_ap1r(1, READ_SYSREG(ICC_AP1R1_EL1));
>>> +        if ( ret )
>>> +            return ret;
>>> +        /* Fall through */
>>> +    default:
>>> +        return gicv3_check_ap1r(0, READ_SYSREG(ICC_AP1R0_EL1));
>>> +    }
>>> +}
>>> +
>>> +static int gicv3_suspend(void)
>>> +{
>>> +    unsigned int i, nr_irqs;
>>> +    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_check_active_priorities(gicv3_ctx.cpu.ctlr);
>>> +    if ( ret )
>>> +        goto out_enable_iface;
>>> +
>>> +    ret = gicv3_disable_redist();
>>> +    if ( ret )
>>> +        goto out_enable_iface;
>> 
>> I am wondering about the timeout case here.
>> 
>> gicv3_disable_redist() has set ProcessorSleep to 1, but returns while
>> ChildrenAsleep is still 0.
>> This new error path then calls gicv3_enable_redist(), which clears
>> ProcessorSleep.
>> 
>> Could that happen before ChildrenAsleep reaches 1?
>> The GIC specification says that transition is UNPREDICTABLE.
>> How should we handle the timeout?
> 
> Yes, ChildrenAsleep may still be 0 when the error path calls
> gicv3_enable_redist(). Clearing ProcessorSleep in that state is
> UNPREDICTABLE.
> 
> I propose calling panic() if waiting for ChildrenAsleep to become
> 1 times out, before entering the rollback path. We cannot safely
> restore the CPU interface without completing the redistributor
> sleep/wake sequence.

Yes it agree we should do that and have a proper log for this.

> 
>> 
>>> +
>>> +    /* Save GICR configuration */
>>> +    gicv3_redist_wait_for_rwp();
>>> +
>>> +    base = GICD_RDIST_BASE;
>>> +
>>> +    rdist->ctlr = readl_relaxed(base + GICR_CTLR);
>>> +
>>> +    rdist->propbase = readq_relaxed(base + GICR_PROPBASER);
>>> +    rdist->pendbase = readq_relaxed(base + GICR_PENDBASER);
>>> +
>>> +    base = GICD_RDIST_SGI_BASE;
>>> +
>>> +    /* 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);
>>> +
>>> +    /* 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++ )
>>> +    {
>>> +        nr_irqs = min(32U, gicv3_info.nr_lines - i * 32);
>>> +        gicv3_store_spi_irq_block(gicv3_ctx.dist.irqs + i - 1, i, nr_irqs,
>>> +                                  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, 32, true);
>>> +#endif
>>> +
>>> +    return 0;
>>> +
>>> + out_enable_iface:
> 
> I also noticed that this label should be immediately before
> gicv3_hyp_enable(true).

ack

> 
>>> +    if ( gicv3_enable_redist() )
>>> +        panic("GICv3: Failed to re-enable redistributor after suspend abort\n");
>>> +
>>> +    gicv3_hyp_enable(true);
>>> +    WRITE_SYSREG(gicv3_ctx.cpu.grpen, ICC_IGRPEN1_EL1);
>>> +    isb();
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static void gicv3_resume(void)
>>> +{
>>> +    int ret;
>>> +    unsigned int i, nr_irqs;
>>> +    uint32_t dist_ctlr;
>>> +    void __iomem *base;
>>> +    struct redist_ctx *rdist = &gicv3_ctx.rdist;
>>> +
>>> +    dist_ctlr = gicv3_ctx.dist.ctlr & GICD_CTLR_ARE_NS;
>>> +
>>> +    /* Disable group forwarding while preserving affinity routing state. */
>>> +    writel_relaxed(dist_ctlr, GICD + GICD_CTLR);
>>> +    gicv3_dist_wait_for_rwp();
>>> +
>>> +    /*
>>> +     * IHI0069H.b 12.9.9 says changing GICD_ICFGR<n>.Int_config
>>> +     * while the interrupt is individually enabled is UNPREDICTABLE.
>>> +     * Disable SPIs first; 4.7.1 defines GICD_ICENABLER<n>, n > 0,
>>> +     * as the per-SPI disable mechanism.
>>> +     */
>>> +    for ( i = 1; i < DIV_ROUND_UP(gicv3_info.nr_lines, 32); i++ )
>>> +        gicv3_disable_spi_irq_block(i, false);
>>> +
>>> +#ifdef CONFIG_GICV3_ESPI
>>> +    for ( i = 0; i < gic_number_espis() / 32; i++ )
>>> +        gicv3_disable_spi_irq_block(i, true);
>>> +#endif
>>> +
>>> +    gicv3_dist_wait_for_rwp();
>>> +
>>> +    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++ )
>>> +    {
>>> +        nr_irqs = min(32U, gicv3_info.nr_lines - i * 32);
>>> +        gicv3_restore_spi_irq_config(gicv3_ctx.dist.irqs + i - 1, i, nr_irqs,
>>> +                                     false);
>>> +    }
>>> +
>>> +#ifdef CONFIG_GICV3_ESPI
>>> +    for ( i = 0; i < gic_number_espis() / 32; i++ )
>>> +    {
>>> +        writel_relaxed(GENMASK(31, 0), GICD + GICD_IGROUPRnE + i * 4);
>>> +        gicv3_restore_spi_irq_config(gicv3_ctx.dist.espi_irqs + i, i, 32,
>>> +                                     true);
>>> +    }
>>> +#endif
>>> +
>>> +    if ( dist_ctlr )
>>> +    {
>>> +        for ( i = 1; i < DIV_ROUND_UP(gicv3_info.nr_lines, 32); i++ )
>>> +        {
>>> +            nr_irqs = min(32U, gicv3_info.nr_lines - i * 32);
>>> +            gicv3_restore_spi_irq_routing(gicv3_ctx.dist.irqs + i - 1, i,
>>> +                                          nr_irqs, false);
>>> +        }
>>> +
>>> +#ifdef CONFIG_GICV3_ESPI
>>> +        for ( i = 0; i < gic_number_espis() / 32; i++ )
>>> +            gicv3_restore_spi_irq_routing(gicv3_ctx.dist.espi_irqs + i, i,
>>> +                                          32, true);
>>> +#endif
>>> +    }
>>> +
>>> +    for ( i = 1; i < DIV_ROUND_UP(gicv3_info.nr_lines, 32); i++ )
>>> +        gicv3_restore_spi_irq_state(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_state(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 */
>> 
>> Comment first letter capitalize:
>> s/extra/Extra/
> 
> I will also fix all the capitalization issues you pointed out.

Ack

Cheers
Bertrand

> 
> Thanks,
> Mykola