Fixes the modules support of dynamic function tracer for MIPS

From: Wu Zhangjin<[email protected]>

The module space may be the same as the kernel space sometimes(with related kernel config and gcc options), but the current in_module() assume they are always different, so, it should be fixed.

As we know, for the limitation of the fixed 32bit long instruction of MIPS, the "jal target" can only jump to an address whose offset from the jal instruction is smaller than 2^28=256M, which means if the address is in kernel space(the kernel image should be always smaller than 256M), no long call is needed to jump from the address to _mcount, and further, if the module use the same space as kernel space, the situation for module will be the same as the kernel. but currently for MIPS, in most of the situations, module has different space(0xc0000000) from the kernel space(0x80000000) and the offset is bigger than 256M, a register is needed to load the address of _mcount and another instruction "jal <register>" is needed to jump from an address to _mcount.

The above can be explained as:

if (in_kernel_space(addr)) {
	jal _mcount;
} else {

load the address of _mcount to a register
jalr<register>
}

As we can see, the above explanation covers all of the situations well and reflect the reality, so, we decide to add a new in_kernel_space() instead of the old in_module().

Based on core_kernel_text() from kernel/extable.c, in_kernel_space() is easily to be added. Because all of the functions in the init sections is anotated with notrace, so, differ from core_kernel_text(), in_kernel_space() doesn't care about init section.

With this new in_kernel_space(), the ftrace_make_{nop,call} can be explained as following.

ftrace_make_call() ftrace_make_nop()

if (in_kernel_space(addr)) {
	jal _mcount;	(jal ftrace_caller)	<-->	nop
} else {
	load the address of _mcount to a register<-->	b label
	jalr<register>
	label:
}

Signed-off-by Wu Zhangjin<[email protected]>

---
  arch/mips/kernel/ftrace.c |   66 ++++++++++++++++++++++++--------------------
  1 files changed, 36 insertions(+), 30 deletions(-)

arch/mips/kernel/ftrace.c +36 -30

index 65f1949..6ff5a54 100644
@@ -17,21 +17,7 @@
  #include<asm/cacheflush.h>
  #include<asm/uasm.h>
 
-/*
- * If the Instruction Pointer is in module space (0xc0000000), return true;
- * otherwise, it is in kernel space (0x80000000), return false.
- *
- * FIXME: This will not work when the kernel space and module space are the
- * same. If they are the same, we need to modify scripts/recordmcount.pl,
- * ftrace_make_nop/call() and the other related parts to ensure the
- * enabling/disabling of the calling site to _mcount is right for both kernel
- * and module.
- */
-
-static inline int in_module(unsigned long ip)
-{
-	return ip&  0x40000000;
-}
+#include<asm-generic/sections.h>
 
  #ifdef CONFIG_DYNAMIC_FTRACE
 

@@ -69,6 +55,20 @@ static inline void ftrace_dyn_arch_init_insns(void)
  #endif
  }
 
+/*
+ * Check if the address is in kernel space
+ *
+ * Clone core_kernel_text() from kernel/extable.c, but don't call
+ * init_kernel_text() for Ftrace don't trace functions in init sections.
+ */
+static inline int in_kernel_space(unsigned long ip)
+{
+	if (ip>= (unsigned long)_stext&&
+	    ip<= (unsigned long)_etext)
+		return 1;
+	return 0;
+}
+
  static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
  {
  	int faulted;

@@ -91,10 +91,16 @@ int ftrace_make_nop(struct module *mod,
  	unsigned long ip = rec->ip;
 
  	/*
-	 * We have compiled module with -mlong-calls, but compiled the kernel
-	 * without it, we need to cope with them respectively.
+	 * If ip is in kernel space, long call is not needed, otherwise, it is
+	 * needed.
  	 */
-	if (in_module(ip)) {
+	if (in_kernel_space(ip)) {
+		/*
+		 * move at, ra
+		 * jal _mcount		-->  nop
+		 */
+		new = INSN_NOP;
+	} else {
  #if defined(KBUILD_MCOUNT_RA_ADDRESS)&&  defined(CONFIG_32BIT)
  		/*
  		 * lui v1, hi_16bit_of_mcount        -->  b 1f (0x10000005)

@@ -117,12 +123,6 @@ int ftrace_make_nop(struct module *mod,
  		 */
  		new = INSN_B_1F_4;
  #endif
-	} else {
-		/*
-		 * move at, ra
-		 * jal _mcount		-->  nop
-		 */
-		new = INSN_NOP;
  	}
  	return ftrace_modify_code(ip, new);
  }

@@ -132,8 +132,12 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
  	unsigned int new;
  	unsigned long ip = rec->ip;
 
-	/* ip, module: 0xc0000000, kernel: 0x80000000 */
-	new = in_module(ip) ? insn_lui_v1_hi16_mcount : insn_jal_ftrace_caller;
+	/*
+	 * If ip is in kernel space, long call is not needed, otherwise, it is
+	 * needed.
+	 */
+	new = in_kernel_space(ip) ? insn_jal_ftrace_caller :
+		insn_lui_v1_hi16_mcount;
 
  	return ftrace_modify_code(ip, new);
  }

@@ -200,11 +204,13 @@ unsigned long ftrace_get_parent_addr(unsigned long self_addr,
  	int faulted;
 
  	/*
-	 * For module, move the ip from calling site of mcount after the
-	 * instruction "lui v1, hi_16bit_of_mcount"(offset is 24), but for
-	 * kernel, move after the instruction "move ra, at"(offset is 16)
+	 * If self_addr is in kernel space, long call is not needed, only need
+	 * to move ip after the instruction "move ra, at"(offset is 16),
+	 * otherwise, long call is needed, need to move ip from calling site of
+	 * mcount after the instruction "lui v1, hi_16bit_of_mcount"(offset is
+	 * 24).
  	 */
-	ip = self_addr - (in_module(self_addr) ? 24 : 16);
+	ip = self_addr - (in_kernel_space(self_addr) ? 16 : 24);
 
  	/*
  	 * search the text until finding the non-store instruction or "s{d,w}

Quoted 19 lines · Wu Zhangjin

From: Wu Zhangjin<[email protected]>

The module space may be the same as the kernel space sometimes(with related kernel config and gcc options), but the current in_module() assume they are always different, so, it should be fixed.

As we know, for the limitation of the fixed 32bit long instruction of MIPS, the "jal target" can only jump to an address whose offset from the jal instruction is smaller than 2^28=256M, which means if the address is in kernel space(the kernel image should be always smaller than 256M), no long call is needed to jump from the address to _mcount, and further, if the module use the same space as kernel space, the situation for module will be the same as the kernel. but currently for MIPS, in most of the situations, module has different space(0xc0000000) from the kernel space(0x80000000) and the offset is bigger than 256M, a register is needed to load the address of _mcount and another instruction "jal <register>" is needed to jump from an address to _mcount.

Quoted 9 lines

The above can be explained as:

if (in_kernel_space(addr)) {
       jal _mcount;
} else {
       load the address of _mcount to a register
       jalr<register>
}

Why can't we just do code examination to determine which case we are patching? There is a very limited and fixed set of instruction sequences that GCC will emit for _mcount calls.  We can easily check for all of them.

The mcount call sequence unconditionally emits:

       MOVE $1,$31

In the JALR case (for -mlong-call), the instruction immediately before this will have the target register be '$3'.

I would rather directly check which case we are patching rather than trying to infer it from the address.

Is there any case where a 'stock' kernel currently fails?  If not, this patch seems like unneeded code churn.

Quoted 158 lines

As we can see, the above explanation covers all of the situations well and reflect the reality, so, we decide to add a new in_kernel_space() instead of the old in_module().

Based on core_kernel_text() from kernel/extable.c, in_kernel_space() is easily to be added. Because all of the functions in the init sections is anotated with notrace, so, differ from core_kernel_text(), in_kernel_space() doesn't care about init section.

With this new in_kernel_space(), the ftrace_make_{nop,call} can be explained as following.

ftrace_make_call()                                      ftrace_make_nop()

if (in_kernel_space(addr)) {
       jal _mcount;    (jal ftrace_caller)     <-->    nop } else {
       load the address of _mcount to a register<-->   b label jalr<register>
       label:
}

Signed-off-by Wu Zhangjin<[email protected]>

---
 arch/mips/kernel/ftrace.c |   66
++++++++++++++++++++++++--------------------

 1 files changed, 36 insertions(+), 30 deletions(-)

arch/mips/kernel/ftrace.c +0 -5

index 65f1949..6ff5a54 100644
@@ -17,21 +17,7 @@
  #include<asm/cacheflush.h>
  #include<asm/uasm.h>
 
-/*
- * If the Instruction Pointer is in module space (0xc0000000), return
 true;
- * otherwise, it is in kernel space (0x80000000), return false.
- *
- * FIXME: This will not work when the kernel space and module space are

the
- * same. If they are the same, we need to modify scripts/recordmcount.pl,
- * ftrace_make_nop/call() and the other related parts to ensure the
- * enabling/disabling of the calling site to _mcount is right for both kernel
- * and module.
- */

arch/mips/kernel/ftrace.c +1 -5

-
-static inline int in_module(unsigned long ip)
-{
-       return ip&  0x40000000;
-}
+#include<asm-generic/sections.h>

 #ifdef CONFIG_DYNAMIC_FTRACE

arch/mips/kernel/ftrace.c +14 -1

@@ -69,6 +55,20 @@ static inline void ftrace_dyn_arch_init_insns(void)
  #endif
  }
 
+/*
+ * Check if the address is in kernel space
+ *
+ * Clone core_kernel_text() from kernel/extable.c, but don't call
+ * init_kernel_text() for Ftrace don't trace functions in init sections.
+ */
+static inline int in_kernel_space(unsigned long ip)
+{
+       if (ip>= (unsigned long)_stext&&
+           ip<= (unsigned long)_etext)
+               return 1;
+       return 0;
+}
+
  static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
  {
        int faulted;

@@ -91,10 +91,16 @@ int ftrace_make_nop(struct module *mod,
        unsigned long ip = rec->ip;
 
        /*
-        * We have compiled module with -mlong-calls, but compiled the

kernel
-        * without it, we need to cope with them respectively.
+        * If ip is in kernel space, long call is not needed, otherwise, it is
+        * needed.
        */
-       if (in_module(ip)) {
+       if (in_kernel_space(ip)) {
+               /*
+                * move at, ra
+                * jal _mcount          -->  nop
+                */
+               new = INSN_NOP;
+       } else {
 #if defined(KBUILD_MCOUNT_RA_ADDRESS)&&  defined(CONFIG_32BIT) /*
                * lui v1, hi_16bit_of_mcount        -->  b 1f (0x10000005)

arch/mips/kernel/ftrace.c +2 -8

@@ -117,12 +123,6 @@ int ftrace_make_nop(struct module *mod,
                 */
                new = INSN_B_1F_4;
  #endif
-       } else {
-               /*
-                * move at, ra
-                * jal _mcount          -->  nop
-                */
-               new = INSN_NOP;
        }
        return ftrace_modify_code(ip, new);
  }

@@ -132,8 +132,12 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned
 long addr)
        unsigned int new;
        unsigned long ip = rec->ip;
 
-       /* ip, module: 0xc0000000, kernel: 0x80000000 */
-       new = in_module(ip) ? insn_lui_v1_hi16_mcount :
 insn_jal_ftrace_caller;
+       /*
+        * If ip is in kernel space, long call is not needed, otherwise,

it is
+        * needed.
+        */
+       new = in_kernel_space(ip) ? insn_jal_ftrace_caller :
+               insn_lui_v1_hi16_mcount;

       return ftrace_modify_code(ip, new);
 }

arch/mips/kernel/ftrace.c +1 -3

@@ -200,11 +204,13 @@ unsigned long ftrace_get_parent_addr(unsigned long
 self_addr,
        int faulted;
 
        /*
-        * For module, move the ip from calling site of mcount after the
-        * instruction "lui v1, hi_16bit_of_mcount"(offset is 24), but for
-        * kernel, move after the instruction "move ra, at"(offset is 16)
+        * If self_addr is in kernel space, long call is not needed, only

need
+        * to move ip after the instruction "move ra, at"(offset is 16),
+        * otherwise, long call is needed, need to move ip from calling site of
+        * mcount after the instruction "lui v1, hi_16bit_of_mcount"(offset is
+        * 24).
        */
-       ip = self_addr - (in_module(self_addr) ? 24 : 16);
+       ip = self_addr - (in_kernel_space(self_addr) ? 16 : 24);

       /*
        * search the text until finding the non-store instruction or "s{d,w}

From: Wu Zhangjin <[email protected]>

The module space may be the same as the kernel space sometimes(with related kernel config and gcc options), but the current in_module() assume they are always different, so, it should be fixed.

As we know, for the limitation of the fixed 32bit long instruction of MIPS, the "jal target" can only jump to an address whose offset from the jal instruction is smaller than 2^28=256M, which means if the address is in kernel space(the kernel image should be always smaller than 256M), no long call is needed to jump from the address to _mcount, and further, if the module use the same space as kernel space, the situation for module will be the same as the kernel. but currently for MIPS, in most of the situations, module has different space(0xc0000000) from the kernel space(0x80000000) and the offset is bigger than 256M, a register is needed to load the address of _mcount and another instruction "jal <register>" is needed to jump from an address to _mcount.

The above can be explained as:

if (in_kernel_space(addr)) {
       jal _mcount;
} else {
       load the address of _mcount to a register
       jalr <register>
}

As we can see, the above explanation covers all of the situations well and reflect the reality, so, we decide to add a new in_kernel_space() instead of the old in_module().

Based on core_kernel_text() from kernel/extable.c, in_kernel_space() is easily to be added. Because all of the functions in the init sections is anotated with notrace, so, differ from core_kernel_text(), in_kernel_space() doesn't care about init section.

With this new in_kernel_space(), the ftrace_make_{nop,call} can be explained as following.

ftrace_make_call()                                      ftrace_make_nop()

if (in_kernel_space(addr)) {
       jal _mcount;    (jal ftrace_caller)       <-->  nop } else {
       load the address of _mcount to a register <-->  b label jalr <register>
       label:
}

Signed-off-by Wu Zhangjin <[email protected]>

---
 arch/mips/kernel/ftrace.c |   66 ++++++++++++++++++++++++--------------------

 1 files changed, 36 insertions(+), 30 deletions(-)

arch/mips/kernel/ftrace.c +36 -31

index 65f1949..6ff5a54 100644
@@ -17,21 +17,7 @@
  #include <asm/cacheflush.h>
  #include <asm/uasm.h>
 
-/*
- * If the Instruction Pointer is in module space (0xc0000000), return true;
- * otherwise, it is in kernel space (0x80000000), return false.
- *
- * FIXME: This will not work when the kernel space and module space are the
- * same. If they are the same, we need to modify scripts/recordmcount.pl,
- * ftrace_make_nop/call() and the other related parts to ensure the
- * enabling/disabling of the calling site to _mcount is right for both kernel
- * and module.
- */
-
-static inline int in_module(unsigned long ip)
-{
-       return ip & 0x40000000;
-}
+#include <asm-generic/sections.h>
 
  #ifdef CONFIG_DYNAMIC_FTRACE
 

@@ -69,6 +55,20 @@ static inline void ftrace_dyn_arch_init_insns(void)
  #endif
  }
 
+/*
+ * Check if the address is in kernel space
+ *
+ * Clone core_kernel_text() from kernel/extable.c, but don't call
+ * init_kernel_text() for Ftrace don't trace functions in init sections.
+ */
+static inline int in_kernel_space(unsigned long ip)
+{
+       if (ip >= (unsigned long)_stext &&
+           ip <= (unsigned long)_etext)
+               return 1;
+       return 0;
+}
+
  static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
  {
        int faulted;

@@ -91,10 +91,16 @@ int ftrace_make_nop(struct module *mod,
        unsigned long ip = rec->ip;
 
        /*
-        * We have compiled module with -mlong-calls, but compiled the kernel
-        * without it, we need to cope with them respectively.
+        * If ip is in kernel space, long call is not needed, otherwise, it is
+        * needed.
         */
-       if (in_module(ip)) {
+       if (in_kernel_space(ip)) {
+               /*
+                * move at, ra
+                * jal _mcount          --> nop
+                */
+               new = INSN_NOP;
+       } else {
  #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
                /*
                 * lui v1, hi_16bit_of_mcount        --> b 1f (0x10000005)

@@ -117,12 +123,6 @@ int ftrace_make_nop(struct module *mod,
                 */
                new = INSN_B_1F_4;
  #endif
-       } else {
-               /*
-                * move at, ra
-                * jal _mcount          --> nop
-                */
-               new = INSN_NOP;
        }
        return ftrace_modify_code(ip, new);
  }

@@ -132,8 +132,12 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
        unsigned int new;
        unsigned long ip = rec->ip;
 
-       /* ip, module: 0xc0000000, kernel: 0x80000000 */
-       new = in_module(ip) ? insn_lui_v1_hi16_mcount : insn_jal_ftrace_caller;
+       /*
+        * If ip is in kernel space, long call is not needed, otherwise, it is
+        * needed.
+        */
+       new = in_kernel_space(ip) ? insn_jal_ftrace_caller :
+               insn_lui_v1_hi16_mcount;
 
        return ftrace_modify_code(ip, new);
  }

@@ -200,11 +204,13 @@ unsigned long ftrace_get_parent_addr(unsigned long self_addr,
        int faulted;
 
        /*
-        * For module, move the ip from calling site of mcount after the
-        * instruction "lui v1, hi_16bit_of_mcount"(offset is 24), but for
-        * kernel, move after the instruction "move ra, at"(offset is 16)
+        * If self_addr is in kernel space, long call is not needed, only need
+        * to move ip after the instruction "move ra, at"(offset is 16),
+        * otherwise, long call is needed, need to move ip from calling site of
+        * mcount after the instruction "lui v1, hi_16bit_of_mcount"(offset is
+        * 24).
         */
-       ip = self_addr - (in_module(self_addr) ? 24 : 16);
+       ip = self_addr - (in_kernel_space(self_addr) ? 16 : 24);
 
        /*
         * search the text until finding the non-store instruction or "s{d,w}
--

1.7.1

From: Wu Zhangjin<[email protected]>

In some situations(with related kernel config and gcc options), the modules may have the same address space as the core kernel space, so mcount_regex for modules should also match R_MIPS_26.

Signed-off-by Wu Zhangjin<[email protected]>

---
  scripts/recordmcount.pl |   46 +++++++++++++++++++++++++++++-----------------
  1 files changed, 29 insertions(+), 17 deletions(-)

scripts/recordmcount.pl +29 -17

index e67f054..e9c1a0f 100755
@@ -299,14 +299,33 @@ if ($arch eq "x86_64") {
      $cc .= " -m64";
      $objcopy .= " -O elf64-sparc";
  } elsif ($arch eq "mips") {
-    # To enable module support, we need to enable the -mlong-calls option
-    # of gcc for module, after using this option, we can not get the real
-    # offset of the calling to _mcount, but the offset of the lui
-    # instruction or the addiu one. herein, we record the address of the
-    # first one, and then we can replace this instruction by a branch
-    # instruction to jump over the profiling function to filter the
-    # indicated functions, or swith back to the lui instruction to trace
-    # them, which means dynamic tracing.
+    #<For kernel>
+    # To disable tracing, just replace "jal _mcount" with nop;
+    # to enable tracing, replace back. so, the offset 14 is
+    # needed to be recorded.
+    #
+    #     10:   03e0082d        move    at,ra
+    #     14:   0c000000        jal     0
+    #                    14: R_MIPS_26   _mcount
+    #                    14: R_MIPS_NONE *ABS*
+    #                    14: R_MIPS_NONE *ABS*
+    #     18:   00020021        nop
+    #
+    #<For module>
+    #
+    # If no long call(-mlong-calls), the same to kernel.
+    #
+    # If the module space differs from the kernel space, long
+    # call is needed, as a result, the address of _mcount is
+    # needed to be recorded in a register and then jump from
+    # module space to kernel space via "jalr<register>". To
+    # disable tracing, "jalr<register>" can be replaced by
+    # nop; to enable tracing, replace it back. Since the
+    # offset of "jalr<register>" is not easy to be matched,
+    # the offset of the 1st _mcount below is recorded and to
+    # disable tracing, "lui v1, 0x0" is substituted with "b
+    # label", which jumps over "jalr<register>"; to enable
+    # tracing, replace it back.
      #
      #       c:	3c030000 	lui	v1,0x0
      #			c: R_MIPS_HI16	_mcount

@@ -318,19 +337,12 @@ if ($arch eq "x86_64") {
      #			10: R_MIPS_NONE	*ABS*
      #      14:	03e0082d 	move	at,ra
      #      18:	0060f809 	jalr	v1
+    #   label:
      #
-    # for the kernel:
-    #
-    #     10:   03e0082d        move    at,ra
-    #	  14:   0c000000        jal     0<loongson_halt>
-    #                    14: R_MIPS_26   _mcount
-    #                    14: R_MIPS_NONE *ABS*
-    #                    14: R_MIPS_NONE *ABS*
-    #	 18:   00020021        nop
      if ($is_module eq "0") {
  	    $mcount_regex = "^\\s*([0-9a-fA-F]+): R_MIPS_26\\s+_mcount\$";
      } else {
-	    $mcount_regex = "^\\s*([0-9a-fA-F]+): R_MIPS_HI16\\s+_mcount\$";
+	    $mcount_regex = "^\\s*([0-9a-fA-F]+): R_MIPS_(HI16|26)\\s+_mcount\$";
      }
      $objdump .= " -Melf-trad".$endian."mips ";
 

Quoted 6 lines · Wu Zhangjin

From: Wu Zhangjin<[email protected]>

In some situations(with related kernel config and gcc options), the modules may have the same address space as the core kernel space, so mcount_regex for modules should also match R_MIPS_26.

I think Steve is rewriting this bit to be a pure C program. Is this file even used anymore? If so for how long?

Quoted 9 lines · David Daney

Quoted 6 lines · Wu Zhangjin

From: Wu Zhangjin<[email protected]>

In some situations(with related kernel config and gcc options), the modules may have the same address space as the core kernel space, so mcount_regex for modules should also match R_MIPS_26.

I think Steve is rewriting this bit to be a pure C program.

Quoted 2 lines

 Is this
file even used anymore?  If so for how long?

It's already in mainline, but is only supported for x86 for now. Until we verify that it works for other archs (which is up to you guys to verify) the script will still be used.

Also, I did not write it, John Reiser did. I just cleaned it up a bit and got it working with the build system.

-- Steve