Kprobes support for MIPS architecture.

PATCH New Himanshu Chauhan 2 participants 5 messages Jun 7, 2010 – Jun 11, 2010 MIPS

Share & tools

Himanshu Chauhan

author

2010-06-07 18:31 UTC #0 lore raw

PATCH 1/1 Kprobes support for MIPS architecture.

This is an initial draft of Kprobes support for MIPS architecture. It is based on a patch series submitted on CELinux mailing list from Sony for 2.6.16 series. Since then many things changed including APIs and file paths. So this is essentially a patch replayed on latest series.

Please review and provide your feedback.

Signed-off-by Himanshu Chauhan <[email protected]>

---
Himanshu Chauhan (1):
MIPS: KProbes support v0.1

 arch/mips/Kconfig               |   13 ++
 arch/mips/include/asm/kdebug.h  |    5 +
 arch/mips/include/asm/kprobes.h |   85 +++++++++
 arch/mips/kernel/Makefile       |    2 +
 arch/mips/kernel/genex.S        |    6 +
 arch/mips/kernel/kprobes.c      |  380 +++++++++++++++++++++++++++++++++++++++
 arch/mips/kernel/traps.c        |   40 ++++-
 arch/mips/mm/fault.c            |   11 +-
 8 files changed, 539 insertions(+), 3 deletions(-)
 create mode 100644 arch/mips/include/asm/kprobes.h
 create mode 100644 arch/mips/kernel/kprobes.c

Himanshu Chauhan

author

2010-06-07 18:31 UTC #1 lore raw

PATCH 1/1 MIPS: KProbes support v0.1

Signed-off-by Himanshu Chauhan <[email protected]>

---
 arch/mips/Kconfig               |   13 ++
 arch/mips/include/asm/kdebug.h  |    5 +
 arch/mips/include/asm/kprobes.h |   85 +++++++++
 arch/mips/kernel/Makefile       |    2 +
 arch/mips/kernel/genex.S        |    6 +
 arch/mips/kernel/kprobes.c      |  380 +++++++++++++++++++++++++++++++++++++++
 arch/mips/kernel/traps.c        |   40 ++++-
 arch/mips/mm/fault.c            |   11 +-
 8 files changed, 539 insertions(+), 3 deletions(-)
 create mode 100644 arch/mips/include/asm/kprobes.h
 create mode 100644 arch/mips/kernel/kprobes.c

arch/mips/Kconfig +13 -0

index cdaae94..ba7cc87 100644
@@ -2243,6 +2243,19 @@ source "kernel/power/Kconfig"
 
 endmenu
 
+menu "Instrumentation Support"
+
+config KPROBES
+       bool "MIPS Kprobes Support (Experimental)"
+       depends on EXPERIMENTAL && MODULES
+       help
+	  Kprobes allow you to trap at almost any kernel address
+	  and execute a callback function. Kprobes is useful for
+	  kernel debugging, non-intrusive instrumetation and testing.
+	  If in doubt, say N.
+
+endmenu
+
 source "arch/mips/kernel/cpufreq/Kconfig"
 
 source "net/Kconfig"

arch/mips/include/asm/kdebug.h +5 -0

index 5bf62aa..52818ac 100644
@@ -8,6 +8,11 @@ enum die_val {
 	DIE_FP,
 	DIE_TRAP,
 	DIE_RI,
+#ifdef CONFIG_KPROBES
+	DIE_PAGE_FAULT,
+	DIE_BREAK,
+	DIE_SSTEPBP,
+#endif
 };
 
 #endif /* _ASM_MIPS_KDEBUG_H */

arch/mips/include/asm/kprobes.h new file +85 -0

85 lines not on this page · raw message

arch/mips/kernel/Makefile +2 -0

