Re: [PATCH Dovetail 6.18 3/3] arm64: irq_pipeline: Fix IPI_KGDB_ROUNDUP and IPI_CPU_BACKTRACE IPIs

[Philippe Gerum <rpm@xenomai.org>]

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

> Both IPIs (IPI_KGDB_ROUNDUP and IPI_CPU_BACKTRACE) are multiplexed in
> case the IRQ pipelining is enabled, so we have to set an IPI reason
> in both cases.
>
> Both cases were directly calling into the IRQ chip implementation and
> forgot to set the reason. IPIs were ignored on the receiving side at
> the end.
>
> As tracing should be bypassed for all IRQs behind NR_IPI we can not
> call smp_cross_call() - where setting the reason / ipi message took
> place.
>
> Setting the reason has been factored out into set_ipi_message() which
> can be called with and without IRQ pipelining.
>
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
>  arch/arm64/kernel/smp.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 716ba0b0c274b..f85392de0e6a6 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -913,6 +913,24 @@ static void __noreturn ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs
>  #endif
>  }
>  
> +#ifdef CONFIG_IRQ_PIPELINE
> +
> +DEFINE_PER_CPU(unsigned long, ipi_messages);
> +
> +static inline void set_ipi_message(const cpumask_t *target, unsigned int ipi)
> +{
> +	int cpu;
> +
> +	for_each_cpu(cpu, target)
> +		set_bit(ipi, &per_cpu(ipi_messages, cpu));
> +
> +	wmb();
> +}
> +#else
> +static inline void set_ipi_message(const cpumask_t *target, unsigned int ipi)
> +{ }
> +#endif
> +
>  static void arm64_send_ipi(const cpumask_t *mask, unsigned int nr)
>  {
>  	unsigned int cpu;
> @@ -926,6 +944,7 @@ static void arm64_send_ipi(const cpumask_t *mask, unsigned int nr)
>  
>  static void arm64_backtrace_ipi(cpumask_t *mask)
>  {
> +	set_ipi_message(mask, IPI_CPU_BACKTRACE);
>  	arm64_send_ipi(mask, IPI_CPU_BACKTRACE);
>  }
>  
> @@ -951,6 +970,7 @@ void kgdb_roundup_cpus(void)
>  		if (cpu == this_cpu)
>  			continue;
>  
> +		set_ipi_message(cpumask_of(cpu), IPI_KGDB_ROUNDUP);
>  		__ipi_send_single(get_ipi_desc(cpu, IPI_KGDB_ROUNDUP), cpu);
>  	}
>  }
> @@ -1047,8 +1067,6 @@ static void __smp_cross_call(const struct cpumask *target, unsigned int ipinr)
>  	arm64_send_ipi(target, ipinr);
>  }
>  
> -static DEFINE_PER_CPU(unsigned long, ipi_messages);
> -
>  static DEFINE_PER_CPU(unsigned int [MAX_IPI], ipi_counts);
>  
>  static irqreturn_t ipi_handler(int irq, void *data)
> @@ -1074,13 +1092,8 @@ static irqreturn_t ipi_handler(int irq, void *data)
>  
>  static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
>  {
> -	unsigned int cpu;
> -
>  	/* regular in-band IPI (multiplexed over SGI0). */
> -	for_each_cpu(cpu, target)
> -		set_bit(ipinr, &per_cpu(ipi_messages, cpu));
> -
> -	wmb();
> +	set_ipi_message(target, ipinr);
>  	__smp_cross_call(target, 0);
>  }

Ouch, yes. Ack.

-- 
Philippe.