Re: [PATCH Dovetail 6.18 1/3] arm64: irq_pipeline: Fix mapping of SGI/LPI IPIs

[Philippe Gerum <rpm@xenomai.org>]

"Bezdeka, Florian" <florian.bezdeka@siemens.com> writes:

> On Tue, 2026-02-03 at 22:48 +0100, Florian Bezdeka wrote:
>> On Tue, 2026-02-03 at 18:22 +0100, Philippe Gerum wrote:
>> > Florian Bezdeka <florian.bezdeka@siemens.com> writes:
>> > 
>> > > On Tue, 2026-02-03 at 14:08 +0100, Philippe Gerum wrote:
>> > > > -static inline void map_oob_ipis(int cpu)
>> > > > +static inline void map_oob_ipis(int cpu, int offset)
>> > > >  {
>> > > >  	int ipi;
>> > > >  
>> > > >  	for (ipi = OOB_IPI_OFFSET; ipi < OOB_NR_IPI + OOB_IPI_OFFSET; ipi++)
>> > > > -		get_ipi_desc(cpu, ipi) = irq_to_desc(ipi_irq_base + ipi);
>> > > > +		get_ipi_desc(cpu, ipi) = irq_to_desc(ipi_irq_base + cpu * offset + ipi);
>> > > >  }
>> > > 
>> > > I'm still quite sure that the loop is wrong and should be 
>> > > 
>> > > +static inline void map_oob_ipis(int cpu, int offset)
>> > >  {
>> > >         int ipi;
>> > >  
>> > > -       for (ipi = OOB_IPI_OFFSET; ipi < OOB_NR_IPI + OOB_IPI_OFFSET; ipi++)
>> > > -               get_ipi_desc(cpu, ipi) = irq_to_desc(ipi_irq_base + ipi);
>> > > +       offset = ipi_irq_base + (cpu * offset);
>> > > +
>> > > +       for (ipi = OOB_IPI_OFFSET; ipi < nr_ipi; ipi++)
>> > > +               get_ipi_desc(cpu, ipi) = irq_to_desc(offset + ipi);
>> > >  }
>> > > 
>> > 
>> > For ipi=SGI-0-7, the upstream implementation of ipi_setup_lpi() does:
>> > 
>> >                 irq = ipi_irq_base + (cpu * nr_ipi) + ipi;
>> > 		get_ipi_desc(cpu, ipi) = irq_to_desc(irq);
>> > 
>> > In pipelined mode, we need to do that for ipi=SGI0 for in-band IPIs,
>> > then for ipi=SGI1-3 for oob ones (see below). All members of
>> > ipi_msg_type become _logical_ IPIs multiplexed over SGI0. That is the
>> > whole point of your fix regarding CPU_BACKTRACE and KGDB_ROUNDUP.
>> > 
>> > 
>> > > instead. (Note OOB_NR_IPI + OOB_IPI_OFFSET vs. nr_ipi)
>> > 
>> > The code above would map the original IPI range from SGI0-7, which we
>> > don't have to in pipelined mode, we only use SGI0-3 here.
>> > 
>> > > 
>> > > Example:
>> > > 
>> > > static void arm64_send_ipi(const cpumask_t *mask, unsigned int nr)
>> > > {
>> > > 	unsigned int cpu;
>> > > 
>> > > 	if (!percpu_ipi_descs)
>> > > 		__ipi_send_mask(get_ipi_desc(0, nr), mask);
>> > > 
>> > > How should that work for IPI_IRQ_WORK? IPI_IRQ_WORK is located behind
>> > > the IPIs used for OOB and get_ipi_desc(0, IPI_IRQ_WORK) would return
>> > > NULL as we skipped the mapping in map_oob_ipis().
>> > > 
>> > 
>> > Any IPI from IPI_RESCHEDULE to IPI_IRQ_WORK are multiplexed over SGI0 by
>> > smp_cross_call(). 
>> > 
>> 
>> Here we go. I missed that smp_cross_call() calls __smp_cross_call() with
>> ipinr set to 0, unconditionally, not forwarding ipinr. I simply
>> overlooked that. That allows us to only map the oob IPIs as the "wrong"
>> mapping in arm64_send_ipi() can not happen anymore. Sorry for the
>> confusion.
>
> That didn't last long. Two ways to fix the problem at the end:
>
>   - Only map OOB IPIs as suggested by Philippe.
>     Means we have to modify arm64_backtrace_ipi() and
>     kgdb_roundup_cpus() for the dovetailing case.
>     Calling arm64_send_ipi() with an ipi-nr != 0 is not allowed in the
>     dovetailing case for inband IPIs.
>
>   - Map all IRQs as suggested by me.
>
> Hopefully correct now...
>
> Which way is the preferred one?
>
>>

Excerpt from arch/arm/kernel/smp.c:

enum ipi_msg_type {
     ...
	/*
	 * SGI8-15 can be reserved by secure firmware, and thus may
	 * not be usable by the kernel. Please keep the above limited
	 * to at most 8 entries.
	 */
	MAX_IPI
};

This still holds true for ATF [1], so we only have 8 available SGIs
although GICv3+ do provide 16. Since we have 8 in-band IPI message types
defined by the arm64 port and we need 3 oob vectors more, we can't have
a 1:1 mapping between IPIs and SGIs.

So far we are using 4 SGIs as explained above (1 in-band + 3 oob), so we
have 4 spares ATM. The reason for moving an in-band IPI to its own SGI
using one of those spares would be to prioritize it over the bulk of
in-band messages. Therefore, the question would be: do we need such
prioritization?

[1] https://trustedfirmware-a.readthedocs.io/en/latest/porting-guide.html#function-bl31-platform-setup-mandatory

-- 
Philippe.