| Message ID | 20200524093822.423487-1-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a6e2c226c3d51fd93636320e47cabc8a8f0824c5 |
| Headers | show |
| Series | [v2,1/4] powerpc/instruction_dump: Fix kernel crash with show_instructions | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (1ef93962cf4293ec9e1bb3163cc4b7dcfc3de84f) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 41 lines checked |
| snowpatch_ozlabs/needsstable | warning | Please consider tagging this patch for stable! |
On Sun, 24 May 2020 15:08:19 +0530, Aneesh Kumar K.V wrote: > With Hard Lockup watchdog, we can hit a BUG() if we take a watchdog > interrupt when in OPAL mode. This happens in show_instructions() > where the kernel takes the watchdog NMI IPI with MSR_IR == 0. > With that show_instructions() updates the variable pc in the loop > and the second iterations will result in BUG(). > > We hit the BUG_ON due the below check in __va() > > [...] Patch 1 applied to powerpc/fixes. [1/4] powerpc: Fix kernel crash in show_instructions() w/DEBUG_VIRTUAL https://git.kernel.org/powerpc/c/a6e2c226c3d51fd93636320e47cabc8a8f0824c5 cheers
Le 24/05/2020 à 11:38, Aneesh Kumar K.V a écrit : > With Hard Lockup watchdog, we can hit a BUG() if we take a watchdog > interrupt when in OPAL mode. This happens in show_instructions() > where the kernel takes the watchdog NMI IPI with MSR_IR == 0. > With that show_instructions() updates the variable pc in the loop > and the second iterations will result in BUG(). > > We hit the BUG_ON due the below check in __va() > > #define __va(x) \ > ({ \ > VIRTUAL_BUG_ON((unsigned long)(x) >= PAGE_OFFSET); \ > (void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET); \ > }) > > Fixes: 4dd7554a6456 ("powerpc/64: Add VIRTUAL_BUG_ON checks for __va and __pa addresses") > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> If still relevant, this series needs rebase. Patch 1 was applied with changes, at least patch 3 won't apply Thanks Christophe > --- > arch/powerpc/kernel/process.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index 048d64c4e115..93bf4a766707 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -1253,29 +1253,32 @@ struct task_struct *__switch_to(struct task_struct *prev, > static void show_instructions(struct pt_regs *regs) > { > int i; > + unsigned long nip = regs->nip; > unsigned long pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int)); > > printk("Instruction dump:"); > > +#if !defined(CONFIG_BOOKE) > + /* If executing with the IMMU off, adjust pc rather > + * than print XXXXXXXX. > + */ > + if (!(regs->msr & MSR_IR)) { > + pc = (unsigned long)phys_to_virt(pc); > + nip = (unsigned long)phys_to_virt(regs->nip); > + } > +#endif > + > for (i = 0; i < NR_INSN_TO_PRINT; i++) { > int instr; > > if (!(i % 8)) > pr_cont("\n"); > > -#if !defined(CONFIG_BOOKE) > - /* If executing with the IMMU off, adjust pc rather > - * than print XXXXXXXX. > - */ > - if (!(regs->msr & MSR_IR)) > - pc = (unsigned long)phys_to_virt(pc); > -#endif > - > if (!__kernel_text_address(pc) || > probe_kernel_address((const void *)pc, instr)) { > pr_cont("XXXXXXXX "); > } else { > - if (regs->nip == pc) > + if (nip == pc) > pr_cont("<%08x> ", instr); > else > pr_cont("%08x ", instr);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 048d64c4e115..93bf4a766707 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -1253,29 +1253,32 @@ struct task_struct *__switch_to(struct task_struct *prev, static void show_instructions(struct pt_regs *regs) { int i; + unsigned long nip = regs->nip; unsigned long pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int)); printk("Instruction dump:"); +#if !defined(CONFIG_BOOKE) + /* If executing with the IMMU off, adjust pc rather + * than print XXXXXXXX. + */ + if (!(regs->msr & MSR_IR)) { + pc = (unsigned long)phys_to_virt(pc); + nip = (unsigned long)phys_to_virt(regs->nip); + } +#endif + for (i = 0; i < NR_INSN_TO_PRINT; i++) { int instr; if (!(i % 8)) pr_cont("\n"); -#if !defined(CONFIG_BOOKE) - /* If executing with the IMMU off, adjust pc rather - * than print XXXXXXXX. - */ - if (!(regs->msr & MSR_IR)) - pc = (unsigned long)phys_to_virt(pc); -#endif - if (!__kernel_text_address(pc) || probe_kernel_address((const void *)pc, instr)) { pr_cont("XXXXXXXX "); } else { - if (regs->nip == pc) + if (nip == pc) pr_cont("<%08x> ", instr); else pr_cont("%08x ", instr);
With Hard Lockup watchdog, we can hit a BUG() if we take a watchdog interrupt when in OPAL mode. This happens in show_instructions() where the kernel takes the watchdog NMI IPI with MSR_IR == 0. With that show_instructions() updates the variable pc in the loop and the second iterations will result in BUG(). We hit the BUG_ON due the below check in __va() #define __va(x) \ ({ \ VIRTUAL_BUG_ON((unsigned long)(x) >= PAGE_OFFSET); \ (void *)(unsigned long)((phys_addr_t)(x) | PAGE_OFFSET); \ }) Fixes: 4dd7554a6456 ("powerpc/64: Add VIRTUAL_BUG_ON checks for __va and __pa addresses") Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- arch/powerpc/kernel/process.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)