Re: [PATCH 4/4] irqchip/gicv3: Add workaround for FUJITSU-MONAKA erratum E#030003

[Kohei Enju <enju.kohei@fujitsu.com>]

On 10/02 10:41, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] irqchip/gicv3: CONSTRAINED UNPREDICTABLE access to sleeping redistributor
> --
> 
> commit d6fd070c2b4a77ad65839152f8e6343482132aff
> Author: Kohei Enju <enju.kohei@fujitsu.com>
> 
> irqchip/gicv3: Add workaround for FUJITSU-MONAKA erratum E#030003
> 
> This commit adds a workaround for FUJITSU-MONAKA erratum E#030003, where
> SGIs generated by system register writes may be lost. The workaround changes
> the IPI delivery mechanism to write directly to the target CPU's
> GICR_ISPENDR0 register instead of using ICC_SGI1R_EL1.
> 
> [ ... ]
> 
> > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> > index 6e1fa5b247fc4..e73fea8a0e279 100644
> > --- a/drivers/irqchip/irq-gic-v3.c
> > +++ b/drivers/irqchip/irq-gic-v3.c
> [ ... ]
> > @@ -1373,6 +1378,13 @@ static void gic_send_sgi(u64 cluster_id, u16 tlist, unsigned int irq)
> >  	gic_write_sgi1r(val);
> >  }
> >  
> > +static void gic_send_sgi_via_rdist(int cpu, unsigned int irq)
> > +{
> > +	void __iomem *base = gic_data_rdist_sgi_base_cpu(cpu);
> > +
> > +	writel_relaxed(BIT(irq), base + GICR_ISPENDR0);
> 
> [Severity: High]
> Does this access handle the case where the target CPU is in deep idle
> (ProcessorSleep == 1)?
> 
> The GICv3 architecture mandates that accessing GICR_ registers (other than
> GICR_WAKER) while ProcessorSleep == 1 is constrained unpredictable.

I could not find such a restriction in the GICv3/v4 architecture
specification.

Section 11.1 states that, when GICR_WAKER.ProcessorSleep == 1 or
GICR_WAKER.ChildrenAsleep == 1 then a write to any GICC_*, GICV_*,
GICH_*, ICC_*, ICV_*, or ICH_* registers, other than those in the
following list, is unpredictable:
- ICC_SRE_EL1.
- ICC_SRE_EL2.
- ICC_SRE_EL3.

Therefore, this finding appears to be based on the incorrect assumption
that accessing GICR_* while ProcessorSleep == 1 is constrained
unpredictable. I believe this is a false positive.

> 
> When a CPU enters deep idle, gic_enable_redist(false) sets
> GICR_WAKER_ProcessorSleep. If another CPU sends an IPI to this idle CPU
> (e.g., a scheduler wake-up), it executes gic_send_sgi_via_rdist() which
> directly writes to the sleeping redistributor's MMIO register.
> 
> Bypassing ICC_SGI1R_EL1 and writing directly to GICR_ISPENDR0 could break
> standard IPI delivery to idle CPUs by either causing a synchronous external
> abort or dropping the IPI.
> 
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261002-monaka-fix-for-upstream-v1-0-4aec0b0cbe34@fujitsu.com?part=4