[PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Daniel Thompson]

This patchset implements arch_trigger_all_cpu_backtrace for arm.

Non-maskable signalling relies on the kernel being able to access FIQ
and therefore, for devices that implement TrustZone, it will work only on
systems that boot the kernel in secure mode.

Tested on Freescale iMX.6 (both via SysRq-l and by deliberately locking
up the kernel with CONFIG_DEBUG_SPINLOCK=y).

Changes since v6:

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

Changes since v5:

* 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 optionall 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.

Changes since v4:

* Rebased on 3.17-rc4.

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

Changes since v3:

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

Changes since v2:

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

Changes since v1:

* 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 (5):
  arm: fiq: Replace default FIQ handler
  arm64: Introduce dummy version of asm/fiq.h
  irqchip: gic: Add support for IPI FIQ
  ARM: add basic support for on-demand backtrace of other CPUs
  arm: smp: Handle ipi_cpu_backtrace() using FIQ (if available)

Russell King (1):
  ARM: remove unused do_unexp_fiq() function

 arch/arm/include/asm/irq.h      |   5 ++
 arch/arm/include/asm/smp.h      |   3 +
 arch/arm/kernel/entry-armv.S    |  98 +++++++++++++++++++++---
 arch/arm/kernel/entry-header.S  |  47 ++++++++++++
 arch/arm/kernel/fiq.c           |  17 ++++-
 arch/arm/kernel/setup.c         |   8 +-
 arch/arm/kernel/smp.c           |  64 ++++++++++++++++
 arch/arm/kernel/traps.c         |  32 +++++++-
 arch/arm64/include/asm/fiq.h    |   8 ++
 drivers/irqchip/irq-gic.c       | 162 ++++++++++++++++++++++++++++++++++++++--
 include/linux/irqchip/arm-gic.h |   3 +
 11 files changed, 421 insertions(+), 26 deletions(-)
 create mode 100644 arch/arm64/include/asm/fiq.h

--
1.9.3

[PATCH 3.17-rc4 v7 1/6] ARM: remove unused do_unexp_fiq() function

[Daniel Thompson]

From: Russell King <rmk+kernel@arm.linux.org.uk>

do_unexp_fiq() has never been called by any code in the last 10 years,
it's about time it was removed!

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 arch/arm/kernel/traps.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index c8e4bb7..a447dcc 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -460,12 +460,6 @@ die_sig:
 	arm_notify_die("Oops - undefined instruction", regs, &info, 0, 6);
 }
 
-asmlinkage void do_unexp_fiq (struct pt_regs *regs)
-{
-	printk("Hmm.  Unexpected FIQ received, but trying to continue\n");
-	printk("You may have a hardware problem...\n");
-}
-
 /*
  * bad_mode handles the impossible case in the vectors.  If you see one of
  * these, then it's extremely serious, and could mean you have buggy hardware.
-- 
1.9.3

[PATCH 3.17-rc4 v7 2/6] arm: fiq: Replace default FIQ handler

[Daniel Thompson]

This patch introduces a new default FIQ handler that is structured in a
similar way to the existing ARM exception handler and result in the FIQ
being handled by C code running on the SVC stack (despite this code run
in the FIQ handler is subject to severe limitations with respect to
locking making normal interaction with the kernel impossible).

This default handler allows concepts that on x86 would be handled using
NMIs to be realized on ARM.

Credit:

    This patch is a near complete re-write of a patch originally
    provided by Anton Vorontsov. Today only a couple of small fragments
    survive, however without Anton's work to build from this patch would
    not exist. Thanks also to Russell King for spoonfeeding me a variety
    of fixes during the review cycle.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Nicolas Pitre <nico@linaro.org>
---
 arch/arm/kernel/entry-armv.S   | 98 +++++++++++++++++++++++++++++++++++++-----
 arch/arm/kernel/entry-header.S | 47 ++++++++++++++++++++
 arch/arm/kernel/fiq.c          | 17 ++++++--
 arch/arm/kernel/setup.c        |  8 +++-
 arch/arm/kernel/traps.c        | 26 +++++++++++
 5 files changed, 180 insertions(+), 16 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 36276cd..859f56c 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -146,7 +146,7 @@ ENDPROC(__und_invalid)
 #define SPFIX(code...)
 #endif
 
-	.macro	svc_entry, stack_hole=0
+	.macro	svc_entry, stack_hole=0, trace=1
  UNWIND(.fnstart		)
  UNWIND(.save {r0 - pc}		)
 	sub	sp, sp, #(S_FRAME_SIZE + \stack_hole - 4)
@@ -182,9 +182,11 @@ ENDPROC(__und_invalid)
 	@
 	stmia	r7, {r2 - r6}
 
+	.if \trace
 #ifdef CONFIG_TRACE_IRQFLAGS
 	bl	trace_hardirqs_off
 #endif
+	.endif
 	.endm
 
 	.align	5
@@ -295,6 +297,15 @@ __pabt_svc:
 ENDPROC(__pabt_svc)
 
 	.align	5
+__fiq_svc:
+	svc_entry trace=0
+	mov	r0, sp				@ struct pt_regs *regs
+	bl	handle_fiq_as_nmi
+	svc_exit_via_fiq
+ UNWIND(.fnend		)
+ENDPROC(__fiq_svc)
+
+	.align	5
 .LCcralign:
 	.word	cr_alignment
 #ifdef MULTI_DABORT
@@ -305,6 +316,46 @@ ENDPROC(__pabt_svc)
 	.word	fp_enter
 
 /*
+ * Abort mode handlers
+ */
+
+@
+@ Taking a FIQ in abort mode is similar to taking a FIQ in SVC mode
+@ and reuses the same macros. However in abort mode we must also
+@ save/restore lr_abt and spsr_abt to make nested aborts safe.
+@
+	.align 5
+__fiq_abt:
+	svc_entry trace=0
+
+ ARM(	msr	cpsr_c, #ABT_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( mov	r0, #ABT_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( msr	cpsr_c, r0 )
+	mov	r1, lr		@ Save lr_abt
+	mrs	r2, spsr	@ Save spsr_abt, abort is now safe
+ ARM(	msr	cpsr_c, #SVC_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( mov	r0, #SVC_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( msr	cpsr_c, r0 )
+	stmfd	sp!, {r1 - r2}
+
+	add	r0, sp, #8			@ struct pt_regs *regs
+	bl	handle_fiq_as_nmi
+
+	ldmfd	sp!, {r1 - r2}
+ ARM(	msr	cpsr_c, #ABT_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( mov	r0, #ABT_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( msr	cpsr_c, r0 )
+	mov	lr, r1		@ Restore lr_abt, abort is unsafe
+	msr	spsr_cxsf, r2	@ Restore spsr_abt
+ ARM(	msr	cpsr_c, #SVC_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( mov	r0, #SVC_MODE | PSR_I_BIT | PSR_F_BIT )
+ THUMB( msr	cpsr_c, r0 )
+
+	svc_exit_via_fiq
+ UNWIND(.fnend		)
+ENDPROC(__fiq_abt)
+
+/*
  * User mode handlers
  *
  * EABI note: sp_svc is always 64-bit aligned here, so should S_FRAME_SIZE
@@ -314,7 +365,7 @@ ENDPROC(__pabt_svc)
 #error "sizeof(struct pt_regs) must be a multiple of 8"
 #endif
 
-	.macro	usr_entry
+	.macro	usr_entry, trace=1
  UNWIND(.fnstart	)
  UNWIND(.cantunwind	)	@ don't unwind the user space
 	sub	sp, sp, #S_FRAME_SIZE
@@ -351,10 +402,12 @@ ENDPROC(__pabt_svc)
 	@
 	zero_fp
 
+	.if	\trace
 #ifdef CONFIG_IRQSOFF_TRACER
 	bl	trace_hardirqs_off
 #endif
 	ct_user_exit save = 0
+	.endif
 	.endm
 
 	.macro	kuser_cmpxchg_check
@@ -683,6 +736,17 @@ ENTRY(ret_from_exception)
 ENDPROC(__pabt_usr)
 ENDPROC(ret_from_exception)
 
+	.align	5
+__fiq_usr:
+	usr_entry trace=0
+	kuser_cmpxchg_check
+	mov	r0, sp				@ struct pt_regs *regs
+	bl	handle_fiq_as_nmi
+	get_thread_info tsk
+	restore_user_regs fast = 0, offset = 0
+ UNWIND(.fnend		)
+ENDPROC(__fiq_usr)
+
 /*
  * Register switch for ARMv3 and ARMv4 processors
  * r0 = previous task_struct, r1 = previous thread_info, r2 = next thread_info
@@ -1118,17 +1182,29 @@ vector_addrexcptn:
 	b	vector_addrexcptn
 
 /*=============================================================================
- * Undefined FIQs
+ * FIQ "NMI" handler
  *-----------------------------------------------------------------------------
- * Enter in FIQ mode, spsr = ANY CPSR, lr = ANY PC
- * MUST PRESERVE SVC SPSR, but need to switch to SVC mode to show our msg.
- * Basically to switch modes, we *HAVE* to clobber one register...  brain
- * damage alert!  I don't think that we can execute any code in here in any
- * other mode than FIQ...  Ok you can switch to another mode, but you can't
- * get out of that mode without clobbering one register.
+ * Handle a FIQ using the SVC stack allowing FIQ act like NMI on x86
+ * systems.
  */
-vector_fiq:
-	subs	pc, lr, #4
+	vector_stub	fiq, FIQ_MODE, 4
+
+	.long	__fiq_usr			@  0  (USR_26 / USR_32)
+	.long	__fiq_svc			@  1  (FIQ_26 / FIQ_32)
+	.long	__fiq_svc			@  2  (IRQ_26 / IRQ_32)
+	.long	__fiq_svc			@  3  (SVC_26 / SVC_32)
+	.long	__fiq_svc			@  4
+	.long	__fiq_svc			@  5
+	.long	__fiq_svc			@  6
+	.long	__fiq_abt			@  7
+	.long	__fiq_svc			@  8
+	.long	__fiq_svc			@  9
+	.long	__fiq_svc			@  a
+	.long	__fiq_svc			@  b
+	.long	__fiq_svc			@  c
+	.long	__fiq_svc			@  d
+	.long	__fiq_svc			@  e
+	.long	__fiq_svc			@  f
 
 	.globl	vector_fiq_offset
 	.equ	vector_fiq_offset, vector_fiq
diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index 2fdf867..0d91ca0 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -216,6 +216,34 @@
 	ldmia	sp, {r0 - pc}^			@ load r0 - pc, cpsr
 	.endm
 
+	@
+	@ svc_exit_via_fiq - like svc_exit but switches to FIQ mode before exit
+	@
+	@ This macro acts in a similar manner to svc_exit but switches to FIQ
+	@ mode to restore the final part of the register state.
+	@
+	@ We cannot use the normal svc_exit procedure because that would
+	@ clobber spsr_svc (FIQ could be delivered during the first few
+	@ instructions of vector_swi meaning its contents have not been
+	@ saved anywhere).
+	@
+	@ Note that, unlike svc_exit, this macro also does not allow a caller
+	@ supplied rpsr. This is because the FIQ exceptions are not re-entrant
+	@ and the handlers cannot call into the scheduler (meaning the value
+	@ on the stack remains correct).
+	@
+	.macro  svc_exit_via_fiq
+	mov	r0, sp
+	ldmib	r0, {r1 - r14}	@ abort is deadly from here onward (it will
+				@ clobber state restored below)
+	msr	cpsr_c, #FIQ_MODE | PSR_I_BIT | PSR_F_BIT
+	add	r8, r0, #S_PC
+	ldr	r9, [r0, #S_PSR]
+	msr	spsr_cxsf, r9
+	ldr	r0, [r0, #S_R0]
+	ldmia	r8, {pc}^
+	.endm
+
 	.macro	restore_user_regs, fast = 0, offset = 0
 	ldr	r1, [sp, #\offset + S_PSR]	@ get calling cpsr
 	ldr	lr, [sp, #\offset + S_PC]!	@ get pc
@@ -267,6 +295,25 @@
 	rfeia	sp!
 	.endm
 
+	@
+	@ svc_exit_via_fiq - like svc_exit but switches to FIQ mode before exit
+	@
+	@ For full details see non-Thumb implementation above.
+	@
+	.macro  svc_exit_via_fiq
+	add	r0, sp, #S_R2
+	ldr	lr, [sp, #S_LR]
+	ldr	sp, [sp, #S_SP] @ abort is deadly from here onward (it will
+			        @ clobber state restored below)
+	ldmia	r0, {r2 - r12}
+	mov	r1, #FIQ_MODE | PSR_I_BIT | PSR_F_BIT
+	msr	cpsr_c, r1
+	sub	r0, #S_R2
+	add	r8, r0, #S_PC
+	ldmia	r0, {r0 - r1}
+	rfeia	r8
+	.endm
+
 #ifdef CONFIG_CPU_V7M
 	/*
 	 * Note we don't need to do clrex here as clearing the local monitor is
diff --git a/arch/arm/kernel/fiq.c b/arch/arm/kernel/fiq.c
index 918875d..b37752a 100644
--- a/arch/arm/kernel/fiq.c
+++ b/arch/arm/kernel/fiq.c
@@ -52,7 +52,8 @@
 		(unsigned)&vector_fiq_offset;		\
 	})
 
-static unsigned long no_fiq_insn;
+static unsigned long dfl_fiq_insn;
+static struct pt_regs dfl_fiq_regs;
 
 /* Default reacquire function
  * - we always relinquish FIQ control
@@ -60,8 +61,15 @@ static unsigned long no_fiq_insn;
  */
 static int fiq_def_op(void *ref, int relinquish)
 {
-	if (!relinquish)
-		set_fiq_handler(&no_fiq_insn, sizeof(no_fiq_insn));
+	if (!relinquish) {
+		/* Restore default handler and registers */
+		local_fiq_disable();
+		set_fiq_regs(&dfl_fiq_regs);
+		set_fiq_handler(&dfl_fiq_insn, sizeof(dfl_fiq_insn));
+		local_fiq_enable();
+
+		/* FIXME: notify irq controller to standard enable FIQs */
+	}
 
 	return 0;
 }
@@ -150,6 +158,7 @@ EXPORT_SYMBOL(disable_fiq);
 void __init init_FIQ(int start)
 {
 	unsigned offset = FIQ_OFFSET;
-	no_fiq_insn = *(unsigned long *)(0xffff0000 + offset);
+	dfl_fiq_insn = *(unsigned long *)(0xffff0000 + offset);
+	get_fiq_regs(&dfl_fiq_regs);
 	fiq_start = start;
 }
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 84db893d..c031063 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -133,6 +133,7 @@ struct stack {
 	u32 irq[3];
 	u32 abt[3];
 	u32 und[3];
+	u32 fiq[3];
 } ____cacheline_aligned;
 
 #ifndef CONFIG_CPU_V7M
@@ -470,7 +471,10 @@ void notrace cpu_init(void)
 	"msr	cpsr_c, %5\n\t"
 	"add	r14, %0, %6\n\t"
 	"mov	sp, r14\n\t"
-	"msr	cpsr_c, %7"
+	"msr	cpsr_c, %7\n\t"
+	"add	r14, %0, %8\n\t"
+	"mov	sp, r14\n\t"
+	"msr	cpsr_c, %9"
 	    :
 	    : "r" (stk),
 	      PLC (PSR_F_BIT | PSR_I_BIT | IRQ_MODE),
@@ -479,6 +483,8 @@ void notrace cpu_init(void)
 	      "I" (offsetof(struct stack, abt[0])),
 	      PLC (PSR_F_BIT | PSR_I_BIT | UND_MODE),
 	      "I" (offsetof(struct stack, und[0])),
+	      PLC (PSR_F_BIT | PSR_I_BIT | FIQ_MODE),
+	      "I" (offsetof(struct stack, fiq[0])),
 	      PLC (PSR_F_BIT | PSR_I_BIT | SVC_MODE)
 	    : "r14");
 #endif
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index a447dcc..439138d 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -25,6 +25,7 @@
 #include <linux/delay.h>
 #include <linux/init.h>
 #include <linux/sched.h>
+#include <linux/irq.h>
 
 #include <linux/atomic.h>
 #include <asm/cacheflush.h>
@@ -461,6 +462,31 @@ die_sig:
 }
 
 /*
+ * Handle FIQ similarly to NMI on x86 systems.
+ *
+ * The runtime environment for NMIs is extremely restrictive
+ * (NMIs can pre-empt critical sections meaning almost all locking is
+ * forbidden) meaning this default FIQ handling must only be used in
+ * circumstances where non-maskability improves robustness, such as
+ * watchdog or debug logic.
+ *
+ * This handler is not appropriate for general purpose use in drivers
+ * platform code and can be overrideen using set_fiq_handler.
+ */
+asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)
+{
+	struct pt_regs *old_regs = set_irq_regs(regs);
+
+	nmi_enter();
+
+	/* nop. FIQ handlers for special arch/arm features can be added here. */
+
+	nmi_exit();
+
+	set_irq_regs(old_regs);
+}
+
+/*
  * bad_mode handles the impossible case in the vectors.  If you see one of
  * these, then it's extremely serious, and could mean you have buggy hardware.
  * It never returns, and never tries to sync.  We hope that we can at least
-- 
1.9.3

[PATCH 3.17-rc4 v7 3/6] arm64: Introduce dummy version of asm/fiq.h

[Daniel Thompson]

Drivers that are shared between arm and arm64 and which employ
FIQ on arm cannot include asm/fiq.h without #ifdef'ing. This patch
introduces a dummy version of asm/fiq.h to arm64 to avoid this.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/fiq.h | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 arch/arm64/include/asm/fiq.h

diff --git a/arch/arm64/include/asm/fiq.h b/arch/arm64/include/asm/fiq.h
new file mode 100644
index 0000000..d3776b8
--- /dev/null
+++ b/arch/arm64/include/asm/fiq.h
@@ -0,0 +1,8 @@
+/*
+ * Placeholder to reduce #ifdef'ing in shared arm/arm64 drivers. It
+ * allows code of the following form to be made unconditional.
+ *
+ * #ifdef CONFIG_FIQ
+ * #include <asm/fiq.h>
+ * #endif
+ */
-- 
1.9.3

[PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Daniel Thompson]

This patch provides support for arm's newly added IPI FIQ. It works
by placing all interrupt sources *except* IPI FIQ in group 1 and
then flips a configuration bit in the GIC such that group 1
interrupts use IRQ and group 0 interrupts use FIQ.

All GIC hardware except GICv1-without-TrustZone support provides a means
to group exceptions into group 0 and group 1. However 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" (a feature that allows grouping to be used to allow
hardware peripherals to send interrupts into the secure world). However
when grouping is not available we can rely on the GIC's RAZ/WI semantics
and avoid conditional code.

gic_raise_softirq() also needs changing to make it safe to call from
FIQ. This is due to its use by wake_up_klogd() during console_unlock().

Tested on Freescale i.MX6 (quad A9, GICv1-with-TrustZone running in
secure mode).

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Cooper <jason@lakedaemon.net>
---
 arch/arm/kernel/traps.c         |   5 +-
 drivers/irqchip/irq-gic.c       | 162 ++++++++++++++++++++++++++++++++++++++--
 include/linux/irqchip/arm-gic.h |   3 +
 3 files changed, 162 insertions(+), 8 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 439138d..92c4ea1 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 4b959e6..05b5d62 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -39,8 +39,10 @@
 #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/fiq.h>
 #include <asm/irq.h>
 #include <asm/exception.h>
 #include <asm/smp_plat.h>
@@ -48,6 +50,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;
@@ -71,6 +77,8 @@ struct gic_chip_data {
 };
 
 static DEFINE_RAW_SPINLOCK(irq_controller_lock);
+/* A fiq-safe spinlock must only be locked when the FIQ is masked */
+static DEFINE_RAW_SPINLOCK(fiq_safe_migration_lock);
 
 /*
  * The GIC mapping of CPU interfaces does not necessarily match
@@ -325,6 +333,94 @@ 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(void __iomem *base, unsigned int hwirq,
+				int group)
+{
+	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 no bits
+	 * set in IGROUP[0] (and all systems which do will have set bits).
+	 */
+	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + 0);
+	if (!grp_val)
+		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);
+	writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
+
+	raw_spin_unlock(&irq_controller_lock);
+}
+
+/*
+ * Test which group an interrupt belongs to.
+ *
+ * Returns 0 if the controller does not support grouping.
+ */
+static int gic_get_group_irq(void __iomem *base, unsigned int hwirq)
+{
+	unsigned int grp_reg = hwirq / 32 * 4;
+	u32 grp_val;
+
+	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
+
+	return (grp_val >> (hwirq % 32)) & 1;
+}
+
+/*
+ * 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)
@@ -373,7 +469,18 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
 
 	gic_dist_config(base, gic_irqs, NULL);
 
-	writel_relaxed(1, base + GIC_DIST_CTRL);
+	/*
+	 * Set all global interrupts to be group 1 (this register is
+	 * RAZ/WI if not accessible in current mode)
+	 */
+	for (i = 32; i < gic_irqs; i += 32)
+		writel_relaxed(0xffffffff, base + GIC_DIST_IGROUP + i * 4 / 32);
+
+	/*
+	 * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
+	 * bit 1 ignored)
+	 */
+	writel_relaxed(3, base + GIC_DIST_CTRL);
 }
 
 static void gic_cpu_init(struct gic_chip_data *gic)
@@ -382,6 +489,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.
@@ -400,8 +508,27 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 
 	gic_cpu_config(dist_base, NULL);
 
+	/*
+	 * Set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
+	 * to be group 1 (this register is RAZ/WI if not accessible)
+	 */
+	writel_relaxed(~SMP_IPI_FIQ_MASK, dist_base + GIC_DIST_IGROUP + 0);
+
+	/*
+	 * Update the priority of any resulting group0 interrupts.
+	 */
+	secure_irqs = ~readl_relaxed(dist_base + GIC_DIST_IGROUP + 0);
+	for_each_set_bit(secure_irq, &secure_irqs, 16)
+		gic_set_group_irq(dist_base, i, 0);
+
 	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
-	writel_relaxed(1, base + GIC_CPU_CTRL);
+
+	/* The bottom most bit will be set for all GIC variants (and is
+	 * called Enable or EnableGrp0 depending on operating mode). The
+	 * remaining four bits (CBPR, FIQEn, AckCtl and EnableGrp1) are
+	 * RAZ/WI if not accessible.
+	 */
+	writel_relaxed(0x1f, base + GIC_CPU_CTRL);
 }
 
 void gic_cpu_if_down(void)
@@ -485,7 +612,7 @@ 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(1, dist_base + GIC_DIST_CTRL);
+	writel_relaxed(3, dist_base + GIC_DIST_CTRL);
 }
 
 static void gic_cpu_save(unsigned int gic_nr)
@@ -542,7 +669,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
 
 	writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
-	writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+	writel_relaxed(0x1f, cpu_base + GIC_CPU_CTRL);
 }
 
 static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
@@ -604,8 +731,19 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 {
 	int cpu;
 	unsigned long flags, map = 0;
+	unsigned long softint;
 
-	raw_spin_lock_irqsave(&irq_controller_lock, flags);
+	/*
+	 * The locking in this function ensures we don't use stale cpu mappings
+	 * and thus we never route an IPI to the wrong physical core during a
+	 * big.LITTLE switch. The switch code takes both of these locks meaning
+	 * we can choose whichever lock is safe to use from our current calling
+	 * context.
+	 */
+	if (in_nmi())
+		raw_spin_lock(&fiq_safe_migration_lock);
+	else
+		raw_spin_lock_irqsave(&irq_controller_lock, flags);
 
 	/* Convert our logical CPU mask into a physical one. */
 	for_each_cpu(cpu, mask)
@@ -618,7 +756,15 @@ 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);
+	softint = map << 16 | irq;
+	if (gic_get_group_irq(gic_data_dist_base(&gic_data[0]), irq))
+		softint |= 0x8000;
+	writel_relaxed(softint,
+		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
+
+	if (in_nmi())
+		raw_spin_unlock(&fiq_safe_migration_lock);
+	else
 
 	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
 }
@@ -668,7 +814,7 @@ 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 with IRQ and FIQ locally disabled.
  */
 void gic_migrate_target(unsigned int new_cpu_id)
 {
@@ -690,6 +836,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
 	ror_val = (cur_cpu_id - new_cpu_id) & 31;
 
 	raw_spin_lock(&irq_controller_lock);
+	raw_spin_lock(&fiq_safe_migration_lock);
 
 	/* Update the target interface for this logical CPU */
 	gic_cpu_map[cpu] = 1 << new_cpu_id;
@@ -709,6 +856,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
 		}
 	}
 
+	raw_spin_unlock(&fiq_safe_migration_lock);
 	raw_spin_unlock(&irq_controller_lock);
 
 	/*
diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index 45e2d8c..52a5676 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -101,5 +101,8 @@ static inline void __init register_routable_domain_ops
 {
 	gic_routable_irq_domain_ops = ops;
 }
+
+void gic_handle_fiq_ipi(void);
+
 #endif /* __ASSEMBLY */
 #endif
-- 
1.9.3

[PATCH 3.17-rc4 v7 5/6] ARM: add basic support for on-demand backtrace of other CPUs

[Daniel Thompson]

Add basic infrastructure for triggering a backtrace of other CPUs
via an IPI, preferably at FIQ level.  It is intended that this shall
be used for cases where we have detected that something has already
failed in the kernel.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 arch/arm/include/asm/irq.h |  5 ++++
 arch/arm/kernel/smp.c      | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index 53c15de..be1d07d 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/kernel/smp.c b/arch/arm/kernel/smp.c
index 9388a3d..94959f9 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -72,8 +72,12 @@ enum ipi_msg_type {
 	IPI_CPU_STOP,
 	IPI_IRQ_WORK,
 	IPI_COMPLETION,
+	IPI_CPU_BACKTRACE,
 };
 
+/* For reliability, we're prepared to waste bits here. */
+static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly;
+
 static DECLARE_COMPLETION(cpu_running);
 
 static struct smp_operations smp_ops;
@@ -539,6 +543,21 @@ static void ipi_cpu_stop(unsigned int cpu)
 		cpu_relax();
 }
 
+static void ipi_cpu_backtrace(struct pt_regs *regs)
+{
+	int cpu = smp_processor_id();
+
+	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
+		static arch_spinlock_t lock = __ARCH_SPIN_LOCK_UNLOCKED;
+
+		arch_spin_lock(&lock);
+		printk(KERN_WARNING "FIQ backtrace for cpu %d\n", cpu);
+		show_regs(regs);
+		arch_spin_unlock(&lock);
+		cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));
+	}
+}
+
 static DEFINE_PER_CPU(struct completion *, cpu_completion);
 
 int register_ipi_completion(struct completion *completion, int cpu)
@@ -618,6 +637,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:
 		printk(KERN_CRIT "CPU%u: Unknown IPI message 0x%x\n",
 		       cpu, ipinr);
@@ -712,3 +737,40 @@ static int __init register_cpufreq_notifier(void)
 core_initcall(register_cpufreq_notifier);
 
 #endif
+
+void arch_trigger_all_cpu_backtrace(bool include_self)
+{
+	static unsigned long backtrace_flag;
+	int i, cpu = get_cpu();
+
+	if (test_and_set_bit(0, &backtrace_flag)) {
+		/*
+		 * If there is already a trigger_all_cpu_backtrace() in progress
+		 * (backtrace_flag == 1), don't output double cpu dump infos.
+		 */
+		put_cpu();
+		return;
+	}
+
+	cpumask_copy(to_cpumask(backtrace_mask), cpu_online_mask);
+	if (!include_self)
+		cpumask_clear_cpu(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);
+	}
+
+	clear_bit(0, &backtrace_flag);
+	smp_mb__after_atomic();
+	put_cpu();
+}
-- 
1.9.3

[PATCH 3.17-rc4 v7 6/6] arm: smp: Handle ipi_cpu_backtrace() using FIQ (if available)

[Daniel Thompson]

Previous changes have introduced a replacement default FIQ handler and
an implementation of arch_trigger_all_cpu_backtrace for ARM but these
are currently independant.

This patch plumbs together these features making it possible, on platforms
that support it, to trigger backtrace using FIQ.

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 arch/arm/include/asm/smp.h | 3 +++
 arch/arm/kernel/smp.c      | 4 +++-
 arch/arm/kernel/traps.c    | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/smp.h b/arch/arm/include/asm/smp.h
index 2ec765c..0580270 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;
@@ -85,6 +87,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 94959f9..7a79d11 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -543,7 +543,7 @@ static void ipi_cpu_stop(unsigned int cpu)
 		cpu_relax();
 }
 
-static void ipi_cpu_backtrace(struct pt_regs *regs)
+void ipi_cpu_backtrace(struct pt_regs *regs)
 {
 	int cpu = smp_processor_id();
 
@@ -584,6 +584,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]);
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 92c4ea1..40b1de7 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();
 
-- 
1.9.3

Re: [PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Russell King - ARM Linux]

On Wed, Sep 17, 2014 at 09:10:16AM -0700, Daniel Thompson wrote:
> This patch provides support for arm's newly added IPI FIQ. It works
> by placing all interrupt sources *except* IPI FIQ in group 1 and
> then flips a configuration bit in the GIC such that group 1
> interrupts use IRQ and group 0 interrupts use FIQ.
> 
> All GIC hardware except GICv1-without-TrustZone support provides a means
> to group exceptions into group 0 and group 1. However 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" (a feature that allows grouping to be used to allow
> hardware peripherals to send interrupts into the secure world). However
> when grouping is not available we can rely on the GIC's RAZ/WI semantics
> and avoid conditional code.

I've been chasing a bug with this on the Versatile Express CT9x4.  It
seems that the GIC there is a GICv1, with secure extensions.  It seems
to support interrupt grouping.

However, setting SPIs to group 1, with the control registers enabling
both group 0 and group 1 (such that both groups are treated as IRQs)
results in no SPIs being delivered to the kernel.  In other words,
setting GIC_DIST_IGROUP for SPIs, and setting both GIC_CPU_CTRL and
GIC_DIST_CTRL to 3.

This is rather worrying, because we seem to have a GIC which for all
intents and purposes appears to be compatible with what we want to do,
appears to conform with the GIC architecture specifications, but doesn't
actually work.

I suspect that running the Versatile Express CT9x4 in non-secure mode
wouldn't work (because in non-secure mode, the GIC only allows access
to group 1 interrupts.)

I've added Will and Mark to this to see whether they have any comment.

> gic_raise_softirq() also needs changing to make it safe to call from
> FIQ. This is due to its use by wake_up_klogd() during console_unlock().
> 
> Tested on Freescale i.MX6 (quad A9, GICv1-with-TrustZone running in
> secure mode).
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> ---
>  arch/arm/kernel/traps.c         |   5 +-
>  drivers/irqchip/irq-gic.c       | 162 ++++++++++++++++++++++++++++++++++++++--
>  include/linux/irqchip/arm-gic.h |   3 +
>  3 files changed, 162 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 439138d..92c4ea1 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 4b959e6..05b5d62 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -39,8 +39,10 @@
>  #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/fiq.h>
>  #include <asm/irq.h>
>  #include <asm/exception.h>
>  #include <asm/smp_plat.h>
> @@ -48,6 +50,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;
> @@ -71,6 +77,8 @@ struct gic_chip_data {
>  };
>  
>  static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> +/* A fiq-safe spinlock must only be locked when the FIQ is masked */
> +static DEFINE_RAW_SPINLOCK(fiq_safe_migration_lock);
>  
>  /*
>   * The GIC mapping of CPU interfaces does not necessarily match
> @@ -325,6 +333,94 @@ 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(void __iomem *base, unsigned int hwirq,
> +				int group)
> +{
> +	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 no bits
> +	 * set in IGROUP[0] (and all systems which do will have set bits).
> +	 */
> +	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + 0);
> +	if (!grp_val)
> +		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);
> +	writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
> +
> +	raw_spin_unlock(&irq_controller_lock);
> +}
> +
> +/*
> + * Test which group an interrupt belongs to.
> + *
> + * Returns 0 if the controller does not support grouping.
> + */
> +static int gic_get_group_irq(void __iomem *base, unsigned int hwirq)
> +{
> +	unsigned int grp_reg = hwirq / 32 * 4;
> +	u32 grp_val;
> +
> +	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
> +
> +	return (grp_val >> (hwirq % 32)) & 1;
> +}
> +
> +/*
> + * 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)
> @@ -373,7 +469,18 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>  
>  	gic_dist_config(base, gic_irqs, NULL);
>  
> -	writel_relaxed(1, base + GIC_DIST_CTRL);
> +	/*
> +	 * Set all global interrupts to be group 1 (this register is
> +	 * RAZ/WI if not accessible in current mode)
> +	 */
> +	for (i = 32; i < gic_irqs; i += 32)
> +		writel_relaxed(0xffffffff, base + GIC_DIST_IGROUP + i * 4 / 32);
> +
> +	/*
> +	 * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
> +	 * bit 1 ignored)
> +	 */
> +	writel_relaxed(3, base + GIC_DIST_CTRL);
>  }
>  
>  static void gic_cpu_init(struct gic_chip_data *gic)
> @@ -382,6 +489,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.
> @@ -400,8 +508,27 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>  
>  	gic_cpu_config(dist_base, NULL);
>  
> +	/*
> +	 * Set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
> +	 * to be group 1 (this register is RAZ/WI if not accessible)
> +	 */
> +	writel_relaxed(~SMP_IPI_FIQ_MASK, dist_base + GIC_DIST_IGROUP + 0);
> +
> +	/*
> +	 * Update the priority of any resulting group0 interrupts.
> +	 */
> +	secure_irqs = ~readl_relaxed(dist_base + GIC_DIST_IGROUP + 0);
> +	for_each_set_bit(secure_irq, &secure_irqs, 16)
> +		gic_set_group_irq(dist_base, i, 0);
> +
>  	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
> -	writel_relaxed(1, base + GIC_CPU_CTRL);
> +
> +	/* The bottom most bit will be set for all GIC variants (and is
> +	 * called Enable or EnableGrp0 depending on operating mode). The
> +	 * remaining four bits (CBPR, FIQEn, AckCtl and EnableGrp1) are
> +	 * RAZ/WI if not accessible.
> +	 */
> +	writel_relaxed(0x1f, base + GIC_CPU_CTRL);
>  }
>  
>  void gic_cpu_if_down(void)
> @@ -485,7 +612,7 @@ 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(1, dist_base + GIC_DIST_CTRL);
> +	writel_relaxed(3, dist_base + GIC_DIST_CTRL);
>  }
>  
>  static void gic_cpu_save(unsigned int gic_nr)
> @@ -542,7 +669,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>  		writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>  
>  	writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
> -	writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
> +	writel_relaxed(0x1f, cpu_base + GIC_CPU_CTRL);
>  }
>  
>  static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
> @@ -604,8 +731,19 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>  {
>  	int cpu;
>  	unsigned long flags, map = 0;
> +	unsigned long softint;
>  
> -	raw_spin_lock_irqsave(&irq_controller_lock, flags);
> +	/*
> +	 * The locking in this function ensures we don't use stale cpu mappings
> +	 * and thus we never route an IPI to the wrong physical core during a
> +	 * big.LITTLE switch. The switch code takes both of these locks meaning
> +	 * we can choose whichever lock is safe to use from our current calling
> +	 * context.
> +	 */
> +	if (in_nmi())
> +		raw_spin_lock(&fiq_safe_migration_lock);
> +	else
> +		raw_spin_lock_irqsave(&irq_controller_lock, flags);
>  
>  	/* Convert our logical CPU mask into a physical one. */
>  	for_each_cpu(cpu, mask)
> @@ -618,7 +756,15 @@ 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);
> +	softint = map << 16 | irq;
> +	if (gic_get_group_irq(gic_data_dist_base(&gic_data[0]), irq))
> +		softint |= 0x8000;
> +	writel_relaxed(softint,
> +		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> +
> +	if (in_nmi())
> +		raw_spin_unlock(&fiq_safe_migration_lock);
> +	else
>  
>  	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
>  }
> @@ -668,7 +814,7 @@ 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 with IRQ and FIQ locally disabled.
>   */
>  void gic_migrate_target(unsigned int new_cpu_id)
>  {
> @@ -690,6 +836,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
>  	ror_val = (cur_cpu_id - new_cpu_id) & 31;
>  
>  	raw_spin_lock(&irq_controller_lock);
> +	raw_spin_lock(&fiq_safe_migration_lock);
>  
>  	/* Update the target interface for this logical CPU */
>  	gic_cpu_map[cpu] = 1 << new_cpu_id;
> @@ -709,6 +856,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
>  		}
>  	}
>  
> +	raw_spin_unlock(&fiq_safe_migration_lock);
>  	raw_spin_unlock(&irq_controller_lock);
>  
>  	/*
> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> index 45e2d8c..52a5676 100644
> --- a/include/linux/irqchip/arm-gic.h
> +++ b/include/linux/irqchip/arm-gic.h
> @@ -101,5 +101,8 @@ static inline void __init register_routable_domain_ops
>  {
>  	gic_routable_irq_domain_ops = ops;
>  }
> +
> +void gic_handle_fiq_ipi(void);
> +
>  #endif /* __ASSEMBLY */
>  #endif
> -- 
> 1.9.3
> 

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Daniel Thompson]

On 17/09/14 11:51, Russell King - ARM Linux wrote:
> On Wed, Sep 17, 2014 at 09:10:16AM -0700, Daniel Thompson wrote:
>> This patch provides support for arm's newly added IPI FIQ. It works
>> by placing all interrupt sources *except* IPI FIQ in group 1 and
>> then flips a configuration bit in the GIC such that group 1
>> interrupts use IRQ and group 0 interrupts use FIQ.
>>
>> All GIC hardware except GICv1-without-TrustZone support provides a means
>> to group exceptions into group 0 and group 1. However 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" (a feature that allows grouping to be used to allow
>> hardware peripherals to send interrupts into the secure world). However
>> when grouping is not available we can rely on the GIC's RAZ/WI semantics
>> and avoid conditional code.
> 
> I've been chasing a bug with this on the Versatile Express CT9x4.  It
> seems that the GIC there is a GICv1, with secure extensions.  It seems
> to support interrupt grouping.
> 
> However, setting SPIs to group 1, with the control registers enabling
> both group 0 and group 1 (such that both groups are treated as IRQs)
> results in no SPIs being delivered to the kernel.  In other words,
> setting GIC_DIST_IGROUP for SPIs, and setting both GIC_CPU_CTRL and
> GIC_DIST_CTRL to 3.

I may have missed something but this sounds like the expected behaviour
to me.

Without AckCtl set (GIC_CPU_CTRL bit 2) then it is not possible to
acknowledge group 1 interrupts from secure mode. Thus in the
circumstances described above I would expect the system to wedge in the
interrupt handler because IRQ is raised but the software is not able to
acknowledge it.

I think there may also problems with leaving CBPR unset. Leaving CBPR
unset certainly causes a change of behaviour compared to an all group0
setup.


Daniel.


> This is rather worrying, because we seem to have a GIC which for all
> intents and purposes appears to be compatible with what we want to do,
> appears to conform with the GIC architecture specifications, but doesn't
> actually work.
> 
> I suspect that running the Versatile Express CT9x4 in non-secure mode
> wouldn't work (because in non-secure mode, the GIC only allows access
> to group 1 interrupts.)
> 
> I've added Will and Mark to this to see whether they have any comment.
> 
>> gic_raise_softirq() also needs changing to make it safe to call from
>> FIQ. This is due to its use by wake_up_klogd() during console_unlock().
>>
>> Tested on Freescale i.MX6 (quad A9, GICv1-with-TrustZone running in
>> secure mode).
>>
>> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Jason Cooper <jason@lakedaemon.net>
>> ---
>>  arch/arm/kernel/traps.c         |   5 +-
>>  drivers/irqchip/irq-gic.c       | 162 ++++++++++++++++++++++++++++++++++++++--
>>  include/linux/irqchip/arm-gic.h |   3 +
>>  3 files changed, 162 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
>> index 439138d..92c4ea1 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 4b959e6..05b5d62 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -39,8 +39,10 @@
>>  #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/fiq.h>
>>  #include <asm/irq.h>
>>  #include <asm/exception.h>
>>  #include <asm/smp_plat.h>
>> @@ -48,6 +50,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;
>> @@ -71,6 +77,8 @@ struct gic_chip_data {
>>  };
>>  
>>  static DEFINE_RAW_SPINLOCK(irq_controller_lock);
>> +/* A fiq-safe spinlock must only be locked when the FIQ is masked */
>> +static DEFINE_RAW_SPINLOCK(fiq_safe_migration_lock);
>>  
>>  /*
>>   * The GIC mapping of CPU interfaces does not necessarily match
>> @@ -325,6 +333,94 @@ 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(void __iomem *base, unsigned int hwirq,
>> +				int group)
>> +{
>> +	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 no bits
>> +	 * set in IGROUP[0] (and all systems which do will have set bits).
>> +	 */
>> +	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + 0);
>> +	if (!grp_val)
>> +		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);
>> +	writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
>> +
>> +	raw_spin_unlock(&irq_controller_lock);
>> +}
>> +
>> +/*
>> + * Test which group an interrupt belongs to.
>> + *
>> + * Returns 0 if the controller does not support grouping.
>> + */
>> +static int gic_get_group_irq(void __iomem *base, unsigned int hwirq)
>> +{
>> +	unsigned int grp_reg = hwirq / 32 * 4;
>> +	u32 grp_val;
>> +
>> +	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
>> +
>> +	return (grp_val >> (hwirq % 32)) & 1;
>> +}
>> +
>> +/*
>> + * 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)
>> @@ -373,7 +469,18 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
>>  
>>  	gic_dist_config(base, gic_irqs, NULL);
>>  
>> -	writel_relaxed(1, base + GIC_DIST_CTRL);
>> +	/*
>> +	 * Set all global interrupts to be group 1 (this register is
>> +	 * RAZ/WI if not accessible in current mode)
>> +	 */
>> +	for (i = 32; i < gic_irqs; i += 32)
>> +		writel_relaxed(0xffffffff, base + GIC_DIST_IGROUP + i * 4 / 32);
>> +
>> +	/*
>> +	 * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
>> +	 * bit 1 ignored)
>> +	 */
>> +	writel_relaxed(3, base + GIC_DIST_CTRL);
>>  }
>>  
>>  static void gic_cpu_init(struct gic_chip_data *gic)
>> @@ -382,6 +489,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.
>> @@ -400,8 +508,27 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>  
>>  	gic_cpu_config(dist_base, NULL);
>>  
>> +	/*
>> +	 * Set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
>> +	 * to be group 1 (this register is RAZ/WI if not accessible)
>> +	 */
>> +	writel_relaxed(~SMP_IPI_FIQ_MASK, dist_base + GIC_DIST_IGROUP + 0);
>> +
>> +	/*
>> +	 * Update the priority of any resulting group0 interrupts.
>> +	 */
>> +	secure_irqs = ~readl_relaxed(dist_base + GIC_DIST_IGROUP + 0);
>> +	for_each_set_bit(secure_irq, &secure_irqs, 16)
>> +		gic_set_group_irq(dist_base, i, 0);
>> +
>>  	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>> -	writel_relaxed(1, base + GIC_CPU_CTRL);
>> +
>> +	/* The bottom most bit will be set for all GIC variants (and is
>> +	 * called Enable or EnableGrp0 depending on operating mode). The
>> +	 * remaining four bits (CBPR, FIQEn, AckCtl and EnableGrp1) are
>> +	 * RAZ/WI if not accessible.
>> +	 */
>> +	writel_relaxed(0x1f, base + GIC_CPU_CTRL);
>>  }
>>  
>>  void gic_cpu_if_down(void)
>> @@ -485,7 +612,7 @@ 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(1, dist_base + GIC_DIST_CTRL);
>> +	writel_relaxed(3, dist_base + GIC_DIST_CTRL);
>>  }
>>  
>>  static void gic_cpu_save(unsigned int gic_nr)
>> @@ -542,7 +669,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>  		writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>  
>>  	writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>> -	writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>> +	writel_relaxed(0x1f, cpu_base + GIC_CPU_CTRL);
>>  }
>>  
>>  static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
>> @@ -604,8 +731,19 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>>  {
>>  	int cpu;
>>  	unsigned long flags, map = 0;
>> +	unsigned long softint;
>>  
>> -	raw_spin_lock_irqsave(&irq_controller_lock, flags);
>> +	/*
>> +	 * The locking in this function ensures we don't use stale cpu mappings
>> +	 * and thus we never route an IPI to the wrong physical core during a
>> +	 * big.LITTLE switch. The switch code takes both of these locks meaning
>> +	 * we can choose whichever lock is safe to use from our current calling
>> +	 * context.
>> +	 */
>> +	if (in_nmi())
>> +		raw_spin_lock(&fiq_safe_migration_lock);
>> +	else
>> +		raw_spin_lock_irqsave(&irq_controller_lock, flags);
>>  
>>  	/* Convert our logical CPU mask into a physical one. */
>>  	for_each_cpu(cpu, mask)
>> @@ -618,7 +756,15 @@ 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);
>> +	softint = map << 16 | irq;
>> +	if (gic_get_group_irq(gic_data_dist_base(&gic_data[0]), irq))
>> +		softint |= 0x8000;
>> +	writel_relaxed(softint,
>> +		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
>> +
>> +	if (in_nmi())
>> +		raw_spin_unlock(&fiq_safe_migration_lock);
>> +	else
>>  
>>  	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
>>  }
>> @@ -668,7 +814,7 @@ 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 with IRQ and FIQ locally disabled.
>>   */
>>  void gic_migrate_target(unsigned int new_cpu_id)
>>  {
>> @@ -690,6 +836,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
>>  	ror_val = (cur_cpu_id - new_cpu_id) & 31;
>>  
>>  	raw_spin_lock(&irq_controller_lock);
>> +	raw_spin_lock(&fiq_safe_migration_lock);
>>  
>>  	/* Update the target interface for this logical CPU */
>>  	gic_cpu_map[cpu] = 1 << new_cpu_id;
>> @@ -709,6 +856,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
>>  		}
>>  	}
>>  
>> +	raw_spin_unlock(&fiq_safe_migration_lock);
>>  	raw_spin_unlock(&irq_controller_lock);
>>  
>>  	/*
>> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
>> index 45e2d8c..52a5676 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -101,5 +101,8 @@ static inline void __init register_routable_domain_ops
>>  {
>>  	gic_routable_irq_domain_ops = ops;
>>  }
>> +
>> +void gic_handle_fiq_ipi(void);
>> +
>>  #endif /* __ASSEMBLY */
>>  #endif
>> -- 
>> 1.9.3
>>
> 

Re: [PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Russell King - ARM Linux]

On Wed, Sep 17, 2014 at 01:12:23PM -0700, Daniel Thompson wrote:
> On 17/09/14 11:51, Russell King - ARM Linux wrote:
> > On Wed, Sep 17, 2014 at 09:10:16AM -0700, Daniel Thompson wrote:
> >> This patch provides support for arm's newly added IPI FIQ. It works
> >> by placing all interrupt sources *except* IPI FIQ in group 1 and
> >> then flips a configuration bit in the GIC such that group 1
> >> interrupts use IRQ and group 0 interrupts use FIQ.
> >>
> >> All GIC hardware except GICv1-without-TrustZone support provides a means
> >> to group exceptions into group 0 and group 1. However 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" (a feature that allows grouping to be used to allow
> >> hardware peripherals to send interrupts into the secure world). However
> >> when grouping is not available we can rely on the GIC's RAZ/WI semantics
> >> and avoid conditional code.
> > 
> > I've been chasing a bug with this on the Versatile Express CT9x4.  It
> > seems that the GIC there is a GICv1, with secure extensions.  It seems
> > to support interrupt grouping.
> > 
> > However, setting SPIs to group 1, with the control registers enabling
> > both group 0 and group 1 (such that both groups are treated as IRQs)
> > results in no SPIs being delivered to the kernel.  In other words,
> > setting GIC_DIST_IGROUP for SPIs, and setting both GIC_CPU_CTRL and
> > GIC_DIST_CTRL to 3.
> 
> I may have missed something but this sounds like the expected behaviour
> to me.
> 
> Without AckCtl set (GIC_CPU_CTRL bit 2) then it is not possible to
> acknowledge group 1 interrupts from secure mode. Thus in the
> circumstances described above I would expect the system to wedge in the
> interrupt handler because IRQ is raised but the software is not able to
> acknowledge it.

Setting AckCtl (0x07) results in no change in behaviour - still crashes
when it tries to calibrate the timer.

> I think there may also problems with leaving CBPR unset. Leaving CBPR
> unset certainly causes a change of behaviour compared to an all group0
> setup.

Also tried setting that (0x17), but still the same.

> Daniel.
> 
> 
> > This is rather worrying, because we seem to have a GIC which for all
> > intents and purposes appears to be compatible with what we want to do,
> > appears to conform with the GIC architecture specifications, but doesn't
> > actually work.
> > 
> > I suspect that running the Versatile Express CT9x4 in non-secure mode
> > wouldn't work (because in non-secure mode, the GIC only allows access
> > to group 1 interrupts.)
> > 
> > I've added Will and Mark to this to see whether they have any comment.
> > 
> >> gic_raise_softirq() also needs changing to make it safe to call from
> >> FIQ. This is due to its use by wake_up_klogd() during console_unlock().
> >>
> >> Tested on Freescale i.MX6 (quad A9, GICv1-with-TrustZone running in
> >> secure mode).
> >>
> >> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> >> Cc: Russell King <linux@arm.linux.org.uk>
> >> Cc: Thomas Gleixner <tglx@linutronix.de>
> >> Cc: Jason Cooper <jason@lakedaemon.net>
> >> ---
> >>  arch/arm/kernel/traps.c         |   5 +-
> >>  drivers/irqchip/irq-gic.c       | 162 ++++++++++++++++++++++++++++++++++++++--
> >>  include/linux/irqchip/arm-gic.h |   3 +
> >>  3 files changed, 162 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> >> index 439138d..92c4ea1 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 4b959e6..05b5d62 100644
> >> --- a/drivers/irqchip/irq-gic.c
> >> +++ b/drivers/irqchip/irq-gic.c
> >> @@ -39,8 +39,10 @@
> >>  #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/fiq.h>
> >>  #include <asm/irq.h>
> >>  #include <asm/exception.h>
> >>  #include <asm/smp_plat.h>
> >> @@ -48,6 +50,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;
> >> @@ -71,6 +77,8 @@ struct gic_chip_data {
> >>  };
> >>  
> >>  static DEFINE_RAW_SPINLOCK(irq_controller_lock);
> >> +/* A fiq-safe spinlock must only be locked when the FIQ is masked */
> >> +static DEFINE_RAW_SPINLOCK(fiq_safe_migration_lock);
> >>  
> >>  /*
> >>   * The GIC mapping of CPU interfaces does not necessarily match
> >> @@ -325,6 +333,94 @@ 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(void __iomem *base, unsigned int hwirq,
> >> +				int group)
> >> +{
> >> +	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 no bits
> >> +	 * set in IGROUP[0] (and all systems which do will have set bits).
> >> +	 */
> >> +	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + 0);
> >> +	if (!grp_val)
> >> +		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);
> >> +	writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);
> >> +
> >> +	raw_spin_unlock(&irq_controller_lock);
> >> +}
> >> +
> >> +/*
> >> + * Test which group an interrupt belongs to.
> >> + *
> >> + * Returns 0 if the controller does not support grouping.
> >> + */
> >> +static int gic_get_group_irq(void __iomem *base, unsigned int hwirq)
> >> +{
> >> +	unsigned int grp_reg = hwirq / 32 * 4;
> >> +	u32 grp_val;
> >> +
> >> +	grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg);
> >> +
> >> +	return (grp_val >> (hwirq % 32)) & 1;
> >> +}
> >> +
> >> +/*
> >> + * 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)
> >> @@ -373,7 +469,18 @@ static void __init gic_dist_init(struct gic_chip_data *gic)
> >>  
> >>  	gic_dist_config(base, gic_irqs, NULL);
> >>  
> >> -	writel_relaxed(1, base + GIC_DIST_CTRL);
> >> +	/*
> >> +	 * Set all global interrupts to be group 1 (this register is
> >> +	 * RAZ/WI if not accessible in current mode)
> >> +	 */
> >> +	for (i = 32; i < gic_irqs; i += 32)
> >> +		writel_relaxed(0xffffffff, base + GIC_DIST_IGROUP + i * 4 / 32);
> >> +
> >> +	/*
> >> +	 * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only,
> >> +	 * bit 1 ignored)
> >> +	 */
> >> +	writel_relaxed(3, base + GIC_DIST_CTRL);
> >>  }
> >>  
> >>  static void gic_cpu_init(struct gic_chip_data *gic)
> >> @@ -382,6 +489,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.
> >> @@ -400,8 +508,27 @@ static void gic_cpu_init(struct gic_chip_data *gic)
> >>  
> >>  	gic_cpu_config(dist_base, NULL);
> >>  
> >> +	/*
> >> +	 * Set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK
> >> +	 * to be group 1 (this register is RAZ/WI if not accessible)
> >> +	 */
> >> +	writel_relaxed(~SMP_IPI_FIQ_MASK, dist_base + GIC_DIST_IGROUP + 0);
> >> +
> >> +	/*
> >> +	 * Update the priority of any resulting group0 interrupts.
> >> +	 */
> >> +	secure_irqs = ~readl_relaxed(dist_base + GIC_DIST_IGROUP + 0);
> >> +	for_each_set_bit(secure_irq, &secure_irqs, 16)
> >> +		gic_set_group_irq(dist_base, i, 0);
> >> +
> >>  	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
> >> -	writel_relaxed(1, base + GIC_CPU_CTRL);
> >> +
> >> +	/* The bottom most bit will be set for all GIC variants (and is
> >> +	 * called Enable or EnableGrp0 depending on operating mode). The
> >> +	 * remaining four bits (CBPR, FIQEn, AckCtl and EnableGrp1) are
> >> +	 * RAZ/WI if not accessible.
> >> +	 */
> >> +	writel_relaxed(0x1f, base + GIC_CPU_CTRL);
> >>  }
> >>  
> >>  void gic_cpu_if_down(void)
> >> @@ -485,7 +612,7 @@ 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(1, dist_base + GIC_DIST_CTRL);
> >> +	writel_relaxed(3, dist_base + GIC_DIST_CTRL);
> >>  }
> >>  
> >>  static void gic_cpu_save(unsigned int gic_nr)
> >> @@ -542,7 +669,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
> >>  		writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
> >>  
> >>  	writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
> >> -	writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
> >> +	writel_relaxed(0x1f, cpu_base + GIC_CPU_CTRL);
> >>  }
> >>  
> >>  static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
> >> @@ -604,8 +731,19 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
> >>  {
> >>  	int cpu;
> >>  	unsigned long flags, map = 0;
> >> +	unsigned long softint;
> >>  
> >> -	raw_spin_lock_irqsave(&irq_controller_lock, flags);
> >> +	/*
> >> +	 * The locking in this function ensures we don't use stale cpu mappings
> >> +	 * and thus we never route an IPI to the wrong physical core during a
> >> +	 * big.LITTLE switch. The switch code takes both of these locks meaning
> >> +	 * we can choose whichever lock is safe to use from our current calling
> >> +	 * context.
> >> +	 */
> >> +	if (in_nmi())
> >> +		raw_spin_lock(&fiq_safe_migration_lock);
> >> +	else
> >> +		raw_spin_lock_irqsave(&irq_controller_lock, flags);
> >>  
> >>  	/* Convert our logical CPU mask into a physical one. */
> >>  	for_each_cpu(cpu, mask)
> >> @@ -618,7 +756,15 @@ 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);
> >> +	softint = map << 16 | irq;
> >> +	if (gic_get_group_irq(gic_data_dist_base(&gic_data[0]), irq))
> >> +		softint |= 0x8000;
> >> +	writel_relaxed(softint,
> >> +		       gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
> >> +
> >> +	if (in_nmi())
> >> +		raw_spin_unlock(&fiq_safe_migration_lock);
> >> +	else
> >>  
> >>  	raw_spin_unlock_irqrestore(&irq_controller_lock, flags);
> >>  }
> >> @@ -668,7 +814,7 @@ 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 with IRQ and FIQ locally disabled.
> >>   */
> >>  void gic_migrate_target(unsigned int new_cpu_id)
> >>  {
> >> @@ -690,6 +836,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
> >>  	ror_val = (cur_cpu_id - new_cpu_id) & 31;
> >>  
> >>  	raw_spin_lock(&irq_controller_lock);
> >> +	raw_spin_lock(&fiq_safe_migration_lock);
> >>  
> >>  	/* Update the target interface for this logical CPU */
> >>  	gic_cpu_map[cpu] = 1 << new_cpu_id;
> >> @@ -709,6 +856,7 @@ void gic_migrate_target(unsigned int new_cpu_id)
> >>  		}
> >>  	}
> >>  
> >> +	raw_spin_unlock(&fiq_safe_migration_lock);
> >>  	raw_spin_unlock(&irq_controller_lock);
> >>  
> >>  	/*
> >> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
> >> index 45e2d8c..52a5676 100644
> >> --- a/include/linux/irqchip/arm-gic.h
> >> +++ b/include/linux/irqchip/arm-gic.h
> >> @@ -101,5 +101,8 @@ static inline void __init register_routable_domain_ops
> >>  {
> >>  	gic_routable_irq_domain_ops = ops;
> >>  }
> >> +
> >> +void gic_handle_fiq_ipi(void);
> >> +
> >>  #endif /* __ASSEMBLY */
> >>  #endif
> >> -- 
> >> 1.9.3
> >>
> > 
> 

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Russell King - ARM Linux]

On Wed, Sep 17, 2014 at 10:07:13PM +0100, Russell King - ARM Linux wrote:
> On Wed, Sep 17, 2014 at 01:12:23PM -0700, Daniel Thompson wrote:
> > I may have missed something but this sounds like the expected behaviour
> > to me.
> > 
> > Without AckCtl set (GIC_CPU_CTRL bit 2) then it is not possible to
> > acknowledge group 1 interrupts from secure mode. Thus in the
> > circumstances described above I would expect the system to wedge in the
> > interrupt handler because IRQ is raised but the software is not able to
> > acknowledge it.
> 
> Setting AckCtl (0x07) results in no change in behaviour - still crashes
> when it tries to calibrate the timer.
> 
> > I think there may also problems with leaving CBPR unset. Leaving CBPR
> > unset certainly causes a change of behaviour compared to an all group0
> > setup.
> 
> Also tried setting that (0x17), but still the same.

Another data point: SDP4430 boots fine with 0x17.

The Versatile Express has GIC ID/TYPER register values of:

GICC IDR: 0x3901043b GICD TYPER: 0x0000fc62

The SDP4430 has:

GICC IDR: 0x3901043b GICD TYPER: 0x0000fc24

so they should in theory be identical GICs, and should behave identically,
but they don't.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Russell King - ARM Linux]

On Wed, Sep 17, 2014 at 09:10:16AM -0700, Daniel Thompson wrote:
> @@ -604,8 +731,19 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>  {
>  	int cpu;
>  	unsigned long flags, map = 0;
> +	unsigned long softint;
>  
> -	raw_spin_lock_irqsave(&irq_controller_lock, flags);
> +	/*
> +	 * The locking in this function ensures we don't use stale cpu mappings
> +	 * and thus we never route an IPI to the wrong physical core during a
> +	 * big.LITTLE switch. The switch code takes both of these locks meaning
> +	 * we can choose whichever lock is safe to use from our current calling
> +	 * context.
> +	 */
> +	if (in_nmi())
> +		raw_spin_lock(&fiq_safe_migration_lock);
> +	else
> +		raw_spin_lock_irqsave(&irq_controller_lock, flags);

BTW, I see this code is still here...

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Daniel Thompson]

On 18/09/14 01:17, Russell King - ARM Linux wrote:
> On Wed, Sep 17, 2014 at 09:10:16AM -0700, Daniel Thompson wrote:
>> @@ -604,8 +731,19 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>>  {
>>  	int cpu;
>>  	unsigned long flags, map = 0;
>> +	unsigned long softint;
>>  
>> -	raw_spin_lock_irqsave(&irq_controller_lock, flags);
>> +	/*
>> +	 * The locking in this function ensures we don't use stale cpu mappings
>> +	 * and thus we never route an IPI to the wrong physical core during a
>> +	 * big.LITTLE switch. The switch code takes both of these locks meaning
>> +	 * we can choose whichever lock is safe to use from our current calling
>> +	 * context.
>> +	 */
>> +	if (in_nmi())
>> +		raw_spin_lock(&fiq_safe_migration_lock);
>> +	else
>> +		raw_spin_lock_irqsave(&irq_controller_lock, flags);
> 
> BTW, I see this code is still here...

Quite so.

I'm afraid I haven't yet re-written it to use r/w locks (as proposed in
mails from the weekend) but I had to respin the default FIQ handler
patch to fix the CONFIG_FIQ build problem I introduced.

Re: [PATCH 3.17-rc4 v7 4/6] irqchip: gic: Add support for IPI FIQ

[Marc Zyngier]

On Wed, Sep 17 2014 at 07:51:38 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

Hi Russell,

> On Wed, Sep 17, 2014 at 09:10:16AM -0700, Daniel Thompson wrote:
>> This patch provides support for arm's newly added IPI FIQ. It works
>> by placing all interrupt sources *except* IPI FIQ in group 1 and
>> then flips a configuration bit in the GIC such that group 1
>> interrupts use IRQ and group 0 interrupts use FIQ.
>>
>> All GIC hardware except GICv1-without-TrustZone support provides a means
>> to group exceptions into group 0 and group 1. However 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" (a feature that allows grouping to be used to allow
>> hardware peripherals to send interrupts into the secure world). However
>> when grouping is not available we can rely on the GIC's RAZ/WI semantics
>> and avoid conditional code.
>
> I've been chasing a bug with this on the Versatile Express CT9x4.  It
> seems that the GIC there is a GICv1, with secure extensions.  It seems
> to support interrupt grouping.
>
> However, setting SPIs to group 1, with the control registers enabling
> both group 0 and group 1 (such that both groups are treated as IRQs)
> results in no SPIs being delivered to the kernel.  In other words,
> setting GIC_DIST_IGROUP for SPIs, and setting both GIC_CPU_CTRL and
> GIC_DIST_CTRL to 3.
>
> This is rather worrying, because we seem to have a GIC which for all
> intents and purposes appears to be compatible with what we want to do,
> appears to conform with the GIC architecture specifications, but doesn't
> actually work.
>
> I suspect that running the Versatile Express CT9x4 in non-secure mode
> wouldn't work (because in non-secure mode, the GIC only allows access
> to group 1 interrupts.)
>
> I've added Will and Mark to this to see whether they have any comment.

I'm rather far away from my VE-A9 board (and won't be to get back to it
for another two weeks), so this is all a shot in the dark...

Can you have a look at the GICC_AIAR register (located at GICC_IAR +
0x14)? It *shouldn't* exist on this HW, assuming this is a real
GICv1. But what you describe makes me think of something like this.

Thanks,

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

Re: [PATCH 3.17-rc4 v7 3/6] arm64: Introduce dummy version of asm/fiq.h

[Catalin Marinas]

On Wed, Sep 17, 2014 at 05:10:15PM +0100, Daniel Thompson wrote:
> Drivers that are shared between arm and arm64 and which employ
> FIQ on arm cannot include asm/fiq.h without #ifdef'ing. This patch
> introduces a dummy version of asm/fiq.h to arm64 to avoid this.
> 
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Daniel Drake]

Hi,

Thanks a lot for working on this!

On Wed, Sep 17, 2014 at 10:10 AM, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
> 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

What's the right way to extend your work in order to get a NMI-like
watchdog hard lockup detector similar to the one on x86?

I'm testing your patches on Exynos4412 and I guess in their current
state they don't go quite this deep, as the only callers of
trigger_all_cpu_backtrace() are sysrq, hung_task and spinlock debug
code - none of which seem as fail-safe as a trigger like a
pre-programmed watchdog NMI interrupt would be.

Do I need to find a way to get CONFIG_FIQ available on this platform
first? and/or CONFIG_HARDLOCKUP_DETECTOR?

Thanks
Daniel

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Russell King - ARM Linux]

On Tue, Oct 14, 2014 at 04:37:51PM -0600, Daniel Drake wrote:
> Hi,
> 
> Thanks a lot for working on this!
> 
> On Wed, Sep 17, 2014 at 10:10 AM, Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> > 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
> 
> What's the right way to extend your work in order to get a NMI-like
> watchdog hard lockup detector similar to the one on x86?
> 
> I'm testing your patches on Exynos4412 and I guess in their current
> state they don't go quite this deep, as the only callers of
> trigger_all_cpu_backtrace() are sysrq, hung_task and spinlock debug
> code - none of which seem as fail-safe as a trigger like a
> pre-programmed watchdog NMI interrupt would be.
> 
> Do I need to find a way to get CONFIG_FIQ available on this platform
> first? and/or CONFIG_HARDLOCKUP_DETECTOR?

The blocker on this work right now is the annoying Versatile Express
platform, which pretty much means that we currently can't push the
code into the GIC to support FIQs.  As long as adding FIQ support to
the GIC results in the Versatile Express becoming non-bootable, the
idea of using FIQs is a total non-starter.

Or we decide that we dump the platform completely (which will upset
a number of developers.)

I have patches I'm using for trigger_all_cpu_backtrace() which I'm
maintaining privately in my tree until we can get the FIQ situation
sorted.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Daniel Thompson]

On 14/10/14 23:37, Daniel Drake wrote:
> Hi,
> 
> Thanks a lot for working on this!
> 
> On Wed, Sep 17, 2014 at 10:10 AM, Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
>> 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
> 
> What's the right way to extend your work in order to get a NMI-like
> watchdog hard lockup detector similar to the one on x86?

There are a few things to get into place for this.

1. Figure out what number to put into the PMU to get an interrupt every
   10s and provide the stub functions for the lock up detector.

2. Modify the current ARM PMU support to make is possible for this code
   to run from a FIQ handler. This should be feasible by replicating
   the design pattern used on x86. Nevertheless this is a fairly big
   chunk of code review and testing.

3. Modify the Linux IRQ support to allow some kind of flag to
   hint/demand that an interrupt be treated as NMI-ish in order to
   switch (unshared) interrupts into FIQ mode and hook this up in
   the GIC.

   [Side note, this approach was suggested by Thomas Gleixner in
   response to some rather hacky patches from me. My patches are
   robust enough but are poorly designed and hard to maintain.
   Thus if you want to do any quick prototyping you might skip this
   step and dig out my old patches:

https://git.linaro.org/people/daniel.thompson/linux.git/shortlog/refs/heads/dev/kdb-fiq

Note also that, as a side effect of the above, tools like oprofile would
also get a very significant boost for kernel profiling because they
would no longer attribute time spent in interrupt handlers to interrupt
unmask functions.

At present I've done a little work towards all three of the above but
none are complete (most of the code has never been executed).


> I'm testing your patches on Exynos4412 and I guess in their current
> state they don't go quite this deep, as the only callers of
> trigger_all_cpu_backtrace() are sysrq, hung_task and spinlock debug
> code - none of which seem as fail-safe as a trigger like a
> pre-programmed watchdog NMI interrupt would be.
> 
> Do I need to find a way to get CONFIG_FIQ available on this platform
> first? and/or CONFIG_HARDLOCKUP_DETECTOR?

You need CONFIG_FIQ working first. Be aware that this may be impossible
on Exynos unless you control the TrustZone. For this reason most of my
development is on Freescale i.MX6 (because i.MX6 boots in secure mode).


Daniel.

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Daniel Thompson]

On 15/10/14 00:31, Russell King - ARM Linux wrote:
> On Tue, Oct 14, 2014 at 04:37:51PM -0600, Daniel Drake wrote:
>> Hi,
>>
>> Thanks a lot for working on this!
>>
>> On Wed, Sep 17, 2014 at 10:10 AM, Daniel Thompson
>> <daniel.thompson@linaro.org> wrote:
>>> 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
>>
>> What's the right way to extend your work in order to get a NMI-like
>> watchdog hard lockup detector similar to the one on x86?
>>
>> I'm testing your patches on Exynos4412 and I guess in their current
>> state they don't go quite this deep, as the only callers of
>> trigger_all_cpu_backtrace() are sysrq, hung_task and spinlock debug
>> code - none of which seem as fail-safe as a trigger like a
>> pre-programmed watchdog NMI interrupt would be.
>>
>> Do I need to find a way to get CONFIG_FIQ available on this platform
>> first? and/or CONFIG_HARDLOCKUP_DETECTOR?
> 
> The blocker on this work right now is the annoying Versatile Express
> platform, which pretty much means that we currently can't push the
> code into the GIC to support FIQs.  As long as adding FIQ support to
> the GIC results in the Versatile Express becoming non-bootable, the
> idea of using FIQs is a total non-starter.
> 
> Or we decide that we dump the platform completely (which will upset
> a number of developers.)
> 
> I have patches I'm using for trigger_all_cpu_backtrace() which I'm
> maintaining privately in my tree until we can get the FIQ situation
> sorted.

I do hope to gain (remote) access to a vexpress at some point just to
pick at this issue a little.

That said your previous description of the issue and of the GIC version
register leaves very little to explore!

In the end I'm expecting to have to use some kind of black list logic. I
assuming that vexpress by its nature as a bring up platform is more
likely to exhibit problems in this sort of area and therefore a black
list is more appropriate than a white list.


Daniel.

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Russell King - ARM Linux]

On Thu, Oct 16, 2014 at 10:23:52AM +0100, Daniel Thompson wrote:
> On 14/10/14 23:37, Daniel Drake wrote:
> > I'm testing your patches on Exynos4412 and I guess in their current
> > state they don't go quite this deep, as the only callers of
> > trigger_all_cpu_backtrace() are sysrq, hung_task and spinlock debug
> > code - none of which seem as fail-safe as a trigger like a
> > pre-programmed watchdog NMI interrupt would be.
> > 
> > Do I need to find a way to get CONFIG_FIQ available on this platform
> > first? and/or CONFIG_HARDLOCKUP_DETECTOR?
> 
> You need CONFIG_FIQ working first. Be aware that this may be impossible
> on Exynos unless you control the TrustZone. For this reason most of my
> development is on Freescale i.MX6 (because i.MX6 boots in secure mode).

CONFIG_FIQ enables the legacy FIQ code which is unsuitable for use on
SMP, so that should not be a requirement.

We still need to validate all the code we're proposing to run in FIQ
context does not violate any locking.  IRQ-safe locks will do not
prevent FIQs occuring, and using IRQ-safe locks which are also taken
in the FIQ path /will/ cause deadlocks.  So, we need to ensure that
the perf internals are safe for this.

Lastly, platforms running in non-secure mode most likely will not be
able to take /any/ advantage from the FIQ stuff because FIQs will 
likely only be available to the secure firmware.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Daniel Thompson]

On 16/10/14 13:23, Russell King - ARM Linux wrote:
> On Thu, Oct 16, 2014 at 10:23:52AM +0100, Daniel Thompson wrote:
>> On 14/10/14 23:37, Daniel Drake wrote:
>>> I'm testing your patches on Exynos4412 and I guess in their current
>>> state they don't go quite this deep, as the only callers of
>>> trigger_all_cpu_backtrace() are sysrq, hung_task and spinlock debug
>>> code - none of which seem as fail-safe as a trigger like a
>>> pre-programmed watchdog NMI interrupt would be.
>>>
>>> Do I need to find a way to get CONFIG_FIQ available on this platform
>>> first? and/or CONFIG_HARDLOCKUP_DETECTOR?
>>
>> You need CONFIG_FIQ working first. Be aware that this may be impossible
>> on Exynos unless you control the TrustZone. For this reason most of my
>> development is on Freescale i.MX6 (because i.MX6 boots in secure mode).
> 
> CONFIG_FIQ enables the legacy FIQ code which is unsuitable for use on
> SMP, so that should not be a requirement.

Sorry. That was rather stupid phrasing on my part.

What I mean is that before doing any other work related to FIQ one
should establish that the platform really is using FIQ to trigger
backtraces! On platforms where FIQ cannot be supported the code falls
back to using IRQs (making the IRQ handling easily spotted in the
backtrace).

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Daniel Thompson]

On 16/10/14 10:33, Daniel Thompson wrote:
>> The blocker on this work right now is the annoying Versatile Express
>> platform, which pretty much means that we currently can't push the
>> code into the GIC to support FIQs.  As long as adding FIQ support to
>> the GIC results in the Versatile Express becoming non-bootable, the
>> idea of using FIQs is a total non-starter.
>>
>> Or we decide that we dump the platform completely (which will upset
>> a number of developers.)
>>
>> I have patches I'm using for trigger_all_cpu_backtrace() which I'm
>> maintaining privately in my tree until we can get the FIQ situation
>> sorted.
> 
> I do hope to gain (remote) access to a vexpress at some point just to
> pick at this issue a little.

This week with the help of one of my colleagues (thanks Tixy) I have
been able to run some tests and figure out what it going on on vexpress-a9.

The summary is that on some GICv1 implementations the bit to enable
group 1 interrupts cannot be accessed using secure memory accesses. More
specifically the presence/absence of the EnableGrp1 bit in the secure
version GICD_CTRL register is implementation defined.

My original patches overlooked this and as a result the existing code
will migrate all interrupts to group but then cannot enable delivery of
group 1 interrupts.

I'm planning to respin the code so it will automatically disable FIQ
support when the EnableGrp1 bit is not implemented. This means
vexpress-a9 will not benefit from FIQ support but will also not be
harmed by it.

Re: [PATCH 3.17-rc4 v7 0/6] arm: Implement arch_trigger_all_cpu_backtrace

[Russell King - ARM Linux]

On Tue, Nov 04, 2014 at 05:05:25PM +0000, Daniel Thompson wrote:
> This week with the help of one of my colleagues (thanks Tixy) I have
> been able to run some tests and figure out what it going on on vexpress-a9.
> 
> The summary is that on some GICv1 implementations the bit to enable
> group 1 interrupts cannot be accessed using secure memory accesses. More
> specifically the presence/absence of the EnableGrp1 bit in the secure
> version GICD_CTRL register is implementation defined.
> 
> My original patches overlooked this and as a result the existing code
> will migrate all interrupts to group but then cannot enable delivery of
> group 1 interrupts.
> 
> I'm planning to respin the code so it will automatically disable FIQ
> support when the EnableGrp1 bit is not implemented. This means
> vexpress-a9 will not benefit from FIQ support but will also not be
> harmed by it.

That's good news, and it should mean that the Versatile Express starts
booting on the nightly boot tests again.  Thanks for looking into it.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.