ARM: ftrace fixes and cleanups
Subsystems (5)
v2 cover ARM: ftrace fixes and cleanups
This series addresses a number of issues in the ARM support code for ftrace, mostly related to Thumb2 but affecting other configurations as well.
Changes since v1:
- add a couple of patches to enable ftrace in Thumb2 mode when building with Clang, which was the one remaining unsupported configuration
- fix up some minor code issues caught by the bots
- add some acks
Ard Biesheuvel (12):
ARM: ftrace: ensure that ADR take Thumb bit into account
ARM: ftrace: use ADD not POP to counter PUSH at entry
ARM: ftrace: use trampolines to keep .init.text in branching range
ARM: ftrace: avoid redundant loads or clobbering IP
ARM: ftrace: avoid unnecessary literal loads
ARM: ftrace: enable HAVE_FUNCTION_GRAPH_FP_TEST
ARM: unwind: track location of LR value in stack frame
ARM: ftrace: enable the graph tracer with the EABI unwinder
ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds drivers/firmware/scmi: disable ftrace for Clang Thumb2 builds
ARM: cacheflush: avoid clobbering the frame pointer in Thumb2 mode Revert "ARM: 9144/1: forbid ftrace with clang and thumb2_kernel"
arch/arm/Kconfig | 4 +- arch/arm/Kconfig.debug | 2 +- arch/arm/include/asm/cacheflush.h | 7 +- arch/arm/include/asm/ftrace.h | 20 +-- arch/arm/include/asm/opcodes.h | 14 +++ arch/arm/include/asm/stacktrace.h | 3 + arch/arm/kernel/Makefile | 6 +- arch/arm/kernel/entry-ftrace.S | 128 +++++++++++--------- arch/arm/kernel/ftrace.c | 51 ++++++-- arch/arm/kernel/unwind.c | 7 +- arch/arm/mach-exynos/mcpm-exynos.c | 7 +- arch/arm/probes/kprobes/actions-common.c | 6 +- arch/arm/probes/kprobes/actions-thumb.c | 38 ++++-- drivers/firmware/arm_scmi/Makefile | 7 ++ 14 files changed, 193 insertions(+), 107 deletions(-)
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 1/12 ARM: ftrace: ensure that ADR take Thumb bit into account
Using ADR to take the address of 'ftrace_stub' via a local label produces an address that has the Thumb bit cleared, which means the subsequent comparison is guaranteed to fail. Instead, use the badr macro, which forces the Thumb bit to be set.
--- arch/arm/kernel/entry-ftrace.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/kernel/entry-ftrace.S +1 -1
@@ -40,7 +40,7 @@ mcount_enter ldr r0, =ftrace_trace_function ldr r2, [r0] - adr r0, .Lftrace_stub + badr r0, .Lftrace_stub cmp r0, r2 bne 1f
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 2/12 ARM: ftrace: use ADD not POP to counter PUSH at entry
In reply to Ard BiesheuvelThe compiler emitted hook used for ftrace consists of a PUSH {LR} to preserve the link register, followed by a branch-and-link (BL) to __gnu_mount_nc. Dynamic ftrace patches away the latter to turn the combined sequence into a NOP, using a POP {LR} instruction.
This is not necessary, since the link register does not get clobbered in this case, and simply adding #4 to the stack pointer is sufficient, and avoids a memory access that may take a few cycles to resolve depending on the micro-architecture.
--- arch/arm/kernel/entry-ftrace.S | 2 +- arch/arm/kernel/ftrace.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/kernel/entry-ftrace.S +1 -1
@@ -27,7 +27,7 @@ * allows it to be clobbered in subroutines and doesn't use it to hold * parameters.) * - * When using dynamic ftrace, we patch out the mcount call by a "pop {lr}" + * When using dynamic ftrace, we patch out the mcount call by a "add sp, #4" * instead of the __gnu_mcount_nc call (see arch/arm/kernel/ftrace.c). */
arch/arm/kernel/ftrace.c +2 -2
@@ -25,9 +25,9 @@#include <asm/patch.h> #ifdef CONFIG_THUMB2_KERNEL -#define NOP 0xf85deb04/* pop.w {lr} */ +#define NOP 0xf10d0d04/* add.w sp, sp, #4 */ #else -#define NOP 0xe8bd4000/* pop {lr} */ +#define NOP 0xe28dd004/* add sp, sp, #4 */ #endif #ifdef CONFIG_DYNAMIC_FTRACE
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 3/12 ARM: ftrace: use trampolines to keep .init.text in branching range
In reply to Ard BiesheuvelKernel images that are large in comparison to the range of a direct branch may fail to work as expected with ftrace, as patching a direct branch to one of the core ftrace routines may not be possible from the .init.text section, if it is emitted too far away from the normal .text section.
This is more likely to affect Thumb2 builds, given that its range is only -/+ 16 MiB (as opposed to ARM which has -/+ 32 MiB), but may occur in either ISA.
To work around this, add a couple of trampolines to .init.text and swap these in when the ftrace patching code is operating on callers in .init.text.
--- arch/arm/kernel/entry-ftrace.S | 16 ++++++++++++++ arch/arm/kernel/ftrace.c | 23 +++++++++++++++++--- 2 files changed, 36 insertions(+), 3 deletions(-)
arch/arm/kernel/entry-ftrace.S +16 -0
@@ -270,3 +270,19 @@ ENTRY(ftrace_stub) .Lftrace_stub: ret lr ENDPROC(ftrace_stub) + +#ifdef CONFIG_DYNAMIC_FTRACE + + __INIT + + .macro init_tramp, dst:req +ENTRY(\dst\()_from_init) + ldr pc, =\dst +ENDPROC(\dst\()_from_init) + .endm + + init_tramp ftrace_caller +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS + init_tramp ftrace_regs_caller +#endif +#endif
arch/arm/kernel/ftrace.c +20 -3
@@ -51,9 +51,20 @@ static unsigned long ftrace_nop_replace(struct dyn_ftrace *rec)return NOP ; } -static unsigned long adjust_address (struct dyn_ftrace *rec,unsigned long addr) +void ftrace_caller_from_init (void ); +void ftrace_regs_caller_from_init (void ); + +static unsigned long __ref adjust_address (struct dyn_ftrace *rec, +unsigned long addr) { -return addr; +if (!IS_ENABLED (CONFIG_DYNAMIC_FTRACE ) || + system_state >=SYSTEM_FREEING_INITMEM || +likely (!is_kernel_inittext (rec->ip))) +return addr; +if (!IS_ENABLED (CONFIG_DYNAMIC_FTRACE_WITH_REGS ) || + addr == (unsigned long )&ftrace_caller) +return (unsigned long )&ftrace_caller_from_init; +return (unsigned long )&ftrace_regs_caller_from_init; }int ftrace_arch_code_modify_prepare (void )
@@ -189,7 +200,13 @@ int ftrace_make_nop(struct module *mod,#endif new =ftrace_nop_replace (rec); - ret =ftrace_modify_code (ip, old, new, true); +/* +* Locations in .init.text may call __gnu_mcount_mc via a linker +* emitted veneer if they are too far away from its implementation, and +* so validation may fail spuriously in such cases. Let's work around +* this by omitting those from validation. +*/ + ret =ftrace_modify_code (ip, old, new, !is_kernel_inittext (ip));return ret; }
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 4/12 ARM: ftrace: avoid redundant loads or clobbering IP
In reply to Ard BiesheuvelTweak the ftrace return paths to avoid redundant loads of SP, as well as unnecessary clobbering of IP.
This also fixes the inconsistency of using MOV to perform a function return, which is sub-optimal on recent micro-architectures but more importantly, does not perform an interworking return, unlike compiler generated function returns in Thumb2 builds.
Let's fix this by popping PC from the stack like most ordinary code does.
--- arch/arm/kernel/entry-ftrace.S | 51 +++++++++----------- 1 file changed, 22 insertions(+), 29 deletions(-)
arch/arm/kernel/entry-ftrace.S +22 -29
@@ -22,10 +22,7 @@ * mcount can be thought of as a function called in the middle of a subroutine * call. As such, it needs to be transparent for both the caller and the * callee: the original lr needs to be restored when leaving mcount, and no - * registers should be clobbered. (In the __gnu_mcount_nc implementation, we - * clobber the ip register. This is OK because the ARM calling convention - * allows it to be clobbered in subroutines and doesn't use it to hold - * parameters.) + * registers should be clobbered. * * When using dynamic ftrace, we patch out the mcount call by a "add sp, #4" * instead of the __gnu_mcount_nc call (see arch/arm/kernel/ftrace.c).
@@ -70,26 +67,25 @@ .macro __ftrace_regs_caller - sub sp, sp, #8 @ space for PC and CPSR OLD_R0, + str lr, [sp, #-8]! @ store LR as PC and make space for CPSR/OLD_R0, @ OLD_R0 will overwrite previous LR - add ip, sp, #12 @ move in IP the value of SP as it was - @ before the push {lr} of the mcount mechanism + ldr lr, [sp, #8] @ get previous LR - str lr, [sp, #0] @ store LR instead of PC + str r0, [sp, #8] @ write r0 as OLD_R0 over previous LR - ldr lr, [sp, #8] @ get previous LR + str lr, [sp, #-4]! @ store previous LR as LR - str r0, [sp, #8] @ write r0 as OLD_R0 over previous LR + add lr, sp, #16 @ move in LR the value of SP as it was + @ before the push {lr} of the mcount mechanism - stmdb sp!, {ip, lr} - stmdb sp!, {r0-r11, lr} + push {r0-r11, ip, lr} @ stack content at this point: @ 0 4 48 52 56 60 64 68 72 - @ R0 | R1 | ... | LR | SP + 4 | previous LR | LR | PSR | OLD_R0 | + @ R0 | R1 | ... | IP | SP + 4 | previous LR | LR | PSR | OLD_R0 | - mov r3, sp @ struct pt_regs* + mov r3, sp @ struct pt_regs* ldr r2, =function_trace_op ldr r2, [r2] @ pointer to the current
@@ -112,11 +108,9 @@ ftrace_graph_regs_call: #endif @ pop saved regs - ldmia sp!, {r0-r12} @ restore r0 through r12 - ldr ip, [sp, #8] @ restore PC - ldr lr, [sp, #4] @ restore LR - ldr sp, [sp, #0] @ restore SP - mov pc, ip @ return + pop {r0-r11, ip, lr} @ restore r0 through r12 + ldr lr, [sp], #4 @ restore LR + ldr pc, [sp], #12 .endm #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -132,11 +126,9 @@ ftrace_graph_regs_call: bl prepare_ftrace_return @ pop registers saved in ftrace_regs_caller - ldmia sp!, {r0-r12} @ restore r0 through r12 - ldr ip, [sp, #8] @ restore PC - ldr lr, [sp, #4] @ restore LR - ldr sp, [sp, #0] @ restore SP - mov pc, ip @ return + pop {r0-r11, ip, lr} @ restore r0 through r12 + ldr lr, [sp], #4 @ restore LR + ldr pc, [sp], #12 .endm #endif
@@ -202,16 +194,17 @@ ftrace_graph_call\suffix: .endm .macro mcount_exit - ldmia sp!, {r0-r3, ip, lr} - ret ip + ldmia sp!, {r0-r3} + ldr lr, [sp, #4] + ldr pc, [sp], #8 .endm ENTRY(__gnu_mcount_nc) UNWIND(.fnstart) #ifdef CONFIG_DYNAMIC_FTRACE - mov ip, lr - ldmia sp!, {lr} - ret ip + push {lr} + ldr lr, [sp, #4] + ldr pc, [sp], #8 #else __mcount #endif
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 5/12 ARM: ftrace: avoid unnecessary literal loads
In reply to Ard BiesheuvelAvoid explicit literal loads and instead, use accessor macros that generate the optimal sequence depending on the architecture revision being targeted.
--- arch/arm/kernel/entry-ftrace.S | 27 ++++++++------------ 1 file changed, 11 insertions(+), 16 deletions(-)
arch/arm/kernel/entry-ftrace.S +11 -16
@@ -35,23 +35,20 @@ .macro __mcount suffix mcount_enter - ldr r0, =ftrace_trace_function - ldr r2, [r0] + ldr_va r2, ftrace_trace_function badr r0, .Lftrace_stub cmp r0, r2 bne 1f #ifdef CONFIG_FUNCTION_GRAPH_TRACER - ldr r1, =ftrace_graph_return - ldr r2, [r1] - cmp r0, r2 - bne ftrace_graph_caller\suffix - - ldr r1, =ftrace_graph_entry - ldr r2, [r1] - ldr r0, =ftrace_graph_entry_stub - cmp r0, r2 - bne ftrace_graph_caller\suffix + ldr_va r2, ftrace_graph_return + cmp r0, r2 + bne ftrace_graph_caller\suffix + + ldr_va r2, ftrace_graph_entry + mov_l r0, ftrace_graph_entry_stub + cmp r0, r2 + bne ftrace_graph_caller\suffix #endif mcount_exit
@@ -87,8 +84,7 @@ mov r3, sp @ struct pt_regs* - ldr r2, =function_trace_op - ldr r2, [r2] @ pointer to the current + ldr_va r2, function_trace_op @ pointer to the current @ function tracing op ldr r1, [sp, #S_LR] @ lr of instrumented func
@@ -141,8 +137,7 @@ ftrace_graph_regs_call: mcount_adjust_addr r0, lr @ instrumented function #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS - ldr r2, =function_trace_op - ldr r2, [r2] @ pointer to the current + ldr_va r2, function_trace_op @ pointer to the current @ function tracing op mov r3, #0 @ regs is NULL #endif
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 6/12 ARM: ftrace: enable HAVE_FUNCTION_GRAPH_FP_TEST
In reply to Ard BiesheuvelFix the frame pointer handling in the function graph tracer entry and exit code so we can enable HAVE_FUNCTION_GRAPH_FP_TEST. Instead of using FP directly (which will have different values between the entry and exit pieces of the function graph tracer), use the value of SP at entry and exit, as we can derive the former value from the frame pointer.
--- arch/arm/include/asm/ftrace.h | 2 ++ arch/arm/kernel/entry-ftrace.S | 2 +- arch/arm/kernel/ftrace.c | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-)
arch/arm/include/asm/ftrace.h +2 -0
@@ -2,6 +2,8 @@#ifndef _ASM_ARM_FTRACE#define _ASM_ARM_FTRACE +#define HAVE_FUNCTION_GRAPH_FP_TEST +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS #define ARCH_SUPPORTS_FTRACE_OPS 1#endif
arch/arm/kernel/entry-ftrace.S +1 -1
@@ -247,7 +247,7 @@ ENDPROC(ftrace_graph_regs_caller) .globl return_to_handler return_to_handler: stmdb sp!, {r0-r3} - mov r0, fp @ frame pointer + add r0, sp, #16 @ sp at exit of instrumented routine bl ftrace_return_to_handler mov lr, r0 @ r0 has real ret addr ldmia sp!, {r0-r3}
arch/arm/kernel/ftrace.c +5 -0
@@ -222,6 +222,11 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,if (unlikely (atomic_read (¤t->tracing_graph_pause)))return ; +if (IS_ENABLED (CONFIG_UNWINDER_FRAME_POINTER )) { +/* FP points one word below parent's top of stack */ + frame_pointer +=4 ; + } + old = *parent; *parent = return_hooker;
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 7/12 ARM: unwind: track location of LR value in stack frame
In reply to Ard BiesheuvelThe ftrace graph tracer needs to override the return address of an instrumented function, in order to install a hook that gets invoked when the function returns again.
Currently, we only support this when building for ARM using GCC with frame pointers, as in this case, it is guaranteed that the function will reload LR from [FP, #-4] in all cases, and we can simply pass that address to the ftrace code.
In order to support this for configurations that rely on the EABI unwinder, such as Thumb2 builds, make the unwinder keep track of the address from which LR was unwound, permitting ftrace to make use of this in a subsequent patch.
Drop the call to is_kernel_text_address(), which is problematic in terms of ftrace recursion, given that it may be instrumented itself. The call is redundant anyway, as no unwind directives will be found unless the PC points to memory that is known to contain executable code.
--- arch/arm/include/asm/stacktrace.h | 3 +++ arch/arm/kernel/Makefile | 1 + arch/arm/kernel/unwind.c | 7 ++++--- 3 files changed, 8 insertions(+), 3 deletions(-)
arch/arm/include/asm/stacktrace.h +3 -0
@@ -14,6 +14,9 @@ struct stackframe {unsigned long sp;unsigned long lr;unsigned long pc; + +/* address of the LR value on the stack */ +unsigned long *lr_addr;#ifdef CONFIG_KRETPROBES struct llist_node *kr_cur;struct task_struct *tsk;
arch/arm/kernel/Makefile +1 -0
@@ -10,6 +10,7 @@ ifdef CONFIG_FUNCTION_TRACER CFLAGS_REMOVE_ftrace.o = -pg CFLAGS_REMOVE_insn.o = -pg CFLAGS_REMOVE_patch.o = -pg +CFLAGS_REMOVE_unwind.o = -pg endif CFLAGS_REMOVE_return_address.o = -pg
arch/arm/kernel/unwind.c +4 -3
@@ -55,6 +55,7 @@ struct unwind_ctrl_block {const unsigned long *insn;/* pointer to the current instructions word */ unsigned long sp_low;/* lowest value of sp allowed */ unsigned long sp_high;/* highest value of sp allowed */ +unsigned long *lr_addr;/* address of LR value on the stack */ /* * 1 : check for stack overflow for each register pop. * 0 : save overhead if there is plenty of stack remaining.
@@ -239,6 +240,8 @@ static int unwind_pop_register(struct unwind_ctrl_block *ctrl,* from being tracked by KASAN. */ ctrl->vrs[reg] =READ_ONCE_NOCHECK (*(*vsp)); +if (reg ==14 ) + ctrl->lr_addr = *vsp; (*vsp)++;return URC_OK ; }
@@ -395,9 +398,6 @@ int unwind_frame(struct stackframe *frame)pr_debug ("%s(pc = %08lx lr = %08lx sp = %08lx)\n" , __func__, frame->pc, frame->lr, frame->sp); -if (!kernel_text_address (frame->pc)) -return -URC_FAILURE ; - idx =unwind_find_idx (frame->pc);if (!idx) {pr_warn ("unwind: Index not found %08lx\n" , frame->pc);
@@ -476,6 +476,7 @@ int unwind_frame(struct stackframe *frame) frame->lr = ctrl.vrs[LR ]; frame->pc = ctrl.vrs[PC ]; frame->sp_low = ctrl.sp_low; + frame->lr_addr = ctrl.lr_addr;return URC_OK ; }
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 8/12 ARM: ftrace: enable the graph tracer with the EABI unwinder
In reply to Ard BiesheuvelEnable the function graph tracer in combination with the EABI unwinder, so that Thumb2 builds or Clang ARM builds can make use of it.
This involves using the unwinder to locate the return address of an instrumented function on the stack, so that it can be overridden and made to refer to the ftrace handling routines that need to be called at function return.
Given that for these builds, it is not guaranteed that the value of the link register is stored on the stack, fall back to the stack slot that will be used by the ftrace exit code to restore LR in the instrumented function's execution context.
--- arch/arm/Kconfig | 2 +- arch/arm/Kconfig.debug | 2 +- arch/arm/include/asm/ftrace.h | 18 ------------- arch/arm/kernel/Makefile | 5 +--- arch/arm/kernel/entry-ftrace.S | 28 ++++++++++++++------ arch/arm/kernel/ftrace.c | 19 +++++++++++-- 6 files changed, 40 insertions(+), 34 deletions(-)
arch/arm/Kconfig +1 -1
@@ -92,7 +92,7 @@ config ARM select HAVE_EXIT_THREAD select HAVE_FAST_GUP if ARM_LPAE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL - select HAVE_FUNCTION_GRAPH_TRACER if !THUMB2_KERNEL && !CC_IS_CLANG + select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !(THUMB2_KERNEL && CC_IS_CLANG) select HAVE_FUTEX_CMPXCHG if FUTEX select HAVE_GCC_PLUGINS
arch/arm/Kconfig.debug +1 -1
@@ -65,7 +65,7 @@ config UNWINDER_FRAME_POINTER config UNWINDER_ARM bool "ARM EABI stack unwinder" - depends on AEABI && !FUNCTION_GRAPH_TRACER + depends on AEABI # https://github.com/ClangBuiltLinux/linux/issues/732 depends on !LD_IS_LLD || LLD_VERSION >= 110000 select ARM_UNWIND
arch/arm/include/asm/ftrace.h +0 -18
@@ -35,26 +35,8 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)#ifndef __ASSEMBLY__ -#if defined(CONFIG_FRAME_POINTER ) && !defined(CONFIG_ARM_UNWIND ) -/* -* return_address uses walk_stackframe to do it's work. If both -* CONFIG_FRAME_POINTER=y and CONFIG_ARM_UNWIND=y walk_stackframe uses unwind -* information. For this to work in the function tracer many functions would -* have to be marked with __notrace. So for now just depend on -* !CONFIG_ARM_UNWIND. -*/ -void *return_address (unsigned int ); -#else - -static inline void *return_address (unsigned int level) -{ -return NULL ; -} - -#endif -#define ftrace_return_address (n) return_address(n)#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
arch/arm/kernel/Makefile +1 -4
@@ -25,10 +25,7 @@ obj-y := elf.o entry-common.o irq.o opcodes.o \ KASAN_SANITIZE_stacktrace.o := n KASAN_SANITIZE_traps.o := n -ifneq ($(CONFIG_ARM_UNWIND),y) -obj-$(CONFIG_FRAME_POINTER) += return_address.o -endif - +obj-y += return_address.o obj-$(CONFIG_ATAGS) += atags_parse.o obj-$(CONFIG_ATAGS_PROC) += atags_proc.o obj-$(CONFIG_DEPRECATED_PARAM_STRUCT) += atags_compat.o
arch/arm/kernel/entry-ftrace.S +20 -8
@@ -100,7 +100,8 @@ ftrace_regs_call: #ifdef CONFIG_FUNCTION_GRAPH_TRACER .globl ftrace_graph_regs_call ftrace_graph_regs_call: - mov r0, r0 +ARM( mov r0, r0 ) +THUMB( nop.w ) #endif @ pop saved regs
@@ -112,13 +113,18 @@ ftrace_graph_regs_call: #ifdef CONFIG_FUNCTION_GRAPH_TRACER .macro __ftrace_graph_regs_caller - sub r0, fp, #4 @ lr of instrumented routine (parent) +#ifdef CONFIG_UNWINDER_FRAME_POINTER + sub r0, fp, #4 @ lr of instrumented routine (parent) +#else + add r0, sp, #S_LR +#endif @ called from __ftrace_regs_caller - ldr r1, [sp, #S_PC] @ instrumented routine (func) + ldr r1, [sp, #S_PC] @ instrumented routine (func) mcount_adjust_addr r1, r1 - mov r2, fp @ frame pointer + mov r2, fpreg @ frame pointer + add r3, sp, #PT_REGS_SIZE bl prepare_ftrace_return @ pop registers saved in ftrace_regs_caller
@@ -149,14 +155,19 @@ ftrace_call\suffix: #ifdef CONFIG_FUNCTION_GRAPH_TRACER .globl ftrace_graph_call\suffix ftrace_graph_call\suffix: - mov r0, r0 +ARM( mov r0, r0 ) +THUMB( nop.w ) #endif mcount_exit .endm .macro __ftrace_graph_caller +#ifdef CONFIG_UNWINDER_FRAME_POINTER sub r0, fp, #4 @ &lr of instrumented routine (&parent) +#else + add r0, sp, #20 +#endif #ifdef CONFIG_DYNAMIC_FTRACE @ called from __ftrace_caller, saved in mcount_enter ldr r1, [sp, #16] @ instrumented routine (func)
@@ -165,7 +176,8 @@ ftrace_graph_call\suffix: @ called from __mcount, untouched in lr mcount_adjust_addr r1, lr @ instrumented routine (func) #endif - mov r2, fp @ frame pointer + mov r2, fpreg @ frame pointer + add r3, sp, #24 bl prepare_ftrace_return mcount_exit .endm
@@ -244,14 +256,14 @@ ENDPROC(ftrace_graph_regs_caller) .purgem mcount_exit #ifdef CONFIG_FUNCTION_GRAPH_TRACER - .globl return_to_handler -return_to_handler: +ENTRY(return_to_handler) stmdb sp!, {r0-r3} add r0, sp, #16 @ sp at exit of instrumented routine bl ftrace_return_to_handler mov lr, r0 @ r0 has real ret addr ldmia sp!, {r0-r3} ret lr +ENDPROC(return_to_handler) #endif ENTRY(ftrace_stub)
arch/arm/kernel/ftrace.c +17 -2
@@ -22,6 +22,7 @@#include <asm/ftrace.h> #include <asm/insn.h> #include <asm/set_memory.h> +#include <asm/stacktrace.h> #include <asm/patch.h> #ifdef CONFIG_THUMB2_KERNEL
@@ -213,8 +214,10 @@ int ftrace_make_nop(struct module *mod,#endif /* CONFIG_DYNAMIC_FTRACE */ #ifdef CONFIG_FUNCTION_GRAPH_TRACER +asmlinkage void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, -unsigned long frame_pointer) +unsigned long frame_pointer, +unsigned long stack_pointer) {unsigned long return_hooker = (unsigned long ) &return_to_handler;unsigned long old;
@@ -225,6 +228,18 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,if (IS_ENABLED (CONFIG_UNWINDER_FRAME_POINTER )) {/* FP points one word below parent's top of stack */ frame_pointer +=4 ; + }else { +struct stackframe frame = { + .fp = frame_pointer, + .sp = stack_pointer, + .lr = self_addr, + .pc = self_addr, + }; +if (unwind_frame (&frame) <0 ) +return ; +if (frame.lr != self_addr) + parent = frame.lr_addr; + frame_pointer = frame.sp; } old = *parent;
@@ -247,7 +262,7 @@ static int __ftrace_modify_caller(unsigned long *callsite,unsigned long caller_fn = (unsigned long ) func;unsigned long pc = (unsigned long ) callsite;unsigned long branch =arm_gen_branch (pc, caller_fn); -unsigned long nop =0xe1a00000 ;/* mov r0, r0 */ +unsigned long nop =arm_gen_nop ();unsigned long old = enable ? nop : branch;unsigned long new = enable ? branch : nop;
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 9/12 ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
In reply to Ard BiesheuvelThumb2 code uses R7 as the frame pointer rather than R11, because the opcodes to access it are generally shorter.
This means that there are cases where we cannot simply add it to the clobber list of an asm() block, but need to preserve/restore it explicitly, or the compiler may complain in some cases (e.g., Clang builds with ftrace enabled).
Since R11 is not special in that case, clobber it instead, and use it to preserve/restore the value of R7.
--- arch/arm/include/asm/opcodes.h | 14 ++++++++ arch/arm/probes/kprobes/actions-common.c | 6 ++-- arch/arm/probes/kprobes/actions-thumb.c | 38 ++++++++++++++++---- 3 files changed, 48 insertions(+), 10 deletions(-)
arch/arm/include/asm/opcodes.h +14 -0
@@ -230,4 +230,18 @@ extern __u32 __opcode_to_mem_thumb32(__u32); ".short " __stringify(first) ", " __stringify(second) "\n\t" #endif +/* +* Which register to preserve and which register can be clobbered in inline asm +* that needs to be compatible with code that emits frame pointers. +*/ +#ifdef CONFIG_THUMB2_KERNEL +#define FPREG_PRESERVE "r7" +#define FPREG_CLOBBER "r11" +#define FPREG_PRESERVE_R7 +#else +#define FPREG_PRESERVE "fp" +#define FPREG_CLOBBER "r7" +#define FPREG_PRESERVE_R11 +#endif +#endif /* __ASM_ARM_OPCODES_H */
arch/arm/probes/kprobes/actions-common.c +3 -3
@@ -84,7 +84,7 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn, register void *rfn asm("lr") = asi->insn_fn; __asm__ __volatile__ ( - "stmdb sp!, {%[regs], r11} \n\t" + "stmdb sp!, {%[regs], " FPREG_PRESERVE "}\n\t" "ldmia %[regs], {r0-r12} \n\t"#if __LINUX_ARM_ARCH__ >=6 "blx %[fn] \n\t"
@@ -96,10 +96,10 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn,#endif "ldr lr, [sp], #4 \n\t" /* lr = regs */ "stmia lr, {r0-r12} \n\t" -"ldr r11, [sp], #4 \n\t" +"ldr " FPREG_PRESERVE ", [sp], #4\n\t" : [regs] "=r" (rregs), [fn] "=r" (rfn) : "0" (rregs), "1" (rfn) - : "r0", "r2", "r3", "r4", "r5", "r6", "r7", + : "r0", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER, "r8", "r9", "r10", "r12", "memory", "cc" ); }
arch/arm/probes/kprobes/actions-thumb.c +31 -7
@@ -447,14 +447,20 @@ t16_emulate_loregs(probes_opcode_t insn, __asm__ __volatile__ ( "msr cpsr_fs, %[oldcpsr] \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov r7, fp \n\t" +#endif"mrs %[newcpsr], cpsr \n\t" : [newcpsr]"=r" (newcpsr) : [oldcpsr]"r" (oldcpsr), [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" , + : "r0", "r1", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER,"lr" ,"memory" ,"cc" );
@@ -524,15 +530,21 @@ t16_emulate_push(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldr r8, [%[regs], #14*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r8" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r8" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }
@@ -558,15 +570,21 @@ t16_emulate_pop_nopc(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }
@@ -577,15 +595,21 @@ t16_emulate_pop_pc(probes_opcode_t insn, registerunsigned long pc asm("r8" ); __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif :"=r" (pc) : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" );bx_write_pc (pc, regs);
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
Quoted 29 lines · Ard Biesheuvel
Thumb2 code uses R7 as the frame pointer rather than R11, because the opcodes to access it are generally shorter.
This means that there are cases where we cannot simply add it to the clobber list of an
asm()block, but need to preserve/restore it explicitly, or the compiler may complain in some cases (e.g., Clang builds with ftrace enabled).Since R11 is not special in that case, clobber it instead, and use it to preserve/restore the value of R7.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm/include/asm/opcodes.h | 14 ++++++++ arch/arm/probes/kprobes/actions-common.c | 6 ++-- arch/arm/probes/kprobes/actions-thumb.c | 38 ++++++++++++++++---- 3 files changed, 48 insertions(+), 10 deletions(-)arch/arm/include/asm/opcodes.h +2
-0@@ -230,4 +230,18 @@ extern __u32 __opcode_to_mem_thumb32(__u32); ".short " __stringify(first) ", " __stringify(second) "\n\t" #endif +/* +* Which register to preserve and which register can be clobbered in inline asm
arch/arm/include/asm/opcodes.h extern __u32 __opcode_to_mem_thumb32(__u32); · line 244
Quoted 10 lines
arch/arm/include/asm/opcodes.h +10
-0+* that needs to be compatible with code that emits frame pointers. +*/ +#ifdef CONFIG_THUMB2_KERNEL +#define FPREG_PRESERVE "r7" +#define FPREG_CLOBBER "r11" +#define FPREG_PRESERVE_R7 +#else +#define FPREG_PRESERVE "fp" +#define FPREG_CLOBBER "r7" +#define FPREG_PRESERVE_R11
I think you might be able to get away without FPREG_PRESERVE_R11 and even without FPREG_PRESERVE_R7. FPREG_PRESERVE_R11 is currently unused and FPREG_PRESERVE_R7 is synonymous with CONFIG_THUMB2_KERNEL at the moment (It doesn't look like patches 11 or 12 change that either). Looking at arch/arm/probes/kprobes/Makefile, arch/arm/probes/kprobes/actions-thumb.c is only built when CONFIG_THUMB2_KERNEL is enabled, so I don't think the #ifdefs are required; those movs could be unguarded by the preprocessor?
Quoted 130 lines
arch/arm/include/asm/opcodes.h +2
-0+#endif +#endif /* __ASM_ARM_OPCODES_H */ arch/arm/probes/kprobes/actions-common.c +3
-3@@ -84,7 +84,7 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn, register void *rfn asm("lr") = asi->insn_fn; __asm__ __volatile__ ( - "stmdb sp!, {%[regs], r11} \n\t" + "stmdb sp!, {%[regs], " FPREG_PRESERVE "}\n\t" "ldmia %[regs], {r0-r12} \n\t"#if __LINUX_ARM_ARCH__ >=6 "blx %[fn] \n\t" @@ -96,10 +96,10 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn,#endif "ldr lr, [sp], #4 \n\t" /* lr = regs */ "stmia lr, {r0-r12} \n\t" -"ldr r11, [sp], #4 \n\t" +"ldr " FPREG_PRESERVE ", [sp], #4\n\t" : [regs] "=r" (rregs), [fn] "=r" (rfn) : "0" (rregs), "1" (rfn) - : "r0", "r2", "r3", "r4", "r5", "r6", "r7", + : "r0", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER, "r8", "r9", "r10", "r12", "memory", "cc" ); }arch/arm/probes/kprobes/actions-thumb.c +31
-8@@ -447,14 +447,20 @@ t16_emulate_loregs(probes_opcode_t insn, __asm__ __volatile__ ( "msr cpsr_fs, %[oldcpsr] \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov r7, fp \n\t" +#endif"mrs %[newcpsr], cpsr \n\t" : [newcpsr]"=r" (newcpsr) : [oldcpsr]"r" (oldcpsr), [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" , + : "r0", "r1", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER,"lr" ,"memory" ,"cc" );@@ -524,15 +530,21 @@ t16_emulate_push(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldr r8, [%[regs], #14*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r8" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r8" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }@@ -558,15 +570,21 @@ t16_emulate_pop_nopc(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }@@ -577,15 +595,21 @@ t16_emulate_pop_pc(probes_opcode_t insn, registerunsigned long pc asm("r8" ); __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif :"=r" (pc) : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" );bx_write_pc (pc, regs); --2.30.2
Signature · 7 lines
Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
Quoted 42 lines · Nick Desaulniers
Quoted 29 lines · Ard Biesheuvel
Thumb2 code uses R7 as the frame pointer rather than R11, because the opcodes to access it are generally shorter.
This means that there are cases where we cannot simply add it to the clobber list of an
asm()block, but need to preserve/restore it explicitly, or the compiler may complain in some cases (e.g., Clang builds with ftrace enabled).Since R11 is not special in that case, clobber it instead, and use it to preserve/restore the value of R7.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm/include/asm/opcodes.h | 14 ++++++++ arch/arm/probes/kprobes/actions-common.c | 6 ++-- arch/arm/probes/kprobes/actions-thumb.c | 38 ++++++++++++++++---- 3 files changed, 48 insertions(+), 10 deletions(-)arch/arm/include/asm/opcodes.h +2
-0@@ -230,4 +230,18 @@ extern __u32 __opcode_to_mem_thumb32(__u32); ".short " __stringify(first) ", " __stringify(second) "\n\t" #endif +/* +* Which register to preserve and which register can be clobbered in inline asm arch/arm/include/asm/opcodes.h extern __u32 __opcode_to_mem_thumb32(__u32); · line 244
Quoted 10 lines
arch/arm/include/asm/opcodes.h +10
-0+* that needs to be compatible with code that emits frame pointers. +*/ +#ifdef CONFIG_THUMB2_KERNEL +#define FPREG_PRESERVE "r7" +#define FPREG_CLOBBER "r11" +#define FPREG_PRESERVE_R7 +#else +#define FPREG_PRESERVE "fp" +#define FPREG_CLOBBER "r7" +#define FPREG_PRESERVE_R11
Quoted 9 lines
I think you might be able to get away without
FPREG_PRESERVE_R11and even withoutFPREG_PRESERVE_R7.FPREG_PRESERVE_R11is currently unused andFPREG_PRESERVE_R7is synonymous withCONFIG_THUMB2_KERNELat the moment (It doesn't look like patches 11 or 12 change that either). Looking atarch/arm/probes/kprobes/Makefile,arch/arm/probes/kprobes/actions-thumb.cis only built whenCONFIG_THUMB2_KERNELis enabled, so I don't think the #ifdefs are required; thosemovs could be unguarded by the preprocessor?
You're quite right. I'll get rid of the macro and the CPP conditionals.
Quoted 126 lines
Quoted 126 lines
arch/arm/include/asm/opcodes.h +2
-0+#endif +#endif /* __ASM_ARM_OPCODES_H */ arch/arm/probes/kprobes/actions-common.c +3
-3@@ -84,7 +84,7 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn, register void *rfn asm("lr") = asi->insn_fn; __asm__ __volatile__ ( - "stmdb sp!, {%[regs], r11} \n\t" + "stmdb sp!, {%[regs], " FPREG_PRESERVE "}\n\t" "ldmia %[regs], {r0-r12} \n\t"#if __LINUX_ARM_ARCH__ >=6 "blx %[fn] \n\t" @@ -96,10 +96,10 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn,#endif "ldr lr, [sp], #4 \n\t" /* lr = regs */ "stmia lr, {r0-r12} \n\t" -"ldr r11, [sp], #4 \n\t" +"ldr " FPREG_PRESERVE ", [sp], #4\n\t" : [regs] "=r" (rregs), [fn] "=r" (rfn) : "0" (rregs), "1" (rfn) - : "r0", "r2", "r3", "r4", "r5", "r6", "r7", + : "r0", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER, "r8", "r9", "r10", "r12", "memory", "cc" ); }arch/arm/probes/kprobes/actions-thumb.c +31
-7@@ -447,14 +447,20 @@ t16_emulate_loregs(probes_opcode_t insn, __asm__ __volatile__ ( "msr cpsr_fs, %[oldcpsr] \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov r7, fp \n\t" +#endif"mrs %[newcpsr], cpsr \n\t" : [newcpsr]"=r" (newcpsr) : [oldcpsr]"r" (oldcpsr), [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" , + : "r0", "r1", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER,"lr" ,"memory" ,"cc" );@@ -524,15 +530,21 @@ t16_emulate_push(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldr r8, [%[regs], #14*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r8" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r8" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }@@ -558,15 +570,21 @@ t16_emulate_pop_nopc(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }@@ -577,15 +595,21 @@ t16_emulate_pop_pc(probes_opcode_t insn, registerunsigned long pc asm("r8" ); __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif :"=r" (pc) : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" );
Quoted 9 lines
Quoted 4 lines
arch/arm/probes/kprobes/actions-thumb.c +0
-1bx_write_pc (pc, regs); --2.30.2
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Masami Hiramatsu
maintainerRe: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
In reply to Ard BiesheuvelQuoted 10 lines · On Mon, 31 Jan 2022 18:03:44 +0100 Ard Biesheuvel <[email protected]> wrote:
Thumb2 code uses R7 as the frame pointer rather than R11, because the opcodes to access it are generally shorter.
This means that there are cases where we cannot simply add it to the clobber list of an
asm()block, but need to preserve/restore it explicitly, or the compiler may complain in some cases (e.g., Clang builds with ftrace enabled).Since R11 is not special in that case, clobber it instead, and use it to preserve/restore the value of R7.
Thanks Ard for fixing thumb2 issue!
BTW, have you build the kernel with CONFIG_KPROBES_SANITY_TEST=y? It should check the backtrace from kprobe and kretprobe at boot time.
Thank you,
Quoted 158 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm/include/asm/opcodes.h | 14 ++++++++ arch/arm/probes/kprobes/actions-common.c | 6 ++-- arch/arm/probes/kprobes/actions-thumb.c | 38 ++++++++++++++++---- 3 files changed, 48 insertions(+), 10 deletions(-)arch/arm/include/asm/opcodes.h +14
-0@@ -230,4 +230,18 @@ extern __u32 __opcode_to_mem_thumb32(__u32); ".short " __stringify(first) ", " __stringify(second) "\n\t" #endif +/* +* Which register to preserve and which register can be clobbered in inline asm +* that needs to be compatible with code that emits frame pointers. +*/ +#ifdef CONFIG_THUMB2_KERNEL +#define FPREG_PRESERVE "r7" +#define FPREG_CLOBBER "r11" +#define FPREG_PRESERVE_R7 +#else +#define FPREG_PRESERVE "fp" +#define FPREG_CLOBBER "r7" +#define FPREG_PRESERVE_R11 +#endif +#endif /* __ASM_ARM_OPCODES_H */ arch/arm/probes/kprobes/actions-common.c +3
-3@@ -84,7 +84,7 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn, register void *rfn asm("lr") = asi->insn_fn; __asm__ __volatile__ ( - "stmdb sp!, {%[regs], r11} \n\t" + "stmdb sp!, {%[regs], " FPREG_PRESERVE "}\n\t" "ldmia %[regs], {r0-r12} \n\t"#if __LINUX_ARM_ARCH__ >=6 "blx %[fn] \n\t" @@ -96,10 +96,10 @@ emulate_generic_r0_12_noflags(probes_opcode_t insn,#endif "ldr lr, [sp], #4 \n\t" /* lr = regs */ "stmia lr, {r0-r12} \n\t" -"ldr r11, [sp], #4 \n\t" +"ldr " FPREG_PRESERVE ", [sp], #4\n\t" : [regs] "=r" (rregs), [fn] "=r" (rfn) : "0" (rregs), "1" (rfn) - : "r0", "r2", "r3", "r4", "r5", "r6", "r7", + : "r0", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER, "r8", "r9", "r10", "r12", "memory", "cc" ); }arch/arm/probes/kprobes/actions-thumb.c +31
-7@@ -447,14 +447,20 @@ t16_emulate_loregs(probes_opcode_t insn, __asm__ __volatile__ ( "msr cpsr_fs, %[oldcpsr] \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" +#ifdef FPREG_PRESERVE_R7 + "mov r7, fp \n\t" +#endif"mrs %[newcpsr], cpsr \n\t" : [newcpsr]"=r" (newcpsr) : [oldcpsr]"r" (oldcpsr), [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" , + : "r0", "r1", "r2", "r3", "r4", "r5", "r6", FPREG_CLOBBER,"lr" ,"memory" ,"cc" );@@ -524,15 +530,21 @@ t16_emulate_push(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldr r8, [%[regs], #14*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r8" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r8" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }@@ -558,15 +570,21 @@ t16_emulate_pop_nopc(probes_opcode_t insn, struct arch_probes_insn *asi, struct pt_regs *regs) { __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif : : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" ); }@@ -577,15 +595,21 @@ t16_emulate_pop_pc(probes_opcode_t insn, registerunsigned long pc asm("r8" ); __asm__ __volatile__ ( +#ifdef FPREG_PRESERVE_R7 + "mov fp, r7 \n\t" +#endif"ldr r9, [%[regs], #13*4] \n\t" "ldmia %[regs], {r0-r7} \n\t" "blx %[fn] \n\t" "stmia %[regs], {r0-r7} \n\t" "str r9, [%[regs], #13*4] \n\t" +#ifdef FPREG_PRESERVE_R7 +"mov r7, fp \n\t" +#endif :"=r" (pc) : [regs]"r" (regs), [fn]"r" (asi->insn_fn) - :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r7" ,"r9" , -"lr" ,"memory" ,"cc" + :"r0" ,"r1" ,"r2" ,"r3" ,"r4" ,"r5" ,"r6" ,"r9" , + FPREG_CLOBBER, "lr", "memory", "cc" );bx_write_pc (pc, regs);Signature · 1 line
2.30.2
Signature · 6 lines
Masami Hiramatsu <[email protected]> _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
Quoted 12 lines · Masami Hiramatsu
Quoted 8 lines · On Mon, 31 Jan 2022 18:03:44 +0100 Ard Biesheuvel <[email protected]> wrote:
Thumb2 code uses R7 as the frame pointer rather than R11, because the opcodes to access it are generally shorter.
This means that there are cases where we cannot simply add it to the clobber list of an
asm()block, but need to preserve/restore it explicitly, or the compiler may complain in some cases (e.g., Clang builds with ftrace enabled).
Quoted 4 lines
Quoted 2 lines
Since R11 is not special in that case, clobber it instead, and use it to preserve/restore the value of R7.
Thanks Ard for fixing thumb2 issue!
No problem. Although I'm still not 100% sure we can simply preserve/restore the frame pointer like that. What is the expected result when an exception occurs in the kprobes emulation code?
The main problem is that with unwind info unwinding, the stack pointer and the frame pointer need to be mutually in sync, and with the kprobes stack frames in the middle, this is no longer correct - the emulated code is executed with the frame pointer value that was captured when the probe was hit, by the stack has a couple of frames added.
Quoted 3 lines
BTW, have you build the kernel with
CONFIG_KPROBES_SANITY_TEST=y? It should check the backtrace from kprobe and kretprobe at boot time.
Just checked, and those work fine.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
Quoted 13 lines · On Tue, 1 Feb 2022 15:05:25 +0100 Ard Biesheuvel <[email protected]> wrote:
Quoted 12 lines · Masami Hiramatsu
Quoted 8 lines · On Mon, 31 Jan 2022 18:03:44 +0100 Ard Biesheuvel <[email protected]> wrote:
Thumb2 code uses R7 as the frame pointer rather than R11, because the opcodes to access it are generally shorter.
This means that there are cases where we cannot simply add it to the clobber list of an
asm()block, but need to preserve/restore it explicitly, or the compiler may complain in some cases (e.g., Clang builds with ftrace enabled).
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Since R11 is not special in that case, clobber it instead, and use it to preserve/restore the value of R7.
Thanks Ard for fixing thumb2 issue!
No problem. Although I'm still not 100% sure we can simply preserve/restore the frame pointer like that. What is the expected result when an exception occurs in the kprobes emulation code?
What kind of exception would you mean? If it is a synchronous exception and can return to where the exception happens, the all registers must be restored. So as long as it is restored it is OK to me.
Quoted 7 lines
The main problem is that with unwind info unwinding, the stack pointer and the frame pointer need to be mutually in sync, and with the kprobes stack frames in the middle, this is no longer correct - the emulated code is executed with the frame pointer value that was captured when the probe was hit, by the stack has a couple of frames added.
Quoted 5 lines
Quoted 3 lines
BTW, have you build the kernel with
CONFIG_KPROBES_SANITY_TEST=y? It should check the backtrace from kprobe and kretprobe at boot time.Just checked, and those work fine.
Thank you for testing!
Thank you,
Signature · 6 lines
Masami Hiramatsu <[email protected]> _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 09/12] ARM: kprobes: treat R7 as the frame pointer register in Thumb2 builds
Quoted 22 lines · Masami Hiramatsu
Quoted 18 lines · On Tue, 1 Feb 2022 15:05:25 +0100 Ard Biesheuvel <[email protected]> wrote:
Quoted 16 lines · Masami Hiramatsu
Quoted 10 lines · On Mon, 31 Jan 2022 18:03:44 +0100 Ard Biesheuvel <[email protected]> wrote:
Thumb2 code uses R7 as the frame pointer rather than R11, because the opcodes to access it are generally shorter.
This means that there are cases where we cannot simply add it to the clobber list of an
asm()block, but need to preserve/restore it explicitly, or the compiler may complain in some cases (e.g., Clang builds with ftrace enabled).Since R11 is not special in that case, clobber it instead, and use it to preserve/restore the value of R7.
Thanks Ard for fixing thumb2 issue!
Quoted 9 lines
Quoted 3 lines
No problem. Although I'm still not 100% sure we can simply preserve/restore the frame pointer like that. What is the expected result when an exception occurs in the kprobes emulation code?
What kind of exception would you mean? If it is a synchronous exception and can return to where the exception happens, the all registers must be restored. So as long as it is restored it is OK to me.
So what should happen if a backtrace is triggered while the kprobes emulation code is calling into a function?
Quoted 11 lines
Quoted 11 lines
The main problem is that with unwind info unwinding, the stack pointer and the frame pointer need to be mutually in sync, and with the kprobes stack frames in the middle, this is no longer correct - the emulated code is executed with the frame pointer value that was captured when the probe was hit, by the stack has a couple of frames added.
Quoted 3 lines
BTW, have you build the kernel with
CONFIG_KPROBES_SANITY_TEST=y? It should check the backtrace from kprobe and kretprobe at boot time.
Quoted 6 lines
Quoted 1 line
Just checked, and those work fine.
Thank you for testing!
Reviewed-by Masami Hiramatsu <[email protected]>
Thanks!
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 10/12 drivers/firmware/scmi: disable ftrace for Clang Thumb2 builds
In reply to Ard BiesheuvelThe SMC calling convention designates R0-R7 as input registers in AArch32 mode, and this conflicts with the compiler's use of R7 as a frame pointer when building in Thumb2 mode. Generally, we don't enable the frame pointer, and GCC happily enables the -pg profiling hooks without them. However, Clang refuses, and errors out with the message below:
drivers/firmware/arm_scmi/smc.c:152:2: error: write to reserved register 'R7'
arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res);
^
include/linux/arm-smccc.h:550:4: note: expanded from macro 'arm_smccc_1_1_invoke'
arm_smccc_1_1_smc(__VA_ARGS__); \
^Let's just disable ftrace for the compilation unit when building this configuration.
--- drivers/firmware/arm_scmi/Makefile | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/firmware/arm_scmi/Makefile +7 -0
@@ -11,3 +11,10 @@ scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \ $(scmi-transport-y) obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o + +ifeq ($(CONFIG_THUMB2_KERNEL)$(CONFIG_CC_IS_CLANG),yy) +# The use of R7 in the SMCCC conflicts with the compiler's use of R7 as a frame +# pointer in Thumb2 mode, which is forcibly enabled by Clang when profiling +# hooks are inserted via the -pg switch. +CFLAGS_REMOVE_smc.o += $(CC_FLAGS_FTRACE) +endif
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 10/12] drivers/firmware/scmi: disable ftrace for Clang Thumb2 builds
Quoted 8 lines · Ard Biesheuvel
The SMC calling convention designates R0-R7 as input registers in AArch32 mode, and this conflicts with the compiler's use of R7 as a frame pointer when building in Thumb2 mode. Generally, we don't enable the frame pointer, and GCC happily enables the -pg profiling hooks without them. However, Clang refuses, and errors out with the message below:
Quoted 8 lines
drivers/firmware/arm_scmi/smc.c:152:2: error: write to reserved register 'R7'arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res); ^ include/linux/arm-smccc.h:550:4: note: expanded from macro 'arm_smccc_1_1_invoke' arm_smccc_1_1_smc(__VA_ARGS__); \ ^Let's just disable ftrace for the compilation unit when building this configuration.
Thanks for the patch!
Would it make sense for clang not to warn in such cases? (i.e. write to r7 for thumb when -fomit-frame-pointers is specified) If so, we can file a feature request and at least add a link in the comment added in the Makefile.
Quoted 24 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/arm_scmi/Makefile | 7 +++++++ 1 file changed, 7 insertions(+)drivers/firmware/arm_scmi/Makefile +7
-1@@ -11,3 +11,10 @@ scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \ $(scmi-transport-y) obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o + +ifeq ($(CONFIG_THUMB2_KERNEL)$(CONFIG_CC_IS_CLANG),yy) +# The use of R7 in the SMCCC conflicts with the compiler's use of R7 as a frame +# pointer in Thumb2 mode, which is forcibly enabled by Clang when profiling +# hooks are inserted via the -pg switch. +CFLAGS_REMOVE_smc.o += $(CC_FLAGS_FTRACE) +endif --2.30.2
Signature · 7 lines
Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 10/12] drivers/firmware/scmi: disable ftrace for Clang Thumb2 builds
Quoted 19 lines · Nick Desaulniers
Quoted 8 lines · Ard Biesheuvel
The SMC calling convention designates R0-R7 as input registers in AArch32 mode, and this conflicts with the compiler's use of R7 as a frame pointer when building in Thumb2 mode. Generally, we don't enable the frame pointer, and GCC happily enables the -pg profiling hooks without them. However, Clang refuses, and errors out with the message below:
Quoted 8 lines
drivers/firmware/arm_scmi/smc.c:152:2: error: write to reserved register 'R7'arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res); ^ include/linux/arm-smccc.h:550:4: note: expanded from macro 'arm_smccc_1_1_invoke' arm_smccc_1_1_smc(__VA_ARGS__); \ ^Let's just disable ftrace for the compilation unit when building this configuration.
Quoted 8 lines
Thanks for the patch!
Reviewed-by Nick Desaulniers <[email protected]>Would it make sense for clang not to warn in such cases? (i.e. write to r7 for thumb when -fomit-frame-pointers is specified) If so, we can file a feature request and at least add a link in the comment added in the
Makefile.
This is a bit tricky. The ftrace code does not rely on a frame pointer at all, which is why this is only an issue on Clang, as GCC happily ignores the use of R7. On Clang, the frame pointer is mandatory with -pg:
clang: error: invalid argument '-fomit-frame-pointer' not allowed with '-pg'
However, the way the unwinder is wired up in Linux means that only R7 is usable as an unwind anchor (apart from SP), which means that clobbering R7 even temporarily is risky. Note that the same applies to FP in places where we preserve/restore it currently. The fact that -pg implies -fno-omit-frame-pointer on Clang makes this even worse, because it means that all emitted unwind info will use R7 rather than SP.
So it would help if we could relax the requirement for -fno-omit-frame-pointer with -pg on ARM (provided that there is no reason it is needed that I have missed). That would improve codegen as well.
But it doesn't fix the underlying issue that clobbering R7 is risky, and we should whether the clobber is sufficient to discourage the compiler from using it as the unwind anchor. If this is the case, we should probably pass -fno-unwind-tables to each function that uses R7 or FP in this manner.
Quoted 21 lines
Quoted 21 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/arm_scmi/Makefile | 7 +++++++ 1 file changed, 7 insertions(+)drivers/firmware/arm_scmi/Makefile +7
-0@@ -11,3 +11,10 @@ scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \ $(scmi-transport-y) obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o + +ifeq ($(CONFIG_THUMB2_KERNEL)$(CONFIG_CC_IS_CLANG),yy) +# The use of R7 in the SMCCC conflicts with the compiler's use of R7 as a frame +# pointer in Thumb2 mode, which is forcibly enabled by Clang when profiling +# hooks are inserted via the -pg switch. +CFLAGS_REMOVE_smc.o += $(CC_FLAGS_FTRACE) +endif
Quoted 8 lines
Quoted 3 lines
--
2.30.2--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 10/12] drivers/firmware/scmi: disable ftrace for Clang Thumb2 builds
In reply to Ard BiesheuvelQuoted 7 lines · Ard Biesheuvel
The SMC calling convention designates R0-R7 as input registers in AArch32 mode, and this conflicts with the compiler's use of R7 as a frame pointer when building in Thumb2 mode. Generally, we don't enable the frame pointer, and GCC happily enables the -pg profiling hooks without them. However, Clang refuses, and errors out with the message below:
Quoted 9 lines
drivers/firmware/arm_scmi/smc.c:152:2: error: write to reserved register 'R7'arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res); ^ include/linux/arm-smccc.h:550:4: note: expanded from macro 'arm_smccc_1_1_invoke' arm_smccc_1_1_smc(__VA_ARGS__); \ ^Let's just disable ftrace for the compilation unit when building this configuration.
--
Regards,
Sudeep
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 11/12 ARM: cacheflush: avoid clobbering the frame pointer in Thumb2 mode
In reply to Ard BiesheuvelThumb2 uses R7 rather than R11 as the frame pointer, and even if we rarely use a frame pointer to begin with when building in Thumb2 mode, there are cases where it is required by the compiler (Clang when inserting profiling hooks via -pg)
So let's preserve whichever register is considered the frame pointer depending on whether we are building for ARM or Thumb2, rather than preserving/restoring R11 explicitly.
--- arch/arm/include/asm/cacheflush.h | 7 ++++--- arch/arm/mach-exynos/mcpm-exynos.c | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-)
arch/arm/include/asm/cacheflush.h +4 -3
@@ -12,6 +12,7 @@#include <asm/glue-cache.h> #include <asm/shmparam.h> #include <asm/cachetype.h> +#include <asm/opcodes.h> #include <asm/outercache.h> #define CACHE_COLOUR (vaddr) ((vaddr & (SHMLBA - 1)) >> PAGE_SHIFT)
@@ -454,7 +455,7 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size)#define v7_exit_coherency_flush (level) \ asm volatile( \ ".arch armv7-a \n\t" \ - "stmfd sp!, {fp, ip} \n\t" \ + "stmfd sp!, {" FPREG_PRESERVE ", ip} \n\t" \ "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ "bic r0, r0, #"__stringify(CR_C)" \n\t" \ "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \
@@ -465,8 +466,8 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size) "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ "isb \n\t" \ "dsb \n\t" \ - "ldmfd sp!, {fp, ip}" \ - : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ + "ldmfd sp!, {" FPREG_PRESERVE ", ip}" \ + : : : "r0","r1","r2","r3","r4","r5","r6", FPREG_CLOBBER, \ "r9","r10","lr","memory" ) void flush_uprobe_xol_access(struct page *page, unsigned long uaddr,
arch/arm/mach-exynos/mcpm-exynos.c +4 -3
@@ -14,6 +14,7 @@#include <asm/cputype.h> #include <asm/cp15.h> #include <asm/mcpm.h> +#include <asm/opcodes.h> #include <asm/smp_plat.h> #include "common.h"
@@ -35,7 +36,7 @@ static bool secure_firmware __ro_after_init;*/ #define exynos_v7_exit_coherency_flush (level) \ asm volatile( \ - "stmfd sp!, {fp, ip}\n\t"\ + "stmfd sp!, {" FPREG_PRESERVE ", ip} \n\t" \ "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR\n\t" \ "bic r0, r0, #"__stringify(CR_C)"\n\t" \ "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR\n\t" \
@@ -50,10 +51,10 @@ static bool secure_firmware __ro_after_init; "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR\n\t" \ "isb\n\t" \ "dsb\n\t" \ - "ldmfd sp!, {fp, ip}" \ + "ldmfd sp!, {" FPREG_PRESERVE ", ip}" \ : \ : "Ir" (pmu_base_addr + S5P_INFORM0) \ -: " r0", " r1", " r2", " r3", " r4", " r5", " r6", " r7", \ +: " r0", " r1", " r2", " r3", " r4", " r5", " r6", FPREG_CLOBBER, \ "r9", "r10", "lr", "memory") static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 11/12] ARM: cacheflush: avoid clobbering the frame pointer in Thumb2 mode
Quoted 11 lines · Ard Biesheuvel
Thumb2 uses R7 rather than R11 as the frame pointer, and even if we rarely use a frame pointer to begin with when building in Thumb2 mode, there are cases where it is required by the compiler (Clang when inserting profiling hooks via -pg)
So let's preserve whichever register is considered the frame pointer depending on whether we are building for ARM or Thumb2, rather than preserving/restoring R11 explicitly.
Signed-off-by Ard Biesheuvel <[email protected]>
Thanks for the patch!
Quoted 74 lines
--- arch/arm/include/asm/cacheflush.h | 7 ++++--- arch/arm/mach-exynos/mcpm-exynos.c | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-)arch/arm/include/asm/cacheflush.h +4
-3@@ -12,6 +12,7 @@#include <asm/glue-cache.h> #include <asm/shmparam.h> #include <asm/cachetype.h> +#include <asm/opcodes.h> #include <asm/outercache.h> #define CACHE_COLOUR (vaddr) ((vaddr & (SHMLBA - 1)) >> PAGE_SHIFT)@@ -454,7 +455,7 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size)#define v7_exit_coherency_flush (level) \ asm volatile( \ ".arch armv7-a \n\t" \ - "stmfd sp!, {fp, ip} \n\t" \ + "stmfd sp!, {" FPREG_PRESERVE ", ip} \n\t" \ "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ "bic r0, r0, #"__stringify(CR_C)" \n\t" \ "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \@@ -465,8 +466,8 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size) "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ "isb \n\t" \ "dsb \n\t" \ - "ldmfd sp!, {fp, ip}" \ - : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ + "ldmfd sp!, {" FPREG_PRESERVE ", ip}" \ + : : : "r0","r1","r2","r3","r4","r5","r6", FPREG_CLOBBER, \ "r9","r10","lr","memory" ) void flush_uprobe_xol_access(struct page *page, unsigned long uaddr,arch/arm/mach-exynos/mcpm-exynos.c +4
-4@@ -14,6 +14,7 @@#include <asm/cputype.h> #include <asm/cp15.h> #include <asm/mcpm.h> +#include <asm/opcodes.h> #include <asm/smp_plat.h> #include "common.h" @@ -35,7 +36,7 @@ static bool secure_firmware __ro_after_init;*/ #define exynos_v7_exit_coherency_flush (level) \ asm volatile( \ - "stmfd sp!, {fp, ip}\n\t"\ + "stmfd sp!, {" FPREG_PRESERVE ", ip} \n\t" \ "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR\n\t" \ "bic r0, r0, #"__stringify(CR_C)"\n\t" \ "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR\n\t" \@@ -50,10 +51,10 @@ static bool secure_firmware __ro_after_init; "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR\n\t" \ "isb\n\t" \ "dsb\n\t" \ - "ldmfd sp!, {fp, ip}" \ + "ldmfd sp!, {" FPREG_PRESERVE ", ip}" \ : \ : "Ir" (pmu_base_addr + S5P_INFORM0) \ - : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", \ +: " r0", " r1", " r2", " r3", " r4", " r5", " r6", FPREG_CLOBBER, \ "r9", "r10", "lr", "memory") static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster) --2.30.2
Signature · 7 lines
Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 12/12 Revert "ARM: 9144/1: forbid ftrace with clang and thumb2_kernel"
In reply to Ard BiesheuvelThis reverts commit ecb108e3e3f7c692082b7c6fce41779c3835854a.
Clang + Thumb2 with ftrace is now supported.
--- arch/arm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/Kconfig +1 -1
@@ -93,7 +93,7 @@ config ARM select HAVE_FAST_GUP if ARM_LPAE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !(THUMB2_KERNEL && CC_IS_CLANG) + select HAVE_FUNCTION_TRACER if !XIP_KERNEL select HAVE_FUTEX_CMPXCHG if FUTEX select HAVE_GCC_PLUGINS select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7)
Signature · 7 lines
2.30.2 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v2 12/12] Revert "ARM: 9144/1: forbid ftrace with clang and thumb2_kernel"
Quoted 6 lines · Ard Biesheuvel
This reverts commit
ecb108e3e3f7c692082b7c6fce41779c3835854a.Clang + Thumb2 with ftrace is now supported.
Signed-off-by Ard Biesheuvel <[email protected]>
Thanks for all of the work that went into getting this fixed properly!
Quoted 20 lines
--- arch/arm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/Kconfig +1
-2@@ -93,7 +93,7 @@ config ARM select HAVE_FAST_GUP if ARM_LPAE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !(THUMB2_KERNEL && CC_IS_CLANG) + select HAVE_FUNCTION_TRACER if !XIP_KERNEL select HAVE_FUTEX_CMPXCHG if FUTEX select HAVE_GCC_PLUGINS select HAVE_HW_BREAKPOINT if PERF_EVENTS && (CPU_V6 || CPU_V6K || CPU_V7) --2.30.2
Signature · 7 lines
Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 19 lines · On Mon, 31 Jan 2022 18:03:35 +0100 Ard Biesheuvel <[email protected]> wrote:
This series addresses a number of issues in the ARM support code for ftrace, mostly related to Thumb2 but affecting other configurations as well.
Changes since v1:
- add a couple of patches to enable ftrace in Thumb2 mode when building with Clang, which was the one remaining unsupported configuration
- fix up some minor code issues caught by the bots
- add some acksCc Steven Rostedt <[email protected]>Cc Sudeep Holla <[email protected]>Cc Cristian Marussi <[email protected]>Cc Nathan Chancellor <[email protected]>Cc Nick Desaulniers <[email protected]>Cc Arnd Bergmann <[email protected]>Cc Linus Walleij <[email protected]>Cc Masami Hiramatsu <[email protected]>
Quoted 14 lines
Ard Biesheuvel (12):
ARM: ftrace: ensure that ADR take Thumb bit into account
ARM: ftrace: use ADD not POP to counter PUSH at entry
ARM: ftrace: use trampolines to keep .init.text in branching range
ARM: ftrace: avoid redundant loads or clobbering IP
ARM: ftrace: avoid unnecessary literal loads
ARM: ftrace: enableHAVE_FUNCTION_GRAPH_FP_TEST
ARM: unwind: track location of LR value in stack frame
ARM: ftrace: enable the graph tracer with the EABI unwinder
ARM: kprobes: treat R7 as the frame pointer register in Thumb2 buildsdrivers/firmware/scmi: disable ftrace for Clang Thumb2 builds
ARM: cacheflush: avoid clobbering the frame pointer in Thumb2 mode Revert "ARM: 9144/1: forbid ftrace with clang andthumb2_kernel"
I'm not strong on ARM assembly, but to the best of my ability and reading the change logs, the "ftrace" portions look good.
-- Steve
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel