Patchwork [4/5] powerpc: ftrace, use create_branch

login
register
mail settings
Submitter Steven Rostedt
Date Nov. 26, 2008, 9:58 p.m.
Message ID <20081126220423.514020907@goodmis.org>
Download mbox | patch
Permalink /patch/11012/
State Accepted, archived
Commit 0029ff87529dff01a4b9c5bf380a0caacb5f7418
Headers show

Comments

Steven Rostedt - Nov. 26, 2008, 9:58 p.m.
From: Steven Rostedt <srostedt@redhat.com>

Impact: clean up

Paul Mackerras pointed out that the code to determine if the branch
can reach the destination is incorrect. Michael Ellerman suggested
to pull out the code from create_branch and use that.

Simply using create_branch is probably the best.

Reported-by: Michael Ellerman <michael@ellerman.id.au>
Reported-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/powerpc/kernel/ftrace.c |   54 +++++++++--------------------------------
 1 files changed, 12 insertions(+), 42 deletions(-)
Michael Ellerman - Nov. 27, 2008, 10:06 p.m.
On Wed, 2008-11-26 at 16:58 -0500, Steven Rostedt wrote:
> plain text document attachment
> (0004-powerpc-ftrace-use-create_branch.patch)
> From: Steven Rostedt <srostedt@redhat.com>
> 
> Impact: clean up
> 
> Paul Mackerras pointed out that the code to determine if the branch
> can reach the destination is incorrect. Michael Ellerman suggested
> to pull out the code from create_branch and use that.
> 
> Simply using create_branch is probably the best.

You're right, that's a lot simpler :)

Acked-by: Michael Ellerman <michael@ellerman.id.au>

cheers

Patch

diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index a4640e4..5355244 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -114,19 +114,9 @@  ftrace_modify_code(unsigned long ip, unsigned char *old_code,
  */
 static int test_24bit_addr(unsigned long ip, unsigned long addr)
 {
-	long diff;
 
-	/*
-	 * Can we get to addr from ip in 24 bits?
-	 *  (26 really, since we mulitply by 4 for 4 byte alignment)
-	 */
-	diff = addr - ip;
-
-	/*
-	 * Return true if diff is less than 1 << 25
-	 *  and greater than -1 << 26.
-	 */
-	return (diff < (1 << 25)) && (diff > (-1 << 26));
+	/* use the create_branch to verify that this offset can be branched */
+	return create_branch((unsigned int *)ip, addr, 0);
 }
 
 static int is_bl_op(unsigned int op)
@@ -134,11 +124,6 @@  static int is_bl_op(unsigned int op)
 	return (op & 0xfc000003) == 0x48000001;
 }
 
-static int test_offset(unsigned long offset)
-{
-	return (offset + 0x2000000 > 0x3ffffff) || ((offset & 3) != 0);
-}
-
 static unsigned long find_bl_target(unsigned long ip, unsigned int op)
 {
 	static int offset;
@@ -151,12 +136,6 @@  static unsigned long find_bl_target(unsigned long ip, unsigned int op)
 	return ip + (long)offset;
 }
 
-static unsigned int branch_offset(unsigned long offset)
-{
-	/* return "bl ip+offset" */
-	return 0x48000001 | (offset & 0x03fffffc);
-}
-
 #ifdef CONFIG_PPC64
 static int
 __ftrace_make_nop(struct module *mod,
@@ -402,7 +381,6 @@  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned int op[2];
 	unsigned long ip = rec->ip;
-	unsigned long offset;
 
 	/* read where this goes */
 	if (probe_kernel_read(op, (void *)ip, MCOUNT_INSN_SIZE * 2))
@@ -424,17 +402,14 @@  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	/* now calculate a jump to the ftrace caller trampoline */
-	offset = rec->arch.mod->arch.tramp - ip;
-
-	if (test_offset(offset)) {
-		printk(KERN_ERR "REL24 %li out of range!\n",
-		       (long int)offset);
+	/* create the branch to the trampoline */
+	op[0] = create_branch((unsigned int *)ip,
+			      rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
+	if (!op[0]) {
+		printk(KERN_ERR "REL24 out of range!\n");
 		return -EINVAL;
 	}
 
-	/* Set to "bl addr" */
-	op[0] = branch_offset(offset);
 	/* ld r2,40(r1) */
 	op[1] = 0xe8410028;
 
@@ -453,7 +428,6 @@  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
 	unsigned int op;
 	unsigned long ip = rec->ip;
-	unsigned long offset;
 
 	/* read where this goes */
 	if (probe_kernel_read(&op, (void *)ip, MCOUNT_INSN_SIZE))
@@ -471,18 +445,14 @@  __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	/* now calculate a jump to the ftrace caller trampoline */
-	offset = rec->arch.mod->arch.tramp - ip;
-
-	if (test_offset(offset)) {
-		printk(KERN_ERR "REL24 %li out of range!\n",
-		       (long int)offset);
+	/* create the branch to the trampoline */
+	op = create_branch((unsigned int *)ip,
+			   rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
+	if (!op) {
+		printk(KERN_ERR "REL24 out of range!\n");
 		return -EINVAL;
 	}
 
-	/* Set to "bl addr" */
-	op = branch_offset(offset);
-
 	DEBUGP("write to %lx\n", rec->ip);
 
 	if (probe_kernel_write((void *)ip, &op, MCOUNT_INSN_SIZE))