Re: [PATCH v9 03/13] xen/arm: gic-v3: tolerate retained redistributor LPI state across CPU_OFF

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Mykola,

> On 12 May 2026, at 18:07, Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
> 
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> 
> PSCI does not guarantee that a GICv3 redistributor is powered down across
> CPU_OFF -> CPU_ON.
> 
> DEN0022F.b says CPU_OFF powers down the calling core (5.5) and CPU_ON
> brings the core back with a defined initial CPU state (5.6, 6.4).
> However, PSCI leaves interrupt migration and GIC re-initialization to the
> supervisory software/firmware stack: the caller must migrate interrupts
> away before CPU_OFF (5.5.2), and the execution context that is lost in a
> powerdown state must be saved and restored by software (6.8). PSCI also
> calls out GIC management explicitly in 6.8, including retargeting SPIs,
> preventing PPIs/SGIs from targeting a powered down CPU, and reinitializing
> the CPU interface after CPU_ON.
> 
> This matches the GIC architecture. IHI0069H.b Chapter 11.1 requires the PE
> and CPU interface to share a power domain, but explicitly allows the
> associated redistributor, distributor, and ITS to remain powered while the
> PE and CPU interface are off. All other GIC power-management behavior is
> IMPLEMENTATION DEFINED. DEN0050D Chapter 4.2, "Generic Interrupt
> Controller (GIC)", says the GICv3 redistributor may live either in the AP
> core power domain or in a relatively always-on parent domain. So after
> CPU_OFF -> CPU_ON a secondary CPU can legitimately come back to a live
> redistributor with GICR_CTLR.EnableLPIs still set.
> 
> Handle that case in the LPI setup path instead of assuming a fully reset
> redistributor.
> 
> The LPI path needs special care because the GIC spec makes redistributor
> LPI state sticky and partially implementation defined. IHI0069H.b 5.1.1
> and 5.1.2 say that changing GICR_PROPBASER or GICR_PENDBASER while
> GICR_CTLR.EnableLPIs == 1 is UNPREDICTABLE. After clearing EnableLPIs,
> software must wait for GICR_CTLR.RWP == 0 before touching the pending
> table. The architecture also permits implementations where, once
> EnableLPIs has been set, clearing it again is not guaranteed to work.
> Where an ITS is present, the spec strongly recommends moving LPIs to
> another redistributor before clearing EnableLPIs.
> 
> Because of that, treat a retained EnableLPIs state as valid when the
> redistributor still points at Xen's expected PROPBASER/PENDBASER tables.
> Only try to clear EnableLPIs when the retained configuration does not
> match Xen's state, and wait for RWP before reprogramming the tables.
> 
> This is also consistent with platform firmware reality: PSCI and the GIC
> architecture allow platform-specific redistributor power handling, and not
> all platform firmware implementations force a full redistributor power-off
> through implementation-defined controls during CPU_OFF. Xen therefore needs
> to tolerate retained redistributor state on secondary CPU bring-up.
> 
> Keep gicv3_populate_rdist() resident as well, because gicv3_cpu_init()
> reuses it on secondary CPU bring-up after init.
> 
> Tested using Xen's non-boot CPU disable/enable path on Arm
> FVP_Base_RevC-2xAEMvA, both with and without:
> -C gic_distributor.allow-LPIEN-clear=1
> -C gic_distributor.GICR-clear-enable-supported=1
> and on Orange Pi 5.
> 
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---

I understand you will send a separate patch to fix RWP.

Looks ok to me, I see you’ve touched some printk in gicv3_populate_rdist() which
were wrongly having %d for smp_processor_id(), sometimes maintainers are not
really ok with that when the changes are not related to the commit, but apart 
from that:

Reviewed-by: Luca Fancellu <luca.fancellu@xxxxxxx>

Cheers,
Luca