linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/5] arm64: Add framework to turn an IPI as NMI
@ 2020-09-11 13:28 Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 1/5] arm64: Add framework to turn " Sumit Garg
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Sumit Garg @ 2020-09-11 13:28 UTC (permalink / raw)
  To: maz, catalin.marinas, will
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason, kgdb-bugreport,
	dianders, linux-kernel, linux-arm-kernel, jason.wessel, tglx,
	julien.thierry.kdev

With pseudo NMIs support available its possible to configure SGIs to be
triggered as pseudo NMIs running in NMI context. And kernel features
such as:
- NMI backtrace can leverage IPI turned as NMI to get a backtrace of CPU
  stuck in hard lockup using magic SYSRQ.
- kgdb relies on NMI support to round up CPUs which are stuck in hard
  lockup state with interrupts disabled.

This patch-set adds framework to turn an IPI as NMI which can be triggered
as a pseudo NMI which in turn invokes registered NMI handlers.

After this patch-set we should be able to get a backtrace for a CPU
stuck in HARDLOCKUP. Have a look at an examples below from a hard lockup
testcase run on Developerbox:

$ echo HARDLOCKUP > /sys/kernel/debug/provoke-crash/DIRECT

NMI backtrace:
==============

# Issue Magic SysRq to dump backtrace

[  376.894502] NMI backtrace for cpu 8
[  376.894506] CPU: 8 PID: 555 Comm: bash Not tainted 5.9.0-rc3-00740-g06ff047-dirty #242
[  376.894510] Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build #73 Apr  6 2020
[  376.894514] pstate: 40000005 (nZcv daif -PAN -UAO BTYPE=--)
[  376.894517] pc : lkdtm_HARDLOCKUP+0x8/0x18
[  376.894520] lr : lkdtm_do_action+0x24/0x30
[  376.894524] sp : ffff800012cebd20
[  376.894527] pmr_save: 00000060
[  376.894530] x29: ffff800012cebd20 x28: ffff000875ae8000 
[  376.894540] x27: 0000000000000000 x26: 0000000000000000 
[  376.894550] x25: 000000000000001a x24: ffff800012cebe40 
[  376.894560] x23: 000000000000000b x22: ffff800010fc5040 
[  376.894569] x21: ffff000878b61000 x20: ffff8000113b2870 
[  376.894579] x19: 000000000000001b x18: 0000000000000010 
[  376.894588] x17: 0000000000000000 x16: 0000000000000000 
[  376.894598] x15: ffff000875ae8470 x14: 00000000000002ad 
[  376.894613] x13: 0000000000000000 x12: 0000000000000000 
[  376.894622] x11: 0000000000000007 x10: 00000000000009c0 
[  376.894631] x9 : ffff800012ceba80 x8 : ffff000875ae8a20 
[  376.894641] x7 : ffff00087f6b3280 x6 : ffff00087f6b3200 
[  376.894651] x5 : 0000000000000000 x4 : ffff00087f6a91f8 
[  376.894660] x3 : ffff00087f6b0120 x2 : 1aa310cec69eb500 
[  376.894670] x1 : 0000000000000000 x0 : 0000000000000060 
[  376.894679] Call trace:
[  376.894683]  lkdtm_HARDLOCKUP+0x8/0x18
[  376.894686]  direct_entry+0x124/0x1c0
[  376.894689]  full_proxy_write+0x60/0xb0
[  376.894693]  vfs_write+0xf0/0x230
[  376.894696]  ksys_write+0x6c/0xf8
[  376.894699]  __arm64_sys_write+0x1c/0x28
[  376.894703]  el0_svc_common.constprop.0+0x74/0x1f0
[  376.894707]  do_el0_svc+0x24/0x90
[  376.894710]  el0_sync_handler+0x180/0x2f8
[  376.894713]  el0_sync+0x158/0x180

KGDB:
=====

# Enter kdb via Magic SysRq

[6]kdb> btc
btc: cpu status: Currently on cpu 6
Available cpus: 0-5(I), 6, 7(I), 8, 9-23(I)
<snip>
Stack traceback for pid 555
0xffff000875ae8000      555      554  1    8   R  0xffff000875ae89c0  bash
CPU: 8 PID: 555 Comm: bash Not tainted 5.9.0-rc3-00740-g06ff047-dirty #242
Hardware name: Socionext SynQuacer E-series DeveloperBox, BIOS build #73 Apr  6 2020
Call trace:
 dump_backtrace+0x0/0x1a0
 show_stack+0x18/0x28
 dump_stack+0xc0/0x11c
 kgdb_cpu_enter+0x648/0x660
 kgdb_nmicallback+0xa0/0xa8
 ipi_kgdb_nmicallback+0x24/0x30
 ipi_nmi_handler+0x48/0x60
 handle_percpu_devid_fasteoi_ipi+0x74/0x88
 generic_handle_irq+0x30/0x48
 handle_domain_nmi+0x48/0x80
 gic_handle_irq+0x18c/0x34c
 el1_irq+0xcc/0x180
 lkdtm_HARDLOCKUP+0x8/0x18
 direct_entry+0x124/0x1c0
 full_proxy_write+0x60/0xb0
 vfs_write+0xf0/0x230
 ksys_write+0x6c/0xf8
 __arm64_sys_write+0x1c/0x28
 el0_svc_common.constprop.0+0x74/0x1f0
 do_el0_svc+0x24/0x90
 el0_sync_handler+0x180/0x2f8
 el0_sync+0x158/0x180
<snip>

Changes in v4:
- Move IPI NMI framework to a separate file.
- Get rid of hard-coded IPI_CALL_NMI_FUNC allocation.
- Add NMI backtrace support leveraged via magic SYSRQ.

Changes in v3:
- Rebased to Marc's latest IPIs patch-set [1].

[1] https://lkml.org/lkml/2020/9/1/603

Changes since RFC version [1]:
- Switch to use generic interrupt framework to turn an IPI as NMI.
- Dependent on Marc's patch-set [2] which turns IPIs into normal
  interrupts.
- Addressed misc. comments from Doug on patch #4.
- Posted kgdb NMI printk() fixup separately which has evolved since
  to be solved using different approach via changing kgdb interception
  of printk() in common printk() code (see patch [3]).

[1] https://lkml.org/lkml/2020/4/24/328
[2] https://lkml.org/lkml/2020/5/19/710
[3] https://lkml.org/lkml/2020/5/20/418

Sumit Garg (5):
  arm64: Add framework to turn IPI as NMI
  irqchip/gic-v3: Enable support for SGIs to act as NMIs
  arm64: smp: Allocate and setup IPI as NMI
  arm64: kgdb: Round up cpus using IPI as NMI
  arm64: ipi_nmi: Add support for NMI backtrace

 arch/arm64/include/asm/irq.h  |  6 +++
 arch/arm64/include/asm/kgdb.h |  8 ++++
 arch/arm64/include/asm/nmi.h  | 16 ++++++++
 arch/arm64/kernel/Makefile    |  2 +-
 arch/arm64/kernel/ipi_nmi.c   | 93 +++++++++++++++++++++++++++++++++++++++++++
 arch/arm64/kernel/kgdb.c      | 21 ++++++++++
 arch/arm64/kernel/smp.c       |  8 ++++
 drivers/irqchip/irq-gic-v3.c  | 13 +++++-
 8 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm64/include/asm/nmi.h
 create mode 100644 arch/arm64/kernel/ipi_nmi.c

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4 1/5] arm64: Add framework to turn IPI as NMI
  2020-09-11 13:28 [PATCH v4 0/5] arm64: Add framework to turn an IPI as NMI Sumit Garg
@ 2020-09-11 13:28 ` Sumit Garg
  2020-10-10  1:58   ` Masayoshi Mizuma
  2020-09-11 13:28 ` [PATCH v4 2/5] irqchip/gic-v3: Enable support for SGIs to act as NMIs Sumit Garg
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Sumit Garg @ 2020-09-11 13:28 UTC (permalink / raw)
  To: maz, catalin.marinas, will
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason, kgdb-bugreport,
	dianders, linux-kernel, linux-arm-kernel, jason.wessel, tglx,
	julien.thierry.kdev

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 <sumit.garg@linaro.org>
---
 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 <sumit.garg@linaro.org>
+ */
+
+#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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 2/5] irqchip/gic-v3: Enable support for SGIs to act as NMIs
  2020-09-11 13:28 [PATCH v4 0/5] arm64: Add framework to turn an IPI as NMI Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 1/5] arm64: Add framework to turn " Sumit Garg
@ 2020-09-11 13:28 ` Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 3/5] arm64: smp: Allocate and setup IPI as NMI Sumit Garg
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Sumit Garg @ 2020-09-11 13:28 UTC (permalink / raw)
  To: maz, catalin.marinas, will
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason, kgdb-bugreport,
	dianders, linux-kernel, linux-arm-kernel, jason.wessel, tglx,
	julien.thierry.kdev

Add support to handle SGIs as regular NMIs. As SGIs or IPIs defaults to a
special flow handler: handle_percpu_devid_fasteoi_ipi(), so skip NMI
handler update in case of SGIs.

Also, enable NMI support prior to gic_smp_init() as allocation of SGIs
as IRQs/NMIs happen as part of this routine.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/irqchip/irq-gic-v3.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 7170645..dfd8e03 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -476,6 +476,11 @@ static int gic_irq_nmi_setup(struct irq_data *d)
 	if (WARN_ON(gic_irq(d) >= 8192))
 		return -EINVAL;
 
+	if (get_intid_range(d) == SGI_RANGE) {
+		gic_irq_set_prio(d, GICD_INT_NMI_PRI);
+		return 0;
+	}
+
 	/* desc lock should already be held */
 	if (gic_irq_in_rdist(d)) {
 		u32 idx = gic_get_ppi_index(d);
@@ -513,6 +518,11 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
 	if (WARN_ON(gic_irq(d) >= 8192))
 		return;
 
+	if (get_intid_range(d) == SGI_RANGE) {
+		gic_irq_set_prio(d, GICD_INT_DEF_PRI);
+		return;
+	}
+
 	/* desc lock should already be held */
 	if (gic_irq_in_rdist(d)) {
 		u32 idx = gic_get_ppi_index(d);
@@ -1666,6 +1676,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
 
 	gic_dist_init();
 	gic_cpu_init();
+	gic_enable_nmi_support();
 	gic_smp_init();
 	gic_cpu_pm_init();
 
@@ -1677,8 +1688,6 @@ static int __init gic_init_bases(void __iomem *dist_base,
 			gicv2m_init(handle, gic_data.domain);
 	}
 
-	gic_enable_nmi_support();
-
 	return 0;
 
 out_free:
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 3/5] arm64: smp: Allocate and setup IPI as NMI
  2020-09-11 13:28 [PATCH v4 0/5] arm64: Add framework to turn an IPI as NMI Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 1/5] arm64: Add framework to turn " Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 2/5] irqchip/gic-v3: Enable support for SGIs to act as NMIs Sumit Garg
@ 2020-09-11 13:28 ` Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 4/5] arm64: kgdb: Round up cpus using " Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 5/5] arm64: ipi_nmi: Add support for NMI backtrace Sumit Garg
  4 siblings, 0 replies; 12+ messages in thread
From: Sumit Garg @ 2020-09-11 13:28 UTC (permalink / raw)
  To: maz, catalin.marinas, will
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason, kgdb-bugreport,
	dianders, linux-kernel, linux-arm-kernel, jason.wessel, tglx,
	julien.thierry.kdev

Allocate an unused IPI that can be turned as NMI using ipi_nmi framework.
Also, invoke corresponding NMI setup/teardown APIs.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 arch/arm64/kernel/smp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index b6bde26..3f3b1ff 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -43,6 +43,7 @@
 #include <asm/daifflags.h>
 #include <asm/kvm_mmu.h>
 #include <asm/mmu_context.h>
+#include <asm/nmi.h>
 #include <asm/numa.h>
 #include <asm/processor.h>
 #include <asm/smp_plat.h>
@@ -962,6 +963,8 @@ 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);
 }
 
 static void ipi_teardown(int cpu)
@@ -973,6 +976,8 @@ static void ipi_teardown(int cpu)
 
 	for (i = 0; i < nr_ipi; i++)
 		disable_percpu_irq(ipi_irq_base + i);
+
+	ipi_nmi_teardown(cpu);
 }
 
 void __init set_smp_ipi_range(int ipi_base, int n)
@@ -993,6 +998,9 @@ void __init set_smp_ipi_range(int ipi_base, int n)
 		irq_set_status_flags(ipi_base + i, IRQ_HIDDEN);
 	}
 
+	if (n > nr_ipi)
+		set_smp_ipi_nmi(ipi_base + nr_ipi);
+
 	ipi_irq_base = ipi_base;
 
 	/* Setup the boot CPU immediately */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 4/5] arm64: kgdb: Round up cpus using IPI as NMI
  2020-09-11 13:28 [PATCH v4 0/5] arm64: Add framework to turn an IPI as NMI Sumit Garg
                   ` (2 preceding siblings ...)
  2020-09-11 13:28 ` [PATCH v4 3/5] arm64: smp: Allocate and setup IPI as NMI Sumit Garg
@ 2020-09-11 13:28 ` Sumit Garg
  2020-09-11 13:28 ` [PATCH v4 5/5] arm64: ipi_nmi: Add support for NMI backtrace Sumit Garg
  4 siblings, 0 replies; 12+ messages in thread
From: Sumit Garg @ 2020-09-11 13:28 UTC (permalink / raw)
  To: maz, catalin.marinas, will
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason, kgdb-bugreport,
	dianders, linux-kernel, linux-arm-kernel, jason.wessel, tglx,
	julien.thierry.kdev

arm64 platforms with GICv3 or later supports pseudo NMIs which can be
leveraged to round up CPUs which are stuck in hard lockup state with
interrupts disabled that wouldn't be possible with a normal IPI.

So instead switch to round up CPUs using IPI turned as NMI. And in
case a particular arm64 platform doesn't supports pseudo NMIs,
this IPI will act as a normal IPI which maintains existing kgdb
functionality.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 arch/arm64/include/asm/kgdb.h |  8 ++++++++
 arch/arm64/kernel/ipi_nmi.c   |  5 ++++-
 arch/arm64/kernel/kgdb.c      | 21 +++++++++++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kgdb.h b/arch/arm64/include/asm/kgdb.h
index 21fc85e..6f3d3af 100644
--- a/arch/arm64/include/asm/kgdb.h
+++ b/arch/arm64/include/asm/kgdb.h
@@ -24,6 +24,14 @@ static inline void arch_kgdb_breakpoint(void)
 extern void kgdb_handle_bus_error(void);
 extern int kgdb_fault_expected;
 
+#ifdef CONFIG_KGDB
+extern void ipi_kgdb_nmicallback(int cpu, void *regs);
+#else
+static inline void ipi_kgdb_nmicallback(int cpu, void *regs)
+{
+}
+#endif
+
 #endif /* !__ASSEMBLY__ */
 
 /*
diff --git a/arch/arm64/kernel/ipi_nmi.c b/arch/arm64/kernel/ipi_nmi.c
index 355ef92..627bc11 100644
--- a/arch/arm64/kernel/ipi_nmi.c
+++ b/arch/arm64/kernel/ipi_nmi.c
@@ -8,6 +8,7 @@
 
 #include <linux/interrupt.h>
 #include <linux/irq.h>
+#include <linux/kgdb.h>
 #include <linux/smp.h>
 
 #include <asm/nmi.h>
@@ -26,7 +27,9 @@ void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask)
 
 static irqreturn_t ipi_nmi_handler(int irq, void *data)
 {
-	/* nop, NMI handlers for special features can be added here. */
+	unsigned int cpu = smp_processor_id();
+
+	ipi_kgdb_nmicallback(cpu, get_irq_regs());
 
 	return IRQ_HANDLED;
 }
diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 1a157ca3..0991275 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -17,6 +17,7 @@
 
 #include <asm/debug-monitors.h>
 #include <asm/insn.h>
+#include <asm/nmi.h>
 #include <asm/traps.h>
 
 struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
@@ -353,3 +354,23 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
 	return aarch64_insn_write((void *)bpt->bpt_addr,
 			*(u32 *)bpt->saved_instr);
 }
+
+void ipi_kgdb_nmicallback(int cpu, void *regs)
+{
+	if (atomic_read(&kgdb_active) != -1)
+		kgdb_nmicallback(cpu, regs);
+}
+
+#ifdef CONFIG_SMP
+void kgdb_roundup_cpus(void)
+{
+	struct cpumask mask;
+
+	cpumask_copy(&mask, cpu_online_mask);
+	cpumask_clear_cpu(raw_smp_processor_id(), &mask);
+	if (cpumask_empty(&mask))
+		return;
+
+	arch_send_call_nmi_func_ipi_mask(&mask);
+}
+#endif
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v4 5/5] arm64: ipi_nmi: Add support for NMI backtrace
  2020-09-11 13:28 [PATCH v4 0/5] arm64: Add framework to turn an IPI as NMI Sumit Garg
                   ` (3 preceding siblings ...)
  2020-09-11 13:28 ` [PATCH v4 4/5] arm64: kgdb: Round up cpus using " Sumit Garg
@ 2020-09-11 13:28 ` Sumit Garg
  4 siblings, 0 replies; 12+ messages in thread
From: Sumit Garg @ 2020-09-11 13:28 UTC (permalink / raw)
  To: maz, catalin.marinas, will
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason, kgdb-bugreport,
	dianders, linux-kernel, linux-arm-kernel, jason.wessel, tglx,
	julien.thierry.kdev

Enable NMI backtrace support on arm64 using IPI turned as an NMI
leveraging pseudo NMIs support. It is now possible for users to get a
backtrace of a CPU stuck in hard-lockup using magic SYSRQ.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 arch/arm64/include/asm/irq.h |  6 ++++++
 arch/arm64/kernel/ipi_nmi.c  | 12 +++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index b2b0c64..e840bf1 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -6,6 +6,12 @@
 
 #include <asm-generic/irq.h>
 
+#ifdef CONFIG_SMP
+extern void arch_trigger_cpumask_backtrace(const cpumask_t *mask,
+					   bool exclude_self);
+#define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace
+#endif
+
 struct pt_regs;
 
 static inline int nr_legacy_irqs(void)
diff --git a/arch/arm64/kernel/ipi_nmi.c b/arch/arm64/kernel/ipi_nmi.c
index 627bc11..d3aa430 100644
--- a/arch/arm64/kernel/ipi_nmi.c
+++ b/arch/arm64/kernel/ipi_nmi.c
@@ -9,6 +9,7 @@
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/kgdb.h>
+#include <linux/nmi.h>
 #include <linux/smp.h>
 
 #include <asm/nmi.h>
@@ -25,12 +26,21 @@ void arch_send_call_nmi_func_ipi_mask(cpumask_t *mask)
 	__ipi_send_mask(ipi_desc, mask);
 }
 
+void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self)
+{
+	nmi_trigger_cpumask_backtrace(mask, exclude_self,
+				      arch_send_call_nmi_func_ipi_mask);
+}
+
 static irqreturn_t ipi_nmi_handler(int irq, void *data)
 {
 	unsigned int cpu = smp_processor_id();
 
-	ipi_kgdb_nmicallback(cpu, get_irq_regs());
+	if (nmi_cpu_backtrace(get_irq_regs()))
+		goto out;
 
+	ipi_kgdb_nmicallback(cpu, get_irq_regs());
+out:
 	return IRQ_HANDLED;
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/5] arm64: Add framework to turn IPI as NMI
  2020-09-11 13:28 ` [PATCH v4 1/5] arm64: Add framework to turn " Sumit Garg
@ 2020-10-10  1:58   ` Masayoshi Mizuma
  2020-10-10  9:34     ` Marc Zyngier
  2020-10-12 12:21     ` Sumit Garg
  0 siblings, 2 replies; 12+ messages in thread
From: Masayoshi Mizuma @ 2020-10-10  1:58 UTC (permalink / raw)
  To: Sumit Garg
  Cc: mark.rutland, daniel.thompson, jason, maz, jason.wessel,
	dianders, linux-kernel, julien.thierry.kdev, catalin.marinas,
	kgdb-bugreport, tglx, will, linux-arm-kernel

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 <sumit.garg@linaro.org>
> ---
>  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 <sumit.garg@linaro.org>
> + */
> +
> +#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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/5] arm64: Add framework to turn IPI as NMI
  2020-10-10  1:58   ` Masayoshi Mizuma
@ 2020-10-10  9:34     ` Marc Zyngier
  2020-10-10 15:13       ` Masayoshi Mizuma
  2020-10-12 12:21     ` Sumit Garg
  1 sibling, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2020-10-10  9:34 UTC (permalink / raw)
  To: Masayoshi Mizuma
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason,
	catalin.marinas, jason.wessel, dianders, linux-kernel,
	julien.thierry.kdev, kgdb-bugreport, tglx, will,
	linux-arm-kernel

