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:

> There were two problems with the mapping:
>   - We stopped mapping after OOB_NR_IPI (3) IPIs.
>     There are more IPIs after the last IPI reserved for OOB

Yes, but we don't need to care those, since the code should never refer
to IPIs past OOB_NR_IPI when pipelining is enabled. The last issue
regarding this would be the out-of-bound access to ipi_count[], which
your other fix addresses.

As I understand this, e.g. on a two-way system, depending on SGI/LPI
modes, we need to hook:

in SGI mode, e.g. with IRQ0-3 globally available for mapping the IPIs on
all CPUs:

CPU0: SGI0 -> IRQ0 (in-band), SGI1 -> IRQ1, SGI2 -> IRQ2, SGI3 -> IRQ3 (oob)
CPU1: SGI0 -> IRQ0 (in-band), SGI1 -> IRQ1, SGI2 -> IRQ2, SGI3 -> IRQ3 (oob)

in LPI mode, e.g. with IRQ0-3 and IRQ4-7 available from CPU0 and CPU1
resp. for mapping the IPIs:

CPU0: SGI0 -> IRQ0 (in-band), SGI1 -> IRQ1, SGI2 -> IRQ2, SGI3 -> IRQ3 (oob)
CPU1: SGI0 -> IRQ4 (in-band), SGI1 -> IRQ5, SGI2 -> IRQ6, SGI3 -> IRQ7 (oob)

The root of the issue is the backtrace code bypassing the logical IPI ->
SGI0 mapping in smp_cross_call(). In this case, requesting
BACKTRACE=IPI7 without remapping to SGI0 would break in LPI mode,
because get_ipi_desc() returns NULL for that one. So my understanding is
that we must ensure that arm64_send_ipi() for in-band IPIs never escapes
the IPI->SGI0 mapping, instead of installing IPI descriptors on the
whole range of IPIs. We must also fix the IRQ mapping in the LPI case,
which wrongly maps everything to the IRQ range used by CPU0, which makes
no sense, as you pointed out.

>
>   - In the LPI case everything was mapped to the IRQ desc of CPU 0,
>     which seems odd. Each CPU has its own LPI used for multiplexing
>     inband IPIs.
>
> The following splat was observed whent trying to handle a backtrace
> IPI. This is one of the IPIs located after the OOB IPIs.
>
> [  115.357495] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000048
> [  115.358088] Mem abort info:

A simpler fix for the mapping issue: we only need to factor in the base
offset for IPIs in LPI mode:

--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -1020,12 +1020,12 @@ static void do_handle_IPI(int ipinr)
 
 #ifdef CONFIG_IRQ_PIPELINE
 
-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);
 }
 
 static void ipi_setup_oob_sgi(void)
@@ -1033,7 +1033,7 @@ static void ipi_setup_oob_sgi(void)
 	int cpu;
 
 	for_each_possible_cpu(cpu)
-		map_oob_ipis(cpu);
+		map_oob_ipis(cpu, 0);
 }
 
 static void ipi_setup_oob_lpi(int ncpus)
@@ -1041,7 +1041,7 @@ static void ipi_setup_oob_lpi(int ncpus)
 	int cpu;
 
 	for (cpu = 0; cpu < ncpus; cpu++)
-		map_oob_ipis(cpu);
+		map_oob_ipis(cpu, nr_ipi);
 }
 
 static void __smp_cross_call(const struct cpumask *target, unsigned int ipinr)

-- 
Philippe.