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

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

On 10/17 13:27, Marc Zyngier wrote:
> On Fri, 16 Oct 2026 11:00:48 +0100,
> Tomohiro Misono <misono.tomohiro@fujitsu.com> wrote:
> > 
> > From: Kohei Enju <enju.kohei@fujitsu.com>
> > 
> > On affected FUJITSU-MONAKA CPUs, the erratum may occur when an SGI is
> > generated by writing to ICC_SGI0R_EL1, ICC_SGI1R_EL1, or ICC_ASGI1R_EL1,
> > if the operation races with CPU interface processing triggered by the
> > arrival of a higher-priority interrupt, an update to a pending
> > interrupt, or a transition of the PE to the Sleep state. When the
> > erratum occurs, the SGI is lost and the system register write does not
> > complete, causing the issuing core to hang.
> > 
> > Guest accesses to these registers are not affected by this erratum, as
> > they are trapped to EL2 and handled without issuing the affected system
> > register write.
> > 
> > Work around the erratum by writing 1 to the bit corresponding to the
> > target SGI INTID in GICR_ISPENDR0 of each target PE's GIC Redistributor,
> > instead of writing to the affected system registers.
> > 
> > Accessing a target PE's GICR_ISPENDR0 requires its Redistributor base to
> > have been discovered. Affected platforms conform to SBBR, which requires
> > PSCI for secondary CPU boot. They therefore do not use the ACPI Parking
> > protocol, which sends an IPI before the secondary CPU has initialized
> > its Redistributor. With PSCI, a secondary CPU discovers its
> > Redistributor before becoming an IPI target.
> > 
> > Signed-off-by: Kohei Enju <enju.kohei@fujitsu.com>
> 
> You are posting a patch on behalf of someone else, you need to add
> your own SoB:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.3-rc7#n448

Thanks for pointing this out. I'll let Tomohiro know.

> 
> > ---
> > Changes in v2:
> > - Clarify the erratum description and document that virtual accesses are
> >   unaffected.
> > - Remove the gic_send_sgi_via_rdist() helper.
> > - Remove the write-count rationale.
> > - Use the CPU MIDR instead of the SoC ID for filtering.
> > 
> > Note that I have left the dsb(st) after the GICR_ISPENDR0 writes
> > unchanged, pending clarification of the required completion point. If
> > the writes must be observed by a specific component before
> > gic_ipi_send_mask() returns, I will use the appropriate mechanism to
> > provide that guarantee.
> 
> See below.
> 
> [...]
> 
> > @@ -1386,6 +1391,17 @@ static void gic_ipi_send_mask(struct irq_data *d, const struct cpumask *mask)
> >  	 */
> >  	dsb(ishst);
> >  
> > +	if (static_branch_unlikely(&gic_fujitsu_030003_erratum)) {
> > +		for_each_cpu(cpu, mask) {
> > +			void __iomem *base = gic_data_rdist_sgi_base_cpu(cpu);
> > +
> > +			writel_relaxed(BIT(d->hwirq), base + GICR_ISPENDR0);
> > +		}
> > +
> > +		dsb(st);
> 
> What is this DSB achieving? As far as I can tell, not much. The memory
> type is nGnRE, so DSB doesn't force writes to reach the end-point, and
> only guarantee ordering that system register SGI generation doesn't
> provide.

Yes, you're right.

> 
> So either
> 
> (a) there is a functional requirement on this HW to have a DSB, and
>     needs to be documented,
> 
> (b) or there is a need for end-point synchronisation, which needs to
>     be enforced by a read back (knowing that the sysreg doesn't
>     provide this guarantee), and this needs to be documented,
> 
> (c) or this is completely superfluous and can be dropped
> 
> Which case are we in?

I think this falls under (c). As far as I can tell, there is no
requirement for ipi_send_mask() to wait for the writes to be observed by
the hardware. I'm not aware of any hardware-specific requirements for
this DSB either. I'll drop the DSB and test the change on our hardware.

Thanks,
Kohei