All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] MIPS: IRQ Stack: Unwind IRQ stack onto task stack
@ 2017-03-21 14:52 ` Matt Redfearn
  0 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-03-21 14:52 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Paolo Bonzini, Marcin Nowakowski,
	Masanari Iida, Chris Metcalf, linux-kernel, James Hogan,
	Paul Burton, Ingo Molnar, Jason A. Donenfeld, Andrew Morton

When the separate IRQ stack was introduced, stack unwinding only
proceeded as far as the top of the IRQ stack, leading to kernel
backtraces being less useful, lacking the trace of what was interrupted.

Fix this by providing a means for the kernel to unwind the IRQ stack
onto the interrupted task stack. The processor state is saved to the
kernel task stack on interrupt. The IRQ_STACK_START macro reserves an
unsigned long at the top of the IRQ stack where the interrupted task
stack pointer can be saved. After the active stack is switched to the
IRQ stack, save the interrupted tasks stack pointer to the reserved
location.

Fix the stack unwinding code to look for the frame being the top of the
IRQ stack and if so get the next frame from the saved location. The
existing test does not work with the separate stack since the ra is no
longer pointed at ret_from_{irq,exception}.

The test to stop unwinding the stack 32 bytes from the top of a stack
must be modified to allow unwinding to continue up to the location of
the saved task stack pointer when on the IRQ stack. The low / high marks
of the stack are set depending on whether the sp is on an irq stack or
not.

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

 arch/mips/include/asm/irq.h    | 15 +++++++++++
 arch/mips/kernel/asm-offsets.c |  1 +
 arch/mips/kernel/genex.S       |  8 ++++--
 arch/mips/kernel/process.c     | 56 ++++++++++++++++++++++++++++--------------
 4 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
index 956db6e201d1..ddd1c918103b 100644
--- a/arch/mips/include/asm/irq.h
+++ b/arch/mips/include/asm/irq.h
@@ -18,9 +18,24 @@
 #include <irq.h>
 
 #define IRQ_STACK_SIZE			THREAD_SIZE
+#define IRQ_STACK_START			(IRQ_STACK_SIZE - sizeof(unsigned long))
 
 extern void *irq_stack[NR_CPUS];
 
+/*
+ * The highest address on the IRQ stack contains a dummy frame put down in
+ * genex.S (handle_int & except_vec_vi_handler) which is structured as follows:
+ *
+ *   top ------------
+ *       | task sp  | <- irq_stack[cpu] + IRQ_STACK_START
+ *       ------------
+ *       |          | <- First frame of IRQ context
+ *       ------------
+ *
+ * task sp holds a copy of the task stack pointer where the struct pt_regs
+ * from exception entry can be found.
+ */
+
 static inline bool on_irq_stack(int cpu, unsigned long sp)
 {
 	unsigned long low = (unsigned long)irq_stack[cpu];
diff --git a/arch/mips/kernel/asm-offsets.c b/arch/mips/kernel/asm-offsets.c
index bb5c5d34ba81..a670c0c11875 100644
--- a/arch/mips/kernel/asm-offsets.c
+++ b/arch/mips/kernel/asm-offsets.c
@@ -102,6 +102,7 @@ void output_thread_info_defines(void)
 	DEFINE(_THREAD_SIZE, THREAD_SIZE);
 	DEFINE(_THREAD_MASK, THREAD_MASK);
 	DEFINE(_IRQ_STACK_SIZE, IRQ_STACK_SIZE);
+	DEFINE(_IRQ_STACK_START, IRQ_STACK_START);
 	BLANK();
 }
 
diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
index 7ec9612cb007..68466dd4bab1 100644
--- a/arch/mips/kernel/genex.S
+++ b/arch/mips/kernel/genex.S
@@ -215,9 +215,11 @@ NESTED(handle_int, PT_SIZE, sp)
 	beq	t0, t1, 2f
 
 	/* Switch to IRQ stack */
-	li	t1, _IRQ_STACK_SIZE
+	li	t1, _IRQ_STACK_START
 	PTR_ADD sp, t0, t1
 
+	/* Save task's sp on IRQ stack so that unwinding can follow it */
+	LONG_S	s1, 0(sp)
 2:
 	jal	plat_irq_dispatch
 
@@ -325,9 +327,11 @@ NESTED(except_vec_vi_handler, 0, sp)
 	beq	t0, t1, 2f
 
 	/* Switch to IRQ stack */
-	li	t1, _IRQ_STACK_SIZE
+	li	t1, _IRQ_STACK_START
 	PTR_ADD sp, t0, t1
 
+	/* Save task's sp on IRQ stack so that unwinding can follow it */
+	LONG_S	s1, 0(sp)
 2:
 	jalr	v0
 
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index fb6b6b650719..b68e10fc453d 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -488,31 +488,52 @@ unsigned long notrace unwind_stack_by_address(unsigned long stack_page,
 					      unsigned long pc,
 					      unsigned long *ra)
 {
+	unsigned long low, high, irq_stack_high;
 	struct mips_frame_info info;
 	unsigned long size, ofs;
+	struct pt_regs *regs;
 	int leaf;
-	extern void ret_from_irq(void);
-	extern void ret_from_exception(void);
 
 	if (!stack_page)
 		return 0;
 
 	/*
-	 * If we reached the bottom of interrupt context,
-	 * return saved pc in pt_regs.
+	 * IRQ stacks start at IRQ_STACK_START
+	 * task stacks at THREAD_SIZE - 32
 	 */
-	if (pc == (unsigned long)ret_from_irq ||
-	    pc == (unsigned long)ret_from_exception) {
-		struct pt_regs *regs;
-		if (*sp >= stack_page &&
-		    *sp + sizeof(*regs) <= stack_page + THREAD_SIZE - 32) {
-			regs = (struct pt_regs *)*sp;
-			pc = regs->cp0_epc;
-			if (!user_mode(regs) && __kernel_text_address(pc)) {
-				*sp = regs->regs[29];
-				*ra = regs->regs[31];
-				return pc;
-			}
+	low = stack_page;
+	if (!preemptible() && on_irq_stack(raw_smp_processor_id(), *sp)) {
+		high = stack_page + IRQ_STACK_START;
+		irq_stack_high = high;
+	} else {
+		high = stack_page + THREAD_SIZE - 32;
+		irq_stack_high = 0;
+	}
+
+	/*
+	 * If we reached the top of the interrupt stack, start unwinding
+	 * the interrupted task stack.
+	 */
+	if (unlikely(*sp == irq_stack_high)) {
+		unsigned long task_sp = *(unsigned long *)*sp;
+
+		/*
+		 * Check that the pointer saved in the IRQ stack head points to
+		 * something within the stack of the current task
+		 */
+		if (!object_is_on_stack((void *)task_sp))
+			return 0;
+
+		/*
+		 * Follow pointer to tasks kernel stack frame where interrupted
+		 * state was saved.
+		 */
+		regs = (struct pt_regs *)task_sp;
+		pc = regs->cp0_epc;
+		if (!user_mode(regs) && __kernel_text_address(pc)) {
+			*sp = regs->regs[29];
+			*ra = regs->regs[31];
+			return pc;
 		}
 		return 0;
 	}
@@ -533,8 +554,7 @@ unsigned long notrace unwind_stack_by_address(unsigned long stack_page,
 	if (leaf < 0)
 		return 0;
 
-	if (*sp < stack_page ||
-	    *sp + info.frame_size > stack_page + THREAD_SIZE - 32)
+	if (*sp < low || *sp + info.frame_size > high)
 		return 0;
 
 	if (leaf)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] MIPS: IRQ Stack: Unwind IRQ stack onto task stack
@ 2017-03-21 14:52 ` Matt Redfearn
  0 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-03-21 14:52 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: linux-mips, Matt Redfearn, Paolo Bonzini, Marcin Nowakowski,
	Masanari Iida, Chris Metcalf, linux-kernel, James Hogan,
	Paul Burton, Ingo Molnar, Jason A. Donenfeld, Andrew Morton

When the separate IRQ stack was introduced, stack unwinding only
proceeded as far as the top of the IRQ stack, leading to kernel
backtraces being less useful, lacking the trace of what was interrupted.

Fix this by providing a means for the kernel to unwind the IRQ stack
onto the interrupted task stack. The processor state is saved to the
kernel task stack on interrupt. The IRQ_STACK_START macro reserves an
unsigned long at the top of the IRQ stack where the interrupted task
stack pointer can be saved. After the active stack is switched to the
IRQ stack, save the interrupted tasks stack pointer to the reserved
location.

Fix the stack unwinding code to look for the frame being the top of the
IRQ stack and if so get the next frame from the saved location. The
existing test does not work with the separate stack since the ra is no
longer pointed at ret_from_{irq,exception}.

The test to stop unwinding the stack 32 bytes from the top of a stack
must be modified to allow unwinding to continue up to the location of
the saved task stack pointer when on the IRQ stack. The low / high marks
of the stack are set depending on whether the sp is on an irq stack or
not.

Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

 arch/mips/include/asm/irq.h    | 15 +++++++++++
 arch/mips/kernel/asm-offsets.c |  1 +
 arch/mips/kernel/genex.S       |  8 ++++--
 arch/mips/kernel/process.c     | 56 ++++++++++++++++++++++++++++--------------
 4 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
index 956db6e201d1..ddd1c918103b 100644
--- a/arch/mips/include/asm/irq.h
+++ b/arch/mips/include/asm/irq.h
@@ -18,9 +18,24 @@
 #include <irq.h>
 
 #define IRQ_STACK_SIZE			THREAD_SIZE
+#define IRQ_STACK_START			(IRQ_STACK_SIZE - sizeof(unsigned long))
 
 extern void *irq_stack[NR_CPUS];
 
+/*
+ * The highest address on the IRQ stack contains a dummy frame put down in
+ * genex.S (handle_int & except_vec_vi_handler) which is structured as follows:
+ *
+ *   top ------------
+ *       | task sp  | <- irq_stack[cpu] + IRQ_STACK_START
+ *       ------------
+ *       |          | <- First frame of IRQ context
+ *       ------------
+ *
+ * task sp holds a copy of the task stack pointer where the struct pt_regs
+ * from exception entry can be found.
+ */
+
 static inline bool on_irq_stack(int cpu, unsigned long sp)
 {
 	unsigned long low = (unsigned long)irq_stack[cpu];
diff --git a/arch/mips/kernel/asm-offsets.c b/arch/mips/kernel/asm-offsets.c
index bb5c5d34ba81..a670c0c11875 100644
--- a/arch/mips/kernel/asm-offsets.c
+++ b/arch/mips/kernel/asm-offsets.c
@@ -102,6 +102,7 @@ void output_thread_info_defines(void)
 	DEFINE(_THREAD_SIZE, THREAD_SIZE);
 	DEFINE(_THREAD_MASK, THREAD_MASK);
 	DEFINE(_IRQ_STACK_SIZE, IRQ_STACK_SIZE);
+	DEFINE(_IRQ_STACK_START, IRQ_STACK_START);
 	BLANK();
 }
 
diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
index 7ec9612cb007..68466dd4bab1 100644
--- a/arch/mips/kernel/genex.S
+++ b/arch/mips/kernel/genex.S
@@ -215,9 +215,11 @@ NESTED(handle_int, PT_SIZE, sp)
 	beq	t0, t1, 2f
 
 	/* Switch to IRQ stack */
-	li	t1, _IRQ_STACK_SIZE
+	li	t1, _IRQ_STACK_START
 	PTR_ADD sp, t0, t1
 
+	/* Save task's sp on IRQ stack so that unwinding can follow it */
+	LONG_S	s1, 0(sp)
 2:
 	jal	plat_irq_dispatch
 
@@ -325,9 +327,11 @@ NESTED(except_vec_vi_handler, 0, sp)
 	beq	t0, t1, 2f
 
 	/* Switch to IRQ stack */
-	li	t1, _IRQ_STACK_SIZE
+	li	t1, _IRQ_STACK_START
 	PTR_ADD sp, t0, t1
 
+	/* Save task's sp on IRQ stack so that unwinding can follow it */
+	LONG_S	s1, 0(sp)
 2:
 	jalr	v0
 
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index fb6b6b650719..b68e10fc453d 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -488,31 +488,52 @@ unsigned long notrace unwind_stack_by_address(unsigned long stack_page,
 					      unsigned long pc,
 					      unsigned long *ra)
 {
+	unsigned long low, high, irq_stack_high;
 	struct mips_frame_info info;
 	unsigned long size, ofs;
+	struct pt_regs *regs;
 	int leaf;
-	extern void ret_from_irq(void);
-	extern void ret_from_exception(void);
 
 	if (!stack_page)
 		return 0;
 
 	/*
-	 * If we reached the bottom of interrupt context,
-	 * return saved pc in pt_regs.
+	 * IRQ stacks start at IRQ_STACK_START
+	 * task stacks at THREAD_SIZE - 32
 	 */
-	if (pc == (unsigned long)ret_from_irq ||
-	    pc == (unsigned long)ret_from_exception) {
-		struct pt_regs *regs;
-		if (*sp >= stack_page &&
-		    *sp + sizeof(*regs) <= stack_page + THREAD_SIZE - 32) {
-			regs = (struct pt_regs *)*sp;
-			pc = regs->cp0_epc;
-			if (!user_mode(regs) && __kernel_text_address(pc)) {
-				*sp = regs->regs[29];
-				*ra = regs->regs[31];
-				return pc;
-			}
+	low = stack_page;
+	if (!preemptible() && on_irq_stack(raw_smp_processor_id(), *sp)) {
+		high = stack_page + IRQ_STACK_START;
+		irq_stack_high = high;
+	} else {
+		high = stack_page + THREAD_SIZE - 32;
+		irq_stack_high = 0;
+	}
+
+	/*
+	 * If we reached the top of the interrupt stack, start unwinding
+	 * the interrupted task stack.
+	 */
+	if (unlikely(*sp == irq_stack_high)) {
+		unsigned long task_sp = *(unsigned long *)*sp;
+
+		/*
+		 * Check that the pointer saved in the IRQ stack head points to
+		 * something within the stack of the current task
+		 */
+		if (!object_is_on_stack((void *)task_sp))
+			return 0;
+
+		/*
+		 * Follow pointer to tasks kernel stack frame where interrupted
+		 * state was saved.
+		 */
+		regs = (struct pt_regs *)task_sp;
+		pc = regs->cp0_epc;
+		if (!user_mode(regs) && __kernel_text_address(pc)) {
+			*sp = regs->regs[29];
+			*ra = regs->regs[31];
+			return pc;
 		}
 		return 0;
 	}
@@ -533,8 +554,7 @@ unsigned long notrace unwind_stack_by_address(unsigned long stack_page,
 	if (leaf < 0)
 		return 0;
 
-	if (*sp < stack_page ||
-	    *sp + info.frame_size > stack_page + THREAD_SIZE - 32)
+	if (*sp < low || *sp + info.frame_size > high)
 		return 0;
 
 	if (leaf)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] MIPS: IRQ Stack: Unwind IRQ stack onto task stack
  2017-03-21 14:52 ` Matt Redfearn
  (?)
@ 2017-04-04 11:58 ` Jason A. Donenfeld
  2017-04-04 12:18     ` Matt Redfearn
  2017-04-04 12:36   ` Ralf Baechle
  -1 siblings, 2 replies; 6+ messages in thread
From: Jason A. Donenfeld @ 2017-04-04 11:58 UTC (permalink / raw)
  To: Matt Redfearn
  Cc: Ralf Baechle, linux-mips, Paolo Bonzini, Marcin Nowakowski,
	Masanari Iida, Chris Metcalf, LKML, James Hogan, Paul Burton,
	Ingo Molnar, Andrew Morton

This indeed is useful. Out of curiosity, are other archs using a
similar technique? In anycase,

Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] MIPS: IRQ Stack: Unwind IRQ stack onto task stack
@ 2017-04-04 12:18     ` Matt Redfearn
  0 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-04-04 12:18 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Ralf Baechle, linux-mips, Paolo Bonzini, Marcin Nowakowski,
	Masanari Iida, Chris Metcalf, LKML, James Hogan, Paul Burton,
	Ingo Molnar, Andrew Morton

Hi Jason,


On 04/04/17 12:58, Jason A. Donenfeld wrote:
> This indeed is useful. Out of curiosity, are other archs using a
> similar technique? In anycase,
>
> Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Yes, at least x86 and ARM64 do the same thing (probably more)

x86 saves the previous stack pointer on the IRQ stack in 
http://lxr.free-electrons.com/source/arch/x86/kernel/irq_32.c#L70
which is then unwound in 
http://lxr.free-electrons.com/source/arch/x86/kernel/dumpstack.c#L51

ARM64 saves the task SP in 
http://lxr.free-electrons.com/source/arch/arm64/kernel/entry.S#L249
And unwinds it in 
http://lxr.free-electrons.com/source/arch/arm64/kernel/traps.c#L140

Thanks,
Matt

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] MIPS: IRQ Stack: Unwind IRQ stack onto task stack
@ 2017-04-04 12:18     ` Matt Redfearn
  0 siblings, 0 replies; 6+ messages in thread
From: Matt Redfearn @ 2017-04-04 12:18 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Ralf Baechle, linux-mips, Paolo Bonzini, Marcin Nowakowski,
	Masanari Iida, Chris Metcalf, LKML, James Hogan, Paul Burton,
	Ingo Molnar, Andrew Morton

Hi Jason,


On 04/04/17 12:58, Jason A. Donenfeld wrote:
> This indeed is useful. Out of curiosity, are other archs using a
> similar technique? In anycase,
>
> Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Yes, at least x86 and ARM64 do the same thing (probably more)

x86 saves the previous stack pointer on the IRQ stack in 
http://lxr.free-electrons.com/source/arch/x86/kernel/irq_32.c#L70
which is then unwound in 
http://lxr.free-electrons.com/source/arch/x86/kernel/dumpstack.c#L51

ARM64 saves the task SP in 
http://lxr.free-electrons.com/source/arch/arm64/kernel/entry.S#L249
And unwinds it in 
http://lxr.free-electrons.com/source/arch/arm64/kernel/traps.c#L140

Thanks,
Matt

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] MIPS: IRQ Stack: Unwind IRQ stack onto task stack
  2017-04-04 11:58 ` Jason A. Donenfeld
  2017-04-04 12:18     ` Matt Redfearn
@ 2017-04-04 12:36   ` Ralf Baechle
  1 sibling, 0 replies; 6+ messages in thread
From: Ralf Baechle @ 2017-04-04 12:36 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Matt Redfearn, linux-mips, Paolo Bonzini, Marcin Nowakowski,
	Masanari Iida, Chris Metcalf, LKML, James Hogan, Paul Burton,
	Ingo Molnar, Andrew Morton

On Tue, Apr 04, 2017 at 01:58:04PM +0200, Jason A. Donenfeld wrote:

> This indeed is useful. Out of curiosity, are other archs using a
> similar technique? In anycase,
> 
> Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Unfortunately MIPS doesn't have anything like a frame pointer or similar
to make backtracing easy so proper stacktraces have always been more of
rocket science than they really ought to ...

  Ralf

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-04-04 12:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-21 14:52 [PATCH] MIPS: IRQ Stack: Unwind IRQ stack onto task stack Matt Redfearn
2017-03-21 14:52 ` Matt Redfearn
2017-04-04 11:58 ` Jason A. Donenfeld
2017-04-04 12:18   ` Matt Redfearn
2017-04-04 12:18     ` Matt Redfearn
2017-04-04 12:36   ` Ralf Baechle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.