sh: Fix dynamic ftrace's NOP action.

Ftrace on sh handles nop'ing out trace function calls differently than
other architectures. Instead of inserting NOP instructions in place of
the call to the function tracer we branch over the call instructions
and continue executing the main body of the function.

This patch fixes a bug in the implementation of ftrace_modify_code()
where we check that the old value of the code we're about to replace
is an expected one. In the ftrace_make_call() code path
ftrace_modify_code() was comparing the old instruction value with NOP
instructions. The compare was failing because we never actually insert
NOP instructions. It makes sense to just get rid of the NOP
instructions in ftrace_nop and compare the old code with the address
of the function body if we're expecting ftrace to have nop'd out the
function trace call.

Signed-off-by: Matt Fleming <matt@console-pimps.org>

diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 040cdc6..066f37d 100644 (file)
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -19,30 +19,37 @@
 #include <asm/ftrace.h>
 #include <asm/cacheflush.h>
 
-static unsigned char ftrace_nop[] = {
-       0x09, 0x00,             /* nop */
-       0x09, 0x00,             /* nop */
-};
-
 static unsigned char ftrace_replaced_code[MCOUNT_INSN_SIZE];
 
-unsigned char *ftrace_nop_replace(void)
+static unsigned char ftrace_nop[4];
+/*
+ * If we're trying to nop out a call to a function, we instead
+ * place a call to the address after the memory table.
+ *
+ * 8c011060 <a>:
+ * 8c011060:       02 d1           mov.l   8c01106c <a+0xc>,r1
+ * 8c011062:       22 4f           sts.l   pr,@-r15
+ * 8c011064:       02 c7           mova    8c011070 <a+0x10>,r0
+ * 8c011066:       2b 41           jmp     @r1
+ * 8c011068:       2a 40           lds     r0,pr
+ * 8c01106a:       09 00           nop
+ * 8c01106c:       68 24           .word 0x2468     <--- ip
+ * 8c01106e:       1d 8c           .word 0x8c1d
+ * 8c011070:       26 4f           lds.l   @r15+,pr <--- ip + MCOUNT_INSN_SIZE
+ *
+ * We write 0x8c011070 to 0x8c01106c so that on entry to a() we branch
+ * past the _mcount call and continue executing code like normal.
+ */
+static unsigned char *ftrace_nop_replace(unsigned long ip)
 {
+       __raw_writel(ip + MCOUNT_INSN_SIZE, ftrace_nop);
        return ftrace_nop;
 }
 
-static int is_sh_nop(unsigned char *ip)
-{
-       return strncmp(ip, ftrace_nop, sizeof(ftrace_nop));
-}
-
-unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
+static unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
 {
        /* Place the address in the memory table. */
-       if (addr == CALLER_ADDR)
-               __raw_writel(addr + MCOUNT_INSN_OFFSET, ftrace_replaced_code);
-       else
-               __raw_writel(addr, ftrace_replaced_code);
+       __raw_writel(addr, ftrace_replaced_code);
 
        /*
         * No locking needed, this must be called via kstop_machine
@@ -51,7 +58,7 @@ unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
        return ftrace_replaced_code;
 }
 
-int ftrace_modify_code(unsigned long ip, unsigned char *old_code,
+static int ftrace_modify_code(unsigned long ip, unsigned char *old_code,
                       unsigned char *new_code)
 {
        unsigned char replaced[MCOUNT_INSN_SIZE];
@@ -66,13 +73,6 @@ int ftrace_modify_code(unsigned long ip, unsigned char *old_code,
         * kstop_machine, or before SMP starts.
         */
 
-       /*
-        * If we're trying to nop out a call to a function, we instead
-        * place a call to the address after the memory table.
-        */
-       if (is_sh_nop(new_code) == 0)
-               __raw_writel(ip + MCOUNT_INSN_SIZE, (unsigned long)new_code);
-
        /* read the text we want to modify */
        if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
                return -EFAULT;
@@ -92,13 +92,13 @@ int ftrace_modify_code(unsigned long ip, unsigned char *old_code,
 
 int ftrace_update_ftrace_func(ftrace_func_t func)
 {
-       unsigned long ip = (unsigned long)(&ftrace_call);
+       unsigned long ip = (unsigned long)(&ftrace_call) + MCOUNT_INSN_OFFSET;
        unsigned char old[MCOUNT_INSN_SIZE], *new;
 
-       memcpy(old, (unsigned char *)(ip + MCOUNT_INSN_OFFSET), MCOUNT_INSN_SIZE);
+       memcpy(old, (unsigned char *)ip, MCOUNT_INSN_SIZE);
        new = ftrace_call_replace(ip, (unsigned long)func);
 
-       return ftrace_modify_code(ip + MCOUNT_INSN_OFFSET, old, new);
+       return ftrace_modify_code(ip, old, new);
 }
 
 int ftrace_make_nop(struct module *mod,
@@ -108,7 +108,7 @@ int ftrace_make_nop(struct module *mod,
        unsigned long ip = rec->ip;
 
        old = ftrace_call_replace(ip, addr);
-       new = ftrace_nop_replace();
+       new = ftrace_nop_replace(ip);
 
        return ftrace_modify_code(rec->ip, old, new);
 }
@@ -118,7 +118,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
        unsigned char *new, *old;
        unsigned long ip = rec->ip;
 
-       old = ftrace_nop_replace();
+       old = ftrace_nop_replace(ip);
        new = ftrace_call_replace(ip, addr);
 
        return ftrace_modify_code(rec->ip, old, new);