[PATCH 0/7] Fix handling of FP in THUMB2 mode

[PATCH 0/7] Fix handling of FP in THUMB2 mode

[Nikolay Borisov]

Currently, most code which deals with stack unwinding relies on the fact 
that the frame pointer is stored in register R11. This is primary done via 
referencing the ARM_fp inside the pt_regs array as well as calling the 
thread_saved_fp macro. Unfortunately, this is not true in the case when 
the kernel is compiled in THUMB2 mode, since gcc uses exclusively R7 
to be the frame pointer. 


This patch series aims to rectify the situation in the following ways: 

 * Patch 1/7 Changes the way the thread_saved_fp macro is defined so that
 THUMB2 case is automatically handled. It has already been tested and reviewed. 
 
 * Patch 2/7 Introduces a new function - arm_get_current_stack_frame, which 
 just copies the appropriate registers from "struct pt_regs" into 
 "struct stackframe". This replaces several places where common code such 
 as "copy regs.{pc, lr, sp, fp} to frame.{pc, lr, sp, fp}" is present. It is
 implemented by using the newly-introduced frame_pointer(regs) macro which
 decides which register to extract based on the current kernel compile option
 (CONFIG_THUMB2_KERNEL).

 * Patches 3-5,7 Actualy "flip the switch" so that code can start
 using the new function. 

 * Patch 6/7 Introduces the correct changes in traps.c where only the
 * frame_pointer and not a full "struct stack" frame is being passed. 


Nikolay Borisov (7):
  ARM: Make thread_save_fp macro aware of THUMB2 mode
  ARM: Introduce arm_get_current_stack_frame()
  ARM: perf: Make perf use arm_get_current_stackframe
  ARM: time: Make use of arm_get_current_stackframe
  ARM: unwind: Use arm_get_current_stackframe
  ARM: traps: Make use of the frame_pointer macro
  ARM: oprofile: Use of arm_get_current_stackframe

 arch/arm/include/asm/ptrace.h      | 3 +++
 arch/arm/include/asm/stacktrace.h  | 5 +++++
 arch/arm/include/asm/thread_info.h | 6 ++++++
 arch/arm/kernel/perf_event.c       | 5 +----
 arch/arm/kernel/stacktrace.c       | 9 +++++++++
 arch/arm/kernel/time.c             | 5 +----
 arch/arm/kernel/traps.c            | 6 ++++--
 arch/arm/kernel/unwind.c           | 8 +++-----
 arch/arm/oprofile/common.c         | 5 +----
 9 files changed, 33 insertions(+), 19 deletions(-)

-- 
1.8.1.5

[PATCH 1/7] ARM: Make thread_save_fp macro aware of THUMB2 mode

[Nikolay Borisov]

The thread_save_fp macro has been defined so that it always reads the fp member
of the cpu_context_save struct. However, in the case of THUMB2 the fp is saved
not in the fp (r11) member but rather in r7.

This patch changes the way the macro is defined such that FP is read from the
correct place depending on whether we are a THUMB2 kernel or not. This enables
the backtrace in sitaution such as "echo t > /proc/sysrq-trigger" or the
function in which a process sleeping when "ps -Al" is invoked.

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
Reviewed-by: Anurag Aggarwal <anurag19aggarwal@gmail.com>
---
 arch/arm/include/asm/thread_info.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
index f989d7c..e4e4208 100644
--- a/arch/arm/include/asm/thread_info.h
+++ b/arch/arm/include/asm/thread_info.h
@@ -114,8 +114,14 @@ static inline struct thread_info *current_thread_info(void)
 	((unsigned long)(task_thread_info(tsk)->cpu_context.pc))
 #define thread_saved_sp(tsk)	\
 	((unsigned long)(task_thread_info(tsk)->cpu_context.sp))
+
+#ifndef CONFIG_THUMB2_KERNEL
 #define thread_saved_fp(tsk)	\
 	((unsigned long)(task_thread_info(tsk)->cpu_context.fp))
+#else
+#define thread_saved_fp(tsk)	\
+	((unsigned long)(task_thread_info(tsk)->cpu_context.r7))
+#endif
 
 extern void crunch_task_disable(struct thread_info *);
 extern void crunch_task_copy(struct thread_info *, void *);
-- 
1.8.1.5

[PATCH 2/7] ARM: Introduce arm_get_current_stack_frame()

[Nikolay Borisov]

Currently there are numerous places where "struct pt_regs" are used to
populate "struct stackframe", however all of those location do not
consider the situation where the kernel might be compiled in THUMB2
mode, in which case the framepointer member of pt_regs become ARM_r7
instead of ARM_fp (r11). 


The easiest solution is to introduce a new function (in the spirit of
https://groups.google.com/forum/#!topic/linux.kernel/dA2YuUcSpZ4)
which would hide the complexity of initializing the stackframe struct
from pt_regs. While we are at it, also document this idiosincratic 
behavior  of the frame pointer in the definition of 
"struct stackframe2. 

Also implement a macro frame_pointer(regs) that would return the correct
register so that we can use it in cases where we just require the frame
pointer and not a whole struct stackframe

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/include/asm/ptrace.h     | 3 +++
 arch/arm/include/asm/stacktrace.h | 5 +++++
 arch/arm/kernel/stacktrace.c      | 9 +++++++++
 3 files changed, 17 insertions(+)

diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h
index c877654..cd5cff3 100644
--- a/arch/arm/include/asm/ptrace.h
+++ b/arch/arm/include/asm/ptrace.h
@@ -83,6 +83,9 @@ static inline long regs_return_value(struct pt_regs *regs)
 }
 
 #define instruction_pointer(regs)	(regs)->ARM_pc
+#define frame_pointer(regs)				\
+	IS_ENABLED(CONFIG_THUMB2_KERNEL) ? regs->ARM_r7 \
+	: regs->ARM_fp					\
 
 static inline void instruction_pointer_set(struct pt_regs *regs,
 					   unsigned long val)
diff --git a/arch/arm/include/asm/stacktrace.h b/arch/arm/include/asm/stacktrace.h
index 4d0a164..9cbdf36 100644
--- a/arch/arm/include/asm/stacktrace.h
+++ b/arch/arm/include/asm/stacktrace.h
@@ -2,12 +2,17 @@
 #define __ASM_STACKTRACE_H
 
 struct stackframe {
+	/* FP member should hold R7 when CONFIG_THUMB2_KERNEL is enabled.
+	 * and R11 otherwise 
+	 */
 	unsigned long fp;
 	unsigned long sp;
 	unsigned long lr;
 	unsigned long pc;
 };
 
+extern void arm_get_current_stackframe(struct pt_regs *regs, struct stackframe
+		*frame);
 extern int unwind_frame(struct stackframe *frame);
 extern void walk_stackframe(struct stackframe *frame,
 			    int (*fn)(struct stackframe *, void *), void *data);
diff --git a/arch/arm/kernel/stacktrace.c b/arch/arm/kernel/stacktrace.c
index af4e8c8..5d6ac4d 100644
--- a/arch/arm/kernel/stacktrace.c
+++ b/arch/arm/kernel/stacktrace.c
@@ -43,6 +43,15 @@ int notrace unwind_frame(struct stackframe *frame)
 }
 #endif
 
+void arm_get_current_stackframe(struct pt_regs *regs, struct stackframe *frame)
+{
+		frame->fp = frame_pointer(regs);
+		frame->sp = regs->ARM_sp;
+		frame->lr = regs->ARM_lr;
+		frame->pc = regs->ARM_pc;
+}
+EXPORT_SYMBOL_GPL(arm_get_current_stackframe);
+
 void notrace walk_stackframe(struct stackframe *frame,
 		     int (*fn)(struct stackframe *, void *), void *data)
 {
-- 
1.8.1.5

[PATCH 3/7] ARM: perf: Make perf use arm_get_current_stackframe

[Nikolay Borisov]

Make the perf backend use the API so that it correctly references the FP
when in THUMB2 mode

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/kernel/perf_event.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index a6bc431..55353fd 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -621,10 +621,7 @@ perf_callchain_kernel(struct perf_callchain_entry *entry, struct pt_regs *regs)
 		return;
 	}
 
-	fr.fp = regs->ARM_fp;
-	fr.sp = regs->ARM_sp;
-	fr.lr = regs->ARM_lr;
-	fr.pc = regs->ARM_pc;
+	arm_get_current_stackframe(regs, &fr);
 	walk_stackframe(&fr, callchain_trace, entry);
 }
 
-- 
1.8.1.5

[PATCH 4/7] ARM: time: Make use of arm_get_current_stackframe

[Nikolay Borisov]

Make use of the arm_get_current_stackframe api so that
the frame pointer is correctly referenced in THUMB2 mode

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/kernel/time.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
index 829a96d..0cc7e58 100644
--- a/arch/arm/kernel/time.c
+++ b/arch/arm/kernel/time.c
@@ -50,10 +50,7 @@ unsigned long profile_pc(struct pt_regs *regs)
 	if (!in_lock_functions(regs->ARM_pc))
 		return regs->ARM_pc;
 
-	frame.fp = regs->ARM_fp;
-	frame.sp = regs->ARM_sp;
-	frame.lr = regs->ARM_lr;
-	frame.pc = regs->ARM_pc;
+	arm_get_current_stackframe(regs, &frame);
 	do {
 		int ret = unwind_frame(&frame);
 		if (ret < 0)
-- 
1.8.1.5

[PATCH 5/7] ARM: unwind: Use arm_get_current_stackframe

[Nikolay Borisov]

Make the unwind code use the correct API so that the frame pointer
is extracted from the correct register.

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/kernel/unwind.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c
index 3c21769..7aaec44 100644
--- a/arch/arm/kernel/unwind.c
+++ b/arch/arm/kernel/unwind.c
@@ -479,12 +479,10 @@ void unwind_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 		tsk = current;
 
 	if (regs) {
-		frame.fp = regs->ARM_fp;
-		frame.sp = regs->ARM_sp;
-		frame.lr = regs->ARM_lr;
+		arm_get_current_stackframe(regs, &frame);
 		/* PC might be corrupted, use LR in that case. */
-		frame.pc = kernel_text_address(regs->ARM_pc)
-			 ? regs->ARM_pc : regs->ARM_lr;
+		if (!kernel_text_address(regs->ARM_pc))
+			frame.pc = regs->ARM_lr;
 	} else if (tsk == current) {
 		frame.fp = (unsigned long)__builtin_frame_address(0);
 		frame.sp = current_sp;
-- 
1.8.1.5

[PATCH 6/7] ARM: traps: Make use of the frame_pointer macro

[Nikolay Borisov]

Use the newly-introduced frame_pointer macro to extract
the correct FP based on whether we are in THUMB2 mode or not.

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/kernel/traps.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index abd2fc0..c8e4bb7 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -31,11 +31,13 @@
 #include <asm/exception.h>
 #include <asm/unistd.h>
 #include <asm/traps.h>
+#include <asm/ptrace.h>
 #include <asm/unwind.h>
 #include <asm/tls.h>
 #include <asm/system_misc.h>
 #include <asm/opcodes.h>
 
+
 static const char *handler[]= {
 	"prefetch abort",
 	"data abort",
@@ -184,7 +186,7 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 		tsk = current;
 
 	if (regs) {
-		fp = regs->ARM_fp;
+		fp = frame_pointer(regs);
 		mode = processor_mode(regs);
 	} else if (tsk != current) {
 		fp = thread_saved_fp(tsk);
@@ -719,7 +721,7 @@ asmlinkage int arm_syscall(int no, struct pt_regs *regs)
 		dump_instr("", regs);
 		if (user_mode(regs)) {
 			__show_regs(regs);
-			c_backtrace(regs->ARM_fp, processor_mode(regs));
+			c_backtrace(frame_pointer(regs), processor_mode(regs));
 		}
 	}
 #endif
-- 
1.8.1.5

[PATCH 7/7] ARM: oprofile: Use of arm_get_current_stackframe

[Nikolay Borisov]

Use the newly introduced API so that FP is correctly referenced from
either R7/R11 based on whether we are running in THUMB2 mode or not.

Signed-off-by: Nikolay Borisov <Nikolay.Borisov@arm.com>
---
 arch/arm/oprofile/common.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 99c63d4b..e6a3c4c 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -107,10 +107,7 @@ static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
 
 	if (!user_mode(regs)) {
 		struct stackframe frame;
-		frame.fp = regs->ARM_fp;
-		frame.sp = regs->ARM_sp;
-		frame.lr = regs->ARM_lr;
-		frame.pc = regs->ARM_pc;
+		arm_get_current_stackframe(regs, &frame);
 		walk_stackframe(&frame, report_trace, &depth);
 		return;
 	}
-- 
1.8.1.5

[PATCH 2/7] ARM: Introduce arm_get_current_stack_frame()

[Robert Richter]

On 13.05.14 10:46:53, Nikolay Borisov wrote:
>  #define instruction_pointer(regs)	(regs)->ARM_pc
> +#define frame_pointer(regs)				\
> +	IS_ENABLED(CONFIG_THUMB2_KERNEL) ? regs->ARM_r7 \
> +	: regs->ARM_fp					\

(regs)->... should be used here both.

-Robert

[PATCH 2/7] ARM: Introduce arm_get_current_stack_frame()

[Nikolay Borisov]

> -----Original Message-----
> From: Robert Richter [mailto:rric.net at gmail.com] On Behalf Of Robert
> Richter
> Sent: 13 May 2014 15:55
> To: Nikolay Borisov
> Cc: linux-arm-kernel at lists.infradead.org; Dave P Martin; Will Deacon;
> anurag19aggarwal at gmail.com; a.p.zijlstra at chello.nl;
> sebastian.hesselbarth at gmail.com
> Subject: Re: [PATCH 2/7] ARM: Introduce arm_get_current_stack_frame()
> 
> On 13.05.14 10:46:53, Nikolay Borisov wrote:
> >  #define instruction_pointer(regs)	(regs)->ARM_pc
> > +#define frame_pointer(regs)				\
> > +	IS_ENABLED(CONFIG_THUMB2_KERNEL) ? regs->ARM_r7 \
> > +	: regs->ARM_fp					\
> 
> (regs)->... should be used here both.
> 

Yeah, I fixed it already in my local version. Thanks. 
> -Robert