[PATCH 0/3][RFC] powerpc: NMI IPIs

[PATCH 0/3][RFC] powerpc: NMI IPIs

[Nicholas Piggin]

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(-)

-- 
2.10.2

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

[Nicholas Piggin]

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(-)

diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 0d02c11..055918d 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -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
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 9c6f3fd..959d9dc 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -85,8 +85,6 @@ volatile unsigned int cpu_callin_map[NR_CPUS];
 
 int smt_enabled_at_boot = 1;
 
-static void (*crash_ipi_function_ptr)(struct pt_regs *) = NULL;
-
 /*
  * Returns 1 if the specified cpu should be brought up during boot.
  * Used to inhibit booting threads if they've been disabled or
@@ -157,17 +155,9 @@ static irqreturn_t tick_broadcast_ipi_action(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t debug_ipi_action(int irq, void *data)
+static irqreturn_t nmi_ipi_safe_action(int irq, void *data)
 {
-	if (crash_ipi_function_ptr) {
-		crash_ipi_function_ptr(get_irq_regs());
-		return IRQ_HANDLED;
-	}
-
-#ifdef CONFIG_DEBUGGER
-	debugger_ipi(get_irq_regs());
-#endif /* CONFIG_DEBUGGER */
-
+	smp_handle_nmi_ipi(get_irq_regs());
 	return IRQ_HANDLED;
 }
 
@@ -175,14 +165,14 @@ static irq_handler_t smp_ipi_action[] = {
 	[PPC_MSG_CALL_FUNCTION] =  call_function_action,
 	[PPC_MSG_RESCHEDULE] = reschedule_action,
 	[PPC_MSG_TICK_BROADCAST] = tick_broadcast_ipi_action,
-	[PPC_MSG_DEBUGGER_BREAK] = debug_ipi_action,
+	[PPC_MSG_NMI_IPI_SAFE] = nmi_ipi_safe_action,
 };
 
 const char *smp_ipi_name[] = {
 	[PPC_MSG_CALL_FUNCTION] =  "ipi call function",
 	[PPC_MSG_RESCHEDULE] = "ipi reschedule",
 	[PPC_MSG_TICK_BROADCAST] = "ipi tick-broadcast",
-	[PPC_MSG_DEBUGGER_BREAK] = "ipi debugger",
+	[PPC_MSG_NMI_IPI_SAFE] = "nmi ipi",
 };
 
 /* optional function to request ipi, for controllers with >= 4 ipis */
@@ -190,14 +180,9 @@ int smp_request_message_ipi(int virq, int msg)
 {
 	int err;
 
-	if (msg < 0 || msg > PPC_MSG_DEBUGGER_BREAK) {
+	if (msg < 0 || msg > PPC_MSG_NMI_IPI_SAFE) {
 		return -EINVAL;
 	}
-#if !defined(CONFIG_DEBUGGER) && !defined(CONFIG_KEXEC)
-	if (msg == PPC_MSG_DEBUGGER_BREAK) {
-		return 1;
-	}
-#endif
 	err = request_irq(virq, smp_ipi_action[msg],
 			  IRQF_PERCPU | IRQF_NO_THREAD | IRQF_NO_SUSPEND,
 			  smp_ipi_name[msg], NULL);
@@ -277,8 +262,8 @@ irqreturn_t smp_ipi_demux(void)
 			scheduler_ipi();
 		if (all & IPI_MESSAGE(PPC_MSG_TICK_BROADCAST))
 			tick_broadcast_ipi_handler();
-		if (all & IPI_MESSAGE(PPC_MSG_DEBUGGER_BREAK))
-			debug_ipi_action(0, NULL);
+		if (all & IPI_MESSAGE(PPC_MSG_NMI_IPI_SAFE))
+			nmi_ipi_safe_action(0, NULL);
 	} while (info->messages);
 
 	return IRQ_HANDLED;
@@ -315,6 +300,210 @@ void arch_send_call_function_ipi_mask(const struct cpumask *mask)
 		do_message_pass(cpu, PPC_MSG_CALL_FUNCTION);
 }
 