On Sat, 10 Oct 2020 02:58:55 +0100,
Masayoshi Mizuma <msys.mizuma@gmail.com> 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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/5] arm64: Add framework to turn IPI as NMI
  2020-10-10  9:34     ` Marc Zyngier
@ 2020-10-10 15:13       ` Masayoshi Mizuma
  2020-10-12 12:19         ` Sumit Garg
  0 siblings, 1 reply; 12+ messages in thread
From: Masayoshi Mizuma @ 2020-10-10 15:13 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: mark.rutland, Sumit Garg, daniel.thompson, jason,
	catalin.marinas, jason.wessel, dianders, linux-kernel,
	julien.thierry.kdev, kgdb-bugreport, tglx, will,
	linux-arm-kernel

On Sat, Oct 10, 2020 at 10:34:04AM +0100, Marc Zyngier wrote:
> On Sat, 10 Oct 2020 02:58:55 +0100,
> Masayoshi Mizuma <msys.mizuma@gmail.com> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/5] arm64: Add framework to turn IPI as NMI
  2020-10-10 15:13       ` Masayoshi Mizuma
@ 2020-10-12 12:19         ` Sumit Garg
  2020-10-13  3:23           ` Masayoshi Mizuma
  0 siblings, 1 reply; 12+ messages in thread
From: Sumit Garg @ 2020-10-12 12:19 UTC (permalink / raw)
  To: Masayoshi Mizuma
  Cc: Mark Rutland, Daniel Thompson, Jason Cooper, Marc Zyngier,
	Jason Wessel, Douglas Anderson, Linux Kernel Mailing List,
	julien.thierry.kdev, Catalin Marinas, kgdb-bugreport,
	Thomas Gleixner, Will Deacon, linux-arm-kernel

Hi Masa,

On Sat, 10 Oct 2020 at 20:43, Masayoshi Mizuma <msys.mizuma@gmail.com> 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 <msys.mizuma@gmail.com> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/5] arm64: Add framework to turn IPI as NMI
  2020-10-10  1:58   ` Masayoshi Mizuma
  2020-10-10  9:34     ` Marc Zyngier
@ 2020-10-12 12:21     ` Sumit Garg
  1 sibling, 0 replies; 12+ messages in thread
From: Sumit Garg @ 2020-10-12 12:21 UTC (permalink / raw)
  To: Masayoshi Mizuma
  Cc: Mark Rutland, Daniel Thompson, Jason Cooper, Marc Zyngier,
	Jason Wessel, Douglas Anderson, Linux Kernel Mailing List,
	julien.thierry.kdev, Catalin Marinas, kgdb-bugreport,
	Thomas Gleixner, Will Deacon, linux-arm-kernel

On Sat, 10 Oct 2020 at 07:28, Masayoshi Mizuma <msys.mizuma@gmail.com> 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 <sumit.garg@linaro.org>
> > ---
> >  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 <sumit.garg@linaro.org>
> > + */
> > +
> > +#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
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4 1/5] arm64: Add framework to turn IPI as NMI
  2020-10-12 12:19         ` Sumit Garg
@ 2020-10-13  3:23           ` Masayoshi Mizuma
  0 siblings, 0 replies; 12+ messages in thread
From: Masayoshi Mizuma @ 2020-10-13  3:23 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Mark Rutland, Daniel Thompson, Jason Cooper, Marc Zyngier,
	Jason Wessel, Douglas Anderson, Linux Kernel Mailing List,
	julien.thierry.kdev, Catalin Marinas, kgdb-bugreport,
	Thomas Gleixner, Will Deacon, 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 <msys.mizuma@gmail.com> 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 <msys.mizuma@gmail.com> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-10-13  3:24 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11 13:28 [PATCH v4 0/5] arm64: Add framework to turn an IPI as NMI Sumit Garg
2020-09-11 13:28 ` [PATCH v4 1/5] arm64: Add framework to turn " Sumit Garg
2020-10-10  1:58   ` Masayoshi Mizuma
2020-10-10  9:34     ` Marc Zyngier
2020-10-10 15:13       ` Masayoshi Mizuma
2020-10-12 12:19         ` Sumit Garg
2020-10-13  3:23           ` Masayoshi Mizuma
2020-10-12 12:21     ` Sumit Garg
2020-09-11 13:28 ` [PATCH v4 2/5] irqchip/gic-v3: Enable support for SGIs to act as NMIs Sumit Garg
2020-09-11 13:28 ` [PATCH v4 3/5] arm64: smp: Allocate and setup IPI as NMI Sumit Garg
2020-09-11 13:28 ` [PATCH v4 4/5] arm64: kgdb: Round up cpus using " Sumit Garg
2020-09-11 13:28 ` [PATCH v4 5/5] arm64: ipi_nmi: Add support for NMI backtrace Sumit Garg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).