* [PATCH] ARM: ftrace: clear zero bit in reported IPs for Thumb-2
@ 2011-12-01 18:06 Rabin Vincent
2011-12-02 12:44 ` Dave Martin
0 siblings, 1 reply; 4+ messages in thread
From: Rabin Vincent @ 2011-12-01 18:06 UTC (permalink / raw)
To: linux-arm-kernel
The dynamic ftrace ops startup test currently fails on Thumb-2 kernels:
Testing tracer function: PASSED
Testing dynamic ftrace: PASSED
Testing dynamic ftrace ops #1: (0 0 0 0 0) FAILED!
This is because while the addresses in the mcount records do not have
the zero bit set, the IP reported by the mcount call does have it set
(because it is copied from the LR). This mismatch causes the ops
filtering in ftrace_ops_list_func() to not call the relevant tracers.
Fix this by clearing the zero bit while adjusting the LR for the mcount
instruction size. Also, combine the mov+sub into a single sub
instruction.
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
arch/arm/kernel/entry-common.S | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index b2a27b6..9d7ce81 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -149,6 +149,12 @@ ENDPROC(ret_from_fork)
#endif
#endif
+.macro mcount_size_adjust rd, rn
+ ARM( sub \rd, \rn, #MCOUNT_INSN_SIZE )
+ /* The zero bit is set on Thumb LRs; clear it. */
+ THUMB( sub \rd, \rn, #MCOUNT_INSN_SIZE + 1 )
+.endm
+
.macro __mcount suffix
mcount_enter
ldr r0, =ftrace_trace_function
@@ -173,8 +179,7 @@ ENDPROC(ret_from_fork)
mcount_exit
1: mcount_get_lr r1 @ lr of instrumented func
- mov r0, lr @ instrumented function
- sub r0, r0, #MCOUNT_INSN_SIZE
+ mcount_size_adjust r0, lr @ instrumented function
adr lr, BSYM(2f)
mov pc, r2
2: mcount_exit
@@ -184,8 +189,7 @@ ENDPROC(ret_from_fork)
mcount_enter
mcount_get_lr r1 @ lr of instrumented func
- mov r0, lr @ instrumented function
- sub r0, r0, #MCOUNT_INSN_SIZE
+ mcount_size_adjust r0, lr @ instrumented function
.globl ftrace_call\suffix
ftrace_call\suffix:
@@ -205,11 +209,11 @@ ftrace_graph_call\suffix:
#ifdef CONFIG_DYNAMIC_FTRACE
@ called from __ftrace_caller, saved in mcount_enter
ldr r1, [sp, #16] @ instrumented routine (func)
+ mcount_size_adjust r1, r1
#else
@ called from __mcount, untouched in lr
- mov r1, lr @ instrumented routine (func)
+ mcount_size_adjust r1, lr @ instrumented routine (func)
#endif
- sub r1, r1, #MCOUNT_INSN_SIZE
mov r2, fp @ frame pointer
bl prepare_ftrace_return
mcount_exit
--
1.7.7.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] ARM: ftrace: clear zero bit in reported IPs for Thumb-2
2011-12-01 18:06 [PATCH] ARM: ftrace: clear zero bit in reported IPs for Thumb-2 Rabin Vincent
@ 2011-12-02 12:44 ` Dave Martin
2012-01-20 5:58 ` [PATCHv2] " Rabin Vincent
0 siblings, 1 reply; 4+ messages in thread
From: Dave Martin @ 2011-12-02 12:44 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Dec 01, 2011 at 11:36:05PM +0530, Rabin Vincent wrote:
> The dynamic ftrace ops startup test currently fails on Thumb-2 kernels:
>
> Testing tracer function: PASSED
> Testing dynamic ftrace: PASSED
> Testing dynamic ftrace ops #1: (0 0 0 0 0) FAILED!
>
> This is because while the addresses in the mcount records do not have
> the zero bit set, the IP reported by the mcount call does have it set
> (because it is copied from the LR). This mismatch causes the ops
> filtering in ftrace_ops_list_func() to not call the relevant tracers.
>
> Fix this by clearing the zero bit while adjusting the LR for the mcount
> instruction size. Also, combine the mov+sub into a single sub
> instruction.
>
> Signed-off-by: Rabin Vincent <rabin@rab.in>
> ---
> arch/arm/kernel/entry-common.S | 16 ++++++++++------
> 1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index b2a27b6..9d7ce81 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -149,6 +149,12 @@ ENDPROC(ret_from_fork)
> #endif
> #endif
>
> +.macro mcount_size_adjust rd, rn
> + ARM( sub \rd, \rn, #MCOUNT_INSN_SIZE )
> + /* The zero bit is set on Thumb LRs; clear it. */
> + THUMB( sub \rd, \rn, #MCOUNT_INSN_SIZE + 1 )
> +.endm
> +
You're conflating two different things here:
1) convert the branch pointer in lr into the corresponding instruction
address in memory
2) subtract MCOUNT_INSN_SIZE
(1) Is an ARM architectural thing and not secific to ftrace in any way,
so I suggest we make that generic and factor it out.
For example, kprobes.c has:
121:void __kprobes arch_arm_kprobe(struct kprobe *p)
122:{
123: uintptr_t addr = (uintptr_t)p->addr & ~1; /* Remove any Thumb flag */
166:int __kprobes __arch_disarm_kprobe(void *p)
167:{
[...]
169-#ifdef CONFIG_THUMB2_KERNEL
170: u16 *addr = (u16 *)((uintptr_t)kp->addr & ~1);
Whether is makes sense to make that all generic for assembler or not,
you can avoid the conditionality by writing:
.macro mcount_size_adjust rd, rn
bic \rd , \rn , #1 @ clear the Thumb bit if present
sub \rd , \rd , #MCOUNT_INSN_SIZE
.endm
This adjustment will be correct regardless of the instruction set.
Otherwise, this looks sensible.
Cheers
---Dave
> .macro __mcount suffix
> mcount_enter
> ldr r0, =ftrace_trace_function
> @@ -173,8 +179,7 @@ ENDPROC(ret_from_fork)
> mcount_exit
>
> 1: mcount_get_lr r1 @ lr of instrumented func
> - mov r0, lr @ instrumented function
> - sub r0, r0, #MCOUNT_INSN_SIZE
> + mcount_size_adjust r0, lr @ instrumented function
> adr lr, BSYM(2f)
> mov pc, r2
> 2: mcount_exit
> @@ -184,8 +189,7 @@ ENDPROC(ret_from_fork)
> mcount_enter
>
> mcount_get_lr r1 @ lr of instrumented func
> - mov r0, lr @ instrumented function
> - sub r0, r0, #MCOUNT_INSN_SIZE
> + mcount_size_adjust r0, lr @ instrumented function
>
> .globl ftrace_call\suffix
> ftrace_call\suffix:
> @@ -205,11 +209,11 @@ ftrace_graph_call\suffix:
> #ifdef CONFIG_DYNAMIC_FTRACE
> @ called from __ftrace_caller, saved in mcount_enter
> ldr r1, [sp, #16] @ instrumented routine (func)
> + mcount_size_adjust r1, r1
> #else
> @ called from __mcount, untouched in lr
> - mov r1, lr @ instrumented routine (func)
> + mcount_size_adjust r1, lr @ instrumented routine (func)
> #endif
> - sub r1, r1, #MCOUNT_INSN_SIZE
> mov r2, fp @ frame pointer
> bl prepare_ftrace_return
> mcount_exit
> --
> 1.7.7.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCHv2] ARM: ftrace: clear zero bit in reported IPs for Thumb-2
2011-12-02 12:44 ` Dave Martin
@ 2012-01-20 5:58 ` Rabin Vincent
2012-01-24 12:05 ` Dave Martin
0 siblings, 1 reply; 4+ messages in thread
From: Rabin Vincent @ 2012-01-20 5:58 UTC (permalink / raw)
To: linux-arm-kernel
The dynamic ftrace ops startup test currently fails on Thumb-2 kernels:
Testing tracer function: PASSED
Testing dynamic ftrace: PASSED
Testing dynamic ftrace ops #1: (0 0 0 0 0) FAILED!
This is because while the addresses in the mcount records do not have
the zero bit set, the IP reported by the mcount call does have it set
(because it is copied from the LR). This mismatch causes the ops
filtering in ftrace_ops_list_func() to not call the relevant tracers.
Fix this by clearing the zero bit before adjusting the LR for the mcount
instruction size. Also, combine the mov+sub into a single sub
instruction.
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
v2: use bic
arch/arm/kernel/entry-common.S | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index b2a27b6..683195c 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -149,6 +149,11 @@ ENDPROC(ret_from_fork)
#endif
#endif
+.macro mcount_adjust_addr rd, rn
+ bic \rd, \rn, #1 @ clear the Thumb bit if present
+ sub \rd, \rd, #MCOUNT_INSN_SIZE
+.endm
+
.macro __mcount suffix
mcount_enter
ldr r0, =ftrace_trace_function
@@ -173,8 +178,7 @@ ENDPROC(ret_from_fork)
mcount_exit
1: mcount_get_lr r1 @ lr of instrumented func
- mov r0, lr @ instrumented function
- sub r0, r0, #MCOUNT_INSN_SIZE
+ mcount_adjust_addr r0, lr @ instrumented function
adr lr, BSYM(2f)
mov pc, r2
2: mcount_exit
@@ -184,8 +188,7 @@ ENDPROC(ret_from_fork)
mcount_enter
mcount_get_lr r1 @ lr of instrumented func
- mov r0, lr @ instrumented function
- sub r0, r0, #MCOUNT_INSN_SIZE
+ mcount_adjust_addr r0, lr @ instrumented function
.globl ftrace_call\suffix
ftrace_call\suffix:
@@ -205,11 +208,11 @@ ftrace_graph_call\suffix:
#ifdef CONFIG_DYNAMIC_FTRACE
@ called from __ftrace_caller, saved in mcount_enter
ldr r1, [sp, #16] @ instrumented routine (func)
+ mcount_adjust_addr r1, r1
#else
@ called from __mcount, untouched in lr
- mov r1, lr @ instrumented routine (func)
+ mcount_adjust_addr r1, lr @ instrumented routine (func)
#endif
- sub r1, r1, #MCOUNT_INSN_SIZE
mov r2, fp @ frame pointer
bl prepare_ftrace_return
mcount_exit
--
1.7.7.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCHv2] ARM: ftrace: clear zero bit in reported IPs for Thumb-2
2012-01-20 5:58 ` [PATCHv2] " Rabin Vincent
@ 2012-01-24 12:05 ` Dave Martin
0 siblings, 0 replies; 4+ messages in thread
From: Dave Martin @ 2012-01-24 12:05 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jan 20, 2012 at 11:28:22AM +0530, Rabin Vincent wrote:
> The dynamic ftrace ops startup test currently fails on Thumb-2 kernels:
>
> Testing tracer function: PASSED
> Testing dynamic ftrace: PASSED
> Testing dynamic ftrace ops #1: (0 0 0 0 0) FAILED!
>
> This is because while the addresses in the mcount records do not have
> the zero bit set, the IP reported by the mcount call does have it set
> (because it is copied from the LR). This mismatch causes the ops
> filtering in ftrace_ops_list_func() to not call the relevant tracers.
>
> Fix this by clearing the zero bit before adjusting the LR for the mcount
> instruction size. Also, combine the mov+sub into a single sub
> instruction.
>
> Signed-off-by: Rabin Vincent <rabin@rab.in>
I'm probably not going to have a lot of time to test this; in the
meantime, I'm satisfied that you addressed my concerns, so
Acked-By: Dave Martin <dave.martin@linaro.org>
Cheers
---Dave
> ---
> v2: use bic
>
> arch/arm/kernel/entry-common.S | 15 +++++++++------
> 1 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
> index b2a27b6..683195c 100644
> --- a/arch/arm/kernel/entry-common.S
> +++ b/arch/arm/kernel/entry-common.S
> @@ -149,6 +149,11 @@ ENDPROC(ret_from_fork)
> #endif
> #endif
>
> +.macro mcount_adjust_addr rd, rn
> + bic \rd, \rn, #1 @ clear the Thumb bit if present
> + sub \rd, \rd, #MCOUNT_INSN_SIZE
> +.endm
> +
> .macro __mcount suffix
> mcount_enter
> ldr r0, =ftrace_trace_function
> @@ -173,8 +178,7 @@ ENDPROC(ret_from_fork)
> mcount_exit
>
> 1: mcount_get_lr r1 @ lr of instrumented func
> - mov r0, lr @ instrumented function
> - sub r0, r0, #MCOUNT_INSN_SIZE
> + mcount_adjust_addr r0, lr @ instrumented function
> adr lr, BSYM(2f)
> mov pc, r2
> 2: mcount_exit
> @@ -184,8 +188,7 @@ ENDPROC(ret_from_fork)
> mcount_enter
>
> mcount_get_lr r1 @ lr of instrumented func
> - mov r0, lr @ instrumented function
> - sub r0, r0, #MCOUNT_INSN_SIZE
> + mcount_adjust_addr r0, lr @ instrumented function
>
> .globl ftrace_call\suffix
> ftrace_call\suffix:
> @@ -205,11 +208,11 @@ ftrace_graph_call\suffix:
> #ifdef CONFIG_DYNAMIC_FTRACE
> @ called from __ftrace_caller, saved in mcount_enter
> ldr r1, [sp, #16] @ instrumented routine (func)
> + mcount_adjust_addr r1, r1
> #else
> @ called from __mcount, untouched in lr
> - mov r1, lr @ instrumented routine (func)
> + mcount_adjust_addr r1, lr @ instrumented routine (func)
> #endif
> - sub r1, r1, #MCOUNT_INSN_SIZE
> mov r2, fp @ frame pointer
> bl prepare_ftrace_return
> mcount_exit
> --
> 1.7.7.3
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-01-24 12:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-01 18:06 [PATCH] ARM: ftrace: clear zero bit in reported IPs for Thumb-2 Rabin Vincent
2011-12-02 12:44 ` Dave Martin
2012-01-20 5:58 ` [PATCHv2] " Rabin Vincent
2012-01-24 12:05 ` Dave Martin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).