+/*
+ * "NMI IPI" infrastructure. The entire NMI call runs single threaded and with
+ * interrupts disabled, to avoid deadlocks and make message passing simpler.
+ * It runs synchronously until all targets enter the nmi handler (or the caller
+ * times out and fails), at which point the caller is released. No new NMI can
+ * be initiated until targets exit the handler.
+ *
+ * In the case of a caller timeout but the IPI subsequently being taken on a
+ * target, there is some logic to gracefully ignore the IPI, however platform
+ * code may not be able to distinguish this from a different source of NMI,
+ * so it may end up doing something like entering a debug mode.
+ *
+ * The NMI can be "safe" or "hard". Safe should not be dangerous to the system,
+ * but may not always succeed in interrupting the target. Hard is more likely
+ * to succeed but may not be recoverable afterwards, so must be used carefully.
+ */
+#define NMI_IPI_ALLBUTSELF		-1
+
+#define NMI_IPI_FUNCTION_STOP		1
+#define NMI_IPI_FUNCTION_DEBUG		2
+#define NMI_IPI_FUNCTION_CRASH		3
+
+static atomic_t __nmi_ipi_lock = ATOMIC_INIT(0);
+static struct cpumask nmi_ipi_pending_mask;
+static int nmi_ipi_busy_count = 0;
+static int nmi_ipi_function = 0;
+static void *nmi_ipi_data = NULL;
+
+static void nmi_ipi_lock_start(unsigned long *flags)
+{
+	raw_local_irq_save(*flags);
+	hard_irq_disable();
+	while (atomic_cmpxchg(&__nmi_ipi_lock, 0, 1) == 1) {
+		raw_local_irq_restore(*flags);
+		cpu_relax();
+		raw_local_irq_save(*flags);
+		hard_irq_disable();
+	}
+}
+
+static void nmi_ipi_lock(void)
+{
+	while (atomic_cmpxchg(&__nmi_ipi_lock, 0, 1) == 1)
+		cpu_relax();
+}
+
+static void nmi_ipi_unlock(void)
+{
+	smp_mb();
+	WARN_ON(atomic_read(&__nmi_ipi_lock) != 1);
+	atomic_set(&__nmi_ipi_lock, 0);
+}
+
+static void nmi_ipi_unlock_end(unsigned long *flags)
+{
+	nmi_ipi_unlock();
+	raw_local_irq_restore(*flags);
+}
+
+static void __noreturn stop_this_cpu(void)
+{
+	/* Remove this CPU */
+	set_cpu_online(smp_processor_id(), false);
+
+	local_irq_disable();
+	hard_irq_disable();
+	while (1)
+		cpu_relax();
+}
+
+/*
+ * Platform NMI handler calls this to ack
+ */
+int smp_handle_nmi_ipi(struct pt_regs *regs)
+{
+	unsigned long flags;
+	int me = raw_smp_processor_id();
+	int ret = 0;
+
+	/*
+	 * Unexpected NMIs are possible here because the interrupt may not
+	 * be able to distinguish NMI IPIs from other types of NMIs, or
+	 * because the caller may have timed out.
+	 */
+	nmi_ipi_lock_start(&flags);
+	if (!nmi_ipi_busy_count)
+		goto out;
+	if (!cpumask_test_cpu(me, &nmi_ipi_pending_mask))
+		goto out;
+
+	cpumask_clear_cpu(me, &nmi_ipi_pending_mask);
+	nmi_ipi_busy_count++;
+	nmi_ipi_unlock();
+
+	ret = 1;
+	if (nmi_ipi_function == NMI_IPI_FUNCTION_STOP) {
+		nmi_ipi_lock();
+		nmi_ipi_busy_count--;
+		nmi_ipi_unlock_end(&flags);
+
+		stop_this_cpu();
+
+#ifdef CONFIG_DEBUGGER
+	} else if (nmi_ipi_function == NMI_IPI_FUNCTION_DEBUG) {
+		debugger_ipi(regs);
+#endif
+
+#ifdef CONFIG_KEXEC
+	} else if (nmi_ipi_function == NMI_IPI_FUNCTION_CRASH) {
+		void (*crash_fn)(struct pt_regs *) = nmi_ipi_data;
+		crash_fn(regs);
+#endif
+
+	} else {
+		pr_warn("Unknown NMI IPI on cpu:%d\n", me);
+	}
+
+	nmi_ipi_lock();
+	nmi_ipi_busy_count--;
+out:
+	nmi_ipi_unlock_end(&flags);
+
+	return ret;
+}
+
+static void do_smp_send_nmi_ipi(int cpu)
+{
+	do_message_pass(cpu, PPC_MSG_NMI_IPI_SAFE);
+}
+
+/*
+ * - cpu is the target CPU, can be NMI_IPI_ALLBUTSELF. Must not be current CPU.
+ * - function is one of NMI_IPI_FUNCTION_.
+ * - data is for the function handler to use.
+ * - safe_udelay > 0 if the "safe" NMI is to be used, specifies delay before
+ *   giving up waiting for targets to enter the handler.
+ * - hard_udelay > 0 similarly to use "hard" NMI. If both are > 0, safe is
+ *   attempted first, then hard. If the platform does not support hard, then
+ *   safe will be used.
+ */
+int smp_send_nmi_ipi(int cpu, int function, void *data,
+		u64 safe_udelay, u64 hard_udelay)
+{
+	unsigned long flags;
+	int c, me = raw_smp_processor_id();
+	int ret = 1;
+
+	BUG_ON(cpu == me);
+	BUG_ON(cpu < 0 && cpu != NMI_IPI_ALLBUTSELF);
+
+	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 */
+	nmi_ipi_lock_start(&flags);
+	while (nmi_ipi_busy_count) {
+		nmi_ipi_unlock_end(&flags);
+		cpu_relax();
+		nmi_ipi_lock_start(&flags);
+	}
+
+	nmi_ipi_function = function;
+	nmi_ipi_data = data;
+
+	if (cpu < 0) {
+		/* ALLBUTSELF */
+		cpumask_copy(&nmi_ipi_pending_mask, cpu_online_mask);
+		cpumask_clear_cpu(me, &nmi_ipi_pending_mask);
+	} else {
+		/* cpumask starts clear */
+		cpumask_set_cpu(cpu, &nmi_ipi_pending_mask);
+	}
+	nmi_ipi_busy_count++;
+	nmi_ipi_unlock();
+
+	if (safe_udelay) {
+		for_each_cpu(c, &nmi_ipi_pending_mask)
+			do_smp_send_nmi_ipi(c);
+
+		do {
+			safe_udelay--;
+			udelay(1);
+			if (cpumask_empty(&nmi_ipi_pending_mask))
+				goto done;
+		} while (safe_udelay);
+	}
+
+	ret = 0; /* Could not gather all CPUs */
+	cpumask_clear(&nmi_ipi_pending_mask);
+done:
+	nmi_ipi_lock();
+	nmi_ipi_busy_count--;
+	nmi_ipi_unlock_end(&flags);
+
+	put_online_cpus();
+
+	return ret;
+}
+
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 void tick_broadcast(const struct cpumask *mask)
 {
@@ -325,45 +514,31 @@ void tick_broadcast(const struct cpumask *mask)
 }
 #endif
 
-#if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC)
+#ifdef CONFIG_DEBUGGER
 void smp_send_debugger_break(void)
 {
-	int cpu;
-	int me = raw_smp_processor_id();
-
-	if (unlikely(!smp_ops))
-		return;
-
-	for_each_online_cpu(cpu)
-		if (cpu != me)
-			do_message_pass(cpu, PPC_MSG_DEBUGGER_BREAK);
+	smp_send_nmi_ipi(NMI_IPI_ALLBUTSELF,
+			NMI_IPI_FUNCTION_DEBUG, NULL,
+			1000000, 0);
 }
 #endif
 
 #ifdef CONFIG_KEXEC
 void crash_send_ipi(void (*crash_ipi_callback)(struct pt_regs *))
 {
-	crash_ipi_function_ptr = crash_ipi_callback;
-	if (crash_ipi_callback) {
-		mb();
-		smp_send_debugger_break();
-	}
+	smp_send_nmi_ipi(NMI_IPI_ALLBUTSELF,
+			NMI_IPI_FUNCTION_CRASH, crash_ipi_callback,
+			0, 1000000);
 }
 #endif
 
-static void stop_this_cpu(void *dummy)
-{
-	/* Remove this CPU */
-	set_cpu_online(smp_processor_id(), false);
-
-	local_irq_disable();
-	while (1)
-		;
-}
-
 void smp_send_stop(void)
 {
-	smp_call_function(stop_this_cpu, NULL, 0);
+	smp_send_nmi_ipi(NMI_IPI_ALLBUTSELF,
+			NMI_IPI_FUNCTION_STOP, NULL,
+			0, 1000000);
+
+	stop_this_cpu();
 }
 
 struct thread_info *current_set[NR_CPUS];
diff --git a/arch/powerpc/platforms/cell/interrupt.c b/arch/powerpc/platforms/cell/interrupt.c
index a6bbbab..fcc94c8 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -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 */
diff --git a/arch/powerpc/platforms/ps3/smp.c b/arch/powerpc/platforms/ps3/smp.c
index 60154d0..5b1fb70 100644
--- a/arch/powerpc/platforms/ps3/smp.c
+++ b/arch/powerpc/platforms/ps3/smp.c
@@ -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);
 	}
-- 
2.10.2

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

[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(-)

diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 055918d..15521c1 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -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);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 959d9dc..b3133e9 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -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:
diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c
index fe9f19e..cde7beb 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -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
diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
index af09bae..020e84a 100644
--- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
+++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
@@ -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,
diff --git a/arch/powerpc/platforms/chrp/smp.c b/arch/powerpc/platforms/chrp/smp.c
index b6c9a0d..1451504 100644
--- a/arch/powerpc/platforms/chrp/smp.c
+++ b/arch/powerpc/platforms/chrp/smp.c
@@ -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,
diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index c9eb7d6..1d76e15 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -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,
diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index c789258..092ec1f 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -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,
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index f6f83ae..0f6522c 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -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,
-- 
2.10.2

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

[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(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 023a462..d1d7fd4 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -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
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 904a677..bb70b26 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -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 */
 }
 
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 0f6522c..4534c5a 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -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,
-- 
2.10.2

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1658 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22506 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22506 bytes --]

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

[Alistair Popple]

On Wed, 9 Nov 2016 01:01:24 AM Nicholas Piggin wrote:
> 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(-)
> 
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 055918d..15521c1 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -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);
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 959d9dc..b3133e9 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -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) {

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.

> +		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

> +			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:
> diff --git a/arch/powerpc/platforms/85xx/smp.c 
b/arch/powerpc/platforms/85xx/smp.c
> index fe9f19e..cde7beb 100644
> --- a/arch/powerpc/platforms/85xx/smp.c
> +++ b/arch/powerpc/platforms/85xx/smp.c
> @@ -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
> diff --git a/arch/powerpc/platforms/86xx/mpc86xx_smp.c 
b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> index af09bae..020e84a 100644
> --- a/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> +++ b/arch/powerpc/platforms/86xx/mpc86xx_smp.c
> @@ -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,
> diff --git a/arch/powerpc/platforms/chrp/smp.c 
b/arch/powerpc/platforms/chrp/smp.c
> index b6c9a0d..1451504 100644
> --- a/arch/powerpc/platforms/chrp/smp.c
> +++ b/arch/powerpc/platforms/chrp/smp.c
> @@ -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,
> diff --git a/arch/powerpc/platforms/powermac/smp.c 
b/arch/powerpc/platforms/powermac/smp.c
> index c9eb7d6..1d76e15 100644
> --- a/arch/powerpc/platforms/powermac/smp.c
> +++ b/arch/powerpc/platforms/powermac/smp.c
> @@ -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,
> diff --git a/arch/powerpc/platforms/powernv/smp.c 
b/arch/powerpc/platforms/powernv/smp.c
> index c789258..092ec1f 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -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,
> diff --git a/arch/powerpc/platforms/pseries/smp.c 
b/arch/powerpc/platforms/pseries/smp.c
> index f6f83ae..0f6522c 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -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,
> 

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

[Nicholas Piggin]

On Fri, 11 Nov 2016 12:00:14 +1100
Alistair Popple <alistair@popple.id.au> wrote:

> On Wed, 9 Nov 2016 01:01:24 AM Nicholas Piggin wrote:
> > 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(-)
> > 
> > diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> > index 055918d..15521c1 100644
> > --- a/arch/powerpc/include/asm/smp.h
> > +++ b/arch/powerpc/include/asm/smp.h
> > @@ -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);
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 959d9dc..b3133e9 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -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) {  
> 
> 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.

> 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.

> 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?

> > +		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