From: james.morse@arm.com (James Morse)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v8 9/4] arm64: remove irq_count and do_softirq_own_stack()
Date: Fri, 18 Dec 2015 16:01:47 +0000 [thread overview]
Message-ID: <1450454507-8907-1-git-send-email-james.morse@arm.com> (raw)
In-Reply-To: <1449226948-14251-1-git-send-email-james.morse@arm.com>
sysrq_handle_reboot() re-enables interrupts while on the irq stack. The
irq_stack implementation wrongly assumed this would only ever happen
via the softirq path, allowing it to update irq_count late, in
do_softirq_own_stack().
This means if an irq occurs in sysrq_handle_reboot(), during
emergency_restart() the stack will be corrupted, as irq_count wasn't
updated.
Lose the optimisation, and instead of moving the adding/subtracting of
irq_count into irq_stack_entry/irq_stack_exit, remove it, and compare
sp_el0 (struct task_info) with sp & ~(THREAD_SIZE - 1). This tells us if
we are on a task stack, if so, we can safely switch to the irq stack.
Finally, remove do_softirq_own_stack(), we don't need it anymore.
Reported-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>
---
arch/arm64/include/asm/irq.h | 2 --
arch/arm64/kernel/entry.S | 17 +++++++++--------
arch/arm64/kernel/irq.c | 38 +-------------------------------------
3 files changed, 10 insertions(+), 47 deletions(-)
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index 3bece4379bd9..b77197d941fc 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -11,8 +11,6 @@
#include <asm-generic/irq.h>
#include <asm/thread_info.h>
-#define __ARCH_HAS_DO_SOFTIRQ
-
struct pt_regs;
DECLARE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack);
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 0667fb7d8bb1..6745a9041f99 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -181,19 +181,20 @@ alternative_endif
.macro irq_stack_entry
mov x19, sp // preserve the original sp
- this_cpu_ptr irq_stack, x25, x26
-
/*
- * Check the lowest address on irq_stack for the irq_count value,
- * incremented by do_softirq_own_stack if we have re-enabled irqs
- * while on the irq_stack.
+ * Compare sp and sp_el0, if the top ~(THREAD_SIZE - 1) bits match,
+ * we are on a task stack, and should switch to the irq stack.
*/
- ldr x26, [x25]
- cbnz x26, 9998f // recursive use?
+ mrs x26, sp_el0 // already masked
+ and x25, x19, #~(THREAD_SIZE - 1)
+ cmp x25, x26
+ b.ne 9998f
- /* switch to the irq stack */
+ this_cpu_ptr irq_stack, x25, x26
mov x26, #IRQ_STACK_START_SP
add x26, x25, x26
+
+ /* switch to the irq stack */
mov sp, x26
/*
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index ff7ebb710e51..2386b26c0712 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -25,24 +25,14 @@
#include <linux/irq.h>
#include <linux/smp.h>
#include <linux/init.h>
-#include <linux/interrupt.h>
#include <linux/irqchip.h>
#include <linux/seq_file.h>
unsigned long irq_err_count;
-/*
- * irq stack only needs to be 16 byte aligned - not IRQ_STACK_SIZE aligned.
- * irq_stack[0] is used as irq_count, a non-zero value indicates the stack
- * is in use, and el?_irq() shouldn't switch to it. This is used to detect
- * recursive use of the irq_stack, it is lazily updated by
- * do_softirq_own_stack(), which is called on the irq_stack, before
- * re-enabling interrupts to process softirqs.
- */
+/* irq stack only needs to be 16 byte aligned - not IRQ_STACK_SIZE aligned. */
DEFINE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack) __aligned(16);
-#define IRQ_COUNT() (*per_cpu(irq_stack, smp_processor_id()))
-
int arch_show_interrupts(struct seq_file *p, int prec)
{
show_ipi_list(p, prec);
@@ -66,29 +56,3 @@ void __init init_IRQ(void)
if (!handle_arch_irq)
panic("No interrupt controller found.");
}
-
-/*
- * do_softirq_own_stack() is called from irq_exit() before __do_softirq()
- * re-enables interrupts, at which point we may re-enter el?_irq(). We
- * increase irq_count here so that el1_irq() knows that it is already on the
- * irq stack.
- *
- * Called with interrupts disabled, so we don't worry about moving cpu, or
- * being interrupted while modifying irq_count.
- *
- * This function doesn't actually switch stack.
- */
-void do_softirq_own_stack(void)
-{
- int cpu = smp_processor_id();
-
- WARN_ON_ONCE(!irqs_disabled());
-
- if (on_irq_stack(current_stack_pointer, cpu)) {
- IRQ_COUNT()++;
- __do_softirq();
- IRQ_COUNT()--;
- } else {
- __do_softirq();
- }
-}
--
2.6.2
next prev parent reply other threads:[~2015-12-18 16:01 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-04 11:02 [PATCH v8 0/4] arm64: Add support for IRQ stack James Morse
2015-12-04 11:02 ` [PATCH v8 1/4] arm64: Store struct task_info in sp_el0 James Morse
2015-12-04 13:27 ` Catalin Marinas
2015-12-04 14:55 ` James Morse
2015-12-04 16:18 ` Catalin Marinas
2015-12-06 13:15 ` Jungseok Lee
2015-12-04 11:02 ` [PATCH v8 2/4] arm64: Modify stack trace and dump for use with irq_stack James Morse
2015-12-04 12:21 ` Jungseok Lee
2015-12-04 14:31 ` Catalin Marinas
2015-12-04 11:02 ` [PATCH v8 3/4] arm64: Add do_softirq_own_stack() and enable irq_stacks James Morse
2015-12-04 13:46 ` Catalin Marinas
2015-12-04 13:47 ` Catalin Marinas
2015-12-07 22:48 ` Catalin Marinas
2015-12-08 11:43 ` Will Deacon
2015-12-08 16:02 ` Jungseok Lee
2015-12-08 17:23 ` James Morse
2015-12-08 17:27 ` Will Deacon
2015-12-08 23:13 ` Jungseok Lee
2015-12-09 9:47 ` James Morse
2015-12-09 11:38 ` Will Deacon
2015-12-09 13:45 ` Will Deacon
2015-12-09 14:36 ` James Morse
2015-12-04 11:02 ` [PATCH v8 4/4] arm64: switch to irq_stack during softirq James Morse
2015-12-04 14:01 ` Catalin Marinas
2015-12-04 14:39 ` James Morse
2015-12-04 18:40 ` Catalin Marinas
2015-12-08 10:29 ` James Morse
2015-12-06 13:51 ` Jungseok Lee
2015-12-04 12:17 ` [PATCH v8 0/4] arm64: Add support for IRQ stack Jungseok Lee
2015-12-06 13:56 ` Jungseok Lee
2015-12-04 13:57 ` Catalin Marinas
2015-12-06 13:33 ` Jungseok Lee
2015-12-10 10:22 ` [PATCH v8 5/4] arm64: Fix off-by-one in stack tracing when stepping off irq stack James Morse
2015-12-10 10:22 ` [PATCH v8 6/4] arm64: Add this_cpu_ptr() assembler macro for use in entry.S James Morse
2015-12-10 10:22 ` [PATCH v8 7/4] arm64: when walking onto the task stack, check sp & fp are in current->stack James Morse
2015-12-10 10:22 ` [PATCH v8 8/4] arm64: don't call C code with el0's fp register James Morse
2015-12-10 14:03 ` [PATCH v8 5/4] arm64: Fix off-by-one in stack tracing when stepping off irq stack Jungseok Lee
2015-12-15 11:21 ` [PATCH v8 9/4] arm64: reduce stack use in irq_handler James Morse
2015-12-18 16:01 ` James Morse [this message]
2015-12-20 11:07 ` [PATCH v8 9/4] arm64: remove irq_count and do_softirq_own_stack() Jungseok Lee
2015-12-21 11:30 ` Will Deacon
2015-12-21 12:19 ` James Morse
2015-12-21 12:21 ` Will Deacon
2015-12-21 14:06 ` 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=1450454507-8907-1-git-send-email-james.morse@arm.com \
--to=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).