| Message ID | 164578465828.74956.6065296024817333750.stgit@jupiter (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | powerpc: Avoid nmi_enter/nmi_exit in real mode interrupt. | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_kernel_qemu | fail | 3 of 20 jobs failed. |
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | fail | sparse (pmac32, ubuntu-21.04, ppc64) failed at step Build. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | fail | 4 of 7 jobs failed. |
Le 25/02/2022 à 11:24, Mahesh Salgaonkar a écrit : > nmi_enter()/nmi_exit() touches per cpu variables which can lead to kernel > crash when invoked during real mode interrupt handling (e.g. early HMI/MCE > interrupt handler) if percpu allocation comes from vmalloc area. > > Early HMI/MCE handlers are called through DEFINE_INTERRUPT_HANDLER_NMI() > wrapper which invokes nmi_enter/nmi_exit calls. We don't see any issue when > percpu allocation is from the embedded first chunk. However with > CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK enabled there are chances where percpu > allocation can come from the vmalloc area. > > With kernel command line "percpu_alloc=page" we can force percpu allocation > to come from vmalloc area and can see kernel crash in machine_check_early: > > [ 1.215714] NIP [c000000000e49eb4] rcu_nmi_enter+0x24/0x110 > [ 1.215717] LR [c0000000000461a0] machine_check_early+0xf0/0x2c0 > [ 1.215719] --- interrupt: 200 > [ 1.215720] [c000000fffd73180] [0000000000000000] 0x0 (unreliable) > [ 1.215722] [c000000fffd731b0] [0000000000000000] 0x0 > [ 1.215724] [c000000fffd73210] [c000000000008364] machine_check_early_common+0x134/0x1f8 > > Fix this by avoiding use of nmi_enter()/nmi_exit() in real mode if percpu > first chunk is not embedded. > > Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> > --- > arch/powerpc/include/asm/interrupt.h | 15 +++++++++++++++ > arch/powerpc/include/asm/percpu.h | 2 ++ > arch/powerpc/kernel/setup_64.c | 3 +++ > 3 files changed, 20 insertions(+) > > diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h > index fc28f46d2f9dc..8c5b3a7ca2ab6 100644 > --- a/arch/powerpc/include/asm/interrupt.h > +++ b/arch/powerpc/include/asm/interrupt.h > @@ -327,6 +327,16 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte > } > #endif > > + /* > + * Do not use nmi_enter() in real mode if percpu first chunk is > + * not embedded. With CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK enabled > + * there are chances where percpu allocation can come from vmalloc > + * area. > + */ > + if (IS_ENABLED(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) && > + !(mfmsr() & MSR_DR) && !__percpu_embed_first_chunk) > + return; > + It would probably be better to refactor with the following test and avoid calling mfmsr() twice. > /* > * Do not use nmi_enter() for pseries hash guest taking a real-mode > * NMI because not everything it touches is within the RMA limit. > @@ -339,6 +349,10 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte > > static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct interrupt_nmi_state *state) > { > + if (IS_ENABLED(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) && > + !(mfmsr() & MSR_DR) && !__percpu_embed_first_chunk) > + goto skip_nmi_exit; > + > if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64) || > !firmware_has_feature(FW_FEATURE_LPAR) || > radix_enabled() || (mfmsr() & MSR_DR)) Same here, if you can avoid calling mfmsr() two times that would be better. > @@ -349,6 +363,7 @@ static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct inter > * new work to do (must use irq_work for that). > */ > > +skip_nmi_exit: > #ifdef CONFIG_PPC64 > #ifdef CONFIG_PPC_BOOK3S > if (arch_irq_disabled_regs(regs)) { > diff --git a/arch/powerpc/include/asm/percpu.h b/arch/powerpc/include/asm/percpu.h > index 8e5b7d0b851c6..45c08f5b0b4e9 100644 > --- a/arch/powerpc/include/asm/percpu.h > +++ b/arch/powerpc/include/asm/percpu.h > @@ -12,6 +12,8 @@ > > #define __my_cpu_offset local_paca->data_offset > > +extern bool __percpu_embed_first_chunk; > + > #endif /* CONFIG_SMP */ > #endif /* __powerpc64__ */ > > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c > index be8577ac93971..39dbf2fb23d61 100644 > --- a/arch/powerpc/kernel/setup_64.c > +++ b/arch/powerpc/kernel/setup_64.c > @@ -786,6 +786,7 @@ static __init int pcpu_cpu_to_node(int cpu) > > unsigned long __per_cpu_offset[NR_CPUS] __read_mostly; > EXPORT_SYMBOL(__per_cpu_offset); > +bool __percpu_embed_first_chunk; > > void __init setup_per_cpu_areas(void) > { > @@ -821,6 +822,8 @@ void __init setup_per_cpu_areas(void) > pr_warn("PERCPU: %s allocator failed (%d), " > "falling back to page size\n", > pcpu_fc_names[pcpu_chosen_fc], rc); > + else > + __percpu_embed_first_chunk = true; If I understand correctly that happens once at system startup ? Then could you use jump_labels in order to avoid redoing the test at each interrupt entry ? > } > > if (rc < 0) > >
Le 25/02/2022 à 11:24, Mahesh Salgaonkar a écrit : > nmi_enter()/nmi_exit() touches per cpu variables which can lead to kernel > crash when invoked during real mode interrupt handling (e.g. early HMI/MCE > interrupt handler) if percpu allocation comes from vmalloc area. > > Early HMI/MCE handlers are called through DEFINE_INTERRUPT_HANDLER_NMI() > wrapper which invokes nmi_enter/nmi_exit calls. We don't see any issue when > percpu allocation is from the embedded first chunk. However with > CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK enabled there are chances where percpu > allocation can come from the vmalloc area. > > With kernel command line "percpu_alloc=page" we can force percpu allocation > to come from vmalloc area and can see kernel crash in machine_check_early: > > [ 1.215714] NIP [c000000000e49eb4] rcu_nmi_enter+0x24/0x110 > [ 1.215717] LR [c0000000000461a0] machine_check_early+0xf0/0x2c0 > [ 1.215719] --- interrupt: 200 > [ 1.215720] [c000000fffd73180] [0000000000000000] 0x0 (unreliable) > [ 1.215722] [c000000fffd731b0] [0000000000000000] 0x0 > [ 1.215724] [c000000fffd73210] [c000000000008364] machine_check_early_common+0x134/0x1f8 > > Fix this by avoiding use of nmi_enter()/nmi_exit() in real mode if percpu > first chunk is not embedded. > > Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> > --- > arch/powerpc/include/asm/interrupt.h | 15 +++++++++++++++ > arch/powerpc/include/asm/percpu.h | 2 ++ > arch/powerpc/kernel/setup_64.c | 3 +++ > 3 files changed, 20 insertions(+) In file included from /linux/arch/powerpc/kernel/irq.c:57:0: /linux/arch/powerpc/include/asm/interrupt.h: In function 'interrupt_nmi_enter_prepare': /linux/arch/powerpc/include/asm/interrupt.h:337:109: error: '__percpu_embed_first_chunk' undeclared (first use in this function) /linux/arch/powerpc/include/asm/interrupt.h:337:109: note: each undeclared identifier is reported only once for each function it appears in In file included from /linux/arch/powerpc/kernel/irq.c:57:0: /linux/arch/powerpc/include/asm/interrupt.h: In function 'interrupt_nmi_exit_prepare': /linux/arch/powerpc/include/asm/interrupt.h:353:109: error: '__percpu_embed_first_chunk' undeclared (first use in this function) In file included from /linux/arch/powerpc/kernel/irq.c:57:0: /linux/arch/powerpc/include/asm/interrupt.h:366:1: error: label at end of compound statement skip_nmi_exit: ^ /linux/scripts/Makefile.build:288: recipe for target 'arch/powerpc/kernel/irq.o' failed make[3]: *** [arch/powerpc/kernel/irq.o] Error 1
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h index fc28f46d2f9dc..8c5b3a7ca2ab6 100644 --- a/arch/powerpc/include/asm/interrupt.h +++ b/arch/powerpc/include/asm/interrupt.h @@ -327,6 +327,16 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte } #endif + /* + * Do not use nmi_enter() in real mode if percpu first chunk is + * not embedded. With CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK enabled + * there are chances where percpu allocation can come from vmalloc + * area. + */ + if (IS_ENABLED(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) && + !(mfmsr() & MSR_DR) && !__percpu_embed_first_chunk) + return; + /* * Do not use nmi_enter() for pseries hash guest taking a real-mode * NMI because not everything it touches is within the RMA limit. @@ -339,6 +349,10 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct interrupt_nmi_state *state) { + if (IS_ENABLED(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) && + !(mfmsr() & MSR_DR) && !__percpu_embed_first_chunk) + goto skip_nmi_exit; + if (!IS_ENABLED(CONFIG_PPC_BOOK3S_64) || !firmware_has_feature(FW_FEATURE_LPAR) || radix_enabled() || (mfmsr() & MSR_DR)) @@ -349,6 +363,7 @@ static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct inter * new work to do (must use irq_work for that). */ +skip_nmi_exit: #ifdef CONFIG_PPC64 #ifdef CONFIG_PPC_BOOK3S if (arch_irq_disabled_regs(regs)) { diff --git a/arch/powerpc/include/asm/percpu.h b/arch/powerpc/include/asm/percpu.h index 8e5b7d0b851c6..45c08f5b0b4e9 100644 --- a/arch/powerpc/include/asm/percpu.h +++ b/arch/powerpc/include/asm/percpu.h @@ -12,6 +12,8 @@ #define __my_cpu_offset local_paca->data_offset +extern bool __percpu_embed_first_chunk; + #endif /* CONFIG_SMP */ #endif /* __powerpc64__ */ diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index be8577ac93971..39dbf2fb23d61 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -786,6 +786,7 @@ static __init int pcpu_cpu_to_node(int cpu) unsigned long __per_cpu_offset[NR_CPUS] __read_mostly; EXPORT_SYMBOL(__per_cpu_offset); +bool __percpu_embed_first_chunk; void __init setup_per_cpu_areas(void) { @@ -821,6 +822,8 @@ void __init setup_per_cpu_areas(void) pr_warn("PERCPU: %s allocator failed (%d), " "falling back to page size\n", pcpu_fc_names[pcpu_chosen_fc], rc); + else + __percpu_embed_first_chunk = true; } if (rc < 0)
nmi_enter()/nmi_exit() touches per cpu variables which can lead to kernel crash when invoked during real mode interrupt handling (e.g. early HMI/MCE interrupt handler) if percpu allocation comes from vmalloc area. Early HMI/MCE handlers are called through DEFINE_INTERRUPT_HANDLER_NMI() wrapper which invokes nmi_enter/nmi_exit calls. We don't see any issue when percpu allocation is from the embedded first chunk. However with CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK enabled there are chances where percpu allocation can come from the vmalloc area. With kernel command line "percpu_alloc=page" we can force percpu allocation to come from vmalloc area and can see kernel crash in machine_check_early: [ 1.215714] NIP [c000000000e49eb4] rcu_nmi_enter+0x24/0x110 [ 1.215717] LR [c0000000000461a0] machine_check_early+0xf0/0x2c0 [ 1.215719] --- interrupt: 200 [ 1.215720] [c000000fffd73180] [0000000000000000] 0x0 (unreliable) [ 1.215722] [c000000fffd731b0] [0000000000000000] 0x0 [ 1.215724] [c000000fffd73210] [c000000000008364] machine_check_early_common+0x134/0x1f8 Fix this by avoiding use of nmi_enter()/nmi_exit() in real mode if percpu first chunk is not embedded. Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> --- arch/powerpc/include/asm/interrupt.h | 15 +++++++++++++++ arch/powerpc/include/asm/percpu.h | 2 ++ arch/powerpc/kernel/setup_64.c | 3 +++ 3 files changed, 20 insertions(+)