index 7a6ac50..714f3c1 100644
@@ -83,6 +83,7 @@ obj-$(CONFIG_MIPS32_N32)	+= binfmt_elfn32.o scall64-n32.o signal_n32.o
 obj-$(CONFIG_MIPS32_O32)	+= binfmt_elfo32.o scall64-o32.o
 
 obj-$(CONFIG_KGDB)		+= kgdb.o
+obj-$(CONFIG_KPROBES)		+= kprobes.o
 obj-$(CONFIG_PROC_FS)		+= proc.o
 
 obj-$(CONFIG_64BIT)		+= cpu-bugs64.o

@@ -95,6 +96,7 @@ obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o
 obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
 obj-$(CONFIG_SPINLOCK_TEST)	+= spinlock_test.o
 
+
 CFLAGS_cpu-bugs64.o	= $(shell if $(CC) $(KBUILD_CFLAGS) -Wa,-mdaddi -c -o /dev/null -xc /dev/null >/dev/null 2>&1; then echo "-DHAVE_AS_SET_DADDI"; fi)
 
 obj-$(CONFIG_HAVE_STD_PC_SERIAL_PORT)	+= 8250-platform.o

arch/mips/kernel/genex.S +6 -0

index 8882e57..e53ac80 100644
@@ -450,7 +450,13 @@ NESTED(nmi_handler, PT_SIZE, sp)
 	BUILD_HANDLER ades ade ade silent		/* #5  */
 	BUILD_HANDLER ibe be cli silent			/* #6  */
 	BUILD_HANDLER dbe be cli silent			/* #7  */
+#ifndef CONFIG_KPROBES
+	/* call do_bp if bp hit and kprobes not configured */
 	BUILD_HANDLER bp bp sti silent			/* #9  */
+#else
+	/* call do_break if bp hit and kprobes are configured */
+	BUILD_HANDLER bp break sti silent		/* #9  */
+#endif
 	BUILD_HANDLER ri ri sti silent			/* #10 */
 	BUILD_HANDLER cpu cpu sti silent		/* #11 */
 	BUILD_HANDLER ov ov sti silent			/* #12 */

arch/mips/kernel/kprobes.c new file +380 -0

380 lines not on this page · raw message

arch/mips/kernel/traps.c +39 -1

index 8bdd6a6..f6b4b41 100644
@@ -27,6 +27,7 @@
 #include <linux/kdebug.h>
 #include <linux/notifier.h>
 #include <linux/kdb.h>
+#include <linux/kprobes.h>
 
 #include <asm/bootinfo.h>
 #include <asm/branch.h>

@@ -334,7 +335,7 @@ void show_regs(struct pt_regs *regs)
 	__show_regs((struct pt_regs *)regs);
 }
 
-void show_registers(const struct pt_regs *regs)
+void show_registers(struct pt_regs *regs)
 {
 	const int field = 2 * sizeof(unsigned long);

@@ -790,6 +791,43 @@ out_sigsegv:
 	force_sig(SIGSEGV, current);
 }
 
+#ifdef CONFIG_KPROBES
+asmlinkage void __kprobes do_break (struct pt_regs *regs)
+{
+	unsigned int opcode, bcode;
+
+	opcode = *(unsigned long *)(regs->cp0_epc);
+
+	bcode = ((opcode >> 6) & ((1 << 20) - 1));
+	if (bcode < (1 << 10))
+		bcode <<= 10;
+
+	/*
+	 * notify the kprobe handlers,if instruction is break 0 or break 5
+	 */
+	switch (bcode) {
+	case BRK_USERBP << 10:
+		if (notify_die(DIE_BREAK, "debug", regs, bcode, 0, 0) == NOTIFY_STOP)
+			return;
+		else
+			break;
+	case BRK_SSTEPBP << 10:
+		if (notify_die(DIE_SSTEPBP, "single_step", regs, bcode, 0, 0) == NOTIFY_STOP)
+			return;
+		else
+			break;
+	default:
+		break;
+	}
+
+	/*
+	 * If the bcode is other than 0 and 5, then call the normal
+	 * break handler do_bp()
+	 */
+	do_bp(regs);
+}
+#endif
+
 asmlinkage void do_tr(struct pt_regs *regs)
 {
 	unsigned int opcode, tcode = 0;

arch/mips/mm/fault.c +9 -2

index b78f7d9..86e2d27 100644
@@ -18,6 +18,8 @@
 #include <linux/smp.h>
 #include <linux/vt_kern.h>		/* For unblank_screen() */
 #include <linux/module.h>
+#include <linux/kprobes.h>
+#include <linux/kdebug.h>		/* notify_die and asm/kdebug.h */
 
 #include <asm/branch.h>
 #include <asm/mmu_context.h>

@@ -31,8 +33,8 @@
  * and the problem, and then passes it off to one of the appropriate
  * routines.
  */
-asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long write,
-			      unsigned long address)
+asmlinkage void __kprobes do_page_fault(struct pt_regs *regs, unsigned long write,
+                                        unsigned long address)
 {
 	struct vm_area_struct * vma = NULL;
 	struct task_struct *tsk = current;

@@ -47,6 +49,11 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long write,
 	       field, regs->cp0_epc);
 #endif
 
+	/* Notify kprobes fault handler. */
+        if (notify_die(DIE_PAGE_FAULT, "page fault",
+                       regs, -1, SEGV_MAPERR, SEGV_MAPERR) == NOTIFY_STOP)
+                return;
+
 	info.si_code = SEGV_MAPERR;
 
 	/*

Signature · 1 line

1.7.0.4

David Daney

2010-06-07 19:41 UTC #2 lore raw

Re: [PATCH] MIPS: KProbes support v0.1

I have a few questions and comments below. Many of them may be due to my lack of understanding about the internals of KProbes.

David Daney.

On 06/07/2010 09:34 AM, Himanshu Chauhan wrote:
[...]

Quoted 5 lines

arch/mips/include/asm/kdebug.h +0 -0
index 5bf62aa..52818ac 100644

arch/mips/include/asm/kdebug.h enum die_val { · line 17

Quoted 10 lines

arch/mips/include/asm/kdebug.h +5 -0

@@ -8,6 +8,11 @@ enum die_val {
  	DIE_FP,
  	DIE_TRAP,
  	DIE_RI,
+#ifdef CONFIG_KPROBES
+	DIE_PAGE_FAULT,
+	DIE_BREAK,
+	DIE_SSTEPBP,
+#endif
  };

It might be cleaner without the #ifdef. These are enum value definitions, so it doesn't affect code size.

Can you also explain how the die notifier chain interacts with KProbes and why it cannot be a seperate notifier chain?

Quoted 7 lines

arch/mips/include/asm/kdebug.h +0 -0
  #endif /* _ASM_MIPS_KDEBUG_H */
arch/mips/include/asm/kprobes.h new file +0 -0
new file mode 100644
index 0000000..0f647bf
@@ -0,0 +1,85 @@

[...]

arch/mips/include/asm/kprobes.h line 45

Quoted 9 lines

arch/mips/include/asm/kprobes.h new file +9 -0

+
+#define BREAKPOINT_INSTRUCTION		0x0000000d
+
+/*
+ * We do not have hardware single-stepping on MIPS.
+ * So we implement software single-stepping with breakpoint
+ * trap 'break 5'.
+ */
+#define BREAKPOINT_INSTRUCTION_2	0x0000014d

The BREAK codes are defined in asm/break.h This should be added there instead.

Why do you use codes (0 and 5) that are already kind of reserved for user space debuggers?

arch/mips/include/asm/kprobes.h line 54

Quoted 9 lines

arch/mips/include/asm/kprobes.h new file +9 -0

+#define MAX_INSN_SIZE 			2
+
+#define flush_insn_slot(p)		do { \
+        /* invalidate I-cache */             \
+        asm volatile("cache 0, 0($0)");      \
+        /* invalidate D-cache */             \
+        asm volatile("cache 9, 0($0)");      \
+        } while(0);
+

You have to call a function in arch/mips/mm/c-* to do this, you cannot open code with CACHE instructions as you need to handle CPU quirks and SMP. It is possible that flush_icache_range() or flush_cache_sigtramp() would work. Or we might need something new.

I see you use flush_icache_range() below, why have this definition, it looks unused?

[...]

Quoted 5 lines

arch/mips/kernel/genex.S +0 -0
index 8882e57..e53ac80 100644

arch/mips/kernel/genex.S NESTED(nmi_handler, PT_SIZE, sp) · line 459

Quoted 10 lines

arch/mips/kernel/genex.S +6 -0

@@ -450,7 +450,13 @@ NESTED(nmi_handler, PT_SIZE, sp)
  	BUILD_HANDLER ades ade ade silent		/* #5  */
  	BUILD_HANDLER ibe be cli silent			/* #6  */
  	BUILD_HANDLER dbe be cli silent			/* #7  */
+#ifndef CONFIG_KPROBES
+	/* call do_bp if bp hit and kprobes not configured */
  	BUILD_HANDLER bp bp sti silent			/* #9  */
+#else
+	/* call do_break if bp hit and kprobes are configured */
+	BUILD_HANDLER bp break sti silent		/* #9  */
+#endif

Why this ugliness? Can't you handle it in do_bp() or do_trap_or_bp()?

Quoted 9 lines

arch/mips/kernel/genex.S +0 -0

  	BUILD_HANDLER ri ri sti silent			/* #10 */
  	BUILD_HANDLER cpu cpu sti silent		/* #11 */
  	BUILD_HANDLER ov ov sti silent			/* #12 */

arch/mips/kernel/kprobes.c new file +0 -0

new file mode 100644
index 0000000..0493791
@@ -0,0 +1,380 @@

[...]

Quoted 29 lines

arch/mips/kernel/kprobes.c new file +29 -0

+
+int __kprobes arch_prepare_kprobe(struct kprobe *p)
+{
+	union mips_instruction insn;
+	int ret = 0;
+
+	insn = *p->addr;
+
+	switch (insn.i_format.opcode) {
+		/*
+		 * This group contains:
+		 * jr and jalr are in r_format format.
+		 */
+	case spec_op:
+
+		/*
+		 * This group contains:
+		 * bltz_op, bgez_op, bltzl_op, bgezl_op,
+		 * bltzal_op, bgezal_op, bltzall_op, bgezall_op.
+		 */
+	case bcond_op:
+
+		/*
+		 * These are unconditional and in j_format.
+		 */
+	case jal_op:
+	case j_op:
+
+		/*

arch/mips/kernel/kprobes.c line 72

Quoted 10 lines

arch/mips/kernel/kprobes.c new file +10 -0

+		 * These are conditional and in i_format.
+		 */
+	case beq_op:
+	case beql_op:
+	case bne_op:
+	case bnel_op:
+	case blez_op:
+	case blezl_op:
+	case bgtz_op:
+	case bgtzl_op:

Need to add or otherwise handle:

#ifdef CONFIG_CPU_CAVIUM_OCTEON
case lwc2_op: /* This is bbit0 on Octeon */ case ldc2_op: /* This is bbit032 on Octeon */ case swc2_op: /* This is bbit1 on Octeon */ case sdc2_op: /* This is bbit132 on Octeon */
#endif

[...]

Quoted 44 lines

arch/mips/kernel/traps.c +21 -1

index 8bdd6a6..f6b4b41 100644
@@ -27,6 +27,7 @@
  #include<linux/kdebug.h>
  #include<linux/notifier.h>
  #include<linux/kdb.h>
+#include<linux/kprobes.h>
 
  #include<asm/bootinfo.h>
  #include<asm/branch.h>

@@ -334,7 +335,7 @@ void show_regs(struct pt_regs *regs)
  	__show_regs((struct pt_regs *)regs);
  }
 
-void show_registers(const struct pt_regs *regs)
+void show_registers(struct pt_regs *regs)
  {
  	const int field = 2 * sizeof(unsigned long);

@@ -790,6 +791,43 @@ out_sigsegv:
  	force_sig(SIGSEGV, current);
  }
 
+#ifdef CONFIG_KPROBES
+asmlinkage void __kprobes do_break (struct pt_regs *regs)
+{
+	unsigned int opcode, bcode;
+
+	opcode = *(unsigned long *)(regs->cp0_epc);
+
+	bcode = ((opcode>>  6)&  ((1<<  20) - 1));
+	if (bcode<  (1<<  10))
+		bcode<<= 10;
+
+	/*
+	 * notify the kprobe handlers,if instruction is break 0 or break 5
+	 */
+	switch (bcode) {
+	case BRK_USERBP<<  10:
+		if (notify_die(DIE_BREAK, "debug", regs, bcode, 0, 0) == NOTIFY_STOP)
+			return;
+		else

arch/mips/kernel/traps.c out_sigsegv: · line 822

Quoted 10 lines

arch/mips/kernel/traps.c +10 -0

+			break;
+	case BRK_SSTEPBP<<  10:
+		if (notify_die(DIE_SSTEPBP, "single_step", regs, bcode, 0, 0) == NOTIFY_STOP)
+			return;
+		else
+			break;
+	default:
+		break;
+	}
+

This should be folded into do_bp().

Quoted 47 lines

arch/mips/kernel/traps.c +8 -0

+	/*
+	 * If the bcode is other than 0 and 5, then call the normal
+	 * break handler do_bp()
+	 */
+	do_bp(regs);
+}
+#endif
+
  asmlinkage void do_tr(struct pt_regs *regs)
  {
  	unsigned int opcode, tcode = 0;

arch/mips/mm/fault.c +9 -2

index b78f7d9..86e2d27 100644
@@ -18,6 +18,8 @@
  #include<linux/smp.h>
  #include<linux/vt_kern.h>		/* For unblank_screen() */
  #include<linux/module.h>
+#include<linux/kprobes.h>
+#include<linux/kdebug.h>		/* notify_die and asm/kdebug.h */
 
  #include<asm/branch.h>
  #include<asm/mmu_context.h>

@@ -31,8 +33,8 @@
   * and the problem, and then passes it off to one of the appropriate
   * routines.
   */
-asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long write,
-			      unsigned long address)
+asmlinkage void __kprobes do_page_fault(struct pt_regs *regs, unsigned long write,
+                                        unsigned long address)
  {
  	struct vm_area_struct * vma = NULL;
  	struct task_struct *tsk = current;

@@ -47,6 +49,11 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long write,
  	       field, regs->cp0_epc);
  #endif
 
+	/* Notify kprobes fault handler. */
+        if (notify_die(DIE_PAGE_FAULT, "page fault",
+                       regs, -1, SEGV_MAPERR, SEGV_MAPERR) == NOTIFY_STOP)
+                return;
+
  	info.si_code = SEGV_MAPERR;
 
  	/*

Himanshu Chauhan

author

2010-06-08 17:58 UTC #3 lore raw

Re: [PATCH] MIPS: KProbes support v0.1

Hi David,

Thanks for taking a look.

Quoted 11 lines

arch/mips/include/asm/kdebug.h enum die_val { · line 16
Quoted 7 lines
arch/mips/include/asm/kdebug.h +5 -0
+#ifdef CONFIG_KPROBES
+	DIE_PAGE_FAULT,
+	DIE_BREAK,
+	DIE_SSTEPBP,
+#endif
 };
It might be cleaner without the #ifdef. These are enum value definitions, so it doesn't affect code size.

Hmm.. I will remove this.

Quoted 3 lines

Can you also explain how the die notifier chain interacts with KProbes and why it cannot be a seperate notifier chain?

Actually, looking at x86 and other code, this is not the proper way to do it. I will try to comeup with common approach.

Quoted 22 lines

Quoted 8 lines
#endif /* _ASM_MIPS_KDEBUG_H */
arch/mips/include/asm/kprobes.h new file +0 -0
new file mode 100644
index 0000000..0f647bf
@@ -0,0 +1,85 @@
[...]
arch/mips/include/asm/kprobes.h line 45
Quoted 9 lines
arch/mips/include/asm/kprobes.h new file +9 -0
+
+#define BREAKPOINT_INSTRUCTION		0x0000000d
+
+/*
+ * We do not have hardware single-stepping on MIPS.
+ * So we implement software single-stepping with breakpoint
+ * trap 'break 5'.
+ */
+#define BREAKPOINT_INSTRUCTION_2	0x0000014d
The BREAK codes are defined in asm/break.h This should be added there instead.

Quoted 2 lines

Why do you use codes (0 and 5) that are already kind of reserved for user space debuggers?

As said ealier, this patch was based on some very older patch of 2.6.16 from Sony Corp, I didn't make much changes like this. But anyways, I wan't aware of this either. What would be the best code then?

Quoted 16 lines

arch/mips/include/asm/kprobes.h line 54
Quoted 10 lines
arch/mips/include/asm/kprobes.h new file +9 -0
+#define MAX_INSN_SIZE 			2
+
+#define flush_insn_slot(p)		do { \
+        /* invalidate I-cache */             \
+        asm volatile("cache 0, 0($0)");      \
+        /* invalidate D-cache */             \
+        asm volatile("cache 9, 0($0)");      \
+        } while(0);
+
You have to call a function in arch/mips/mm/c-* to do this, you cannot open code with CACHE instructions as you need to handle CPU quirks and SMP. It is possible that flush_icache_range() or flush_cache_sigtramp() would work. Or we might need something new.

Quoted 2 lines

I see you use flush_icache_range() below, why have this definition, it looks unused?

Framework needs you to define this.

Quoted 2 lines

Why this ugliness? Can't you handle it in do_bp() or do_trap_or_bp()?

Let me see what I can do about this.

Quoted 9 lines

Need to add or otherwise handle:
#ifdef CONFIG_CPU_CAVIUM_OCTEON
case lwc2_op: /* This is bbit0 on Octeon */ case ldc2_op: /* This is bbit032 on Octeon */ case swc2_op: /* This is bbit1 on Octeon */ case sdc2_op: /* This is bbit132 on Octeon */
#endif

Though I have worked on octeon before but I don't remember nitty-gritties. And I have no clue about these ops :) No way to test either!

With all that, I will soon send an updated patch.
Thanks for the review!

Regards
-Himanshu

David Daney

2010-06-11 00:13 UTC #4 lore raw

Re: [PATCH] MIPS: KProbes support v0.1

Quoted 4 lines · Himanshu Chauhan

Hi David,
Thanks for taking a look.

[...]

Quoted 13 lines

Quoted 13 lines
arch/mips/include/asm/kprobes.h line 45
Quoted 9 lines
arch/mips/include/asm/kprobes.h new file +9 -0
+
+#define BREAKPOINT_INSTRUCTION		0x0000000d
+
+/*
+ * We do not have hardware single-stepping on MIPS.
+ * So we implement software single-stepping with breakpoint
+ * trap 'break 5'.
+ */
+#define BREAKPOINT_INSTRUCTION_2	0x0000014d
The BREAK codes are defined in asm/break.h This should be added there instead.

Quoted 7 lines

Quoted 2 lines
Why do you use codes (0 and 5) that are already kind of reserved for user space debuggers?
As said ealier, this patch was based on some very older patch of 2.6.16 from Sony Corp, I didn't make much changes like this. But anyways, I wan't aware of this either. What would be the best code then?

How about allocating them after BRK_MEMU? Say 515 and 516 or something like that.

David Daney