On Wed, 18 Jun 2025 11:17:33 +0100,
Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx> wrote:
>
> From: Marc Zyngier <maz@xxxxxxxxxx>
>
> The arm64 arch has relied so far on GIC architectural software
> generated interrupt (SGIs) to handle IPIs. Those are per-cpu
> software generated interrupts.
>
> arm64 architecture code that allocates the IPIs virtual IRQs and
> IRQ descriptors was written accordingly.
>
> On GICv5 systems, IPIs are implemented using LPIs that are not
> per-cpu interrupts - they are just normal routable IRQs.
>
> Add arch code to set-up IPIs on systems where they are handled
> using normal routable IRQs.
>
> For those systems, force the IRQ affinity (and make it immutable)
> to the cpu a given IRQ was assigned to.
>
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> [timothy.hayes@xxxxxxx: fixed ipi/irq conversion, irq flags]
> Signed-off-by: Timothy Hayes <timothy.hayes@xxxxxxx>
> [lpieralisi: changed affinity set-up, log]
> Signed-off-by: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> ---
> arch/arm64/include/asm/smp.h | 7 ++-
> arch/arm64/kernel/smp.c | 142 ++++++++++++++++++++++++++++++++-----------
> 2 files changed, 114 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm64/include/asm/smp.h b/arch/arm64/include/asm/smp.h
> index 2510eec026f7..d6fd6efb66a6 100644
> --- a/arch/arm64/include/asm/smp.h
> +++ b/arch/arm64/include/asm/smp.h
> @@ -53,7 +53,12 @@ extern void smp_init_cpus(void);
> /*
> * Register IPI interrupts with the arch SMP code
> */
> -extern void set_smp_ipi_range(int ipi_base, int nr_ipi);
> +extern void set_smp_ipi_range_percpu(int ipi_base, int nr_ipi, int ncpus);
> +
> +static inline void set_smp_ipi_range(int ipi_base, int n)
> +{
> + set_smp_ipi_range_percpu(ipi_base, n, 0);
> +}
>
> /*
> * Called from the secondary holding pen, this is the secondary CPU entry point.
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 3b3f6b56e733..7fd6bec80750 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -83,7 +83,31 @@ enum ipi_msg_type {
>
> static int ipi_irq_base __ro_after_init;
> static int nr_ipi __ro_after_init = NR_IPI;
> -static struct irq_desc *ipi_desc[MAX_IPI] __ro_after_init;
> +
> +struct ipi_descs {
> + struct irq_desc *descs[MAX_IPI];
> +};
> +
> +static DEFINE_PER_CPU(struct ipi_descs, pcpu_ipi_desc);
I wish we would make this __ro_after_init, but it doesn't see to be
possible to do that. At least make it read_mostly, which may help a
bit.
> +
> +#define get_ipi_desc(__cpu, __ipi) (per_cpu_ptr(&pcpu_ipi_desc, __cpu)->descs[__ipi])
> +
> +static bool percpu_ipi_descs __ro_after_init;
> +
> +static int ipi_to_irq_percpu(int ipi, int cpu)
> +{
> + return ipi_irq_base + (cpu * nr_ipi) + ipi;
> +}
> +
> +static int ipi_to_irq(int ipi)
> +{
> + return ipi_to_irq_percpu(ipi, 0);
> +}
> +
> +static int irq_to_ipi(int irq)
> +{
> + return (irq - ipi_irq_base) % nr_ipi;
> +}
Most of these helpers are used only once, and they are so similar that
I get cross-eyed. Consider expanding them in their calling spot.
>
> static bool crash_stop;
>
> @@ -844,7 +868,7 @@ int arch_show_interrupts(struct seq_file *p, int prec)
> seq_printf(p, "%*s%u:%s", prec - 1, "IPI", i,
> prec >= 4 ? " " : "");
> for_each_online_cpu(cpu)
> - seq_printf(p, "%10u ", irq_desc_kstat_cpu(ipi_desc[i], cpu));
> + seq_printf(p, "%10u ", irq_desc_kstat_cpu(get_ipi_desc(cpu, i), cpu));
> seq_printf(p, " %s\n", ipi_types[i]);
> }
>
> @@ -919,7 +943,13 @@ static void __noreturn ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs
>
> static void arm64_backtrace_ipi(cpumask_t *mask)
> {
> - __ipi_send_mask(ipi_desc[IPI_CPU_BACKTRACE], mask);
> + unsigned int cpu;
> +
> + if (!percpu_ipi_descs)
> + __ipi_send_mask(get_ipi_desc(0, IPI_CPU_BACKTRACE), mask);
> + else
> + for_each_cpu(cpu, mask)
> + __ipi_send_single(get_ipi_desc(cpu, IPI_CPU_BACKTRACE), cpu);
> }
>
> void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu)
> @@ -944,7 +974,7 @@ void kgdb_roundup_cpus(void)
> if (cpu == this_cpu)
> continue;
>
> - __ipi_send_single(ipi_desc[IPI_KGDB_ROUNDUP], cpu);
> + __ipi_send_single(get_ipi_desc(cpu, IPI_KGDB_ROUNDUP), cpu);
> }
> }
> #endif
> @@ -1013,14 +1043,21 @@ static void do_handle_IPI(int ipinr)
>
> static irqreturn_t ipi_handler(int irq, void *data)
> {
> - do_handle_IPI(irq - ipi_irq_base);
> + do_handle_IPI(irq_to_ipi(irq));
> return IRQ_HANDLED;
> }
>
> static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
> {
> + unsigned int cpu;
> +
> trace_ipi_raise(target, ipi_types[ipinr]);
> - __ipi_send_mask(ipi_desc[ipinr], target);
> +
> + if (!percpu_ipi_descs)
> + __ipi_send_mask(get_ipi_desc(0, ipinr), target);
> + else
> + for_each_cpu(cpu, target)
> + __ipi_send_single(get_ipi_desc(cpu, ipinr), cpu);
Having a helper for this construct would definitely be a good thing:
@@ -924,15 +919,20 @@ static void __noreturn ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs
#endif
}
-static void arm64_backtrace_ipi(cpumask_t *mask)
+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, IPI_CPU_BACKTRACE), mask);
+ __ipi_send_mask(get_ipi_desc(0, nr), mask);
else
for_each_cpu(cpu, mask)
- __ipi_send_single(get_ipi_desc(cpu, IPI_CPU_BACKTRACE), cpu);
+ __ipi_send_single(get_ipi_desc(cpu, nr), cpu);
+}
+
+static void arm64_backtrace_ipi(cpumask_t *mask)
+{
+ arm64_send_ipi(mask, IPI_CPU_BACKTRACE);
}
and similarly for smp_cross_call().
> }
>
> static bool ipi_should_be_nmi(enum ipi_msg_type ipi)
> @@ -1046,11 +1083,15 @@ static void ipi_setup(int cpu)
> return;
>
> for (i = 0; i < nr_ipi; i++) {
> - if (ipi_should_be_nmi(i)) {
> - prepare_percpu_nmi(ipi_irq_base + i);
> - enable_percpu_nmi(ipi_irq_base + i, 0);
> + if (!percpu_ipi_descs) {
> + if (ipi_should_be_nmi(i)) {
> + prepare_percpu_nmi(ipi_irq_base + i);
> + enable_percpu_nmi(ipi_irq_base + i, 0);
> + } else {
> + enable_percpu_irq(ipi_irq_base + i, 0);
> + }
> } else {
> - enable_percpu_irq(ipi_irq_base + i, 0);
> + enable_irq(irq_desc_get_irq(get_ipi_desc(cpu, i)));
> }
> }
> }
> @@ -1064,44 +1105,77 @@ static void ipi_teardown(int cpu)
> return;
>
> for (i = 0; i < nr_ipi; i++) {
> - if (ipi_should_be_nmi(i)) {
> - disable_percpu_nmi(ipi_irq_base + i);
> - teardown_percpu_nmi(ipi_irq_base + i);
> + if (!percpu_ipi_descs) {
> + if (ipi_should_be_nmi(i)) {
> + disable_percpu_nmi(ipi_irq_base + i);
> + teardown_percpu_nmi(ipi_irq_base + i);
> + } else {
> + disable_percpu_irq(ipi_irq_base + i);
> + }
> } else {
> - disable_percpu_irq(ipi_irq_base + i);
> + disable_irq(irq_desc_get_irq(get_ipi_desc(cpu, i)));
> }
> }
> }
> #endif
>
> -void __init set_smp_ipi_range(int ipi_base, int n)
> +static void ipi_setup_ppi(int ipi)
This sets up SGIs, not PPIs. They are indeed Per Processor Interrupts,
but given that you use "lpi" for GICv5, consider naming it
consistently.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
