All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Thompson <daniel.thompson@linaro.org>
To: Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>
Cc: Daniel Thompson <daniel.thompson@linaro.org>,
	Russell King <linux@arm.linux.org.uk>,
	Will Deacon <will.deacon@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	John Stultz <john.stultz@linaro.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, patches@linaro.org,
	linaro-kernel@lists.linaro.org,
	Sumit Semwal <sumit.semwal@linaro.org>,
	Dirk Behme <dirk.behme@de.bosch.com>,
	Daniel Drake <drake@endlessm.com>,
	Dmitry Pervushin <dpervushin@gmail.com>,
	Tim Sander <tim@krieglstein.org>
Subject: [RESEND PATCH 4.0-rc5 v19 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe
Date: Tue,  7 Apr 2015 16:37:59 +0100	[thread overview]
Message-ID: <1428421083-9137-3-git-send-email-daniel.thompson@linaro.org> (raw)
In-Reply-To: <1428421083-9137-1-git-send-email-daniel.thompson@linaro.org>

It is currently possible for FIQ handlers to re-enter gic_raise_softirq()
and lock up.

    	gic_raise_softirq()
	   lock(x);
-~-> FIQ
        handle_fiq()
	   gic_raise_softirq()
	      lock(x);		<-- Lockup

arch/arm/ uses IPIs to implement arch_irq_work_raise(), thus this issue
renders it difficult for FIQ handlers to safely defer work to less
restrictive calling contexts.

This patch fixes the problem by converting the cpu_map_migration_lock
into a rwlock making it safe to re-enter the function.

Note that having made it safe to re-enter gic_raise_softirq() we no
longer need to mask interrupts during gic_raise_softirq() because the
b.L migration is always performed from task context.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 drivers/irqchip/irq-gic.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index a181b836d5ea..578ffc5ec087 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -75,22 +75,25 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 /*
  * This lock is used by the big.LITTLE migration code to ensure no IPIs
  * can be pended on the old core after the map has been updated.
+ *
+ * This lock may be locked for reading from both IRQ and FIQ handlers
+ * and therefore must not be locked for writing when these are enabled.
  */
 #ifdef CONFIG_BL_SWITCHER
-static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock);
+static DEFINE_RWLOCK(cpu_map_migration_lock);
 
-static inline void gic_migration_lock(unsigned long *flags)
+static inline void gic_migration_lock(void)
 {
-	raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags);
+	read_lock(&cpu_map_migration_lock);
 }
 
-static inline void gic_migration_unlock(unsigned long flags)
+static inline void gic_migration_unlock(void)
 {
-	raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags);
+	read_unlock(&cpu_map_migration_lock);
 }
 #else
-static inline void gic_migration_lock(unsigned long *flags) {}
-static inline void gic_migration_unlock(unsigned long flags) {}
+static inline void gic_migration_lock(void) {}
+static inline void gic_migration_unlock(void) {}
 #endif
 
 /*
@@ -647,12 +650,20 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
 #endif
 
 #ifdef CONFIG_SMP
+/*
+ * Raise the specified IPI on all cpus set in mask.
+ *
+ * This function is safe to call from all calling contexts, including
+ * FIQ handlers. It relies on gic_migration_lock() being multiply acquirable
+ * to avoid deadlocks when the function is re-entered at different
+ * exception levels.
+ */
 static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 {
 	int cpu;
-	unsigned long flags, map = 0;
+	unsigned long map = 0;
 
-	gic_migration_lock(&flags);
+	gic_migration_lock();
 
 	/* Convert our logical CPU mask into a physical one. */
 	for_each_cpu(cpu, mask)
@@ -667,7 +678,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	/* this always happens on GIC0 */
 	writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
 
-	gic_migration_unlock(flags);
+	gic_migration_unlock();
 }
 #endif
 
@@ -715,7 +726,8 @@ int gic_get_cpu_id(unsigned int cpu)
  * Migrate all peripheral interrupts with a target matching the current CPU
  * to the interface corresponding to @new_cpu_id.  The CPU interface mapping
  * is also updated.  Targets to other CPU interfaces are unchanged.
