public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] MIPS Kprobes: Support branch instructions probing
@ 2011-10-13  9:07 Maneesh Soni
  2011-10-13  9:41 ` Ananth N Mavinakayanahalli
  2011-10-13 17:28 ` David Daney
  0 siblings, 2 replies; 10+ messages in thread
From: Maneesh Soni @ 2011-10-13  9:07 UTC (permalink / raw)
  To: ralf; +Cc: linux-kernel, linux-mips, ananth, david.daney, kamensky


From: Maneesh Soni <manesoni@cisco.com>

This patch provides support for kprobes on branch instructions. The branch
instruction at the probed address is actually emulated and not executed
out-of-line like other normal instructions. Instead the delay-slot instruction
is copied and single stepped out of line.

At the time of probe hit, the original branch instruction is evaluated
and the target cp0_epc is computed similar to compute_retrun_epc(). It
is also checked if the delay slot instruction can be skipped, which is
true if there is a NOP in delay slot or branch is taken in case of
branch likely instructions. Once the delay slot instruction is single
stepped the normal execution resume with the cp0_epc updated the earlier
computed cp0_epc as per the branch instructions.

Signed-off-by: Maneesh Soni <manesoni@cisco.com>
---
 arch/mips/include/asm/kprobes.h |    7 +
 arch/mips/kernel/kprobes.c      |  341 +++++++++++++++++++++++++++++++++++----
 2 files changed, 320 insertions(+), 28 deletions(-)

diff --git a/arch/mips/include/asm/kprobes.h b/arch/mips/include/asm/kprobes.h
index e6ea4d4..c0a3e6f 100644
--- a/arch/mips/include/asm/kprobes.h
+++ b/arch/mips/include/asm/kprobes.h
@@ -74,6 +74,8 @@ struct prev_kprobe {
 		: MAX_JPROBES_STACK_SIZE)
 
 
+#define SKIP_DELAYSLOT	1
+
 /* per-cpu kprobe control block */
 struct kprobe_ctlblk {
 	unsigned long kprobe_status;
@@ -81,6 +83,11 @@ struct kprobe_ctlblk {
 	unsigned long kprobe_saved_SR;
 	unsigned long kprobe_saved_epc;
 	unsigned long jprobe_saved_sp;
+
+	/* Per-thread fields, used while emulating branches */
+	unsigned long flags;
+	unsigned long target_epc;
+
 	struct pt_regs jprobe_saved_regs;
 	u8 jprobes_stack[MAX_JPROBES_STACK_SIZE];
 	struct prev_kprobe prev_kprobe;
diff --git a/arch/mips/kernel/kprobes.c b/arch/mips/kernel/kprobes.c
index ee28683..0a4a61c 100644
--- a/arch/mips/kernel/kprobes.c
+++ b/arch/mips/kernel/kprobes.c
@@ -121,8 +121,8 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 	prev_insn = p->addr[-1];
 	insn = p->addr[0];
 
-	if (insn_has_delayslot(insn) || insn_has_delayslot(prev_insn)) {
-		pr_notice("Kprobes for branch and jump instructions are not supported\n");
+	if (insn_has_delayslot(prev_insn)) {
+		pr_notice("Kprobes for branch delayslot are not supported\n");
 		ret = -EINVAL;
 		goto out;
 	}
@@ -138,9 +138,20 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
 	 * In the kprobe->ainsn.insn[] array we store the original
 	 * instruction at index zero and a break trap instruction at
 	 * index one.
+	 *
+	 * On MIPS arch if the instruction at probed address is a
+	 * branch instruction, we need to execute the instruction at
+	 * Branch Delayslot (BD) at the time of probe hit. As MIPS also
+	 * doesn't have single stepping support, the BD instruction can
+	 * not be executed in-line and it would be executed on SSOL slot
+	 * using a normal breakpoint instruction in the next slot.
+	 * So, read the instruction and save it for later execution.
 	 */
+	if (insn_has_delayslot(insn))
+		memcpy(&p->ainsn.insn[0], p->addr + 1, sizeof(kprobe_opcode_t));
+	else
+		memcpy(&p->ainsn.insn[0], p->addr, sizeof(kprobe_opcode_t));
 
-	memcpy(&p->ainsn.insn[0], p->addr, sizeof(kprobe_opcode_t));
 	p->ainsn.insn[1] = breakpoint2_insn;
 	p->opcode = *p->addr;
 
@@ -191,16 +202,297 @@ static void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
 	kcb->kprobe_saved_epc = regs->cp0_epc;
 }
 
-static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
+/*
+ * Borrowed heavily from arch/mips/kernel/branch.c:__compute_return_epc()
+ *
+ * Evaluate the branch instruction at probed address during probe hit. The
+ * result of evaluation would be the updated epc. The insturction in delayslot
+ * would actually be single stepped using a normal breakpoint) on SSOL slot.
+ *
+ * The result is also saved in the kprobe control block for later use,
+ * in case we need to execute the delayslot instruction. The latter will be
+ * false for NOP instruction in dealyslot and the branch-likely instructions
+ * when the branch is taken. And for those cases we set a flag as
+ * SKIP_DELAYSLOT in the kprobe control block
+ */
+static int evaluate_branch_instruction(struct kprobe *p, struct pt_regs *regs,
+					struct kprobe_ctlblk *kcb)
 {
+	union mips_instruction insn = p->opcode;
+	unsigned int dspcontrol;
+	long epc;
+
+	epc = regs->cp0_epc;
+	if (epc & 3)
+		goto unaligned;
+
+	if (p->ainsn.insn->word == 0)
+		kcb->flags |= SKIP_DELAYSLOT;
+	else
+		kcb->flags &= ~SKIP_DELAYSLOT;
+
+	switch (insn.i_format.opcode) {
+	/*
+	 * jr and jalr are in r_format format.
+	 */
+	case spec_op:
+		switch (insn.r_format.func) {
+		case jalr_op:
+			regs->regs[insn.r_format.rd] = epc + 8;
+
+			/* Fall through */
+		case jr_op:
+			regs->cp0_epc = regs->regs[insn.r_format.rs];
+			break;
+		}
+		break;
+
+	/*
+	 * This group contains:
+	 * bltz_op, bgez_op, bltzl_op, bgezl_op,
+	 * bltzal_op, bgezal_op, bltzall_op, bgezall_op.
+	 */
+	case bcond_op:
+		switch (insn.i_format.rt) {
+		case bltz_op:
+			if ((long)regs->regs[insn.i_format.rs] < 0)
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+			else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+
+		case bltzl_op:
+			if ((long)regs->regs[insn.i_format.rs] < 0) {
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+				kcb->flags |= SKIP_DELAYSLOT;
+			} else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+
+		case bgez_op:
+			if ((long)regs->regs[insn.i_format.rs] >= 0)
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+			else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+		case bgezl_op:
+			if ((long)regs->regs[insn.i_format.rs] >= 0) {
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+				kcb->flags |= SKIP_DELAYSLOT;
+			} else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+
+		case bltzal_op:
+			regs->regs[31] = epc + 8;
+			if ((long)regs->regs[insn.i_format.rs] < 0)
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+			else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+
+		case bltzall_op:
+			regs->regs[31] = epc + 8;
+			if ((long)regs->regs[insn.i_format.rs] < 0) {
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+				kcb->flags |= SKIP_DELAYSLOT;
+			} else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+
+		case bgezal_op:
+			regs->regs[31] = epc + 8;
+			if ((long)regs->regs[insn.i_format.rs] >= 0)
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+			else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+
+		case bgezall_op:
+			regs->regs[31] = epc + 8;
+			if ((long)regs->regs[insn.i_format.rs] >= 0) {
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+				kcb->flags |= SKIP_DELAYSLOT;
+			} else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+
+		case bposge32_op:
+			if (!cpu_has_dsp)
+				goto sigill;
+
+			dspcontrol = rddsp(0x01);
+
+			if (dspcontrol >= 32)
+				epc = epc + 4 + (insn.i_format.simmediate << 2);
+			else
+				epc += 8;
+			regs->cp0_epc = epc;
+			break;
+		}
+		break;
+
+	/*
+	 * These are unconditional and in j_format.
+	 */
+	case jal_op:
+		regs->regs[31] = regs->cp0_epc + 8;
+	case j_op:
+		epc += 4;
+		epc >>= 28;
+		epc <<= 28;
+		epc |= (insn.j_format.target << 2);
+		regs->cp0_epc = epc;
+		break;
+
+	/*
+	 * These are conditional and in i_format.
+	 */
+	case beq_op:
+		if (regs->regs[insn.i_format.rs] ==
+		    regs->regs[insn.i_format.rt])
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+		else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+
+	case beql_op:
+		if (regs->regs[insn.i_format.rs] ==
+		    regs->regs[insn.i_format.rt]) {
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+			kcb->flags |= SKIP_DELAYSLOT;
+		} else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+
+	case bne_op:
+		if (regs->regs[insn.i_format.rs] !=
+		    regs->regs[insn.i_format.rt])
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+		else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+
+	case bnel_op:
+		if (regs->regs[insn.i_format.rs] !=
+		    regs->regs[insn.i_format.rt]) {
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+			kcb->flags |= SKIP_DELAYSLOT;
+		} else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+
+	case blez_op: /* not really i_format */
+		/* rt field assumed to be zero */
+		if ((long)regs->regs[insn.i_format.rs] <= 0)
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+		else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+
+	case blezl_op:
+		/* rt field assumed to be zero */
+		if ((long)regs->regs[insn.i_format.rs] <= 0) {
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+			kcb->flags |= SKIP_DELAYSLOT;
+		} else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+
+	case bgtz_op:
+		/* rt field assumed to be zero */
+		if ((long)regs->regs[insn.i_format.rs] > 0)
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+		else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+
+	case bgtzl_op:
+		/* rt field assumed to be zero */
+		if ((long)regs->regs[insn.i_format.rs] > 0) {
+			epc = epc + 4 + (insn.i_format.simmediate << 2);
+			kcb->flags |= SKIP_DELAYSLOT;
+		} else
+			epc += 8;
+		regs->cp0_epc = epc;
+		break;
+	}
+
+	kcb->target_epc = regs->cp0_epc;
+
+	return 1;
+
+unaligned:
+	printk(KERN_ERR "%s: unaligned epc - sending SIGBUS.\n", current->comm);
+	force_sig(SIGBUS, current);
+	return -EFAULT;
+
+sigill:
+	printk(KERN_ERR "%s: DSP branch but not DSP ASE - sending SIGBUS.\n",
+		current->comm);
+	force_sig(SIGBUS, current);
+	return -EFAULT;
+
+}
+
+static void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
+						struct kprobe_ctlblk *kcb)
+{
+	int ret = 0;
+
 	regs->cp0_status &= ~ST0_IE;
 
 	/* single step inline if the instruction is a break */
 	if (p->opcode.word == breakpoint_insn.word ||
 	    p->opcode.word == breakpoint2_insn.word)
 		regs->cp0_epc = (unsigned long)p->addr;
-	else
-		regs->cp0_epc = (unsigned long)&p->ainsn.insn[0];
+	else if (insn_has_delayslot(p->opcode)) {
+		ret = evaluate_branch_instruction(p, regs, kcb);
+		if (ret < 0) {
+			printk(KERN_ERR "Kprobes: Error evaluating branch\n");
+			return;
+		}
+	}
+	regs->cp0_epc = (unsigned long)&p->ainsn.insn[0];
+}
+
+/*
+ * Called after single-stepping.  p->addr is the address of the
+ * instruction whose first byte has been replaced by the "break 0"
+ * instruction.  To avoid the SMP problems that can occur when we
+ * temporarily put back the original opcode to single-step, we
+ * single-stepped a copy of the instruction.  The address of this
+ * copy is p->ainsn.insn.
+ *
+ * This function prepares to return from the post-single-step
+ * breakpoint trap. In case of branch instructions, the target
+ * epc to be restored.
+ */
+static void __kprobes resume_execution(struct kprobe *p,
+				       struct pt_regs *regs,
+				       struct kprobe_ctlblk *kcb)
+{
+	if (insn_has_delayslot(p->opcode))
+		regs->cp0_epc = kcb->target_epc;
+	else {
+		unsigned long orig_epc = kcb->kprobe_saved_epc;
+		regs->cp0_epc = orig_epc + 4;
+	}
 }
 
 static int __kprobes kprobe_handler(struct pt_regs *regs)
@@ -239,8 +531,13 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 			save_previous_kprobe(kcb);
 			set_current_kprobe(p, regs, kcb);
 			kprobes_inc_nmissed_count(p);
-			prepare_singlestep(p, regs);
+			prepare_singlestep(p, regs, kcb);
 			kcb->kprobe_status = KPROBE_REENTER;
+			if (kcb->flags & SKIP_DELAYSLOT) {
+				resume_execution(p, regs, kcb);
+				restore_previous_kprobe(kcb);
+				preempt_enable_no_resched();
+			}
 			return 1;
 		} else {
 			if (addr->word != breakpoint_insn.word) {
@@ -284,8 +581,15 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 	}
 
 ss_probe:
-	prepare_singlestep(p, regs);
-	kcb->kprobe_status = KPROBE_HIT_SS;
+	prepare_singlestep(p, regs, kcb);
+	if (kcb->flags & SKIP_DELAYSLOT) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		if (p->post_handler)
+			p->post_handler(p, regs, 0);
+		resume_execution(p, regs, kcb);
+	} else
+		kcb->kprobe_status = KPROBE_HIT_SS;
+
 	return 1;
 
 no_kprobe:
@@ -294,25 +598,6 @@ no_kprobe:
 
 }
 
-/*
- * Called after single-stepping.  p->addr is the address of the
- * instruction whose first byte has been replaced by the "break 0"
- * instruction.  To avoid the SMP problems that can occur when we
- * temporarily put back the original opcode to single-step, we
- * single-stepped a copy of the instruction.  The address of this
- * copy is p->ainsn.insn.
- *
- * This function prepares to return from the post-single-step
- * breakpoint trap.
- */
-static void __kprobes resume_execution(struct kprobe *p,
-				       struct pt_regs *regs,
-				       struct kprobe_ctlblk *kcb)
-{
-	unsigned long orig_epc = kcb->kprobe_saved_epc;
-	regs->cp0_epc = orig_epc + 4;
-}
-
 static inline int post_kprobe_handler(struct pt_regs *regs)
 {
 	struct kprobe *cur = kprobe_running();
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13  9:07 [PATCH] MIPS Kprobes: Support branch instructions probing Maneesh Soni
@ 2011-10-13  9:41 ` Ananth N Mavinakayanahalli
  2011-10-13 10:12   ` Ananth N Mavinakayanahalli
  2011-10-13 10:12   ` Maneesh Soni
  2011-10-13 17:28 ` David Daney
  1 sibling, 2 replies; 10+ messages in thread
From: Ananth N Mavinakayanahalli @ 2011-10-13  9:41 UTC (permalink / raw)
  To: Maneesh Soni; +Cc: ralf, linux-kernel, linux-mips, david.daney, kamensky

On Thu, Oct 13, 2011 at 02:37:49PM +0530, Maneesh Soni wrote:

...

I know nothing of MIPS internals, but...
 
>  static int __kprobes kprobe_handler(struct pt_regs *regs)
> @@ -239,8 +531,13 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
>  			save_previous_kprobe(kcb);
>  			set_current_kprobe(p, regs, kcb);
>  			kprobes_inc_nmissed_count(p);
> -			prepare_singlestep(p, regs);
> +			prepare_singlestep(p, regs, kcb);
>  			kcb->kprobe_status = KPROBE_REENTER;
> +			if (kcb->flags & SKIP_DELAYSLOT) {
> +				resume_execution(p, regs, kcb);
> +				restore_previous_kprobe(kcb);
> +				preempt_enable_no_resched();
> +			}
>  			return 1;
>  		} else {
>  			if (addr->word != breakpoint_insn.word) {
> @@ -284,8 +581,15 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
>  	}
> 
>  ss_probe:
> -	prepare_singlestep(p, regs);
> -	kcb->kprobe_status = KPROBE_HIT_SS;
> +	prepare_singlestep(p, regs, kcb);
> +	if (kcb->flags & SKIP_DELAYSLOT) {
> +		kcb->kprobe_status = KPROBE_HIT_SSDONE;
> +		if (p->post_handler)
> +			p->post_handler(p, regs, 0);
> +		resume_execution(p, regs, kcb);

You are missing a preempt_disable_no_resched() here.

Ananth


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13  9:41 ` Ananth N Mavinakayanahalli
@ 2011-10-13 10:12   ` Ananth N Mavinakayanahalli
  2011-10-13 10:12   ` Maneesh Soni
  1 sibling, 0 replies; 10+ messages in thread
From: Ananth N Mavinakayanahalli @ 2011-10-13 10:12 UTC (permalink / raw)
  To: Maneesh Soni; +Cc: ralf, linux-kernel, linux-mips, david.daney, kamensky

On Thu, Oct 13, 2011 at 03:11:37PM +0530, Ananth N Mavinakayanahalli wrote:
> On Thu, Oct 13, 2011 at 02:37:49PM +0530, Maneesh Soni wrote:
> 
> ...
> 
> I know nothing of MIPS internals, but...
>  
> >  static int __kprobes kprobe_handler(struct pt_regs *regs)
> > @@ -239,8 +531,13 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
> >  			save_previous_kprobe(kcb);
> >  			set_current_kprobe(p, regs, kcb);
> >  			kprobes_inc_nmissed_count(p);
> > -			prepare_singlestep(p, regs);
> > +			prepare_singlestep(p, regs, kcb);
> >  			kcb->kprobe_status = KPROBE_REENTER;
> > +			if (kcb->flags & SKIP_DELAYSLOT) {
> > +				resume_execution(p, regs, kcb);
> > +				restore_previous_kprobe(kcb);
> > +				preempt_enable_no_resched();
> > +			}
> >  			return 1;
> >  		} else {
> >  			if (addr->word != breakpoint_insn.word) {
> > @@ -284,8 +581,15 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
> >  	}
> > 
> >  ss_probe:
> > -	prepare_singlestep(p, regs);
> > -	kcb->kprobe_status = KPROBE_HIT_SS;
> > +	prepare_singlestep(p, regs, kcb);
> > +	if (kcb->flags & SKIP_DELAYSLOT) {
> > +		kcb->kprobe_status = KPROBE_HIT_SSDONE;
> > +		if (p->post_handler)
> > +			p->post_handler(p, regs, 0);
> > +		resume_execution(p, regs, kcb);
> 
> You are missing a preempt_disable_no_resched() here.

Oops! I meant preempt_enable_no_resched().

Ananth


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13  9:41 ` Ananth N Mavinakayanahalli
  2011-10-13 10:12   ` Ananth N Mavinakayanahalli
@ 2011-10-13 10:12   ` Maneesh Soni
  1 sibling, 0 replies; 10+ messages in thread
From: Maneesh Soni @ 2011-10-13 10:12 UTC (permalink / raw)
  To: Ananth N Mavinakayanahalli
  Cc: ralf, linux-kernel, linux-mips, david.daney, kamensky

On Thu, Oct 13, 2011 at 03:11:38PM +0530, Ananth N Mavinakayanahalli wrote:
> On Thu, Oct 13, 2011 at 02:37:49PM +0530, Maneesh Soni wrote:
> 
> ...
> 
> I know nothing of MIPS internals, but...
>  
> >  static int __kprobes kprobe_handler(struct pt_regs *regs)
> > @@ -239,8 +531,13 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
> >  			save_previous_kprobe(kcb);
> >  			set_current_kprobe(p, regs, kcb);
> >  			kprobes_inc_nmissed_count(p);
> > -			prepare_singlestep(p, regs);
> > +			prepare_singlestep(p, regs, kcb);
> >  			kcb->kprobe_status = KPROBE_REENTER;
> > +			if (kcb->flags & SKIP_DELAYSLOT) {
> > +				resume_execution(p, regs, kcb);
> > +				restore_previous_kprobe(kcb);
> > +				preempt_enable_no_resched();
> > +			}
> >  			return 1;
> >  		} else {
> >  			if (addr->word != breakpoint_insn.word) {
> > @@ -284,8 +581,15 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
> >  	}
> > 
> >  ss_probe:
> > -	prepare_singlestep(p, regs);
> > -	kcb->kprobe_status = KPROBE_HIT_SS;
> > +	prepare_singlestep(p, regs, kcb);
> > +	if (kcb->flags & SKIP_DELAYSLOT) {
> > +		kcb->kprobe_status = KPROBE_HIT_SSDONE;
> > +		if (p->post_handler)
> > +			p->post_handler(p, regs, 0);
> > +		resume_execution(p, regs, kcb);
> 
> You are missing a preempt_disable_no_resched() here.
> 
ok.. you meant preempt_enable_no_resched(). Will add it in the next
version. Thanks for pointing it out.

Thanks
Maneesh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13  9:07 [PATCH] MIPS Kprobes: Support branch instructions probing Maneesh Soni
  2011-10-13  9:41 ` Ananth N Mavinakayanahalli
@ 2011-10-13 17:28 ` David Daney
  2011-10-13 18:07   ` Ralf Baechle
  2011-10-14 17:28   ` Maneesh Soni
  1 sibling, 2 replies; 10+ messages in thread
From: David Daney @ 2011-10-13 17:28 UTC (permalink / raw)
  To: manesoni; +Cc: ralf, linux-kernel, linux-mips, ananth, kamensky

On 10/13/2011 02:07 AM, Maneesh Soni wrote:
>
> From: Maneesh Soni<manesoni@cisco.com>
>
> This patch provides support for kprobes on branch instructions. The branch
> instruction at the probed address is actually emulated and not executed
> out-of-line like other normal instructions. Instead the delay-slot instruction
> is copied and single stepped out of line.
>
> At the time of probe hit, the original branch instruction is evaluated
> and the target cp0_epc is computed similar to compute_retrun_epc(). It
> is also checked if the delay slot instruction can be skipped, which is
> true if there is a NOP in delay slot or branch is taken in case of
> branch likely instructions. Once the delay slot instruction is single
> stepped the normal execution resume with the cp0_epc updated the earlier
> computed cp0_epc as per the branch instructions.
>

I haven't tested it but...


> Signed-off-by: Maneesh Soni<manesoni@cisco.com>
> ---
>   arch/mips/include/asm/kprobes.h |    7 +
>   arch/mips/kernel/kprobes.c      |  341 +++++++++++++++++++++++++++++++++++----
>   2 files changed, 320 insertions(+), 28 deletions(-)
>
[...]
> +static int evaluate_branch_instruction(struct kprobe *p, struct pt_regs *regs,
> +					struct kprobe_ctlblk *kcb)
>   {
> +	union mips_instruction insn = p->opcode;
> +	unsigned int dspcontrol;
> +	long epc;
> +
> +	epc = regs->cp0_epc;
> +	if (epc&  3)
> +		goto unaligned;
> +
> +	if (p->ainsn.insn->word == 0)
> +		kcb->flags |= SKIP_DELAYSLOT;
> +	else
> +		kcb->flags&= ~SKIP_DELAYSLOT;
> +
> +	switch (insn.i_format.opcode) {
> +	/*
> +	 * jr and jalr are in r_format format.
> +	 */
> +	case spec_op:
[...]
> +	case bgtzl_op:
> +		/* rt field assumed to be zero */
> +		if ((long)regs->regs[insn.i_format.rs]>  0) {
> +			epc = epc + 4 + (insn.i_format.simmediate<<  2);
> +			kcb->flags |= SKIP_DELAYSLOT;
> +		} else
> +			epc += 8;
> +		regs->cp0_epc = epc;
> +		break;



Where is the handling for:

	case cop1_op:

#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

These are all defined in insn_has_delayslot() but not here.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13 17:28 ` David Daney
@ 2011-10-13 18:07   ` Ralf Baechle
  2011-10-13 19:16     ` Victor Kamensky
  2011-10-14 17:31     ` Maneesh Soni
  2011-10-14 17:28   ` Maneesh Soni
  1 sibling, 2 replies; 10+ messages in thread
From: Ralf Baechle @ 2011-10-13 18:07 UTC (permalink / raw)
  To: David Daney; +Cc: manesoni, linux-kernel, linux-mips, ananth, kamensky

On Thu, Oct 13, 2011 at 10:28:51AM -0700, David Daney wrote:

> Where is the handling for:
> 
> 	case cop1_op:
> 
> #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
> 
> These are all defined in insn_has_delayslot() but not here.

Which is a wonderful demonstration for why duplicating such a large
function from branch.c was a baaad thing to do.

Maneesh, can you refactor the code to share everything that was copied
from __compute_return_epc() can be shared with kprobes?  Idealy make
everything a two part series, first one patch to refactor branch.c and
the 2nd patch to deal with kprobes.

Thanks,

  Ralf

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13 18:07   ` Ralf Baechle
@ 2011-10-13 19:16     ` Victor Kamensky
  2011-10-13 22:59       ` Ralf Baechle
  2011-10-14 17:31     ` Maneesh Soni
  1 sibling, 1 reply; 10+ messages in thread
From: Victor Kamensky @ 2011-10-13 19:16 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: David Daney, manesoni, linux-kernel, linux-mips, ananth



On Thu, 13 Oct 2011, Ralf Baechle wrote:

> On Thu, Oct 13, 2011 at 10:28:51AM -0700, David Daney wrote:
>
> > Where is the handling for:
> >
> > 	case cop1_op:
> >
> > #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
> >
> > These are all defined in insn_has_delayslot() but not here.

David, nice catch!

> Which is a wonderful demonstration for why duplicating such a large
> function from branch.c was a baaad thing to do.
>
> Maneesh, can you refactor the code to share everything that was copied
> from __compute_return_epc() can be shared with kprobes?  Idealy make
> everything a two part series, first one patch to refactor branch.c

Yes, it does make a lot of sense. Don't you think we need to do
EXPORT_SYMBOL for __compute_return_epc as well? So it could be used by
klms.

Actually we have yet another copy of it in mips uprobes code, which
normally is built as klm, if we refactor and export __compute_return_epc
all three places could use the same function.

Thanks,
Victor

> and
> the 2nd patch to deal with kprobes.
>
> Thanks,
>
>   Ralf
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13 19:16     ` Victor Kamensky
@ 2011-10-13 22:59       ` Ralf Baechle
  0 siblings, 0 replies; 10+ messages in thread
From: Ralf Baechle @ 2011-10-13 22:59 UTC (permalink / raw)
  To: Victor Kamensky; +Cc: David Daney, manesoni, linux-kernel, linux-mips, ananth

On Thu, Oct 13, 2011 at 12:16:27PM -0700, Victor Kamensky wrote:

> Yes, it does make a lot of sense. Don't you think we need to do
> EXPORT_SYMBOL for __compute_return_epc as well? So it could be used by
> klms.
> 
> Actually we have yet another copy of it in mips uprobes code, which
> normally is built as klm, if we refactor and export __compute_return_epc
> all three places could use the same function.

Nothing wrong with exporting __compute_return_epc() as long as there are
actually users of the exported symbol.  So far it's only used from the
kernel proper which is why it's not been exported.  However I'd prefer it
to be exported as EXPORT_SYMBOL_GPL().

  Ralf

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13 17:28 ` David Daney
  2011-10-13 18:07   ` Ralf Baechle
@ 2011-10-14 17:28   ` Maneesh Soni
  1 sibling, 0 replies; 10+ messages in thread
From: Maneesh Soni @ 2011-10-14 17:28 UTC (permalink / raw)
  To: David Daney; +Cc: ralf, linux-kernel, linux-mips, ananth, kamensky

On Thu, Oct 13, 2011 at 10:28:51AM -0700, David Daney wrote:
> On 10/13/2011 02:07 AM, Maneesh Soni wrote:
> >
> >From: Maneesh Soni<manesoni@cisco.com>
> >
> >This patch provides support for kprobes on branch instructions. The branch
> >instruction at the probed address is actually emulated and not executed
> >out-of-line like other normal instructions. Instead the delay-slot instruction
> >is copied and single stepped out of line.
> >
> >At the time of probe hit, the original branch instruction is evaluated
> >and the target cp0_epc is computed similar to compute_retrun_epc(). It
> >is also checked if the delay slot instruction can be skipped, which is
> >true if there is a NOP in delay slot or branch is taken in case of
> >branch likely instructions. Once the delay slot instruction is single
> >stepped the normal execution resume with the cp0_epc updated the earlier
> >computed cp0_epc as per the branch instructions.
> >
> 
> I haven't tested it but...
> 
> 
> >Signed-off-by: Maneesh Soni<manesoni@cisco.com>
> >---
> >  arch/mips/include/asm/kprobes.h |    7 +
> >  arch/mips/kernel/kprobes.c      |  341 +++++++++++++++++++++++++++++++++++----
> >  2 files changed, 320 insertions(+), 28 deletions(-)
> >
> [...]
> >+static int evaluate_branch_instruction(struct kprobe *p, struct pt_regs *regs,
> >+					struct kprobe_ctlblk *kcb)
> >  {
> >+	union mips_instruction insn = p->opcode;
> >+	unsigned int dspcontrol;
> >+	long epc;
> >+
> >+	epc = regs->cp0_epc;
> >+	if (epc&  3)
> >+		goto unaligned;
> >+
> >+	if (p->ainsn.insn->word == 0)
> >+		kcb->flags |= SKIP_DELAYSLOT;
> >+	else
> >+		kcb->flags&= ~SKIP_DELAYSLOT;
> >+
> >+	switch (insn.i_format.opcode) {
> >+	/*
> >+	 * jr and jalr are in r_format format.
> >+	 */
> >+	case spec_op:
> [...]
> >+	case bgtzl_op:
> >+		/* rt field assumed to be zero */
> >+		if ((long)regs->regs[insn.i_format.rs]>  0) {
> >+			epc = epc + 4 + (insn.i_format.simmediate<<  2);
> >+			kcb->flags |= SKIP_DELAYSLOT;
> >+		} else
> >+			epc += 8;
> >+		regs->cp0_epc = epc;
> >+		break;
> 
> 
> 
> Where is the handling for:
> 
> 	case cop1_op:
> 
> #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
> 
> These are all defined in insn_has_delayslot() but not here.

My bad.. will include them as well. Actually as Ralf suggested,
will keep this as common code.

Thanks
Maneesh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] MIPS Kprobes: Support branch instructions probing
  2011-10-13 18:07   ` Ralf Baechle
  2011-10-13 19:16     ` Victor Kamensky
@ 2011-10-14 17:31     ` Maneesh Soni
  1 sibling, 0 replies; 10+ messages in thread
From: Maneesh Soni @ 2011-10-14 17:31 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: David Daney, linux-kernel, linux-mips, ananth, kamensky

On Thu, Oct 13, 2011 at 07:07:14PM +0100, Ralf Baechle wrote:
> On Thu, Oct 13, 2011 at 10:28:51AM -0700, David Daney wrote:
> 
> > Where is the handling for:
> > 
> > 	case cop1_op:
> > 
> > #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
> > 
> > These are all defined in insn_has_delayslot() but not here.
> 
> Which is a wonderful demonstration for why duplicating such a large
> function from branch.c was a baaad thing to do.
> 
> Maneesh, can you refactor the code to share everything that was copied
> from __compute_return_epc() can be shared with kprobes?  Idealy make
> everything a two part series, first one patch to refactor branch.c and
> the 2nd patch to deal with kprobes.
> 

Sure.. the branch likely instructions are not make it look good but
still do it in the next version.

Thanks for the comments.

Regards,
Maneesh

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-10-14 17:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-13  9:07 [PATCH] MIPS Kprobes: Support branch instructions probing Maneesh Soni
2011-10-13  9:41 ` Ananth N Mavinakayanahalli
2011-10-13 10:12   ` Ananth N Mavinakayanahalli
2011-10-13 10:12   ` Maneesh Soni
2011-10-13 17:28 ` David Daney
2011-10-13 18:07   ` Ralf Baechle
2011-10-13 19:16     ` Victor Kamensky
2011-10-13 22:59       ` Ralf Baechle
2011-10-14 17:31     ` Maneesh Soni
2011-10-14 17:28   ` Maneesh Soni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox