Introduce framework to turn an IPI as NMI using pseudo NMIs. In case a
particular platform doesn't support pseudo NMIs, then request IPI as a
regular IRQ.
The main motivation for this feature is to have an IPI that can be
leveraged to invoke NMI functions on other CPUs. And current prospective
users are NMI backtrace and KGDB CPUs round-up whose support is added
via future patches.
Signed-off-by: Sumit Garg <[email protected]>
---
arch/arm64/include/asm/nmi.h | 16 +++++++++
arch/arm64/kernel/Makefile | 2 +-
arch/arm64/kernel/ipi_nmi.c | 80 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/nmi.h
create mode 100644 arch/arm64/kernel/ipi_nmi.c
diff --git a/arch/arm64/include/asm/nmi.h b/arch/arm64/include/asm/nmi.h
new file mode 100644
index 0000000..3433c55
--- /dev/null
+++ b/arch/arm64/include/asm/nmi.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_NMI_H
+#define __ASM_NMI_H
+
+#ifndef __ASSEMBLER__
+
+#include <linux/cpumask.h>
+
+extern void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask);
+
+void set_smp_ipi_nmi(int ipi);
+void ipi_nmi_setup(int cpu);
+void ipi_nmi_teardown(int cpu);
+
+#endif /* !__ASSEMBLER__ */
+#endif
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index a561cbb..022c26b 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -19,7 +19,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
return_address.o cpuinfo.o cpu_errata.o \
cpufeature.o alternative.o cacheinfo.o \
smp.o smp_spin_table.o topology.o smccc-call.o \
- syscall.o
+ syscall.o ipi_nmi.o
targets += efi-entry.o
diff --git a/arch/arm64/kernel/ipi_nmi.c b/arch/arm64/kernel/ipi_nmi.c
new file mode 100644
index 0000000..355ef92
--- /dev/null
+++ b/arch/arm64/kernel/ipi_nmi.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * NMI support for IPIs
+ *
+ * Copyright (C) 2020 Linaro Limited
+ * Author: Sumit Garg <[email protected]>
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/smp.h>
+
+#include <asm/nmi.h>
+
+static struct irq_desc *ipi_desc __read_mostly;
+static int ipi_id __read_mostly;
+static bool is_nmi __read_mostly;
+
+void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask)
+{
+ if (WARN_ON_ONCE(!ipi_desc))
+ return;
+
+ __ipi_send_mask(ipi_desc, mask);
+}
+
+static irqreturn_t ipi_nmi_handler(int irq, void *data)
+{
+ /* nop, NMI handlers for special features can be added here. */
+
+ return IRQ_HANDLED;
+}
+
+void ipi_nmi_setup(int cpu)
+{
+ if (!ipi_desc)
+ return;
+
+ if (is_nmi) {
+ if (!prepare_percpu_nmi(ipi_id))
+ enable_percpu_nmi(ipi_id, 0);
+ } else {
+ enable_percpu_irq(ipi_id, 0);
+ }
+}
+
+void ipi_nmi_teardown(int cpu)
+{
+ if (!ipi_desc)
+ return;
+
+ if (is_nmi) {
+ disable_percpu_nmi(ipi_id);
+ teardown_percpu_nmi(ipi_id);
+ } else {
+ disable_percpu_irq(ipi_id);
+ }
+}
+
+void __init set_smp_ipi_nmi(int ipi)
+{
+ int err;
+
+ err = request_percpu_nmi(ipi, ipi_nmi_handler, "IPI", &cpu_number);
+ if (err) {
+ err = request_percpu_irq(ipi, ipi_nmi_handler, "IPI",
+ &cpu_number);
+ WARN_ON(err);
+ is_nmi = false;
+ } else {
+ is_nmi = true;
+ }
+
+ ipi_desc = irq_to_desc(ipi);
+ irq_set_status_flags(ipi, IRQ_HIDDEN);
+ ipi_id = ipi;
+
+ /* Setup the boot CPU immediately */
+ ipi_nmi_setup(smp_processor_id());
+}
--
2.7.4
On Sat, 10 Oct 2020 02:58:55 +0100,
Masayoshi Mizuma <[email protected]> wrote:
[...]
> > +void ipi_nmi_setup(int cpu)
> > +{
> > + if (!ipi_desc)
> > + return;
>
> ipi_nmi_setup() may be called twice for CPU0:
>
> set_smp_ipi_range => set_smp_ipi_nmi => ipi_nmi_setup
> => ipi_setup => ipi_nmi_setup
>
> Actually, I got the following error message via the second ipi_nmi_setup():
>
> GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation
> GICv3: Cannot set NMI property of enabled IRQ 8
> genirq: Failed to setup NMI delivery: irq 8
>
> Why don't we have a check to prevent that? Like as:
>
> if (cpumask_test_cpu(cpu, ipi_desc->percpu_enabled))
> return;
That's definitely the wrong thing to do. prepare_nmi_setup() shouldn't
be called twice, and papering over it isn't acceptable.
M.
--
Without deviation from the norm, progress is not possible.
On Sat, Oct 10, 2020 at 10:34:04AM +0100, Marc Zyngier wrote:
> On Sat, 10 Oct 2020 02:58:55 +0100,
> Masayoshi Mizuma <[email protected]> wrote:
>
> [...]
>
> > > +void ipi_nmi_setup(int cpu)
> > > +{
> > > + if (!ipi_desc)
> > > + return;
> >
> > ipi_nmi_setup() may be called twice for CPU0:
> >
> > set_smp_ipi_range => set_smp_ipi_nmi => ipi_nmi_setup
> > => ipi_setup => ipi_nmi_setup
> >
> > Actually, I got the following error message via the second ipi_nmi_setup():
> >
> > GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation
> > GICv3: Cannot set NMI property of enabled IRQ 8
> > genirq: Failed to setup NMI delivery: irq 8
> >
> > Why don't we have a check to prevent that? Like as:
> >
> > if (cpumask_test_cpu(cpu, ipi_desc->percpu_enabled))
> > return;
>
> That's definitely the wrong thing to do. prepare_nmi_setup() shouldn't
> be called twice, and papering over it isn't acceptable.
Got it. How about moving ipi_nmi_setup() from ipi_setup() to
secondary_start_kernel() ? so that CPU0 can call ipi_nmi_setup() only
from set_smp_ipi_nmi().
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -245,6 +245,7 @@ asmlinkage notrace void secondary_start_kernel(void)
notify_cpu_starting(cpu);
ipi_setup(cpu);
+ ipi_nmi_setup(cpu);
store_cpu_topology(cpu);
numa_add_cpu(cpu);
@@ -966,8 +967,6 @@ static void ipi_setup(int cpu)
for (i = 0; i < nr_ipi; i++)
enable_percpu_irq(ipi_irq_base + i, 0);
-
- ipi_nmi_setup(cpu);
}
#ifdef CONFIG_HOTPLUG_CPU
Thanks,
Masa
Hi Sumit,
On Fri, Sep 11, 2020 at 06:58:40PM +0530, Sumit Garg wrote:
> Introduce framework to turn an IPI as NMI using pseudo NMIs. In case a
> particular platform doesn't support pseudo NMIs, then request IPI as a
> regular IRQ.
>
> The main motivation for this feature is to have an IPI that can be
> leveraged to invoke NMI functions on other CPUs. And current prospective
> users are NMI backtrace and KGDB CPUs round-up whose support is added
> via future patches.
>
> Signed-off-by: Sumit Garg <[email protected]>
> ---
> arch/arm64/include/asm/nmi.h | 16 +++++++++
> arch/arm64/kernel/Makefile | 2 +-
> arch/arm64/kernel/ipi_nmi.c | 80 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 97 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/nmi.h
> create mode 100644 arch/arm64/kernel/ipi_nmi.c
>
> diff --git a/arch/arm64/include/asm/nmi.h b/arch/arm64/include/asm/nmi.h
> new file mode 100644
> index 0000000..3433c55
> --- /dev/null
> +++ b/arch/arm64/include/asm/nmi.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_NMI_H
> +#define __ASM_NMI_H
> +
> +#ifndef __ASSEMBLER__
> +
> +#include <linux/cpumask.h>
> +
> +extern void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask);
> +
> +void set_smp_ipi_nmi(int ipi);
> +void ipi_nmi_setup(int cpu);
> +void ipi_nmi_teardown(int cpu);
> +
> +#endif /* !__ASSEMBLER__ */
> +#endif
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index a561cbb..022c26b 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -19,7 +19,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
> return_address.o cpuinfo.o cpu_errata.o \
> cpufeature.o alternative.o cacheinfo.o \
> smp.o smp_spin_table.o topology.o smccc-call.o \
> - syscall.o
> + syscall.o ipi_nmi.o
>
> targets += efi-entry.o
>
> diff --git a/arch/arm64/kernel/ipi_nmi.c b/arch/arm64/kernel/ipi_nmi.c
> new file mode 100644
> index 0000000..355ef92
> --- /dev/null
> +++ b/arch/arm64/kernel/ipi_nmi.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * NMI support for IPIs
> + *
> + * Copyright (C) 2020 Linaro Limited
> + * Author: Sumit Garg <[email protected]>
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/smp.h>
> +
> +#include <asm/nmi.h>
> +
> +static struct irq_desc *ipi_desc __read_mostly;
> +static int ipi_id __read_mostly;
> +static bool is_nmi __read_mostly;
> +
> +void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask)
> +{
> + if (WARN_ON_ONCE(!ipi_desc))
> + return;
> +
> + __ipi_send_mask(ipi_desc, mask);
> +}
> +
> +static irqreturn_t ipi_nmi_handler(int irq, void *data)
> +{
> + /* nop, NMI handlers for special features can be added here. */
> +
> + return IRQ_HANDLED;
> +}
> +
> +void ipi_nmi_setup(int cpu)
> +{
> + if (!ipi_desc)
> + return;
ipi_nmi_setup() may be called twice for CPU0:
set_smp_ipi_range => set_smp_ipi_nmi => ipi_nmi_setup
=> ipi_setup => ipi_nmi_setup
Actually, I got the following error message via the second ipi_nmi_setup():
GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation
GICv3: Cannot set NMI property of enabled IRQ 8
genirq: Failed to setup NMI delivery: irq 8
Why don't we have a check to prevent that? Like as:
if (cpumask_test_cpu(cpu, ipi_desc->percpu_enabled))
return;
> +
> + if (is_nmi) {
> + if (!prepare_percpu_nmi(ipi_id))
> + enable_percpu_nmi(ipi_id, 0);
It would be better to use IRQ_TYPE_NONE instead of 0.
enable_percpu_nmi(ipi_id, IRQ_TYPE_NONE);
> + } else {
> + enable_percpu_irq(ipi_id, 0);
Same as here:
enable_percpu_irq(ipi_id, IRQ_TYPE_NONE);
Thanks,
Masa
> + }
> +}
> +
> +void ipi_nmi_teardown(int cpu)
> +{
> + if (!ipi_desc)
> + return;
> +
> + if (is_nmi) {
> + disable_percpu_nmi(ipi_id);
> + teardown_percpu_nmi(ipi_id);
> + } else {
> + disable_percpu_irq(ipi_id);
> + }
> +}
> +
> +void __init set_smp_ipi_nmi(int ipi)
> +{
> + int err;
> +
> + err = request_percpu_nmi(ipi, ipi_nmi_handler, "IPI", &cpu_number);
> + if (err) {
> + err = request_percpu_irq(ipi, ipi_nmi_handler, "IPI",
> + &cpu_number);
> + WARN_ON(err);
> + is_nmi = false;
> + } else {
> + is_nmi = true;
> + }
> +
> + ipi_desc = irq_to_desc(ipi);
> + irq_set_status_flags(ipi, IRQ_HIDDEN);
> + ipi_id = ipi;
> +
> + /* Setup the boot CPU immediately */
> + ipi_nmi_setup(smp_processor_id());
> +}
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Masa,
On Sat, 10 Oct 2020 at 20:43, Masayoshi Mizuma <[email protected]> wrote:
>
> On Sat, Oct 10, 2020 at 10:34:04AM +0100, Marc Zyngier wrote:
> > On Sat, 10 Oct 2020 02:58:55 +0100,
> > Masayoshi Mizuma <[email protected]> wrote:
> >
> > [...]
> >
> > > > +void ipi_nmi_setup(int cpu)
> > > > +{
> > > > + if (!ipi_desc)
> > > > + return;
> > >
> > > ipi_nmi_setup() may be called twice for CPU0:
> > >
> > > set_smp_ipi_range => set_smp_ipi_nmi => ipi_nmi_setup
> > > => ipi_setup => ipi_nmi_setup
> > >
> > > Actually, I got the following error message via the second ipi_nmi_setup():
> > >
> > > GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation
> > > GICv3: Cannot set NMI property of enabled IRQ 8
> > > genirq: Failed to setup NMI delivery: irq 8
> > >
Ah, thanks for catching this which I missed during my testing.
> > > Why don't we have a check to prevent that? Like as:
> > >
> > > if (cpumask_test_cpu(cpu, ipi_desc->percpu_enabled))
> > > return;
> >
> > That's definitely the wrong thing to do. prepare_nmi_setup() shouldn't
> > be called twice, and papering over it isn't acceptable.
>
> Got it. How about moving ipi_nmi_setup() from ipi_setup() to
> secondary_start_kernel() ? so that CPU0 can call ipi_nmi_setup() only
> from set_smp_ipi_nmi().
>
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -245,6 +245,7 @@ asmlinkage notrace void secondary_start_kernel(void)
> notify_cpu_starting(cpu);
>
> ipi_setup(cpu);
> + ipi_nmi_setup(cpu);
>
> store_cpu_topology(cpu);
> numa_add_cpu(cpu);
> @@ -966,8 +967,6 @@ static void ipi_setup(int cpu)
>
> for (i = 0; i < nr_ipi; i++)
> enable_percpu_irq(ipi_irq_base + i, 0);
> -
> - ipi_nmi_setup(cpu);
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
>
IMO, it would be more consistent to keep invocation of ipi_nmi_setup()
from ipi_setup(). So let me remove other invocation from
set_smp_ipi_nmi():
diff --git a/arch/arm64/kernel/ipi_nmi.c b/arch/arm64/kernel/ipi_nmi.c
index d3aa430..000e457 100644
--- a/arch/arm64/kernel/ipi_nmi.c
+++ b/arch/arm64/kernel/ipi_nmi.c
@@ -87,7 +87,4 @@ void __init set_smp_ipi_nmi(int ipi)
ipi_desc = irq_to_desc(ipi);
irq_set_status_flags(ipi, IRQ_HIDDEN);
ipi_id = ipi;
-
- /* Setup the boot CPU immediately */
- ipi_nmi_setup(smp_processor_id());
}
Do let me know if this works for you?
-Sumit
> Thanks,
> Masa
On Sat, 10 Oct 2020 at 07:28, Masayoshi Mizuma <[email protected]> wrote:
>
> Hi Sumit,
>
> On Fri, Sep 11, 2020 at 06:58:40PM +0530, Sumit Garg wrote:
> > Introduce framework to turn an IPI as NMI using pseudo NMIs. In case a
> > particular platform doesn't support pseudo NMIs, then request IPI as a
> > regular IRQ.
> >
> > The main motivation for this feature is to have an IPI that can be
> > leveraged to invoke NMI functions on other CPUs. And current prospective
> > users are NMI backtrace and KGDB CPUs round-up whose support is added
> > via future patches.
> >
> > Signed-off-by: Sumit Garg <[email protected]>
> > ---
> > arch/arm64/include/asm/nmi.h | 16 +++++++++
> > arch/arm64/kernel/Makefile | 2 +-
> > arch/arm64/kernel/ipi_nmi.c | 80 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 97 insertions(+), 1 deletion(-)
> > create mode 100644 arch/arm64/include/asm/nmi.h
> > create mode 100644 arch/arm64/kernel/ipi_nmi.c
> >
> > diff --git a/arch/arm64/include/asm/nmi.h b/arch/arm64/include/asm/nmi.h
> > new file mode 100644
> > index 0000000..3433c55
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/nmi.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __ASM_NMI_H
> > +#define __ASM_NMI_H
> > +
> > +#ifndef __ASSEMBLER__
> > +
> > +#include <linux/cpumask.h>
> > +
> > +extern void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask);
> > +
> > +void set_smp_ipi_nmi(int ipi);
> > +void ipi_nmi_setup(int cpu);
> > +void ipi_nmi_teardown(int cpu);
> > +
> > +#endif /* !__ASSEMBLER__ */
> > +#endif
> > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> > index a561cbb..022c26b 100644
> > --- a/arch/arm64/kernel/Makefile
> > +++ b/arch/arm64/kernel/Makefile
> > @@ -19,7 +19,7 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
> > return_address.o cpuinfo.o cpu_errata.o \
> > cpufeature.o alternative.o cacheinfo.o \
> > smp.o smp_spin_table.o topology.o smccc-call.o \
> > - syscall.o
> > + syscall.o ipi_nmi.o
> >
> > targets += efi-entry.o
> >
> > diff --git a/arch/arm64/kernel/ipi_nmi.c b/arch/arm64/kernel/ipi_nmi.c
> > new file mode 100644
> > index 0000000..355ef92
> > --- /dev/null
> > +++ b/arch/arm64/kernel/ipi_nmi.c
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * NMI support for IPIs
> > + *
> > + * Copyright (C) 2020 Linaro Limited
> > + * Author: Sumit Garg <[email protected]>
> > + */
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/irq.h>
> > +#include <linux/smp.h>
> > +
> > +#include <asm/nmi.h>
> > +
> > +static struct irq_desc *ipi_desc __read_mostly;
> > +static int ipi_id __read_mostly;
> > +static bool is_nmi __read_mostly;
> > +
> > +void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask)
> > +{
> > + if (WARN_ON_ONCE(!ipi_desc))
> > + return;
> > +
> > + __ipi_send_mask(ipi_desc, mask);
> > +}
> > +
> > +static irqreturn_t ipi_nmi_handler(int irq, void *data)
> > +{
> > + /* nop, NMI handlers for special features can be added here. */
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +void ipi_nmi_setup(int cpu)
> > +{
> > + if (!ipi_desc)
> > + return;
>
> ipi_nmi_setup() may be called twice for CPU0:
>
> set_smp_ipi_range => set_smp_ipi_nmi => ipi_nmi_setup
> => ipi_setup => ipi_nmi_setup
>
> Actually, I got the following error message via the second ipi_nmi_setup():
>
> GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation
> GICv3: Cannot set NMI property of enabled IRQ 8
> genirq: Failed to setup NMI delivery: irq 8
>
> Why don't we have a check to prevent that? Like as:
>
> if (cpumask_test_cpu(cpu, ipi_desc->percpu_enabled))
> return;
>
See my reply in the other thread.
> > +
> > + if (is_nmi) {
> > + if (!prepare_percpu_nmi(ipi_id))
> > + enable_percpu_nmi(ipi_id, 0);
>
> It would be better to use IRQ_TYPE_NONE instead of 0.
>
> enable_percpu_nmi(ipi_id, IRQ_TYPE_NONE);
>
Ack.
> > + } else {
> > + enable_percpu_irq(ipi_id, 0);
>
> Same as here:
> enable_percpu_irq(ipi_id, IRQ_TYPE_NONE);
>
Ack.
-Sumit
> Thanks,
> Masa
>
> > + }
> > +}
> > +
> > +void ipi_nmi_teardown(int cpu)
> > +{
> > + if (!ipi_desc)
> > + return;
> > +
> > + if (is_nmi) {
> > + disable_percpu_nmi(ipi_id);
> > + teardown_percpu_nmi(ipi_id);
> > + } else {
> > + disable_percpu_irq(ipi_id);
> > + }
> > +}
> > +
> > +void __init set_smp_ipi_nmi(int ipi)
> > +{
> > + int err;
> > +
> > + err = request_percpu_nmi(ipi, ipi_nmi_handler, "IPI", &cpu_number);
> > + if (err) {
> > + err = request_percpu_irq(ipi, ipi_nmi_handler, "IPI",
> > + &cpu_number);
> > + WARN_ON(err);
> > + is_nmi = false;
> > + } else {
> > + is_nmi = true;
> > + }
> > +
> > + ipi_desc = irq_to_desc(ipi);
> > + irq_set_status_flags(ipi, IRQ_HIDDEN);
> > + ipi_id = ipi;
> > +
> > + /* Setup the boot CPU immediately */
> > + ipi_nmi_setup(smp_processor_id());
> > +}
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Oct 12, 2020 at 05:49:09PM +0530, Sumit Garg wrote:
> Hi Masa,
>
> On Sat, 10 Oct 2020 at 20:43, Masayoshi Mizuma <[email protected]> wrote:
> >
> > On Sat, Oct 10, 2020 at 10:34:04AM +0100, Marc Zyngier wrote:
> > > On Sat, 10 Oct 2020 02:58:55 +0100,
> > > Masayoshi Mizuma <[email protected]> wrote:
> > >
> > > [...]
> > >
> > > > > +void ipi_nmi_setup(int cpu)
> > > > > +{
> > > > > + if (!ipi_desc)
> > > > > + return;
> > > >
> > > > ipi_nmi_setup() may be called twice for CPU0:
> > > >
> > > > set_smp_ipi_range => set_smp_ipi_nmi => ipi_nmi_setup
> > > > => ipi_setup => ipi_nmi_setup
> > > >
> > > > Actually, I got the following error message via the second ipi_nmi_setup():
> > > >
> > > > GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation
> > > > GICv3: Cannot set NMI property of enabled IRQ 8
> > > > genirq: Failed to setup NMI delivery: irq 8
> > > >
>
> Ah, thanks for catching this which I missed during my testing.
>
> > > > Why don't we have a check to prevent that? Like as:
> > > >
> > > > if (cpumask_test_cpu(cpu, ipi_desc->percpu_enabled))
> > > > return;
> > >
> > > That's definitely the wrong thing to do. prepare_nmi_setup() shouldn't
> > > be called twice, and papering over it isn't acceptable.
> >
> > Got it. How about moving ipi_nmi_setup() from ipi_setup() to
> > secondary_start_kernel() ? so that CPU0 can call ipi_nmi_setup() only
> > from set_smp_ipi_nmi().
> >
> > --- a/arch/arm64/kernel/smp.c
> > +++ b/arch/arm64/kernel/smp.c
> > @@ -245,6 +245,7 @@ asmlinkage notrace void secondary_start_kernel(void)
> > notify_cpu_starting(cpu);
> >
> > ipi_setup(cpu);
> > + ipi_nmi_setup(cpu);
> >
> > store_cpu_topology(cpu);
> > numa_add_cpu(cpu);
> > @@ -966,8 +967,6 @@ static void ipi_setup(int cpu)
> >
> > for (i = 0; i < nr_ipi; i++)
> > enable_percpu_irq(ipi_irq_base + i, 0);
> > -
> > - ipi_nmi_setup(cpu);
> > }
> >
> > #ifdef CONFIG_HOTPLUG_CPU
> >
>
> IMO, it would be more consistent to keep invocation of ipi_nmi_setup()
> from ipi_setup(). So let me remove other invocation from
> set_smp_ipi_nmi():
>
> diff --git a/arch/arm64/kernel/ipi_nmi.c b/arch/arm64/kernel/ipi_nmi.c
> index d3aa430..000e457 100644
> --- a/arch/arm64/kernel/ipi_nmi.c
> +++ b/arch/arm64/kernel/ipi_nmi.c
> @@ -87,7 +87,4 @@ void __init set_smp_ipi_nmi(int ipi)
> ipi_desc = irq_to_desc(ipi);
> irq_set_status_flags(ipi, IRQ_HIDDEN);
> ipi_id = ipi;
> -
> - /* Setup the boot CPU immediately */
> - ipi_nmi_setup(smp_processor_id());
> }
>
> Do let me know if this works for you?
Yes, make sense to me.
Thanks!
Masa