[PATCH 21/31] Add debugger entry points for MIPS

[PATCH 21/31] Add debugger entry points for MIPS

[Jeffrey Merkey]

This patch series adds an export which can be set by system debuggers to
direct the hard lockup and soft lockup detector to trigger a breakpoint
exception and enter a debugger if one is active.  It is assumed that if
someone sets this variable, then an breakpoint handler of some sort will
be actively loaded or registered via the notify die handler chain.

This addition is extremely useful for debugging hard and soft lockups
real time and quickly from a console debugger.

Signed-off-by: Jeffrey Merkey <jeffmerkey@gmail.com>
---
 arch/mips/include/asm/kdebug.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/mips/include/asm/kdebug.h b/arch/mips/include/asm/kdebug.h
index 8e3d08e..af5999e 100644
--- a/arch/mips/include/asm/kdebug.h
+++ b/arch/mips/include/asm/kdebug.h
@@ -16,4 +16,16 @@ enum die_val {
 	DIE_UPROBE_XOL,
 };
 
+
+void arch_breakpoint(void)
+{
+	__asm__ __volatile__(
+		".globl breakinst\n\t"
+		".set\tnoreorder\n\t"
+		"nop\n"
+		"breakinst:\tbreak\n\t"
+		"nop\n\t"
+		".set\treorder");
+}
+
 #endif /* _ASM_MIPS_KDEBUG_H */
-- 
1.8.3.1

Re: [PATCH 21/31] Add debugger entry points for MIPS

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]

Hi Jeffrey,

[auto build test ERROR on v4.5-rc1]
[also build test ERROR on next-20160128]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Jeffrey-Merkey/Add-hard-soft-lockup-debugger-entry-points/20160129-035852
config: mips-jz4740 (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   kernel/sys.o: In function `arch_breakpoint':
>> sys.c:(.text+0x880): multiple definition of `arch_breakpoint'
   kernel/sysctl.o:sysctl.c:(.text+0x134c): first defined here
   kernel/sys.o: In function `arch_breakpoint':
>> sys.c:(.text+0x884): multiple definition of `breakinst'
   kernel/sysctl.o:sysctl.c:(.text+0x1350): first defined here
   kernel/notifier.o: In function `arch_breakpoint':
   notifier.c:(.text+0x678): multiple definition of `arch_breakpoint'
   kernel/sysctl.o:sysctl.c:(.text+0x134c): first defined here
   kernel/notifier.o: In function `arch_breakpoint':
   notifier.c:(.text+0x67c): multiple definition of `breakinst'
   kernel/sysctl.o:sysctl.c:(.text+0x1350): first defined here
   kernel/sched/built-in.o: In function `arch_breakpoint':
   (.text+0x3b0): multiple definition of `arch_breakpoint'
   kernel/sysctl.o:sysctl.c:(.text+0x134c): first defined here
   kernel/sched/built-in.o: In function `arch_breakpoint':
   (.text+0x3b4): multiple definition of `breakinst'
   kernel/sysctl.o:sysctl.c:(.text+0x1350): first defined here
   kernel/kprobes.o: In function `arch_breakpoint':
   kprobes.c:(.text+0x1690): multiple definition of `arch_breakpoint'
   kernel/sysctl.o:sysctl.c:(.text+0x134c): first defined here
   kernel/kprobes.o: In function `arch_breakpoint':
   kprobes.c:(.text+0x1694): multiple definition of `breakinst'
   kernel/sysctl.o:sysctl.c:(.text+0x1350): first defined here

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 18092 bytes --]

Re: [PATCH 21/31] Add debugger entry points for MIPS

[Maciej W. Rozycki]

On Thu, 28 Jan 2016, Jeffrey Merkey wrote:

> This patch series adds an export which can be set by system debuggers to
> direct the hard lockup and soft lockup detector to trigger a breakpoint
> exception and enter a debugger if one is active.  It is assumed that if
> someone sets this variable, then an breakpoint handler of some sort will
> be actively loaded or registered via the notify die handler chain.
> 
> This addition is extremely useful for debugging hard and soft lockups
> real time and quickly from a console debugger.

 What's the intended use case for this hook and what do you call a console 
debugger?

 I'm asking because from the debugging perspective the Linux kernel is a 
bare metal application and for such the BREAK instruction is not the usual 
choice on the MIPS target.  This instruction is normally used for userland 
debugging, it traps to the kernel and is handled there.  Furthermore there 
are a few other applications of this instruction defined as a part of the 
MIPS ABI, also handled by the kernel, determined by the breakpoint code 
embedded with the instruction word, e.g. to trap integer division errors.  
See arch/mips/include/uapi/asm/break.h for the codes defined so far.

 All this means the trap may not be appropriate for debugging the kernel 
itself as you don't want to intercept it or the system won't run 
correctly.  You'd have to hook into the kernel itself to intercept it too.

 But if you do have a working debug environment already set up around this 
arrangement, then it might be fine after all.  In that case I think using 
a non-zero breakpoint code would make sense though, as 0 is often treated 
as a random (spurious) trap (it was used by IRIX though).  Or do you 
detect it by the symbol name somehow?

 I've got a couple of further notes on the patch itself below.

> diff --git a/arch/mips/include/asm/kdebug.h b/arch/mips/include/asm/kdebug.h
> index 8e3d08e..af5999e 100644
> --- a/arch/mips/include/asm/kdebug.h
> +++ b/arch/mips/include/asm/kdebug.h
> @@ -16,4 +16,16 @@ enum die_val {
>  	DIE_UPROBE_XOL,
>  };
>  
> +
> +void arch_breakpoint(void)
> +{
> +	__asm__ __volatile__(
> +		".globl breakinst\n\t"

 Please keep formatting consistent -- the rest of code below uses `\t' as 
the separator, so use it here as well.  We don't have an established 
inline assembly formatting style, so please just keep your chosen one 
consistent.

> +		".set\tnoreorder\n\t"
> +		"nop\n"
> +		"breakinst:\tbreak\n\t"
> +		"nop\n\t"
> +		".set\treorder");
> +}
> +
>  #endif /* _ASM_MIPS_KDEBUG_H */

 Why do you need these NOPs around the breakpoint?  You also need to mark 
`breakinst' as a function symbol (`.aent' might do) or otherwise you'll 
get garbled disassembly if this is built as microMIPS code.

  Maciej

Re: [PATCH 21/31] Add debugger entry points for MIPS

[Maciej W. Rozycki]

On Thu, 28 Jan 2016, Jeffrey Merkey wrote:

> This patch series adds an export which can be set by system debuggers to
> direct the hard lockup and soft lockup detector to trigger a breakpoint
> exception and enter a debugger if one is active.  It is assumed that if
> someone sets this variable, then an breakpoint handler of some sort will
> be actively loaded or registered via the notify die handler chain.
> 
> This addition is extremely useful for debugging hard and soft lockups
> real time and quickly from a console debugger.

 What's the intended use case for this hook and what do you call a console 
debugger?

 I'm asking because from the debugging perspective the Linux kernel is a 
bare metal application and for such the BREAK instruction is not the usual 
choice on the MIPS target.  This instruction is normally used for userland 
debugging, it traps to the kernel and is handled there.  Furthermore there 
are a few other applications of this instruction defined as a part of the 
MIPS ABI, also handled by the kernel, determined by the breakpoint code 
embedded with the instruction word, e.g. to trap integer division errors.  
See arch/mips/include/uapi/asm/break.h for the codes defined so far.

 All this means the trap may not be appropriate for debugging the kernel 
itself as you don't want to intercept it or the system won't run 
correctly.  You'd have to hook into the kernel itself to intercept it too.

 But if you do have a working debug environment already set up around this 
arrangement, then it might be fine after all.  In that case I think using 
a non-zero breakpoint code would make sense though, as 0 is often treated 
as a random (spurious) trap (it was used by IRIX though).  Or do you 
detect it by the symbol name somehow?

 I've got a couple of further notes on the patch itself below.

> diff --git a/arch/mips/include/asm/kdebug.h b/arch/mips/include/asm/kdebug.h
> index 8e3d08e..af5999e 100644
> --- a/arch/mips/include/asm/kdebug.h
> +++ b/arch/mips/include/asm/kdebug.h
> @@ -16,4 +16,16 @@ enum die_val {
>  	DIE_UPROBE_XOL,
>  };
>  
> +
> +void arch_breakpoint(void)
> +{
> +	__asm__ __volatile__(
> +		".globl breakinst\n\t"

 Please keep formatting consistent -- the rest of code below uses `\t' as 
the separator, so use it here as well.  We don't have an established 
inline assembly formatting style, so please just keep your chosen one 
consistent.

> +		".set\tnoreorder\n\t"
> +		"nop\n"
> +		"breakinst:\tbreak\n\t"
> +		"nop\n\t"
> +		".set\treorder");
> +}
> +
>  #endif /* _ASM_MIPS_KDEBUG_H */

 Why do you need these NOPs around the breakpoint?  You also need to mark 
`breakinst' as a function symbol (`.aent' might do) or otherwise you'll 
get garbled disassembly if this is built as microMIPS code.

  Maciej

Re: [PATCH 21/31] Add debugger entry points for MIPS

[Jeff Merkey]

On 2/1/16, Maciej W. Rozycki <macro@imgtec.com> wrote:
> On Thu, 28 Jan 2016, Jeffrey Merkey wrote:
>
>> This patch series adds an export which can be set by system debuggers to
>> direct the hard lockup and soft lockup detector to trigger a breakpoint
>> exception and enter a debugger if one is active.  It is assumed that if
>> someone sets this variable, then an breakpoint handler of some sort will
>> be actively loaded or registered via the notify die handler chain.
>>
>> This addition is extremely useful for debugging hard and soft lockups
>> real time and quickly from a console debugger.
>
>  What's the intended use case for this hook and what do you call a console
> debugger?
>
>  I'm asking because from the debugging perspective the Linux kernel is a
> bare metal application and for such the BREAK instruction is not the usual
> choice on the MIPS target.  This instruction is normally used for userland
> debugging, it traps to the kernel and is handled there.  Furthermore there
> are a few other applications of this instruction defined as a part of the
> MIPS ABI, also handled by the kernel, determined by the breakpoint code
> embedded with the instruction word, e.g. to trap integer division errors.
> See arch/mips/include/uapi/asm/break.h for the codes defined so far.
>
>  All this means the trap may not be appropriate for debugging the kernel
> itself as you don't want to intercept it or the system won't run
> correctly.  You'd have to hook into the kernel itself to intercept it too.
>
>  But if you do have a working debug environment already set up around this
> arrangement, then it might be fine after all.  In that case I think using
> a non-zero breakpoint code would make sense though, as 0 is often treated
> as a random (spurious) trap (it was used by IRIX though).  Or do you
> detect it by the symbol name somehow?
>
>  I've got a couple of further notes on the patch itself below.
>
>> diff --git a/arch/mips/include/asm/kdebug.h
>> b/arch/mips/include/asm/kdebug.h
>> index 8e3d08e..af5999e 100644
>> --- a/arch/mips/include/asm/kdebug.h
>> +++ b/arch/mips/include/asm/kdebug.h
>> @@ -16,4 +16,16 @@ enum die_val {
>>  	DIE_UPROBE_XOL,
>>  };
>>
>> +
>> +void arch_breakpoint(void)
>> +{
>> +	__asm__ __volatile__(
>> +		".globl breakinst\n\t"
>
>  Please keep formatting consistent -- the rest of code below uses `\t' as
> the separator, so use it here as well.  We don't have an established
> inline assembly formatting style, so please just keep your chosen one
> consistent.
>
>> +		".set\tnoreorder\n\t"
>> +		"nop\n"
>> +		"breakinst:\tbreak\n\t"
>> +		"nop\n\t"
>> +		".set\treorder");
>> +}
>> +
>>  #endif /* _ASM_MIPS_KDEBUG_H */
>
>  Why do you need these NOPs around the breakpoint?  You also need to mark
> `breakinst' as a function symbol (`.aent' might do) or otherwise you'll
> get garbled disassembly if this is built as microMIPS code.
>
>   Maciej
>

You can drop this patch series, I am rethinking a better way to do this.

Jeff

Re: [PATCH 21/31] Add debugger entry points for MIPS

[Ralf Baechle]

On Mon, Feb 01, 2016 at 09:03:54AM -0700, Jeff Merkey wrote:

> You can drop this patch series, I am rethinking a better way to do this.

Dropped.

  Ralf