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

[Philippe Gerum <rpm@xenomai.org>]

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

> On Wed, 2026-02-04 at 12:44 +0100, Philippe Gerum wrote:
>> "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
>> };
>
> That is now taken from arch/arm instead of arch/arm64. Is that also the
> case for arm64?
>


Yes, the issue is with the trusted firmware, which is the reference
implementation for armv7-A and armv8-A.

>> 
>> 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?
>
> I have no strong opinion on that and tried to restore what we had in the
> past. Looking at 6.12 at [2] now I had to realize that this was way more
> broken regarding IPI_BACKTRACE and IPI_KGDB_ROUNDUP than I had realized
> up to now.
>

I depends on what you mean by "more broken", triggering a CPU backtrace
does not crash the kernel on 6.12 (unlike 6.18) because we have at least
valid ipi_desc[] on the whole IPI range although no backtrace ever shows
up. :)

>> 
>> [1] https://trustedfirmware-a.readthedocs.io/en/latest/porting-guide.html#function-bl31-platform-setup-mandatory
>
> [2] https://gitlab.com/Xenomai/linux-dovetail/-/blob/v6.12.y-dovetail/arch/arm64/kernel/smp.c?ref_type=heads#L1190
>
> What about the following end-result:
>
> enum ipi_msg_type {
> 	IPI_RESCHEDULE,			// SGI0, used for inband multiplexing
> 					// RESCHDULE set as reason
>
> 	IPI_CALL_FUNC,			// SGI1 reserved for OOB
> 					// CALL_FUNC via SGI0 with reason set
>
> 	IPI_CPU_STOP,			// SGI2 reserved for OOB
> 					// CPU_STOP via SGI0 with reason set
>
> 	IPI_CPU_STOP_NMI,		// SGI3 reserved for OOB
> 					// STOP_NMI via SGI0 with reason set
>
>
> 					// SGI4-7 unused
>
> 	IPI_TIMER,			// via SGI0 with reason set
> 	IPI_IRQ_WORK,			// via SGI0 with reason set
> 	NR_IPI,
> 	/*
> 	 * Any enum >= NR_IPI and < MAX_IPI is special and not tracable
> 	 * with trace_ipi_*
> 	 */
> 	IPI_CPU_BACKTRACE = NR_IPI,	// via SGI0 with reason set
> 	IPI_KGDB_ROUNDUP,		// via SGI0 with reason set

Yep, this is what we should have if the current scheme is kept.

> 	MAX_IPI
> };
>
>
>
> Final questions: 
>
> Assuming SGI mode: Where are the request_percpu_irq() calls for all the
> OOB SGIs?

The real-time core takes care of this (e.g. v3 does this in
pipeline_request_resched_ipi(), pipeline_install_tick_proxy()).

> Or: How is something like irq_send_oob_ipi(2, mask) expected
> to get the right IRQ descriptor in arm64_send_ipi()? Currently the
> descriptor for SGI0 is returned.
>

No, SGI1-3 may be returned, never SGI0.

arch/arm64/include/asm/irq_pipeline.h:
#define OOB_IPI_OFFSET		1 /* SGI1 */
#define TIMER_OOB_IPI		(ipi_irq_base + OOB_IPI_OFFSET)
#define RESCHEDULE_OOB_IPI	(TIMER_OOB_IPI + 1)
#define CALL_FUNCTION_OOB_IPI	(RESCHEDULE_OOB_IPI + 1)

and:

unsigned int sgi = irq - ipi_irq_base;

The irq numbers passed to irq_send_oob_ipi() must be one of these IPI
values otherwise the debug assertion on entry would fail, so the mapping
is correct.

> Is that all just broken for a long time?

No, that part is ok. Only CPU backtracing and KGDB were broken due to
bypassing the mapping to SGI0, due to a merge fallout after upstream
reorganized their own mapping code. Otherwise no tick, no rescheduling
would have been received ever by Xenomai. You would have noticed :)

>
> In addition I think that the trace_ipi_raise() in __smp_cross_call() is
> incorrect for OOB SGIs...
>

Yes, would not break anything, but wrong IPI label in the traces for sure.

-- 
Philippe.