linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace
@ 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
                   ` (7 more replies)
  0 siblings, 8 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-03-24 16:53 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Jason/Thomas:
  This would be a resend except Steven Rostedt noticed a redundant
  memory barrier I had copied from the x86 code. The redundant barrier
  is now removed and there are no other changes since the code was posted
  a fortnight ago. Any chance of taking the first five of these patches
  via the irqchip route? The x86 patch has an ack from Ingo, printk has no
  explicit  maintainer and I've done plenty of bisectability tests on the
  patchset so leaving the last patch for the next dev. cycle should be
  no trouble.

This patchset modifies the GIC driver to allow it, on supported
platforms, to route IPI interrupts to FIQ. It then uses this
feature to implement arch_trigger_all_cpu_backtrace for arm.
In order to neatly bring in the changes for the arm we also rearrange
some of the existing x86 NMI code to make it architecture neutral.

The patchset http://thread.gmane.org/gmane.linux.kernel/1897765 , which
makes sched_clock() NMI/FIQ-safe, should be treated as a prerequisite
for the sixth and final patch in the series (which enables the feature
on ARM).  Although sched_clock() is not called directly by any of the
code that runs from a FIQ handler it is possible for sched_clock() to be
called indirectly when the function tracer is enabled.

The patches have been runtime tested on two systems capable of
supporting FIQ (Freescale i.MX6 and STiH416) and two that do not
(vexpress-a9 and Qualcomm Snapdragon 600), the changes to the x86
logic were tested on qemu and all patches have been compile tested
on x86, arm and arm64.

Note: On platforms not capable of supporting FIQ, the IPI to generate a
      backtrace will fall back to using IRQ for propagation instead.
      The backtrace logic contains a timeout to we will not permanently
      wedge the requesting CPU if other CPUs are not responsive.

v19:

* Remove redundant memory barrier inherited from the x86 code (Steven
  Rostedt).

v18:

* Move printk_nmi_ functions out of printk.c and into their own
  file, nmi_callback.c (Joe Perches/Steven Rostedt).

* Rename printk_nmi_ functions so their name matches their new home
  (Joe Perches)

v17:

* Rename bl_migration_lock/unlock to gic_migration_lock/unlock
  (Nicolas Pitre).

v16:

* Significant clean up of the printk patches (Thomas Gleixner).
  Replacing macros with real functions, CONFIG_ARCH_WANT_NMI_PRINTK
  -> CONFIG_PRINTK_NMI, prefixing global functions with printk_nmi,
  removing pointless exports, removing cpu_mask from the interfaces,
  removal of just-in-time initialization of trace buffers, prevented
  call sites having to save state, rolled up variable declarations
  into single lines.

* Dropped the sched_clock() patches from *this* patchset and managed
  them separately (http://thread.gmane.org/gmane.linux.kernel/1879261 ).
  The cross-dependancies between the patches are minimal; the backtrace
  code only calls sched_clock() if we are ftracing and backtracing is
  normally only triggered to report information about about a broken
  system (although users can type SysRq-l for amusement, most use it
  to find out why the system it dead).

* Squashed together the final two patches. Essentially these duplicated
  the x86 code and slavishly avoided changing it before, in the next
  patch, fixing it to work better on ARM. It seems better that the code
  just works first time!

v15:

* Added a patch to make sched_clock safe to call from NMI (Stephen
  Boyd). Note that sched_clock() is not called by the NMI handlers that
  have been added for the arm but it could be called if tools such as
  ftrace are deployed.

* Fixed some warnings picked up during bisectability testing.

v14:

* Moved a nmi_vprintk() and friends from arch/x86/kernel/apic/hw_nmi.c
  to printk.c (Steven Rostedt)

v13:

* Updated the code to print the backtrace to replicate Steven Rostedt's
  x86 work to make SysRq-l safe. This is pretty much a total rewrite of
  patches 4 and 5.

v12:

* Squash first two patches into a single one and re-describe
  (Thomas Gleixner).

* Improve description of "irqchip: gic: Make gic_raise_softirq FIQ-safe"
  (Thomas Gleixner).

v11:

* Optimized gic_raise_softirq() by replacing a register read with
  a memory read (Jason Cooper).

v10:

* Add a further patch to optimize away some of the locking on systems
  where CONFIG_BL_SWITCHER is not set (Marc Zyngier). Compiles OK with
  exynos_defconfig (which is the only defconfig to set this option).

* Whitespace fixes in patch 4. That patch previously used spaces for
  alignment of new constants but the rest of the file used tabs.

v9:

* Improved documentation and structure of initial patch (now initial
  two patches) to make gic_raise_softirq() safe to call from FIQ
  (Thomas Gleixner).

* Avoid masking interrupts during gic_raise_softirq(). The use of the
  read lock makes this redundant (because we can safely re-enter the
  function).

v8:

* Fixed build on arm64 causes by a spurious include file in irq-gic.c.

v7-2 (accidentally released twice with same number):

* Fixed boot regression on vexpress-a9 (reported by Russell King).

* Rebased on v3.18-rc3; removed one patch from set that is already
  included in mainline.

* Dropped arm64/fiq.h patch from the set (still useful but not related
  to issuing backtraces).

v7:

* Re-arranged code within the patch series to fix a regression
  introduced midway through the series and corrected by a later patch
  (testing by Olof's autobuilder). Tested offending patch in isolation
  using defconfig identified by the autobuilder.

v6:

* Renamed svc_entry's call_trace argument to just trace (example code
  from Russell King).

* Fixed mismatched ENDPROC() in __fiq_abt (example code from Russell
  King).

* Modified usr_entry to optional avoid calling into the trace code and
  used this in FIQ entry from usr path. Modified corresponding exit code
  to avoid calling into trace code and the scheduler (example code from
  Russell King).

* Ensured the default FIQ register state is restored when the default
  FIQ handler is reinstalled (example code from Russell King).

* Renamed no_fiq_insn to dfl_fiq_insn to reflect the effect of adopting
  a default FIQ handler.

* Re-instated fiq_safe_migration_lock and associated logic in
  gic_raise_softirq(). gic_raise_softirq() is called by wake_up_klogd()
  in the console unlock logic.

v5:

* Rebased on 3.17-rc4.

* Removed a spurious line from the final "glue it together" patch
  that broke the build.

v4:

* Replaced push/pop with stmfd/ldmfd respectively (review of Nicolas
  Pitre).

* Really fix bad pt_regs pointer generation in __fiq_abt.

* Remove fiq_safe_migration_lock and associated logic in
  gic_raise_softirq() (review of Russell King)

* Restructured to introduce the default FIQ handler first, before the
  new features (review of Russell King).

v3:

* Removed redundant header guards from arch/arm64/include/asm/fiq.h
  (review of Catalin Marinas).

* Moved svc_exit_via_fiq macro to entry-header.S (review of Nicolas
  Pitre).

v2:

* Restructured to sit nicely on a similar FYI patchset from Russell
  King. It now effectively replaces the work in progress final patch
  with something much more complete.

* Implemented (and tested) a Thumb-2 implementation of svc_exit_via_fiq
  (review of Nicolas Pitre)

* Dropped the GIC group 0 workaround patch. The issue of FIQ interrupts
  being acknowledged by the IRQ handler does still exist but should be
  harmless because the IRQ handler will still wind up calling
  ipi_cpu_backtrace().

* Removed any dependency on CONFIG_FIQ; all cpu backtrace effectively
  becomes a platform feature (although the use of non-maskable
  interrupts to implement it is best effort rather than guaranteed).

* Better comments highlighting usage of RAZ/WI registers (and parts of
  registers) in the GIC code.

Changes *before* v1:

* This patchset is a hugely cut-down successor to "[PATCH v11 00/19]
  arm: KGDB NMI/FIQ support". Thanks to Thomas Gleixner for suggesting
  the new structure. For historic details see:
        https://lkml.org/lkml/2014/9/2/227

* Fix bug in __fiq_abt (no longer passes a bad struct pt_regs value).
  In fixing this we also remove the useless indirection previously
  found in the fiq_handler macro.

* Make default fiq handler "always on" by migrating from fiq.c to
  traps.c and replace do_unexp_fiq with the new handler (review
  of Russell King).

* Add arm64 version of fiq.h (review of Russell King)

* Removed conditional branching and code from irq-gic.c, this is
  replaced by much simpler code that relies on the GIC specification's
  heavy use of read-as-zero/write-ignored (review of Russell King)


Daniel Thompson (6):
  irqchip: gic: Optimize locking in gic_raise_softirq
  irqchip: gic: Make gic_raise_softirq FIQ-safe
  irqchip: gic: Introduce plumbing for IPI FIQ
  printk: Simple implementation for NMI backtracing
  x86/nmi: Use common printk functions
  ARM: Add support for on-demand backtrace of other CPUs

 arch/arm/Kconfig                |   1 +
 arch/arm/include/asm/hardirq.h  |   2 +-
 arch/arm/include/asm/irq.h      |   5 +
 arch/arm/include/asm/smp.h      |   3 +
 arch/arm/kernel/smp.c           |  81 ++++++++++++++++
 arch/arm/kernel/traps.c         |   8 +-
 arch/x86/Kconfig                |   1 +
 arch/x86/kernel/apic/hw_nmi.c   | 101 ++------------------
 drivers/irqchip/irq-gic.c       | 203 +++++++++++++++++++++++++++++++++++++---
 include/linux/irqchip/arm-gic.h |   8 ++
 include/linux/printk.h          |  20 ++++
 init/Kconfig                    |   3 +
 kernel/printk/Makefile          |   1 +
 kernel/printk/nmi_backtrace.c   | 147 +++++++++++++++++++++++++++++
 14 files changed, 473 insertions(+), 111 deletions(-)
 create mode 100644 kernel/printk/nmi_backtrace.c

--
2.1.0


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq
  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 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe Daniel Thompson
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-03-24 16:53 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently gic_raise_softirq() is locked using upon irq_controller_lock.
This lock is primarily used to make register read-modify-write sequences
atomic but gic_raise_softirq() uses it instead to ensure that the
big.LITTLE migration logic can figure out when it is safe to migrate
interrupts between physical cores.

This is sub-optimal in closely related ways:

1. No locking at all is required on systems where the b.L switcher is
   not configured.

2. Finer grain locking can be used on systems where the b.L switcher is
   present.

This patch resolves both of the above by introducing a separate finer
grain lock and providing conditionally compiled inlines to lock/unlock
it.

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 | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 471e1cdc1933..a181b836d5ea 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -73,6 +73,27 @@ struct gic_chip_data {
 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.
+ */
+#ifdef CONFIG_BL_SWITCHER
+static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock);
+
+static inline void gic_migration_lock(unsigned long *flags)
+{
+	raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags);
+}
+
+static inline void gic_migration_unlock(unsigned long flags)
+{
+	raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags);
+}
+#else
+static inline void gic_migration_lock(unsigned long *flags) {}
+static inline void gic_migration_unlock(unsigned long flags) {}
+#endif
+
+/*
  * The GIC mapping of CPU interfaces does not necessarily match
  * the logical CPU numbering.  Let's use a mapping as returned
  * by the GIC itself.
@@ -631,7 +652,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	int cpu;
 	unsigned long flags, map = 0;
 
-	raw_spin_lock_irqsave(&irq_controller_lock, flags);
+	gic_migration_lock(&flags);
 
 	/* Convert our logical CPU mask into a physical one. */
 	for_each_cpu(cpu, mask)
@@ -646,7 +667,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);
 
-	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
+	gic_migration_unlock(flags);
 }
 #endif
 
@@ -717,8 +738,17 @@ void gic_migrate_target(unsigned int new_cpu_id)
 
 	raw_spin_lock(&irq_controller_lock);
 
-	/* Update the target interface for this logical CPU */
+	/*
+	 * Update the target interface for this logical CPU
+	 *
+	 * From the point we release the cpu_map_migration_lock any new
+	 * SGIs will be pended on the new cpu which makes the set of SGIs
+	 * 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);
 	gic_cpu_map[cpu] = 1 << new_cpu_id;
+	raw_spin_unlock(&cpu_map_migration_lock);
 
 	/*
 	 * Find all the peripheral interrupts targetting the current
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4.0-rc5 v19 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe
  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 ` [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 3/6] irqchip: gic: Introduce plumbing for IPI FIQ Daniel Thompson
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-03-24 16:53 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4.0-rc5 v19 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  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 ` [PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq 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 4/6] printk: Simple implementation for NMI backtracing Daniel Thompson
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-03-24 16:53 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently it is not possible to exploit FIQ for systems with a GIC, even if
the systems are otherwise capable of it. This patch makes it possible
for IPIs to be delivered using FIQ.

To do so it modifies the register state so that normal interrupts are
placed in group 1 and specific IPIs are placed into group 0. It also
configures the controller to raise group 0 interrupts using the FIQ
signal. It provides a means for architecture code to define which IPIs
shall use FIQ and to acknowledge any IPIs that are raised.

All GIC hardware except GICv1-without-TrustZone support provides a means
to group exceptions into group 0 and group 1 but the hardware
functionality is unavailable to the kernel when a secure monitor is
present because access to the grouping registers are prohibited outside
"secure world". However when grouping is not available (or in the case
of early GICv1 implementations is very hard to configure) the code to
change groups does not deploy and all IPIs will be raised via IRQ.

It has been tested and shown working on two systems capable of
supporting grouping (Freescale i.MX6 and STiH416). It has also been
tested for boot regressions on two systems that do not support grouping
(vexpress-a9 and Qualcomm Snapdragon 600).

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>
Tested-by: Jon Medhurst <tixy@linaro.org>
---
 arch/arm/kernel/traps.c         |   5 +-
 drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
 include/linux/irqchip/arm-gic.h |   8 +++
 3 files changed, 153 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 788e23fe64d8..b35e220ae1b1 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -26,6 +26,7 @@
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/irq.h>
+#include <linux/irqchip/arm-gic.h>
 
 #include <linux/atomic.h>
 #include <asm/cacheflush.h>
@@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
 
 	nmi_enter();
 
-	/* nop. FIQ handlers for special arch/arm features can be added here. */
+#ifdef CONFIG_ARM_GIC
+	gic_handle_fiq_ipi();
+#endif
 
 	nmi_exit();
 
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 578ffc5ec087..ffd1c0fe44b2 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -39,6 +39,7 @@
 #include <linux/slab.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqchip/arm-gic.h>
+#include <linux/ratelimit.h>
 
 #include <asm/cputype.h>
 #include <asm/irq.h>
@@ -48,6 +49,10 @@
 #include "irq-gic-common.h"
 #include "irqchip.h"
 
+#ifndef SMP_IPI_FIQ_MASK
+#define SMP_IPI_FIQ_MASK 0
+#endif
+
 union gic_base {
 	void __iomem *common_base;
 	void __percpu * __iomem *percpu_base;
@@ -65,6 +70,7 @@ struct gic_chip_data {
 #endif
 	struct irq_domain *domain;
 	unsigned int gic_irqs;
+	u32 igroup0_shadow;
 #ifdef CONFIG_GIC_NON_BANKED
 	void __iomem *(*get_base)(union gic_base *);
 #endif
@@ -355,6 +361,83 @@ static struct irq_chip gic_chip = {
 	.irq_set_wake		= gic_set_wake,
 };
 
+/*
+ * Shift an interrupt between Group 0 and Group 1.
+ *
+ * In addition to changing the group we also modify the priority to
+ * match what "ARM strongly recommends" for a system where no Group 1
+ * interrupt must ever preempt a Group 0 interrupt.
+ *
+ * If is safe to call this function on systems which do not support
+ * grouping (it will have no effect).
+ */
+static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq,
+			      int group)
+{
+	void __iomem *base = gic_data_dist_base(gic);
+	unsigned int grp_reg = hwirq / 32 * 4;
+	u32 grp_mask = BIT(hwirq % 32);
+	u32 grp_val;
+
+	unsigned int pri_reg = (hwirq / 4) * 4;
+	u32 pri_mask = BIT(7 + ((hwirq % 4) * 8));
+	u32 pri_val;
+
+	/*
+	 * Systems which do not support grouping will have not have
+	 * the EnableGrp1 bit set.
+	 */
+	if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)))
+		return;
+
+	raw_spin_lock(&irq_controller_lock);
+
+	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
+	pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg);
+
+	if (group) {
+		grp_val |= grp_mask;
+		pri_val |= pri_mask;
+	} else {
+		grp_val &= ~grp_mask;
+		pri_val &= ~pri_mask;
+	}
+
+	writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg);
+	if (grp_reg == 0)
+		gic->igroup0_shadow = grp_val;
+
+	writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
+
+	raw_spin_unlock(&irq_controller_lock);
+}
+
+
+/*
+ * Fully acknowledge (both ack and eoi) any outstanding FIQ-based IPI,
+ * otherwise do nothing.
+ */
+void gic_handle_fiq_ipi(void)
+{
+	struct gic_chip_data *gic = &gic_data[0];
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
+	unsigned long irqstat, irqnr;
+
+	if (WARN_ON(!in_nmi()))
+		return;
+
+	while ((1u << readl_relaxed(cpu_base + GIC_CPU_HIGHPRI)) &
+	       SMP_IPI_FIQ_MASK) {
+		irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
+		writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
+
+		irqnr = irqstat & GICC_IAR_INT_ID_MASK;
+		WARN_RATELIMIT(irqnr > 16,
+			       "Unexpected irqnr %lu (bad prioritization?)\n",
+			       irqnr);
+	}
+}
+
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 {
 	if (gic_nr >= MAX_GIC_NR)
@@ -386,15 +469,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
 static void gic_cpu_if_up(void)
 {
 	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
-	u32 bypass = 0;
+	void __iomem *dist_base = gic_data_dist_base(&gic_data[0]);
+	u32 ctrl = 0;
 
 	/*
-	* Preserve bypass disable bits to be written back later
-	*/
-	bypass = readl(cpu_base + GIC_CPU_CTRL);
-	bypass &= GICC_DIS_BYPASS_MASK;
+	 * Preserve bypass disable bits to be written back later
+	 */
+	ctrl = readl(cpu_base + GIC_CPU_CTRL);
+	ctrl &= GICC_DIS_BYPASS_MASK;
 
-	writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
+	/*
+	 * If EnableGrp1 is set in the distributor then enable group 1
+	 * support for this CPU (and route group 0 interrupts to FIQ).
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL))
+		ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL |
+			GICC_ENABLE_GRP1;
+
+	writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
 }
 
 
@@ -418,7 +510,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
 
 	gic_dist_config(base, gic_irqs, NULL);
 
-	writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
+	/*
+	 * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
+	 * bit 1 ignored) depending on current mode.
+	 */
+	writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL);
+
+	/*
+	 * Set all global interrupts to be group 1 if (and only if) it
+	 * is possible to enable group 1 interrupts. This register is RAZ/WI
+	 * if not accessible or not implemented, however some GICv1 devices
+	 * do not implement the EnableGrp1 bit making it unsafe to set
+	 * this register unconditionally.
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))
+		for (i = 32; i < gic_irqs; i += 32)
+			writel_relaxed(0xffffffff,
+				       base + GIC_DIST_IGROUP + i * 4 / 32);
 }
 
 static void gic_cpu_init(struct gic_chip_data *gic)
@@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	void __iomem *base = gic_data_cpu_base(gic);
 	unsigned int cpu_mask, cpu = smp_processor_id();
 	int i;
+	unsigned long secure_irqs, secure_irq;
 
 	/*
 	 * Get what the GIC says our CPU mask is.
@@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 
 	gic_cpu_config(dist_base, NULL);
 
+	/*
+	 * If the distributor is configured to support interrupt grouping
+	 * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
+	 * to be group1 and ensure any remaining group 0 interrupts have
+	 * the right priority.
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
+		secure_irqs = SMP_IPI_FIQ_MASK;
+		writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
+		gic->igroup0_shadow = ~secure_irqs;
+		for_each_set_bit(secure_irq, &secure_irqs, 16)
+			gic_set_group_irq(gic, secure_irq, 0);
+	}
+
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
 	gic_cpu_if_up();
 }
@@ -534,7 +657,8 @@ static void gic_dist_restore(unsigned int gic_nr)
 		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
-	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
+	writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE,
+		       dist_base + GIC_DIST_CTRL);
 }
 
 static void gic_cpu_save(unsigned int gic_nr)
@@ -662,6 +786,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 {
 	int cpu;
 	unsigned long map = 0;
+	unsigned long softint;
 
 	gic_migration_lock();
 
@@ -675,8 +800,14 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	 */
 	dmb(ishst);
 
-	/* this always happens on GIC0 */
-	writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+	/* We avoid a readl here by using the shadow copy of IGROUP[0] */
+	softint = map << 16 | irq;
+	if (gic_data[0].igroup0_shadow & BIT(irq))
+		softint |= 0x8000;
+
+	/* This always happens on GIC0 */
+	writel_relaxed(softint,
+		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
 
 	gic_migration_unlock();
 }
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 71d706d5f169..7690f70049a3 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -22,6 +22,10 @@
 #define GIC_CPU_IDENT			0xfc
 
 #define GICC_ENABLE			0x1
+#define GICC_ENABLE_GRP1		0x2
+#define GICC_ACK_CTL			0x4
+#define GICC_FIQ_EN			0x8
+#define GICC_COMMON_BPR			0x10
 #define GICC_INT_PRI_THRESHOLD		0xf0
 #define GICC_IAR_INT_ID_MASK		0x3ff
 #define GICC_INT_SPURIOUS		1023
@@ -44,6 +48,7 @@
 #define GIC_DIST_SGI_PENDING_SET	0xf20
 
 #define GICD_ENABLE			0x1
+#define GICD_ENABLE_GRP1		0x2
 #define GICD_DISABLE			0x0
 #define GICD_INT_ACTLOW_LVLTRIG		0x0
 #define GICD_INT_EN_CLR_X32		0xffffffff
@@ -121,5 +126,8 @@ static inline void __init register_routable_domain_ops
 {
 	gic_routable_irq_domain_ops = ops;
 }
+
+void gic_handle_fiq_ipi(void);
+
 #endif /* __ASSEMBLY */
 #endif
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4.0-rc5 v19 4/6] printk: Simple implementation for NMI backtracing
  2015-03-24 16:53 [PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                   ` (2 preceding siblings ...)
  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 5/6] x86/nmi: Use common printk functions Daniel Thompson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-03-24 16:53 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently there is a quite a pile of code sitting in
arch/x86/kernel/apic/hw_nmi.c to support safe all-cpu backtracing from NMI.
The code is inaccessible to backtrace implementations for other
architectures, which is a shame because they would probably like to be
safe too.

Copy this code into printk, reworking it a little as we do so to make
it easier to exploit as library code.

We'll port the x86 NMI backtrace logic to it in a later patch.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/printk.h        |  20 ++++++
 init/Kconfig                  |   3 +
 kernel/printk/Makefile        |   1 +
 kernel/printk/nmi_backtrace.c | 147 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 171 insertions(+)
 create mode 100644 kernel/printk/nmi_backtrace.c

diff --git a/include/linux/printk.h b/include/linux/printk.h
index baa3f97d8ce8..44bb85ad1f62 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -228,6 +228,26 @@ static inline void show_regs_print_info(const char *log_lvl)
 }
 #endif
 
+#ifdef CONFIG_PRINTK_NMI_BACKTRACE
+/*
+ * printk_nmi_backtrace_prepare/complete are called to prepare the
+ * system for some or all cores to issue trace from NMI.
+ * printk_nmi_backtrace_complete will print buffered output and cannot
+ * (safely) be called from NMI.
+ */
+extern int printk_nmi_backtrace_prepare(void);
+extern void printk_nmi_backtrace_complete(void);
+
+/*
+ * printk_nmi_backtrace_this_cpu_begin/end are used divert/restore printk
+ * on this cpu. The result is the output of printk() (by this CPU) will be
+ * stored in temporary buffers for later printing by
+ * printk_nmi_backtrace_complete.
+ */
+extern void printk_nmi_backtrace_this_cpu_begin(void);
+extern void printk_nmi_backtrace_this_cpu_end(void);
+#endif
+
 extern asmlinkage void dump_stack(void) __cold;
 
 #ifndef pr_fmt
diff --git a/init/Kconfig b/init/Kconfig
index f5dbc6d4261b..0107e9b4d2cf 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1421,6 +1421,9 @@ config PRINTK
 	  very difficult to diagnose system problems, saying N here is
 	  strongly discouraged.
 
+config PRINTK_NMI_BACKTRACE
+	bool
+
 config BUG
 	bool "BUG() support" if EXPERT
 	default y
diff --git a/kernel/printk/Makefile b/kernel/printk/Makefile
index 85405bdcf2b3..1849b001384a 100644
--- a/kernel/printk/Makefile
+++ b/kernel/printk/Makefile
@@ -1,2 +1,3 @@
 obj-y	= printk.o
+obj-$(CONFIG_PRINTK_NMI_BACKTRACE)	+= nmi_backtrace.o
 obj-$(CONFIG_A11Y_BRAILLE_CONSOLE)	+= braille.o
diff --git a/kernel/printk/nmi_backtrace.c b/kernel/printk/nmi_backtrace.c
new file mode 100644
index 000000000000..f24761262756
--- /dev/null
+++ b/kernel/printk/nmi_backtrace.c
@@ -0,0 +1,147 @@
+#include <linux/kernel.h>
+#include <linux/seq_buf.h>
+
+#define NMI_BUF_SIZE		4096
+
+struct nmi_seq_buf {
+	unsigned char		buffer[NMI_BUF_SIZE];
+	struct seq_buf		seq;
+};
+
+/* Safe printing in NMI context */
+static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq);
+
+static DEFINE_PER_CPU(printk_func_t, nmi_print_saved_print_func);
+
+/* "in progress" flag of NMI printing */
+static unsigned long nmi_print_flag;
+
+static int __init printk_nmi_backtrace_init(void)
+{
+	struct nmi_seq_buf *s;
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		s = &per_cpu(nmi_print_seq, cpu);
+		seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE);
+	}
+
+	return 0;
+}
+pure_initcall(printk_nmi_backtrace_init);
+
+/*
+ * It is not safe to call printk() directly from NMI handlers.
+ * It may be fine if the NMI detected a lock up and we have no choice
+ * but to do so, but doing a NMI on all other CPUs to get a back trace
+ * can be done with a sysrq-l. We don't want that to lock up, which
+ * can happen if the NMI interrupts a printk in progress.
+ *
+ * Instead, we redirect the vprintk() to this nmi_vprintk() that writes
+ * the content into a per cpu seq_buf buffer. Then when the NMIs are
+ * all done, we can safely dump the contents of the seq_buf to a printk()
+ * from a non NMI context.
+ *
+ * This is not a generic printk() implementation and must be used with
+ * great care. In particular there is a static limit on the quantity of
+ * data that may be emitted during NMI, only one client can be active at
+ * one time (arbitrated by the return value of printk_nmi_begin() and
+ * it is required that something at task or interrupt context be scheduled
+ * to issue the output.
+ */
+static int nmi_vprintk(const char *fmt, va_list args)
+{
+	struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
+	unsigned int len = seq_buf_used(&s->seq);
+
+	seq_buf_vprintf(&s->seq, fmt, args);
+	return seq_buf_used(&s->seq) - len;
+}
+
+/*
+ * Reserve the NMI printk mechanism. Return an error if some other component
+ * is already using it.
+ */
+int printk_nmi_backtrace_prepare(void)
+{
+	if (test_and_set_bit(0, &nmi_print_flag)) {
+		/*
+		 * If something is already using the NMI print facility we
+		 * can't allow a second one...
+		 */
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static void print_seq_line(struct nmi_seq_buf *s, int start, int end)
+{
+	const char *buf = s->buffer + start;
+
+	printk("%.*s", (end - start) + 1, buf);
+}
+
+void printk_nmi_backtrace_complete(void)
+{
+	struct nmi_seq_buf *s;
+	int len, cpu, i, last_i;
+
+	/*
+	 * Now that all the NMIs have triggered, we can dump out their
+	 * back traces safely to the console.
+	 */
+	for_each_possible_cpu(cpu) {
+		s = &per_cpu(nmi_print_seq, cpu);
+		last_i = 0;
+
+		len = seq_buf_used(&s->seq);
+		if (!len)
+			continue;
+
+		/* Print line by line. */
+		for (i = 0; i < len; i++) {
+			if (s->buffer[i] == '\n') {
+				print_seq_line(s, last_i, i);
+				last_i = i + 1;
+			}
+		}
+		/* Check if there was a partial line. */
+		if (last_i < len) {
+			print_seq_line(s, last_i, len - 1);
+			pr_cont("\n");
+		}
+
+		/* Wipe out the buffer ready for the next time around. */
+		seq_buf_clear(&s->seq);
+	}
+
+	clear_bit(0, &nmi_print_flag);
+}
+
+void printk_nmi_backtrace_this_cpu_begin(void)
+{
+	/*
+	 * Detect double-begins and report them. This code is unsafe (because
+	 * it will print from NMI) but things are pretty badly damaged if the
+	 * NMI re-enters and is somehow granted permission to use NMI printk,
+	 * so how much worse can it get? Also since this code interferes with
+	 * the operation of printk it is unlikely that any consequential
+	 * failures will be able to log anything making this our last
+	 * opportunity to tell anyone that something is wrong.
+	 */
+	if (this_cpu_read(nmi_print_saved_print_func)) {
+		this_cpu_write(printk_func,
+			       this_cpu_read(nmi_print_saved_print_func));
+		BUG();
+	}
+
+	this_cpu_write(nmi_print_saved_print_func, this_cpu_read(printk_func));
+	this_cpu_write(printk_func, nmi_vprintk);
+}
+
+void printk_nmi_backtrace_this_cpu_end(void)
+{
+	this_cpu_write(printk_func, this_cpu_read(nmi_print_saved_print_func));
+	this_cpu_write(nmi_print_saved_print_func, NULL);
+}
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions
  2015-03-24 16:53 [PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                   ` (3 preceding siblings ...)
  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 6/6] ARM: Add support for on-demand backtrace of other CPUs Daniel Thompson
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-03-24 16:53 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander, H. Peter Anvin, x86

Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support safe
all-cpu backtracing from NMI has been copied to printk.c to make it
accessible to other architectures.

Port the x86 NMI backtrace to the generic code.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/Kconfig              |   1 +
 arch/x86/kernel/apic/hw_nmi.c | 101 +++---------------------------------------
 2 files changed, 8 insertions(+), 94 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b7d31ca55187..a1a54570f2d0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -141,6 +141,7 @@ config X86
 	select ACPI_LEGACY_TABLES_LOOKUP if ACPI
 	select X86_FEATURE_NAMES if PROC_FS
 	select SRCU
+	select PRINTK_NMI_BACKTRACE if X86_LOCAL_APIC
 
 config INSTRUCTION_DECODER
 	def_bool y
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 6873ab925d00..db934f9461ed 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -30,40 +30,16 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh)
 #ifdef arch_trigger_all_cpu_backtrace
 /* For reliability, we're prepared to waste bits here. */
 static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
-static cpumask_t printtrace_mask;
-
-#define NMI_BUF_SIZE		4096
-
-struct nmi_seq_buf {
-	unsigned char		buffer[NMI_BUF_SIZE];
-	struct seq_buf		seq;
-};
-
-/* Safe printing in NMI context */
-static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq);
-
-/* "in progress" flag of arch_trigger_all_cpu_backtrace */
-static unsigned long backtrace_flag;
-
-static void print_seq_line(struct nmi_seq_buf *s, int start, int end)
-{
-	const char *buf = s->buffer + start;
-
-	printk("%.*s", (end - start) + 1, buf);
-}
 
 void arch_trigger_all_cpu_backtrace(bool include_self)
 {
-	struct nmi_seq_buf *s;
-	int len;
-	int cpu;
 	int i;
 	int this_cpu = get_cpu();
 
-	if (test_and_set_bit(0, &backtrace_flag)) {
+	if (0 != printk_nmi_backtrace_prepare()) {
 		/*
-		 * If there is already a trigger_all_cpu_backtrace() in progress
-		 * (backtrace_flag == 1), don't output double cpu dump infos.
+		 * If there is already an nmi printk sequence in
+		 * progress then just give up...
 		 */
 		put_cpu();
 		return;
@@ -73,16 +49,6 @@ void arch_trigger_all_cpu_backtrace(bool include_self)
 	if (!include_self)
 		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
 
-	cpumask_copy(&printtrace_mask, to_cpumask(backtrace_mask));
-	/*
-	 * Set up per_cpu seq_buf buffers that the NMIs running on the other
-	 * CPUs will write to.
-	 */
-	for_each_cpu(cpu, to_cpumask(backtrace_mask)) {
-		s = &per_cpu(nmi_print_seq, cpu);
-		seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE);
-	}
-
 	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
 		pr_info("sending NMI to %s CPUs:\n",
 			(include_self ? "all" : "other"));
@@ -97,73 +63,20 @@ void arch_trigger_all_cpu_backtrace(bool include_self)
 		touch_softlockup_watchdog();
 	}
 
-	/*
-	 * Now that all the NMIs have triggered, we can dump out their
-	 * back traces safely to the console.
-	 */
-	for_each_cpu(cpu, &printtrace_mask) {
-		int last_i = 0;
-
-		s = &per_cpu(nmi_print_seq, cpu);
-		len = seq_buf_used(&s->seq);
-		if (!len)
-			continue;
-
-		/* Print line by line. */
-		for (i = 0; i < len; i++) {
-			if (s->buffer[i] == '\n') {
-				print_seq_line(s, last_i, i);
-				last_i = i + 1;
-			}
-		}
-		/* Check if there was a partial line. */
-		if (last_i < len) {
-			print_seq_line(s, last_i, len - 1);
-			pr_cont("\n");
-		}
-	}
-
-	clear_bit(0, &backtrace_flag);
-	smp_mb__after_atomic();
+	printk_nmi_backtrace_complete();
 	put_cpu();
 }
 
-/*
- * It is not safe to call printk() directly from NMI handlers.
- * It may be fine if the NMI detected a lock up and we have no choice
- * but to do so, but doing a NMI on all other CPUs to get a back trace
- * can be done with a sysrq-l. We don't want that to lock up, which
- * can happen if the NMI interrupts a printk in progress.
- *
- * Instead, we redirect the vprintk() to this nmi_vprintk() that writes
- * the content into a per cpu seq_buf buffer. Then when the NMIs are
- * all done, we can safely dump the contents of the seq_buf to a printk()
- * from a non NMI context.
- */
-static int nmi_vprintk(const char *fmt, va_list args)
-{
-	struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
-	unsigned int len = seq_buf_used(&s->seq);
-
-	seq_buf_vprintf(&s->seq, fmt, args);
-	return seq_buf_used(&s->seq) - len;
-}
-
 static int
 arch_trigger_all_cpu_backtrace_handler(unsigned int cmd, struct pt_regs *regs)
 {
-	int cpu;
-
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
 	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
-		printk_func_t printk_func_save = this_cpu_read(printk_func);
-
-		/* Replace printk to write into the NMI seq */
-		this_cpu_write(printk_func, nmi_vprintk);
+		printk_nmi_backtrace_this_cpu_begin();
 		printk(KERN_WARNING "NMI backtrace for cpu %d\n", cpu);
 		show_regs(regs);
-		this_cpu_write(printk_func, printk_func_save);
+		printk_nmi_backtrace_this_cpu_end();
 
 		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
 		return NMI_HANDLED;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 4.0-rc5 v19 6/6] ARM: Add support for on-demand backtrace of other CPUs
  2015-03-24 16:53 [PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                   ` (4 preceding siblings ...)
  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-04-07 15:37 ` [RESEND PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace 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
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-03-24 16:53 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Replicate the x86 code to trigger a backtrace using an NMI and hook
it up to IPI on ARM.

The code differs slightly from the code on x86 because, on ARM, we do
now know at compile time whether a platform is capable of supporting FIQ.
We must avoid using an IPI to request a backtrace from the CPU on which
the backtrace was requested if interrupts are disabled and fall back to
generating it directly.

In addition the implementation of arch_trigger_all_cpu_backtrace() the
patch also includes a few small items of plumbing that must be hooked
up for the new code to work.

Credit:
  Russell King provided the initial prototype implementing this feature
  for ARM. Today the patch has been reworked and, mostly, rewriten to
  keep it aligned with x86. However this patch does still include some
  code from Russell's original prototype.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/arm/Kconfig               |  1 +
 arch/arm/include/asm/hardirq.h |  2 +-
 arch/arm/include/asm/irq.h     |  5 +++
 arch/arm/include/asm/smp.h     |  3 ++
 arch/arm/kernel/smp.c          | 81 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/traps.c        |  3 ++
 6 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9f1f09a2bc9b..f3c95a44945d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -76,6 +76,7 @@ config ARM
 	select OLD_SIGACTION
 	select OLD_SIGSUSPEND3
 	select PERF_USE_VMALLOC
+	select PRINTK_NMI_BACKTRACE
 	select RTC_LIB
 	select SYS_SUPPORTS_APM_EMULATION
 	# Above selects are sorted alphabetically; please add new ones
diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index fe3ea776dc34..5df33e30ae1b 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -5,7 +5,7 @@
 #include <linux/threads.h>
 #include <asm/irq.h>
 
-#define NR_IPI	8
+#define NR_IPI	9
 
 typedef struct {
 	unsigned int __softirq_pending;
diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index 53c15dec7af6..be1d07d59ee9 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -35,6 +35,11 @@ extern void (*handle_arch_irq)(struct pt_regs *);
 extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
 #endif
 
+#ifdef CONFIG_SMP
+extern void arch_trigger_all_cpu_backtrace(bool);
+#define arch_trigger_all_cpu_backtrace(x) arch_trigger_all_cpu_backtrace(x)
+#endif
+
 #endif
 
 #endif
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 18f5a554134f..b076584ac0fa 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -18,6 +18,8 @@
 # error "<asm/smp.h> included in non-SMP build"
 #endif
 
+#define SMP_IPI_FIQ_MASK 0x0100
+
 #define raw_smp_processor_id() (current_thread_info()->cpu)
 
 struct seq_file;
@@ -79,6 +81,7 @@ extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
 extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
 
+extern void ipi_cpu_backtrace(struct pt_regs *regs);
 extern int register_ipi_completion(struct completion *completion, int cpu);
 
 struct smp_operations {
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 86ef244c5a24..7eb6241e99d1 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -26,6 +26,7 @@
 #include <linux/completion.h>
 #include <linux/cpufreq.h>
 #include <linux/irq_work.h>
+#include <linux/seq_buf.h>
 
 #include <linux/atomic.h>
 #include <asm/smp.h>
@@ -72,6 +73,7 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_IRQ_WORK,
 	IPI_COMPLETION,
+	IPI_CPU_BACKTRACE,
 };
 
 static DECLARE_COMPLETION(cpu_running);
@@ -456,6 +458,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
 	S(IPI_CPU_STOP, "CPU stop interrupts"),
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
 	S(IPI_COMPLETION, "completion interrupts"),
+	S(IPI_CPU_BACKTRACE, "backtrace interrupts"),
 };
 
 static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
@@ -570,6 +573,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	unsigned int cpu = smp_processor_id();
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
+	BUILD_BUG_ON(SMP_IPI_FIQ_MASK != BIT(IPI_CPU_BACKTRACE));
+
 	if ((unsigned)ipinr < NR_IPI) {
 		trace_ipi_entry(ipi_types[ipinr]);
 		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
@@ -623,6 +628,12 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		irq_exit();
 		break;
 
+	case IPI_CPU_BACKTRACE:
+		irq_enter();
+		ipi_cpu_backtrace(regs);
+		irq_exit();
+		break;
+
 	default:
 		pr_crit("CPU%u: Unknown IPI message 0x%x\n",
 		        cpu, ipinr);
@@ -717,3 +728,73 @@ static int __init register_cpufreq_notifier(void)
 core_initcall(register_cpufreq_notifier);
 
 #endif
+
+/* For reliability, we're prepared to waste bits here. */
+static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
+
+void arch_trigger_all_cpu_backtrace(bool include_self)
+{
+	int i;
+	int this_cpu = get_cpu();
+
+	if (0 != printk_nmi_backtrace_prepare()) {
+		/*
+		 * If there is already an nmi printk sequence in
+		 * progress then just give up...
+		 */
+		put_cpu();
+		return;
+	}
+
+	cpumask_copy(to_cpumask(backtrace_mask), cpu_online_mask);
+
+	/*
+	 * If irqs are disabled on the current processor and
+	 * IPI_CPU_BACKTRACE is delivered using IRQ then we aren't be able to
+	 * react to IPI_CPU_BACKTRACE until we leave this function. This
+	 * would force us to get stuck and, eventually, timeout. We avoid
+	 * the timeout (and the resulting failure to print useful information)
+	 * by calling the backtrace logic directly whenever irqs are disabled.
+	 */
+	if (include_self && irqs_disabled()) {
+		ipi_cpu_backtrace(in_interrupt() ? get_irq_regs() : NULL);
+		include_self = false;
+	}
+
+	if (!include_self)
+		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
+
+	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
+		pr_info("Sending FIQ to %s CPUs:\n",
+			(include_self ? "all" : "other"));
+		smp_cross_call(to_cpumask(backtrace_mask), IPI_CPU_BACKTRACE);
+	}
+
+	/* Wait for up to 10 seconds for all CPUs to do the backtrace */
+	for (i = 0; i < 10 * 1000; i++) {
+		if (cpumask_empty(to_cpumask(backtrace_mask)))
+			break;
+		mdelay(1);
+		touch_softlockup_watchdog();
+	}
+
+	printk_nmi_backtrace_complete();
+	put_cpu();
+}
+
+void ipi_cpu_backtrace(struct pt_regs *regs)
+{
+	int cpu = smp_processor_id();
+
+	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
+		printk_nmi_backtrace_this_cpu_begin();
+		pr_warn("FIQ backtrace for cpu %d\n", cpu);
+		if (regs != NULL)
+			show_regs(regs);
+		else
+			dump_stack();
+		printk_nmi_backtrace_this_cpu_end();
+
+		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
+	}
+}
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index b35e220ae1b1..1836415b8a5c 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -483,6 +483,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
 #ifdef CONFIG_ARM_GIC
 	gic_handle_fiq_ipi();
 #endif
+#ifdef CONFIG_SMP
+	ipi_cpu_backtrace(regs);
+#endif
 
 	nmi_exit();
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace
  2015-03-24 16:53 [PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                   ` (5 preceding siblings ...)
  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-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
                     ` (5 more replies)
  2015-04-10  9:51 ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
  7 siblings, 6 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-07 15:37 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Jason/Thomas:
  Any chance of taking the first five of these patches via the irqchip
  route? The x86 patch has an ack from Ingo, printk has no explicit
  maintainer and I've done plenty of bisectability tests on the patchset
  so leaving the last patch for the next dev. cycle should be no
  trouble.

This patchset modifies the GIC driver to allow it, on supported
platforms, to route IPI interrupts to FIQ. It then uses this
feature to implement arch_trigger_all_cpu_backtrace for arm.
In order to neatly bring in the changes for the arm we also rearrange
some of the existing x86 NMI code to make it architecture neutral.

The patchset http://thread.gmane.org/gmane.linux.kernel/1897765 , which
makes sched_clock() NMI/FIQ-safe, should be treated as a prerequisite
for the sixth and final patch in the series (which enables the feature
on ARM).  Although sched_clock() is not called directly by any of the
code that runs from a FIQ handler it is possible for sched_clock() to be
called indirectly when the function tracer is enabled.

The patches have been runtime tested on two systems capable of
supporting FIQ (Freescale i.MX6 and STiH416) and two that do not
(vexpress-a9 and Qualcomm Snapdragon 600), the changes to the x86
logic were tested on qemu and all patches have been compile tested
on x86, arm and arm64.

Note: On platforms not capable of supporting FIQ, the IPI to generate a
      backtrace will fall back to using IRQ for propagation instead.
      The backtrace logic contains a timeout to we will not permanently
      wedge the requesting CPU if other CPUs are not responsive.

v19:

* Remove redundant memory barrier inherited from the x86 code (Steven
  Rostedt).

v18:

* Move printk_nmi_ functions out of printk.c and into their own
  file, nmi_callback.c (Joe Perches/Steven Rostedt).

* Rename printk_nmi_ functions so their name matches their new home
  (Joe Perches)

v17:

* Rename bl_migration_lock/unlock to gic_migration_lock/unlock
  (Nicolas Pitre).

v16:

* Significant clean up of the printk patches (Thomas Gleixner).
  Replacing macros with real functions, CONFIG_ARCH_WANT_NMI_PRINTK
  -> CONFIG_PRINTK_NMI, prefixing global functions with printk_nmi,
  removing pointless exports, removing cpu_mask from the interfaces,
  removal of just-in-time initialization of trace buffers, prevented
  call sites having to save state, rolled up variable declarations
  into single lines.

* Dropped the sched_clock() patches from *this* patchset and managed
  them separately (http://thread.gmane.org/gmane.linux.kernel/1879261 ).
  The cross-dependancies between the patches are minimal; the backtrace
  code only calls sched_clock() if we are ftracing and backtracing is
  normally only triggered to report information about about a broken
  system (although users can type SysRq-l for amusement, most use it
  to find out why the system it dead).

* Squashed together the final two patches. Essentially these duplicated
  the x86 code and slavishly avoided changing it before, in the next
  patch, fixing it to work better on ARM. It seems better that the code
  just works first time!

v15:

* Added a patch to make sched_clock safe to call from NMI (Stephen
  Boyd). Note that sched_clock() is not called by the NMI handlers that
  have been added for the arm but it could be called if tools such as
  ftrace are deployed.

* Fixed some warnings picked up during bisectability testing.

v14:

* Moved a nmi_vprintk() and friends from arch/x86/kernel/apic/hw_nmi.c
  to printk.c (Steven Rostedt)

v13:

* Updated the code to print the backtrace to replicate Steven Rostedt's
  x86 work to make SysRq-l safe. This is pretty much a total rewrite of
  patches 4 and 5.

v12:

* Squash first two patches into a single one and re-describe
  (Thomas Gleixner).

* Improve description of "irqchip: gic: Make gic_raise_softirq FIQ-safe"
  (Thomas Gleixner).

v11:

* Optimized gic_raise_softirq() by replacing a register read with
  a memory read (Jason Cooper).

v10:

* Add a further patch to optimize away some of the locking on systems
  where CONFIG_BL_SWITCHER is not set (Marc Zyngier). Compiles OK with
  exynos_defconfig (which is the only defconfig to set this option).

* Whitespace fixes in patch 4. That patch previously used spaces for
  alignment of new constants but the rest of the file used tabs.

v9:

* Improved documentation and structure of initial patch (now initial
  two patches) to make gic_raise_softirq() safe to call from FIQ
  (Thomas Gleixner).

* Avoid masking interrupts during gic_raise_softirq(). The use of the
  read lock makes this redundant (because we can safely re-enter the
  function).

v8:

* Fixed build on arm64 causes by a spurious include file in irq-gic.c.

v7-2 (accidentally released twice with same number):

* Fixed boot regression on vexpress-a9 (reported by Russell King).

* Rebased on v3.18-rc3; removed one patch from set that is already
  included in mainline.

* Dropped arm64/fiq.h patch from the set (still useful but not related
  to issuing backtraces).

v7:

* Re-arranged code within the patch series to fix a regression
  introduced midway through the series and corrected by a later patch
  (testing by Olof's autobuilder). Tested offending patch in isolation
  using defconfig identified by the autobuilder.

v6:

* Renamed svc_entry's call_trace argument to just trace (example code
  from Russell King).

* Fixed mismatched ENDPROC() in __fiq_abt (example code from Russell
  King).

* Modified usr_entry to optional avoid calling into the trace code and
  used this in FIQ entry from usr path. Modified corresponding exit code
  to avoid calling into trace code and the scheduler (example code from
  Russell King).

* Ensured the default FIQ register state is restored when the default
  FIQ handler is reinstalled (example code from Russell King).

* Renamed no_fiq_insn to dfl_fiq_insn to reflect the effect of adopting
  a default FIQ handler.

* Re-instated fiq_safe_migration_lock and associated logic in
  gic_raise_softirq(). gic_raise_softirq() is called by wake_up_klogd()
  in the console unlock logic.

v5:

* Rebased on 3.17-rc4.

* Removed a spurious line from the final "glue it together" patch
  that broke the build.

v4:

* Replaced push/pop with stmfd/ldmfd respectively (review of Nicolas
  Pitre).

* Really fix bad pt_regs pointer generation in __fiq_abt.

* Remove fiq_safe_migration_lock and associated logic in
  gic_raise_softirq() (review of Russell King)

* Restructured to introduce the default FIQ handler first, before the
  new features (review of Russell King).

v3:

* Removed redundant header guards from arch/arm64/include/asm/fiq.h
  (review of Catalin Marinas).

* Moved svc_exit_via_fiq macro to entry-header.S (review of Nicolas
  Pitre).

v2:

* Restructured to sit nicely on a similar FYI patchset from Russell
  King. It now effectively replaces the work in progress final patch
  with something much more complete.

* Implemented (and tested) a Thumb-2 implementation of svc_exit_via_fiq
  (review of Nicolas Pitre)

* Dropped the GIC group 0 workaround patch. The issue of FIQ interrupts
  being acknowledged by the IRQ handler does still exist but should be
  harmless because the IRQ handler will still wind up calling
  ipi_cpu_backtrace().

* Removed any dependency on CONFIG_FIQ; all cpu backtrace effectively
  becomes a platform feature (although the use of non-maskable
  interrupts to implement it is best effort rather than guaranteed).

* Better comments highlighting usage of RAZ/WI registers (and parts of
  registers) in the GIC code.

Changes *before* v1:

* This patchset is a hugely cut-down successor to "[PATCH v11 00/19]
  arm: KGDB NMI/FIQ support". Thanks to Thomas Gleixner for suggesting
  the new structure. For historic details see:
        https://lkml.org/lkml/2014/9/2/227

* Fix bug in __fiq_abt (no longer passes a bad struct pt_regs value).
  In fixing this we also remove the useless indirection previously
  found in the fiq_handler macro.

* Make default fiq handler "always on" by migrating from fiq.c to
  traps.c and replace do_unexp_fiq with the new handler (review
  of Russell King).

* Add arm64 version of fiq.h (review of Russell King)

* Removed conditional branching and code from irq-gic.c, this is
  replaced by much simpler code that relies on the GIC specification's
  heavy use of read-as-zero/write-ignored (review of Russell King)


Daniel Thompson (6):
  irqchip: gic: Optimize locking in gic_raise_softirq
  irqchip: gic: Make gic_raise_softirq FIQ-safe
  irqchip: gic: Introduce plumbing for IPI FIQ
  printk: Simple implementation for NMI backtracing
  x86/nmi: Use common printk functions
  ARM: Add support for on-demand backtrace of other CPUs

 arch/arm/Kconfig                |   1 +
 arch/arm/include/asm/hardirq.h  |   2 +-
 arch/arm/include/asm/irq.h      |   5 +
 arch/arm/include/asm/smp.h      |   3 +
 arch/arm/kernel/smp.c           |  81 ++++++++++++++++
 arch/arm/kernel/traps.c         |   8 +-
 arch/x86/Kconfig                |   1 +
 arch/x86/kernel/apic/hw_nmi.c   | 101 ++------------------
 drivers/irqchip/irq-gic.c       | 203 +++++++++++++++++++++++++++++++++++++---
 include/linux/irqchip/arm-gic.h |   8 ++
 include/linux/printk.h          |  20 ++++
 init/Kconfig                    |   3 +
 kernel/printk/Makefile          |   1 +
 kernel/printk/nmi_backtrace.c   | 147 +++++++++++++++++++++++++++++
 14 files changed, 473 insertions(+), 111 deletions(-)
 create mode 100644 kernel/printk/nmi_backtrace.c

--
2.1.0


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq
  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 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe Daniel Thompson
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-07 15:37 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently gic_raise_softirq() is locked using upon irq_controller_lock.
This lock is primarily used to make register read-modify-write sequences
atomic but gic_raise_softirq() uses it instead to ensure that the
big.LITTLE migration logic can figure out when it is safe to migrate
interrupts between physical cores.

This is sub-optimal in closely related ways:

1. No locking at all is required on systems where the b.L switcher is
   not configured.

2. Finer grain locking can be used on systems where the b.L switcher is
   present.

This patch resolves both of the above by introducing a separate finer
grain lock and providing conditionally compiled inlines to lock/unlock
it.

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 | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 471e1cdc1933..a181b836d5ea 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -73,6 +73,27 @@ struct gic_chip_data {
 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.
+ */
+#ifdef CONFIG_BL_SWITCHER
+static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock);
+
+static inline void gic_migration_lock(unsigned long *flags)
+{
+	raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags);
+}
+
+static inline void gic_migration_unlock(unsigned long flags)
+{
+	raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags);
+}
+#else
+static inline void gic_migration_lock(unsigned long *flags) {}
+static inline void gic_migration_unlock(unsigned long flags) {}
+#endif
+
+/*
  * The GIC mapping of CPU interfaces does not necessarily match
  * the logical CPU numbering.  Let's use a mapping as returned
  * by the GIC itself.
@@ -631,7 +652,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	int cpu;
 	unsigned long flags, map = 0;
 
-	raw_spin_lock_irqsave(&irq_controller_lock, flags);
+	gic_migration_lock(&flags);
 
 	/* Convert our logical CPU mask into a physical one. */
 	for_each_cpu(cpu, mask)
@@ -646,7 +667,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);
 
-	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
+	gic_migration_unlock(flags);
 }
 #endif
 
@@ -717,8 +738,17 @@ void gic_migrate_target(unsigned int new_cpu_id)
 
 	raw_spin_lock(&irq_controller_lock);
 
-	/* Update the target interface for this logical CPU */
+	/*
+	 * Update the target interface for this logical CPU
+	 *
+	 * From the point we release the cpu_map_migration_lock any new
+	 * SGIs will be pended on the new cpu which makes the set of SGIs
+	 * 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);
 	gic_cpu_map[cpu] = 1 << new_cpu_id;
+	raw_spin_unlock(&cpu_map_migration_lock);
 
 	/*
 	 * Find all the peripheral interrupts targetting the current
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc5 v19 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe
  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   ` [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:38   ` [RESEND PATCH 4.0-rc5 v19 3/6] irqchip: gic: Introduce plumbing for IPI FIQ Daniel Thompson
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-07 15:37 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc5 v19 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  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   ` [RESEND PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq Daniel Thompson
  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   ` Daniel Thompson
  2015-04-07 15:38   ` [RESEND PATCH 4.0-rc5 v19 4/6] printk: Simple implementation for NMI backtracing Daniel Thompson
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-07 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently it is not possible to exploit FIQ for systems with a GIC, even if
the systems are otherwise capable of it. This patch makes it possible
for IPIs to be delivered using FIQ.

To do so it modifies the register state so that normal interrupts are
placed in group 1 and specific IPIs are placed into group 0. It also
configures the controller to raise group 0 interrupts using the FIQ
signal. It provides a means for architecture code to define which IPIs
shall use FIQ and to acknowledge any IPIs that are raised.

All GIC hardware except GICv1-without-TrustZone support provides a means
to group exceptions into group 0 and group 1 but the hardware
functionality is unavailable to the kernel when a secure monitor is
present because access to the grouping registers are prohibited outside
"secure world". However when grouping is not available (or in the case
of early GICv1 implementations is very hard to configure) the code to
change groups does not deploy and all IPIs will be raised via IRQ.

It has been tested and shown working on two systems capable of
supporting grouping (Freescale i.MX6 and STiH416). It has also been
tested for boot regressions on two systems that do not support grouping
(vexpress-a9 and Qualcomm Snapdragon 600).

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>
Tested-by: Jon Medhurst <tixy@linaro.org>
---
 arch/arm/kernel/traps.c         |   5 +-
 drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
 include/linux/irqchip/arm-gic.h |   8 +++
 3 files changed, 153 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 788e23fe64d8..b35e220ae1b1 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -26,6 +26,7 @@
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/irq.h>
+#include <linux/irqchip/arm-gic.h>
 
 #include <linux/atomic.h>
 #include <asm/cacheflush.h>
@@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
 
 	nmi_enter();
 
-	/* nop. FIQ handlers for special arch/arm features can be added here. */
+#ifdef CONFIG_ARM_GIC
+	gic_handle_fiq_ipi();
+#endif
 
 	nmi_exit();
 
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 578ffc5ec087..ffd1c0fe44b2 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -39,6 +39,7 @@
 #include <linux/slab.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqchip/arm-gic.h>
+#include <linux/ratelimit.h>
 
 #include <asm/cputype.h>
 #include <asm/irq.h>
@@ -48,6 +49,10 @@
 #include "irq-gic-common.h"
 #include "irqchip.h"
 
+#ifndef SMP_IPI_FIQ_MASK
+#define SMP_IPI_FIQ_MASK 0
+#endif
+
 union gic_base {
 	void __iomem *common_base;
 	void __percpu * __iomem *percpu_base;
@@ -65,6 +70,7 @@ struct gic_chip_data {
 #endif
 	struct irq_domain *domain;
 	unsigned int gic_irqs;
+	u32 igroup0_shadow;
 #ifdef CONFIG_GIC_NON_BANKED
 	void __iomem *(*get_base)(union gic_base *);
 #endif
@@ -355,6 +361,83 @@ static struct irq_chip gic_chip = {
 	.irq_set_wake		= gic_set_wake,
 };
 
+/*
+ * Shift an interrupt between Group 0 and Group 1.
+ *
+ * In addition to changing the group we also modify the priority to
+ * match what "ARM strongly recommends" for a system where no Group 1
+ * interrupt must ever preempt a Group 0 interrupt.
+ *
+ * If is safe to call this function on systems which do not support
+ * grouping (it will have no effect).
+ */
+static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq,
+			      int group)
+{
+	void __iomem *base = gic_data_dist_base(gic);
+	unsigned int grp_reg = hwirq / 32 * 4;
+	u32 grp_mask = BIT(hwirq % 32);
+	u32 grp_val;
+
+	unsigned int pri_reg = (hwirq / 4) * 4;
+	u32 pri_mask = BIT(7 + ((hwirq % 4) * 8));
+	u32 pri_val;
+
+	/*
+	 * Systems which do not support grouping will have not have
+	 * the EnableGrp1 bit set.
+	 */
+	if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)))
+		return;
+
+	raw_spin_lock(&irq_controller_lock);
+
+	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
+	pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg);
+
+	if (group) {
+		grp_val |= grp_mask;
+		pri_val |= pri_mask;
+	} else {
+		grp_val &= ~grp_mask;
+		pri_val &= ~pri_mask;
+	}
+
+	writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg);
+	if (grp_reg == 0)
+		gic->igroup0_shadow = grp_val;
+
+	writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
+
+	raw_spin_unlock(&irq_controller_lock);
+}
+
+
+/*
+ * Fully acknowledge (both ack and eoi) any outstanding FIQ-based IPI,
+ * otherwise do nothing.
+ */
+void gic_handle_fiq_ipi(void)
+{
+	struct gic_chip_data *gic = &gic_data[0];
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
+	unsigned long irqstat, irqnr;
+
+	if (WARN_ON(!in_nmi()))
+		return;
+
+	while ((1u << readl_relaxed(cpu_base + GIC_CPU_HIGHPRI)) &
+	       SMP_IPI_FIQ_MASK) {
+		irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
+		writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
+
+		irqnr = irqstat & GICC_IAR_INT_ID_MASK;
+		WARN_RATELIMIT(irqnr > 16,
+			       "Unexpected irqnr %lu (bad prioritization?)\n",
+			       irqnr);
+	}
+}
+
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 {
 	if (gic_nr >= MAX_GIC_NR)
@@ -386,15 +469,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
 static void gic_cpu_if_up(void)
 {
 	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
-	u32 bypass = 0;
+	void __iomem *dist_base = gic_data_dist_base(&gic_data[0]);
+	u32 ctrl = 0;
 
 	/*
-	* Preserve bypass disable bits to be written back later
-	*/
-	bypass = readl(cpu_base + GIC_CPU_CTRL);
-	bypass &= GICC_DIS_BYPASS_MASK;
+	 * Preserve bypass disable bits to be written back later
+	 */
+	ctrl = readl(cpu_base + GIC_CPU_CTRL);
+	ctrl &= GICC_DIS_BYPASS_MASK;
 
-	writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
+	/*
+	 * If EnableGrp1 is set in the distributor then enable group 1
+	 * support for this CPU (and route group 0 interrupts to FIQ).
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL))
+		ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL |
+			GICC_ENABLE_GRP1;
+
+	writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
 }
 
 
@@ -418,7 +510,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
 
 	gic_dist_config(base, gic_irqs, NULL);
 
-	writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
+	/*
+	 * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
+	 * bit 1 ignored) depending on current mode.
+	 */
+	writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL);
+
+	/*
+	 * Set all global interrupts to be group 1 if (and only if) it
+	 * is possible to enable group 1 interrupts. This register is RAZ/WI
+	 * if not accessible or not implemented, however some GICv1 devices
+	 * do not implement the EnableGrp1 bit making it unsafe to set
+	 * this register unconditionally.
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))
+		for (i = 32; i < gic_irqs; i += 32)
+			writel_relaxed(0xffffffff,
+				       base + GIC_DIST_IGROUP + i * 4 / 32);
 }
 
 static void gic_cpu_init(struct gic_chip_data *gic)
@@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	void __iomem *base = gic_data_cpu_base(gic);
 	unsigned int cpu_mask, cpu = smp_processor_id();
 	int i;
+	unsigned long secure_irqs, secure_irq;
 
 	/*
 	 * Get what the GIC says our CPU mask is.
@@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 
 	gic_cpu_config(dist_base, NULL);
 
+	/*
+	 * If the distributor is configured to support interrupt grouping
+	 * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
+	 * to be group1 and ensure any remaining group 0 interrupts have
+	 * the right priority.
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
+		secure_irqs = SMP_IPI_FIQ_MASK;
+		writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
+		gic->igroup0_shadow = ~secure_irqs;
+		for_each_set_bit(secure_irq, &secure_irqs, 16)
+			gic_set_group_irq(gic, secure_irq, 0);
+	}
+
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
 	gic_cpu_if_up();
 }
@@ -534,7 +657,8 @@ static void gic_dist_restore(unsigned int gic_nr)
 		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
-	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
+	writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE,
+		       dist_base + GIC_DIST_CTRL);
 }
 
 static void gic_cpu_save(unsigned int gic_nr)
@@ -662,6 +786,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 {
 	int cpu;
 	unsigned long map = 0;
+	unsigned long softint;
 
 	gic_migration_lock();
 
@@ -675,8 +800,14 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	 */
 	dmb(ishst);
 
-	/* this always happens on GIC0 */
-	writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+	/* We avoid a readl here by using the shadow copy of IGROUP[0] */
+	softint = map << 16 | irq;
+	if (gic_data[0].igroup0_shadow & BIT(irq))
+		softint |= 0x8000;
+
+	/* This always happens on GIC0 */
+	writel_relaxed(softint,
+		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
 
 	gic_migration_unlock();
 }
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 71d706d5f169..7690f70049a3 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -22,6 +22,10 @@
 #define GIC_CPU_IDENT			0xfc
 
 #define GICC_ENABLE			0x1
+#define GICC_ENABLE_GRP1		0x2
+#define GICC_ACK_CTL			0x4
+#define GICC_FIQ_EN			0x8
+#define GICC_COMMON_BPR			0x10
 #define GICC_INT_PRI_THRESHOLD		0xf0
 #define GICC_IAR_INT_ID_MASK		0x3ff
 #define GICC_INT_SPURIOUS		1023
@@ -44,6 +48,7 @@
 #define GIC_DIST_SGI_PENDING_SET	0xf20
 
 #define GICD_ENABLE			0x1
+#define GICD_ENABLE_GRP1		0x2
 #define GICD_DISABLE			0x0
 #define GICD_INT_ACTLOW_LVLTRIG		0x0
 #define GICD_INT_EN_CLR_X32		0xffffffff
@@ -121,5 +126,8 @@ static inline void __init register_routable_domain_ops
 {
 	gic_routable_irq_domain_ops = ops;
 }
+
+void gic_handle_fiq_ipi(void);
+
 #endif /* __ASSEMBLY */
 #endif
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc5 v19 4/6] printk: Simple implementation for NMI backtracing
  2015-04-07 15:37 ` [RESEND PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (2 preceding siblings ...)
  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 5/6] x86/nmi: Use common printk functions 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
  5 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-07 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently there is a quite a pile of code sitting in
arch/x86/kernel/apic/hw_nmi.c to support safe all-cpu backtracing from NMI.
The code is inaccessible to backtrace implementations for other
architectures, which is a shame because they would probably like to be
safe too.

Copy this code into printk, reworking it a little as we do so to make
it easier to exploit as library code.

We'll port the x86 NMI backtrace logic to it in a later patch.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/printk.h        |  20 ++++++
 init/Kconfig                  |   3 +
 kernel/printk/Makefile        |   1 +
 kernel/printk/nmi_backtrace.c | 147 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 171 insertions(+)
 create mode 100644 kernel/printk/nmi_backtrace.c

diff --git a/include/linux/printk.h b/include/linux/printk.h
index baa3f97d8ce8..44bb85ad1f62 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -228,6 +228,26 @@ static inline void show_regs_print_info(const char *log_lvl)
 }
 #endif
 
+#ifdef CONFIG_PRINTK_NMI_BACKTRACE
+/*
+ * printk_nmi_backtrace_prepare/complete are called to prepare the
+ * system for some or all cores to issue trace from NMI.
+ * printk_nmi_backtrace_complete will print buffered output and cannot
+ * (safely) be called from NMI.
+ */
+extern int printk_nmi_backtrace_prepare(void);
+extern void printk_nmi_backtrace_complete(void);
+
+/*
+ * printk_nmi_backtrace_this_cpu_begin/end are used divert/restore printk
+ * on this cpu. The result is the output of printk() (by this CPU) will be
+ * stored in temporary buffers for later printing by
+ * printk_nmi_backtrace_complete.
+ */
+extern void printk_nmi_backtrace_this_cpu_begin(void);
+extern void printk_nmi_backtrace_this_cpu_end(void);
+#endif
+
 extern asmlinkage void dump_stack(void) __cold;
 
 #ifndef pr_fmt
diff --git a/init/Kconfig b/init/Kconfig
index f5dbc6d4261b..0107e9b4d2cf 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1421,6 +1421,9 @@ config PRINTK
 	  very difficult to diagnose system problems, saying N here is
 	  strongly discouraged.
 
+config PRINTK_NMI_BACKTRACE
+	bool
+
 config BUG
 	bool "BUG() support" if EXPERT
 	default y
diff --git a/kernel/printk/Makefile b/kernel/printk/Makefile
index 85405bdcf2b3..1849b001384a 100644
--- a/kernel/printk/Makefile
+++ b/kernel/printk/Makefile
@@ -1,2 +1,3 @@
 obj-y	= printk.o
+obj-$(CONFIG_PRINTK_NMI_BACKTRACE)	+= nmi_backtrace.o
 obj-$(CONFIG_A11Y_BRAILLE_CONSOLE)	+= braille.o
diff --git a/kernel/printk/nmi_backtrace.c b/kernel/printk/nmi_backtrace.c
new file mode 100644
index 000000000000..f24761262756
--- /dev/null
+++ b/kernel/printk/nmi_backtrace.c
@@ -0,0 +1,147 @@
+#include <linux/kernel.h>
+#include <linux/seq_buf.h>
+
+#define NMI_BUF_SIZE		4096
+
+struct nmi_seq_buf {
+	unsigned char		buffer[NMI_BUF_SIZE];
+	struct seq_buf		seq;
+};
+
+/* Safe printing in NMI context */
+static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq);
+
+static DEFINE_PER_CPU(printk_func_t, nmi_print_saved_print_func);
+
+/* "in progress" flag of NMI printing */
+static unsigned long nmi_print_flag;
+
+static int __init printk_nmi_backtrace_init(void)
+{
+	struct nmi_seq_buf *s;
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		s = &per_cpu(nmi_print_seq, cpu);
+		seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE);
+	}
+
+	return 0;
+}
+pure_initcall(printk_nmi_backtrace_init);
+
+/*
+ * It is not safe to call printk() directly from NMI handlers.
+ * It may be fine if the NMI detected a lock up and we have no choice
+ * but to do so, but doing a NMI on all other CPUs to get a back trace
+ * can be done with a sysrq-l. We don't want that to lock up, which
+ * can happen if the NMI interrupts a printk in progress.
+ *
+ * Instead, we redirect the vprintk() to this nmi_vprintk() that writes
+ * the content into a per cpu seq_buf buffer. Then when the NMIs are
+ * all done, we can safely dump the contents of the seq_buf to a printk()
+ * from a non NMI context.
+ *
+ * This is not a generic printk() implementation and must be used with
+ * great care. In particular there is a static limit on the quantity of
+ * data that may be emitted during NMI, only one client can be active at
+ * one time (arbitrated by the return value of printk_nmi_begin() and
+ * it is required that something at task or interrupt context be scheduled
+ * to issue the output.
+ */
+static int nmi_vprintk(const char *fmt, va_list args)
+{
+	struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
+	unsigned int len = seq_buf_used(&s->seq);
+
+	seq_buf_vprintf(&s->seq, fmt, args);
+	return seq_buf_used(&s->seq) - len;
+}
+
+/*
+ * Reserve the NMI printk mechanism. Return an error if some other component
+ * is already using it.
+ */
+int printk_nmi_backtrace_prepare(void)
+{
+	if (test_and_set_bit(0, &nmi_print_flag)) {
+		/*
+		 * If something is already using the NMI print facility we
+		 * can't allow a second one...
+		 */
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static void print_seq_line(struct nmi_seq_buf *s, int start, int end)
+{
+	const char *buf = s->buffer + start;
+
+	printk("%.*s", (end - start) + 1, buf);
+}
+
+void printk_nmi_backtrace_complete(void)
+{
+	struct nmi_seq_buf *s;
+	int len, cpu, i, last_i;
+
+	/*
+	 * Now that all the NMIs have triggered, we can dump out their
+	 * back traces safely to the console.
+	 */
+	for_each_possible_cpu(cpu) {
+		s = &per_cpu(nmi_print_seq, cpu);
+		last_i = 0;
+
+		len = seq_buf_used(&s->seq);
+		if (!len)
+			continue;
+
+		/* Print line by line. */
+		for (i = 0; i < len; i++) {
+			if (s->buffer[i] == '\n') {
+				print_seq_line(s, last_i, i);
+				last_i = i + 1;
+			}
+		}
+		/* Check if there was a partial line. */
+		if (last_i < len) {
+			print_seq_line(s, last_i, len - 1);
+			pr_cont("\n");
+		}
+
+		/* Wipe out the buffer ready for the next time around. */
+		seq_buf_clear(&s->seq);
+	}
+
+	clear_bit(0, &nmi_print_flag);
+}
+
+void printk_nmi_backtrace_this_cpu_begin(void)
+{
+	/*
+	 * Detect double-begins and report them. This code is unsafe (because
+	 * it will print from NMI) but things are pretty badly damaged if the
+	 * NMI re-enters and is somehow granted permission to use NMI printk,
+	 * so how much worse can it get? Also since this code interferes with
+	 * the operation of printk it is unlikely that any consequential
+	 * failures will be able to log anything making this our last
+	 * opportunity to tell anyone that something is wrong.
+	 */
+	if (this_cpu_read(nmi_print_saved_print_func)) {
+		this_cpu_write(printk_func,
+			       this_cpu_read(nmi_print_saved_print_func));
+		BUG();
+	}
+
+	this_cpu_write(nmi_print_saved_print_func, this_cpu_read(printk_func));
+	this_cpu_write(printk_func, nmi_vprintk);
+}
+
+void printk_nmi_backtrace_this_cpu_end(void)
+{
+	this_cpu_write(printk_func, this_cpu_read(nmi_print_saved_print_func));
+	this_cpu_write(nmi_print_saved_print_func, NULL);
+}
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions
  2015-04-07 15:37 ` [RESEND PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (3 preceding siblings ...)
  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 16:19     ` Steven Rostedt
  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
  5 siblings, 1 reply; 40+ messages in thread
From: Daniel Thompson @ 2015-04-07 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander, H. Peter Anvin, x86

Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support safe
all-cpu backtracing from NMI has been copied to printk.c to make it
accessible to other architectures.

Port the x86 NMI backtrace to the generic code.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/Kconfig              |   1 +
 arch/x86/kernel/apic/hw_nmi.c | 101 +++---------------------------------------
 2 files changed, 8 insertions(+), 94 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b7d31ca55187..a1a54570f2d0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -141,6 +141,7 @@ config X86
 	select ACPI_LEGACY_TABLES_LOOKUP if ACPI
 	select X86_FEATURE_NAMES if PROC_FS
 	select SRCU
+	select PRINTK_NMI_BACKTRACE if X86_LOCAL_APIC
 
 config INSTRUCTION_DECODER
 	def_bool y
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 6873ab925d00..db934f9461ed 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -30,40 +30,16 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh)
 #ifdef arch_trigger_all_cpu_backtrace
 /* For reliability, we're prepared to waste bits here. */
 static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
-static cpumask_t printtrace_mask;
-
-#define NMI_BUF_SIZE		4096
-
-struct nmi_seq_buf {
-	unsigned char		buffer[NMI_BUF_SIZE];
-	struct seq_buf		seq;
-};
-
-/* Safe printing in NMI context */
-static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq);
-
-/* "in progress" flag of arch_trigger_all_cpu_backtrace */
-static unsigned long backtrace_flag;
-
-static void print_seq_line(struct nmi_seq_buf *s, int start, int end)
-{
-	const char *buf = s->buffer + start;
-
-	printk("%.*s", (end - start) + 1, buf);
-}
 
 void arch_trigger_all_cpu_backtrace(bool include_self)
 {
-	struct nmi_seq_buf *s;
-	int len;
-	int cpu;
 	int i;
 	int this_cpu = get_cpu();
 
-	if (test_and_set_bit(0, &backtrace_flag)) {
+	if (0 != printk_nmi_backtrace_prepare()) {
 		/*
-		 * If there is already a trigger_all_cpu_backtrace() in progress
-		 * (backtrace_flag == 1), don't output double cpu dump infos.
+		 * If there is already an nmi printk sequence in
+		 * progress then just give up...
 		 */
 		put_cpu();
 		return;
@@ -73,16 +49,6 @@ void arch_trigger_all_cpu_backtrace(bool include_self)
 	if (!include_self)
 		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
 
-	cpumask_copy(&printtrace_mask, to_cpumask(backtrace_mask));
-	/*
-	 * Set up per_cpu seq_buf buffers that the NMIs running on the other
-	 * CPUs will write to.
-	 */
-	for_each_cpu(cpu, to_cpumask(backtrace_mask)) {
-		s = &per_cpu(nmi_print_seq, cpu);
-		seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE);
-	}
-
 	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
 		pr_info("sending NMI to %s CPUs:\n",
 			(include_self ? "all" : "other"));
@@ -97,73 +63,20 @@ void arch_trigger_all_cpu_backtrace(bool include_self)
 		touch_softlockup_watchdog();
 	}
 
-	/*
-	 * Now that all the NMIs have triggered, we can dump out their
-	 * back traces safely to the console.
-	 */
-	for_each_cpu(cpu, &printtrace_mask) {
-		int last_i = 0;
-
-		s = &per_cpu(nmi_print_seq, cpu);
-		len = seq_buf_used(&s->seq);
-		if (!len)
-			continue;
-
-		/* Print line by line. */
-		for (i = 0; i < len; i++) {
-			if (s->buffer[i] == '\n') {
-				print_seq_line(s, last_i, i);
-				last_i = i + 1;
-			}
-		}
-		/* Check if there was a partial line. */
-		if (last_i < len) {
-			print_seq_line(s, last_i, len - 1);
-			pr_cont("\n");
-		}
-	}
-
-	clear_bit(0, &backtrace_flag);
-	smp_mb__after_atomic();
+	printk_nmi_backtrace_complete();
 	put_cpu();
 }
 
-/*
- * It is not safe to call printk() directly from NMI handlers.
- * It may be fine if the NMI detected a lock up and we have no choice
- * but to do so, but doing a NMI on all other CPUs to get a back trace
- * can be done with a sysrq-l. We don't want that to lock up, which
- * can happen if the NMI interrupts a printk in progress.
- *
- * Instead, we redirect the vprintk() to this nmi_vprintk() that writes
- * the content into a per cpu seq_buf buffer. Then when the NMIs are
- * all done, we can safely dump the contents of the seq_buf to a printk()
- * from a non NMI context.
- */
-static int nmi_vprintk(const char *fmt, va_list args)
-{
-	struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
-	unsigned int len = seq_buf_used(&s->seq);
-
-	seq_buf_vprintf(&s->seq, fmt, args);
-	return seq_buf_used(&s->seq) - len;
-}
-
 static int
 arch_trigger_all_cpu_backtrace_handler(unsigned int cmd, struct pt_regs *regs)
 {
-	int cpu;
-
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
 	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
-		printk_func_t printk_func_save = this_cpu_read(printk_func);
-
-		/* Replace printk to write into the NMI seq */
-		this_cpu_write(printk_func, nmi_vprintk);
+		printk_nmi_backtrace_this_cpu_begin();
 		printk(KERN_WARNING "NMI backtrace for cpu %d\n", cpu);
 		show_regs(regs);
-		this_cpu_write(printk_func, printk_func_save);
+		printk_nmi_backtrace_this_cpu_end();
 
 		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
 		return NMI_HANDLED;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc5 v19 6/6] ARM: Add support for on-demand backtrace of other CPUs
  2015-04-07 15:37 ` [RESEND PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (4 preceding siblings ...)
  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
  5 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-07 15:38 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Replicate the x86 code to trigger a backtrace using an NMI and hook
it up to IPI on ARM.

The code differs slightly from the code on x86 because, on ARM, we do
now know at compile time whether a platform is capable of supporting FIQ.
We must avoid using an IPI to request a backtrace from the CPU on which
the backtrace was requested if interrupts are disabled and fall back to
generating it directly.

In addition the implementation of arch_trigger_all_cpu_backtrace() the
patch also includes a few small items of plumbing that must be hooked
up for the new code to work.

Credit:
  Russell King provided the initial prototype implementing this feature
  for ARM. Today the patch has been reworked and, mostly, rewriten to
  keep it aligned with x86. However this patch does still include some
  code from Russell's original prototype.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/arm/Kconfig               |  1 +
 arch/arm/include/asm/hardirq.h |  2 +-
 arch/arm/include/asm/irq.h     |  5 +++
 arch/arm/include/asm/smp.h     |  3 ++
 arch/arm/kernel/smp.c          | 81 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/traps.c        |  3 ++
 6 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9f1f09a2bc9b..f3c95a44945d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -76,6 +76,7 @@ config ARM
 	select OLD_SIGACTION
 	select OLD_SIGSUSPEND3
 	select PERF_USE_VMALLOC
+	select PRINTK_NMI_BACKTRACE
 	select RTC_LIB
 	select SYS_SUPPORTS_APM_EMULATION
 	# Above selects are sorted alphabetically; please add new ones
diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index fe3ea776dc34..5df33e30ae1b 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -5,7 +5,7 @@
 #include <linux/threads.h>
 #include <asm/irq.h>
 
-#define NR_IPI	8
+#define NR_IPI	9
 
 typedef struct {
 	unsigned int __softirq_pending;
diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index 53c15dec7af6..be1d07d59ee9 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -35,6 +35,11 @@ extern void (*handle_arch_irq)(struct pt_regs *);
 extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
 #endif
 
+#ifdef CONFIG_SMP
+extern void arch_trigger_all_cpu_backtrace(bool);
+#define arch_trigger_all_cpu_backtrace(x) arch_trigger_all_cpu_backtrace(x)
+#endif
+
 #endif
 
 #endif
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 18f5a554134f..b076584ac0fa 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -18,6 +18,8 @@
 # error "<asm/smp.h> included in non-SMP build"
 #endif
 
+#define SMP_IPI_FIQ_MASK 0x0100
+
 #define raw_smp_processor_id() (current_thread_info()->cpu)
 
 struct seq_file;
@@ -79,6 +81,7 @@ extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
 extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
 
+extern void ipi_cpu_backtrace(struct pt_regs *regs);
 extern int register_ipi_completion(struct completion *completion, int cpu);
 
 struct smp_operations {
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 86ef244c5a24..7eb6241e99d1 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -26,6 +26,7 @@
 #include <linux/completion.h>
 #include <linux/cpufreq.h>
 #include <linux/irq_work.h>
+#include <linux/seq_buf.h>
 
 #include <linux/atomic.h>
 #include <asm/smp.h>
@@ -72,6 +73,7 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_IRQ_WORK,
 	IPI_COMPLETION,
+	IPI_CPU_BACKTRACE,
 };
 
 static DECLARE_COMPLETION(cpu_running);
@@ -456,6 +458,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
 	S(IPI_CPU_STOP, "CPU stop interrupts"),
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
 	S(IPI_COMPLETION, "completion interrupts"),
+	S(IPI_CPU_BACKTRACE, "backtrace interrupts"),
 };
 
 static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
@@ -570,6 +573,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	unsigned int cpu = smp_processor_id();
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
+	BUILD_BUG_ON(SMP_IPI_FIQ_MASK != BIT(IPI_CPU_BACKTRACE));
+
 	if ((unsigned)ipinr < NR_IPI) {
 		trace_ipi_entry(ipi_types[ipinr]);
 		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
@@ -623,6 +628,12 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		irq_exit();
 		break;
 
+	case IPI_CPU_BACKTRACE:
+		irq_enter();
+		ipi_cpu_backtrace(regs);
+		irq_exit();
+		break;
+
 	default:
 		pr_crit("CPU%u: Unknown IPI message 0x%x\n",
 		        cpu, ipinr);
@@ -717,3 +728,73 @@ static int __init register_cpufreq_notifier(void)
 core_initcall(register_cpufreq_notifier);
 
 #endif
+
+/* For reliability, we're prepared to waste bits here. */
+static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
+
+void arch_trigger_all_cpu_backtrace(bool include_self)
+{
+	int i;
+	int this_cpu = get_cpu();
+
+	if (0 != printk_nmi_backtrace_prepare()) {
+		/*
+		 * If there is already an nmi printk sequence in
+		 * progress then just give up...
+		 */
+		put_cpu();
+		return;
+	}
+
+	cpumask_copy(to_cpumask(backtrace_mask), cpu_online_mask);
+
+	/*
+	 * If irqs are disabled on the current processor and
+	 * IPI_CPU_BACKTRACE is delivered using IRQ then we aren't be able to
+	 * react to IPI_CPU_BACKTRACE until we leave this function. This
+	 * would force us to get stuck and, eventually, timeout. We avoid
+	 * the timeout (and the resulting failure to print useful information)
+	 * by calling the backtrace logic directly whenever irqs are disabled.
+	 */
+	if (include_self && irqs_disabled()) {
+		ipi_cpu_backtrace(in_interrupt() ? get_irq_regs() : NULL);
+		include_self = false;
+	}
+
+	if (!include_self)
+		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
+
+	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
+		pr_info("Sending FIQ to %s CPUs:\n",
+			(include_self ? "all" : "other"));
+		smp_cross_call(to_cpumask(backtrace_mask), IPI_CPU_BACKTRACE);
+	}
+
+	/* Wait for up to 10 seconds for all CPUs to do the backtrace */
+	for (i = 0; i < 10 * 1000; i++) {
+		if (cpumask_empty(to_cpumask(backtrace_mask)))
+			break;
+		mdelay(1);
+		touch_softlockup_watchdog();
+	}
+
+	printk_nmi_backtrace_complete();
+	put_cpu();
+}
+
+void ipi_cpu_backtrace(struct pt_regs *regs)
+{
+	int cpu = smp_processor_id();
+
+	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
+		printk_nmi_backtrace_this_cpu_begin();
+		pr_warn("FIQ backtrace for cpu %d\n", cpu);
+		if (regs != NULL)
+			show_regs(regs);
+		else
+			dump_stack();
+		printk_nmi_backtrace_this_cpu_end();
+
+		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
+	}
+}
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index b35e220ae1b1..1836415b8a5c 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -483,6 +483,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
 #ifdef CONFIG_ARM_GIC
 	gic_handle_fiq_ipi();
 #endif
+#ifdef CONFIG_SMP
+	ipi_cpu_backtrace(regs);
+#endif
 
 	nmi_exit();
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions
  2015-04-07 15:38   ` [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions Daniel Thompson
@ 2015-04-07 16:19     ` Steven Rostedt
  2015-04-07 16:37       ` Borislav Petkov
  0 siblings, 1 reply; 40+ messages in thread
From: Steven Rostedt @ 2015-04-07 16:19 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Thomas Gleixner, Jason Cooper, Russell King, Will Deacon,
	Catalin Marinas, Marc Zyngier, Stephen Boyd, John Stultz,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander, H. Peter Anvin, x86

On Tue,  7 Apr 2015 16:38:02 +0100
Daniel Thompson <daniel.thompson@linaro.org> wrote:

  
>  void arch_trigger_all_cpu_backtrace(bool include_self)
>  {
> -	struct nmi_seq_buf *s;
> -	int len;
> -	int cpu;
>  	int i;
>  	int this_cpu = get_cpu();
>  
> -	if (test_and_set_bit(0, &backtrace_flag)) {
> +	if (0 != printk_nmi_backtrace_prepare()) {

Not sure what the others think, but I hate this polish notation for
compares. One does not say "if zero does not equal
printk_nmi_backtrace_prepare()", they say "if
printk_nmi_backtrace_prepare() does not return zero".

And the reason for polish notation is to prevent the:

	if (x = 0)

mistake. Which gcc warns about anyway. Also, this doesn't even pertain
to this code because:

	if (printk_nmi_backtrace_prepare() = 0)

would fail to compile.

-- Steve


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions
  2015-04-07 16:19     ` Steven Rostedt
@ 2015-04-07 16:37       ` Borislav Petkov
  2015-04-07 16:43         ` Steven Rostedt
  0 siblings, 1 reply; 40+ messages in thread
From: Borislav Petkov @ 2015-04-07 16:37 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Daniel Thompson, Thomas Gleixner, Jason Cooper, Russell King,
	Will Deacon, Catalin Marinas, Marc Zyngier, Stephen Boyd,
	John Stultz, linux-kernel, linux-arm-kernel, patches,
	linaro-kernel, Sumit Semwal, Dirk Behme, Daniel Drake,
	Dmitry Pervushin, Tim Sander, H. Peter Anvin, x86

On Tue, Apr 07, 2015 at 12:19:42PM -0400, Steven Rostedt wrote:
> Not sure what the others think, but I hate this polish notation for
> compares. One does not say "if zero does not equal
> printk_nmi_backtrace_prepare()", they say "if
> printk_nmi_backtrace_prepare() does not return zero".
> 
> And the reason for polish notation is to prevent the:
> 
> 	if (x = 0)
> 
> mistake. Which gcc warns about anyway. Also, this doesn't even pertain
> to this code because:
> 
> 	if (printk_nmi_backtrace_prepare() = 0)
> 
> would fail to compile.

I would simply say:

	err = printk_nmi_backtrace_prepare();
	if (err)

like sane kernel code does.

Besides, there's not a lot of such comparisons in the kernel anyway:

$ git grep -E "if\s+\(+[0-9]+\!?=.*"
drivers/ide/au1xxx-ide.c:246:                   if (1==i)

but my regex doesn't cover all possible variants, just the single-line
ones.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions
  2015-04-07 16:37       ` Borislav Petkov
@ 2015-04-07 16:43         ` Steven Rostedt
  2015-04-08 12:08           ` Daniel Thompson
  0 siblings, 1 reply; 40+ messages in thread
From: Steven Rostedt @ 2015-04-07 16:43 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Daniel Thompson, Thomas Gleixner, Jason Cooper, Russell King,
	Will Deacon, Catalin Marinas, Marc Zyngier, Stephen Boyd,
	John Stultz, linux-kernel, linux-arm-kernel, patches,
	linaro-kernel, Sumit Semwal, Dirk Behme, Daniel Drake,
	Dmitry Pervushin, Tim Sander, H. Peter Anvin, x86

On Tue, 7 Apr 2015 18:37:46 +0200
Borislav Petkov <bp@alien8.de> wrote:


> I would simply say:
> 
> 	err = printk_nmi_backtrace_prepare();
> 	if (err)
> 
> like sane kernel code does.
> 

Yes, that is even the better solution.

-- Steve

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions
  2015-04-07 16:43         ` Steven Rostedt
@ 2015-04-08 12:08           ` Daniel Thompson
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-08 12:08 UTC (permalink / raw)
  To: Steven Rostedt, Borislav Petkov
  Cc: Thomas Gleixner, Jason Cooper, Russell King, Will Deacon,
	Catalin Marinas, Marc Zyngier, Stephen Boyd, John Stultz,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander, H. Peter Anvin, x86

On 07/04/15 17:43, Steven Rostedt wrote:
> On Tue, 7 Apr 2015 18:37:46 +0200
> Borislav Petkov <bp@alien8.de> wrote:
>
>
>> I would simply say:
>>
>> 	err = printk_nmi_backtrace_prepare();
>> 	if (err)
>>
>> like sane kernel code does.
>>
>
> Yes, that is even the better solution.

Ok. I'll do this.

The constant on the left habit is so ingrained for me I often don't 
notice it. I agree with Steven that modern compiler warnings render it 
an obsolete habit...


Thanks.


Daniel.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace
  2015-03-24 16:53 [PATCH 4.0-rc5 v19 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                   ` (6 preceding siblings ...)
  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-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
                     ` (7 more replies)
  7 siblings, 8 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Jason/Thomas:
  Any chance of taking the first five of these patches via the irqchip
  route? The x86 patch has an ack from Ingo, printk has no explicit
  maintainer and I've done plenty of bisectability tests on the patchset
  so leaving the last patch for the next dev. cycle should be no
  trouble.

This patchset modifies the GIC driver to allow it, on supported
platforms, to route IPI interrupts to FIQ. It then uses this
feature to implement arch_trigger_all_cpu_backtrace for arm.
In order to neatly bring in the changes for the arm we also rearrange
some of the existing x86 NMI code to make it architecture neutral.

The patchset http://thread.gmane.org/gmane.linux.kernel/1897765 , which
makes sched_clock() NMI/FIQ-safe, is a prerequisite for the sixth and
final patch in the series. It is already included in tip.git
(timers/core).

The patches have been runtime tested on two systems capable of
supporting FIQ (Freescale i.MX6 and STiH416) and two that do not
(vexpress-a9 and Qualcomm Snapdragon 600), the changes to the x86
logic were tested on qemu and all patches have been compile tested
on x86, arm and arm64.

Note: On platforms not capable of supporting FIQ, the IPI to generate a
      backtrace will fall back to using IRQ for propagation instead.
      The backtrace logic contains a timeout to we will not permanently
      wedge the requesting CPU if other CPUs are not responsive.

v20:

* Fixed bad coding style (removed "if (0 != func())") (Steven Rostedt
  and Borislav Petkov).
* Rebased on v4.0-rc7.

v19:

* Remove redundant memory barrier inherited from the x86 code (Steven
  Rostedt).

v18:

* Move printk_nmi_ functions out of printk.c and into their own
  file, nmi_callback.c (Joe Perches/Steven Rostedt).

* Rename printk_nmi_ functions so their name matches their new home
  (Joe Perches)

v17:

* Rename bl_migration_lock/unlock to gic_migration_lock/unlock
  (Nicolas Pitre).

v16:

* Significant clean up of the printk patches (Thomas Gleixner).
  Replacing macros with real functions, CONFIG_ARCH_WANT_NMI_PRINTK
  -> CONFIG_PRINTK_NMI, prefixing global functions with printk_nmi,
  removing pointless exports, removing cpu_mask from the interfaces,
  removal of just-in-time initialization of trace buffers, prevented
  call sites having to save state, rolled up variable declarations
  into single lines.

* Dropped the sched_clock() patches from *this* patchset and managed
  them separately (http://thread.gmane.org/gmane.linux.kernel/1879261 ).
  The cross-dependancies between the patches are minimal; the backtrace
  code only calls sched_clock() if we are ftracing and backtracing is
  normally only triggered to report information about about a broken
  system (although users can type SysRq-l for amusement, most use it
  to find out why the system it dead).

* Squashed together the final two patches. Essentially these duplicated
  the x86 code and slavishly avoided changing it before, in the next
  patch, fixing it to work better on ARM. It seems better that the code
  just works first time!

v15:

* Added a patch to make sched_clock safe to call from NMI (Stephen
  Boyd). Note that sched_clock() is not called by the NMI handlers that
  have been added for the arm but it could be called if tools such as
  ftrace are deployed.

* Fixed some warnings picked up during bisectability testing.

v14:

* Moved a nmi_vprintk() and friends from arch/x86/kernel/apic/hw_nmi.c
  to printk.c (Steven Rostedt)

v13:

* Updated the code to print the backtrace to replicate Steven Rostedt's
  x86 work to make SysRq-l safe. This is pretty much a total rewrite of
  patches 4 and 5.

v12:

* Squash first two patches into a single one and re-describe
  (Thomas Gleixner).

* Improve description of "irqchip: gic: Make gic_raise_softirq FIQ-safe"
  (Thomas Gleixner).

v11:

* Optimized gic_raise_softirq() by replacing a register read with
  a memory read (Jason Cooper).

v10:

* Add a further patch to optimize away some of the locking on systems
  where CONFIG_BL_SWITCHER is not set (Marc Zyngier). Compiles OK with
  exynos_defconfig (which is the only defconfig to set this option).

* Whitespace fixes in patch 4. That patch previously used spaces for
  alignment of new constants but the rest of the file used tabs.

v9:

* Improved documentation and structure of initial patch (now initial
  two patches) to make gic_raise_softirq() safe to call from FIQ
  (Thomas Gleixner).

* Avoid masking interrupts during gic_raise_softirq(). The use of the
  read lock makes this redundant (because we can safely re-enter the
  function).

v8:

* Fixed build on arm64 causes by a spurious include file in irq-gic.c.

v7-2 (accidentally released twice with same number):

* Fixed boot regression on vexpress-a9 (reported by Russell King).

* Rebased on v3.18-rc3; removed one patch from set that is already
  included in mainline.

* Dropped arm64/fiq.h patch from the set (still useful but not related
  to issuing backtraces).

v7:

* Re-arranged code within the patch series to fix a regression
  introduced midway through the series and corrected by a later patch
  (testing by Olof's autobuilder). Tested offending patch in isolation
  using defconfig identified by the autobuilder.

v6:

* Renamed svc_entry's call_trace argument to just trace (example code
  from Russell King).

* Fixed mismatched ENDPROC() in __fiq_abt (example code from Russell
  King).

* Modified usr_entry to optional avoid calling into the trace code and
  used this in FIQ entry from usr path. Modified corresponding exit code
  to avoid calling into trace code and the scheduler (example code from
  Russell King).

* Ensured the default FIQ register state is restored when the default
  FIQ handler is reinstalled (example code from Russell King).

* Renamed no_fiq_insn to dfl_fiq_insn to reflect the effect of adopting
  a default FIQ handler.

* Re-instated fiq_safe_migration_lock and associated logic in
  gic_raise_softirq(). gic_raise_softirq() is called by wake_up_klogd()
  in the console unlock logic.

v5:

* Rebased on 3.17-rc4.

* Removed a spurious line from the final "glue it together" patch
  that broke the build.

v4:

* Replaced push/pop with stmfd/ldmfd respectively (review of Nicolas
  Pitre).

* Really fix bad pt_regs pointer generation in __fiq_abt.

* Remove fiq_safe_migration_lock and associated logic in
  gic_raise_softirq() (review of Russell King)

* Restructured to introduce the default FIQ handler first, before the
  new features (review of Russell King).

v3:

* Removed redundant header guards from arch/arm64/include/asm/fiq.h
  (review of Catalin Marinas).

* Moved svc_exit_via_fiq macro to entry-header.S (review of Nicolas
  Pitre).

v2:

* Restructured to sit nicely on a similar FYI patchset from Russell
  King. It now effectively replaces the work in progress final patch
  with something much more complete.

* Implemented (and tested) a Thumb-2 implementation of svc_exit_via_fiq
  (review of Nicolas Pitre)

* Dropped the GIC group 0 workaround patch. The issue of FIQ interrupts
  being acknowledged by the IRQ handler does still exist but should be
  harmless because the IRQ handler will still wind up calling
  ipi_cpu_backtrace().

* Removed any dependency on CONFIG_FIQ; all cpu backtrace effectively
  becomes a platform feature (although the use of non-maskable
  interrupts to implement it is best effort rather than guaranteed).

* Better comments highlighting usage of RAZ/WI registers (and parts of
  registers) in the GIC code.

Changes *before* v1:

* This patchset is a hugely cut-down successor to "[PATCH v11 00/19]
  arm: KGDB NMI/FIQ support". Thanks to Thomas Gleixner for suggesting
  the new structure. For historic details see:
        https://lkml.org/lkml/2014/9/2/227

* Fix bug in __fiq_abt (no longer passes a bad struct pt_regs value).
  In fixing this we also remove the useless indirection previously
  found in the fiq_handler macro.

* Make default fiq handler "always on" by migrating from fiq.c to
  traps.c and replace do_unexp_fiq with the new handler (review
  of Russell King).

* Add arm64 version of fiq.h (review of Russell King)

* Removed conditional branching and code from irq-gic.c, this is
  replaced by much simpler code that relies on the GIC specification's
  heavy use of read-as-zero/write-ignored (review of Russell King)


Daniel Thompson (6):
  irqchip: gic: Optimize locking in gic_raise_softirq
  irqchip: gic: Make gic_raise_softirq FIQ-safe
  irqchip: gic: Introduce plumbing for IPI FIQ
  printk: Simple implementation for NMI backtracing
  x86/nmi: Use common printk functions
  ARM: Add support for on-demand backtrace of other CPUs

 arch/arm/Kconfig                |   1 +
 arch/arm/include/asm/hardirq.h  |   2 +-
 arch/arm/include/asm/irq.h      |   5 +
 arch/arm/include/asm/smp.h      |   3 +
 arch/arm/kernel/smp.c           |  82 ++++++++++++++++
 arch/arm/kernel/traps.c         |   8 +-
 arch/x86/Kconfig                |   1 +
 arch/x86/kernel/apic/hw_nmi.c   | 104 ++------------------
 drivers/irqchip/irq-gic.c       | 203 +++++++++++++++++++++++++++++++++++++---
 include/linux/irqchip/arm-gic.h |   8 ++
 include/linux/printk.h          |  20 ++++
 init/Kconfig                    |   3 +
 kernel/printk/Makefile          |   1 +
 kernel/printk/nmi_backtrace.c   | 147 +++++++++++++++++++++++++++++
 14 files changed, 476 insertions(+), 112 deletions(-)
 create mode 100644 kernel/printk/nmi_backtrace.c

--
2.1.0


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc7 v20 1/6] irqchip: gic: Optimize locking in gic_raise_softirq
  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-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
                     ` (6 subsequent siblings)
  7 siblings, 1 reply; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently gic_raise_softirq() is locked using upon irq_controller_lock.
This lock is primarily used to make register read-modify-write sequences
atomic but gic_raise_softirq() uses it instead to ensure that the
big.LITTLE migration logic can figure out when it is safe to migrate
interrupts between physical cores.

This is sub-optimal in closely related ways:

1. No locking at all is required on systems where the b.L switcher is
   not configured.

2. Finer grain locking can be used on systems where the b.L switcher is
   present.

This patch resolves both of the above by introducing a separate finer
grain lock and providing conditionally compiled inlines to lock/unlock
it.

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 | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 471e1cdc1933..a181b836d5ea 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -73,6 +73,27 @@ struct gic_chip_data {
 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.
+ */
+#ifdef CONFIG_BL_SWITCHER
+static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock);
+
+static inline void gic_migration_lock(unsigned long *flags)
+{
+	raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags);
+}
+
+static inline void gic_migration_unlock(unsigned long flags)
+{
+	raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags);
+}
+#else
+static inline void gic_migration_lock(unsigned long *flags) {}
+static inline void gic_migration_unlock(unsigned long flags) {}
+#endif
+
+/*
  * The GIC mapping of CPU interfaces does not necessarily match
  * the logical CPU numbering.  Let's use a mapping as returned
  * by the GIC itself.
@@ -631,7 +652,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	int cpu;
 	unsigned long flags, map = 0;
 
-	raw_spin_lock_irqsave(&irq_controller_lock, flags);
+	gic_migration_lock(&flags);
 
 	/* Convert our logical CPU mask into a physical one. */
 	for_each_cpu(cpu, mask)
