| Message ID | 20180521132108.7938-1-msuchanek@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/xmon: really enable xmon when a breakpoint is set | expand |
Thanks for the patch Michal, Michal Suchanek <msuchanek@suse.de> writes: > When single-stepping kernel code from xmon without a debug hook enabled > the kernel crashes. This can happen when kernel starts with xmon on > crash disabled but xmon is entered using sysrq. > > Commit e1368d0c9edb ("powerpc/xmon: Setup debugger hooks when first > break-point is set") adds force_enable_xmon function that prints > "xmon: Enabling debugger hooks" but does not enable them. Debugger hooks are enabled just befores debugger() is entered from sysrq_handle_xmon(). Thats why force_enable_xmon() simply sets sets 'xmon_on=1' and exits. The problem you are seeing is probably due to sysrq_handle_xmon() clearing the debugger hooks on return from debugger() as xmon_on was never set for the 's' xmon command. > Add the call to xmon_init to install the debugger hooks in > force_enable_xmon and also call force_enable_xmon when single-stepping > in xmon. Only calling force_enable_xmon() from do_step() should be suffice as on exit from the debugger() the value of xmon_on is checked and if required the debugger hooks are kept instead of getting cleared. > arch/powerpc/xmon/xmon.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index a0842f1ff72c..504bd1c3d8b0 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -1275,6 +1279,7 @@ static inline void force_enable_xmon(void) > if (!xmon_on) { > printf("xmon: Enabling debugger hooks\n"); > xmon_on = 1; > + xmon_init(1); > } > } As mentioned above call to force_enable_xmon() is usually done in context of sysrq_handle_xmon() which sets the debugger hooks as soon as its entered. So I think that this hunk is not really needed.
On Tue, 22 May 2018 12:53:53 +0530 Vaibhav Jain <vaibhav@linux.vnet.ibm.com> wrote: > Thanks for the patch Michal, > > Michal Suchanek <msuchanek@suse.de> writes: > > > When single-stepping kernel code from xmon without a debug hook > > enabled the kernel crashes. This can happen when kernel starts with > > xmon on crash disabled but xmon is entered using sysrq. > > > > Commit e1368d0c9edb ("powerpc/xmon: Setup debugger hooks when first > > break-point is set") adds force_enable_xmon function that prints > > "xmon: Enabling debugger hooks" but does not enable them. > Debugger hooks are enabled just befores debugger() is entered from > sysrq_handle_xmon(). Thats why force_enable_xmon() simply sets sets > 'xmon_on=1' and exits. > > The problem you are seeing is probably due to sysrq_handle_xmon() > clearing the debugger hooks on return from debugger() as xmon_on was > never set for the 's' xmon command. Indeed, setting xmon_on is sufficient. Will resend the patch. Thanks Michal > > > Add the call to xmon_init to install the debugger hooks in > > force_enable_xmon and also call force_enable_xmon when > > single-stepping in xmon. > Only calling force_enable_xmon() from do_step() should be suffice as > on exit from the debugger() the value of xmon_on is checked and if > required the debugger hooks are kept instead of getting cleared. > > > > arch/powerpc/xmon/xmon.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > > index a0842f1ff72c..504bd1c3d8b0 100644 > > --- a/arch/powerpc/xmon/xmon.c > > +++ b/arch/powerpc/xmon/xmon.c > > > > @@ -1275,6 +1279,7 @@ static inline void force_enable_xmon(void) > > if (!xmon_on) { > > printf("xmon: Enabling debugger hooks\n"); > > xmon_on = 1; > > + xmon_init(1); > > } > > } > > As mentioned above call to force_enable_xmon() is usually done in > context of sysrq_handle_xmon() which sets the debugger hooks as soon > as its entered. So I think that this hunk is not really needed. > >
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index a0842f1ff72c..504bd1c3d8b0 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -179,6 +179,9 @@ static const char *getvecname(unsigned long vec); static int do_spu_cmd(void); +static void xmon_init(int enable); +static inline void force_enable_xmon(void); + #ifdef CONFIG_44x static void dump_tlb_44x(void); #endif @@ -1094,6 +1097,7 @@ static int do_step(struct pt_regs *regs) unsigned int instr; int stepped; + force_enable_xmon(); /* check we are in 64-bit kernel mode, translation enabled */ if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR)) { if (mread(regs->nip, &instr, 4) == 4) { @@ -1275,6 +1279,7 @@ static inline void force_enable_xmon(void) if (!xmon_on) { printf("xmon: Enabling debugger hooks\n"); xmon_on = 1; + xmon_init(1); } }
When single-stepping kernel code from xmon without a debug hook enabled the kernel crashes. This can happen when kernel starts with xmon on crash disabled but xmon is entered using sysrq. Commit e1368d0c9edb ("powerpc/xmon: Setup debugger hooks when first break-point is set") adds force_enable_xmon function that prints "xmon: Enabling debugger hooks" but does not enable them. Add the call to xmon_init to install the debugger hooks in force_enable_xmon and also call force_enable_xmon when single-stepping in xmon. Fixes: e1368d0c9edb ("powerpc/xmon: Setup debugger hooks when first break-point is set") Signed-off-by: Michal Suchanek <msuchanek@suse.de> --- arch/powerpc/xmon/xmon.c | 5 +++++ 1 file changed, 5 insertions(+)