All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jungseok Lee <jungseoklee85@gmail.com>
To: catalin.marinas@arm.com, will.deacon@arm.com,
	linux-arm-kernel@lists.infradead.org
Cc: james.morse@arm.com, takahiro.akashi@linaro.org,
	mark.rutland@arm.com, barami97@gmail.com,
	linux-kernel@vger.kernel.org
Subject: [PATCH v4 2/2] arm64: Expand the stack trace feature to support IRQ stack
Date: Wed,  7 Oct 2015 15:28:12 +0000	[thread overview]
Message-ID: <1444231692-32722-3-git-send-email-jungseoklee85@gmail.com> (raw)
In-Reply-To: <1444231692-32722-1-git-send-email-jungseoklee85@gmail.com>

Currently, a call trace drops a process stack walk when a separate IRQ
stack is used. It makes a call trace information much less useful when
a system gets paniked in interrupt context.

This patch addresses the issue with the following schemes:

  - Store aborted stack frame data
  - Decide whether another stack walk is needed or not via current sp
  - Loosen the frame pointer upper bound condition

Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: James Morse <james.morse@arm.com>
Signed-off-by: Jungseok Lee <jungseoklee85@gmail.com>
---
 arch/arm64/include/asm/irq.h    | 12 +++++++++++
 arch/arm64/kernel/asm-offsets.c |  3 +++
 arch/arm64/kernel/entry.S       | 10 ++++++++--
 arch/arm64/kernel/stacktrace.c  | 22 ++++++++++++++++++++-
 arch/arm64/kernel/traps.c       | 13 ++++++++++++
 5 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index 6ea82e8..e5904a1 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -2,13 +2,25 @@
 #define __ASM_IRQ_H
 
 #include <linux/irqchip/arm-gic-acpi.h>
+#include <asm/stacktrace.h>
 
 #include <asm-generic/irq.h>
 
 struct irq_stack {
 	void *stack;
+	struct stackframe frame;
 };
 
+DECLARE_PER_CPU(struct irq_stack, irq_stacks);
+
+static inline bool in_irq_stack(unsigned int cpu)
+{
+	unsigned long high = (unsigned long)per_cpu(irq_stacks, cpu).stack;
+
+	return (current_stack_pointer >= round_down(high, THREAD_SIZE)) &&
+		current_stack_pointer < high;
+}
+
 struct pt_regs;
 
 extern void migrate_irqs(void);
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index b16e3cf..fbb52f2d 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -42,6 +42,9 @@ int main(void)
   DEFINE(THREAD_CPU_CONTEXT,	offsetof(struct task_struct, thread.cpu_context));
   BLANK();
   DEFINE(IRQ_STACK,		offsetof(struct irq_stack, stack));
+  DEFINE(IRQ_FRAME_FP,		offsetof(struct irq_stack, frame.fp));
+  DEFINE(IRQ_FRAME_SP,		offsetof(struct irq_stack, frame.sp));
+  DEFINE(IRQ_FRAME_PC,		offsetof(struct irq_stack, frame.pc));
   BLANK();
   DEFINE(S_X0,			offsetof(struct pt_regs, regs[0]));
   DEFINE(S_X1,			offsetof(struct pt_regs, regs[1]));
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 6d4e8c5..650cc05 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -121,7 +121,8 @@
 	 * x21 - aborted SP
 	 * x22 - aborted PC
 	 * x23 - aborted PSTATE
-	*/
+	 * x29 - aborted FP
+	 */
 	.endm
 
 	.macro	kernel_exit, el
@@ -184,7 +185,12 @@ alternative_endif
 	mov	x23, sp
 	and	x23, x23, #~(THREAD_SIZE - 1)
 	cmp	x20, x23			// check irq re-enterance
-	mov	x19, sp
+	beq	1f
+	str	x29, [x19, #IRQ_FRAME_FP]
+	str	x21, [x19, #IRQ_FRAME_SP]
+	str	x22, [x19, #IRQ_FRAME_PC]
+	mov	x29, x24
+1:	mov	x19, sp
 	csel	x23, x19, x24, eq		// x24 = top of irq stack
 	mov	sp, x23
 	.endm
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 407991b..5124649 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -43,7 +43,27 @@ int notrace unwind_frame(struct stackframe *frame)
 	low  = frame->sp;
 	high = ALIGN(low, THREAD_SIZE);
 
-	if (fp < low || fp > high - 0x18 || fp & 0xf)
+	/*
+	 * A frame pointer would reach an upper bound if a prologue of the
+	 * first function of call trace looks as follows:
+	 *
+	 *	stp     x29, x30, [sp,#-16]!
+	 *	mov     x29, sp
+	 *
+	 * Thus, the upper bound is (top of stack - 0x20) with consideration
+	 * of a 16-byte empty space in THREAD_START_SP.
+	 *
+	 * The value, 0x20, however, does not cover all cases as interrupts
+	 * are handled using a separate stack. That is, a call trace can start
+	 * from elx_irq exception vectors. The symbols could not be promoted
+	 * to candidates for a stack trace under the restriction, 0x20.
+	 *
+	 * The scenario is handled without complexity as 1) considering
+	 * (bottom of stack + THREAD_START_SP) as a dummy frame pointer, the
+	 * content of which is 0, and 2) allowing the case, which changes
+	 * the value to 0x10 from 0x20.
+	 */
+	if (fp < low || fp > high - 0x10 || fp & 0xf)
 		return -EINVAL;
 
 	frame->sp = fp + 0x10;
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index f93aae5..44b2f828 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -146,6 +146,8 @@ static void dump_instr(const char *lvl, struct pt_regs *regs)
 static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 {
 	struct stackframe frame;
+	unsigned int cpu = smp_processor_id();
+	bool in_irq = in_irq_stack(cpu);
 
 	pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
 
@@ -170,6 +172,10 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 	}
 
 	pr_emerg("Call trace:\n");
+repeat:
+	if (in_irq)
+		pr_emerg("<IRQ>\n");
+
 	while (1) {
 		unsigned long where = frame.pc;
 		int ret;
@@ -179,6 +185,13 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 			break;
 		dump_backtrace_entry(where, frame.sp);
 	}
+
+	if (in_irq) {
+		frame = per_cpu(irq_stacks, cpu).frame;
+		in_irq = false;
+		pr_emerg("<EOI>\n");
+		goto repeat;
+	}
 }
 
 void show_stack(struct task_struct *tsk, unsigned long *sp)
-- 
2.5.0


WARNING: multiple messages have this Message-ID (diff)
From: jungseoklee85@gmail.com (Jungseok Lee)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 2/2] arm64: Expand the stack trace feature to support IRQ stack
Date: Wed,  7 Oct 2015 15:28:12 +0000	[thread overview]
Message-ID: <1444231692-32722-3-git-send-email-jungseoklee85@gmail.com> (raw)
In-Reply-To: <1444231692-32722-1-git-send-email-jungseoklee85@gmail.com>

Currently, a call trace drops a process stack walk when a separate IRQ
stack is used. It makes a call trace information much less useful when
a system gets paniked in interrupt context.

This patch addresses the issue with the following schemes:

  - Store aborted stack frame data
  - Decide whether another stack walk is needed or not via current sp
  - Loosen the frame pointer upper bound condition

Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Cc: James Morse <james.morse@arm.com>
Signed-off-by: Jungseok Lee <jungseoklee85@gmail.com>
---
 arch/arm64/include/asm/irq.h    | 12 +++++++++++
 arch/arm64/kernel/asm-offsets.c |  3 +++
 arch/arm64/kernel/entry.S       | 10 ++++++++--
 arch/arm64/kernel/stacktrace.c  | 22 ++++++++++++++++++++-
 arch/arm64/kernel/traps.c       | 13 ++++++++++++
 5 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index 6ea82e8..e5904a1 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -2,13 +2,25 @@
 #define __ASM_IRQ_H
 
 #include <linux/irqchip/arm-gic-acpi.h>
+#include <asm/stacktrace.h>
 
 #include <asm-generic/irq.h>
 
 struct irq_stack {
 	void *stack;
+	struct stackframe frame;
 };
 
+DECLARE_PER_CPU(struct irq_stack, irq_stacks);
+
+static inline bool in_irq_stack(unsigned int cpu)
+{
+	unsigned long high = (unsigned long)per_cpu(irq_stacks, cpu).stack;
+
+	return (current_stack_pointer >= round_down(high, THREAD_SIZE)) &&
+		current_stack_pointer < high;
+}
+
 struct pt_regs;
 
 extern void migrate_irqs(void);
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index b16e3cf..fbb52f2d 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -42,6 +42,9 @@ int main(void)
   DEFINE(THREAD_CPU_CONTEXT,	offsetof(struct task_struct, thread.cpu_context));
   BLANK();
   DEFINE(IRQ_STACK,		offsetof(struct irq_stack, stack));
+  DEFINE(IRQ_FRAME_FP,		offsetof(struct irq_stack, frame.fp));
+  DEFINE(IRQ_FRAME_SP,		offsetof(struct irq_stack, frame.sp));
+  DEFINE(IRQ_FRAME_PC,		offsetof(struct irq_stack, frame.pc));
   BLANK();
   DEFINE(S_X0,			offsetof(struct pt_regs, regs[0]));
   DEFINE(S_X1,			offsetof(struct pt_regs, regs[1]));
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 6d4e8c5..650cc05 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -121,7 +121,8 @@
 	 * x21 - aborted SP
 	 * x22 - aborted PC
 	 * x23 - aborted PSTATE
-	*/
+	 * x29 - aborted FP
+	 */
 	.endm
 
 	.macro	kernel_exit, el
@@ -184,7 +185,12 @@ alternative_endif
 	mov	x23, sp
 	and	x23, x23, #~(THREAD_SIZE - 1)
 	cmp	x20, x23			// check irq re-enterance
-	mov	x19, sp
+	beq	1f
+	str	x29, [x19, #IRQ_FRAME_FP]
+	str	x21, [x19, #IRQ_FRAME_SP]
+	str	x22, [x19, #IRQ_FRAME_PC]
+	mov	x29, x24
+1:	mov	x19, sp
 	csel	x23, x19, x24, eq		// x24 = top of irq stack
 	mov	sp, x23
 	.endm
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 407991b..5124649 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -43,7 +43,27 @@ int notrace unwind_frame(struct stackframe *frame)
 	low  = frame->sp;
 	high = ALIGN(low, THREAD_SIZE);
 
-	if (fp < low || fp > high - 0x18 || fp & 0xf)
+	/*
+	 * A frame pointer would reach an upper bound if a prologue of the
+	 * first function of call trace looks as follows:
+	 *
+	 *	stp     x29, x30, [sp,#-16]!
+	 *	mov     x29, sp
+	 *
+	 * Thus, the upper bound is (top of stack - 0x20) with consideration
+	 * of a 16-byte empty space in THREAD_START_SP.
+	 *
+	 * The value, 0x20, however, does not cover all cases as interrupts
+	 * are handled using a separate stack. That is, a call trace can start
+	 * from elx_irq exception vectors. The symbols could not be promoted
+	 * to candidates for a stack trace under the restriction, 0x20.
+	 *
+	 * The scenario is handled without complexity as 1) considering
+	 * (bottom of stack + THREAD_START_SP) as a dummy frame pointer, the
+	 * content of which is 0, and 2) allowing the case, which changes
+	 * the value to 0x10 from 0x20.
+	 */
+	if (fp < low || fp > high - 0x10 || fp & 0xf)
 		return -EINVAL;
 
 	frame->sp = fp + 0x10;
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index f93aae5..44b2f828 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -146,6 +146,8 @@ static void dump_instr(const char *lvl, struct pt_regs *regs)
 static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 {
 	struct stackframe frame;
+	unsigned int cpu = smp_processor_id();
+	bool in_irq = in_irq_stack(cpu);
 
 	pr_debug("%s(regs = %p tsk = %p)\n", __func__, regs, tsk);
 
@@ -170,6 +172,10 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 	}
 
 	pr_emerg("Call trace:\n");
+repeat:
+	if (in_irq)
+		pr_emerg("<IRQ>\n");
+
 	while (1) {
 		unsigned long where = frame.pc;
 		int ret;
@@ -179,6 +185,13 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)
 			break;
 		dump_backtrace_entry(where, frame.sp);
 	}
+
+	if (in_irq) {
+		frame = per_cpu(irq_stacks, cpu).frame;
+		in_irq = false;
+		pr_emerg("<EOI>\n");
+		goto repeat;
+	}
 }
 
 void show_stack(struct task_struct *tsk, unsigned long *sp)
-- 
2.5.0

  parent reply	other threads:[~2015-10-07 15:28 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-07 15:28 [PATCH v4 0/2] arm64: Introduce IRQ stack Jungseok Lee
2015-10-07 15:28 ` Jungseok Lee
2015-10-07 15:28 ` [PATCH v4 1/2] " Jungseok Lee
2015-10-07 15:28   ` Jungseok Lee
2015-10-08 10:25   ` Pratyush Anand
2015-10-08 10:25     ` Pratyush Anand
2015-10-08 14:32     ` Jungseok Lee
2015-10-08 14:32       ` Jungseok Lee
2015-10-08 16:51       ` Pratyush Anand
2015-10-08 16:51         ` Pratyush Anand
2015-10-07 15:28 ` Jungseok Lee [this message]
2015-10-07 15:28   ` [PATCH v4 2/2] arm64: Expand the stack trace feature to support " Jungseok Lee
2015-10-09 14:24   ` James Morse
2015-10-09 14:24     ` James Morse
2015-10-12 14:53     ` Jungseok Lee
2015-10-12 14:53       ` Jungseok Lee
2015-10-12 16:34       ` James Morse
2015-10-12 16:34         ` James Morse
2015-10-12 22:13         ` Jungseok Lee
2015-10-12 22:13           ` Jungseok Lee
2015-10-13 11:00           ` James Morse
2015-10-13 11:00             ` James Morse
2015-10-13 15:00             ` Jungseok Lee
2015-10-13 15:00               ` Jungseok Lee
2015-10-14 12:12               ` Jungseok Lee
2015-10-14 12:12                 ` Jungseok Lee
2015-10-15 15:59                 ` James Morse
2015-10-15 15:59                   ` James Morse
2015-10-16 13:01                   ` Jungseok Lee
2015-10-16 13:01                     ` Jungseok Lee
2015-10-16 16:06                     ` Catalin Marinas
2015-10-16 16:06                       ` Catalin Marinas
2015-10-17 13:38                       ` Jungseok Lee
2015-10-17 13:38                         ` Jungseok Lee
2015-10-19 16:18                         ` Catalin Marinas
2015-10-19 16:18                           ` Catalin Marinas
2015-10-20 13:08                           ` Jungseok Lee
2015-10-20 13:08                             ` Jungseok Lee
2015-10-21 15:14                             ` Jungseok Lee
2015-10-21 15:14                               ` Jungseok Lee
2015-10-14  7:13     ` AKASHI Takahiro
2015-10-14  7:13       ` AKASHI Takahiro
2015-10-14 12:24       ` Jungseok Lee
2015-10-14 12:24         ` Jungseok Lee
2015-10-14 12:55         ` Jungseok Lee
2015-10-14 12:55           ` Jungseok Lee
2015-10-15  4:19           ` AKASHI Takahiro
2015-10-15  4:19             ` AKASHI Takahiro
2015-10-15 13:39             ` Jungseok Lee
2015-10-15 13:39               ` Jungseok Lee
2015-10-19  6:47               ` AKASHI Takahiro
2015-10-19  6:47                 ` AKASHI Takahiro
2015-10-20 13:19                 ` Jungseok Lee
2015-10-20 13:19                   ` Jungseok Lee
2015-10-15 14:24     ` Jungseok Lee
2015-10-15 14:24       ` Jungseok Lee
2015-10-15 16:01       ` James Morse
2015-10-15 16:01         ` James Morse
2015-10-16 13:02         ` Jungseok Lee
2015-10-16 13:02           ` Jungseok Lee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1444231692-32722-3-git-send-email-jungseoklee85@gmail.com \
    --to=jungseoklee85@gmail.com \
    --cc=barami97@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=takahiro.akashi@linaro.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.