[patch 1/4] Kprobes support for IA64

[patch 1/4] Kprobes support for IA64

[Anil S Keshavamurthy]

[-- Attachment #1: kprobes-ia64-kdebug.patch --]
[-- Type: text/plain, Size: 6090 bytes --]


This patch adds the kdebug die notification mechanism needed by Kprobes.

For break instruction on Branch type slot, imm21 is ignored and value
zero is placed in IIM register, hence we need to handle kprobes
for switch case zero.
=========================================================================

signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
signed-off-by: Rusty Lynch <Rusty.lynch@intel.com>
=========================================================================

 arch/ia64/kernel/traps.c  |   33 ++++++++++++++++++++++++
 arch/ia64/mm/fault.c      |    8 ++++++
 include/asm-ia64/break.h  |    2 +
 include/asm-ia64/kdebug.h |   61 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 103 insertions(+), 1 deletion(-)

Index: linux-2.6.12-rc4/arch/ia64/kernel/traps.c
===================================================================
--- linux-2.6.12-rc4.orig/arch/ia64/kernel/traps.c	2005-05-19 15:39:40.000000000 -0700
+++ linux-2.6.12-rc4/arch/ia64/kernel/traps.c	2005-05-22 23:27:53.000000000 -0700
@@ -21,12 +21,26 @@
 #include <asm/intrinsics.h>
 #include <asm/processor.h>
 #include <asm/uaccess.h>
+#include <asm/kdebug.h>
 
 extern spinlock_t timerlist_lock;
 
 fpswa_interface_t *fpswa_interface;
 EXPORT_SYMBOL(fpswa_interface);
 
+struct notifier_block *ia64die_chain;
+static DEFINE_SPINLOCK(die_notifier_lock);
+
+int register_die_notifier(struct notifier_block *nb)
+{
+	int err = 0;
+	unsigned long flags;
+	spin_lock_irqsave(&die_notifier_lock, flags);
+	err = notifier_chain_register(&ia64die_chain, nb);
+	spin_unlock_irqrestore(&die_notifier_lock, flags);
+	return err;
+}
+
 void __init
 trap_init (void)
 {
@@ -119,6 +133,10 @@
 
 	switch (break_num) {
 	      case 0: /* unknown error (used by GCC for __builtin_abort()) */
+		if (notify_die(DIE_BREAK, "kprobe", regs, break_num, TRAP_BRKPT, SIGTRAP)
+			       	== NOTIFY_STOP) {
+			return;
+		}
 		die_if_kernel("bugcheck!", regs, break_num);
 		sig = SIGILL; code = ILL_ILLOPC;
 		break;
@@ -171,6 +189,15 @@
 		sig = SIGILL; code = __ILL_BNDMOD;
 		break;
 
+	      case 0x80200:
+	      case 0x80300:
+		if (notify_die(DIE_BREAK, "kprobe", regs, break_num, TRAP_BRKPT, SIGTRAP)
+			       	== NOTIFY_STOP) {
+			return;
+		}
+		sig = SIGTRAP; code = TRAP_BRKPT;
+		break;
+
 	      default:
 		if (break_num < 0x40000 || break_num > 0x100000)
 			die_if_kernel("Bad break", regs, break_num);
@@ -521,7 +548,11 @@
 #endif
 			break;
 		      case 35: siginfo.si_code = TRAP_BRANCH; ifa = 0; break;
-		      case 36: siginfo.si_code = TRAP_TRACE; ifa = 0; break;
+		      case 36:
+			      if (notify_die(DIE_SS, "ss", &regs, vector,
+					     vector, SIGTRAP) == NOTIFY_STOP)
+				      return;
+			      siginfo.si_code = TRAP_TRACE; ifa = 0; break;
 		}
 		siginfo.si_signo = SIGTRAP;
 		siginfo.si_errno = 0;
Index: linux-2.6.12-rc4/arch/ia64/mm/fault.c
===================================================================
--- linux-2.6.12-rc4.orig/arch/ia64/mm/fault.c	2005-05-19 15:39:40.000000000 -0700
+++ linux-2.6.12-rc4/arch/ia64/mm/fault.c	2005-05-22 23:05:02.000000000 -0700
@@ -14,6 +14,7 @@
 #include <asm/processor.h>
 #include <asm/system.h>
 #include <asm/uaccess.h>
+#include <asm/kdebug.h>
 
 extern void die (char *, struct pt_regs *, long);
 
@@ -102,6 +103,13 @@
 		goto bad_area_no_up;
 #endif
 
+	/*
+	 * This is to handle the kprobes on user space access instructions
+	 */
+	if (notify_die(DIE_PAGE_FAULT, "page fault", regs, code, TRAP_BRKPT,
+					SIGSEGV) == NOTIFY_STOP)
+		return;
+
 	down_read(&mm->mmap_sem);
 
 	vma = find_vma_prev(mm, address, &prev_vma);
Index: linux-2.6.12-rc4/include/asm-ia64/break.h
===================================================================
--- linux-2.6.12-rc4.orig/include/asm-ia64/break.h	2005-05-19 15:39:40.000000000 -0700
+++ linux-2.6.12-rc4/include/asm-ia64/break.h	2005-05-19 15:39:44.000000000 -0700
@@ -12,6 +12,8 @@
  * OS-specific debug break numbers:
  */
 #define __IA64_BREAK_KDB		0x80100
+#define __IA64_BREAK_KPROBE		0x80200
+#define __IA64_BREAK_JPROBE		0x80300
 
 /*
  * OS-specific break numbers:
Index: linux-2.6.12-rc4/include/asm-ia64/kdebug.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.12-rc4/include/asm-ia64/kdebug.h	2005-05-19 15:39:44.000000000 -0700
@@ -0,0 +1,61 @@
+#ifndef _IA64_KDEBUG_H
+#define _IA64_KDEBUG_H 1
+/*
+ * include/asm-ia64/kdebug.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) Intel Corporation, 2005
+ *
+ * 2005-Apr     Rusty Lynch <rusty.lynch@intel.com> and Anil S Keshavamurthy
+ *              <anil.s.keshavamurthy@intel.com> adopted from
+ *              include/asm-x86_64/kdebug.h
+ */
+#include <linux/notifier.h>
+
+struct pt_regs;
+
+struct die_args {
+	struct pt_regs *regs;
+	const char *str;
+	long err;
+	int trapnr;
+	int signr;
+};
+
+int register_die_notifier(struct notifier_block *nb);
+extern struct notifier_block *ia64die_chain;
+
+enum die_val {
+	DIE_BREAK = 1,
+	DIE_SS,
+	DIE_PAGE_FAULT,
+};
+
+static inline int notify_die(enum die_val val, char *str, struct pt_regs *regs,
+			     long err, int trap, int sig)
+{
+	struct die_args args = {
+		.regs   = regs,
+		.str    = str,
+		.err    = err,
+		.trapnr = trap,
+		.signr  = sig
+	};
+
+	return notifier_call_chain(&ia64die_chain, val, &args);
+}
+
+#endif

--

Re: [patch 1/4] Kprobes support for IA64

[Keith Owens]

On Mon, 23 May 2005 08:39:07 -0700, 
Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> wrote:
>
>This patch adds the kdebug die notification mechanism needed by Kprobes.
> 	      case 0: /* unknown error (used by GCC for __builtin_abort()) */
>+		if (notify_die(DIE_BREAK, "kprobe", regs, break_num, TRAP_BRKPT, SIGTRAP)
>+			       	== NOTIFY_STOP) {
>+			return;
>+		}
> 		die_if_kernel("bugcheck!", regs, break_num);
> 		sig = SIGILL; code = ILL_ILLOPC;
> 		break;

Nit pick.  Any break instruction in a B slot will set break_num 0, so
you cannot tell if the break was inserted by kprobe or by another
debugger.  Setting the string to "kprobe" is misleading here, change it
to "break 0".

RE: [patch 1/4] Kprobes support for IA64

[Lynch, Rusty]

>From: Keith Owens [mailto:kaos@sgi.com]
>Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> wrote:
>>
>>This patch adds the kdebug die notification mechanism needed by
Kprobes.
>> 	      case 0: /* unknown error (used by GCC for
__builtin_abort()) */
>>+		if (notify_die(DIE_BREAK, "kprobe", regs, break_num,
>TRAP_BRKPT, SIGTRAP)
>>+			       	== NOTIFY_STOP) {
>>+			return;
>>+		}
>> 		die_if_kernel("bugcheck!", regs, break_num);
>> 		sig = SIGILL; code = ILL_ILLOPC;
>> 		break;
>
>Nit pick.  Any break instruction in a B slot will set break_num 0, so
>you cannot tell if the break was inserted by kprobe or by another
>debugger.  Setting the string to "kprobe" is misleading here, change it
>to "break 0".

Good catch.  We'll update the informational string.

    --rusty

Re: [patch 1/4] Kprobes support for IA64

[Alan D. Brunelle]

Isn't the real issue here that if kprobes attempts to put in a 'break 
0x80200' into a B-slot that it instead becomes a 'break.b 0' -- as the 
break.b does not accept an immediate value? Which probably means that 
either kprobes (a) should not rely on the immediate value of the break 
at all (always put in an immediate value of 0), or (b) kprobes should 
not allow a probe on a B-slot of an instruction bundle.

Kprobes does have the two cases covered in traps.c (case 0 - when a 
B-slot break is used, and case 0x80200 for a non-B-slot break). But this 
doesn't seem very clean. (If it was decided that one should not overload 
the break 0 case, and instead use a uniquely defined break number, then 
it fails on a B-slot probe. If it is OK to overload the break 0 case, 
why have another break number at all?)

I started doing a port of kprobes, ran into this, and decided to try a 
different mechanism that replaced the whole instruction bundle - so that 
I could format the instruction bundle to allow a break instruction with 
an immediate value (and thus uniquely identify KPROBE breaks). 
[Basically put the break in the 1st slot (all the time), and then go 
execute the original instruction *bundle* elsewhere when the break is hit.]

PS. I don't see the 0x80300 defined __IA64_BREAK_JPROBE being used 
anywhere...

Alan D. Brunelle
Hewlett-Packard


Lynch, Rusty wrote:

>>From: Keith Owens [mailto:kaos@sgi.com]
>>Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> wrote:
>>    
>>
>>>This patch adds the kdebug die notification mechanism needed by
>>>      
>>>
>Kprobes.
>  
>
>>>	      case 0: /* unknown error (used by GCC for
>>>      
>>>
>__builtin_abort()) */
>  
>
>>>+		if (notify_die(DIE_BREAK, "kprobe", regs, break_num,
>>>      
>>>
>>TRAP_BRKPT, SIGTRAP)
>>    
>>
>>>+			       	== NOTIFY_STOP) {
>>>+			return;
>>>+		}
>>>		die_if_kernel("bugcheck!", regs, break_num);
>>>		sig = SIGILL; code = ILL_ILLOPC;
>>>		break;
>>>      
>>>
>>Nit pick.  Any break instruction in a B slot will set break_num 0, so
>>you cannot tell if the break was inserted by kprobe or by another
>>debugger.  Setting the string to "kprobe" is misleading here, change it
>>to "break 0".
>>    
>>
>
>Good catch.  We'll update the informational string.
>
>    --rusty
>-
>To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>  
>

RE: [patch 1/4] Kprobes support for IA64

[Lynch, Rusty]

From: Alan D. Brunelle [mailto:Alan.Brunelle@hp.com]
>Isn't the real issue here that if kprobes attempts to put in a 'break
>0x80200' into a B-slot that it instead becomes a 'break.b 0' -- as the
>break.b does not accept an immediate value? Which probably means that
>either kprobes (a) should not rely on the immediate value of the break
>at all (always put in an immediate value of 0), or (b) kprobes should
>not allow a probe on a B-slot of an instruction bundle.
>
>Kprobes does have the two cases covered in traps.c (case 0 - when a
>B-slot break is used, and case 0x80200 for a non-B-slot break). But
this
>doesn't seem very clean. (If it was decided that one should not
overload
>the break 0 case, and instead use a uniquely defined break number, then
>it fails on a B-slot probe. If it is OK to overload the break 0 case,
>why have another break number at all?)
 
The realization that breaking on a b-slot caused the immediate value to
be ignored happened after we had the main portion of kprobes and jprobes
implemented.  Now that I think about it, there really isn't any reason
to ever use anything but an immediate value of zero for all cases.

Well... except for maybe gaining control back from a jprobe.  The
current patch doesn't take advantage of the fact that a jprobe uses a
different immediate value (as you point out below).  Instead it is up to
pre_kprobe_handler() to recognize the case (see the jprobe comment in
the middle of the function.)  I would rather have
kprobes_exceptions_notify() call a separate jprobe handler function.

The Jprobe break is not inserted in an existing bundle like the break
used to gain initial control for kprobes, to the break0 thing is not an
issue.

>
>I started doing a port of kprobes, ran into this, and decided to try a
>different mechanism that replaced the whole instruction bundle - so
that
>I could format the instruction bundle to allow a break instruction with
>an immediate value (and thus uniquely identify KPROBE breaks).
>[Basically put the break in the 1st slot (all the time), and then go
>execute the original instruction *bundle* elsewhere when the break is
hit.]
>

We explored this for a while, but you start getting lots of
complications when you consider that other probes could be inserted on
the remaining slots of the same bundle.

I think we can have a sensible implementation that always inserts a
break with a zero immediate value, and then have jprobes execute a break
with a reserved non-zero immediate value.

>PS. I don't see the 0x80300 defined __IA64_BREAK_JPROBE being used
>anywhere...

The number is being used in jprobes.S:jprobe_break, but not the #define
for some reason (but it should.)

Although, like I pointed out before, the current handler does not take
advantage of this.

>
>Alan D. Brunelle
>Hewlett-Packard
>
>
>Lynch, Rusty wrote:
>
>>>From: Keith Owens [mailto:kaos@sgi.com]
>>>Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> wrote:
>>>
>>>
>>>>This patch adds the kdebug die notification mechanism needed by
>>>>
>>>>
>>Kprobes.
>>
>>
>>>>	      case 0: /* unknown error (used by GCC for
>>>>
>>>>
>>__builtin_abort()) */
>>
>>
>>>>+		if (notify_die(DIE_BREAK, "kprobe", regs, break_num,
>>>>
>>>>
>>>TRAP_BRKPT, SIGTRAP)
>>>
>>>
>>>>+			       	== NOTIFY_STOP) {
>>>>+			return;
>>>>+		}
>>>>		die_if_kernel("bugcheck!", regs, break_num);
>>>>		sig = SIGILL; code = ILL_ILLOPC;
>>>>		break;
>>>>
>>>>
>>>Nit pick.  Any break instruction in a B slot will set break_num 0, so
>>>you cannot tell if the break was inserted by kprobe or by another
>>>debugger.  Setting the string to "kprobe" is misleading here, change
it
>>>to "break 0".
>>>
>>>
>>
>>Good catch.  We'll update the informational string.
>>
>>    --rusty
>>-
>>To unsubscribe from this list: send the line "unsubscribe linux-ia64"
in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>
>>

Re: [patch 1/4] Kprobes support for IA64

[Keith Owens]

On Wed, 25 May 2005 10:00:18 -0400,
"Alan D. Brunelle" <Alan.Brunelle@hp.com> wrote:
>Isn't the real issue here that if kprobes attempts to put in a 'break
>0x80200' into a B-slot that it instead becomes a 'break.b 0' -- as the
>break.b does not accept an immediate value?

break.b is a B9 type instruction, which does take an imm21 value.  It
is the hardware that does not store imm21 in CR.IIM when break.b is
issued.

>Kprobes does have the two cases covered in traps.c (case 0 - when a
>B-slot break is used, and case 0x80200 for a non-B-slot break). But this
>doesn't seem very clean. (If it was decided that one should not overload
>the break 0 case, and instead use a uniquely defined break number, then
>it fails on a B-slot probe. If it is OK to overload the break 0 case,
>why have another break number at all?)

Mainly for documentation when looking at the assembler code.  break 0
is used for BUG(), coding a different value in the break instruction
for the debugger helps the person debugging the debugger :(.  I have no
problem with coding two cases in ia64_bad_break() in order to work
around the hardware "feature".

Also consider the case where your debugger allows users to code a
deliberate entry to the debugger, like KDB_ENTER().  That case always
requires a separate break imm21 value, because the break point is not
known to the debugger until the code is executed.

Re: [patch 1/4] Kprobes support for IA64

[Keshavamurthy Anil S]

On Thu, May 26, 2005 at 10:49:02AM +1000, Keith Owens wrote:
> On Wed, 25 May 2005 10:00:18 -0400,
> "Alan D. Brunelle" <Alan.Brunelle@hp.com> wrote:
> >Isn't the real issue here that if kprobes attempts to put in a 'break
> >0x80200' into a B-slot that it instead becomes a 'break.b 0' -- as the
> >break.b does not accept an immediate value?
> 
> break.b is a B9 type instruction, which does take an imm21 value.  It
> is the hardware that does not store imm21 in CR.IIM when break.b is
> issued.
> 
> >Kprobes does have the two cases covered in traps.c (case 0 - when a
> >B-slot break is used, and case 0x80200 for a non-B-slot break). But this
> >doesn't seem very clean. (If it was decided that one should not overload
> >the break 0 case, and instead use a uniquely defined break number, then
> >it fails on a B-slot probe. If it is OK to overload the break 0 case,
> >why have another break number at all?)
> 
> Mainly for documentation when looking at the assembler code.  break 0
> is used for BUG(), coding a different value in the break instruction
> for the debugger helps the person debugging the debugger :(.  I have no
> problem with coding two cases in ia64_bad_break() in order to work
> around the hardware "feature".

I agree with Keith, when a person taking a instructin dump, a 
different value will help uniquely identify that this
is a kprobe break instruction which is a replaced instrucion
of the original instruction. So we will leave with what we have
now, i.e handle the same with two cases.


> 
> Also consider the case where your debugger allows users to code a
> deliberate entry to the debugger, like KDB_ENTER().  That case always
> requires a separate break imm21 value, because the break point is not
> known to the debugger until the code is executed.
> 

Re: [patch 1/4] Kprobes support for IA64

[David Mosberger]

>>>>> On Wed, 25 May 2005 18:06:52 -0700, Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com> said:

  Keshavamurthy> I agree with Keith, when a person taking a instructin
  Keshavamurthy> dump, a different value will help uniquely identify
  Keshavamurthy> that this is a kprobe break instruction which is a
  Keshavamurthy> replaced instrucion of the original instruction. So
  Keshavamurthy> we will leave with what we have now, i.e handle the
  Keshavamurthy> same with two cases.

We could read the imm21 value from the break.b instruction.

	--david