powerpc: NMI IPIs
Subsystems (5)
Nicholas Piggin
reviewercover 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
Nicholas Piggin
reviewerPATCH 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
@@ -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
@@ -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
@@ -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
kbuild test robot
build botRe: [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
Nicholas Piggin
reviewerPATCH 2/3 powerpc: add struct smp_ops_t.cause_nmi_ipi operation
In reply to Nicholas PigginAllow 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
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@@ -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@@ -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,
Nicholas Piggin
reviewerRe: [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@@ -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@@ -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_CALLsupport 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_maskso 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
Nicholas Piggin
reviewerPATCH 3/3 powerpc/pseries: implement nmi ipi with H_SIGNAL_SYS_RESET
In reply to Nicholas PigginH_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
@@ -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
@@ -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
@@ -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
kbuild test robot
build botRe: [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