| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aCRy4K/jvLr95GOp@lpieralisi>
Date: Wed, 14 May 2025 12:39:28 +0200
From: Lorenzo Pieralisi <lpieralisi@...nel.org>
To: Marc Zyngier <maz@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>
Cc: Arnd Bergmann <arnd@...db.de>,
Sascha Bischoff <sascha.bischoff@....com>,
Timothy Hayes <timothy.hayes@....com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Mark Rutland <mark.rutland@....com>,
Jiri Slaby <jirislaby@...nel.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH v4 18/26] arm64: smp: Support non-SGIs for IPIs
On Tue, May 13, 2025 at 07:48:11PM +0200, Lorenzo Pieralisi wrote:
[...]
> /*
> * 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 3b3f6b56e733039cad7ff5b8995db16a68f3c762..3f3712e47c94c62836fb89cd4bfb3595fbb41557 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -83,7 +83,26 @@ 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);
> +
> +#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(int ipi, int cpu)
> +{
> + return ipi_irq_base + (cpu * nr_ipi) + ipi;
> +}
> +
> +static int irq_to_ipi(int irq)
> +{
> + return (irq - ipi_irq_base) % nr_ipi;
> +}
>
> static bool crash_stop;
>
> @@ -844,7 +863,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 +938,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 +969,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 +1038,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);
> }
>
> static bool ipi_should_be_nmi(enum ipi_msg_type ipi)
> @@ -1046,11 +1078,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 +1100,79 @@ 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)
> +{
> + int err, irq, cpu;
> +
> + irq = ipi_irq_base + ipi;
> +
> + if (ipi_should_be_nmi(irq)) {
> + err = request_percpu_nmi(irq, ipi_handler, "IPI", &irq_stat);
> + WARN(err, "Could not request IRQ %d as NMI, err=%d\n", irq, err);
> + } else {
> + err = request_percpu_irq(irq, ipi_handler, "IPI", &irq_stat);
> + WARN(err, "Could not request IRQ %d as IRQ, err=%d\n", irq, err);
> + }
> +
> + for_each_possible_cpu(cpu)
> + get_ipi_desc(cpu, ipi) = irq_to_desc(irq);
> +
> + irq_set_status_flags(irq, IRQ_HIDDEN);
> +}
> +
> +static void ipi_setup_lpi(int ipi, int ncpus)
> +{
> + for (int cpu = 0; cpu < ncpus; cpu++) {
> + int err, irq;
> +
> + irq = ipi_to_irq(ipi, cpu);
> +
> + err = irq_force_affinity(irq, cpumask_of(cpu));
> +
> + WARN(err, "Could not force affinity IRQ %d, err=%d\n", irq, err);
> +
> + err = request_irq(irq, ipi_handler, IRQF_NO_AUTOEN, "IPI",
> + &irq_stat);
Heads-up, kbuild bot (sparse) barfed (correctly) at this, because the
&irq_stat pointer does not match the request_irq() void *dev_id parameter
signature (it is void __percpu *).
Of course, the &irq_stat parameter is unused so this is harmless.
I would just pass NULL (because AFAICS irq_stat in the action handler is
unused), the question is why are we passing &irq_stat in
request_percpu_irq() if that's unused in ipi_handler() ?
Was it used before and we removed its usage ? Should we clean it up
for completeness ?
Thanks,
Lorenzo
> +
> + WARN(err, "Could not request IRQ %d, err=%d\n", irq, err);
> +
> + irq_set_status_flags(irq, (IRQ_HIDDEN | IRQ_NO_BALANCING_MASK));
> +
> + get_ipi_desc(cpu, ipi) = irq_to_desc(irq);
> + }
> +}
> +
> +void __init set_smp_ipi_range_percpu(int ipi_base, int n, int ncpus)
> {
> int i;
>
> WARN_ON(n < MAX_IPI);
> nr_ipi = min(n, MAX_IPI);
>
> - for (i = 0; i < nr_ipi; i++) {
> - int err;
> -
> - if (ipi_should_be_nmi(i)) {
> - err = request_percpu_nmi(ipi_base + i, ipi_handler,
> - "IPI", &irq_stat);
> - WARN(err, "Could not request IPI %d as NMI, err=%d\n",
> - i, err);
> - } else {
> - err = request_percpu_irq(ipi_base + i, ipi_handler,
> - "IPI", &irq_stat);
> - WARN(err, "Could not request IPI %d as IRQ, err=%d\n",
> - i, err);
> - }
> -
> - ipi_desc[i] = irq_to_desc(ipi_base + i);
> - irq_set_status_flags(ipi_base + i, IRQ_HIDDEN);
> - }
> -
> + percpu_ipi_descs = !!ncpus;
> ipi_irq_base = ipi_base;
>
> + for (i = 0; i < nr_ipi; i++) {
> + if (!percpu_ipi_descs)
> + ipi_setup_ppi(i);
> + else
> + ipi_setup_lpi(i, ncpus);
> + }
> +
> /* Setup the boot CPU immediately */
> ipi_setup(smp_processor_id());
> }
>
> --
> 2.48.0
>
Powered by blists - more mailing lists