Commit 7596abf2 authored by Will Deacon's avatar Will Deacon

arm64: irq: fix walking from irq stack to task stack

Running with CONFIG_DEBUG_SPINLOCK=y can trigger a BUG with the new IRQ
stack code:

  BUG: spinlock lockup suspected on CPU#1

This is due to the IRQ_STACK_TO_TASK_STACK macro incorrectly retrieving
the task stack pointer stashed at the top of the IRQ stack.

Sayeth James:

| Yup, this is what is happening. Its an off-by-one due to broken
| thinking about how the stack works. My broken thinking was:
|
| >   top ------------
| >       | dummy_lr | <- irq_stack_ptr
| >       ------------
| >       |   x29    |
| >       ------------
| >       |   x19    | <- irq_stack_ptr - 0x10
| >       ------------
| >       |   xzr    |
| >       ------------
|
| But the stack-pointer is decreased before use. So it actually looks
| like this:
|
| >       ------------
| >       |          |  <- irq_stack_ptr
| >   top ------------
| >       | dummy_lr |
| >       ------------
| >       |   x29    | <- irq_stack_ptr - 0x10
| >       ------------
| >       |   x19    |
| >       ------------
| >       |   xzr    | <- irq_stack_ptr - 0x20
| >       ------------
|
| The value being used as the original stack is x29, which in all the
| tests is sp but without the current frames data, hence there are no
| missing frames in the output.
|
| Jungseok Lee picked it up with a 32bit user space because aarch32
| can't use x29, so it remains 0 forever. The fix he posted is correct.

This patch fixes the macro and adds some of this wisdom to a comment,
so that the layout of the IRQ stack is well understood.

Cc: James Morse <james.morse@arm.com>
Reported-by: default avatarJungseok Lee <jungseoklee85@gmail.com>
Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
parent 8e23dacd
...@@ -19,7 +19,23 @@ DECLARE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack); ...@@ -19,7 +19,23 @@ DECLARE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack);
/* /*
* The highest address on the stack, and the first to be used. Used to * The highest address on the stack, and the first to be used. Used to
* find the dummy-stack frame put down by el?_irq() in entry.S. * find the dummy-stack frame put down by el?_irq() in entry.S, which
* is structured as follows:
*
* ------------
* | | <- irq_stack_ptr
* top ------------
* | elr_el1 |
* ------------
* | x29 | <- irq_stack_ptr - 0x10
* ------------
* | xzr |
* ------------
* | x19 | <- irq_stack_ptr - 0x20
* ------------
*
* where x19 holds a copy of the task stack pointer.
*
*/ */
#define IRQ_STACK_PTR(cpu) ((unsigned long)per_cpu(irq_stack, cpu) + IRQ_STACK_START_SP) #define IRQ_STACK_PTR(cpu) ((unsigned long)per_cpu(irq_stack, cpu) + IRQ_STACK_START_SP)
...@@ -27,7 +43,7 @@ DECLARE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack); ...@@ -27,7 +43,7 @@ DECLARE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack);
* The offset from irq_stack_ptr where entry.S will store the original * The offset from irq_stack_ptr where entry.S will store the original
* stack pointer. Used by unwind_frame() and dump_backtrace(). * stack pointer. Used by unwind_frame() and dump_backtrace().
*/ */
#define IRQ_STACK_TO_TASK_STACK(ptr) *((unsigned long *)(ptr - 0x10)); #define IRQ_STACK_TO_TASK_STACK(ptr) *((unsigned long *)(ptr - 0x20));
extern void set_handle_irq(void (*handle_irq)(struct pt_regs *)); extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
......
...@@ -199,7 +199,7 @@ alternative_endif ...@@ -199,7 +199,7 @@ alternative_endif
/* Add a dummy stack frame */ /* Add a dummy stack frame */
stp x29, \dummy_lr, [sp, #-16]! // dummy stack frame stp x29, \dummy_lr, [sp, #-16]! // dummy stack frame
mov x29, sp mov x29, sp
stp xzr, x19, [sp, #-16]! stp x19, xzr, [sp, #-16]!
9998: 9998:
.endm .endm
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment