Skip to contentSkip to conversation

ARM: add support for IRQ stacks

PATCH Superseded Ard Biesheuvel 2 participants 16 messages Oct 17, 2021 – Oct 18, 2021 Arm (32-bit)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover ARM: add support for IRQ stacks

Compared to user space, the kernel's task stacks are tiny and inflexible, as we don't grow them dynamically using demand paging. This is the reason we tend to obsess about functions with disproportionately large stack frames, given that it is hard to predict statically how calls to those functions may combine at runtime, and exhaust the stack and crash the kernel.

This becomes even less predictable when taking interrupt handling into account, as their handlers are normally executed on the stack of the task that was interrupted, regardless of how deep its call stack was at the time of the interruption. To decouple these, and reduce the risk of hitting a pathological worst case where IRQ handling and the task below it happen to exhaust the available stack space when combined, we can switch to a different stack when taking interrupts, similar to how this is done already on some other architectures. This series implements this approach for ARM.

Note that a good chunk of the changes below are related to supporting non-contiguous call stacks, which is also relevant in the context of vmap'ed stacks, which use a separate overflow stack to handle stack overflows. The changes preserve all functionality related to walking the call stack and dumping exception stacks and register contents.

Changes since v2:
- improve Clang support, by emitting code that is compatible with its frame pointer unwinder if that is the unwinder being used;
- add acks from Arnd and Linus (thanks!)

Changes since v1:
- drop the first bugfix patch, which has been queued as a fix in the meantime;
- preserve/restore FP in the irq_handler entry code;
- add missing include to arch/arm/kernel/irq.c to silence warnings about missing prototypes.

Patch #1 removes some code that I spotted that is no longer used.

Patch #2 introduces a pair of macros that will be used later in the series to emit the optimal indirect call sequence for older and newer cores.

Patch #3 tweaks the IRQ asm entry point to generate better code for v7 CPUs.

Patch #4 updates the unwind info based unwinder so it can deal with call stacks that are non-contiguous.

Patch #5 exports dump_mem() to other compilation units so the ARM unwinder can cal it directly. This is needed by the next patch.

Patch #6 refactors the ARM unwinder to dump the exception stack from a context where it can figure out if it lives on the current stack or on the task stack.

Patch #7 fixes an issue in the Clang frame pointer unwinder, which may get into an endless recursive fault if any of the stack frames have a bogus value for the link register.

Patch #8 implements the actual IRQ stacks support, by allocating one for each CPU, and adding the code to switch to it in the IRQ entry path. It also contains some related changes to allow the frame pointer based unwinder to deal with the new situation.

Patch #9 modifies call_with_stack() so both the frame pointer unwinder as well as the ARM unwinder know how to deal with it.

Patch #10 adds the IRQ stack switching for softIRQ handling initiated from task context.

The patches are based on my arm32-ti-in-task-v5 branch [0], which is a prerequisite for these changes, given that we can no longer rely on thread_info being accessible by masking the stack pointer when we are jumping between stacks. A pull request is outstanding for those changes.

[0] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm32-ti-in-task-v5

Cc Russell King <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Kees Cook <[email protected]>
Cc Keith Packard <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Nick Desaulniers <[email protected]>
Cc Nicolas Pitre <[email protected]>

Ard Biesheuvel (10):
ARM: remove some dead code
ARM: assembler: introduce bl_r and bl_m macros
ARM: optimize indirect call to handle_arch_irq for v7 cores
ARM: unwind: support unwinding across multiple stacks
ARM: export dump_mem() to other objects
ARM: unwind: dump exception stack from calling frame
ARM: backtrace-clang: avoid crash on bogus frame pointer
ARM: implement IRQ stacks
ARM: call_with_stack: add unwind support
ARM: run softirqs on the per-CPU IRQ stack

 arch/arm/Kconfig                         |  6 ++
 arch/arm/include/asm/assembler.h         | 41 +++++++++++++
 arch/arm/include/asm/entry-macro-multi.S | 24 --------
 arch/arm/include/asm/smp.h               |  5 --
 arch/arm/include/asm/stacktrace.h        | 13 +++++
 arch/arm/kernel/entry-armv.S             | 60 +++++++++++++++++---
 arch/arm/kernel/irq.c                    | 37 ++++++++++++
 arch/arm/kernel/smp.c                    |  5 --
 arch/arm/kernel/traps.c                  | 25 ++++++--
 arch/arm/kernel/unwind.c                 | 33 +++++++----
 arch/arm/lib/backtrace-clang.S           | 14 ++++-
 arch/arm/lib/backtrace.S                 |  8 +++
 arch/arm/lib/call_with_stack.S           | 33 ++++++++---
 13 files changed, 236 insertions(+), 68 deletions(-)
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#1 lore raw

PATCH v3 1/10 ARM: remove some dead code

This code appears to be no longer used so let's get rid of it.

---
 arch/arm/include/asm/entry-macro-multi.S | 24 --------------------
 arch/arm/include/asm/smp.h               |  5 ----
 arch/arm/kernel/smp.c                    |  5 ----
 3 files changed, 34 deletions(-)
arch/arm/include/asm/entry-macro-multi.S +0 -24
index dfc6bfa43012..24486dad9e19 100644
@@ -13,28 +13,4 @@
 	@
 	badrne	lr, 1b
 	bne	asm_do_IRQ
-
-#ifdef CONFIG_SMP
-	/*
-	 * XXX
-	 *
-	 * this macro assumes that irqstat (r2) and base (r6) are
-	 * preserved from get_irqnr_and_base above
-	 */
-	ALT_SMP(test_for_ipi r0, r2, r6, lr)
-	ALT_UP_B(9997f)
-	movne	r1, sp
-	badrne	lr, 1b
-	bne	do_IPI
-#endif
-9997:
-	.endm
-
-	.macro	arch_irq_handler, symbol_name
-	.align	5
-	.global \symbol_name
-\symbol_name:
-	mov	r8, lr
-	arch_irq_handler_default
-	ret	r8
 	.endm
arch/arm/include/asm/smp.h +0 -5
index f16cbbd5cda4..7c1c90d9f582 100644
@@ -24,11 +24,6 @@ struct seq_file;
  */
 extern void show_ipi_list(struct seq_file *, int);
 
-/*
- * Called from assembly code, this handles an IPI.
- */
-asmlinkage void do_IPI(int ipinr, struct pt_regs *regs);
-
 /*
  * Called from C code, this handles an IPI.
  */
arch/arm/kernel/smp.c +0 -5
index cde5b6d8bac5..9c55ca915ba4 100644
@@ -631,11 +631,6 @@ static void ipi_complete(unsigned int cpu)
 /*
  * Main handler for inter-processor interrupts
  */
-asmlinkage void __exception_irq_entry do_IPI(int ipinr, struct pt_regs *regs)
-{
-	handle_IPI(ipinr, regs);
-}
-
 static void do_handle_IPI(int ipinr)
 {
 	unsigned int cpu = smp_processor_id();
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#2 lore raw

PATCH v3 2/10 ARM: assembler: introduce bl_r and bl_m macros

In reply to Ard Biesheuvel

Add some macros that abstract the difference between the ways indirect calls are performed on older and newer ARM architecture revisions.

The main difference is to prefer blx instructions over explicit LR assignments when possible, as these tend to confuse the prediction logic in out-of-order cores when speculating across a function return.

---
 arch/arm/include/asm/assembler.h | 37 ++++++++++++++++++++
 1 file changed, 37 insertions(+)
arch/arm/include/asm/assembler.h +37 -0
index 7d23d4bb2168..abb8202ef0da 100644
@@ -624,4 +624,41 @@ THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 	.endif
 	.endm
 
+	/*
+	 * bl_r - branch and link to register
+	 *
+	 * @dst: target to branch to
+	 * @c: conditional opcode suffix
+	 */
+	.macro		bl_r, dst:req, c
+	.if		__LINUX_ARM_ARCH__ < 6
+	mov\c		lr, pc
+	mov\c		pc, \dst
+	.else
+	blx\c		\dst
+	.endif
+	.endm
+
+	/*
+	 * bl_m - branch and link to address held in memory
+	 *
+	 * @dst: memory reference to load the branch target from
+	 *
+	 * This uses the same syntax as ldr instructions, including things like
+	 * register writeback (where it makes sense). For example,
+	 *
+	 *    bl_m    [ip, #<offset>]
+	 *
+	 * Note that @dst should not reference lr as it may be clobbered early.
+	 */
+	.macro		bl_m, dst:vararg
+	.if		__LINUX_ARM_ARCH__ < 6
+	mov		lr, pc
+	ldr		pc, \dst
+	.else
+	ldr		lr, \dst
+	blx		lr
+	.endif
+	.endm
+
 #endif /* __ASM_ASSEMBLER_H__ */
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#3 lore raw

PATCH v3 3/10 ARM: optimize indirect call to handle_arch_irq for v7 cores

In reply to Ard Biesheuvel

The current irq_handler macro uses a literal load followed by an explicit assignment of lr before an indirect call, both of which are sub-optimal on recent ARM cores. Replace it by a mov_l call, which will evaluate to a movw/movt pair on v7 cores, avoiding one of the two loads, followed by a call to a newly introduced 'bl_m' macro, which will evaluate to a blx instruction on cores that support it. (Setting lr explicitly rather than via a bl or blx instruction confuses the return address prediction that modern out-of-order cores use to speculatively perform the function return.)

---
 arch/arm/kernel/entry-armv.S | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
arch/arm/kernel/entry-armv.S +2 -4
index a54b5044d406..21896f702447 100644
@@ -38,14 +38,12 @@
  */
 	.macro	irq_handler
 #ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
-	ldr	r1, =handle_arch_irq
+	mov_l	r1, handle_arch_irq
 	mov	r0, sp
-	badr	lr, 9997f
-	ldr	pc, [r1]
+	bl_m	[r1]
 #else
 	arch_irq_handler_default
 #endif
-9997:
 	.endm
 
 	.macro	pabt_helper
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#4 lore raw

PATCH v3 4/10 ARM: unwind: support unwinding across multiple stacks

In reply to Ard Biesheuvel

Implement support in the unwinder for dealing with multiple stacks. This will be needed once we add support for IRQ stacks, or for the overflow stack used by the vmap'ed stacks code.

This involves tracking the unwind opcodes that either update the virtual stack pointer from another virtual register, or perform an explicit subtract on the virtual stack pointer, and updating the low and high bounds that we use to sanitize the stack pointer accordingly.

---
 arch/arm/kernel/unwind.c | 25 +++++++++++++-------
 1 file changed, 16 insertions(+), 9 deletions(-)
arch/arm/kernel/unwind.c +16 -9
index 59fdf257bf8b..9cb9af3fc433 100644
@@ -52,6 +52,7 @@ EXPORT_SYMBOL(__aeabi_unwind_cpp_pr2);
 struct unwind_ctrl_block {
 	unsigned long vrs[16];		/* virtual register set */
 	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 */
 	/*
 	 * 1 : check for stack overflow for each register pop.
@@ -256,8 +257,12 @@ static int unwind_exec_pop_subset_r4_to_r13(struct unwind_ctrl_block *ctrl,
 		mask >>= 1;
 		reg++;
 	}
-	if (!load_sp)
+	if (!load_sp) {
 		ctrl->vrs[SP] = (unsigned long)vsp;
+	} else {
+		ctrl->sp_low = ctrl->vrs[SP];
+		ctrl->sp_high = ALIGN(ctrl->sp_low, THREAD_SIZE);
+	}
 
 	return URC_OK;
 }
@@ -313,9 +318,10 @@ static int unwind_exec_insn(struct unwind_ctrl_block *ctrl)
 
 	if ((insn & 0xc0) == 0x00)
 		ctrl->vrs[SP] += ((insn & 0x3f) << 2) + 4;
-	else if ((insn & 0xc0) == 0x40)
+	else if ((insn & 0xc0) == 0x40) {
 		ctrl->vrs[SP] -= ((insn & 0x3f) << 2) + 4;
-	else if ((insn & 0xf0) == 0x80) {
+		ctrl->sp_low = ctrl->vrs[SP];
+	} else if ((insn & 0xf0) == 0x80) {
 		unsigned long mask;
 
 		insn = (insn << 8) | unwind_get_byte(ctrl);
@@ -330,9 +336,11 @@ static int unwind_exec_insn(struct unwind_ctrl_block *ctrl)
 		if (ret)
 			goto error;
 	} else if ((insn & 0xf0) == 0x90 &&
-		   (insn & 0x0d) != 0x0d)
+		   (insn & 0x0d) != 0x0d) {
 		ctrl->vrs[SP] = ctrl->vrs[insn & 0x0f];
-	else if ((insn & 0xf0) == 0xa0) {
+		ctrl->sp_low = ctrl->vrs[SP];
+		ctrl->sp_high = ALIGN(ctrl->sp_low, THREAD_SIZE);
+	} else if ((insn & 0xf0) == 0xa0) {
 		ret = unwind_exec_pop_r4_to_rN(ctrl, insn);
 		if (ret)
 			goto error;
@@ -375,13 +383,12 @@ static int unwind_exec_insn(struct unwind_ctrl_block *ctrl)
  */
 int unwind_frame(struct stackframe *frame)
 {
-	unsigned long low;
 	const struct unwind_idx *idx;
 	struct unwind_ctrl_block ctrl;
 
 	/* store the highest address on the stack to avoid crossing it*/
-	low = frame->sp;
-	ctrl.sp_high = ALIGN(low, THREAD_SIZE);
+	ctrl.sp_low = frame->sp;
+	ctrl.sp_high = ALIGN(ctrl.sp_low, THREAD_SIZE);
 
 	pr_debug("%s(pc = %08lx lr = %08lx sp = %08lx)\n", __func__,
 		 frame->pc, frame->lr, frame->sp);
@@ -437,7 +444,7 @@ int unwind_frame(struct stackframe *frame)
 		urc = unwind_exec_insn(&ctrl);
 		if (urc < 0)
 			return urc;
-		if (ctrl.vrs[SP] < low || ctrl.vrs[SP] >= ctrl.sp_high)
+		if (ctrl.vrs[SP] < ctrl.sp_low || ctrl.vrs[SP] > ctrl.sp_high)
 			return -URC_FAILURE;
 	}
 
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#5 lore raw

PATCH v3 5/10 ARM: export dump_mem() to other objects

In reply to Ard Biesheuvel

The unwind info based stack unwinder will make its own call to dump_mem() to dump the exception stack, so give it external linkage.

---
 arch/arm/include/asm/stacktrace.h | 2 ++
 arch/arm/kernel/traps.c           | 7 +++----
 2 files changed, 5 insertions(+), 4 deletions(-)
arch/arm/include/asm/stacktrace.h +2 -0
index 2d76a2e29f05..e86f55155500 100644
@@ -27,5 +27,7 @@ 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);
+extern void dump_mem(const char *lvl, const char *str, unsigned long bottom,
+		     unsigned long top);
 
 #endif	/* __ASM_STACKTRACE_H */
arch/arm/kernel/traps.c +3 -4
index 4a7edc6e848f..b1ea2e4870ce 100644
@@ -35,6 +35,7 @@
 #include <asm/ptrace.h>
 #include <asm/unwind.h>
 #include <asm/tls.h>
+#include <asm/stacktrace.h>
 #include <asm/system_misc.h>
 #include <asm/opcodes.h>
 
@@ -60,8 +61,6 @@ static int __init user_debug_setup(char *str)
 __setup("user_debug=", user_debug_setup);
 #endif
 
-static void dump_mem(const char *, const char *, unsigned long, unsigned long);
-
 void dump_backtrace_entry(unsigned long where, unsigned long from,
 			  unsigned long frame, const char *loglvl)
 {
@@ -118,8 +117,8 @@ static int verify_stack(unsigned long sp)
 /*
  * Dump out the contents of some memory nicely...
  */
-static void dump_mem(const char *lvl, const char *str, unsigned long bottom,
-		     unsigned long top)
+void dump_mem(const char *lvl, const char *str, unsigned long bottom,
+	      unsigned long top)
 {
 	unsigned long first;
 	int i;
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#6 lore raw

PATCH v3 6/10 ARM: unwind: dump exception stack from calling frame

In reply to Ard Biesheuvel

The existing code that dumps the contents of the pt_regs structure passed to __entry routines does so while unwinding the callee frame, and dereferences the stack pointer as a struct pt_regs*. This will no longer work when we enable support for IRQ or overflow stacks, because the struct pt_regs may live on the task stack, while we are executing from another stack.

The unwinder has access to this information, but only while unwinding the calling frame. So let's combine the exception stack dumping code with the handling of the calling frame as well. By printing it before dumping the caller/callee addresses, the output order is preserved.

---
 arch/arm/include/asm/stacktrace.h | 11 +++++++++++
 arch/arm/kernel/traps.c           |  3 ++-
 arch/arm/kernel/unwind.c          |  8 +++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)
arch/arm/include/asm/stacktrace.h +11 -0
index e86f55155500..8dcad8156561 100644
@@ -13,6 +13,17 @@ struct stackframe {
 	unsigned long sp;
 	unsigned long lr;
 	unsigned long pc;
+
+#ifdef CONFIG_ARM_UNWIND
+	/*
+	 * This field is used to track the stack pointer value when calling
+	 * __entry routines. This is needed when IRQ stacks and overflow stacks
+	 * are used, because in that case, the struct pt_regs passed to these
+	 * __entry routines may be at the top of the task stack, while we are
+	 * executing from another stack.
+	 */
+	unsigned long sp_low;
+#endif
 };
 
 static __always_inline
arch/arm/kernel/traps.c +2 -1
index b1ea2e4870ce..cda32385552b 100644
@@ -74,7 +74,8 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
 		loglvl, where, from);
 #endif
 
-	if (in_entry_text(from) && end <= ALIGN(frame, THREAD_SIZE))
+	if (!IS_ENABLED(CONFIG_UNWINDER_ARM) &&
+	    in_entry_text(from) && end <= ALIGN(frame, THREAD_SIZE))
 		dump_mem(loglvl, "Exception stack", frame + 4, end);
 }
 
arch/arm/kernel/unwind.c +7 -1
index 9cb9af3fc433..b7a6141c342f 100644
@@ -29,6 +29,7 @@
 #include <linux/spinlock.h>
 #include <linux/list.h>
 
+#include <asm/sections.h>
 #include <asm/stacktrace.h>
 #include <asm/traps.h>
 #include <asm/unwind.h>
@@ -459,6 +460,7 @@ int unwind_frame(struct stackframe *frame)
 	frame->sp = ctrl.vrs[SP];
 	frame->lr = ctrl.vrs[LR];
 	frame->pc = ctrl.vrs[PC];
+	frame->sp_low = ctrl.sp_low;
 
 	return URC_OK;
 }
@@ -502,7 +504,11 @@ void unwind_backtrace(struct pt_regs *regs, struct task_struct *tsk,
 		urc = unwind_frame(&frame);
 		if (urc < 0)
 			break;
-		dump_backtrace_entry(where, frame.pc, frame.sp - 4, loglvl);
+		if (in_entry_text(where))
+			dump_mem(loglvl, "Exception stack", frame.sp_low,
+				 frame.sp_low + sizeof(struct pt_regs));
+
+		dump_backtrace_entry(where, frame.pc, 0, loglvl);
 	}
 }
 
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#7 lore raw

PATCH v3 7/10 ARM: backtrace-clang: avoid crash on bogus frame pointer

In reply to Ard Biesheuvel

The Clang backtrace code dereferences the link register value pulled from the stack to decide whether the caller was a branch-and-link instruction, in order to subsequently decode the offset to find the start of the calling function. Unlike other loads in this routine, this one is not protected by a fixup, and may therefore cause a crash if the address in question is bogus.

So let's fix this, by treating the fault as a failure to decode the 'bl' instruction. To avoid a label renum, reuse a fixup label that guards an instruction that cannot fault to begin with.

---
 arch/arm/lib/backtrace-clang.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/lib/backtrace-clang.S +3 -3
index 5b2cdb1003e3..5b4bca85d06d 100644
@@ -144,7 +144,7 @@ for_each_frame:	tst	frame, mask		@ Check for address exceptions
  */
 1003:		ldr	sv_lr, [sv_fp, #4]	@ get saved lr from next frame
 
-		ldr	r0, [sv_lr, #-4]	@ get call instruction
+1004:		ldr	r0, [sv_lr, #-4]	@ get call instruction
 		ldr	r3, .Lopcode+4
 		and	r2, r3, r0		@ is this a bl call
 		teq	r2, r3
@@ -164,7 +164,7 @@ finished_setup:
 /*
  * Print the function (sv_pc) and where it was called from (sv_lr).
  */
-1004:		mov	r0, sv_pc
+		mov	r0, sv_pc
 
 		mov	r1, sv_lr
 		mov	r2, frame
@@ -210,7 +210,7 @@ ENDPROC(c_backtrace)
 		.long	1001b, 1006b
 		.long	1002b, 1006b
 		.long	1003b, 1006b
-		.long	1004b, 1006b
+		.long	1004b, finished_setup
 		.long   1005b, 1006b
 		.popsection
 
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Reviewed-by on v3 7/10
#8 lore raw

Re: [PATCH v3 07/10] ARM: backtrace-clang: avoid crash on bogus frame pointer

Quoted 11 lines · Ard Biesheuvel

The Clang backtrace code dereferences the link register value pulled from the stack to decide whether the caller was a branch-and-link instruction, in order to subsequently decode the offset to find the start of the calling function. Unlike other loads in this routine, this one is not protected by a fixup, and may therefore cause a crash if the address in question is bogus.

So let's fix this, by treating the fault as a failure to decode the 'bl' instruction. To avoid a label renum, reuse a fixup label that guards an instruction that cannot fault to begin with.

Thanks for fixing this. My understanding of fixups was non-existent when I initially reviewed this code. (Taking the time to write
http://nickdesaulniers.github.io/blog/2020/04/06/off-by-two/ helped a lot).

Quoted 40 lines
---
 arch/arm/lib/backtrace-clang.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm/lib/backtrace-clang.S +3 -4
index 5b2cdb1003e3..5b4bca85d06d 100644
@@ -144,7 +144,7 @@ for_each_frame:     tst     frame, mask             @ Check for address exceptions
  */
 1003:          ldr     sv_lr, [sv_fp, #4]      @ get saved lr from next frame
 
-               ldr     r0, [sv_lr, #-4]        @ get call instruction
+1004:          ldr     r0, [sv_lr, #-4]        @ get call instruction
                ldr     r3, .Lopcode+4
                and     r2, r3, r0              @ is this a bl call
                teq     r2, r3
@@ -164,7 +164,7 @@ finished_setup:
 /*
  * Print the function (sv_pc) and where it was called from (sv_lr).
  */
-1004:          mov     r0, sv_pc
+               mov     r0, sv_pc
 
                mov     r1, sv_lr
                mov     r2, frame
@@ -210,7 +210,7 @@ ENDPROC(c_backtrace)
                .long   1001b, 1006b
                .long   1002b, 1006b
                .long   1003b, 1006b
-               .long   1004b, 1006b
+               .long   1004b, finished_setup
                .long   1005b, 1006b
                .popsection
 
--

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
#9 lore raw

PATCH v3 8/10 ARM: implement IRQ stacks

In reply to Ard Biesheuvel

Now that we no longer rely on the stack pointer to access the current task struct or thread info, we can implement support for IRQ stacks cleanly as well.

Define a per-CPU IRQ stack and switch to this stack when taking an IRQ, provided that we were not already using that stack in the interrupted context. This is never the case for IRQs taken from user space, but ones taken while running in the kernel could fire while one taken from user space has not completed yet.

---
 arch/arm/Kconfig                 |  4 ++
 arch/arm/include/asm/assembler.h |  4 ++
 arch/arm/kernel/entry-armv.S     | 54 ++++++++++++++++++--
 arch/arm/kernel/irq.c            | 23 +++++++++
 arch/arm/kernel/traps.c          | 15 +++++-
 arch/arm/lib/backtrace-clang.S   |  8 +++
 arch/arm/lib/backtrace.S         |  8 +++
 7 files changed, 112 insertions(+), 4 deletions(-)
arch/arm/Kconfig +4 -0
index 4f61c9789e7f..a8c1db0736f3 100644
@@ -1163,6 +1163,10 @@ config CURRENT_POINTER_IN_TPIDRURO
 	def_bool y
 	depends on SMP && CPU_32v6K && !CPU_V6
 
+config IRQSTACKS
+	def_bool y
+	depends on GENERIC_IRQ_MULTI_HANDLER && THREAD_INFO_IN_TASK
+
 config ARM_CPU_TOPOLOGY
 	bool "Support cpu topology definition"
 	depends on SMP && CPU_V7
arch/arm/include/asm/assembler.h +4 -0
index abb8202ef0da..405950494208 100644
@@ -86,6 +86,10 @@
 
 #define IMM12_MASK 0xfff
 
+/* the frame pointer used for stack unwinding */
+ARM(	fpreg	.req	r11	)
+THUMB(	fpreg	.req	r7	)
+
 /*
  * Enable and disable interrupts
  */
arch/arm/kernel/entry-armv.S +51 -3

76 lines not on this page · raw message

arch/arm/kernel/irq.c +23 -0
index 20ab1e607522..58af2adb1583 100644
@@ -43,6 +43,25 @@
 
 unsigned long irq_err_count;
 
+#ifdef CONFIG_IRQSTACKS
+
+asmlinkage DEFINE_PER_CPU_READ_MOSTLY(u8 *, irq_stack_ptr);
+
+static void __init init_irq_stacks(void)
+{
+	u8 *stack;
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		stack = (u8 *)__get_free_pages(GFP_KERNEL, THREAD_SIZE_ORDER);
+		if (WARN_ON(!stack))
+			break;
+		per_cpu(irq_stack_ptr, cpu) = &stack[THREAD_SIZE];
+	}
+}
+
+#endif
+
 int arch_show_interrupts(struct seq_file *p, int prec)
 {
 #ifdef CONFIG_FIQ
@@ -99,6 +118,10 @@ void __init init_IRQ(void)
 {
 	int ret;
 
+#ifdef CONFIG_IRQSTACKS
+	init_irq_stacks();
+#endif
+
 	if (IS_ENABLED(CONFIG_OF) && !machine_desc->init_irq)
 		irqchip_init();
 	else
arch/arm/kernel/traps.c +14 -1
index cda32385552b..1ba9fc2eb564 100644
@@ -66,6 +66,19 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
 {
 	unsigned long end = frame + 4 + sizeof(struct pt_regs);
 
+	if (IS_ENABLED(CONFIG_UNWINDER_FRAME_POINTER) &&
+	    IS_ENABLED(CONFIG_CC_IS_GCC) &&
+	    end > ALIGN(frame, THREAD_SIZE)) {
+		/*
+		 * If we are walking past the end of the stack, it may be due
+		 * to the fact that we are on an IRQ or overflow stack. In this
+		 * case, we can load the address of the other stack from the
+		 * frame record.
+		 */
+		frame = ((unsigned long *)frame)[-2] - 4;
+		end = frame + 4 + sizeof(struct pt_regs);
+	}
+
 #ifdef CONFIG_KALLSYMS
 	printk("%s[<%08lx>] (%ps) from [<%08lx>] (%pS)\n",
 		loglvl, where, (void *)where, from, (void *)from);
@@ -278,7 +291,7 @@ static int __die(const char *str, int err, struct pt_regs *regs)
 
 	if (!user_mode(regs) || in_interrupt()) {
 		dump_mem(KERN_EMERG, "Stack: ", regs->ARM_sp,
-			 THREAD_SIZE + (unsigned long)task_stack_page(tsk));
+			 ALIGN(regs->ARM_sp, THREAD_SIZE));
 		dump_backtrace(regs, tsk, KERN_EMERG);
 		dump_instr(KERN_EMERG, regs);
 	}
arch/arm/lib/backtrace-clang.S +8 -0
index 5b4bca85d06d..290c52a60fc6 100644
@@ -197,6 +197,14 @@ finished_setup:
 
 		cmp	sv_fp, frame		@ next frame must be
 		mov	frame, sv_fp		@ above the current frame
+#ifdef CONFIG_IRQSTACKS
+		@
+		@ Kernel stacks may be discontiguous in memory. If the next
+		@ frame is below the previous frame, accept it as long as it
+		@ lives in kernel memory.
+		@
+		cmpls	sv_fp, #PAGE_OFFSET
+#endif
 		bhi	for_each_frame
 
 1006:		adr	r0, .Lbad
arch/arm/lib/backtrace.S +8 -0
index e8408f22d4dc..293a2716bd20 100644
@@ -98,6 +98,14 @@ for_each_frame:	tst	frame, mask		@ Check for address exceptions
 
 		cmp	sv_fp, frame		@ next frame must be
 		mov	frame, sv_fp		@ above the current frame
+#ifdef CONFIG_IRQSTACKS
+		@
+		@ Kernel stacks may be discontiguous in memory. If the next
+		@ frame is below the previous frame, accept it as long as it
+		@ lives in kernel memory.
+		@
+		cmpls	sv_fp, #PAGE_OFFSET
+#endif
 		bhi	for_each_frame
 
 1006:		adr	r0, .Lbad
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#10 lore raw

Re: [PATCH v3 08/10] ARM: implement IRQ stacks

Quoted 183 lines · Ard Biesheuvel

Now that we no longer rely on the stack pointer to access the current task struct or thread info, we can implement support for IRQ stacks cleanly as well.

Define a per-CPU IRQ stack and switch to this stack when taking an IRQ, provided that we were not already using that stack in the interrupted context. This is never the case for IRQs taken from user space, but ones taken while running in the kernel could fire while one taken from user space has not completed yet.

---
 arch/arm/Kconfig                 |  4 ++
 arch/arm/include/asm/assembler.h |  4 ++
 arch/arm/kernel/entry-armv.S     | 54 ++++++++++++++++++--
 arch/arm/kernel/irq.c            | 23 +++++++++
 arch/arm/kernel/traps.c          | 15 +++++-
 arch/arm/lib/backtrace-clang.S   |  8 +++
 arch/arm/lib/backtrace.S         |  8 +++
 7 files changed, 112 insertions(+), 4 deletions(-)
arch/arm/Kconfig +4 -0
index 4f61c9789e7f..a8c1db0736f3 100644
@@ -1163,6 +1163,10 @@ config CURRENT_POINTER_IN_TPIDRURO
        def_bool y
        depends on SMP && CPU_32v6K && !CPU_V6
 
+config IRQSTACKS
+       def_bool y
+       depends on GENERIC_IRQ_MULTI_HANDLER && THREAD_INFO_IN_TASK
+
 config ARM_CPU_TOPOLOGY
        bool "Support cpu topology definition"
        depends on SMP && CPU_V7
arch/arm/include/asm/assembler.h +4 -0
index abb8202ef0da..405950494208 100644
@@ -86,6 +86,10 @@
 
 #define IMM12_MASK 0xfff
 
+/* the frame pointer used for stack unwinding */
+ARM(   fpreg   .req    r11     )
+THUMB( fpreg   .req    r7      )
+
 /*
  * Enable and disable interrupts
  */
arch/arm/kernel/entry-armv.S +51 -3
index 21896f702447..2a426bd27a69 100644
@@ -36,11 +36,59 @@
 /*
  * Interrupt handling.
  */
-       .macro  irq_handler
+       .macro  irq_handler, from_user:req
 #ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
        mov_l   r1, handle_arch_irq
        mov     r0, sp
+#ifdef CONFIG_IRQSTACKS
+       mov_l   r2, irq_stack_ptr       @ Take base address
+       mrc     p15, 0, r3, c13, c0, 4  @ Get CPU offset
+#ifdef CONFIG_UNWINDER_ARM
+       mov     fpreg, sp               @ Preserve original SP
+#else
+       mov     r8, fp                  @ Preserve original FP
+       mov     r9, sp                  @ Preserve original SP
+#endif
+       ldr     sp, [r2, r3]            @ Load SP from per-CPU var
+       .if     \from_user == 0
+UNWIND(        .setfp  fpreg, sp               )
+       @
+       @ If we took the interrupt while running in the kernel, we may already
+       @ be using the IRQ stack, so revert to the original value in that case.
+       @
+       subs    r2, sp, r0              @ SP above bottom of IRQ stack?
+       rsbscs  r2, r2, #THREAD_SIZE    @ ... and below the top?
+       movcs   sp, r0                  @ If so, revert to incoming SP
+
+#ifndef CONFIG_UNWINDER_ARM
+       @
+       @ Inform the frame pointer unwinder where the next frame lives
+       @
+       movcc   lr, pc                  @ Make LR point into .entry.text so
+                                       @ that we will get a dump of the
+                                       @ exception stack for this frame.
+#ifdef CONFIG_CC_IS_GCC
+       movcc   ip, r0                  @ Store the old SP in the frame record.
+       stmdbcc sp!, {fp, ip, lr, pc}   @ Push frame record
+       addcc   fp, sp, #12
+#else
+       stmdbcc sp!, {fp, lr}           @ Push frame record
+       movcc   fp, sp
+#endif // CONFIG_CC_IS_GCC
+#endif // CONFIG_UNWINDER_ARM
+       .endif
+#endif // CONFIG_IRQSTACKS
+
        bl_m    [r1]
+
+#ifdef CONFIG_IRQSTACKS
+#ifdef CONFIG_UNWINDER_ARM
+       mov     sp, fpreg               @ Restore original SP
+#else
+       mov     fp, r8                  @ Restore original FP
+       mov     sp, r9                  @ Restore original SP
+#endif // CONFIG_UNWINDER_ARM
+#endif // CONFIG_IRQSTACKS
 #else
        arch_irq_handler_default
 #endif
@@ -200,7 +248,7 @@ ENDPROC(__dabt_svc)
        .align  5
 __irq_svc:
        svc_entry
-       irq_handler
+       irq_handler from_user=0
 
 #ifdef CONFIG_PREEMPTION
        ldr     r8, [tsk, #TI_PREEMPT]          @ get preempt count
@@ -427,7 +475,7 @@ ENDPROC(__dabt_usr)
 __irq_usr:
        usr_entry
        kuser_cmpxchg_check
-       irq_handler
+       irq_handler from_user=1
        get_thread_info tsk
        mov     why, #0
        b       ret_to_user_from_irq
arch/arm/kernel/irq.c +23 -0
index 20ab1e607522..58af2adb1583 100644
@@ -43,6 +43,25 @@
 
 unsigned long irq_err_count;
 
+#ifdef CONFIG_IRQSTACKS
+
+asmlinkage DEFINE_PER_CPU_READ_MOSTLY(u8 *, irq_stack_ptr);
+
+static void __init init_irq_stacks(void)
+{
+       u8 *stack;
+       int cpu;
+
+       for_each_possible_cpu(cpu) {
+               stack = (u8 *)__get_free_pages(GFP_KERNEL, THREAD_SIZE_ORDER);
+               if (WARN_ON(!stack))
+                       break;
+               per_cpu(irq_stack_ptr, cpu) = &stack[THREAD_SIZE];
+       }
+}
+
+#endif
+
 int arch_show_interrupts(struct seq_file *p, int prec)
 {
 #ifdef CONFIG_FIQ
@@ -99,6 +118,10 @@ void __init init_IRQ(void)
 {
        int ret;
 
+#ifdef CONFIG_IRQSTACKS
+       init_irq_stacks();
+#endif
+
        if (IS_ENABLED(CONFIG_OF) && !machine_desc->init_irq)
                irqchip_init();
        else
arch/arm/kernel/traps.c +0 -0
index cda32385552b..1ba9fc2eb564 100644

arch/arm/kernel/traps.c void dump_backtrace_entry(unsigned long where, unsigned long from, · line 70

Quoted 5 lines
arch/arm/kernel/traps.c +2 -0
@@ -66,6 +66,19 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
 {
        unsigned long end = frame + 4 + sizeof(struct pt_regs);
 
+       if (IS_ENABLED(CONFIG_UNWINDER_FRAME_POINTER) &&
+           IS_ENABLED(CONFIG_CC_IS_GCC) &&

Anything that needs to be adjusted here in the case of CONFIG_CC_IS_CLANG?

Quoted 64 lines
arch/arm/kernel/traps.c +12 -1
+           end > ALIGN(frame, THREAD_SIZE)) {
+               /*
+                * If we are walking past the end of the stack, it may be due
+                * to the fact that we are on an IRQ or overflow stack. In this
+                * case, we can load the address of the other stack from the
+                * frame record.
+                */
+               frame = ((unsigned long *)frame)[-2] - 4;
+               end = frame + 4 + sizeof(struct pt_regs);
+       }
+
 #ifdef CONFIG_KALLSYMS
        printk("%s[<%08lx>] (%ps) from [<%08lx>] (%pS)\n",
                loglvl, where, (void *)where, from, (void *)from);
@@ -278,7 +291,7 @@ static int __die(const char *str, int err, struct pt_regs *regs)
 
        if (!user_mode(regs) || in_interrupt()) {
                dump_mem(KERN_EMERG, "Stack: ", regs->ARM_sp,
-                        THREAD_SIZE + (unsigned long)task_stack_page(tsk));
+                        ALIGN(regs->ARM_sp, THREAD_SIZE));
                dump_backtrace(regs, tsk, KERN_EMERG);
                dump_instr(KERN_EMERG, regs);
        }
arch/arm/lib/backtrace-clang.S +8 -0
index 5b4bca85d06d..290c52a60fc6 100644
@@ -197,6 +197,14 @@ finished_setup:
 
                cmp     sv_fp, frame            @ next frame must be
                mov     frame, sv_fp            @ above the current frame
+#ifdef CONFIG_IRQSTACKS
+               @
+               @ Kernel stacks may be discontiguous in memory. If the next
+               @ frame is below the previous frame, accept it as long as it
+               @ lives in kernel memory.
+               @
+               cmpls   sv_fp, #PAGE_OFFSET
+#endif
                bhi     for_each_frame
 
 1006:          adr     r0, .Lbad
arch/arm/lib/backtrace.S +8 -1
index e8408f22d4dc..293a2716bd20 100644
@@ -98,6 +98,14 @@ for_each_frame:      tst     frame, mask             @ Check for address exceptions
 
                cmp     sv_fp, frame            @ next frame must be
                mov     frame, sv_fp            @ above the current frame
+#ifdef CONFIG_IRQSTACKS
+               @
+               @ Kernel stacks may be discontiguous in memory. If the next
+               @ frame is below the previous frame, accept it as long as it
+               @ lives in kernel memory.
+               @
+               cmpls   sv_fp, #PAGE_OFFSET
+#endif
                bhi     for_each_frame
 
 1006:          adr     r0, .Lbad
--

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
#11 lore raw

Re: [PATCH v3 08/10] ARM: implement IRQ stacks

Quoted 188 lines · Nick Desaulniers
Quoted 186 lines · Ard Biesheuvel

Now that we no longer rely on the stack pointer to access the current task struct or thread info, we can implement support for IRQ stacks cleanly as well.

Define a per-CPU IRQ stack and switch to this stack when taking an IRQ, provided that we were not already using that stack in the interrupted context. This is never the case for IRQs taken from user space, but ones taken while running in the kernel could fire while one taken from user space has not completed yet.

---
 arch/arm/Kconfig                 |  4 ++
 arch/arm/include/asm/assembler.h |  4 ++
 arch/arm/kernel/entry-armv.S     | 54 ++++++++++++++++++--
 arch/arm/kernel/irq.c            | 23 +++++++++
 arch/arm/kernel/traps.c          | 15 +++++-
 arch/arm/lib/backtrace-clang.S   |  8 +++
 arch/arm/lib/backtrace.S         |  8 +++
 7 files changed, 112 insertions(+), 4 deletions(-)
arch/arm/Kconfig +4 -0
index 4f61c9789e7f..a8c1db0736f3 100644
@@ -1163,6 +1163,10 @@ config CURRENT_POINTER_IN_TPIDRURO
        def_bool y
        depends on SMP && CPU_32v6K && !CPU_V6
 
+config IRQSTACKS
+       def_bool y
+       depends on GENERIC_IRQ_MULTI_HANDLER && THREAD_INFO_IN_TASK
+
 config ARM_CPU_TOPOLOGY
        bool "Support cpu topology definition"
        depends on SMP && CPU_V7
arch/arm/include/asm/assembler.h +4 -0
index abb8202ef0da..405950494208 100644
@@ -86,6 +86,10 @@
 
 #define IMM12_MASK 0xfff
 
+/* the frame pointer used for stack unwinding */
+ARM(   fpreg   .req    r11     )
+THUMB( fpreg   .req    r7      )
+
 /*
  * Enable and disable interrupts
  */
arch/arm/kernel/entry-armv.S +51 -3
index 21896f702447..2a426bd27a69 100644
@@ -36,11 +36,59 @@
 /*
  * Interrupt handling.
  */
-       .macro  irq_handler
+       .macro  irq_handler, from_user:req
 #ifdef CONFIG_GENERIC_IRQ_MULTI_HANDLER
        mov_l   r1, handle_arch_irq
        mov     r0, sp
+#ifdef CONFIG_IRQSTACKS
+       mov_l   r2, irq_stack_ptr       @ Take base address
+       mrc     p15, 0, r3, c13, c0, 4  @ Get CPU offset
+#ifdef CONFIG_UNWINDER_ARM
+       mov     fpreg, sp               @ Preserve original SP
+#else
+       mov     r8, fp                  @ Preserve original FP
+       mov     r9, sp                  @ Preserve original SP
+#endif
+       ldr     sp, [r2, r3]            @ Load SP from per-CPU var
+       .if     \from_user == 0
+UNWIND(        .setfp  fpreg, sp               )
+       @
+       @ If we took the interrupt while running in the kernel, we may already
+       @ be using the IRQ stack, so revert to the original value in that case.
+       @
+       subs    r2, sp, r0              @ SP above bottom of IRQ stack?
+       rsbscs  r2, r2, #THREAD_SIZE    @ ... and below the top?
+       movcs   sp, r0                  @ If so, revert to incoming SP
+
+#ifndef CONFIG_UNWINDER_ARM
+       @
+       @ Inform the frame pointer unwinder where the next frame lives
+       @
+       movcc   lr, pc                  @ Make LR point into .entry.text so
+                                       @ that we will get a dump of the
+                                       @ exception stack for this frame.
+#ifdef CONFIG_CC_IS_GCC
+       movcc   ip, r0                  @ Store the old SP in the frame record.
+       stmdbcc sp!, {fp, ip, lr, pc}   @ Push frame record
+       addcc   fp, sp, #12
+#else
+       stmdbcc sp!, {fp, lr}           @ Push frame record
+       movcc   fp, sp
+#endif // CONFIG_CC_IS_GCC
+#endif // CONFIG_UNWINDER_ARM
+       .endif
+#endif // CONFIG_IRQSTACKS
+
        bl_m    [r1]
+
+#ifdef CONFIG_IRQSTACKS
+#ifdef CONFIG_UNWINDER_ARM
+       mov     sp, fpreg               @ Restore original SP
+#else
+       mov     fp, r8                  @ Restore original FP
+       mov     sp, r9                  @ Restore original SP
+#endif // CONFIG_UNWINDER_ARM
+#endif // CONFIG_IRQSTACKS
 #else
        arch_irq_handler_default
 #endif
@@ -200,7 +248,7 @@ ENDPROC(__dabt_svc)
        .align  5
 __irq_svc:
        svc_entry
-       irq_handler
+       irq_handler from_user=0
 
 #ifdef CONFIG_PREEMPTION
        ldr     r8, [tsk, #TI_PREEMPT]          @ get preempt count
@@ -427,7 +475,7 @@ ENDPROC(__dabt_usr)
 __irq_usr:
        usr_entry
        kuser_cmpxchg_check
-       irq_handler
+       irq_handler from_user=1
        get_thread_info tsk
        mov     why, #0
        b       ret_to_user_from_irq
arch/arm/kernel/irq.c +23 -0
index 20ab1e607522..58af2adb1583 100644
@@ -43,6 +43,25 @@
 
 unsigned long irq_err_count;
 
+#ifdef CONFIG_IRQSTACKS
+
+asmlinkage DEFINE_PER_CPU_READ_MOSTLY(u8 *, irq_stack_ptr);
+
+static void __init init_irq_stacks(void)
+{
+       u8 *stack;
+       int cpu;
+
+       for_each_possible_cpu(cpu) {
+               stack = (u8 *)__get_free_pages(GFP_KERNEL, THREAD_SIZE_ORDER);
+               if (WARN_ON(!stack))
+                       break;
+               per_cpu(irq_stack_ptr, cpu) = &stack[THREAD_SIZE];
+       }
+}
+
+#endif
+
 int arch_show_interrupts(struct seq_file *p, int prec)
 {
 #ifdef CONFIG_FIQ
@@ -99,6 +118,10 @@ void __init init_IRQ(void)
 {
        int ret;
 
+#ifdef CONFIG_IRQSTACKS
+       init_irq_stacks();
+#endif
+
        if (IS_ENABLED(CONFIG_OF) && !machine_desc->init_irq)
                irqchip_init();
        else
arch/arm/kernel/traps.c +0 -0
index cda32385552b..1ba9fc2eb564 100644
@@ -66,6 +66,19 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
 {
        unsigned long end = frame + 4 + sizeof(struct pt_regs);
 
Quoted 5 lines

arch/arm/kernel/traps.c void dump_backtrace_entry(unsigned long where, unsigned long from, · line 70

Quoted 2 lines
arch/arm/kernel/traps.c +2 -0
+       if (IS_ENABLED(CONFIG_UNWINDER_FRAME_POINTER) &&
+           IS_ENABLED(CONFIG_CC_IS_GCC) &&

Anything that needs to be adjusted here in the case of CONFIG_CC_IS_CLANG?

Not really. Clang simply does not record this information in the frame record, so there is no way to recover it.

Note that this only affects whether or not the exception stack is dumped for this frame, so it is not the end of the world.

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#12 lore raw

Re: [PATCH v3 08/10] ARM: implement IRQ stacks

Quoted 15 lines · Ard Biesheuvel
Quoted 9 lines
Quoted 9 lines
arch/arm/kernel/traps.c +0 -0
index cda32385552b..1ba9fc2eb564 100644
@@ -66,6 +66,19 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
 {
        unsigned long end = frame + 4 + sizeof(struct pt_regs);
 
Quoted 5 lines

arch/arm/kernel/traps.c void dump_backtrace_entry(unsigned long where, unsigned long from, · line 70

Quoted 2 lines
arch/arm/kernel/traps.c +2 -0
+       if (IS_ENABLED(CONFIG_UNWINDER_FRAME_POINTER) &&
+           IS_ENABLED(CONFIG_CC_IS_GCC) &&

Anything that needs to be adjusted here in the case of CONFIG_CC_IS_CLANG?

Quoted 5 lines

Not really. Clang simply does not record this information in the frame record, so there is no way to recover it.

Note that this only affects whether or not the exception stack is dumped for this frame, so it is not the end of the world.

Ok. Thanks for the clarification.

Signature · 7 lines
Thanks,
~Nick Desaulniers

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#13 lore raw

PATCH v3 9/10 ARM: call_with_stack: add unwind support

In reply to Ard Biesheuvel

Restructure the code and add the unwind annotations so that both the frame pointer unwinder as well as the EHABI unwind info based unwinder will be able to follow the call stack through call_with_stack().

Since GCC and Clang use different formats for the stack frame, two methods are implemented: a GCC version that pushes fp, sp, lr and pc for compatibility with the frame pointer unwinder, and a second version that works with Clang, as well as with the EHABI unwinder both in ARM and Thumb2 modes.

---
 arch/arm/lib/call_with_stack.S | 33 +++++++++++++++-----
 1 file changed, 25 insertions(+), 8 deletions(-)
arch/arm/lib/call_with_stack.S +25 -8
index 28b0341ae786..0a268a6c513c 100644
@@ -8,25 +8,42 @@
 
 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include <asm/unwind.h>
 
 /*
  * void call_with_stack(void (*fn)(void *), void *arg, void *sp)
  *
  * Change the stack to that pointed at by sp, then invoke fn(arg) with
  * the new stack.
+ *
+ * The sequence below follows the APCS frame convention for frame pointer
+ * unwinding, and implements the unwinder annotations needed by the EABI
+ * unwinder.
  */
-ENTRY(call_with_stack)
-	str	sp, [r2, #-4]!
-	str	lr, [r2, #-4]!
 
+ENTRY(call_with_stack)
+#if defined(CONFIG_UNWINDER_FRAME_POINTER) && defined(CONFIG_CC_IS_GCC)
+	mov	ip, sp
+	push	{fp, ip, lr, pc}
+	sub	fp, ip, #4
+#else
+UNWIND( .fnstart		)
+UNWIND( .save	{fpreg, lr}	)
+	push	{fpreg, lr}
+UNWIND( .setfp	fpreg, sp	)
+	mov	fpreg, sp
+#endif
 	mov	sp, r2
 	mov	r2, r0
 	mov	r0, r1
 
-	badr	lr, 1f
-	ret	r2
+	bl_r	r2
 
-1:	ldr	lr, [sp]
-	ldr	sp, [sp, #4]
-	ret	lr
+#if defined(CONFIG_UNWINDER_FRAME_POINTER) && defined(CONFIG_CC_IS_GCC)
+	ldmdb	fp, {fp, sp, pc}
+#else
+	mov	sp, fpreg
+	pop	{fpreg, pc}
+UNWIND( .fnend			)
+#endif
 ENDPROC(call_with_stack)
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Reviewed-by on v3 9/10
#14 lore raw

Re: [PATCH v3 09/10] ARM: call_with_stack: add unwind support

Quoted 10 lines · Ard Biesheuvel

Restructure the code and add the unwind annotations so that both the frame pointer unwinder as well as the EHABI unwind info based unwinder will be able to follow the call stack through call_with_stack().

Since GCC and Clang use different formats for the stack frame, two methods are implemented: a GCC version that pushes fp, sp, lr and pc for compatibility with the frame pointer unwinder, and a second version that works with Clang, as well as with the EHABI unwinder both in ARM and Thumb2 modes.

Thanks for the patch!

Quoted 66 lines
---
 arch/arm/lib/call_with_stack.S | 33 +++++++++++++++-----
 1 file changed, 25 insertions(+), 8 deletions(-)
arch/arm/lib/call_with_stack.S +25 -9
index 28b0341ae786..0a268a6c513c 100644
@@ -8,25 +8,42 @@
 
 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include <asm/unwind.h>
 
 /*
  * void call_with_stack(void (*fn)(void *), void *arg, void *sp)
  *
  * Change the stack to that pointed at by sp, then invoke fn(arg) with
  * the new stack.
+ *
+ * The sequence below follows the APCS frame convention for frame pointer
+ * unwinding, and implements the unwinder annotations needed by the EABI
+ * unwinder.
  */
-ENTRY(call_with_stack)
-       str     sp, [r2, #-4]!
-       str     lr, [r2, #-4]!
 
+ENTRY(call_with_stack)
+#if defined(CONFIG_UNWINDER_FRAME_POINTER) && defined(CONFIG_CC_IS_GCC)
+       mov     ip, sp
+       push    {fp, ip, lr, pc}
+       sub     fp, ip, #4
+#else
+UNWIND( .fnstart               )
+UNWIND( .save  {fpreg, lr}     )
+       push    {fpreg, lr}
+UNWIND( .setfp fpreg, sp       )
+       mov     fpreg, sp
+#endif
        mov     sp, r2
        mov     r2, r0
        mov     r0, r1
 
-       badr    lr, 1f
-       ret     r2
+       bl_r    r2
 
-1:     ldr     lr, [sp]
-       ldr     sp, [sp, #4]
-       ret     lr
+#if defined(CONFIG_UNWINDER_FRAME_POINTER) && defined(CONFIG_CC_IS_GCC)
+       ldmdb   fp, {fp, sp, pc}
+#else
+       mov     sp, fpreg
+       pop     {fpreg, pc}
+UNWIND( .fnend                 )
+#endif
 ENDPROC(call_with_stack)
--

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
#15 lore raw

PATCH v3 10/10 ARM: run softirqs on the per-CPU IRQ stack

In reply to Ard Biesheuvel

Now that we have enabled IRQ stacks, any softIRQs that are handled over the back of a hard IRQ will run from the IRQ stack as well. However, any synchronous softirq processing that happens when re-enabling softIRQs from task context will still execute on that task's stack.

Since any call to local_bh_enable() at any level in the task's call stack may trigger a softIRQ processing run, which could potentially cause a task stack overflow if the combined stack footprints exceed the stack's size, let's run these synchronous invocations of do_softirq() on the IRQ stack as well.

---
 arch/arm/Kconfig      |  2 ++
 arch/arm/kernel/irq.c | 14 ++++++++++++++
 2 files changed, 16 insertions(+)
arch/arm/Kconfig +2 -0
index a8c1db0736f3..d46b243e1b26 100644
@@ -1166,6 +1166,8 @@ config CURRENT_POINTER_IN_TPIDRURO
 config IRQSTACKS
 	def_bool y
 	depends on GENERIC_IRQ_MULTI_HANDLER && THREAD_INFO_IN_TASK
+	select HAVE_IRQ_EXIT_ON_IRQ_STACK
+	select HAVE_SOFTIRQ_ON_OWN_STACK
 
 config ARM_CPU_TOPOLOGY
 	bool "Support cpu topology definition"
arch/arm/kernel/irq.c +14 -0
index 58af2adb1583..bab3159b9074 100644
@@ -36,11 +36,14 @@
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/hardware/cache-uniphier.h>
 #include <asm/outercache.h>
+#include <asm/softirq_stack.h>
 #include <asm/exception.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/irq.h>
 #include <asm/mach/time.h>
 
+#include "reboot.h"
+
 unsigned long irq_err_count;
 
 #ifdef CONFIG_IRQSTACKS
@@ -60,6 +63,17 @@ static void __init init_irq_stacks(void)
 	}
 }
 
+static void ____do_softirq(void *arg)
+{
+	__do_softirq();
+}
+
+void do_softirq_own_stack(void)
+{
+	call_with_stack(____do_softirq, NULL,
+			__this_cpu_read(irq_stack_ptr));
+}
+
 #endif
 
 int arch_show_interrupts(struct seq_file *p, int prec)
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel