2018-05-21 13:22:53

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH] powerpc/xmon: really enable xmon when a breakpoint is set

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 <[email protected]>
---
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
@@ -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);
}
}

--
2.13.6



2018-05-22 07:25:19

by Vaibhav Jain

[permalink] [raw]
Subject: Re: [PATCH] powerpc/xmon: really enable xmon when a breakpoint is set

Thanks for the patch Michal,

Michal Suchanek <[email protected]> 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.


--
Vaibhav Jain <[email protected]>
Linux Technology Center, IBM India Pvt. Ltd.


2018-05-23 17:59:18

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH] powerpc/xmon: really enable xmon when a breakpoint is set

On Tue, 22 May 2018 12:53:53 +0530
Vaibhav Jain <[email protected]> wrote:

> Thanks for the patch Michal,
>
> Michal Suchanek <[email protected]> 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.
>
>