* [PATCH] xmon: Setup xmon debugger hooks when first break-point is set
@ 2018-02-26 11:36 Vaibhav Jain
2018-02-27 0:26 ` Balbir Singh
2018-03-01 3:00 ` Michael Ellerman
0 siblings, 2 replies; 3+ messages in thread
From: Vaibhav Jain @ 2018-02-26 11:36 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
Cc: Vaibhav Jain, Benjamin Herrenschmidt, Paul Mackerras,
Balbir Singh, Nicholas Piggin, Douglas Miller, Frederic Barrat
Presently sysrq key for xmon('x') is registered during kernel init
irrespective of the value of kernel param 'xmon'. Thus xmon is enabled
even if 'xmon=off' is passed on the kernel command line. However this
doesn't enable the kernel debugger hooks needed for instruction or data
breakpoints. Thus when a break-point is hit with xmon=off a kernel oops
of the form below is reported:
Oops: Exception in kernel mode, sig: 5 [#1]
< snip >
Trace/breakpoint trap
To fix this the patch checks and enables debugger hooks when an
instruction or data break-point is set via xmon console. It also clears
all breakpoints when xmon is disabled via debugfs.
Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
---
arch/powerpc/xmon/xmon.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 82e1a3ee6e0f..3679f5417a7e 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1295,6 +1295,7 @@ bpt_cmds(void)
switch (cmd) {
#ifndef CONFIG_PPC_8xx
static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n";
+ static const char warnxmon[] = "xmon: Enabling debugger hooks\n";
int mode;
case 'd': /* bd - hardware data breakpoint */
mode = 7;
@@ -1315,6 +1316,11 @@ bpt_cmds(void)
dabr.address &= ~HW_BRK_TYPE_DABR;
dabr.enabled = mode | BP_DABR;
}
+ /* Enable xmon hooks if needed */
+ if (!xmon_on) {
+ printf(warnxmon);
+ xmon_on = 1;
+ }
break;
case 'i': /* bi - hardware instr breakpoint */
@@ -1335,6 +1341,12 @@ bpt_cmds(void)
if (bp != NULL) {
bp->enabled |= BP_CIABR;
iabr = bp;
+
+ /* Enable xmon hooks if needed */
+ if (!xmon_on) {
+ printf(warnxmon);
+ xmon_on = 1;
+ }
}
break;
#endif
@@ -1399,8 +1411,15 @@ bpt_cmds(void)
if (!check_bp_loc(a))
break;
bp = new_breakpoint(a);
- if (bp != NULL)
+ if (bp != NULL) {
bp->enabled |= BP_TRAP;
+
+ /* Enable xmon hooks if needed */
+ if (!xmon_on) {
+ printf(warnxmon);
+ xmon_on = 1;
+ }
+ }
break;
}
}
@@ -3651,9 +3670,22 @@ device_initcall(setup_xmon_sysrq);
#ifdef CONFIG_DEBUG_FS
static int xmon_dbgfs_set(void *data, u64 val)
{
+ int i;
+
xmon_on = !!val;
xmon_init(xmon_on);
+ /* make sure all breakpoints removed when disabling */
+ if (!xmon_on) {
+ for (i = 0; i < NBPTS; ++i)
+ bpts[i].enabled = 0;
+ /* if anything set inform user that breakpoints are cleared */
+ if (iabr || dabr.enabled)
+ pr_info("xmon: All breakpoints cleared\n");
+
+ iabr = NULL;
+ dabr.enabled = 0;
+ }
return 0;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] xmon: Setup xmon debugger hooks when first break-point is set
2018-02-26 11:36 [PATCH] xmon: Setup xmon debugger hooks when first break-point is set Vaibhav Jain
@ 2018-02-27 0:26 ` Balbir Singh
2018-03-01 3:00 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Balbir Singh @ 2018-02-27 0:26 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev, Michael Ellerman
Cc: Benjamin Herrenschmidt, Paul Mackerras, Nicholas Piggin,
Douglas Miller, Frederic Barrat
On Mon, 2018-02-26 at 17:06 +0530, Vaibhav Jain wrote:
> Presently sysrq key for xmon('x') is registered during kernel init
> irrespective of the value of kernel param 'xmon'. Thus xmon is enabled
> even if 'xmon=off' is passed on the kernel command line. However this
> doesn't enable the kernel debugger hooks needed for instruction or data
> breakpoints. Thus when a break-point is hit with xmon=off a kernel oops
> of the form below is reported:
>
> Oops: Exception in kernel mode, sig: 5 [#1]
> < snip >
> Trace/breakpoint trap
>
> To fix this the patch checks and enables debugger hooks when an
> instruction or data break-point is set via xmon console. It also clears
> all breakpoints when xmon is disabled via debugfs.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---
> arch/powerpc/xmon/xmon.c | 34 +++++++++++++++++++++++++++++++++-
> 1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 82e1a3ee6e0f..3679f5417a7e 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -1295,6 +1295,7 @@ bpt_cmds(void)
> switch (cmd) {
> #ifndef CONFIG_PPC_8xx
> static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n";
> + static const char warnxmon[] = "xmon: Enabling debugger hooks\n";
> int mode;
> case 'd': /* bd - hardware data breakpoint */
> mode = 7;
> @@ -1315,6 +1316,11 @@ bpt_cmds(void)
> dabr.address &= ~HW_BRK_TYPE_DABR;
> dabr.enabled = mode | BP_DABR;
> }
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
Can we get this three liner into an inline function?
> break;
>
> case 'i': /* bi - hardware instr breakpoint */
> @@ -1335,6 +1341,12 @@ bpt_cmds(void)
> if (bp != NULL) {
> bp->enabled |= BP_CIABR;
> iabr = bp;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> }
> break;
> #endif
> @@ -1399,8 +1411,15 @@ bpt_cmds(void)
> if (!check_bp_loc(a))
> break;
> bp = new_breakpoint(a);
> - if (bp != NULL)
> + if (bp != NULL) {
> bp->enabled |= BP_TRAP;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> + }
> break;
> }
> }
> @@ -3651,9 +3670,22 @@ device_initcall(setup_xmon_sysrq);
> #ifdef CONFIG_DEBUG_FS
> static int xmon_dbgfs_set(void *data, u64 val)
> {
> + int i;
> +
> xmon_on = !!val;
> xmon_init(xmon_on);
>
> + /* make sure all breakpoints removed when disabling */
> + if (!xmon_on) {
> + for (i = 0; i < NBPTS; ++i)
> + bpts[i].enabled = 0;
> + /* if anything set inform user that breakpoints are cleared */
> + if (iabr || dabr.enabled)
> + pr_info("xmon: All breakpoints cleared\n");
> +
> + iabr = NULL;
> + dabr.enabled = 0;
Is this sufficient or do we need remove_bpts()
Balbir Singh
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] xmon: Setup xmon debugger hooks when first break-point is set
2018-02-26 11:36 [PATCH] xmon: Setup xmon debugger hooks when first break-point is set Vaibhav Jain
2018-02-27 0:26 ` Balbir Singh
@ 2018-03-01 3:00 ` Michael Ellerman
1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2018-03-01 3:00 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev
Cc: Vaibhav Jain, Benjamin Herrenschmidt, Paul Mackerras,
Balbir Singh, Nicholas Piggin, Douglas Miller, Frederic Barrat
Vaibhav Jain <vaibhav@linux.vnet.ibm.com> writes:
> Presently sysrq key for xmon('x') is registered during kernel init
> irrespective of the value of kernel param 'xmon'. Thus xmon is enabled
> even if 'xmon=off' is passed on the kernel command line. However this
> doesn't enable the kernel debugger hooks needed for instruction or data
> breakpoints. Thus when a break-point is hit with xmon=off a kernel oops
> of the form below is reported:
>
> Oops: Exception in kernel mode, sig: 5 [#1]
> < snip >
> Trace/breakpoint trap
>
> To fix this the patch checks and enables debugger hooks when an
> instruction or data break-point is set via xmon console.
> It also clears
^^^^
> all breakpoints when xmon is disabled via debugfs.
Can you split that into a separate patch please.
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 82e1a3ee6e0f..3679f5417a7e 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -1295,6 +1295,7 @@ bpt_cmds(void)
> switch (cmd) {
> #ifndef CONFIG_PPC_8xx
> static const char badaddr[] = "Only kernel addresses are permitted for breakpoints\n";
> + static const char warnxmon[] = "xmon: Enabling debugger hooks\n";
> int mode;
> case 'd': /* bd - hardware data breakpoint */
> mode = 7;
> @@ -1315,6 +1316,11 @@ bpt_cmds(void)
> dabr.address &= ~HW_BRK_TYPE_DABR;
> dabr.enabled = mode | BP_DABR;
> }
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> break;
>
> case 'i': /* bi - hardware instr breakpoint */
> @@ -1335,6 +1341,12 @@ bpt_cmds(void)
> if (bp != NULL) {
> bp->enabled |= BP_CIABR;
> iabr = bp;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> }
> break;
> #endif
> @@ -1399,8 +1411,15 @@ bpt_cmds(void)
> if (!check_bp_loc(a))
> break;
> bp = new_breakpoint(a);
> - if (bp != NULL)
> + if (bp != NULL) {
> bp->enabled |= BP_TRAP;
> +
> + /* Enable xmon hooks if needed */
> + if (!xmon_on) {
> + printf(warnxmon);
> + xmon_on = 1;
> + }
> + }
Yeah Balbir is right, repeating that three times is ugly. A static
function would do, and also avoids you feeling like you need to make
warnxmon a static char[].
cheers
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-01 3:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-26 11:36 [PATCH] xmon: Setup xmon debugger hooks when first break-point is set Vaibhav Jain
2018-02-27 0:26 ` Balbir Singh
2018-03-01 3:00 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).