@@ -646,7 +667,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);
 
-	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
+	gic_migration_unlock(flags);
 }
 #endif
 
@@ -717,8 +738,17 @@ void gic_migrate_target(unsigned int new_cpu_id)
 
 	raw_spin_lock(&irq_controller_lock);
 
-	/* Update the target interface for this logical CPU */
+	/*
+	 * Update the target interface for this logical CPU
+	 *
+	 * From the point we release the cpu_map_migration_lock any new
+	 * SGIs will be pended on the new cpu which makes the set of SGIs
+	 * 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);
 	gic_cpu_map[cpu] = 1 << new_cpu_id;
+	raw_spin_unlock(&cpu_map_migration_lock);
 
 	/*
 	 * Find all the peripheral interrupts targetting the current
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc7 v20 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe
  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   ` [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: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
                     ` (5 subsequent siblings)
  7 siblings, 1 reply; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  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   ` [RESEND PATCH 4.0-rc7 v20 1/6] irqchip: gic: Optimize locking in gic_raise_softirq Daniel Thompson
  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 13:45     ` Marc Zyngier
  2015-04-21 14:50     ` Mark Rutland
  2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 4/6] printk: Simple implementation for NMI backtracing Daniel Thompson
                     ` (4 subsequent siblings)
  7 siblings, 2 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently it is not possible to exploit FIQ for systems with a GIC, even if
the systems are otherwise capable of it. This patch makes it possible
for IPIs to be delivered using FIQ.

To do so it modifies the register state so that normal interrupts are
placed in group 1 and specific IPIs are placed into group 0. It also
configures the controller to raise group 0 interrupts using the FIQ
signal. It provides a means for architecture code to define which IPIs
shall use FIQ and to acknowledge any IPIs that are raised.

All GIC hardware except GICv1-without-TrustZone support provides a means
to group exceptions into group 0 and group 1 but the hardware
functionality is unavailable to the kernel when a secure monitor is
present because access to the grouping registers are prohibited outside
"secure world". However when grouping is not available (or in the case
of early GICv1 implementations is very hard to configure) the code to
change groups does not deploy and all IPIs will be raised via IRQ.

It has been tested and shown working on two systems capable of
supporting grouping (Freescale i.MX6 and STiH416). It has also been
tested for boot regressions on two systems that do not support grouping
(vexpress-a9 and Qualcomm Snapdragon 600).

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>
Tested-by: Jon Medhurst <tixy@linaro.org>
---
 arch/arm/kernel/traps.c         |   5 +-
 drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
 include/linux/irqchip/arm-gic.h |   8 +++
 3 files changed, 153 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 788e23fe64d8..b35e220ae1b1 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -26,6 +26,7 @@
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/irq.h>
+#include <linux/irqchip/arm-gic.h>
 
 #include <linux/atomic.h>
 #include <asm/cacheflush.h>
@@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
 
 	nmi_enter();
 
-	/* nop. FIQ handlers for special arch/arm features can be added here. */
+#ifdef CONFIG_ARM_GIC
+	gic_handle_fiq_ipi();
+#endif
 
 	nmi_exit();
 
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 578ffc5ec087..ffd1c0fe44b2 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -39,6 +39,7 @@
 #include <linux/slab.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqchip/arm-gic.h>
+#include <linux/ratelimit.h>
 
 #include <asm/cputype.h>
 #include <asm/irq.h>
@@ -48,6 +49,10 @@
 #include "irq-gic-common.h"
 #include "irqchip.h"
 
+#ifndef SMP_IPI_FIQ_MASK
+#define SMP_IPI_FIQ_MASK 0
+#endif
+
 union gic_base {
 	void __iomem *common_base;
 	void __percpu * __iomem *percpu_base;
@@ -65,6 +70,7 @@ struct gic_chip_data {
 #endif
 	struct irq_domain *domain;
 	unsigned int gic_irqs;
+	u32 igroup0_shadow;
 #ifdef CONFIG_GIC_NON_BANKED
 	void __iomem *(*get_base)(union gic_base *);
 #endif
@@ -355,6 +361,83 @@ static struct irq_chip gic_chip = {
 	.irq_set_wake		= gic_set_wake,
 };
 
+/*
+ * Shift an interrupt between Group 0 and Group 1.
+ *
+ * In addition to changing the group we also modify the priority to
+ * match what "ARM strongly recommends" for a system where no Group 1
+ * interrupt must ever preempt a Group 0 interrupt.
+ *
+ * If is safe to call this function on systems which do not support
+ * grouping (it will have no effect).
+ */
+static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq,
+			      int group)
+{
+	void __iomem *base = gic_data_dist_base(gic);
+	unsigned int grp_reg = hwirq / 32 * 4;
+	u32 grp_mask = BIT(hwirq % 32);
+	u32 grp_val;
+
+	unsigned int pri_reg = (hwirq / 4) * 4;
+	u32 pri_mask = BIT(7 + ((hwirq % 4) * 8));
+	u32 pri_val;
+
+	/*
+	 * Systems which do not support grouping will have not have
+	 * the EnableGrp1 bit set.
+	 */
+	if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)))
+		return;
+
+	raw_spin_lock(&irq_controller_lock);
+
+	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
+	pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg);
+
+	if (group) {
+		grp_val |= grp_mask;
+		pri_val |= pri_mask;
+	} else {
+		grp_val &= ~grp_mask;
+		pri_val &= ~pri_mask;
+	}
+
+	writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg);
+	if (grp_reg == 0)
+		gic->igroup0_shadow = grp_val;
+
+	writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
+
+	raw_spin_unlock(&irq_controller_lock);
+}
+
+
+/*
+ * Fully acknowledge (both ack and eoi) any outstanding FIQ-based IPI,
+ * otherwise do nothing.
+ */
+void gic_handle_fiq_ipi(void)
+{
+	struct gic_chip_data *gic = &gic_data[0];
+	void __iomem *cpu_base = gic_data_cpu_base(gic);
+	unsigned long irqstat, irqnr;
+
+	if (WARN_ON(!in_nmi()))
+		return;
+
+	while ((1u << readl_relaxed(cpu_base + GIC_CPU_HIGHPRI)) &
+	       SMP_IPI_FIQ_MASK) {
+		irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
+		writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
+
+		irqnr = irqstat & GICC_IAR_INT_ID_MASK;
+		WARN_RATELIMIT(irqnr > 16,
+			       "Unexpected irqnr %lu (bad prioritization?)\n",
+			       irqnr);
+	}
+}
+
 void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
 {
 	if (gic_nr >= MAX_GIC_NR)
@@ -386,15 +469,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
 static void gic_cpu_if_up(void)
 {
 	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
-	u32 bypass = 0;
+	void __iomem *dist_base = gic_data_dist_base(&gic_data[0]);
+	u32 ctrl = 0;
 
 	/*
-	* Preserve bypass disable bits to be written back later
-	*/
-	bypass = readl(cpu_base + GIC_CPU_CTRL);
-	bypass &= GICC_DIS_BYPASS_MASK;
+	 * Preserve bypass disable bits to be written back later
+	 */
+	ctrl = readl(cpu_base + GIC_CPU_CTRL);
+	ctrl &= GICC_DIS_BYPASS_MASK;
 
-	writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
+	/*
+	 * If EnableGrp1 is set in the distributor then enable group 1
+	 * support for this CPU (and route group 0 interrupts to FIQ).
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL))
+		ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL |
+			GICC_ENABLE_GRP1;
+
+	writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
 }
 
 
@@ -418,7 +510,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
 
 	gic_dist_config(base, gic_irqs, NULL);
 
-	writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
+	/*
+	 * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
+	 * bit 1 ignored) depending on current mode.
+	 */
+	writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL);
+
+	/*
+	 * Set all global interrupts to be group 1 if (and only if) it
+	 * is possible to enable group 1 interrupts. This register is RAZ/WI
+	 * if not accessible or not implemented, however some GICv1 devices
+	 * do not implement the EnableGrp1 bit making it unsafe to set
+	 * this register unconditionally.
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))
+		for (i = 32; i < gic_irqs; i += 32)
+			writel_relaxed(0xffffffff,
+				       base + GIC_DIST_IGROUP + i * 4 / 32);
 }
 
 static void gic_cpu_init(struct gic_chip_data *gic)
@@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	void __iomem *base = gic_data_cpu_base(gic);
 	unsigned int cpu_mask, cpu = smp_processor_id();
 	int i;
+	unsigned long secure_irqs, secure_irq;
 
 	/*
 	 * Get what the GIC says our CPU mask is.
@@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 
 	gic_cpu_config(dist_base, NULL);
 
+	/*
+	 * If the distributor is configured to support interrupt grouping
+	 * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
+	 * to be group1 and ensure any remaining group 0 interrupts have
+	 * the right priority.
+	 */
+	if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
+		secure_irqs = SMP_IPI_FIQ_MASK;
+		writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
+		gic->igroup0_shadow = ~secure_irqs;
+		for_each_set_bit(secure_irq, &secure_irqs, 16)
+			gic_set_group_irq(gic, secure_irq, 0);
+	}
+
 	writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
 	gic_cpu_if_up();
 }
@@ -534,7 +657,8 @@ static void gic_dist_restore(unsigned int gic_nr)
 		writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
 			dist_base + GIC_DIST_ENABLE_SET + i * 4);
 
-	writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
+	writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE,
+		       dist_base + GIC_DIST_CTRL);
 }
 
 static void gic_cpu_save(unsigned int gic_nr)
@@ -662,6 +786,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 {
 	int cpu;
 	unsigned long map = 0;
+	unsigned long softint;
 
 	gic_migration_lock();
 
@@ -675,8 +800,14 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	 */
 	dmb(ishst);
 
-	/* this always happens on GIC0 */
-	writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+	/* We avoid a readl here by using the shadow copy of IGROUP[0] */
+	softint = map << 16 | irq;
+	if (gic_data[0].igroup0_shadow & BIT(irq))
+		softint |= 0x8000;
+
+	/* This always happens on GIC0 */
+	writel_relaxed(softint,
+		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
 
 	gic_migration_unlock();
 }
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 71d706d5f169..7690f70049a3 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -22,6 +22,10 @@
 #define GIC_CPU_IDENT			0xfc
 
 #define GICC_ENABLE			0x1
+#define GICC_ENABLE_GRP1		0x2
+#define GICC_ACK_CTL			0x4
+#define GICC_FIQ_EN			0x8
+#define GICC_COMMON_BPR			0x10
 #define GICC_INT_PRI_THRESHOLD		0xf0
 #define GICC_IAR_INT_ID_MASK		0x3ff
 #define GICC_INT_SPURIOUS		1023
@@ -44,6 +48,7 @@
 #define GIC_DIST_SGI_PENDING_SET	0xf20
 
 #define GICD_ENABLE			0x1
+#define GICD_ENABLE_GRP1		0x2
 #define GICD_DISABLE			0x0
 #define GICD_INT_ACTLOW_LVLTRIG		0x0
 #define GICD_INT_EN_CLR_X32		0xffffffff
@@ -121,5 +126,8 @@ static inline void __init register_routable_domain_ops
 {
 	gic_routable_irq_domain_ops = ops;
 }
+
+void gic_handle_fiq_ipi(void);
+
 #endif /* __ASSEMBLY */
 #endif
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc7 v20 4/6] printk: Simple implementation for NMI backtracing
  2015-04-10  9:51 ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (2 preceding siblings ...)
  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-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 5/6] x86/nmi: Use common printk functions Daniel Thompson
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Currently there is a quite a pile of code sitting in
arch/x86/kernel/apic/hw_nmi.c to support safe all-cpu backtracing from NMI.
The code is inaccessible to backtrace implementations for other
architectures, which is a shame because they would probably like to be
safe too.

Copy this code into printk, reworking it a little as we do so to make
it easier to exploit as library code.

We'll port the x86 NMI backtrace logic to it in a later patch.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/printk.h        |  20 ++++++
 init/Kconfig                  |   3 +
 kernel/printk/Makefile        |   1 +
 kernel/printk/nmi_backtrace.c | 147 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 171 insertions(+)
 create mode 100644 kernel/printk/nmi_backtrace.c

diff --git a/include/linux/printk.h b/include/linux/printk.h
index baa3f97d8ce8..44bb85ad1f62 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -228,6 +228,26 @@ static inline void show_regs_print_info(const char *log_lvl)
 }
 #endif
 
+#ifdef CONFIG_PRINTK_NMI_BACKTRACE
+/*
+ * printk_nmi_backtrace_prepare/complete are called to prepare the
+ * system for some or all cores to issue trace from NMI.
+ * printk_nmi_backtrace_complete will print buffered output and cannot
+ * (safely) be called from NMI.
+ */
+extern int printk_nmi_backtrace_prepare(void);
+extern void printk_nmi_backtrace_complete(void);
+
+/*
+ * printk_nmi_backtrace_this_cpu_begin/end are used divert/restore printk
+ * on this cpu. The result is the output of printk() (by this CPU) will be
+ * stored in temporary buffers for later printing by
+ * printk_nmi_backtrace_complete.
+ */
+extern void printk_nmi_backtrace_this_cpu_begin(void);
+extern void printk_nmi_backtrace_this_cpu_end(void);
+#endif
+
 extern asmlinkage void dump_stack(void) __cold;
 
 #ifndef pr_fmt
diff --git a/init/Kconfig b/init/Kconfig
index f5dbc6d4261b..0107e9b4d2cf 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1421,6 +1421,9 @@ config PRINTK
 	  very difficult to diagnose system problems, saying N here is
 	  strongly discouraged.
 
+config PRINTK_NMI_BACKTRACE
+	bool
+
 config BUG
 	bool "BUG() support" if EXPERT
 	default y
diff --git a/kernel/printk/Makefile b/kernel/printk/Makefile
index 85405bdcf2b3..1849b001384a 100644
--- a/kernel/printk/Makefile
+++ b/kernel/printk/Makefile
@@ -1,2 +1,3 @@
 obj-y	= printk.o
+obj-$(CONFIG_PRINTK_NMI_BACKTRACE)	+= nmi_backtrace.o
 obj-$(CONFIG_A11Y_BRAILLE_CONSOLE)	+= braille.o
diff --git a/kernel/printk/nmi_backtrace.c b/kernel/printk/nmi_backtrace.c
new file mode 100644
index 000000000000..f24761262756
--- /dev/null
+++ b/kernel/printk/nmi_backtrace.c
@@ -0,0 +1,147 @@
+#include <linux/kernel.h>
+#include <linux/seq_buf.h>
+
+#define NMI_BUF_SIZE		4096
+
+struct nmi_seq_buf {
+	unsigned char		buffer[NMI_BUF_SIZE];
+	struct seq_buf		seq;
+};
+
+/* Safe printing in NMI context */
+static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq);
+
+static DEFINE_PER_CPU(printk_func_t, nmi_print_saved_print_func);
+
+/* "in progress" flag of NMI printing */
+static unsigned long nmi_print_flag;
+
+static int __init printk_nmi_backtrace_init(void)
+{
+	struct nmi_seq_buf *s;
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		s = &per_cpu(nmi_print_seq, cpu);
+		seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE);
+	}
+
+	return 0;
+}
+pure_initcall(printk_nmi_backtrace_init);
+
+/*
+ * It is not safe to call printk() directly from NMI handlers.
+ * It may be fine if the NMI detected a lock up and we have no choice
+ * but to do so, but doing a NMI on all other CPUs to get a back trace
+ * can be done with a sysrq-l. We don't want that to lock up, which
+ * can happen if the NMI interrupts a printk in progress.
+ *
+ * Instead, we redirect the vprintk() to this nmi_vprintk() that writes
+ * the content into a per cpu seq_buf buffer. Then when the NMIs are
+ * all done, we can safely dump the contents of the seq_buf to a printk()
+ * from a non NMI context.
+ *
+ * This is not a generic printk() implementation and must be used with
+ * great care. In particular there is a static limit on the quantity of
+ * data that may be emitted during NMI, only one client can be active at
+ * one time (arbitrated by the return value of printk_nmi_begin() and
+ * it is required that something at task or interrupt context be scheduled
+ * to issue the output.
+ */
+static int nmi_vprintk(const char *fmt, va_list args)
+{
+	struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
+	unsigned int len = seq_buf_used(&s->seq);
+
+	seq_buf_vprintf(&s->seq, fmt, args);
+	return seq_buf_used(&s->seq) - len;
+}
+
+/*
+ * Reserve the NMI printk mechanism. Return an error if some other component
+ * is already using it.
+ */
+int printk_nmi_backtrace_prepare(void)
+{
+	if (test_and_set_bit(0, &nmi_print_flag)) {
+		/*
+		 * If something is already using the NMI print facility we
+		 * can't allow a second one...
+		 */
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+static void print_seq_line(struct nmi_seq_buf *s, int start, int end)
+{
+	const char *buf = s->buffer + start;
+
+	printk("%.*s", (end - start) + 1, buf);
+}
+
+void printk_nmi_backtrace_complete(void)
+{
+	struct nmi_seq_buf *s;
+	int len, cpu, i, last_i;
+
+	/*
+	 * Now that all the NMIs have triggered, we can dump out their
+	 * back traces safely to the console.
+	 */
+	for_each_possible_cpu(cpu) {
+		s = &per_cpu(nmi_print_seq, cpu);
+		last_i = 0;
+
+		len = seq_buf_used(&s->seq);
+		if (!len)
+			continue;
+
+		/* Print line by line. */
+		for (i = 0; i < len; i++) {
+			if (s->buffer[i] == '\n') {
+				print_seq_line(s, last_i, i);
+				last_i = i + 1;
+			}
+		}
+		/* Check if there was a partial line. */
+		if (last_i < len) {
+			print_seq_line(s, last_i, len - 1);
+			pr_cont("\n");
+		}
+
+		/* Wipe out the buffer ready for the next time around. */
+		seq_buf_clear(&s->seq);
+	}
+
+	clear_bit(0, &nmi_print_flag);
+}
+
+void printk_nmi_backtrace_this_cpu_begin(void)
+{
+	/*
+	 * Detect double-begins and report them. This code is unsafe (because
+	 * it will print from NMI) but things are pretty badly damaged if the
+	 * NMI re-enters and is somehow granted permission to use NMI printk,
+	 * so how much worse can it get? Also since this code interferes with
+	 * the operation of printk it is unlikely that any consequential
+	 * failures will be able to log anything making this our last
+	 * opportunity to tell anyone that something is wrong.
+	 */
+	if (this_cpu_read(nmi_print_saved_print_func)) {
+		this_cpu_write(printk_func,
+			       this_cpu_read(nmi_print_saved_print_func));
+		BUG();
+	}
+
+	this_cpu_write(nmi_print_saved_print_func, this_cpu_read(printk_func));
+	this_cpu_write(printk_func, nmi_vprintk);
+}
+
+void printk_nmi_backtrace_this_cpu_end(void)
+{
+	this_cpu_write(printk_func, this_cpu_read(nmi_print_saved_print_func));
+	this_cpu_write(nmi_print_saved_print_func, NULL);
+}
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc7 v20 5/6] x86/nmi: Use common printk functions
  2015-04-10  9:51 ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (3 preceding siblings ...)
  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 6/6] ARM: Add support for on-demand backtrace of other CPUs Daniel Thompson
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander, H. Peter Anvin, x86

Much of the code sitting in arch/x86/kernel/apic/hw_nmi.c to support safe
all-cpu backtracing from NMI has been copied to printk.c to make it
accessible to other architectures.

Port the x86 NMI backtrace to the generic code.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/Kconfig              |   1 +
 arch/x86/kernel/apic/hw_nmi.c | 104 ++++--------------------------------------
 2 files changed, 10 insertions(+), 95 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b7d31ca55187..a1a54570f2d0 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -141,6 +141,7 @@ config X86
 	select ACPI_LEGACY_TABLES_LOOKUP if ACPI
 	select X86_FEATURE_NAMES if PROC_FS
 	select SRCU
+	select PRINTK_NMI_BACKTRACE if X86_LOCAL_APIC
 
 config INSTRUCTION_DECODER
 	def_bool y
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 6873ab925d00..7a682beac3a0 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -30,40 +30,17 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh)
 #ifdef arch_trigger_all_cpu_backtrace
 /* For reliability, we're prepared to waste bits here. */
 static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
-static cpumask_t printtrace_mask;
-
-#define NMI_BUF_SIZE		4096
-
-struct nmi_seq_buf {
-	unsigned char		buffer[NMI_BUF_SIZE];
-	struct seq_buf		seq;
-};
-
-/* Safe printing in NMI context */
-static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq);
-
-/* "in progress" flag of arch_trigger_all_cpu_backtrace */
-static unsigned long backtrace_flag;
-
-static void print_seq_line(struct nmi_seq_buf *s, int start, int end)
-{
-	const char *buf = s->buffer + start;
-
-	printk("%.*s", (end - start) + 1, buf);
-}
 
 void arch_trigger_all_cpu_backtrace(bool include_self)
 {
-	struct nmi_seq_buf *s;
-	int len;
-	int cpu;
-	int i;
+	int err, i;
 	int this_cpu = get_cpu();
 
-	if (test_and_set_bit(0, &backtrace_flag)) {
+	err = printk_nmi_backtrace_prepare();
+	if (err) {
 		/*
-		 * If there is already a trigger_all_cpu_backtrace() in progress
-		 * (backtrace_flag == 1), don't output double cpu dump infos.
+		 * If there is already an nmi printk sequence in
+		 * progress then just give up...
 		 */
 		put_cpu();
 		return;
@@ -73,16 +50,6 @@ void arch_trigger_all_cpu_backtrace(bool include_self)
 	if (!include_self)
 		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
 
-	cpumask_copy(&printtrace_mask, to_cpumask(backtrace_mask));
-	/*
-	 * Set up per_cpu seq_buf buffers that the NMIs running on the other
-	 * CPUs will write to.
-	 */
-	for_each_cpu(cpu, to_cpumask(backtrace_mask)) {
-		s = &per_cpu(nmi_print_seq, cpu);
-		seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE);
-	}
-
 	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
 		pr_info("sending NMI to %s CPUs:\n",
 			(include_self ? "all" : "other"));
@@ -97,73 +64,20 @@ void arch_trigger_all_cpu_backtrace(bool include_self)
 		touch_softlockup_watchdog();
 	}
 
-	/*
-	 * Now that all the NMIs have triggered, we can dump out their
-	 * back traces safely to the console.
-	 */
-	for_each_cpu(cpu, &printtrace_mask) {
-		int last_i = 0;
-
-		s = &per_cpu(nmi_print_seq, cpu);
-		len = seq_buf_used(&s->seq);
-		if (!len)
-			continue;
-
-		/* Print line by line. */
-		for (i = 0; i < len; i++) {
-			if (s->buffer[i] == '\n') {
-				print_seq_line(s, last_i, i);
-				last_i = i + 1;
-			}
-		}
-		/* Check if there was a partial line. */
-		if (last_i < len) {
-			print_seq_line(s, last_i, len - 1);
-			pr_cont("\n");
-		}
-	}
-
-	clear_bit(0, &backtrace_flag);
-	smp_mb__after_atomic();
+	printk_nmi_backtrace_complete();
 	put_cpu();
 }
 
-/*
- * It is not safe to call printk() directly from NMI handlers.
- * It may be fine if the NMI detected a lock up and we have no choice
- * but to do so, but doing a NMI on all other CPUs to get a back trace
- * can be done with a sysrq-l. We don't want that to lock up, which
- * can happen if the NMI interrupts a printk in progress.
- *
- * Instead, we redirect the vprintk() to this nmi_vprintk() that writes
- * the content into a per cpu seq_buf buffer. Then when the NMIs are
- * all done, we can safely dump the contents of the seq_buf to a printk()
- * from a non NMI context.
- */
-static int nmi_vprintk(const char *fmt, va_list args)
-{
-	struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq);
-	unsigned int len = seq_buf_used(&s->seq);
-
-	seq_buf_vprintf(&s->seq, fmt, args);
-	return seq_buf_used(&s->seq) - len;
-}
-
 static int
 arch_trigger_all_cpu_backtrace_handler(unsigned int cmd, struct pt_regs *regs)
 {
-	int cpu;
-
-	cpu = smp_processor_id();
+	int cpu = smp_processor_id();
 
 	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
-		printk_func_t printk_func_save = this_cpu_read(printk_func);
-
-		/* Replace printk to write into the NMI seq */
-		this_cpu_write(printk_func, nmi_vprintk);
+		printk_nmi_backtrace_this_cpu_begin();
 		printk(KERN_WARNING "NMI backtrace for cpu %d\n", cpu);
 		show_regs(regs);
-		this_cpu_write(printk_func, printk_func_save);
+		printk_nmi_backtrace_this_cpu_end();
 
 		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
 		return NMI_HANDLED;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [RESEND PATCH 4.0-rc7 v20 6/6] ARM: Add support for on-demand backtrace of other CPUs
  2015-04-10  9:51 ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (4 preceding siblings ...)
  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 10:47   ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
  2015-04-21 12:46   ` Thomas Gleixner
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10  9:51 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Daniel Thompson, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

Replicate the x86 code to trigger a backtrace using an NMI and hook
it up to IPI on ARM.

The code differs slightly from the code on x86 because, on ARM, we do
now know at compile time whether a platform is capable of supporting FIQ.
We must avoid using an IPI to request a backtrace from the CPU on which
the backtrace was requested if interrupts are disabled and fall back to
generating it directly.

In addition the implementation of arch_trigger_all_cpu_backtrace() the
patch also includes a few small items of plumbing that must be hooked
up for the new code to work.

Credit:
  Russell King provided the initial prototype implementing this feature
  for ARM. Today the patch has been reworked and, mostly, rewriten to
  keep it aligned with x86. However this patch does still include some
  code from Russell's original prototype.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 arch/arm/Kconfig               |  1 +
 arch/arm/include/asm/hardirq.h |  2 +-
 arch/arm/include/asm/irq.h     |  5 +++
 arch/arm/include/asm/smp.h     |  3 ++
 arch/arm/kernel/smp.c          | 82 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/traps.c        |  3 ++
 6 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index cf4c0c99aa25..6f412a347cc1 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -76,6 +76,7 @@ config ARM
 	select OLD_SIGACTION
 	select OLD_SIGSUSPEND3
 	select PERF_USE_VMALLOC
+	select PRINTK_NMI_BACKTRACE
 	select RTC_LIB
 	select SYS_SUPPORTS_APM_EMULATION
 	# Above selects are sorted alphabetically; please add new ones
diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index fe3ea776dc34..5df33e30ae1b 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -5,7 +5,7 @@
 #include <linux/threads.h>
 #include <asm/irq.h>
 
-#define NR_IPI	8
+#define NR_IPI	9
 
 typedef struct {
 	unsigned int __softirq_pending;
diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index 53c15dec7af6..be1d07d59ee9 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -35,6 +35,11 @@ extern void (*handle_arch_irq)(struct pt_regs *);
 extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
 #endif
 
+#ifdef CONFIG_SMP
+extern void arch_trigger_all_cpu_backtrace(bool);
+#define arch_trigger_all_cpu_backtrace(x) arch_trigger_all_cpu_backtrace(x)
+#endif
+
 #endif
 
 #endif
diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 18f5a554134f..b076584ac0fa 100644
--- a/arch/arm/include/asm/smp.h
+++ b/arch/arm/include/asm/smp.h
@@ -18,6 +18,8 @@
 # error "<asm/smp.h> included in non-SMP build"
 #endif
 
+#define SMP_IPI_FIQ_MASK 0x0100
+
 #define raw_smp_processor_id() (current_thread_info()->cpu)
 
 struct seq_file;
@@ -79,6 +81,7 @@ extern void arch_send_call_function_single_ipi(int cpu);
 extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
 extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);
 
+extern void ipi_cpu_backtrace(struct pt_regs *regs);
 extern int register_ipi_completion(struct completion *completion, int cpu);
 
 struct smp_operations {
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 86ef244c5a24..e00514421078 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -26,6 +26,7 @@
 #include <linux/completion.h>
 #include <linux/cpufreq.h>
 #include <linux/irq_work.h>
+#include <linux/seq_buf.h>
 
 #include <linux/atomic.h>
 #include <asm/smp.h>
@@ -72,6 +73,7 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_IRQ_WORK,
 	IPI_COMPLETION,
+	IPI_CPU_BACKTRACE,
 };
 
 static DECLARE_COMPLETION(cpu_running);
@@ -456,6 +458,7 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
 	S(IPI_CPU_STOP, "CPU stop interrupts"),
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
 	S(IPI_COMPLETION, "completion interrupts"),
+	S(IPI_CPU_BACKTRACE, "backtrace interrupts"),
 };
 
 static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
@@ -570,6 +573,8 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	unsigned int cpu = smp_processor_id();
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
+	BUILD_BUG_ON(SMP_IPI_FIQ_MASK != BIT(IPI_CPU_BACKTRACE));
+
 	if ((unsigned)ipinr < NR_IPI) {
 		trace_ipi_entry(ipi_types[ipinr]);
 		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
@@ -623,6 +628,12 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		irq_exit();
 		break;
 
+	case IPI_CPU_BACKTRACE:
+		irq_enter();
+		ipi_cpu_backtrace(regs);
+		irq_exit();
+		break;
+
 	default:
 		pr_crit("CPU%u: Unknown IPI message 0x%x\n",
 		        cpu, ipinr);
@@ -717,3 +728,74 @@ static int __init register_cpufreq_notifier(void)
 core_initcall(register_cpufreq_notifier);
 
 #endif
+
+/* For reliability, we're prepared to waste bits here. */
+static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
+
+void arch_trigger_all_cpu_backtrace(bool include_self)
+{
+	int err, i;
+	int this_cpu = get_cpu();
+
+	err = printk_nmi_backtrace_prepare();
+	if (err) {
+		/*
+		 * If there is already an nmi printk sequence in
+		 * progress then just give up...
+		 */
+		put_cpu();
+		return;
+	}
+
+	cpumask_copy(to_cpumask(backtrace_mask), cpu_online_mask);
+
+	/*
+	 * If irqs are disabled on the current processor and
+	 * IPI_CPU_BACKTRACE is delivered using IRQ then we aren't be able to
+	 * react to IPI_CPU_BACKTRACE until we leave this function. This
+	 * would force us to get stuck and, eventually, timeout. We avoid
+	 * the timeout (and the resulting failure to print useful information)
+	 * by calling the backtrace logic directly whenever irqs are disabled.
+	 */
+	if (include_self && irqs_disabled()) {
+		ipi_cpu_backtrace(in_interrupt() ? get_irq_regs() : NULL);
+		include_self = false;
+	}
+
+	if (!include_self)
+		cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask));
+
+	if (!cpumask_empty(to_cpumask(backtrace_mask))) {
+		pr_info("Sending FIQ to %s CPUs:\n",
+			(include_self ? "all" : "other"));
+		smp_cross_call(to_cpumask(backtrace_mask), IPI_CPU_BACKTRACE);
+	}
+
+	/* Wait for up to 10 seconds for all CPUs to do the backtrace */
+	for (i = 0; i < 10 * 1000; i++) {
+		if (cpumask_empty(to_cpumask(backtrace_mask)))
+			break;
+		mdelay(1);
+		touch_softlockup_watchdog();
+	}
+
+	printk_nmi_backtrace_complete();
+	put_cpu();
+}
+
+void ipi_cpu_backtrace(struct pt_regs *regs)
+{
+	int cpu = smp_processor_id();
+
+	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
+		printk_nmi_backtrace_this_cpu_begin();
+		pr_warn("FIQ backtrace for cpu %d\n", cpu);
+		if (regs != NULL)
+			show_regs(regs);
+		else
+			dump_stack();
+		printk_nmi_backtrace_this_cpu_end();
+
+		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
+	}
+}
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index b35e220ae1b1..1836415b8a5c 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -483,6 +483,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
 #ifdef CONFIG_ARM_GIC
 	gic_handle_fiq_ipi();
 #endif
+#ifdef CONFIG_SMP
+	ipi_cpu_backtrace(regs);
+#endif
 
 	nmi_exit();
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace
  2015-04-10  9:51 ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (5 preceding siblings ...)
  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 10:47   ` Daniel Thompson
  2015-04-21 12:46   ` Thomas Gleixner
  7 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-10 10:47 UTC (permalink / raw)
  To: Thomas Gleixner, Jason Cooper
  Cc: Russell King, Will Deacon, Catalin Marinas, Marc Zyngier,
	Stephen Boyd, John Stultz, Steven Rostedt, linux-kernel,
	linux-arm-kernel, patches, linaro-kernel, Sumit Semwal,
	Dirk Behme, Daniel Drake, Dmitry Pervushin, Tim Sander

On 10/04/15 10:51, Daniel Thompson wrote:
> ...
> v20:
>
> * Fixed bad coding style (removed "if (0 != func())") (Steven Rostedt
>    and Borislav Petkov).
> * Rebased on v4.0-rc7.

I just spotted I incorrectly tagged this with RESEND. That was a mistake 
on my part (forgot to remove it when I bumped the revision to v20). Sorry.

Today's mail is the first time I have posted v20.


Daniel.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace
  2015-04-10  9:51 ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
                     ` (6 preceding siblings ...)
  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-21 12:46   ` Thomas Gleixner
  2015-04-21 13:08     ` Marc Zyngier
  7 siblings, 1 reply; 40+ messages in thread
From: Thomas Gleixner @ 2015-04-21 12:46 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Jason Cooper, Russell King, Will Deacon, Catalin Marinas,
	Marc Zyngier, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

On Fri, 10 Apr 2015, Daniel Thompson wrote:

> Jason/Thomas:
>   Any chance of taking the first five of these patches via the irqchip
>   route? The x86 patch has an ack from Ingo, printk has no explicit
>   maintainer and I've done plenty of bisectability tests on the patchset
>   so leaving the last patch for the next dev. cycle should be no
>   trouble.

First of all I need an acked/reviewed from Marc or Russell,
preferrably both of them.

Once that is sorted out, I can move that lot through a seperate branch.

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 1/6] irqchip: gic: Optimize locking in gic_raise_softirq
  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-21 12:51     ` Marc Zyngier
  0 siblings, 0 replies; 40+ messages in thread
From: Marc Zyngier @ 2015-04-21 12:51 UTC (permalink / raw)
  To: Daniel Thompson, Thomas Gleixner, Jason Cooper
  Cc: Russell King, Will Deacon, Catalin Marinas, Stephen Boyd,
	John Stultz, Steven Rostedt, linux-kernel, linux-arm-kernel,
	patches, linaro-kernel, Sumit Semwal, Dirk Behme, Daniel Drake,
	Dmitry Pervushin, Tim Sander

On 10/04/15 10:51, Daniel Thompson wrote:
> Currently gic_raise_softirq() is locked using upon irq_controller_lock.
> This lock is primarily used to make register read-modify-write sequences
> atomic but gic_raise_softirq() uses it instead to ensure that the
> big.LITTLE migration logic can figure out when it is safe to migrate
> interrupts between physical cores.
> 
> This is sub-optimal in closely related ways:
> 
> 1. No locking at all is required on systems where the b.L switcher is
>    not configured.
> 
> 2. Finer grain locking can be used on systems where the b.L switcher is
>    present.
> 
> This patch resolves both of the above by introducing a separate finer
> grain lock and providing conditionally compiled inlines to lock/unlock
> it.
> 
> 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>

Looks good to me.

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 2/6] irqchip: gic: Make gic_raise_softirq FIQ-safe
  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-21 12:54     ` Marc Zyngier
  0 siblings, 0 replies; 40+ messages in thread
From: Marc Zyngier @ 2015-04-21 12:54 UTC (permalink / raw)
  To: Daniel Thompson, Thomas Gleixner, Jason Cooper
  Cc: Russell King, Will Deacon, Catalin Marinas, Stephen Boyd,
	John Stultz, Steven Rostedt, linux-kernel, linux-arm-kernel,
	patches, linaro-kernel, Sumit Semwal, Dirk Behme, Daniel Drake,
	Dmitry Pervushin, Tim Sander

On 10/04/15 10:51, Daniel Thompson wrote:
> 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>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace
  2015-04-21 12:46   ` Thomas Gleixner
@ 2015-04-21 13:08     ` Marc Zyngier
  0 siblings, 0 replies; 40+ messages in thread
From: Marc Zyngier @ 2015-04-21 13:08 UTC (permalink / raw)
  To: Thomas Gleixner, Daniel Thompson
  Cc: linaro-kernel, Russell King, Jason Cooper, patches, Stephen Boyd,
	Will Deacon, linux-kernel, Steven Rostedt, Daniel Drake,
	Dmitry Pervushin, Dirk Behme, John Stultz, Tim Sander,
	Catalin Marinas, Sumit Semwal, linux-arm-kernel

On 21/04/15 13:46, Thomas Gleixner wrote:
> On Fri, 10 Apr 2015, Daniel Thompson wrote:
> 
>> Jason/Thomas:
>>   Any chance of taking the first five of these patches via the irqchip
>>   route? The x86 patch has an ack from Ingo, printk has no explicit
>>   maintainer and I've done plenty of bisectability tests on the patchset
>>   so leaving the last patch for the next dev. cycle should be no
>>   trouble.
> 
> First of all I need an acked/reviewed from Marc or Russell,
> preferrably both of them.

I'm on it.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ Daniel Thompson
@ 2015-04-21 13:45     ` Marc Zyngier
  2015-04-21 21:03       ` Daniel Thompson
  2015-04-21 14:50     ` Mark Rutland
  1 sibling, 1 reply; 40+ messages in thread
From: Marc Zyngier @ 2015-04-21 13:45 UTC (permalink / raw)
  To: Daniel Thompson, Thomas Gleixner, Jason Cooper
  Cc: Russell King, Will Deacon, Catalin Marinas, Stephen Boyd,
	John Stultz, Steven Rostedt, linux-kernel, linux-arm-kernel,
	patches, linaro-kernel, Sumit Semwal, Dirk Behme, Daniel Drake,
	Dmitry Pervushin, Tim Sander

On 10/04/15 10:51, Daniel Thompson wrote:
> Currently it is not possible to exploit FIQ for systems with a GIC, even if
> the systems are otherwise capable of it. This patch makes it possible
> for IPIs to be delivered using FIQ.
> 
> To do so it modifies the register state so that normal interrupts are
> placed in group 1 and specific IPIs are placed into group 0. It also
> configures the controller to raise group 0 interrupts using the FIQ
> signal. It provides a means for architecture code to define which IPIs
> shall use FIQ and to acknowledge any IPIs that are raised.
> 
> All GIC hardware except GICv1-without-TrustZone support provides a means
> to group exceptions into group 0 and group 1 but the hardware
> functionality is unavailable to the kernel when a secure monitor is
> present because access to the grouping registers are prohibited outside
> "secure world". However when grouping is not available (or in the case
> of early GICv1 implementations is very hard to configure) the code to
> change groups does not deploy and all IPIs will be raised via IRQ.
> 
> It has been tested and shown working on two systems capable of
> supporting grouping (Freescale i.MX6 and STiH416). It has also been
> tested for boot regressions on two systems that do not support grouping
> (vexpress-a9 and Qualcomm Snapdragon 600).
> 
> 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>
> Tested-by: Jon Medhurst <tixy@linaro.org>
> ---
>  arch/arm/kernel/traps.c         |   5 +-
>  drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
>  include/linux/irqchip/arm-gic.h |   8 +++
>  3 files changed, 153 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 788e23fe64d8..b35e220ae1b1 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -26,6 +26,7 @@
>  #include <linux/init.h>
>  #include <linux/sched.h>
>  #include <linux/irq.h>
> +#include <linux/irqchip/arm-gic.h>
> 
>  #include <linux/atomic.h>
>  #include <asm/cacheflush.h>
> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
> 
>         nmi_enter();
> 
> -       /* nop. FIQ handlers for special arch/arm features can be added here. */
> +#ifdef CONFIG_ARM_GIC
> +       gic_handle_fiq_ipi();
> +#endif

This hunk is what irritates me. It creates a hard dependency between
core ARM code and the GIC, and I don't really see how this works with
multiplatform, where the interrupt controller is not necessarily a GIC.
In that case, you will die a horrible death.

Why can't we just call handle_arch_irq(), and let the normal handler do
its thing? You can have a "if (in_nmi())" in there, and call your FIQ
function. It would at least save us the above problem.

> 
>         nmi_exit();
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 578ffc5ec087..ffd1c0fe44b2 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -39,6 +39,7 @@
>  #include <linux/slab.h>
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/irqchip/arm-gic.h>
> +#include <linux/ratelimit.h>
> 
>  #include <asm/cputype.h>
>  #include <asm/irq.h>
> @@ -48,6 +49,10 @@
>  #include "irq-gic-common.h"
>  #include "irqchip.h"
> 
> +#ifndef SMP_IPI_FIQ_MASK
> +#define SMP_IPI_FIQ_MASK 0
> +#endif
> +
>  union gic_base {
>         void __iomem *common_base;
>         void __percpu * __iomem *percpu_base;
> @@ -65,6 +70,7 @@ struct gic_chip_data {
>  #endif
>         struct irq_domain *domain;
>         unsigned int gic_irqs;
> +       u32 igroup0_shadow;
>  #ifdef CONFIG_GIC_NON_BANKED
>         void __iomem *(*get_base)(union gic_base *);
>  #endif
> @@ -355,6 +361,83 @@ static struct irq_chip gic_chip = {
>         .irq_set_wake           = gic_set_wake,
>  };
> 
> +/*
> + * Shift an interrupt between Group 0 and Group 1.
> + *
> + * In addition to changing the group we also modify the priority to
> + * match what "ARM strongly recommends" for a system where no Group 1
> + * interrupt must ever preempt a Group 0 interrupt.
> + *
> + * If is safe to call this function on systems which do not support
> + * grouping (it will have no effect).
> + */
> +static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq,
> +                             int group)
> +{
> +       void __iomem *base = gic_data_dist_base(gic);
> +       unsigned int grp_reg = hwirq / 32 * 4;
> +       u32 grp_mask = BIT(hwirq % 32);
> +       u32 grp_val;
> +
> +       unsigned int pri_reg = (hwirq / 4) * 4;
> +       u32 pri_mask = BIT(7 + ((hwirq % 4) * 8));
> +       u32 pri_val;
> +
> +       /*
> +        * Systems which do not support grouping will have not have
> +        * the EnableGrp1 bit set.
> +        */
> +       if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)))

I took me a moment to parse this. Can we please have the constants on
the right hand side? It is very much a nitpick, but still...

> +               return;
> +
> +       raw_spin_lock(&irq_controller_lock);
> +
> +       grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
> +       pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg);
> +
> +       if (group) {
> +               grp_val |= grp_mask;
> +               pri_val |= pri_mask;
> +       } else {
> +               grp_val &= ~grp_mask;
> +               pri_val &= ~pri_mask;
> +       }
> +
> +       writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg);
> +       if (grp_reg == 0)
> +               gic->igroup0_shadow = grp_val;
> +
> +       writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
> +
> +       raw_spin_unlock(&irq_controller_lock);
> +}
> +
> +
> +/*
> + * Fully acknowledge (both ack and eoi) any outstanding FIQ-based IPI,
> + * otherwise do nothing.
> + */
> +void gic_handle_fiq_ipi(void)
> +{
> +       struct gic_chip_data *gic = &gic_data[0];
> +       void __iomem *cpu_base = gic_data_cpu_base(gic);
> +       unsigned long irqstat, irqnr;
> +
> +       if (WARN_ON(!in_nmi()))
> +               return;
> +
> +       while ((1u << readl_relaxed(cpu_base + GIC_CPU_HIGHPRI)) &
> +              SMP_IPI_FIQ_MASK) {
> +               irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
> +               writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
> +
> +               irqnr = irqstat & GICC_IAR_INT_ID_MASK;
> +               WARN_RATELIMIT(irqnr > 16,
> +                              "Unexpected irqnr %lu (bad prioritization?)\n",
> +                              irqnr);
> +       }
> +}
> +
>  void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
>  {
>         if (gic_nr >= MAX_GIC_NR)
> @@ -386,15 +469,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
>  static void gic_cpu_if_up(void)
>  {
>         void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> -       u32 bypass = 0;
> +       void __iomem *dist_base = gic_data_dist_base(&gic_data[0]);
> +       u32 ctrl = 0;
> 
>         /*
> -       * Preserve bypass disable bits to be written back later
> -       */
> -       bypass = readl(cpu_base + GIC_CPU_CTRL);
> -       bypass &= GICC_DIS_BYPASS_MASK;
> +        * Preserve bypass disable bits to be written back later
> +        */
> +       ctrl = readl(cpu_base + GIC_CPU_CTRL);
> +       ctrl &= GICC_DIS_BYPASS_MASK;
> 
> -       writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
> +       /*
> +        * If EnableGrp1 is set in the distributor then enable group 1
> +        * support for this CPU (and route group 0 interrupts to FIQ).
> +        */
> +       if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL))
> +               ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL |
> +                       GICC_ENABLE_GRP1;
> +
> +       writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
>  }
> 
> 
> @@ -418,7 +510,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
> 
>         gic_dist_config(base, gic_irqs, NULL);
> 
> -       writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
> +       /*
> +        * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
> +        * bit 1 ignored) depending on current mode.
> +        */
> +       writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL);
> +
> +       /*
> +        * Set all global interrupts to be group 1 if (and only if) it
> +        * is possible to enable group 1 interrupts. This register is RAZ/WI
> +        * if not accessible or not implemented, however some GICv1 devices
> +        * do not implement the EnableGrp1 bit making it unsafe to set
> +        * this register unconditionally.
> +        */
> +       if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))
> +               for (i = 32; i < gic_irqs; i += 32)
> +                       writel_relaxed(0xffffffff,
> +                                      base + GIC_DIST_IGROUP + i * 4 / 32);
>  }
> 
>  static void gic_cpu_init(struct gic_chip_data *gic)
> @@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>         void __iomem *base = gic_data_cpu_base(gic);
>         unsigned int cpu_mask, cpu = smp_processor_id();
>         int i;
> +       unsigned long secure_irqs, secure_irq;

I'd rather see u32 being used for secure_irqs. Remember that this used
on arm64 as well.

> 
>         /*
>          * Get what the GIC says our CPU mask is.
> @@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> 
>         gic_cpu_config(dist_base, NULL);
> 
> +       /*
> +        * If the distributor is configured to support interrupt grouping
> +        * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
> +        * to be group1 and ensure any remaining group 0 interrupts have
> +        * the right priority.
> +        */
> +       if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
> +               secure_irqs = SMP_IPI_FIQ_MASK;
> +               writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
> +               gic->igroup0_shadow = ~secure_irqs;
> +               for_each_set_bit(secure_irq, &secure_irqs, 16)
> +                       gic_set_group_irq(gic, secure_irq, 0);
> +       }
> +
>         writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
>         gic_cpu_if_up();
>  }
> @@ -534,7 +657,8 @@ static void gic_dist_restore(unsigned int gic_nr)
>                 writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
>                         dist_base + GIC_DIST_ENABLE_SET + i * 4);
> 
> -       writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
> +       writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE,
> +                      dist_base + GIC_DIST_CTRL);
>  }
> 
>  static void gic_cpu_save(unsigned int gic_nr)
> @@ -662,6 +786,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>  {
>         int cpu;
>         unsigned long map = 0;
> +       unsigned long softint;

u32, please.

> 
>         gic_migration_lock();
> 
> @@ -675,8 +800,14 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>          */
>         dmb(ishst);
> 
> -       /* this always happens on GIC0 */
> -       writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> +       /* We avoid a readl here by using the shadow copy of IGROUP[0] */
> +       softint = map << 16 | irq;
> +       if (gic_data[0].igroup0_shadow & BIT(irq))
> +               softint |= 0x8000;
> +
> +       /* This always happens on GIC0 */
> +       writel_relaxed(softint,
> +                      gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> 
>         gic_migration_unlock();
>  }
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 71d706d5f169..7690f70049a3 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -22,6 +22,10 @@
>  #define GIC_CPU_IDENT                  0xfc
> 
>  #define GICC_ENABLE                    0x1
> +#define GICC_ENABLE_GRP1               0x2
> +#define GICC_ACK_CTL                   0x4
> +#define GICC_FIQ_EN                    0x8
> +#define GICC_COMMON_BPR                        0x10
>  #define GICC_INT_PRI_THRESHOLD         0xf0
>  #define GICC_IAR_INT_ID_MASK           0x3ff
>  #define GICC_INT_SPURIOUS              1023
> @@ -44,6 +48,7 @@
>  #define GIC_DIST_SGI_PENDING_SET       0xf20
> 
>  #define GICD_ENABLE                    0x1
> +#define GICD_ENABLE_GRP1               0x2
>  #define GICD_DISABLE                   0x0
>  #define GICD_INT_ACTLOW_LVLTRIG                0x0
>  #define GICD_INT_EN_CLR_X32            0xffffffff
> @@ -121,5 +126,8 @@ static inline void __init register_routable_domain_ops
>  {
>         gic_routable_irq_domain_ops = ops;
>  }
> +
> +void gic_handle_fiq_ipi(void);
> +
>  #endif /* __ASSEMBLY */
>  #endif
> --
> 2.1.0
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-10  9:51   ` [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ Daniel Thompson
  2015-04-21 13:45     ` Marc Zyngier
@ 2015-04-21 14:50     ` Mark Rutland
  2015-04-21 21:15       ` Daniel Thompson
  1 sibling, 1 reply; 40+ messages in thread
From: Mark Rutland @ 2015-04-21 14:50 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Thomas Gleixner, Jason Cooper, linaro-kernel, Russell King,
	patches, Marc Zyngier, Stephen Boyd, Will Deacon, linux-kernel,
	Steven Rostedt, Daniel Drake, Dmitry Pervushin, Dirk Behme,
	John Stultz, Tim Sander, Catalin Marinas, Sumit Semwal,
	linux-arm-kernel

Hi,

On Fri, Apr 10, 2015 at 10:51:48AM +0100, Daniel Thompson wrote:
> Currently it is not possible to exploit FIQ for systems with a GIC, even if
> the systems are otherwise capable of it. This patch makes it possible
> for IPIs to be delivered using FIQ.
> 
> To do so it modifies the register state so that normal interrupts are
> placed in group 1 and specific IPIs are placed into group 0. It also
> configures the controller to raise group 0 interrupts using the FIQ
> signal. It provides a means for architecture code to define which IPIs
> shall use FIQ and to acknowledge any IPIs that are raised.
> 
> All GIC hardware except GICv1-without-TrustZone support provides a means
> to group exceptions into group 0 and group 1 but the hardware
> functionality is unavailable to the kernel when a secure monitor is
> present because access to the grouping registers are prohibited outside
> "secure world". However when grouping is not available (or in the case
> of early GICv1 implementations is very hard to configure) the code to
> change groups does not deploy and all IPIs will be raised via IRQ.
> 
> It has been tested and shown working on two systems capable of
> supporting grouping (Freescale i.MX6 and STiH416). It has also been
> tested for boot regressions on two systems that do not support grouping
> (vexpress-a9 and Qualcomm Snapdragon 600).

I just gave this a spin on my (non-MCPM) TC2, and secondaries don't come
up:

CPU1: failed to boot: -38
CPU2: failed to boot: -38
CPU3: failed to boot: -38
CPU4: failed to boot: -38
Brought up 1 CPUs
SMP: Total of 1 processors activated (48.00 BogoMIPS).

I tried investigating with a debugger. The unbooted CPUs look to be
stuck at the FW's spin loop, but the text doesn't look right (I see a
load of ADDEQ r0, r0, r0, #LSL 1 where there was previously a WFI loop).
That could be a bug with my debugger though.

If I pause the CPUs at the right point, they sometimes enter the kernel
successfully. I don't have a good explanation for that.

[...]

> @@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>         void __iomem *base = gic_data_cpu_base(gic);
>         unsigned int cpu_mask, cpu = smp_processor_id();
>         int i;
> +       unsigned long secure_irqs, secure_irq;

I think secure_irq(s) is a misnomer here. It's just a mask of FIQ bits.

> 
>         /*
>          * Get what the GIC says our CPU mask is.
> @@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> 
>         gic_cpu_config(dist_base, NULL);
> 
> +       /*
> +        * If the distributor is configured to support interrupt grouping
> +        * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
> +        * to be group1 and ensure any remaining group 0 interrupts have
> +        * the right priority.
> +        */
> +       if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
> +               secure_irqs = SMP_IPI_FIQ_MASK;
> +               writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
> +               gic->igroup0_shadow = ~secure_irqs;
> +               for_each_set_bit(secure_irq, &secure_irqs, 16)
> +                       gic_set_group_irq(gic, secure_irq, 0);
> +       }

This only pokes GICD registers. Why isn't this in gic_dist_init?

Mark.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-21 13:45     ` Marc Zyngier
@ 2015-04-21 21:03       ` Daniel Thompson
  2015-04-22  9:15         ` Marc Zyngier
  0 siblings, 1 reply; 40+ messages in thread
From: Daniel Thompson @ 2015-04-21 21:03 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Jason Cooper
  Cc: Russell King, Will Deacon, Catalin Marinas, Stephen Boyd,
	John Stultz, Steven Rostedt, linux-kernel, linux-arm-kernel,
	patches, linaro-kernel, Sumit Semwal, Dirk Behme, Daniel Drake,
	Dmitry Pervushin, Tim Sander

On 21/04/15 14:45, Marc Zyngier wrote:
> On 10/04/15 10:51, Daniel Thompson wrote:
>> Currently it is not possible to exploit FIQ for systems with a GIC, even if
>> the systems are otherwise capable of it. This patch makes it possible
>> for IPIs to be delivered using FIQ.
>>
>> To do so it modifies the register state so that normal interrupts are
>> placed in group 1 and specific IPIs are placed into group 0. It also
>> configures the controller to raise group 0 interrupts using the FIQ
>> signal. It provides a means for architecture code to define which IPIs
>> shall use FIQ and to acknowledge any IPIs that are raised.
>>
>> All GIC hardware except GICv1-without-TrustZone support provides a means
>> to group exceptions into group 0 and group 1 but the hardware
>> functionality is unavailable to the kernel when a secure monitor is
>> present because access to the grouping registers are prohibited outside
>> "secure world". However when grouping is not available (or in the case
>> of early GICv1 implementations is very hard to configure) the code to
>> change groups does not deploy and all IPIs will be raised via IRQ.
>>
>> It has been tested and shown working on two systems capable of
>> supporting grouping (Freescale i.MX6 and STiH416). It has also been
>> tested for boot regressions on two systems that do not support grouping
>> (vexpress-a9 and Qualcomm Snapdragon 600).
>>
>> 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>
>> Tested-by: Jon Medhurst <tixy@linaro.org>
>> ---
>>   arch/arm/kernel/traps.c         |   5 +-
>>   drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
>>   include/linux/irqchip/arm-gic.h |   8 +++
>>   3 files changed, 153 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>> index 788e23fe64d8..b35e220ae1b1 100644
>> --- a/arch/arm/kernel/traps.c
>> +++ b/arch/arm/kernel/traps.c
>> @@ -26,6 +26,7 @@
>>   #include <linux/init.h>
>>   #include <linux/sched.h>
>>   #include <linux/irq.h>
>> +#include <linux/irqchip/arm-gic.h>
>>
>>   #include <linux/atomic.h>
>>   #include <asm/cacheflush.h>
>> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
>>
>>          nmi_enter();
>>
>> -       /* nop. FIQ handlers for special arch/arm features can be added here. */
>> +#ifdef CONFIG_ARM_GIC
>> +       gic_handle_fiq_ipi();
>> +#endif
>
> This hunk is what irritates me. It creates a hard dependency between
> core ARM code and the GIC, and I don't really see how this works with
> multiplatform, where the interrupt controller is not necessarily a GIC.
> In that case, you will die a horrible death.

I was just about to reassure you that there is no bug here... but then I 
read the code.

gic_handle_fiq_ipi() was *supposed* to do a check to make it safe to 
call when there is no gic meaning multi-platform support could be 
achieved by calling into multiple handlers.

It looks like I forgot to write the code that would make this possible. 
Maybe I was too disgusted with the approach to implement it correctly. 
Looking at this with fresher eyes (I've been having a bit of a break 
from FIQ recently) I can see how bad the current approach is.


> Why can't we just call handle_arch_irq(), and let the normal handler do
> its thing? You can have a "if (in_nmi())" in there, and call your FIQ
> function. It would at least save us the above problem.

It should certainly work although it feels odd to reuse the IRQ handler 
for FIQ.

All other comments below are fine. I'll fix these.


Daniel.

>
>>
>>          nmi_exit();
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index 578ffc5ec087..ffd1c0fe44b2 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -39,6 +39,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/irqchip/chained_irq.h>
>>   #include <linux/irqchip/arm-gic.h>
>> +#include <linux/ratelimit.h>
>>
>>   #include <asm/cputype.h>
>>   #include <asm/irq.h>
>> @@ -48,6 +49,10 @@
>>   #include "irq-gic-common.h"
>>   #include "irqchip.h"
>>
>> +#ifndef SMP_IPI_FIQ_MASK
>> +#define SMP_IPI_FIQ_MASK 0
>> +#endif
>> +
>>   union gic_base {
>>          void __iomem *common_base;
>>          void __percpu * __iomem *percpu_base;
>> @@ -65,6 +70,7 @@ struct gic_chip_data {
>>   #endif
>>          struct irq_domain *domain;
>>          unsigned int gic_irqs;
>> +       u32 igroup0_shadow;
>>   #ifdef CONFIG_GIC_NON_BANKED
>>          void __iomem *(*get_base)(union gic_base *);
>>   #endif
>> @@ -355,6 +361,83 @@ static struct irq_chip gic_chip = {
>>          .irq_set_wake           = gic_set_wake,
>>   };
>>
>> +/*
>> + * Shift an interrupt between Group 0 and Group 1.
>> + *
>> + * In addition to changing the group we also modify the priority to
>> + * match what "ARM strongly recommends" for a system where no Group 1
>> + * interrupt must ever preempt a Group 0 interrupt.
>> + *
>> + * If is safe to call this function on systems which do not support
>> + * grouping (it will have no effect).
>> + */
>> +static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq,
>> +                             int group)
>> +{
>> +       void __iomem *base = gic_data_dist_base(gic);
>> +       unsigned int grp_reg = hwirq / 32 * 4;
>> +       u32 grp_mask = BIT(hwirq % 32);
>> +       u32 grp_val;
>> +
>> +       unsigned int pri_reg = (hwirq / 4) * 4;
>> +       u32 pri_mask = BIT(7 + ((hwirq % 4) * 8));
>> +       u32 pri_val;
>> +
>> +       /*
>> +        * Systems which do not support grouping will have not have
>> +        * the EnableGrp1 bit set.
>> +        */
>> +       if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)))
>
> I took me a moment to parse this. Can we please have the constants on
> the right hand side? It is very much a nitpick, but still...
>
>> +               return;
>> +
>> +       raw_spin_lock(&irq_controller_lock);
>> +
>> +       grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
>> +       pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg);
>> +
>> +       if (group) {
>> +               grp_val |= grp_mask;
>> +               pri_val |= pri_mask;
>> +       } else {
>> +               grp_val &= ~grp_mask;
>> +               pri_val &= ~pri_mask;
>> +       }
>> +
>> +       writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg);
>> +       if (grp_reg == 0)
>> +               gic->igroup0_shadow = grp_val;
>> +
>> +       writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
>> +
>> +       raw_spin_unlock(&irq_controller_lock);
>> +}
>> +
>> +
>> +/*
>> + * Fully acknowledge (both ack and eoi) any outstanding FIQ-based IPI,
>> + * otherwise do nothing.
>> + */
>> +void gic_handle_fiq_ipi(void)
>> +{
>> +       struct gic_chip_data *gic = &gic_data[0];
>> +       void __iomem *cpu_base = gic_data_cpu_base(gic);
>> +       unsigned long irqstat, irqnr;
>> +
>> +       if (WARN_ON(!in_nmi()))
>> +               return;
>> +
>> +       while ((1u << readl_relaxed(cpu_base + GIC_CPU_HIGHPRI)) &
>> +              SMP_IPI_FIQ_MASK) {
>> +               irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK);
>> +               writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI);
>> +
>> +               irqnr = irqstat & GICC_IAR_INT_ID_MASK;
>> +               WARN_RATELIMIT(irqnr > 16,
>> +                              "Unexpected irqnr %lu (bad prioritization?)\n",
>> +                              irqnr);
>> +       }
>> +}
>> +
>>   void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
>>   {
>>          if (gic_nr >= MAX_GIC_NR)
>> @@ -386,15 +469,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic)
>>   static void gic_cpu_if_up(void)
>>   {
>>          void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
>> -       u32 bypass = 0;
>> +       void __iomem *dist_base = gic_data_dist_base(&gic_data[0]);
>> +       u32 ctrl = 0;
>>
>>          /*
>> -       * Preserve bypass disable bits to be written back later
>> -       */
>> -       bypass = readl(cpu_base + GIC_CPU_CTRL);
>> -       bypass &= GICC_DIS_BYPASS_MASK;
>> +        * Preserve bypass disable bits to be written back later
>> +        */
>> +       ctrl = readl(cpu_base + GIC_CPU_CTRL);
>> +       ctrl &= GICC_DIS_BYPASS_MASK;
>>
>> -       writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
>> +       /*
>> +        * If EnableGrp1 is set in the distributor then enable group 1
>> +        * support for this CPU (and route group 0 interrupts to FIQ).
>> +        */
>> +       if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL))
>> +               ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL |
>> +                       GICC_ENABLE_GRP1;
>> +
>> +       writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL);
>>   }
>>
>>
>> @@ -418,7 +510,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>>
>>          gic_dist_config(base, gic_irqs, NULL);
>>
>> -       writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL);
>> +       /*
>> +        * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
>> +        * bit 1 ignored) depending on current mode.
>> +        */
>> +       writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL);
>> +
>> +       /*
>> +        * Set all global interrupts to be group 1 if (and only if) it
>> +        * is possible to enable group 1 interrupts. This register is RAZ/WI
>> +        * if not accessible or not implemented, however some GICv1 devices
>> +        * do not implement the EnableGrp1 bit making it unsafe to set
>> +        * this register unconditionally.
>> +        */
>> +       if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))
>> +               for (i = 32; i < gic_irqs; i += 32)
>> +                       writel_relaxed(0xffffffff,
>> +                                      base + GIC_DIST_IGROUP + i * 4 / 32);
>>   }
>>
>>   static void gic_cpu_init(struct gic_chip_data *gic)
>> @@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>          void __iomem *base = gic_data_cpu_base(gic);
>>          unsigned int cpu_mask, cpu = smp_processor_id();
>>          int i;
>> +       unsigned long secure_irqs, secure_irq;
>
> I'd rather see u32 being used for secure_irqs. Remember that this used
> on arm64 as well.
>
>>
>>          /*
>>           * Get what the GIC says our CPU mask is.
>> @@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>
>>          gic_cpu_config(dist_base, NULL);
>>
>> +       /*
>> +        * If the distributor is configured to support interrupt grouping
>> +        * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
>> +        * to be group1 and ensure any remaining group 0 interrupts have
>> +        * the right priority.
>> +        */
>> +       if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
>> +               secure_irqs = SMP_IPI_FIQ_MASK;
>> +               writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
>> +               gic->igroup0_shadow = ~secure_irqs;
>> +               for_each_set_bit(secure_irq, &secure_irqs, 16)
>> +                       gic_set_group_irq(gic, secure_irq, 0);
>> +       }
>> +
>>          writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK);
>>          gic_cpu_if_up();
>>   }
>> @@ -534,7 +657,8 @@ static void gic_dist_restore(unsigned int gic_nr)
>>                  writel_relaxed(gic_data[gic_nr].saved_spi_enable[i],
>>                          dist_base + GIC_DIST_ENABLE_SET + i * 4);
>>
>> -       writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL);
>> +       writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE,
>> +                      dist_base + GIC_DIST_CTRL);
>>   }
>>
>>   static void gic_cpu_save(unsigned int gic_nr)
>> @@ -662,6 +786,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>>   {
>>          int cpu;
>>          unsigned long map = 0;
>> +       unsigned long softint;
>
> u32, please.
>
>>
>>          gic_migration_lock();
>>
>> @@ -675,8 +800,14 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>>           */
>>          dmb(ishst);
>>
>> -       /* this always happens on GIC0 */
>> -       writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
>> +       /* We avoid a readl here by using the shadow copy of IGROUP[0] */
>> +       softint = map << 16 | irq;
>> +       if (gic_data[0].igroup0_shadow & BIT(irq))
>> +               softint |= 0x8000;
>> +
>> +       /* This always happens on GIC0 */
>> +       writel_relaxed(softint,
>> +                      gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
>>
>>          gic_migration_unlock();
>>   }
>> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
>> index 71d706d5f169..7690f70049a3 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -22,6 +22,10 @@
>>   #define GIC_CPU_IDENT                  0xfc
>>
>>   #define GICC_ENABLE                    0x1
>> +#define GICC_ENABLE_GRP1               0x2
>> +#define GICC_ACK_CTL                   0x4
>> +#define GICC_FIQ_EN                    0x8
>> +#define GICC_COMMON_BPR                        0x10
>>   #define GICC_INT_PRI_THRESHOLD         0xf0
>>   #define GICC_IAR_INT_ID_MASK           0x3ff
>>   #define GICC_INT_SPURIOUS              1023
>> @@ -44,6 +48,7 @@
>>   #define GIC_DIST_SGI_PENDING_SET       0xf20
>>
>>   #define GICD_ENABLE                    0x1
>> +#define GICD_ENABLE_GRP1               0x2
>>   #define GICD_DISABLE                   0x0
>>   #define GICD_INT_ACTLOW_LVLTRIG                0x0
>>   #define GICD_INT_EN_CLR_X32            0xffffffff
>> @@ -121,5 +126,8 @@ static inline void __init register_routable_domain_ops
>>   {
>>          gic_routable_irq_domain_ops = ops;
>>   }
>> +
>> +void gic_handle_fiq_ipi(void);
>> +
>>   #endif /* __ASSEMBLY */
>>   #endif
>> --
>> 2.1.0
>>
>
> Thanks,
>
> 	M.
>


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-21 14:50     ` Mark Rutland
@ 2015-04-21 21:15       ` Daniel Thompson
  2015-04-22 10:38         ` Mark Rutland
  0 siblings, 1 reply; 40+ messages in thread
From: Daniel Thompson @ 2015-04-21 21:15 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Thomas Gleixner, Jason Cooper, linaro-kernel, Russell King,
	patches, Marc Zyngier, Stephen Boyd, Will Deacon, linux-kernel,
	Steven Rostedt, Daniel Drake, Dmitry Pervushin, Dirk Behme,
	John Stultz, Tim Sander, Catalin Marinas, Sumit Semwal,
	linux-arm-kernel

On 21/04/15 15:50, Mark Rutland wrote:
> Hi,
>
> On Fri, Apr 10, 2015 at 10:51:48AM +0100, Daniel Thompson wrote:
>> Currently it is not possible to exploit FIQ for systems with a GIC, even if
>> the systems are otherwise capable of it. This patch makes it possible
>> for IPIs to be delivered using FIQ.
>>
>> To do so it modifies the register state so that normal interrupts are
>> placed in group 1 and specific IPIs are placed into group 0. It also
>> configures the controller to raise group 0 interrupts using the FIQ
>> signal. It provides a means for architecture code to define which IPIs
>> shall use FIQ and to acknowledge any IPIs that are raised.
>>
>> All GIC hardware except GICv1-without-TrustZone support provides a means
>> to group exceptions into group 0 and group 1 but the hardware
>> functionality is unavailable to the kernel when a secure monitor is
>> present because access to the grouping registers are prohibited outside
>> "secure world". However when grouping is not available (or in the case
>> of early GICv1 implementations is very hard to configure) the code to
>> change groups does not deploy and all IPIs will be raised via IRQ.
>>
>> It has been tested and shown working on two systems capable of
>> supporting grouping (Freescale i.MX6 and STiH416). It has also been
>> tested for boot regressions on two systems that do not support grouping
>> (vexpress-a9 and Qualcomm Snapdragon 600).
>
> I just gave this a spin on my (non-MCPM) TC2, and secondaries don't come
> up:
>
> CPU1: failed to boot: -38
> CPU2: failed to boot: -38
> CPU3: failed to boot: -38
> CPU4: failed to boot: -38
> Brought up 1 CPUs
> SMP: Total of 1 processors activated (48.00 BogoMIPS).
>
> I tried investigating with a debugger. The unbooted CPUs look to be
> stuck at the FW's spin loop, but the text doesn't look right (I see a
> load of ADDEQ r0, r0, r0, #LSL 1 where there was previously a WFI loop).
> That could be a bug with my debugger though.
>
> If I pause the CPUs at the right point, they sometimes enter the kernel
> successfully. I don't have a good explanation for that.
>
> [...]

Rats!

I presume it is patch 3 that causes the regression? Patch 3 is the one 
that causes the GIC to adopt a different configuration if it find the 
kernel running in secure world (it sets all interrupts to group 1 and 
routes group 0 to FIQ).

I only ask because it isn't until patch 6 that we actually place any 
interrupt sources into group 0.



>> @@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>          void __iomem *base = gic_data_cpu_base(gic);
>>          unsigned int cpu_mask, cpu = smp_processor_id();
>>          int i;
>> +       unsigned long secure_irqs, secure_irq;
>
> I think secure_irq(s) is a misnomer here. It's just a mask of FIQ bits.

I guess so, on GICv2 without security extentions these are not secure 
irqs. This is one of the places were IRQ, FIQ, irq and hwirq meet 
together and naming things is hard.

What sort of name do you like: fiq(s), fiq_hwirq(s)?

>>
>>          /*
>>           * Get what the GIC says our CPU mask is.
>> @@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>
>>          gic_cpu_config(dist_base, NULL);
>>
>> +       /*
>> +        * If the distributor is configured to support interrupt grouping
>> +        * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
>> +        * to be group1 and ensure any remaining group 0 interrupts have
>> +        * the right priority.
>> +        */
>> +       if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
>> +               secure_irqs = SMP_IPI_FIQ_MASK;
>> +               writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
>> +               gic->igroup0_shadow = ~secure_irqs;
>> +               for_each_set_bit(secure_irq, &secure_irqs, 16)
>> +                       gic_set_group_irq(gic, secure_irq, 0);
>> +       }
>
> This only pokes GICD registers. Why isn't this in gic_dist_init?

GIC_DIST_IGROUP[0] (which controls grouping for SGIs and PPIs) is banked 
per-cpu and form part of the per-cpu configuration.


Daniel.


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-21 21:03       ` Daniel Thompson
@ 2015-04-22  9:15         ` Marc Zyngier
  2015-04-22 12:45           ` Daniel Thompson
  0 siblings, 1 reply; 40+ messages in thread
From: Marc Zyngier @ 2015-04-22  9:15 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Thomas Gleixner, Jason Cooper, Russell King, Will Deacon,
	Catalin Marinas, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

On Tue, 21 Apr 2015 22:03:25 +0100
Daniel Thompson <daniel.thompson@linaro.org> wrote:

Hi Daniel,

> On 21/04/15 14:45, Marc Zyngier wrote:
> > On 10/04/15 10:51, Daniel Thompson wrote:
> >> Currently it is not possible to exploit FIQ for systems with a GIC, even if
> >> the systems are otherwise capable of it. This patch makes it possible
> >> for IPIs to be delivered using FIQ.
> >>
> >> To do so it modifies the register state so that normal interrupts are
> >> placed in group 1 and specific IPIs are placed into group 0. It also
> >> configures the controller to raise group 0 interrupts using the FIQ
> >> signal. It provides a means for architecture code to define which IPIs
> >> shall use FIQ and to acknowledge any IPIs that are raised.
> >>
> >> All GIC hardware except GICv1-without-TrustZone support provides a means
> >> to group exceptions into group 0 and group 1 but the hardware
> >> functionality is unavailable to the kernel when a secure monitor is
> >> present because access to the grouping registers are prohibited outside
> >> "secure world". However when grouping is not available (or in the case
> >> of early GICv1 implementations is very hard to configure) the code to
> >> change groups does not deploy and all IPIs will be raised via IRQ.
> >>
> >> It has been tested and shown working on two systems capable of
> >> supporting grouping (Freescale i.MX6 and STiH416). It has also been
> >> tested for boot regressions on two systems that do not support grouping
> >> (vexpress-a9 and Qualcomm Snapdragon 600).
> >>
> >> 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>
> >> Tested-by: Jon Medhurst <tixy@linaro.org>
> >> ---
> >>   arch/arm/kernel/traps.c         |   5 +-
> >>   drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
> >>   include/linux/irqchip/arm-gic.h |   8 +++
> >>   3 files changed, 153 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> >> index 788e23fe64d8..b35e220ae1b1 100644
> >> --- a/arch/arm/kernel/traps.c
> >> +++ b/arch/arm/kernel/traps.c
> >> @@ -26,6 +26,7 @@
> >>   #include <linux/init.h>
> >>   #include <linux/sched.h>
> >>   #include <linux/irq.h>
> >> +#include <linux/irqchip/arm-gic.h>
> >>
> >>   #include <linux/atomic.h>
> >>   #include <asm/cacheflush.h>
> >> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
> >>
> >>          nmi_enter();
> >>
> >> -       /* nop. FIQ handlers for special arch/arm features can be added here. */
> >> +#ifdef CONFIG_ARM_GIC
> >> +       gic_handle_fiq_ipi();
> >> +#endif
> >
> > This hunk is what irritates me. It creates a hard dependency between
> > core ARM code and the GIC, and I don't really see how this works with
> > multiplatform, where the interrupt controller is not necessarily a GIC.
> > In that case, you will die a horrible death.
> 
> I was just about to reassure you that there is no bug here... but then I
> read the code.
> 
> gic_handle_fiq_ipi() was *supposed* to do a check to make it safe to
> call when there is no gic meaning multi-platform support could be
> achieved by calling into multiple handlers.
> 
> It looks like I forgot to write the code that would make this possible.
> Maybe I was too disgusted with the approach to implement it correctly.
> Looking at this with fresher eyes (I've been having a bit of a break
> from FIQ recently) I can see how bad the current approach is.
> 
> 
> > Why can't we just call handle_arch_irq(), and let the normal handler do
> > its thing? You can have a "if (in_nmi())" in there, and call your FIQ
> > function. It would at least save us the above problem.
> 
> It should certainly work although it feels odd to reuse the IRQ handler
> for FIQ.

I can see three options:

- (a) Either we have an interrupt controller specific, FIQ only entry
  point, and we add calls in traps.c: this implies that each driver has
  to defend itself against spurious calls.

- (b) We add a separate handle_arch_fiq() indirection that only deals
  with FIQ. Much better, but it also means that we have to keep this in
  sync with arm64, for which the interest is relatively limited (FIQ
  only works if you have a single security domain like XGene, or for a
  VM).

- (c) We call handle_arch_irq(), and let the interrupt controller code
  sort the mess.

I really hate (a) with a passion, because it litters both the ARM core
code with IC specific code *and* introduce some defensive programming
in the IC code, which is a waste...

Option (b) is nicer, but requires additional work and buy-in from the
arm64 maintainers, for a non obvious gain (I quite like the idea of
injecting FIQs in a VM though, just for fun...).

Option (c) is the simplest, if a little ugly on the side.

Thoughts?

	M.
-- 
Jazz is not dead. It just smells funny.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-21 21:15       ` Daniel Thompson
@ 2015-04-22 10:38         ` Mark Rutland
  2015-07-02 13:31           ` Daniel Thompson
  0 siblings, 1 reply; 40+ messages in thread
From: Mark Rutland @ 2015-04-22 10:38 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Thomas Gleixner, Jason Cooper, linaro-kernel, Russell King,
	patches, Marc Zyngier, Stephen Boyd, Will Deacon, linux-kernel,
	Steven Rostedt, Daniel Drake, Dmitry Pervushin, Dirk Behme,
	John Stultz, Tim Sander, Catalin Marinas, Sumit Semwal,
	linux-arm-kernel

> > I just gave this a spin on my (non-MCPM) TC2, and secondaries don't come
> > up:
> >
> > CPU1: failed to boot: -38
> > CPU2: failed to boot: -38
> > CPU3: failed to boot: -38
> > CPU4: failed to boot: -38
> > Brought up 1 CPUs
> > SMP: Total of 1 processors activated (48.00 BogoMIPS).
> >
> > I tried investigating with a debugger. The unbooted CPUs look to be
> > stuck at the FW's spin loop, but the text doesn't look right (I see a
> > load of ADDEQ r0, r0, r0, #LSL 1 where there was previously a WFI loop).
> > That could be a bug with my debugger though.
> >
> > If I pause the CPUs at the right point, they sometimes enter the kernel
> > successfully. I don't have a good explanation for that.
> >
> > [...]
> 
> Rats!
> 
> I presume it is patch 3 that causes the regression? Patch 3 is the one 
> that causes the GIC to adopt a different configuration if it find the 
> kernel running in secure world (it sets all interrupts to group 1 and 
> routes group 0 to FIQ).
> 
> I only ask because it isn't until patch 6 that we actually place any 
> interrupt sources into group 0.

Patch 3 appears to be to blame. I see the issue with patches 1-3 alone
applied atop of v4.0. With patch 3 reverted secondaries come up as
expected.

> >> @@ -427,6 +535,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> >>          void __iomem *base = gic_data_cpu_base(gic);
> >>          unsigned int cpu_mask, cpu = smp_processor_id();
> >>          int i;
> >> +       unsigned long secure_irqs, secure_irq;
> >
> > I think secure_irq(s) is a misnomer here. It's just a mask of FIQ bits.
> 
> I guess so, on GICv2 without security extentions these are not secure 
> irqs. This is one of the places were IRQ, FIQ, irq and hwirq meet 
> together and naming things is hard.
> 
> What sort of name do you like: fiq(s), fiq_hwirq(s)?

I'd go for fiq_mask and fiq, or group1_mask and group1_irq.

[...]

> >> @@ -445,6 +554,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> >>
> >>          gic_cpu_config(dist_base, NULL);
> >>
> >> +       /*
> >> +        * If the distributor is configured to support interrupt grouping
> >> +        * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
> >> +        * to be group1 and ensure any remaining group 0 interrupts have
> >> +        * the right priority.
> >> +        */
> >> +       if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) {
> >> +               secure_irqs = SMP_IPI_FIQ_MASK;
> >> +               writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0);
> >> +               gic->igroup0_shadow = ~secure_irqs;
> >> +               for_each_set_bit(secure_irq, &secure_irqs, 16)
> >> +                       gic_set_group_irq(gic, secure_irq, 0);
> >> +       }
> >
> > This only pokes GICD registers. Why isn't this in gic_dist_init?
> 
> GIC_DIST_IGROUP[0] (which controls grouping for SGIs and PPIs) is banked 
> per-cpu and form part of the per-cpu configuration.

Ah. Would you mind adding a note to the comment w.r.t. GICD_IGROUPR0
being banked per-cpu? I suspect I won't be the only one who fails to
recall that being the case.

We might want to rethink the gic_dist_init/gic_cpu_init naming if
they're no longer cleanly split across distributor and cpu interface
initialisation.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-22  9:15         ` Marc Zyngier
@ 2015-04-22 12:45           ` Daniel Thompson
  2015-04-22 12:57             ` Marc Zyngier
  0 siblings, 1 reply; 40+ messages in thread
From: Daniel Thompson @ 2015-04-22 12:45 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Jason Cooper, Russell King, Will Deacon,
	Catalin Marinas, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

On 22/04/15 10:15, Marc Zyngier wrote:
> On Tue, 21 Apr 2015 22:03:25 +0100
> Daniel Thompson <daniel.thompson@linaro.org> wrote:
>
> Hi Daniel,
>
>> On 21/04/15 14:45, Marc Zyngier wrote:
>>> On 10/04/15 10:51, Daniel Thompson wrote:
>>>> Currently it is not possible to exploit FIQ for systems with a GIC, even if
>>>> the systems are otherwise capable of it. This patch makes it possible
>>>> for IPIs to be delivered using FIQ.
>>>>
>>>> To do so it modifies the register state so that normal interrupts are
>>>> placed in group 1 and specific IPIs are placed into group 0. It also
>>>> configures the controller to raise group 0 interrupts using the FIQ
>>>> signal. It provides a means for architecture code to define which IPIs
>>>> shall use FIQ and to acknowledge any IPIs that are raised.
>>>>
>>>> All GIC hardware except GICv1-without-TrustZone support provides a means
>>>> to group exceptions into group 0 and group 1 but the hardware
>>>> functionality is unavailable to the kernel when a secure monitor is
>>>> present because access to the grouping registers are prohibited outside
>>>> "secure world". However when grouping is not available (or in the case
>>>> of early GICv1 implementations is very hard to configure) the code to
>>>> change groups does not deploy and all IPIs will be raised via IRQ.
>>>>
>>>> It has been tested and shown working on two systems capable of
>>>> supporting grouping (Freescale i.MX6 and STiH416). It has also been
>>>> tested for boot regressions on two systems that do not support grouping
>>>> (vexpress-a9 and Qualcomm Snapdragon 600).
>>>>
>>>> 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>
>>>> Tested-by: Jon Medhurst <tixy@linaro.org>
>>>> ---
>>>>    arch/arm/kernel/traps.c         |   5 +-
>>>>    drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
>>>>    include/linux/irqchip/arm-gic.h |   8 +++
>>>>    3 files changed, 153 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>>>> index 788e23fe64d8..b35e220ae1b1 100644
>>>> --- a/arch/arm/kernel/traps.c
>>>> +++ b/arch/arm/kernel/traps.c
>>>> @@ -26,6 +26,7 @@
>>>>    #include <linux/init.h>
>>>>    #include <linux/sched.h>
>>>>    #include <linux/irq.h>
>>>> +#include <linux/irqchip/arm-gic.h>
>>>>
>>>>    #include <linux/atomic.h>
>>>>    #include <asm/cacheflush.h>
>>>> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
>>>>
>>>>           nmi_enter();
>>>>
>>>> -       /* nop. FIQ handlers for special arch/arm features can be added here. */
>>>> +#ifdef CONFIG_ARM_GIC
>>>> +       gic_handle_fiq_ipi();
>>>> +#endif
>>>
>>> This hunk is what irritates me. It creates a hard dependency between
>>> core ARM code and the GIC, and I don't really see how this works with
>>> multiplatform, where the interrupt controller is not necessarily a GIC.
>>> In that case, you will die a horrible death.
>>
>> I was just about to reassure you that there is no bug here... but then I
>> read the code.
>>
>> gic_handle_fiq_ipi() was *supposed* to do a check to make it safe to
>> call when there is no gic meaning multi-platform support could be
>> achieved by calling into multiple handlers.
>>
>> It looks like I forgot to write the code that would make this possible.
>> Maybe I was too disgusted with the approach to implement it correctly.
>> Looking at this with fresher eyes (I've been having a bit of a break
>> from FIQ recently) I can see how bad the current approach is.
>>
>>
>>> Why can't we just call handle_arch_irq(), and let the normal handler do
>>> its thing? You can have a "if (in_nmi())" in there, and call your FIQ
>>> function. It would at least save us the above problem.
>>
>> It should certainly work although it feels odd to reuse the IRQ handler
>> for FIQ.
>
> I can see three options:
>
> - (a) Either we have an interrupt controller specific, FIQ only entry
>    point, and we add calls in traps.c: this implies that each driver has
>    to defend itself against spurious calls.
>
> - (b) We add a separate handle_arch_fiq() indirection that only deals
>    with FIQ. Much better, but it also means that we have to keep this in
>    sync with arm64, for which the interest is relatively limited (FIQ
>    only works if you have a single security domain like XGene, or for a
>    VM).
>
> - (c) We call handle_arch_irq(), and let the interrupt controller code
>    sort the mess.
>
> I really hate (a) with a passion, because it litters both the ARM core
> code with IC specific code *and* introduce some defensive programming
> in the IC code, which is a waste...
>
> Option (b) is nicer, but requires additional work and buy-in from the
> arm64 maintainers, for a non obvious gain (I quite like the idea of
> injecting FIQs in a VM though, just for fun...).
>
> Option (c) is the simplest, if a little ugly on the side.
>
> Thoughts?

For FIQs, do you anticipate handle_arch_irq() having a role like the 
current gic_handle_fiq_ipi(), which is acknowledge an IPI and get out? 
Alternatively it could behave more like its current role for IRQ and 
call into the handlers itself.

The later seems more likely to work out well when I take another look at 
hooking up the perf interrupt.


Daniel.




^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-22 12:45           ` Daniel Thompson
@ 2015-04-22 12:57             ` Marc Zyngier
  2015-04-22 15:40               ` Daniel Thompson
  0 siblings, 1 reply; 40+ messages in thread
From: Marc Zyngier @ 2015-04-22 12:57 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Thomas Gleixner, Jason Cooper, Russell King, Will Deacon,
	Catalin Marinas, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

On Wed, 22 Apr 2015 13:45:33 +0100
Daniel Thompson <daniel.thompson@linaro.org> wrote:

> On 22/04/15 10:15, Marc Zyngier wrote:
> > On Tue, 21 Apr 2015 22:03:25 +0100
> > Daniel Thompson <daniel.thompson@linaro.org> wrote:
> >
> > Hi Daniel,
> >
> >> On 21/04/15 14:45, Marc Zyngier wrote:
> >>> On 10/04/15 10:51, Daniel Thompson wrote:
> >>>> Currently it is not possible to exploit FIQ for systems with a GIC, even if
> >>>> the systems are otherwise capable of it. This patch makes it possible
> >>>> for IPIs to be delivered using FIQ.
> >>>>
> >>>> To do so it modifies the register state so that normal interrupts are
> >>>> placed in group 1 and specific IPIs are placed into group 0. It also
> >>>> configures the controller to raise group 0 interrupts using the FIQ
> >>>> signal. It provides a means for architecture code to define which IPIs
> >>>> shall use FIQ and to acknowledge any IPIs that are raised.
> >>>>
> >>>> All GIC hardware except GICv1-without-TrustZone support provides a means
> >>>> to group exceptions into group 0 and group 1 but the hardware
> >>>> functionality is unavailable to the kernel when a secure monitor is
> >>>> present because access to the grouping registers are prohibited outside
> >>>> "secure world". However when grouping is not available (or in the case
> >>>> of early GICv1 implementations is very hard to configure) the code to
> >>>> change groups does not deploy and all IPIs will be raised via IRQ.
> >>>>
> >>>> It has been tested and shown working on two systems capable of
> >>>> supporting grouping (Freescale i.MX6 and STiH416). It has also been
> >>>> tested for boot regressions on two systems that do not support grouping
> >>>> (vexpress-a9 and Qualcomm Snapdragon 600).
> >>>>
> >>>> 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>
> >>>> Tested-by: Jon Medhurst <tixy@linaro.org>
> >>>> ---
> >>>>    arch/arm/kernel/traps.c         |   5 +-
> >>>>    drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
> >>>>    include/linux/irqchip/arm-gic.h |   8 +++
> >>>>    3 files changed, 153 insertions(+), 11 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> >>>> index 788e23fe64d8..b35e220ae1b1 100644
> >>>> --- a/arch/arm/kernel/traps.c
> >>>> +++ b/arch/arm/kernel/traps.c
> >>>> @@ -26,6 +26,7 @@
> >>>>    #include <linux/init.h>
> >>>>    #include <linux/sched.h>
> >>>>    #include <linux/irq.h>
> >>>> +#include <linux/irqchip/arm-gic.h>
> >>>>
> >>>>    #include <linux/atomic.h>
> >>>>    #include <asm/cacheflush.h>
> >>>> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
> >>>>
> >>>>           nmi_enter();
> >>>>
> >>>> -       /* nop. FIQ handlers for special arch/arm features can be added here. */
> >>>> +#ifdef CONFIG_ARM_GIC
> >>>> +       gic_handle_fiq_ipi();
> >>>> +#endif
> >>>
> >>> This hunk is what irritates me. It creates a hard dependency between
> >>> core ARM code and the GIC, and I don't really see how this works with
> >>> multiplatform, where the interrupt controller is not necessarily a GIC.
> >>> In that case, you will die a horrible death.
> >>
> >> I was just about to reassure you that there is no bug here... but then I
> >> read the code.
> >>
> >> gic_handle_fiq_ipi() was *supposed* to do a check to make it safe to
> >> call when there is no gic meaning multi-platform support could be
> >> achieved by calling into multiple handlers.
> >>
> >> It looks like I forgot to write the code that would make this possible.
> >> Maybe I was too disgusted with the approach to implement it correctly.
> >> Looking at this with fresher eyes (I've been having a bit of a break
> >> from FIQ recently) I can see how bad the current approach is.
> >>
> >>
> >>> Why can't we just call handle_arch_irq(), and let the normal handler do
> >>> its thing? You can have a "if (in_nmi())" in there, and call your FIQ
> >>> function. It would at least save us the above problem.
> >>
> >> It should certainly work although it feels odd to reuse the IRQ handler
> >> for FIQ.
> >
> > I can see three options:
> >
> > - (a) Either we have an interrupt controller specific, FIQ only entry
> >    point, and we add calls in traps.c: this implies that each driver has
> >    to defend itself against spurious calls.
> >
> > - (b) We add a separate handle_arch_fiq() indirection that only deals
> >    with FIQ. Much better, but it also means that we have to keep this in
> >    sync with arm64, for which the interest is relatively limited (FIQ
> >    only works if you have a single security domain like XGene, or for a
> >    VM).
> >
> > - (c) We call handle_arch_irq(), and let the interrupt controller code
> >    sort the mess.
> >
> > I really hate (a) with a passion, because it litters both the ARM core
> > code with IC specific code *and* introduce some defensive programming
> > in the IC code, which is a waste...
> >
> > Option (b) is nicer, but requires additional work and buy-in from the
> > arm64 maintainers, for a non obvious gain (I quite like the idea of
> > injecting FIQs in a VM though, just for fun...).
> >
> > Option (c) is the simplest, if a little ugly on the side.
> >
> > Thoughts?
> 
> For FIQs, do you anticipate handle_arch_irq() having a role like the 
> current gic_handle_fiq_ipi(), which is acknowledge an IPI and get out? 
> Alternatively it could behave more like its current role for IRQ and 
> call into the handlers itself.
> 
> The later seems more likely to work out well when I take another look at 
> hooking up the perf interrupt.

Assuming your mention of handle_arch_irq() is actually
handle_arch_fiq(), I'd expect some interesting problems if you try to
handle a Linux interrupt while already handling one, as the core IRQ
code is not designed to be reentrant... Your code works so far because
you have been careful to keep the IRQ code at bay. Putting it back into
the equation is going to be hairy at best.

	M.
-- 
Without deviation from the norm, progress is not possible.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-22 12:57             ` Marc Zyngier
@ 2015-04-22 15:40               ` Daniel Thompson
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-04-22 15:40 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Jason Cooper, Russell King, Will Deacon,
	Catalin Marinas, Stephen Boyd, John Stultz, Steven Rostedt,
	linux-kernel, linux-arm-kernel, patches, linaro-kernel,
	Sumit Semwal, Dirk Behme, Daniel Drake, Dmitry Pervushin,
	Tim Sander

On 22/04/15 13:57, Marc Zyngier wrote:
> On Wed, 22 Apr 2015 13:45:33 +0100
> Daniel Thompson <daniel.thompson@linaro.org> wrote:
>
>> On 22/04/15 10:15, Marc Zyngier wrote:
>>> On Tue, 21 Apr 2015 22:03:25 +0100
>>> Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>>
>>> Hi Daniel,
>>>
>>>> On 21/04/15 14:45, Marc Zyngier wrote:
>>>>> On 10/04/15 10:51, Daniel Thompson wrote:
>>>>>> Currently it is not possible to exploit FIQ for systems with a GIC, even if
>>>>>> the systems are otherwise capable of it. This patch makes it possible
>>>>>> for IPIs to be delivered using FIQ.
>>>>>>
>>>>>> To do so it modifies the register state so that normal interrupts are
>>>>>> placed in group 1 and specific IPIs are placed into group 0. It also
>>>>>> configures the controller to raise group 0 interrupts using the FIQ
>>>>>> signal. It provides a means for architecture code to define which IPIs
>>>>>> shall use FIQ and to acknowledge any IPIs that are raised.
>>>>>>
>>>>>> All GIC hardware except GICv1-without-TrustZone support provides a means
>>>>>> to group exceptions into group 0 and group 1 but the hardware
>>>>>> functionality is unavailable to the kernel when a secure monitor is
>>>>>> present because access to the grouping registers are prohibited outside
>>>>>> "secure world". However when grouping is not available (or in the case
>>>>>> of early GICv1 implementations is very hard to configure) the code to
>>>>>> change groups does not deploy and all IPIs will be raised via IRQ.
>>>>>>
>>>>>> It has been tested and shown working on two systems capable of
>>>>>> supporting grouping (Freescale i.MX6 and STiH416). It has also been
>>>>>> tested for boot regressions on two systems that do not support grouping
>>>>>> (vexpress-a9 and Qualcomm Snapdragon 600).
>>>>>>
>>>>>> 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>
>>>>>> Tested-by: Jon Medhurst <tixy@linaro.org>
>>>>>> ---
>>>>>>     arch/arm/kernel/traps.c         |   5 +-
>>>>>>     drivers/irqchip/irq-gic.c       | 151 +++++++++++++++++++++++++++++++++++++---
>>>>>>     include/linux/irqchip/arm-gic.h |   8 +++
>>>>>>     3 files changed, 153 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>>>>>> index 788e23fe64d8..b35e220ae1b1 100644
>>>>>> --- a/arch/arm/kernel/traps.c
>>>>>> +++ b/arch/arm/kernel/traps.c
>>>>>> @@ -26,6 +26,7 @@
>>>>>>     #include <linux/init.h>
>>>>>>     #include <linux/sched.h>
>>>>>>     #include <linux/irq.h>
>>>>>> +#include <linux/irqchip/arm-gic.h>
>>>>>>
>>>>>>     #include <linux/atomic.h>
>>>>>>     #include <asm/cacheflush.h>
>>>>>> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
>>>>>>
>>>>>>            nmi_enter();
>>>>>>
>>>>>> -       /* nop. FIQ handlers for special arch/arm features can be added here. */
>>>>>> +#ifdef CONFIG_ARM_GIC
>>>>>> +       gic_handle_fiq_ipi();
>>>>>> +#endif
>>>>>
>>>>> This hunk is what irritates me. It creates a hard dependency between
>>>>> core ARM code and the GIC, and I don't really see how this works with
>>>>> multiplatform, where the interrupt controller is not necessarily a GIC.
>>>>> In that case, you will die a horrible death.
>>>>
>>>> I was just about to reassure you that there is no bug here... but then I
>>>> read the code.
>>>>
>>>> gic_handle_fiq_ipi() was *supposed* to do a check to make it safe to
>>>> call when there is no gic meaning multi-platform support could be
>>>> achieved by calling into multiple handlers.
>>>>
>>>> It looks like I forgot to write the code that would make this possible.
>>>> Maybe I was too disgusted with the approach to implement it correctly.
>>>> Looking at this with fresher eyes (I've been having a bit of a break
>>>> from FIQ recently) I can see how bad the current approach is.
>>>>
>>>>
>>>>> Why can't we just call handle_arch_irq(), and let the normal handler do
>>>>> its thing? You can have a "if (in_nmi())" in there, and call your FIQ
>>>>> function. It would at least save us the above problem.
>>>>
>>>> It should certainly work although it feels odd to reuse the IRQ handler
>>>> for FIQ.
>>>
>>> I can see three options:
>>>
>>> - (a) Either we have an interrupt controller specific, FIQ only entry
>>>     point, and we add calls in traps.c: this implies that each driver has
>>>     to defend itself against spurious calls.
>>>
>>> - (b) We add a separate handle_arch_fiq() indirection that only deals
>>>     with FIQ. Much better, but it also means that we have to keep this in
>>>     sync with arm64, for which the interest is relatively limited (FIQ
>>>     only works if you have a single security domain like XGene, or for a
>>>     VM).
>>>
>>> - (c) We call handle_arch_irq(), and let the interrupt controller code
>>>     sort the mess.
>>>
>>> I really hate (a) with a passion, because it litters both the ARM core
>>> code with IC specific code *and* introduce some defensive programming
>>> in the IC code, which is a waste...
>>>
>>> Option (b) is nicer, but requires additional work and buy-in from the
>>> arm64 maintainers, for a non obvious gain (I quite like the idea of
>>> injecting FIQs in a VM though, just for fun...).
>>>
>>> Option (c) is the simplest, if a little ugly on the side.
>>>
>>> Thoughts?
>>
>> For FIQs, do you anticipate handle_arch_irq() having a role like the
>> current gic_handle_fiq_ipi(), which is acknowledge an IPI and get out?
>> Alternatively it could behave more like its current role for IRQ and
>> call into the handlers itself.
>>
>> The later seems more likely to work out well when I take another look at
>> hooking up the perf interrupt.
>
> Assuming your mention of handle_arch_irq() is actually
> handle_arch_fiq(), I'd expect some interesting problems if you try to
> handle a Linux interrupt while already handling one, as the core IRQ
> code is not designed to be reentrant... Your code works so far because
> you have been careful to keep the IRQ code at bay. Putting it back into
> the equation is going to be hairy at best.

I was actually thinking of option (c) but the question would apply in 
both cases.

To be clear, I agree we cannot call into big piles of irq code from an 
NMI. We'd have to introduce new NMI-only ways to dispatch FIQs from real 
hwirqs (SPIs and PPIs).

In fact, at present we can't even call into handle_IPI() at the moment 
(because it will call irq_enter) although we could try to modify things 
and make that possible.

These issues apply whether we have conditional code in handle_arch_irq() 
or if we introduce handle_arch_fiq().

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [RESEND PATCH 4.0-rc7 v20 3/6] irqchip: gic: Introduce plumbing for IPI FIQ
  2015-04-22 10:38         ` Mark Rutland
@ 2015-07-02 13:31           ` Daniel Thompson
  0 siblings, 0 replies; 40+ messages in thread
From: Daniel Thompson @ 2015-07-02 13:31 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Thomas Gleixner, Jason Cooper, linaro-kernel, Russell King,
	patches, Marc Zyngier, Stephen Boyd, Will Deacon, linux-kernel,
	Steven Rostedt, Daniel Drake, Dmitry Pervushin, Dirk Behme,
	John Stultz, Tim Sander, Catalin Marinas, Sumit Semwal,
	linux-arm-kernel

On 22/04/15 11:38, Mark Rutland wrote:
>>> I just gave this a spin on my (non-MCPM) TC2, and secondaries don't come
>>> up:
>>>
>>> CPU1: failed to boot: -38
>>> CPU2: failed to boot: -38
>>> CPU3: failed to boot: -38
>>> CPU4: failed to boot: -38
>>> Brought up 1 CPUs
>>> SMP: Total of 1 processors activated (48.00 BogoMIPS).
>>>
>>> I tried investigating with a debugger. The unbooted CPUs look to be
>>> stuck at the FW's spin loop, but the text doesn't look right (I see a
>>> load of ADDEQ r0, r0, r0, #LSL 1 where there was previously a WFI loop).
>>> That could be a bug with my debugger though.
>>>
>>> If I pause the CPUs at the right point, they sometimes enter the kernel
>>> successfully. I don't have a good explanation for that.
>>>
>>> [...]
>>
>> Rats!
>>
>> I presume it is patch 3 that causes the regression? Patch 3 is the one
>> that causes the GIC to adopt a different configuration if it find the
>> kernel running in secure world (it sets all interrupts to group 1 and
>> routes group 0 to FIQ).
>>
>> I only ask because it isn't until patch 6 that we actually place any
>> interrupt sources into group 0.
>
> Patch 3 appears to be to blame. I see the issue with patches 1-3 alone
> applied atop of v4.0. With patch 3 reverted secondaries come up as
> expected.

So I'm back looking at this after a bit of a break.

The problem is almost certainly due to mismanaging the NSATT bit within 
GICD_SGIR. Specifically we must use a different value for NSATT before a CPU is 
booted for the first time because that CPU will not have setup its banked copy 
of IGROUP[0] yet.

I have played with a couple of fixes but I think the simplest
is to detect if we are running from secure mode and, if we are, to write
to GICD_SGIR twice (once without NSATT, once with).

Note that we do have to detect ourselves to be running from secure mode before 
trying the double-write approach. If we were running from non-secure mode then 
the double write could risk two IPIs being generated.

Anyhow the main benefit of this approach is that it is stateless so we don't 
have to do any state tracking (which I think would require using rwlocks).

I plan to react to the outstanding review comments and roll the fix into the 
existing patches but, for clarity, here are the fixes that I think are needed to 
solve the TC2 boot problems. I have tested both from secure and non-secure modes 
but have not been able to test on TC2.

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4f9e4296438c..a7d721e43db6 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -73,6 +73,7 @@ struct gic_chip_data {
  	struct irq_domain *domain;
  	unsigned int gic_irqs;
  	u32 igroup0_shadow;
+	bool sgi_with_nsatt;
  #ifdef CONFIG_GIC_NON_BANKED
  	void __iomem *(*get_base)(union gic_base *);
  #endif
@@ -512,16 +513,27 @@ static void __init gic_dist_init(struct gic_chip_data
  	writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL);

  	/*
-	 * Set all global interrupts to be group 1 if (and only if) it
-	 * is possible to enable group 1 interrupts. This register is RAZ/WI
-	 * if not accessible or not implemented, however some GICv1 devices
-	 * do not implement the EnableGrp1 bit making it unsafe to set
-	 * this register unconditionally.
+	 * Some GICv1 devices (even those with security extensions) do not
+	 * implement EnableGrp1 meaning some parts of the above write might
+	 * be ignored. We will only enable FIQ support if the bit can be set.
  	 */
-	if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))
+	if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)) {
+		/*
+		 * Set all global interrupts to be group 1 (signalled with
+		 * IRQ).
+		 */
  		for (i = 32; i < gic_irqs; i += 32)
  			writel_relaxed(0xffffffff,
  				       base + GIC_DIST_IGROUP + i * 4 / 32);
+
+		/*
+		 * If the GIC supports the security extension then SGIs
+		 * will be filtered based on the value of NSATT. If the
+		 * GIC has this support then enable NSATT support.
+		 */
+		if (GICD_SECURITY_EXTN & readl_relaxed(base + GIC_DIST_CTR))
+			gic->sgi_with_nsatt = true;
+	}
  }

  static void gic_cpu_init(struct gic_chip_data *gic)
@@ -782,6 +794,7 @@ static void gic_raise_softirq(const struct cpumask *mask,
  	int cpu;
  	unsigned long map = 0;
  	unsigned long softint;
+	void __iomem *dist_base;

  	gic_migration_lock();

@@ -789,20 +802,20 @@ static void gic_raise_softirq(const struct cpumask *mask,
  	for_each_cpu(cpu, mask)
  		map |= gic_cpu_map[cpu];

+	/* This always happens on GIC0 */
+	dist_base = gic_data_dist_base(&gic_data[0]);
+
  	/*
  	 * Ensure that stores to Normal memory are visible to the
  	 * other CPUs before they observe us issuing the IPI.
  	 */
  	dmb(ishst);

-	/* We avoid a readl here by using the shadow copy of IGROUP[0] */
  	softint = map << 16 | irq;
-	if (gic_data[0].igroup0_shadow & BIT(irq))
-		softint |= 0x8000;

-	/* This always happens on GIC0 */
-	writel_relaxed(softint,
-		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+	writel_relaxed(softint, dist_base + GIC_DIST_SOFTINT);
+	if (gic_data[0].sgi_with_nsatt)
+		writel_relaxed(softint | 0x8000, dist_base + GIC_DIST_SOFTINT);

  	gic_migration_unlock();
  }
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 361dddfe205a..22cf475e1deb 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -50,6 +50,7 @@
  #define GICD_ENABLE			0x1
  #define GICD_ENABLE_GRP1		0x2
  #define GICD_DISABLE			0x0
+#define GICD_SECURITY_EXTN		0x400
  #define GICD_INT_ACTLOW_LVLTRIG		0x0
  #define GICD_INT_EN_CLR_X32		0xffffffff
  #define GICD_INT_EN_SET_SGI		0x0000ffff
--

Daniel.

^ permalink raw reply related	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2015-07-02 13:31 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq 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 ` [PATCH 4.0-rc5 v19 3/6] irqchip: gic: Introduce plumbing for IPI FIQ 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 ` [PATCH 4.0-rc5 v19 5/6] x86/nmi: Use common printk functions 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-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   ` [RESEND PATCH 4.0-rc5 v19 1/6] irqchip: gic: Optimize locking in gic_raise_softirq Daniel Thompson
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   ` [RESEND PATCH 4.0-rc5 v19 4/6] printk: Simple implementation for NMI backtracing 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 16:19     ` Steven Rostedt
2015-04-07 16:37       ` Borislav Petkov
2015-04-07 16:43         ` Steven Rostedt
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-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   ` [RESEND PATCH 4.0-rc7 v20 1/6] irqchip: gic: Optimize locking in gic_raise_softirq Daniel Thompson
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-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-21 13:45     ` Marc Zyngier
2015-04-21 21:03       ` Daniel Thompson
2015-04-22  9:15         ` Marc Zyngier
2015-04-22 12:45           ` Daniel Thompson
2015-04-22 12:57             ` Marc Zyngier
2015-04-22 15:40               ` Daniel Thompson
2015-04-21 14:50     ` Mark Rutland
2015-04-21 21:15       ` Daniel Thompson
2015-04-22 10:38         ` Mark Rutland
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   ` [RESEND PATCH 4.0-rc7 v20 5/6] x86/nmi: Use common printk functions 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 10:47   ` [RESEND PATCH 4.0-rc7 v20 0/6] irq/arm: Implement arch_trigger_all_cpu_backtrace Daniel Thompson
2015-04-21 12:46   ` Thomas Gleixner
2015-04-21 13:08     ` Marc Zyngier

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).