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

Philippe Gerum
2026-02-04T18:04:58Z

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.