Skip to contentSkip to conversation

powerpc: NMI IPIs

RFC PATCH In review Nicholas Piggin 3 participants 8 messages Nov 8, 2016 – Nov 11, 2016

Subsystems (5)
lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

cover powerpc: NMI IPIs

Hi,

This is a significant reworking of earlier patches, it should be getting closer to the right shape now, but I'm always open to change anything.

Thanks,
Nick

Nicholas Piggin (3):
powerpc: add NMI IPI infrastructure
powerpc: add struct smp_ops_t.cause_nmi_ipi operation powerpc/pseries: implement nmi ipi with H_SIGNAL_SYS_RESET

 arch/powerpc/include/asm/smp.h            |   7 +-
 arch/powerpc/kernel/smp.c                 | 282 +++++++++++++++++++++++++-----
 arch/powerpc/kernel/traps.c               |   7 +-
 arch/powerpc/platforms/85xx/smp.c         |   1 +
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |   1 +
 arch/powerpc/platforms/cell/interrupt.c   |   2 +-
 arch/powerpc/platforms/chrp/smp.c         |   1 +
 arch/powerpc/platforms/powermac/smp.c     |   1 +
 arch/powerpc/platforms/powernv/smp.c      |   1 +
 arch/powerpc/platforms/ps3/smp.c          |   4 +-
 arch/powerpc/platforms/pseries/ras.c      |   4 +
 arch/powerpc/platforms/pseries/smp.c      |  11 ++
 12 files changed, 268 insertions(+), 54 deletions(-)
Signature · 1 line
2.10.2
author reviewer
#1 lore raw

PATCH 1/3 powerpc: add NMI IPI infrastructure

Add an NMI IPI system that handles platform differences and concurrency and reentrancy issues.

The platform does not have to implement a true non-maskable interrupt. The default is to simply uses the debugger break IPI message.

The debugger break users (debugger and crash) have been reimplemented on top of the NMI.

smp_send_stop has also been implemented as NMI.

---
 arch/powerpc/include/asm/smp.h          |   2 +-
 arch/powerpc/kernel/smp.c               | 271 ++++++++++++++++++++++++++------
 arch/powerpc/platforms/cell/interrupt.c |   2 +-
 arch/powerpc/platforms/ps3/smp.c        |   4 +-
 4 files changed, 227 insertions(+), 52 deletions(-)
arch/powerpc/include/asm/smp.h +1 -1
index 0d02c11..055918d 100644
@@ -115,7 +115,7 @@ extern int cpu_to_core_id(int cpu);
 #define PPC_MSG_CALL_FUNCTION   0
 #define PPC_MSG_RESCHEDULE      1
 #define PPC_MSG_TICK_BROADCAST	2
-#define PPC_MSG_DEBUGGER_BREAK  3
+#define PPC_MSG_NMI_IPI_SAFE	3
 
 /* This is only used by the powernv kernel */
 #define PPC_MSG_RM_HOST_ACTION	4
arch/powerpc/kernel/smp.c +223 -48

335 lines not on this page · raw message

arch/powerpc/platforms/cell/interrupt.c +1 -1
index a6bbbab..fcc94c8 100644
@@ -211,7 +211,7 @@ void iic_request_IPIs(void)
 	iic_request_ipi(PPC_MSG_CALL_FUNCTION);
 	iic_request_ipi(PPC_MSG_RESCHEDULE);
 	iic_request_ipi(PPC_MSG_TICK_BROADCAST);
-	iic_request_ipi(PPC_MSG_DEBUGGER_BREAK);
+	iic_request_ipi(PPC_MSG_NMI_IPI_SAFE);
 }
 
 #endif /* CONFIG_SMP */
arch/powerpc/platforms/ps3/smp.c +2 -2
index 60154d0..5b1fb70 100644
@@ -77,7 +77,7 @@ static void __init ps3_smp_probe(void)
 		BUILD_BUG_ON(PPC_MSG_CALL_FUNCTION    != 0);
 		BUILD_BUG_ON(PPC_MSG_RESCHEDULE       != 1);
 		BUILD_BUG_ON(PPC_MSG_TICK_BROADCAST   != 2);
-		BUILD_BUG_ON(PPC_MSG_DEBUGGER_BREAK   != 3);
+		BUILD_BUG_ON(PPC_MSG_NMI_IPI_SAFE   != 3);
 
 		for (i = 0; i < MSG_COUNT; i++) {
 			result = ps3_event_receive_port_setup(cpu, &virqs[i]);
@@ -96,7 +96,7 @@ static void __init ps3_smp_probe(void)
 				ps3_register_ipi_irq(cpu, virqs[i]);
 		}
 
-		ps3_register_ipi_debug_brk(cpu, virqs[PPC_MSG_DEBUGGER_BREAK]);
+		ps3_register_ipi_debug_brk(cpu, virqs[PPC_MSG_NMI_IPI_SAFE]);
 
 		DBG(" <- %s:%d: (%d)\n", __func__, __LINE__, cpu);
 	}
Signature · 1 line
2.10.2
#2 lore raw

Re: [PATCH 1/3] powerpc: add NMI IPI infrastructure

reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi Nicholas,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.9-rc4]
[cannot apply to next-20161108]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-NMI-IPIs/20161109-005049
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705

reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc

Note: the linux-review/Nicholas-Piggin/powerpc-NMI-IPIs/20161109-005049 HEAD ebfcf2d2f068a2e00d385864b5ba9371e15adcef builds fine.
It only hurts bisectibility.

All errors (new ones prefixed by >>):

arch/powerpc/kernel/smp.c: In function 'nmi_ipi_safe_action':

>> arch/powerpc/kernel/smp.c:160:2: error: implicit declaration of function 'smp_handle_nmi_ipi' [-Werror=implicit-function-declaration]
     smp_handle_nmi_ipi(get_irq_regs());
     ^~~~~~~~~~~~~~~~~~

cc1: all warnings being treated as errors

vim +/smp_handle_nmi_ipi +160 arch/powerpc/kernel/smp.c

154 tick_broadcast_ipi_handler();
155 return IRQ_HANDLED;
156 }
157
158 static irqreturn_t nmi_ipi_safe_action(int irq, void *data)
159 {

 > 160		smp_handle_nmi_ipi(get_irq_regs());

161 return IRQ_HANDLED;
162 }
163

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

author reviewer
#3 lore raw

PATCH 2/3 powerpc: add struct smp_ops_t.cause_nmi_ipi operation

In reply to Nicholas Piggin

Allow platforms to provide an NMI IPI, and wire that to the NMI system.

---
 arch/powerpc/include/asm/smp.h            |  5 +++++
 arch/powerpc/kernel/smp.c                 | 21 ++++++++++++++++-----
 arch/powerpc/platforms/85xx/smp.c         |  1 +
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |  1 +
 arch/powerpc/platforms/chrp/smp.c         |  1 +
 arch/powerpc/platforms/powermac/smp.c     |  1 +
 arch/powerpc/platforms/powernv/smp.c      |  1 +
 arch/powerpc/platforms/pseries/smp.c      |  1 +
 8 files changed, 27 insertions(+), 5 deletions(-)
arch/powerpc/include/asm/smp.h +5 -0
index 055918d..15521c1 100644
@@ -37,11 +37,15 @@ extern int cpu_to_chip_id(int cpu);
 
 #ifdef CONFIG_SMP
 
+#define SMP_OP_NMI_TYPE_SAFE	1
+#define SMP_OP_NMI_TYPE_HARD	2
+
 struct smp_ops_t {
 	void  (*message_pass)(int cpu, int msg);
 #ifdef CONFIG_PPC_SMP_MUXED_IPI
 	void  (*cause_ipi)(int cpu, unsigned long data);
 #endif
+	int   (*cause_nmi_ipi)(int cpu, int type);
 	void  (*probe)(void);
 	int   (*kick_cpu)(int nr);
 	void  (*setup_cpu)(int nr);
@@ -53,6 +57,7 @@ struct smp_ops_t {
 	int   (*cpu_bootable)(unsigned int nr);
 };
 
+extern int smp_handle_nmi_ipi(struct pt_regs *regs);
 extern void smp_send_debugger_break(void);
 extern void start_secondary_resume(void);
 extern void smp_generic_give_timebase(void);
arch/powerpc/kernel/smp.c +16 -5
index 959d9dc..b3133e9 100644
@@ -425,8 +425,10 @@ int smp_handle_nmi_ipi(struct pt_regs *regs)
 	return ret;
 }
 
-static void do_smp_send_nmi_ipi(int cpu)
+static void do_smp_send_nmi_ipi(int cpu, int type)
 {
+	if (smp_ops->cause_nmi_ipi && smp_ops->cause_nmi_ipi(cpu, type))
+		return;
 	do_message_pass(cpu, PPC_MSG_NMI_IPI_SAFE);
 }
 
@@ -453,9 +455,6 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 	if (unlikely(!smp_ops))
 		return 0;
 
-	/* Have no real NMI capability yet */
-	safe_udelay += hard_udelay;
-
 	get_online_cpus();
 
 	/* Take the nmi_ipi_busy count/lock with interrupts hard disabled */
@@ -482,7 +481,7 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 
 	if (safe_udelay) {
 		for_each_cpu(c, &nmi_ipi_pending_mask)
-			do_smp_send_nmi_ipi(c);
+			do_smp_send_nmi_ipi(c, SMP_OP_NMI_TYPE_SAFE);
 
 		do {
 			safe_udelay--;
@@ -492,6 +491,18 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 		} while (safe_udelay);
 	}
 
+	if (hard_udelay) {
+		for_each_cpu(c, &nmi_ipi_pending_mask)
+			do_smp_send_nmi_ipi(c, SMP_OP_NMI_TYPE_HARD);
+
+		do {
+			hard_udelay--;
+			udelay(1);
+			if (cpumask_empty(&nmi_ipi_pending_mask))
+				goto done;
+		} while (hard_udelay);
+	}
+
 	ret = 0; /* Could not gather all CPUs */
 	cpumask_clear(&nmi_ipi_pending_mask);
 done:
arch/powerpc/platforms/85xx/smp.c +1 -0
index fe9f19e..cde7beb 100644
@@ -343,6 +343,7 @@ static int smp_85xx_kick_cpu(int nr)
 }
 
 struct smp_ops_t smp_85xx_ops = {
+	.cause_nmi_ipi = NULL,
 	.kick_cpu = smp_85xx_kick_cpu,
 	.cpu_bootable = smp_generic_cpu_bootable,
 #ifdef CONFIG_HOTPLUG_CPU
arch/powerpc/platforms/86xx/mpc86xx_smp.c +1 -0
index af09bae..020e84a 100644
@@ -105,6 +105,7 @@ smp_86xx_setup_cpu(int cpu_nr)
 
 
 struct smp_ops_t smp_86xx_ops = {
+	.cause_nmi_ipi = NULL,
 	.message_pass = smp_mpic_message_pass,
 	.probe = smp_mpic_probe,
 	.kick_cpu = smp_86xx_kick_cpu,
arch/powerpc/platforms/chrp/smp.c +1 -0
index b6c9a0d..1451504 100644
@@ -44,6 +44,7 @@ static void smp_chrp_setup_cpu(int cpu_nr)
 
 /* CHRP with openpic */
 struct smp_ops_t chrp_smp_ops = {
+	.cause_nmi_ipi = NULL,
 	.message_pass = smp_mpic_message_pass,
 	.probe = smp_mpic_probe,
 	.kick_cpu = smp_chrp_kick_cpu,
arch/powerpc/platforms/powermac/smp.c +1 -0
index c9eb7d6..1d76e15 100644
@@ -446,6 +446,7 @@ void __init smp_psurge_give_timebase(void)
 struct smp_ops_t psurge_smp_ops = {
 	.message_pass	= NULL,	/* Use smp_muxed_ipi_message_pass */
 	.cause_ipi	= smp_psurge_cause_ipi,
+	.cause_nmi_ipi	= NULL,
 	.probe		= smp_psurge_probe,
 	.kick_cpu	= smp_psurge_kick_cpu,
 	.setup_cpu	= smp_psurge_setup_cpu,
arch/powerpc/platforms/powernv/smp.c +1 -0
index c789258..092ec1f 100644
@@ -244,6 +244,7 @@ static int pnv_cpu_bootable(unsigned int nr)
 static struct smp_ops_t pnv_smp_ops = {
 	.message_pass	= smp_muxed_ipi_message_pass,
 	.cause_ipi	= NULL,	/* Filled at runtime by xics_smp_probe() */
+	.cause_nmi_ipi	= NULL,
 	.probe		= xics_smp_probe,
 	.kick_cpu	= pnv_smp_kick_cpu,
 	.setup_cpu	= pnv_smp_setup_cpu,
arch/powerpc/platforms/pseries/smp.c +1 -0
index f6f83ae..0f6522c 100644
@@ -209,6 +209,7 @@ static __init void pSeries_smp_probe(void)
 static struct smp_ops_t pseries_smp_ops = {
 	.message_pass	= NULL,	/* Use smp_muxed_ipi_message_pass */
 	.cause_ipi	= NULL,	/* Filled at runtime by pSeries_smp_probe() */
+	.cause_nmi_ipi	= NULL,
 	.probe		= pSeries_smp_probe,
 	.kick_cpu	= smp_pSeries_kick_cpu,
 	.setup_cpu	= smp_setup_cpu,
Signature · 1 line
2.10.2
#4 lore raw

Re: [PATCH 2/3] powerpc: add struct smp_ops_t.cause_nmi_ipi operation

Quoted 77 lines · Nicholas Piggin

Allow platforms to provide an NMI IPI, and wire that to the NMI system.

---
 arch/powerpc/include/asm/smp.h            |  5 +++++
 arch/powerpc/kernel/smp.c                 | 21 ++++++++++++++++-----
 arch/powerpc/platforms/85xx/smp.c         |  1 +
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |  1 +
 arch/powerpc/platforms/chrp/smp.c         |  1 +
 arch/powerpc/platforms/powermac/smp.c     |  1 +
 arch/powerpc/platforms/powernv/smp.c      |  1 +
 arch/powerpc/platforms/pseries/smp.c      |  1 +
 8 files changed, 27 insertions(+), 5 deletions(-)
arch/powerpc/include/asm/smp.h +5 -0
index 055918d..15521c1 100644
@@ -37,11 +37,15 @@ extern int cpu_to_chip_id(int cpu);
 
 #ifdef CONFIG_SMP
 
+#define SMP_OP_NMI_TYPE_SAFE	1
+#define SMP_OP_NMI_TYPE_HARD	2
+
 struct smp_ops_t {
 	void  (*message_pass)(int cpu, int msg);
 #ifdef CONFIG_PPC_SMP_MUXED_IPI
 	void  (*cause_ipi)(int cpu, unsigned long data);
 #endif
+	int   (*cause_nmi_ipi)(int cpu, int type);
 	void  (*probe)(void);
 	int   (*kick_cpu)(int nr);
 	void  (*setup_cpu)(int nr);
@@ -53,6 +57,7 @@ struct smp_ops_t {
 	int   (*cpu_bootable)(unsigned int nr);
 };
 
+extern int smp_handle_nmi_ipi(struct pt_regs *regs);
 extern void smp_send_debugger_break(void);
 extern void start_secondary_resume(void);
 extern void smp_generic_give_timebase(void);
arch/powerpc/kernel/smp.c +4 -5
index 959d9dc..b3133e9 100644
@@ -425,8 +425,10 @@ int smp_handle_nmi_ipi(struct pt_regs *regs)
 	return ret;
 }
 
-static void do_smp_send_nmi_ipi(int cpu)
+static void do_smp_send_nmi_ipi(int cpu, int type)
 {
+	if (smp_ops->cause_nmi_ipi && smp_ops->cause_nmi_ipi(cpu, type))
+		return;
 	do_message_pass(cpu, PPC_MSG_NMI_IPI_SAFE);
 }
 
@@ -453,9 +455,6 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 	if (unlikely(!smp_ops))
 		return 0;
 
-	/* Have no real NMI capability yet */
-	safe_udelay += hard_udelay;
-
 	get_online_cpus();
 
 	/* Take the nmi_ipi_busy count/lock with interrupts hard disabled */
@@ -482,7 +481,7 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 
 	if (safe_udelay) {
 		for_each_cpu(c, &nmi_ipi_pending_mask)
-			do_smp_send_nmi_ipi(c);
+			do_smp_send_nmi_ipi(c, SMP_OP_NMI_TYPE_SAFE);
 
 		do {
 			safe_udelay--;

arch/powerpc/kernel/smp.c int smp_send_nmi_ipi(int cpu, int function, void *data, · line 494

Quoted 4 lines
arch/powerpc/kernel/smp.c +1 -0
@@ -492,6 +491,18 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 		} while (safe_udelay);
 	}
 
+	if (hard_udelay) {

So if the "safe" NMI fails we go onto the one that might break your system? Why would the safe NMI fail? Does it only fail when other CPUs are stuck with interrupts turned off? Or are there other cases? I'm just wondering how safe it is - is there a chance that the safe one might corrupt state we're interested in debugging?

I'm fairly confident we can make the hard NMI reliable (although perhaps not as good as the safe variant) so the safe one may not be worth it if there's a risk we loose debug state.

arch/powerpc/kernel/smp.c int smp_send_nmi_ipi(int cpu, int function, void *data, · line 495

Quoted 1 line
arch/powerpc/kernel/smp.c +1 -0
+		for_each_cpu(c, &nmi_ipi_pending_mask)

Doesn't the H_CALL support a flag for all but self? For PowerNV/OPAL I was looking to implement this as well and on bare-metal it is (perhaps marginally) more efficient to loop through the CPUs in firmware.

Of course the issue with that is that I'm not sure firmware is aware of cpu_online_mask so ALLBUTSELF would reset all CPUs, not just online ones. Not sure if that would be an issue?

- Alistair

Review

Quoted 14 lines
arch/powerpc/kernel/smp.c +10 -0
+			do_smp_send_nmi_ipi(c, SMP_OP_NMI_TYPE_HARD);
+
+		do {
+			hard_udelay--;
+			udelay(1);
+			if (cpumask_empty(&nmi_ipi_pending_mask))
+				goto done;
+		} while (hard_udelay);
+	}
+
 	ret = 0; /* Could not gather all CPUs */
 	cpumask_clear(&nmi_ipi_pending_mask);
 done:
(quoted diff) +0 -0

b/arch/powerpc/platforms/85xx/smp.c

Review

Quoted 12 lines

index fe9f19e..cde7beb 100644

arch/powerpc/platforms/85xx/smp.c +1 -0
@@ -343,6 +343,7 @@ static int smp_85xx_kick_cpu(int nr)
 }
 
 struct smp_ops_t smp_85xx_ops = {
+	.cause_nmi_ipi = NULL,
 	.kick_cpu = smp_85xx_kick_cpu,
 	.cpu_bootable = smp_generic_cpu_bootable,
 #ifdef CONFIG_HOTPLUG_CPU
(quoted diff) +0 -0

b/arch/powerpc/platforms/86xx/mpc86xx_smp.c

Review

Quoted 12 lines

index af09bae..020e84a 100644

arch/powerpc/platforms/86xx/mpc86xx_smp.c +1 -0
@@ -105,6 +105,7 @@ smp_86xx_setup_cpu(int cpu_nr)
 
 
 struct smp_ops_t smp_86xx_ops = {
+	.cause_nmi_ipi = NULL,
 	.message_pass = smp_mpic_message_pass,
 	.probe = smp_mpic_probe,
 	.kick_cpu = smp_86xx_kick_cpu,
(quoted diff) +0 -0

b/arch/powerpc/platforms/chrp/smp.c

Review

Quoted 12 lines

index b6c9a0d..1451504 100644

arch/powerpc/platforms/chrp/smp.c +1 -0
@@ -44,6 +44,7 @@ static void smp_chrp_setup_cpu(int cpu_nr)
 
 /* CHRP with openpic */
 struct smp_ops_t chrp_smp_ops = {
+	.cause_nmi_ipi = NULL,
 	.message_pass = smp_mpic_message_pass,
 	.probe = smp_mpic_probe,
 	.kick_cpu = smp_chrp_kick_cpu,
(quoted diff) +0 -0

b/arch/powerpc/platforms/powermac/smp.c

Review

Quoted 12 lines

index c9eb7d6..1d76e15 100644

arch/powerpc/platforms/powermac/smp.c +1 -0
@@ -446,6 +446,7 @@ void __init smp_psurge_give_timebase(void)
 struct smp_ops_t psurge_smp_ops = {
 	.message_pass	= NULL,	/* Use smp_muxed_ipi_message_pass */
 	.cause_ipi	= smp_psurge_cause_ipi,
+	.cause_nmi_ipi	= NULL,
 	.probe		= smp_psurge_probe,
 	.kick_cpu	= smp_psurge_kick_cpu,
 	.setup_cpu	= smp_psurge_setup_cpu,
(quoted diff) +0 -0

b/arch/powerpc/platforms/powernv/smp.c

Review

Quoted 12 lines

index c789258..092ec1f 100644

arch/powerpc/platforms/powernv/smp.c +1 -0
@@ -244,6 +244,7 @@ static int pnv_cpu_bootable(unsigned int nr)
 static struct smp_ops_t pnv_smp_ops = {
 	.message_pass	= smp_muxed_ipi_message_pass,
 	.cause_ipi	= NULL,	/* Filled at runtime by xics_smp_probe() */
+	.cause_nmi_ipi	= NULL,
 	.probe		= xics_smp_probe,
 	.kick_cpu	= pnv_smp_kick_cpu,
 	.setup_cpu	= pnv_smp_setup_cpu,
(quoted diff) +0 -0

b/arch/powerpc/platforms/pseries/smp.c

Quoted 12 lines

index f6f83ae..0f6522c 100644

arch/powerpc/platforms/pseries/smp.c +1 -0
@@ -209,6 +209,7 @@ static __init void pSeries_smp_probe(void)
 static struct smp_ops_t pseries_smp_ops = {
 	.message_pass	= NULL,	/* Use smp_muxed_ipi_message_pass */
 	.cause_ipi	= NULL,	/* Filled at runtime by pSeries_smp_probe() */
+	.cause_nmi_ipi	= NULL,
 	.probe		= pSeries_smp_probe,
 	.kick_cpu	= smp_pSeries_kick_cpu,
 	.setup_cpu	= smp_setup_cpu,
author reviewer
#5 lore raw

Re: [PATCH 2/3] powerpc: add struct smp_ops_t.cause_nmi_ipi operation

Quoted 81 lines · On Fri, 11 Nov 2016 12:00:14 +1100 Alistair Popple <[email protected]>…
Quoted 80 lines · Nicholas Piggin

Allow platforms to provide an NMI IPI, and wire that to the NMI system.

---
 arch/powerpc/include/asm/smp.h            |  5 +++++
 arch/powerpc/kernel/smp.c                 | 21 ++++++++++++++++-----
 arch/powerpc/platforms/85xx/smp.c         |  1 +
 arch/powerpc/platforms/86xx/mpc86xx_smp.c |  1 +
 arch/powerpc/platforms/chrp/smp.c         |  1 +
 arch/powerpc/platforms/powermac/smp.c     |  1 +
 arch/powerpc/platforms/powernv/smp.c      |  1 +
 arch/powerpc/platforms/pseries/smp.c      |  1 +
 8 files changed, 27 insertions(+), 5 deletions(-)
arch/powerpc/include/asm/smp.h +5 -0
index 055918d..15521c1 100644
@@ -37,11 +37,15 @@ extern int cpu_to_chip_id(int cpu);
 
 #ifdef CONFIG_SMP
 
+#define SMP_OP_NMI_TYPE_SAFE	1
+#define SMP_OP_NMI_TYPE_HARD	2
+
 struct smp_ops_t {
 	void  (*message_pass)(int cpu, int msg);
 #ifdef CONFIG_PPC_SMP_MUXED_IPI
 	void  (*cause_ipi)(int cpu, unsigned long data);
 #endif
+	int   (*cause_nmi_ipi)(int cpu, int type);
 	void  (*probe)(void);
 	int   (*kick_cpu)(int nr);
 	void  (*setup_cpu)(int nr);
@@ -53,6 +57,7 @@ struct smp_ops_t {
 	int   (*cpu_bootable)(unsigned int nr);
 };
 
+extern int smp_handle_nmi_ipi(struct pt_regs *regs);
 extern void smp_send_debugger_break(void);
 extern void start_secondary_resume(void);
 extern void smp_generic_give_timebase(void);
arch/powerpc/kernel/smp.c +4 -5
index 959d9dc..b3133e9 100644
@@ -425,8 +425,10 @@ int smp_handle_nmi_ipi(struct pt_regs *regs)
 	return ret;
 }
 
-static void do_smp_send_nmi_ipi(int cpu)
+static void do_smp_send_nmi_ipi(int cpu, int type)
 {
+	if (smp_ops->cause_nmi_ipi && smp_ops->cause_nmi_ipi(cpu, type))
+		return;
 	do_message_pass(cpu, PPC_MSG_NMI_IPI_SAFE);
 }
 
@@ -453,9 +455,6 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 	if (unlikely(!smp_ops))
 		return 0;
 
-	/* Have no real NMI capability yet */
-	safe_udelay += hard_udelay;
-
 	get_online_cpus();
 
 	/* Take the nmi_ipi_busy count/lock with interrupts hard disabled */
@@ -482,7 +481,7 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 
 	if (safe_udelay) {
 		for_each_cpu(c, &nmi_ipi_pending_mask)
-			do_smp_send_nmi_ipi(c);
+			do_smp_send_nmi_ipi(c, SMP_OP_NMI_TYPE_SAFE);
 
 		do {
 			safe_udelay--;
@@ -492,6 +491,18 @@ int smp_send_nmi_ipi(int cpu, int function, void *data,
 		} while (safe_udelay);
 	}
 
Quoted 5 lines

arch/powerpc/kernel/smp.c int smp_send_nmi_ipi(int cpu, int function, void *data, · line 494

Quoted 1 line
arch/powerpc/kernel/smp.c +1 -0
+	if (hard_udelay) {  

So if the "safe" NMI fails we go onto the one that might break your system? Why would the safe NMI fail? Does it only fail when other CPUs are stuck with interrupts turned off?

Yes. I don't *really* know if this is worth the added complexity, mind you. But in theory it's nice to be able to resume after debugger.

Quoted 3 lines

Or are there other cases? I'm just wondering how safe
it is - is there a chance that the safe one might corrupt state we're interested in debugging?

Yes, that's a possibility for sure. For example in the case where we saw two CPUs sharing the same stack, the doorbell interrupt would have tried to use the shared stack and got into even more problems. When we switch system reset interrupt to using a dedicated NMI stack, it will become quite minimal and uninvasive path to crashdump.

So for crashes/crashdumps, I think we definitely do just want to do the "hard" NMI immediately. We have no real possibility to recover in that case.

Quoted 3 lines

I'm fairly confident we can make the hard NMI reliable (although perhaps not as good as the safe variant) so the safe one may not be worth it if there's a risk we loose debug state.

Okay. Well so far the 3 main users are debugger, crashdump, and stop cpu.

stop and crashdump never have to recover.

debugger has the option to recover, which does work for the most part. Maybe we are happy to just say it should not run on production systems because it could cause a crash in rare cases?

Quoted 9 lines

arch/powerpc/kernel/smp.c int smp_send_nmi_ipi(int cpu, int function, void *data, · line 495

Quoted 1 line
arch/powerpc/kernel/smp.c +1 -0
+		for_each_cpu(c, &nmi_ipi_pending_mask)  

Doesn't the H_CALL support a flag for all but self? For PowerNV/OPAL I was looking to implement this as well and on bare-metal it is (perhaps marginally) more efficient to loop through the CPUs in firmware.

Of course the issue with that is that I'm not sure firmware is aware of cpu_online_mask so ALLBUTSELF would reset all CPUs, not just online ones. Not sure if that would be an issue?

Yes the hcall does support all and allbutself. I don't actually know if it would be a problem for offline CPUs to get system resets. The powernv platform can probably add a system_reset handler, so it could test the online mask and avoid calling to the IPI handler if the CPU is not online AFAIKS.

On the other hand, I wonder if simpler is better. Would there be any significant benefit for this in a crash/debug situation?

Thanks,
Nick

author reviewer
#6 lore raw

PATCH 3/3 powerpc/pseries: implement nmi ipi with H_SIGNAL_SYS_RESET

In reply to Nicholas Piggin

H_SIGNAL_SYS_RESET can provide a hard NMI (it is not recoverable if raised when the target has MSR_RI clear).

This patch makes a couple of changes to generic system_reset_exception handler, which should be split out and platforms defining a handler audited.

---
 arch/powerpc/kernel/traps.c          |  7 +++++--
 arch/powerpc/platforms/pseries/ras.c |  4 ++++
 arch/powerpc/platforms/pseries/smp.c | 12 +++++++++++-
 3 files changed, 20 insertions(+), 3 deletions(-)
arch/powerpc/kernel/traps.c +5 -2
index 023a462..d1d7fd4 100644
@@ -275,19 +275,22 @@ void _exception(int signr, struct pt_regs *regs, int code, unsigned long addr)
 
 void system_reset_exception(struct pt_regs *regs)
 {
+	nmi_enter();
+
 	/* See if any machine dependent calls */
 	if (ppc_md.system_reset_exception) {
 		if (ppc_md.system_reset_exception(regs))
-			return;
+			goto done;
 	}
 
 	die("System Reset", regs, SIGABRT);
 
+done:
 	/* Must die if the interrupt is not recoverable */
 	if (!(regs->msr & MSR_RI))
 		panic("Unrecoverable System Reset");
 
-	/* What should we do here? We could issue a shutdown or hard reset. */
+	nmi_exit();
 }
 
 #ifdef CONFIG_PPC64
arch/powerpc/platforms/pseries/ras.c +4 -0
index 904a677..bb70b26 100644
@@ -386,6 +386,10 @@ int pSeries_system_reset_exception(struct pt_regs *regs)
 		}
 		fwnmi_release_errinfo();
 	}
+
+	if (smp_handle_nmi_ipi(regs))
+		return 1;
+
 	return 0; /* need to perform reset */
 }
 
arch/powerpc/platforms/pseries/smp.c +11 -1
index 0f6522c..4534c5a 100644
@@ -196,6 +196,16 @@ static void pSeries_cause_ipi_mux(int cpu, unsigned long data)
 		xics_cause_ipi(cpu, data);
 }
 
+static int pseries_cause_nmi_ipi(int cpu, int type)
+{
+	if (type == SMP_OP_NMI_TYPE_HARD) {
+		if (plapr_signal_sys_reset(cpu) == H_SUCCESS)
+			return 1;
+	}
+
+	return 0;
+}
+
 static __init void pSeries_smp_probe(void)
 {
 	xics_smp_probe();
@@ -209,7 +219,7 @@ static __init void pSeries_smp_probe(void)
 static struct smp_ops_t pseries_smp_ops = {
 	.message_pass	= NULL,	/* Use smp_muxed_ipi_message_pass */
 	.cause_ipi	= NULL,	/* Filled at runtime by pSeries_smp_probe() */
-	.cause_nmi_ipi	= NULL,
+	.cause_nmi_ipi	= pseries_cause_nmi_ipi,
 	.probe		= pSeries_smp_probe,
 	.kick_cpu	= smp_pSeries_kick_cpu,
 	.setup_cpu	= smp_setup_cpu,
Signature · 1 line
2.10.2
#7 lore raw

Re: [PATCH 3/3] powerpc/pseries: implement nmi ipi with H_SIGNAL_SYS_RESET

reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree…

Hi Nicholas,

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.9-rc4]
[cannot apply to next-20161108]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-NMI-IPIs/20161109-005049
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705

reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc

All errors (new ones prefixed by >>):

arch/powerpc/platforms/pseries/smp.c: In function 'pseries_cause_nmi_ipi':

>> arch/powerpc/platforms/pseries/smp.c:202:7: error: implicit declaration of function 'plapr_signal_sys_reset' [-Werror=implicit-function-declaration]
      if (plapr_signal_sys_reset(cpu) == H_SUCCESS)
          ^~~~~~~~~~~~~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/plapr_signal_sys_reset +202 arch/powerpc/platforms/pseries/smp.c

196 xics_cause_ipi(cpu, data);
197 }
198
199 static int pseries_cause_nmi_ipi(int cpu, int type)
200 {
201 if (type == SMP_OP_NMI_TYPE_HARD) {

 > 202			if (plapr_signal_sys_reset(cpu) == H_SUCCESS)

203 return 1;
204 }
205

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation