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 <[email protected]>
signed-off-by: Rusty Lynch <[email protected]>
=========================================================================
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", ®s, 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 <[email protected]> and Anil S Keshavamurthy
+ * <[email protected]> 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
--
On Mon, 23 May 2005 08:39:07 -0700,
Anil S Keshavamurthy <[email protected]> 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".
>From: Keith Owens [mailto:[email protected]]
>Anil S Keshavamurthy <[email protected]> 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
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:[email protected]]
>>Anil S Keshavamurthy <[email protected]> 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 [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
From: Alan D. Brunelle [mailto:[email protected]]
>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:[email protected]]
>>>Anil S Keshavamurthy <[email protected]> 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 [email protected]
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
On Wed, 25 May 2005 10:00:18 -0400,
"Alan D. Brunelle" <[email protected]> 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.
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" <[email protected]> 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.
>
>>>>> On Wed, 25 May 2005 18:06:52 -0700, Keshavamurthy Anil S <[email protected]> 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