- * This must be called with IRQs locally disabled.
+ * This must be called from a task context and with IRQ and FIQ locally
+ * disabled.
  */
 void gic_migrate_target(unsigned int new_cpu_id)
 {
@@ -746,9 +758,9 @@ void gic_migrate_target(unsigned int new_cpu_id)
 	 * pending on the old cpu static. That means we can defer the
 	 * migration until after we have released the irq_controller_lock.
 	 */
-	raw_spin_lock(&cpu_map_migration_lock);
+	write_lock(&cpu_map_migration_lock);
 	gic_cpu_map[cpu] = 1 << new_cpu_id;
-	raw_spin_unlock(&cpu_map_migration_lock);
+	write_unlock(&cpu_map_migration_lock);
 
 	/*
 	 * Find all the peripheral interrupts targetting the current
-- 
2.1.0


WARNING: multiple messages have this Message-ID (diff)
From: daniel.thompson@linaro.org (Daniel Thompson)
To: linux-arm-kernel@lists.infradead.org
Subject: [RESEND PATCH 4.0-rc5 v19 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe
Date: Tue,  7 Apr 2015 16:37:59 +0100	[thread overview]
Message-ID: <1428421083-9137-3-git-send-email-daniel.thompson@linaro.org> (raw)
In-Reply-To: <1428421083-9137-1-git-send-email-daniel.thompson@linaro.org>

It is currently possible for FIQ handlers to re-enter gic_raise_softirq()
and lock up.

    	gic_raise_softirq()
	   lock(x);
-~-> FIQ
        handle_fiq()
	   gic_raise_softirq()
	      lock(x);		<-- Lockup

arch/arm/ uses IPIs to implement arch_irq_work_raise(), thus this issue
renders it difficult for FIQ handlers to safely defer work to less
restrictive calling contexts.

This patch fixes the problem by converting the cpu_map_migration_lock
into a rwlock making it safe to re-enter the function.

Note that having made it safe to re-enter gic_raise_softirq() we no
longer need to mask interrupts during gic_raise_softirq() because the
b.L migration is always performed from task context.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 drivers/irqchip/irq-gic.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index a181b836d5ea..578ffc5ec087 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -75,22 +75,25 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 /*
  * This lock is used by the big.LITTLE migration code to ensure no IPIs
  * can be pended on the old core after the map has been updated.
+ *
+ * This lock may be locked for reading from both IRQ and FIQ handlers
+ * and therefore must not be locked for writing when these are enabled.
  */
 #ifdef CONFIG_BL_SWITCHER
-static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock);
+static DEFINE_RWLOCK(cpu_map_migration_lock);
 
-static inline void gic_migration_lock(unsigned long *flags)
+static inline void gic_migration_lock(void)
 {
-	raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags);
+	read_lock(&cpu_map_migration_lock);
 }
 
-static inline void gic_migration_unlock(unsigned long flags)
+static inline void gic_migration_unlock(void)
 {
-	raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags);
+	read_unlock(&cpu_map_migration_lock);
 }
 #else
-static inline void gic_migration_lock(unsigned long *flags) {}
-static inline void gic_migration_unlock(unsigned long flags) {}
+static inline void gic_migration_lock(void) {}
+static inline void gic_migration_unlock(void) {}
 #endif
 
 /*
@@ -647,12 +650,20 @@ static void __init gic_pm_init(struct gic_chip_data *gic)
 #endif
 
 #ifdef CONFIG_SMP
+/*
+ * Raise the specified IPI on all cpus set in mask.
+ *
+ * This function is safe to call from all calling contexts, including
+ * FIQ handlers. It relies on gic_migration_lock() being multiply acquirable
+ * to avoid deadlocks when the function is re-entered at different
+ * exception levels.
+ */
 static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 {
 	int cpu;
-	unsigned long flags, map = 0;
+	unsigned long map = 0;
 
-	gic_migration_lock(&flags);
+	gic_migration_lock();
 
 	/* Convert our logical CPU mask into a physical one. */
 	for_each_cpu(cpu, mask)
@@ -667,7 +678,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	/* this always happens on GIC0 */
 	writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
 
-	gic_migration_unlock(flags);
+	gic_migration_unlock();
 }
 #endif
 
@@ -715,7 +726,8 @@ int gic_get_cpu_id(unsigned int cpu)
  * Migrate all peripheral interrupts with a target matching the current CPU
  * to the interface corresponding to @new_cpu_id.  The CPU interface mapping
  * is also updated.  Targets to other CPU interfaces are unchanged.
- * This must be called with IRQs locally disabled.
+ * This must be called from a task context and with IRQ and FIQ locally
+ * disabled.
  */
 void gic_migrate_target(unsigned int new_cpu_id)
 {
@@ -746,9 +758,9 @@ void gic_migrate_target(unsigned int new_cpu_id)
 	 * pending on the old cpu static. That means we can defer the
 	 * migration until after we have released the irq_controller_lock.
 	 */
-	raw_spin_lock(&cpu_map_migration_lock);
+	write_lock(&cpu_map_migration_lock);
 	gic_cpu_map[cpu] = 1 << new_cpu_id;
-	raw_spin_unlock(&cpu_map_migration_lock);
+	write_unlock(&cpu_map_migration_lock);
 
 	/*
 	 * Find all the peripheral interrupts targetting the current
-- 
2.1.0

  parent reply	other threads:[~2015-04-07 15:41 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-24 16:53 [PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
2015-03-24 16:53 ` Daniel Thompson
2015-03-24 16:53 ` [PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq Daniel Thompson
2015-03-24 16:53   ` Daniel Thompson
2015-03-24 16:53 ` [PATCH 4.0-rc5 v19 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe Daniel Thompson
2015-03-24 16:53   ` Daniel Thompson
2015-03-24 16:53 ` [PATCH 4.0-rc5 v19 3/6] irqchip: gic: Introduce plumbing for IPI FIQ Daniel Thompson
2015-03-24 16:53   ` Daniel Thompson
2015-03-24 16:53 ` [PATCH 4.0-rc5 v19 4/6] printk: Simple implementation for NMI backtracing Daniel Thompson
2015-03-24 16:53   ` Daniel Thompson
2015-03-24 16:53 ` [PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions Daniel Thompson
2015-03-24 16:53   ` Daniel Thompson
2015-03-24 16:53 ` [PATCH 4.0-rc5 v19 6/6] ARM: Add support for on-demand backtrace of other CPUs Daniel Thompson
2015-03-24 16:53   ` Daniel Thompson
2015-04-07 15:37 ` [RESEND PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
2015-04-07 15:37   ` Daniel Thompson
2015-04-07 15:37   ` [RESEND PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq Daniel Thompson
2015-04-07 15:37     ` Daniel Thompson
2015-04-07 15:37   ` Daniel Thompson [this message]
2015-04-07 15:37     ` [RESEND PATCH 4.0-rc5 v19 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe Daniel Thompson
2015-04-07 15:38   ` [RESEND PATCH 4.0-rc5 v19 3/6] irqchip: gic: Introduce plumbing for IPI FIQ Daniel Thompson
2015-04-07 15:38     ` Daniel Thompson
2015-04-07 15:38   ` [RESEND PATCH 4.0-rc5 v19 4/6] printk: Simple implementation for NMI backtracing Daniel Thompson
2015-04-07 15:38     ` Daniel Thompson
2015-04-07 15:38   ` [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions Daniel Thompson
2015-04-07 15:38     ` Daniel Thompson
2015-04-07 16:19     ` Steven Rostedt
2015-04-07 16:19       ` Steven Rostedt
2015-04-07 16:37       ` Borislav Petkov
2015-04-07 16:37         ` Borislav Petkov
2015-04-07 16:43         ` Steven Rostedt
2015-04-07 16:43           ` Steven Rostedt
2015-04-08 12:08           ` Daniel Thompson
2015-04-08 12:08             ` Daniel Thompson
2015-04-07 15:38   ` [RESEND PATCH 4.0-rc5 v19 6/6] ARM: Add support for on-demand backtrace of other CPUs Daniel Thompson
2015-04-07 15:38     ` Daniel Thompson
2015-04-10  9:51 ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
2015-04-10  9:51   ` Daniel Thompson
2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 1/6] irqchip: gic: Optimize locking in gic_raise_softirq Daniel Thompson
2015-04-10  9:51     ` Daniel Thompson
2015-04-21 12:51     ` Marc Zyngier
2015-04-21 12:51       ` Marc Zyngier
2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe Daniel Thompson
2015-04-10  9:51     ` Daniel Thompson
2015-04-21 12:54     ` Marc Zyngier
2015-04-21 12:54       ` Marc Zyngier
2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ Daniel Thompson
2015-04-10  9:51     ` Daniel Thompson
2015-04-21 13:45     ` Marc Zyngier
2015-04-21 13:45       ` Marc Zyngier
2015-04-21 21:03       ` Daniel Thompson
2015-04-21 21:03         ` Daniel Thompson
2015-04-22  9:15         ` Marc Zyngier
2015-04-22  9:15           ` Marc Zyngier
2015-04-22 12:45           ` Daniel Thompson
2015-04-22 12:45             ` Daniel Thompson
2015-04-22 12:57             ` Marc Zyngier
2015-04-22 12:57               ` Marc Zyngier
2015-04-22 15:40               ` Daniel Thompson
2015-04-22 15:40                 ` Daniel Thompson
2015-04-21 14:50     ` Mark Rutland
2015-04-21 14:50       ` Mark Rutland
2015-04-21 21:15       ` Daniel Thompson
2015-04-21 21:15         ` Daniel Thompson
2015-04-22 10:38         ` Mark Rutland
2015-04-22 10:38           ` Mark Rutland
2015-07-02 13:31           ` Daniel Thompson
2015-07-02 13:31             ` Daniel Thompson
2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 4/6] printk: Simple implementation for NMI backtracing Daniel Thompson
2015-04-10  9:51     ` Daniel Thompson
2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 5/6] x86/nmi: Use common printk functions Daniel Thompson
2015-04-10  9:51     ` Daniel Thompson
2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 6/6] ARM: Add support for on-demand backtrace of other CPUs Daniel Thompson
2015-04-10  9:51     ` Daniel Thompson
2015-04-10 10:47   ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
2015-04-10 10:47     ` Daniel Thompson
2015-04-21 12:46   ` Thomas Gleixner
2015-04-21 12:46     ` Thomas Gleixner
2015-04-21 13:08     ` Marc Zyngier
2015-04-21 13:08       ` Marc Zyngier

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=1428421083-9137-3-git-send-email-daniel.thompson@linaro.org \
    --to=daniel.thompson@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=dirk.behme@de.bosch.com \
    --cc=dpervushin@gmail.com \
    --cc=drake@endlessm.com \
    --cc=jason@lakedaemon.net \
    --cc=john.stultz@linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=marc.zyngier@arm.com \
    --cc=patches@linaro.org \
    --cc=rostedt@goodmis.org \
    --cc=sboyd@codeaurora.org \
    --cc=sumit.semwal@linaro.org \
    --cc=tglx@linutronix.de \
    --cc=tim@krieglstein.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.