Currently calling printk() from a FIQ can result in deadlock on irq_controller_lock within gic_raise_softirq(). This occurs because printk(), which is otherwise structured to survive calls from FIQ/NMI, calls this function to raise an IPI when it needs to wake_up_klogd().

This patch fixes the problem by introducing an additional rwlock and using that to prevent softirqs being raised whilst the b.L switcher is updating the cpu map.

Other parts of the code are not updated to use the new fiq_safe_cpu_map_lock because other users of gic_cpu_map either rely on external locking or upon irq_controller_lock. Both locks are held by the b.L switcher code.

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com --- drivers/irqchip/irq-gic.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..0db62a6f1ee3 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,13 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock);

/* + * This lock may be locked for reading by FIQ handlers. Thus although + * read locking may be used liberally, write locking must only take + * place only when local FIQ handling is disabled. + */ +static DEFINE_RWLOCK(fiq_safe_cpu_map_lock); + +/* * The GIC mapping of CPU interfaces does not necessarily match * the logical CPU numbering. Let's use a mapping as returned * by the GIC itself. @@ -624,7 +631,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

- raw_spin_lock_irqsave(&irq_controller_lock, flags); + read_lock_irqsave(&fiq_safe_cpu_map_lock, flags);

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -639,7 +646,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- raw_spin_unlock_irqrestore(&irq_controller_lock, flags); + read_unlock_irqrestore(&fiq_safe_cpu_map_lock, flags); } #endif

@@ -687,7 +694,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) { @@ -709,6 +716,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); + write_lock(&fiq_safe_cpu_map_lock);

/* Update the target interface for this logical CPU */ gic_cpu_map[cpu] = 1 << new_cpu_id; @@ -728,6 +736,7 @@ void gic_migrate_target(unsigned int new_cpu_id) } }

+ write_unlock(&fiq_safe_cpu_map_lock); raw_spin_unlock(&irq_controller_lock);

/*

On 24/11/14 18:20, Thomas Gleixner wrote:

On Fri, 14 Nov 2014, Daniel Thompson wrote:

...

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..0db62a6f1ee3 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,13 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock); /*

• • This lock may be locked for reading by FIQ handlers. Thus although
• • read locking may be used liberally, write locking must only take
• • place only when local FIQ handling is disabled.
• */

+static DEFINE_RWLOCK(fiq_safe_cpu_map_lock);

+/*

• The GIC mapping of CPU interfaces does not necessarily match
• the logical CPU numbering. Let's use a mapping as returned
• by the GIC itself.

@@ -624,7 +631,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

• raw_spin_lock_irqsave(&irq_controller_lock, flags);

• read_lock_irqsave(&fiq_safe_cpu_map_lock, flags);

Just for the record:

You might have noticed that you replace a raw lock with a non raw one. That's not an issue on mainline, but that pretty much renders that code broken for RT.

Indeed. For that reason I've been pretty anxious to hear your views on this one.

Older versions of this patch did retain the raw lock but the code ends up looking a bit weird and resulted in negative comments during review:

if (in_nmi()) raw_spin_lock(&fiq_exclusive_cpu_map_lock); else raw_spin_lock_irqsave(&irq_controller_lock, flags);

The above form relies for correctness on the fact the b.L switcher code can take both locks and already runs with FIQ disabled.

Surely nothing I worry too much about given the current state of RT.

Hobby or not, I don't want to make your work here any harder. I could go back to the old form.

Alternatively I could provide a patch to go in -rt that converts the rw locks to spin locks but that just sounds like a maintenance hassle for you.

Daniel.

On 24/11/14 18:48, Thomas Gleixner wrote:

On Mon, 24 Nov 2014, Thomas Gleixner wrote:

...

On Fri, 14 Nov 2014, Daniel Thompson wrote:

...

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..0db62a6f1ee3 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,13 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock); /*

• • This lock may be locked for reading by FIQ handlers. Thus although
• • read locking may be used liberally, write locking must only take
• • place only when local FIQ handling is disabled.
• */

+static DEFINE_RWLOCK(fiq_safe_cpu_map_lock);

+/*

• The GIC mapping of CPU interfaces does not necessarily match
• the logical CPU numbering. Let's use a mapping as returned
• by the GIC itself.

@@ -624,7 +631,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

• raw_spin_lock_irqsave(&irq_controller_lock, flags);

• read_lock_irqsave(&fiq_safe_cpu_map_lock, flags);

Just for the record:

You might have noticed that you replace a raw lock with a non raw one. That's not an issue on mainline, but that pretty much renders that code broken for RT. Surely nothing I worry too much about given the current state of RT.

And having a second thought here. Looking at the protection scope independent of the spin vs. rw lock

gic_raise_softirq()

lock();

/* Does not need any protection */ for_each_cpu(cpu, mask) map |= gic_cpu_map[cpu];

/* * Can be outside the lock region as well as it makes sure * that previous writes (usually the IPI data) are visible * before the write to the SOFTINT register. */ dmb(ishst);

/* Why needs this protection? */ write(map, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT));

If gic_cpu_map changed value during the execution of this function then this could raise an IPI on the wrong CPU.

Most of the rest of this mail is explaining how this could happen.

unlock();

gic_migrate_target()

.... lock();

Also: Value of gic_cpu_map is updated.

/* Migrate all peripheral interrupts */

unlock();

Also: /* Migrate all IPIs pending on the old core */

So what's the point of that protection?

gic_raise_softirq() is used to send IPIs, which are PPIs on the target CPUs so they are not affected from the migration of the peripheral interrupts at all.

The write to the SOFTINT register in gic_migrate_target() is not inside the lock region. So what's serialized by the lock in gic_raise_softirq() at all?

At the point that gic_migrate_target() takes the lock it knows that no further IPIs can be submitted.

Once the gic_cpu_map is updated we can permit new IPIs to be submitted because these will be routed to the correct core.

As a result we don't actually need to hold the lock to migrate the pending IPIs since we know that no new IPIs can possibly be sent to the wrong core.

Either I'm missing something really important here or this locking exercise in gic_raise_softirq() and therefor the rwlock conversion is completely pointless.

I did want to remove the lock too. However when I reviewed this code I concluded the lock was still required. Without it I think it is possible for gic_raise_softirq() to raise an IPI on the old core *after* the code to migrate pending IPIs has been run.

Daniel.

On 24/11/14 20:38, Thomas Gleixner wrote:

On Mon, 24 Nov 2014, Thomas Gleixner wrote:

...

On Mon, 24 Nov 2014, Thomas Gleixner wrote:

...

On Fri, 14 Nov 2014, Daniel Thompson wrote:

...

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..0db62a6f1ee3 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,13 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock); /*

• • This lock may be locked for reading by FIQ handlers. Thus although
• • read locking may be used liberally, write locking must only take
• • place only when local FIQ handling is disabled.
• */

+static DEFINE_RWLOCK(fiq_safe_cpu_map_lock);

+/*

• The GIC mapping of CPU interfaces does not necessarily match
• the logical CPU numbering. Let's use a mapping as returned
• by the GIC itself.

@@ -624,7 +631,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

• raw_spin_lock_irqsave(&irq_controller_lock, flags);

• read_lock_irqsave(&fiq_safe_cpu_map_lock, flags);

Just for the record:

You might have noticed that you replace a raw lock with a non raw one. That's not an issue on mainline, but that pretty much renders that code broken for RT. Surely nothing I worry too much about given the current state of RT.

And having a second thought here. Looking at the protection scope independent of the spin vs. rw lock

gic_raise_softirq()

lock();

/* Does not need any protection */ for_each_cpu(cpu, mask) map |= gic_cpu_map[cpu];

/* * Can be outside the lock region as well as it makes sure * that previous writes (usually the IPI data) are visible * before the write to the SOFTINT register. */ dmb(ishst);

/* Why needs this protection? */ write(map, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT));

unlock();

gic_migrate_target()

.... lock();

/* Migrate all peripheral interrupts */

unlock();

So what's the point of that protection?

gic_raise_softirq() is used to send IPIs, which are PPIs on the target CPUs so they are not affected from the migration of the peripheral interrupts at all.

The write to the SOFTINT register in gic_migrate_target() is not inside the lock region. So what's serialized by the lock in gic_raise_softirq() at all?

Either I'm missing something really important here or this locking exercise in gic_raise_softirq() and therefor the rwlock conversion is completely pointless.

Thanks to Marc I figured it out now what I'm missing. That stuff is part of the bl switcher horror. Well documented as all of that ...

So the lock protects against an IPI being sent to the current cpu while the target map is redirected and the pending state of the current cpu is migrated to another cpu.

It's not your fault, that the initial authors of that just abused irq_controller_lock for that purpose instead of introducing a seperate lock with a clear description of the protection scope in the first place.

Now you came up with the rw lock to handle the following FIQ related case: gic_raise_softirq() lock(x); ---> FIQ handle_fiq() gic_raise_softirq() lock(x); <-- Live lock

Now the rwlock lets you avoid that, and it only lets you avoid that because rwlocks are not fair.

So while I cannot come up with a brilliant replacement, it would be really helpful documentation wise if you could do the following:

1. Create a patch which introduces irq_migration_lock as a raw spinlock and replaces the usage of irq_controller_lock in gic_raise_softirq() and gic_migrate_target() along with a proper explanation in the code and the changelog of course.

Replace irq_controller_lock or augment it with a new one?

irq_raise_softirq() can share a single r/w lock with irq_set_affinity() because irq_set_affinity() would have to lock it for writing and that would bring the deadlock back for a badly timed FIQ.

Thus if we want calls to gic_raise_softirq() to be FIQ-safe there there must be two locks taken in gic_migrate_target().

We can eliminate irq_controller_lock but we cannot replace it with one r/w lock.

2. Make the rwlock conversion on top of that with a proper documentation in the code of the only relevant reason (See above).

   The protection scope which prevents IPIs being sent while switching over is still the same and not affected.

That's not the first time that I stumble over this bl switcher mess which got boltet into the kernel mindlessly.

If the scope of the issue would have been clear up front, I wouldn't have complained about the RT relevance for this as it is simple to either disable FIQs for RT or just handle the above case differently.

Thanks,

tglx

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

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com Tested-by: Jon Medhurst tixy@linaro.org --- arch/arm/kernel/traps.c | 5 +- drivers/irqchip/irq-gic.c | 155 +++++++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic.h | 8 +++ 3 files changed, 158 insertions(+), 10 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 0c8b10801d36..4dc45b38e56e 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 0db62a6f1ee3..fe6a35f891ac 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -39,6 +39,7 @@ #include <linux/slab.h> #include <linux/irqchip/chained_irq.h> #include <linux/irqchip/arm-gic.h> +#include <linux/ratelimit.h>

#include <asm/cputype.h> #include <asm/irq.h> @@ -48,6 +49,10 @@ #include "irq-gic-common.h" #include "irqchip.h"

+#ifndef SMP_IPI_FIQ_MASK +#define SMP_IPI_FIQ_MASK 0 +#endif + union gic_base { void __iomem *common_base; void __percpu * __iomem *percpu_base; @@ -331,6 +336,93 @@ 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 not have + * the EnableGrp1 bit set. + */ + if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))) + return; + + raw_spin_lock(&irq_controller_lock); + + grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg); + pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg); + + if (group) { + grp_val |= grp_mask; + pri_val |= pri_mask; + } else { + grp_val &= ~grp_mask; + pri_val &= ~pri_mask; + } + + writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg); + 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) @@ -362,15 +454,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic) static void gic_cpu_if_up(void) { void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); - u32 bypass = 0; + void __iomem *dist_base = gic_data_dist_base(&gic_data[0]); + u32 ctrl = 0;

/* - * Preserve bypass disable bits to be written back later - */ - bypass = readl(cpu_base + GIC_CPU_CTRL); - bypass &= GICC_DIS_BYPASS_MASK; + * Preserve bypass disable bits to be written back later + */ + ctrl = readl(cpu_base + GIC_CPU_CTRL); + ctrl &= GICC_DIS_BYPASS_MASK;

- writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); + /* + * If EnableGrp1 is set in the distributor then enable group 1 + * support for this CPU (and route group 0 interrupts to FIQ). + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) + ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL | + GICC_ENABLE_GRP1; + + writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); }

@@ -394,7 +495,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)

gic_dist_config(base, gic_irqs, NULL);

- writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL); + /* + * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only, + * bit 1 ignored) depending on current mode. + */ + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL); + + /* + * Set all global interrupts to be group 1 if (and only if) it + * is possible to enable group 1 interrupts. This register is RAZ/WI + * if not accessible or not implemented, however some GICv1 devices + * do not implement the EnableGrp1 bit making it unsafe to set + * this register unconditionally. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)) + for (i = 32; i < gic_irqs; i += 32) + writel_relaxed(0xffffffff, + base + GIC_DIST_IGROUP + i * 4 / 32); }

static void gic_cpu_init(struct gic_chip_data *gic) @@ -403,6 +520,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. @@ -421,6 +539,19 @@ static void gic_cpu_init(struct gic_chip_data *gic)

gic_cpu_config(dist_base, NULL);

+ /* + * If the distributor is configured to support interrupt grouping + * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK + * to be group1 and ensure any remaining group 0 interrupts have + * the right priority. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) { + secure_irqs = SMP_IPI_FIQ_MASK; + writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0); + for_each_set_bit(secure_irq, &secure_irqs, 16) + gic_set_group_irq(dist_base, secure_irq, 0); + } + writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK); gic_cpu_if_up(); } @@ -510,7 +641,8 @@ static void gic_dist_restore(unsigned int gic_nr) writel_relaxed(gic_data[gic_nr].saved_spi_enable[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);

- writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL); + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, + dist_base + GIC_DIST_CTRL); }

static void gic_cpu_save(unsigned int gic_nr) @@ -630,6 +762,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) { int cpu; unsigned long flags, map = 0; + unsigned long softint;

read_lock_irqsave(&fiq_safe_cpu_map_lock, flags);

@@ -644,7 +777,11 @@ 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);

read_unlock_irqrestore(&fiq_safe_cpu_map_lock, flags); } diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h index 13eed92c7d24..a906fb7ac11f 100644 --- a/include/linux/irqchip/arm-gic.h +++ b/include/linux/irqchip/arm-gic.h @@ -22,6 +22,10 @@ #define GIC_CPU_IDENT 0xfc

#define GICC_ENABLE 0x1 +#define GICC_ENABLE_GRP1 0x2 +#define GICC_ACK_CTL 0x4 +#define GICC_FIQ_EN 0x8 +#define GICC_COMMON_BPR 0x10 #define GICC_INT_PRI_THRESHOLD 0xf0 #define GICC_IAR_INT_ID_MASK 0x3ff #define GICC_INT_SPURIOUS 1023 @@ -44,6 +48,7 @@ #define GIC_DIST_SGI_PENDING_SET 0xf20

#define GICD_ENABLE 0x1 +#define GICD_ENABLE_GRP1 0x2 #define GICD_DISABLE 0x0 #define GICD_INT_ACTLOW_LVLTRIG 0x0 #define GICD_INT_EN_CLR_X32 0xffffffff @@ -117,5 +122,8 @@ static inline void __init register_routable_domain_ops { gic_routable_irq_domain_ops = ops; } + +void gic_handle_fiq_ipi(void); + #endif /* __ASSEMBLY */ #endif

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

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 18f5a554134f..b076584ac0fa 100644 --- a/arch/arm/include/asm/smp.h +++ b/arch/arm/include/asm/smp.h @@ -18,6 +18,8 @@ # error "<asm/smp.h> included in non-SMP build" #endif

+#define SMP_IPI_FIQ_MASK 0x0100 + #define raw_smp_processor_id() (current_thread_info()->cpu)

struct seq_file; @@ -79,6 +81,7 @@ extern void arch_send_call_function_single_ipi(int cpu); extern void arch_send_call_function_ipi_mask(const struct cpumask *mask); extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);

+extern void ipi_cpu_backtrace(struct pt_regs *regs); extern int register_ipi_completion(struct completion *completion, int cpu);

struct smp_operations { diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 14c594a12bef..e923843562d9 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -539,7 +539,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();

@@ -580,6 +580,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 4dc45b38e56e..9eb05be9526e 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();

Hi Daniel,

On 25/11/14 17:26, Daniel Thompson wrote:

irq_controller_lock is used for multiple purposes within the gic driver. Primarily it is used to make register read-modify-write sequences atomic. It is also used by gic_raise_softirq() in order that the big.LITTLE migration logic can figure out when it is safe to migrate interrupts between physical cores.

The second usage of irq_controller_lock is difficult to discern when reviewing the code because the migration itself takes place outside the lock.

This patch makes the second usage more explicit by splitting it out into a separate lock and providing better comments.

While we're at it, how about an additional patch that would make this lock disappear entirely when the big-little stuff is not compiled in, which is likely to be the case on a lot of (dare I say most?) systems? That will save expensive barriers that we definitely could do without.

It otherwise looks good to me.

Thanks,

M.

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com

---

drivers/irqchip/irq-gic.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..bb4bc20573ea 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,12 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock); /*

• • This lock is used by the big.LITTLE migration code to ensure no
• • IPIs can be pended on the old core after the map has been updated.
• */

+static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock);

+/*

• The GIC mapping of CPU interfaces does not necessarily match
• the logical CPU numbering. Let's use a mapping as returned
• by the GIC itself.

@@ -624,7 +630,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

• raw_spin_lock_irqsave(&irq_controller_lock, flags);

• raw_spin_lock_irqsave(&cpu_map_migration_lock, flags);

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -639,7 +645,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

• raw_spin_unlock_irqrestore(&irq_controller_lock, flags);

• raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags);

} #endif @@ -710,8 +716,17 @@ void gic_migrate_target(unsigned int new_cpu_id) raw_spin_lock(&irq_controller_lock);

• /* Update the target interface for this logical CPU */

• /*

• * Update the target interface for this logical CPU
  

• *
  

• * From the point we release the cpu_map_migration_lock any new
  

• * SGIs will be pended on the new cpu which makes the set of SGIs
  

• * pending on the old cpu static. That means we can defer the
  

• * migration until after we have released the irq_controller_lock.
  

• */
  

• raw_spin_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id;
• raw_spin_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

On 25/11/14 17:40, Marc Zyngier wrote:

Hi Daniel,

On 25/11/14 17:26, Daniel Thompson wrote:

...

irq_controller_lock is used for multiple purposes within the gic driver. Primarily it is used to make register read-modify-write sequences atomic. It is also used by gic_raise_softirq() in order that the big.LITTLE migration logic can figure out when it is safe to migrate interrupts between physical cores.

The second usage of irq_controller_lock is difficult to discern when reviewing the code because the migration itself takes place outside the lock.

This patch makes the second usage more explicit by splitting it out into a separate lock and providing better comments.

While we're at it, how about an additional patch that would make this lock disappear entirely when the big-little stuff is not compiled in, which is likely to be the case on a lot of (dare I say most?) systems? That will save expensive barriers that we definitely could do without.

Will do.

It otherwise looks good to me.

Thanks,

M.

...

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com

---

drivers/irqchip/irq-gic.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..bb4bc20573ea 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,12 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock); /*

• • This lock is used by the big.LITTLE migration code to ensure no
• • IPIs can be pended on the old core after the map has been updated.
• */

+static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock);

+/*

• The GIC mapping of CPU interfaces does not necessarily match
• the logical CPU numbering. Let's use a mapping as returned
• by the GIC itself.

@@ -624,7 +630,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

• raw_spin_lock_irqsave(&irq_controller_lock, flags);

• raw_spin_lock_irqsave(&cpu_map_migration_lock, flags);

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -639,7 +645,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

• raw_spin_unlock_irqrestore(&irq_controller_lock, flags);

• raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags);

} #endif @@ -710,8 +716,17 @@ void gic_migrate_target(unsigned int new_cpu_id) raw_spin_lock(&irq_controller_lock);

• /* Update the target interface for this logical CPU */

• /*

• * Update the target interface for this logical CPU
  

• *
  

• * From the point we release the cpu_map_migration_lock any new
  

• * SGIs will be pended on the new cpu which makes the set of SGIs
  

• * pending on the old cpu static. That means we can defer the
  

• * migration until after we have released the irq_controller_lock.
  

• */
  

• raw_spin_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id;
• raw_spin_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

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

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com Tested-by: Jon Medhurst tixy@linaro.org --- arch/arm/kernel/traps.c | 5 +- drivers/irqchip/irq-gic.c | 155 +++++++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic.h | 8 +++ 3 files changed, 158 insertions(+), 10 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 0c8b10801d36..4dc45b38e56e 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 a53aa11e4f17..dfec7a4c1c64 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -39,6 +39,7 @@ #include <linux/slab.h> #include <linux/irqchip/chained_irq.h> #include <linux/irqchip/arm-gic.h> +#include <linux/ratelimit.h>

#include <asm/cputype.h> #include <asm/irq.h> @@ -48,6 +49,10 @@ #include "irq-gic-common.h" #include "irqchip.h"

+#ifndef SMP_IPI_FIQ_MASK +#define SMP_IPI_FIQ_MASK 0 +#endif + union gic_base { void __iomem *common_base; void __percpu * __iomem *percpu_base; @@ -333,6 +338,93 @@ 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 not have + * the EnableGrp1 bit set. + */ + if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))) + return; + + raw_spin_lock(&irq_controller_lock); + + grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg); + pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg); + + if (group) { + grp_val |= grp_mask; + pri_val |= pri_mask; + } else { + grp_val &= ~grp_mask; + pri_val &= ~pri_mask; + } + + writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg); + 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) @@ -364,15 +456,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic) static void gic_cpu_if_up(void) { void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); - u32 bypass = 0; + void __iomem *dist_base = gic_data_dist_base(&gic_data[0]); + u32 ctrl = 0;

/* - * Preserve bypass disable bits to be written back later - */ - bypass = readl(cpu_base + GIC_CPU_CTRL); - bypass &= GICC_DIS_BYPASS_MASK; + * Preserve bypass disable bits to be written back later + */ + ctrl = readl(cpu_base + GIC_CPU_CTRL); + ctrl &= GICC_DIS_BYPASS_MASK;

- writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); + /* + * If EnableGrp1 is set in the distributor then enable group 1 + * support for this CPU (and route group 0 interrupts to FIQ). + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) + ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL | + GICC_ENABLE_GRP1; + + writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); }

@@ -396,7 +497,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)

gic_dist_config(base, gic_irqs, NULL);

- writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL); + /* + * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only, + * bit 1 ignored) depending on current mode. + */ + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL); + + /* + * Set all global interrupts to be group 1 if (and only if) it + * is possible to enable group 1 interrupts. This register is RAZ/WI + * if not accessible or not implemented, however some GICv1 devices + * do not implement the EnableGrp1 bit making it unsafe to set + * this register unconditionally. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)) + for (i = 32; i < gic_irqs; i += 32) + writel_relaxed(0xffffffff, + base + GIC_DIST_IGROUP + i * 4 / 32); }

static void gic_cpu_init(struct gic_chip_data *gic) @@ -405,6 +522,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. @@ -423,6 +541,19 @@ static void gic_cpu_init(struct gic_chip_data *gic)

gic_cpu_config(dist_base, NULL);

+ /* + * If the distributor is configured to support interrupt grouping + * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK + * to be group1 and ensure any remaining group 0 interrupts have + * the right priority. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) { + secure_irqs = SMP_IPI_FIQ_MASK; + writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0); + for_each_set_bit(secure_irq, &secure_irqs, 16) + gic_set_group_irq(dist_base, secure_irq, 0); + } + writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK); gic_cpu_if_up(); } @@ -512,7 +643,8 @@ static void gic_dist_restore(unsigned int gic_nr) writel_relaxed(gic_data[gic_nr].saved_spi_enable[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);

- writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL); + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, + dist_base + GIC_DIST_CTRL); }

static void gic_cpu_save(unsigned int gic_nr) @@ -640,6 +772,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) { int cpu; unsigned long map = 0; + unsigned long softint;

read_lock(&cpu_map_migration_lock);

@@ -654,7 +787,11 @@ 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);

read_unlock(&cpu_map_migration_lock); } diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h index 13eed92c7d24..a906fb7ac11f 100644 --- a/include/linux/irqchip/arm-gic.h +++ b/include/linux/irqchip/arm-gic.h @@ -22,6 +22,10 @@ #define GIC_CPU_IDENT 0xfc

#define GICC_ENABLE 0x1 +#define GICC_ENABLE_GRP1 0x2 +#define GICC_ACK_CTL 0x4 +#define GICC_FIQ_EN 0x8 +#define GICC_COMMON_BPR 0x10 #define GICC_INT_PRI_THRESHOLD 0xf0 #define GICC_IAR_INT_ID_MASK 0x3ff #define GICC_INT_SPURIOUS 1023 @@ -44,6 +48,7 @@ #define GIC_DIST_SGI_PENDING_SET 0xf20

#define GICD_ENABLE 0x1 +#define GICD_ENABLE_GRP1 0x2 #define GICD_DISABLE 0x0 #define GICD_INT_ACTLOW_LVLTRIG 0x0 #define GICD_INT_EN_CLR_X32 0xffffffff @@ -117,5 +122,8 @@ static inline void __init register_routable_domain_ops { gic_routable_irq_domain_ops = ops; } + +void gic_handle_fiq_ipi(void); + #endif /* __ASSEMBLY */ #endif

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 53c15dec7af6..be1d07d59ee9 100644 --- a/arch/arm/include/asm/irq.h +++ b/arch/arm/include/asm/irq.h @@ -35,6 +35,11 @@ extern void (*handle_arch_irq)(struct pt_regs *); extern void set_handle_irq(void (*handle_irq)(struct pt_regs *)); #endif

+#ifdef CONFIG_SMP +extern void arch_trigger_all_cpu_backtrace(bool); +#define arch_trigger_all_cpu_backtrace(x) arch_trigger_all_cpu_backtrace(x) +#endif + #endif

#endif diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 13396d3d600e..14c594a12bef 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; @@ -535,6 +539,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) @@ -614,6 +633,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); @@ -708,3 +733,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(); +}

Hi Daniel

Am Dienstag, 25. November 2014, 17:26:41 schrieb Daniel Thompson:

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

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

Does this ipi handler interfere in any way with set_fiq_handler?

As far as i know there is only one FIQ handler vector so i guess there is a potential conflict. But i have not worked with IPI's so i might be completley wrong.

Regards Tim

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 18f5a554134f..b076584ac0fa 100644 --- a/arch/arm/include/asm/smp.h +++ b/arch/arm/include/asm/smp.h @@ -18,6 +18,8 @@ # error "<asm/smp.h> included in non-SMP build" #endif

+#define SMP_IPI_FIQ_MASK 0x0100

#define raw_smp_processor_id() (current_thread_info()->cpu)

struct seq_file; @@ -79,6 +81,7 @@ extern void arch_send_call_function_single_ipi(int cpu); extern void arch_send_call_function_ipi_mask(const struct cpumask *mask); extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);

+extern void ipi_cpu_backtrace(struct pt_regs *regs); extern int register_ipi_completion(struct completion *completion, int cpu);

struct smp_operations { diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 14c594a12bef..e923843562d9 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -539,7 +539,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();

@@ -580,6 +580,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 4dc45b38e56e..9eb05be9526e 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();

On Mon, Dec 01, 2014 at 11:32:00AM +0100, Tim Sander wrote:

Hi Russel, Daniel

Am Freitag, 28. November 2014, 10:08:28 schrieb Russell King - ARM Linux:

...

The two things are mutually exclusive. You can either have FIQ being used for debug purposes, where we decode the FIQ reason and call some function (which means that we will only service one FIQ at a time) or you can use it in exclusive mode (provided by fiq.c) where your handler has sole usage of the vector, and benefits from fast and immediate servicing of the event.

As far as i am aware, die CONFIG_FIQ symbol is not pulled by all ARM platforms. Since there are ARM platforms which don't use this symbol but the hardware is fully capable of handling FIQ requests i would expect, that adding CONFIG_FIQ to a plattform, that this platform honors the set_fiq_handler functionality.

That whole paragraph doesn't make much sense to me.

Look, in my mind it is very simple. If you are using CONFIG_FIQ on a SMP platform, your life will be very difficult. The FIQ code enabled by that symbol is not designed to be used on SMP systems, *period*.

If you decide to enable CONFIG_FIQ, and you use that code on a SMP platform, I'm going to say right now so it's totally clear: if you encounter a problem, I don't want to know about it. The code is not designed for use on that situation.

Therefore, as far as I'm concerned, the two facilities are mututally exclusive.

I had thought about whether the IPI FIQ should be disabled when a replacement FIQ handler is installed, I deem it not to be a use case that the mainline kernel needs to be concerned about.

Yes, but if the FIQ handler is also used for IPI, set_fiq_handler gets IPI interrupts (with the patch starting this thread)? So i think that the patch needs to look like: --- 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) +#ifndef CONFIG_FIQ #ifdef CONFIG_ARM_GIC gic_handle_fiq_ipi(); #endif +#endif

No. With a single zImage kernel, you could very well have SMP and FIQ both enabled, but have a non-SMP platform using FIQ, but also support SMP platforms as well. Your change prevents that happening.

On 01/12/14 13:54, Tim Sander wrote:

...

Look, in my mind it is very simple. If you are using CONFIG_FIQ on a SMP platform, your life will be very difficult. The FIQ code enabled by that symbol is not designed to be used on SMP systems, *period*.

Well the only extra thing you had to do is set up the FIQ registers on every cpu, but i would not call that very difficult. Other than that i am not aware of any problems that are not also present on a uniprocessor system. So i have a hard time following your reasoning why SMP is different from UP in regard to the CONFIG_FIQ.

...

If you decide to enable CONFIG_FIQ, and you use that code on a SMP platform, I'm going to say right now so it's totally clear: if you encounter a problem, I don't want to know about it. The code is not designed for use on that situation.

Even with using the FIQ on a Linux SMP system you have not heard from me before, as i knew that this is not your problem (and that is not to say that there where none!). The only interface Linux has been making available is set_fiq_handler. So it was clear that the FIQ is its own domain otherwise untouched by the kernel. Now the line gets blurried with the linux kernel moving to use the FIQ. And with the descicions forthcoming its not only grabbing land it also claims a previous public path for its own. So it doesn't help that its planting some flowers along the way. So please be nice to the natural inhabitants...

Surely only upstream code could claim to be a natural inhabitant.

Whenever I've been working on code that, for whatever reason, cannot be upstreamed I'd probably best be regarded as a tourist.

And i really don't get it, that neither ARM nor the kernel community sees fast interrupts as a worthwhile usecase. Unfortunatly the interrupt latencies with Linux are at least a order of magnitude higher than the pure hardware even with longer pipelines can deliver.

...

Therefore, as far as I'm concerned, the two facilities are mututally exclusive.

Well can't have the cake and eat it too.

...

I had thought about whether the IPI FIQ should be disabled when a replacement FIQ handler is installed, I deem it not to be a use case that the mainline kernel needs to be concerned about.

That would be nice.

Just to be clear, this is exactly the dynamic switching that I mentioned a couple of mails ago.

As I said such code should not especially hard to write but, with the current mainline kernel, the code would be unreachable and, as a result, likely also to be more or less untested.

...

...

Yes, but if the FIQ handler is also used for IPI, set_fiq_handler gets IPI interrupts (with the patch starting this thread)? So i think that the patch needs to look like: --- 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) +#ifndef CONFIG_FIQ

#ifdef CONFIG_ARM_GIC gic_handle_fiq_ipi(); #endif

+#endif

No. With a single zImage kernel, you could very well have SMP and FIQ both enabled, but have a non-SMP platform using FIQ, but also support SMP platforms as well. Your change prevents that happening.

Ah, well i have to get used to this "new" devicetree thingy, where one size fits all...

Still if you boot a single process system which has FIQ available and has a GIC with such a kernel, then you also reprogramm the IPI's as FIQs. But i guess thats not a problem as Linux does not self IPI the kernel as other os'es do?

Best regards Tim

irq_controller_lock is used for multiple purposes within the gic driver. Primarily it is used to make register read-modify-write sequences atomic. It is also used by gic_raise_softirq() in order that the big.LITTLE migration logic can figure out when it is safe to migrate interrupts between physical cores.

The second usage of irq_controller_lock is difficult to discern when reviewing the code because the migration itself takes place outside the lock.

This patch makes the second usage more explicit by splitting it out into a separate lock and providing better comments.

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com --- drivers/irqchip/irq-gic.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..94d77118efa8 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,12 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock);

/* + * This lock is used by the big.LITTLE migration code to ensure no IPIs + * can be pended on the old core after the map has been updated. + */ +static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock); + +/* * The GIC mapping of CPU interfaces does not necessarily match * the logical CPU numbering. Let's use a mapping as returned * by the GIC itself. @@ -624,7 +630,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

- raw_spin_lock_irqsave(&irq_controller_lock, flags); + raw_spin_lock_irqsave(&cpu_map_migration_lock, flags);

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -639,7 +645,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- raw_spin_unlock_irqrestore(&irq_controller_lock, flags); + raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags); } #endif

@@ -710,8 +716,17 @@ void gic_migrate_target(unsigned int new_cpu_id)

raw_spin_lock(&irq_controller_lock);

- /* Update the target interface for this logical CPU */ + /* + * Update the target interface for this logical CPU + * + * From the point we release the cpu_map_migration_lock any new + * SGIs will be pended on the new cpu which makes the set of SGIs + * pending on the old cpu static. That means we can defer the + * migration until after we have released the irq_controller_lock. + */ + raw_spin_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id; + raw_spin_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

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

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

Calling printk() from a FIQ handler can trigger this problem because printk() raises an IPI when it needs to wake_up_klogd(). More generally, IPIs are the only means for FIQ handlers to safely defer work to less restrictive calling context so the function to raise them really needs to be FIQ-safe.

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

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

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

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index e875da93f24a..5d72823bc5e9 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -75,22 +75,25 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock); /* * This lock is used by the big.LITTLE migration code to ensure no IPIs * can be pended on the old core after the map has been updated. + * + * This lock may be locked for reading from both IRQ and FIQ handlers + * and therefore must not be locked for writing when these are enabled. */ #ifdef CONFIG_BL_SWITCHER -static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock); +static DEFINE_RWLOCK(cpu_map_migration_lock);

-static inline void bl_migration_lock(unsigned long *flags) +static inline void bl_migration_lock(void) { - raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags); + read_lock(&cpu_map_migration_lock); }

-static inline void bl_migration_unlock(unsigned long flags) +static inline void bl_migration_unlock(void) { - raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags); + read_unlock(&cpu_map_migration_lock); } #else -static inline void bl_migration_lock(unsigned long *flags) {} -static inline void bl_migration_unlock(unsigned long flags) {} +static inline void bl_migration_lock(void) {} +static inline void bl_migration_unlock(void) {} #endif

/* @@ -640,12 +643,20 @@ static void __init gic_pm_init(struct gic_chip_data *gic) #endif

#ifdef CONFIG_SMP +/* + * Raise the specified IPI on all cpus set in mask. + * + * This function is safe to call from all calling contexts, including + * FIQ handlers. It relies on bl_migration_lock() being multiply acquirable + * to avoid deadlocks when the function is re-entered at different + * exception levels. + */ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) { int cpu; - unsigned long flags, map = 0; + unsigned long map = 0;

- bl_migration_lock(&flags); + bl_migration_lock();

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -660,7 +671,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- bl_migration_unlock(flags); + bl_migration_unlock(); } #endif

@@ -708,7 +719,8 @@ int gic_get_cpu_id(unsigned int cpu) * Migrate all peripheral interrupts with a target matching the current CPU * to the interface corresponding to @new_cpu_id. The CPU interface mapping * is also updated. Targets to other CPU interfaces are unchanged. - * This must be called with IRQs locally disabled. + * This must be called from a task context and with IRQ and FIQ locally + * disabled. */ void gic_migrate_target(unsigned int new_cpu_id) { @@ -739,9 +751,9 @@ void gic_migrate_target(unsigned int new_cpu_id) * pending on the old cpu static. That means we can defer the * migration until after we have released the irq_controller_lock. */ - raw_spin_lock(&cpu_map_migration_lock); + write_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id; - raw_spin_unlock(&cpu_map_migration_lock); + write_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

Daniel,

I've been a bit swamped this cycle and haven't kept as close an eye on this as I should have. :( fwiw, it's looking really good. I have one question below:

On Wed, Nov 26, 2014 at 04:23:28PM +0000, Daniel Thompson wrote:

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

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com Tested-by: Jon Medhurst tixy@linaro.org

---

arch/arm/kernel/traps.c | 5 +- drivers/irqchip/irq-gic.c | 155 +++++++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic.h | 8 +++ 3 files changed, 158 insertions(+), 10 deletions(-)

...

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 5d72823bc5e9..978e5e48d5c1 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c

...

+/*

• • 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;

+}

...

@@ -669,7 +802,11 @@ 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);
  

bl_migration_unlock(); }

Is it worth the code complication to optimize this if the controller doesn't support grouping? Maybe set group_enabled at init so the above would become:

softint = map << 16 | irq; if (group_enabled && gic_get_group_irq(gic_data_dist_base(&gic_data[0]), irq)) softint |= 0x8000; writel_relaxed(...);

thx,

Jason.

Daniel,

On Thu, Nov 27, 2014 at 01:39:01PM +0000, Daniel Thompson wrote:

On 26/11/14 17:42, Jason Cooper wrote:

...

On Wed, Nov 26, 2014 at 04:23:28PM +0000, Daniel Thompson wrote:

...

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

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com Tested-by: Jon Medhurst tixy@linaro.org

---

arch/arm/kernel/traps.c | 5 +- drivers/irqchip/irq-gic.c | 155 +++++++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic.h | 8 +++ 3 files changed, 158 insertions(+), 10 deletions(-)

...

...

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 5d72823bc5e9..978e5e48d5c1 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c

...

...

+/*

• • 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;

+}

...

...

@@ -669,7 +802,11 @@ 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);
  

bl_migration_unlock(); }

Is it worth the code complication to optimize this if the controller doesn't support grouping? Maybe set group_enabled at init so the above would become:

softint = map << 16 | irq; if (group_enabled && gic_get_group_irq(gic_data_dist_base(&gic_data[0]), irq)) softint |= 0x8000; writel_relaxed(...);

No objections.

However given this code always calls gic_get_group_irq() with irq < 16 we might be able to do better even than this. The lower 16-bits of IGROUP[0] are constant after boot so if we keep a shadow copy around instead of just a boolean then we can avoid the register read on all code paths.

Hmm, I'd look at that as a performance enhancement. I'm more concerned about performance regressions for current users of the gic (non-group enabled).

Let's go ahead and do the change (well, a working facsimile) I suggested above, and we can do a follow on patch to increase performance for the group enabled use case.

If there's no objections, I'd like to try to get this in for v3.19, but it's really late. So we'll see how it goes.

thx,

Jason.

On 27/11/14 19:42, Daniel Thompson wrote:

...

Hmm, I'd look at that as a performance enhancement. I'm more concerned about performance regressions for current users of the gic (non-group enabled).

"Current users of the gic" doesn't imply "non-group enabled". Whether or not grouping is enabled is a property of the hardware or (secure) bootloader.

If we are seriously worried about a performance regression here we actually have to care about both cases.

...

Let's go ahead and do the change (well, a working facsimile) I suggested above, and we can do a follow on patch to increase performance for the group enabled use case.

Hmnnn...

I've have a new patch ready to go that shadows the IGROUP[0]. Its looks OK to me and I think it is actually fewer lines of code than v10 because we can remove gic_get_group_irq() completely.

Fianlly from me. If you are worried about large "last minute changes" involved in v11 here is the v10 -> v11 diff.

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 978e5e48d5c1..5c36aefa67ea 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -70,6 +70,7 @@ struct gic_chip_data { #endif struct irq_domain *domain; unsigned int gic_irqs; + u32 igroup0_shadow; #ifdef CONFIG_GIC_NON_BANKED void __iomem *(*get_base)(union gic_base *); #endif @@ -363,9 +364,10 @@ static struct irq_chip gic_chip = { * 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) +static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq, + int group) { + void __iomem *base = gic_data_dist_base(gic); unsigned int grp_reg = hwirq / 32 * 4; u32 grp_mask = BIT(hwirq % 32); u32 grp_val; @@ -395,25 +397,14 @@ static void gic_set_group_irq(void __iomem *base, unsigned int hwirq, }

writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg); + if (grp_reg == 0) + gic->igroup0_shadow = grp_val; + writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg);

raw_spin_unlock(&irq_controller_lock); }

-/* - * 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, @@ -565,8 +556,9 @@ static void gic_cpu_init(struct gic_chip_data *gic) if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) { secure_irqs = SMP_IPI_FIQ_MASK; writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0); + gic->igroup0_shadow = ~secure_irqs; for_each_set_bit(secure_irq, &secure_irqs, 16) - gic_set_group_irq(dist_base, secure_irq, 0); + gic_set_group_irq(gic, secure_irq, 0); }

writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK); @@ -801,10 +793,12 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) */ dmb(ishst);

- /* this always happens on GIC0 */ + /* We avoid a readl here by using the shadow copy of IGROUP[0] */ softint = map << 16 | irq; - if (gic_get_group_irq(gic_data_dist_base(&gic_data[0]), irq)) + if (gic_data[0].igroup0_shadow & BIT(irq)) softint |= 0x8000; + + /* This always happens on GIC0 */ writel_relaxed(softint, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

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

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 18f5a554134f..b076584ac0fa 100644 --- a/arch/arm/include/asm/smp.h +++ b/arch/arm/include/asm/smp.h @@ -18,6 +18,8 @@ # error "<asm/smp.h> included in non-SMP build" #endif

+#define SMP_IPI_FIQ_MASK 0x0100 + #define raw_smp_processor_id() (current_thread_info()->cpu)

struct seq_file; @@ -79,6 +81,7 @@ extern void arch_send_call_function_single_ipi(int cpu); extern void arch_send_call_function_ipi_mask(const struct cpumask *mask); extern void arch_send_wakeup_ipi_mask(const struct cpumask *mask);

+extern void ipi_cpu_backtrace(struct pt_regs *regs); extern int register_ipi_completion(struct completion *completion, int cpu);

struct smp_operations { diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 14c594a12bef..e923843562d9 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -539,7 +539,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();

@@ -580,6 +580,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 4dc45b38e56e..9eb05be9526e 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();

irq_controller_lock is used for multiple purposes within the gic driver. Primarily it is used to make register read-modify-write sequences atomic. It is also used by gic_raise_softirq() in order that the big.LITTLE migration logic can figure out when it is safe to migrate interrupts between physical cores.

The second usage of irq_controller_lock is difficult to discern when reviewing the code because the migration itself takes place outside the lock.

This patch makes the second usage more explicit by splitting it out into a separate lock and providing better comments.

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com --- drivers/irqchip/irq-gic.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 38493ff28fa5..94d77118efa8 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,12 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock);

/* + * This lock is used by the big.LITTLE migration code to ensure no IPIs + * can be pended on the old core after the map has been updated. + */ +static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock); + +/* * The GIC mapping of CPU interfaces does not necessarily match * the logical CPU numbering. Let's use a mapping as returned * by the GIC itself. @@ -624,7 +630,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

- raw_spin_lock_irqsave(&irq_controller_lock, flags); + raw_spin_lock_irqsave(&cpu_map_migration_lock, flags);

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -639,7 +645,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- raw_spin_unlock_irqrestore(&irq_controller_lock, flags); + raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags); } #endif

@@ -710,8 +716,17 @@ void gic_migrate_target(unsigned int new_cpu_id)

raw_spin_lock(&irq_controller_lock);

- /* Update the target interface for this logical CPU */ + /* + * Update the target interface for this logical CPU + * + * From the point we release the cpu_map_migration_lock any new + * SGIs will be pended on the new cpu which makes the set of SGIs + * pending on the old cpu static. That means we can defer the + * migration until after we have released the irq_controller_lock. + */ + raw_spin_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id; + raw_spin_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

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

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

Calling printk() from a FIQ handler can trigger this problem because printk() raises an IPI when it needs to wake_up_klogd(). More generally, IPIs are the only means for FIQ handlers to safely defer work to less restrictive calling context so the function to raise them really needs to be FIQ-safe.

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

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

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

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index e875da93f24a..5d72823bc5e9 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -75,22 +75,25 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock); /* * This lock is used by the big.LITTLE migration code to ensure no IPIs * can be pended on the old core after the map has been updated. + * + * This lock may be locked for reading from both IRQ and FIQ handlers + * and therefore must not be locked for writing when these are enabled. */ #ifdef CONFIG_BL_SWITCHER -static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock); +static DEFINE_RWLOCK(cpu_map_migration_lock);

-static inline void bl_migration_lock(unsigned long *flags) +static inline void bl_migration_lock(void) { - raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags); + read_lock(&cpu_map_migration_lock); }

-static inline void bl_migration_unlock(unsigned long flags) +static inline void bl_migration_unlock(void) { - raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags); + read_unlock(&cpu_map_migration_lock); } #else -static inline void bl_migration_lock(unsigned long *flags) {} -static inline void bl_migration_unlock(unsigned long flags) {} +static inline void bl_migration_lock(void) {} +static inline void bl_migration_unlock(void) {} #endif

/* @@ -640,12 +643,20 @@ static void __init gic_pm_init(struct gic_chip_data *gic) #endif

#ifdef CONFIG_SMP +/* + * Raise the specified IPI on all cpus set in mask. + * + * This function is safe to call from all calling contexts, including + * FIQ handlers. It relies on bl_migration_lock() being multiply acquirable + * to avoid deadlocks when the function is re-entered at different + * exception levels. + */ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) { int cpu; - unsigned long flags, map = 0; + unsigned long map = 0;

- bl_migration_lock(&flags); + bl_migration_lock();

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -660,7 +671,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- bl_migration_unlock(flags); + bl_migration_unlock(); } #endif

@@ -708,7 +719,8 @@ int gic_get_cpu_id(unsigned int cpu) * Migrate all peripheral interrupts with a target matching the current CPU * to the interface corresponding to @new_cpu_id. The CPU interface mapping * is also updated. Targets to other CPU interfaces are unchanged. - * This must be called with IRQs locally disabled. + * This must be called from a task context and with IRQ and FIQ locally + * disabled. */ void gic_migrate_target(unsigned int new_cpu_id) { @@ -739,9 +751,9 @@ void gic_migrate_target(unsigned int new_cpu_id) * pending on the old cpu static. That means we can defer the * migration until after we have released the irq_controller_lock. */ - raw_spin_lock(&cpu_map_migration_lock); + write_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id; - raw_spin_unlock(&cpu_map_migration_lock); + write_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

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 53c15dec7af6..be1d07d59ee9 100644 --- a/arch/arm/include/asm/irq.h +++ b/arch/arm/include/asm/irq.h @@ -35,6 +35,11 @@ extern void (*handle_arch_irq)(struct pt_regs *); extern void set_handle_irq(void (*handle_irq)(struct pt_regs *)); #endif

+#ifdef CONFIG_SMP +extern void arch_trigger_all_cpu_backtrace(bool); +#define arch_trigger_all_cpu_backtrace(x) arch_trigger_all_cpu_backtrace(x) +#endif + #endif

#endif diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 13396d3d600e..14c594a12bef 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; @@ -535,6 +539,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) @@ -614,6 +633,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); @@ -708,3 +733,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(); +}

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

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

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

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

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

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

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index e875da93f24a..5d72823bc5e9 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -75,22 +75,25 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock); /* * This lock is used by the big.LITTLE migration code to ensure no IPIs * can be pended on the old core after the map has been updated. + * + * This lock may be locked for reading from both IRQ and FIQ handlers + * and therefore must not be locked for writing when these are enabled. */ #ifdef CONFIG_BL_SWITCHER -static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock); +static DEFINE_RWLOCK(cpu_map_migration_lock);

-static inline void bl_migration_lock(unsigned long *flags) +static inline void bl_migration_lock(void) { - raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags); + read_lock(&cpu_map_migration_lock); }

-static inline void bl_migration_unlock(unsigned long flags) +static inline void bl_migration_unlock(void) { - raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags); + read_unlock(&cpu_map_migration_lock); } #else -static inline void bl_migration_lock(unsigned long *flags) {} -static inline void bl_migration_unlock(unsigned long flags) {} +static inline void bl_migration_lock(void) {} +static inline void bl_migration_unlock(void) {} #endif

/* @@ -640,12 +643,20 @@ static void __init gic_pm_init(struct gic_chip_data *gic) #endif

#ifdef CONFIG_SMP +/* + * Raise the specified IPI on all cpus set in mask. + * + * This function is safe to call from all calling contexts, including + * FIQ handlers. It relies on bl_migration_lock() being multiply acquirable + * to avoid deadlocks when the function is re-entered at different + * exception levels. + */ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) { int cpu; - unsigned long flags, map = 0; + unsigned long map = 0;

- bl_migration_lock(&flags); + bl_migration_lock();

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -660,7 +671,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- bl_migration_unlock(flags); + bl_migration_unlock(); } #endif

@@ -708,7 +719,8 @@ int gic_get_cpu_id(unsigned int cpu) * Migrate all peripheral interrupts with a target matching the current CPU * to the interface corresponding to @new_cpu_id. The CPU interface mapping * is also updated. Targets to other CPU interfaces are unchanged. - * This must be called with IRQs locally disabled. + * This must be called from a task context and with IRQ and FIQ locally + * disabled. */ void gic_migrate_target(unsigned int new_cpu_id) { @@ -739,9 +751,9 @@ void gic_migrate_target(unsigned int new_cpu_id) * pending on the old cpu static. That means we can defer the * migration until after we have released the irq_controller_lock. */ - raw_spin_lock(&cpu_map_migration_lock); + write_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id; - raw_spin_unlock(&cpu_map_migration_lock); + write_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

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 53c15dec7af6..be1d07d59ee9 100644 --- a/arch/arm/include/asm/irq.h +++ b/arch/arm/include/asm/irq.h @@ -35,6 +35,11 @@ extern void (*handle_arch_irq)(struct pt_regs *); extern void set_handle_irq(void (*handle_irq)(struct pt_regs *)); #endif

+#ifdef CONFIG_SMP +extern void arch_trigger_all_cpu_backtrace(bool); +#define arch_trigger_all_cpu_backtrace(x) arch_trigger_all_cpu_backtrace(x) +#endif + #endif

#endif diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 13396d3d600e..14c594a12bef 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; @@ -535,6 +539,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) @@ -614,6 +633,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); @@ -708,3 +733,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(); +}

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

This is sub-optimal in closely related ways:

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com --- drivers/irqchip/irq-gic.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index d617ee5a3d8a..a9ed64dcc84b 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,27 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock);

/* + * This lock is used by the big.LITTLE migration code to ensure no IPIs + * can be pended on the old core after the map has been updated. + */ +#ifdef CONFIG_BL_SWITCHER +static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock); + +static inline void bl_migration_lock(unsigned long *flags) +{ + raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags); +} + +static inline void bl_migration_unlock(unsigned long flags) +{ + raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags); +} +#else +static inline void bl_migration_lock(unsigned long *flags) {} +static inline void bl_migration_unlock(unsigned long flags) {} +#endif + +/* * The GIC mapping of CPU interfaces does not necessarily match * the logical CPU numbering. Let's use a mapping as returned * by the GIC itself. @@ -624,7 +645,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

- raw_spin_lock_irqsave(&irq_controller_lock, flags); + bl_migration_lock(&flags);

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -639,7 +660,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- raw_spin_unlock_irqrestore(&irq_controller_lock, flags); + bl_migration_unlock(flags); } #endif

@@ -710,8 +731,17 @@ void gic_migrate_target(unsigned int new_cpu_id)

raw_spin_lock(&irq_controller_lock);

- /* Update the target interface for this logical CPU */ + /* + * Update the target interface for this logical CPU + * + * From the point we release the cpu_map_migration_lock any new + * SGIs will be pended on the new cpu which makes the set of SGIs + * pending on the old cpu static. That means we can defer the + * migration until after we have released the irq_controller_lock. + */ + raw_spin_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id; + raw_spin_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

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

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com Tested-by: Jon Medhurst tixy@linaro.org --- arch/arm/kernel/traps.c | 5 +- drivers/irqchip/irq-gic.c | 151 +++++++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic.h | 8 +++ 3 files changed, 153 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 788e23fe64d8..b35e220ae1b1 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -26,6 +26,7 @@ #include <linux/init.h> #include <linux/sched.h> #include <linux/irq.h> +#include <linux/irqchip/arm-gic.h>

#include <linux/atomic.h> #include <asm/cacheflush.h> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)

nmi_enter();

- /* nop. FIQ handlers for special arch/arm features can be added here. */ +#ifdef CONFIG_ARM_GIC + gic_handle_fiq_ipi(); +#endif

nmi_exit();

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index c172176499f6..c4f4a8827ed8 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -39,6 +39,7 @@ #include <linux/slab.h> #include <linux/irqchip/chained_irq.h> #include <linux/irqchip/arm-gic.h> +#include <linux/ratelimit.h>

#include <asm/cputype.h> #include <asm/irq.h> @@ -48,6 +49,10 @@ #include "irq-gic-common.h" #include "irqchip.h"

+#ifndef SMP_IPI_FIQ_MASK +#define SMP_IPI_FIQ_MASK 0 +#endif + union gic_base { void __iomem *common_base; void __percpu * __iomem *percpu_base; @@ -65,6 +70,7 @@ struct gic_chip_data { #endif struct irq_domain *domain; unsigned int gic_irqs; + u32 igroup0_shadow; #ifdef CONFIG_GIC_NON_BANKED void __iomem *(*get_base)(union gic_base *); #endif @@ -348,6 +354,83 @@ static struct irq_chip gic_chip = { .irq_set_wake = gic_set_wake, };

+/* + * Shift an interrupt between Group 0 and Group 1. + * + * In addition to changing the group we also modify the priority to + * match what "ARM strongly recommends" for a system where no Group 1 + * interrupt must ever preempt a Group 0 interrupt. + * + * If is safe to call this function on systems which do not support + * grouping (it will have no effect). + */ +static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq, + int group) +{ + void __iomem *base = gic_data_dist_base(gic); + unsigned int grp_reg = hwirq / 32 * 4; + u32 grp_mask = BIT(hwirq % 32); + u32 grp_val; + + unsigned int pri_reg = (hwirq / 4) * 4; + u32 pri_mask = BIT(7 + ((hwirq % 4) * 8)); + u32 pri_val; + + /* + * Systems which do not support grouping will have not have + * the EnableGrp1 bit set. + */ + if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))) + return; + + raw_spin_lock(&irq_controller_lock); + + grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg); + pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg); + + if (group) { + grp_val |= grp_mask; + pri_val |= pri_mask; + } else { + grp_val &= ~grp_mask; + pri_val &= ~pri_mask; + } + + writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg); + if (grp_reg == 0) + gic->igroup0_shadow = grp_val; + + writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg); + + raw_spin_unlock(&irq_controller_lock); +} + + +/* + * Fully acknowledge (both ack and eoi) any outstanding FIQ-based IPI, + * otherwise do nothing. + */ +void gic_handle_fiq_ipi(void) +{ + struct gic_chip_data *gic = &gic_data[0]; + void __iomem *cpu_base = gic_data_cpu_base(gic); + unsigned long irqstat, irqnr; + + if (WARN_ON(!in_nmi())) + return; + + while ((1u << readl_relaxed(cpu_base + GIC_CPU_HIGHPRI)) & + SMP_IPI_FIQ_MASK) { + irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK); + writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI); + + irqnr = irqstat & GICC_IAR_INT_ID_MASK; + WARN_RATELIMIT(irqnr > 16, + "Unexpected irqnr %lu (bad prioritization?)\n", + irqnr); + } +} + void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq) { if (gic_nr >= MAX_GIC_NR) @@ -379,15 +462,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic) static void gic_cpu_if_up(void) { void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); - u32 bypass = 0; + void __iomem *dist_base = gic_data_dist_base(&gic_data[0]); + u32 ctrl = 0;

/* - * Preserve bypass disable bits to be written back later - */ - bypass = readl(cpu_base + GIC_CPU_CTRL); - bypass &= GICC_DIS_BYPASS_MASK; + * Preserve bypass disable bits to be written back later + */ + ctrl = readl(cpu_base + GIC_CPU_CTRL); + ctrl &= GICC_DIS_BYPASS_MASK;

- writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); + /* + * If EnableGrp1 is set in the distributor then enable group 1 + * support for this CPU (and route group 0 interrupts to FIQ). + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) + ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL | + GICC_ENABLE_GRP1; + + writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); }

@@ -411,7 +503,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)

gic_dist_config(base, gic_irqs, NULL);

- writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL); + /* + * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only, + * bit 1 ignored) depending on current mode. + */ + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL); + + /* + * Set all global interrupts to be group 1 if (and only if) it + * is possible to enable group 1 interrupts. This register is RAZ/WI + * if not accessible or not implemented, however some GICv1 devices + * do not implement the EnableGrp1 bit making it unsafe to set + * this register unconditionally. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)) + for (i = 32; i < gic_irqs; i += 32) + writel_relaxed(0xffffffff, + base + GIC_DIST_IGROUP + i * 4 / 32); }

static void gic_cpu_init(struct gic_chip_data *gic) @@ -420,6 +528,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. @@ -438,6 +547,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)

gic_cpu_config(dist_base, NULL);

+ /* + * If the distributor is configured to support interrupt grouping + * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK + * to be group1 and ensure any remaining group 0 interrupts have + * the right priority. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) { + secure_irqs = SMP_IPI_FIQ_MASK; + writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0); + gic->igroup0_shadow = ~secure_irqs; + for_each_set_bit(secure_irq, &secure_irqs, 16) + gic_set_group_irq(gic, secure_irq, 0); + } + writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK); gic_cpu_if_up(); } @@ -527,7 +650,8 @@ static void gic_dist_restore(unsigned int gic_nr) writel_relaxed(gic_data[gic_nr].saved_spi_enable[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);

- writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL); + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, + dist_base + GIC_DIST_CTRL); }

static void gic_cpu_save(unsigned int gic_nr) @@ -655,6 +779,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) { int cpu; unsigned long map = 0; + unsigned long softint;

bl_migration_lock();

@@ -668,8 +793,14 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) */ dmb(ishst);

- /* this always happens on GIC0 */ - writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT); + /* We avoid a readl here by using the shadow copy of IGROUP[0] */ + softint = map << 16 | irq; + if (gic_data[0].igroup0_shadow & BIT(irq)) + softint |= 0x8000; + + /* This always happens on GIC0 */ + writel_relaxed(softint, + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

bl_migration_unlock(); } diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h index 71d706d5f169..7690f70049a3 100644 --- a/include/linux/irqchip/arm-gic.h +++ b/include/linux/irqchip/arm-gic.h @@ -22,6 +22,10 @@ #define GIC_CPU_IDENT 0xfc

#define GICC_ENABLE 0x1 +#define GICC_ENABLE_GRP1 0x2 +#define GICC_ACK_CTL 0x4 +#define GICC_FIQ_EN 0x8 +#define GICC_COMMON_BPR 0x10 #define GICC_INT_PRI_THRESHOLD 0xf0 #define GICC_IAR_INT_ID_MASK 0x3ff #define GICC_INT_SPURIOUS 1023 @@ -44,6 +48,7 @@ #define GIC_DIST_SGI_PENDING_SET 0xf20

#define GICD_ENABLE 0x1 +#define GICD_ENABLE_GRP1 0x2 #define GICD_DISABLE 0x0 #define GICD_INT_ACTLOW_LVLTRIG 0x0 #define GICD_INT_EN_CLR_X32 0xffffffff @@ -121,5 +126,8 @@ static inline void __init register_routable_domain_ops { gic_routable_irq_domain_ops = ops; } + +void gic_handle_fiq_ipi(void); + #endif /* __ASSEMBLY */ #endif

Currently if arch_trigger_all_cpu_backtrace() is called with interrupts disabled and on a platform the delivers IPI_CPU_BACKTRACE using regular IRQ requests the system will wedge for ten seconds waiting for the current CPU to react to a masked interrupt.

This patch resolves this issue by calling directly into the backtrace dump code instead of generating an IPI.

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Steven Rostedt rostedt@goodmis.org --- arch/arm/kernel/smp.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 12667eb68198..644f654f7a7e 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -767,7 +767,10 @@ void ipi_cpu_backtrace(struct pt_regs *regs) /* Replace printk to write into the NMI seq */ this_cpu_write(printk_func, nmi_vprintk); printk(KERN_WARNING "NMI backtrace for cpu %d\n", cpu); - show_regs(regs); + if (regs != NULL) + show_regs(regs); + else + dump_stack(); this_cpu_write(printk_func, printk_func_save);

cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask)); @@ -812,6 +815,16 @@ void arch_trigger_all_cpu_backtrace(bool include_self) seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE); }

+ /* + * If irqs are disabled on the current processor then, if + * IPI_CPU_BACKTRACE is delivered using IRQ, we will won't be able to + * react to IPI_CPU_BACKTRACE until we leave this function. We avoid + * the potential timeout (not to mention the failure to print useful + * information) by calling the backtrace directly. + */ + if (include_self && irqs_disabled()) + ipi_cpu_backtrace(in_interrupt() ? get_irq_regs() : NULL); + if (!cpumask_empty(to_cpumask(backtrace_mask))) { pr_info("Sending NMI to %s CPUs:\n", (include_self ? "all" : "other"));

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

This is sub-optimal in closely related ways:

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com --- drivers/irqchip/irq-gic.c | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index d617ee5a3d8a..a9ed64dcc84b 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -73,6 +73,27 @@ struct gic_chip_data { static DEFINE_RAW_SPINLOCK(irq_controller_lock);

/* + * This lock is used by the big.LITTLE migration code to ensure no IPIs + * can be pended on the old core after the map has been updated. + */ +#ifdef CONFIG_BL_SWITCHER +static DEFINE_RAW_SPINLOCK(cpu_map_migration_lock); + +static inline void bl_migration_lock(unsigned long *flags) +{ + raw_spin_lock_irqsave(&cpu_map_migration_lock, *flags); +} + +static inline void bl_migration_unlock(unsigned long flags) +{ + raw_spin_unlock_irqrestore(&cpu_map_migration_lock, flags); +} +#else +static inline void bl_migration_lock(unsigned long *flags) {} +static inline void bl_migration_unlock(unsigned long flags) {} +#endif + +/* * The GIC mapping of CPU interfaces does not necessarily match * the logical CPU numbering. Let's use a mapping as returned * by the GIC itself. @@ -624,7 +645,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0;

- raw_spin_lock_irqsave(&irq_controller_lock, flags); + bl_migration_lock(&flags);

/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -639,7 +660,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

- raw_spin_unlock_irqrestore(&irq_controller_lock, flags); + bl_migration_unlock(flags); } #endif

@@ -710,8 +731,17 @@ void gic_migrate_target(unsigned int new_cpu_id)

raw_spin_lock(&irq_controller_lock);

- /* Update the target interface for this logical CPU */ + /* + * Update the target interface for this logical CPU + * + * From the point we release the cpu_map_migration_lock any new + * SGIs will be pended on the new cpu which makes the set of SGIs + * pending on the old cpu static. That means we can defer the + * migration until after we have released the irq_controller_lock. + */ + raw_spin_lock(&cpu_map_migration_lock); gic_cpu_map[cpu] = 1 << new_cpu_id; + raw_spin_unlock(&cpu_map_migration_lock);

/* * Find all the peripheral interrupts targetting the current

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

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

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

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

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jason Cooper jason@lakedaemon.net Cc: Russell King linux@arm.linux.org.uk Cc: Marc Zyngier marc.zyngier@arm.com Tested-by: Jon Medhurst tixy@linaro.org --- arch/arm/kernel/traps.c | 5 +- drivers/irqchip/irq-gic.c | 151 +++++++++++++++++++++++++++++++++++++--- include/linux/irqchip/arm-gic.h | 8 +++ 3 files changed, 153 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 788e23fe64d8..b35e220ae1b1 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -26,6 +26,7 @@ #include <linux/init.h> #include <linux/sched.h> #include <linux/irq.h> +#include <linux/irqchip/arm-gic.h>

#include <linux/atomic.h> #include <asm/cacheflush.h> @@ -479,7 +480,9 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs)

nmi_enter();

- /* nop. FIQ handlers for special arch/arm features can be added here. */ +#ifdef CONFIG_ARM_GIC + gic_handle_fiq_ipi(); +#endif

nmi_exit();

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index c172176499f6..c4f4a8827ed8 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -39,6 +39,7 @@ #include <linux/slab.h> #include <linux/irqchip/chained_irq.h> #include <linux/irqchip/arm-gic.h> +#include <linux/ratelimit.h>

#include <asm/cputype.h> #include <asm/irq.h> @@ -48,6 +49,10 @@ #include "irq-gic-common.h" #include "irqchip.h"

+#ifndef SMP_IPI_FIQ_MASK +#define SMP_IPI_FIQ_MASK 0 +#endif + union gic_base { void __iomem *common_base; void __percpu * __iomem *percpu_base; @@ -65,6 +70,7 @@ struct gic_chip_data { #endif struct irq_domain *domain; unsigned int gic_irqs; + u32 igroup0_shadow; #ifdef CONFIG_GIC_NON_BANKED void __iomem *(*get_base)(union gic_base *); #endif @@ -348,6 +354,83 @@ static struct irq_chip gic_chip = { .irq_set_wake = gic_set_wake, };

+/* + * Shift an interrupt between Group 0 and Group 1. + * + * In addition to changing the group we also modify the priority to + * match what "ARM strongly recommends" for a system where no Group 1 + * interrupt must ever preempt a Group 0 interrupt. + * + * If is safe to call this function on systems which do not support + * grouping (it will have no effect). + */ +static void gic_set_group_irq(struct gic_chip_data *gic, unsigned int hwirq, + int group) +{ + void __iomem *base = gic_data_dist_base(gic); + unsigned int grp_reg = hwirq / 32 * 4; + u32 grp_mask = BIT(hwirq % 32); + u32 grp_val; + + unsigned int pri_reg = (hwirq / 4) * 4; + u32 pri_mask = BIT(7 + ((hwirq % 4) * 8)); + u32 pri_val; + + /* + * Systems which do not support grouping will have not have + * the EnableGrp1 bit set. + */ + if (!(GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL))) + return; + + raw_spin_lock(&irq_controller_lock); + + grp_val = readl_relaxed(base + GIC_DIST_IGROUP + grp_reg); + pri_val = readl_relaxed(base + GIC_DIST_PRI + pri_reg); + + if (group) { + grp_val |= grp_mask; + pri_val |= pri_mask; + } else { + grp_val &= ~grp_mask; + pri_val &= ~pri_mask; + } + + writel_relaxed(grp_val, base + GIC_DIST_IGROUP + grp_reg); + if (grp_reg == 0) + gic->igroup0_shadow = grp_val; + + writel_relaxed(pri_val, base + GIC_DIST_PRI + pri_reg); + + raw_spin_unlock(&irq_controller_lock); +} + + +/* + * Fully acknowledge (both ack and eoi) any outstanding FIQ-based IPI, + * otherwise do nothing. + */ +void gic_handle_fiq_ipi(void) +{ + struct gic_chip_data *gic = &gic_data[0]; + void __iomem *cpu_base = gic_data_cpu_base(gic); + unsigned long irqstat, irqnr; + + if (WARN_ON(!in_nmi())) + return; + + while ((1u << readl_relaxed(cpu_base + GIC_CPU_HIGHPRI)) & + SMP_IPI_FIQ_MASK) { + irqstat = readl_relaxed(cpu_base + GIC_CPU_INTACK); + writel_relaxed(irqstat, cpu_base + GIC_CPU_EOI); + + irqnr = irqstat & GICC_IAR_INT_ID_MASK; + WARN_RATELIMIT(irqnr > 16, + "Unexpected irqnr %lu (bad prioritization?)\n", + irqnr); + } +} + void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq) { if (gic_nr >= MAX_GIC_NR) @@ -379,15 +462,24 @@ static u8 gic_get_cpumask(struct gic_chip_data *gic) static void gic_cpu_if_up(void) { void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]); - u32 bypass = 0; + void __iomem *dist_base = gic_data_dist_base(&gic_data[0]); + u32 ctrl = 0;

/* - * Preserve bypass disable bits to be written back later - */ - bypass = readl(cpu_base + GIC_CPU_CTRL); - bypass &= GICC_DIS_BYPASS_MASK; + * Preserve bypass disable bits to be written back later + */ + ctrl = readl(cpu_base + GIC_CPU_CTRL); + ctrl &= GICC_DIS_BYPASS_MASK;

- writel_relaxed(bypass | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); + /* + * If EnableGrp1 is set in the distributor then enable group 1 + * support for this CPU (and route group 0 interrupts to FIQ). + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) + ctrl |= GICC_COMMON_BPR | GICC_FIQ_EN | GICC_ACK_CTL | + GICC_ENABLE_GRP1; + + writel_relaxed(ctrl | GICC_ENABLE, cpu_base + GIC_CPU_CTRL); }

@@ -411,7 +503,23 @@ static void __init gic_dist_init(struct gic_chip_data *gic)

gic_dist_config(base, gic_irqs, NULL);

- writel_relaxed(GICD_ENABLE, base + GIC_DIST_CTRL); + /* + * Set EnableGrp1/EnableGrp0 (bit 1 and 0) or EnableGrp (bit 0 only, + * bit 1 ignored) depending on current mode. + */ + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, base + GIC_DIST_CTRL); + + /* + * Set all global interrupts to be group 1 if (and only if) it + * is possible to enable group 1 interrupts. This register is RAZ/WI + * if not accessible or not implemented, however some GICv1 devices + * do not implement the EnableGrp1 bit making it unsafe to set + * this register unconditionally. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(base + GIC_DIST_CTRL)) + for (i = 32; i < gic_irqs; i += 32) + writel_relaxed(0xffffffff, + base + GIC_DIST_IGROUP + i * 4 / 32); }

static void gic_cpu_init(struct gic_chip_data *gic) @@ -420,6 +528,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. @@ -438,6 +547,20 @@ static void gic_cpu_init(struct gic_chip_data *gic)

gic_cpu_config(dist_base, NULL);

+ /* + * If the distributor is configured to support interrupt grouping + * then set any PPI and SGI interrupts not set in SMP_IPI_FIQ_MASK + * to be group1 and ensure any remaining group 0 interrupts have + * the right priority. + */ + if (GICD_ENABLE_GRP1 & readl_relaxed(dist_base + GIC_DIST_CTRL)) { + secure_irqs = SMP_IPI_FIQ_MASK; + writel_relaxed(~secure_irqs, dist_base + GIC_DIST_IGROUP + 0); + gic->igroup0_shadow = ~secure_irqs; + for_each_set_bit(secure_irq, &secure_irqs, 16) + gic_set_group_irq(gic, secure_irq, 0); + } + writel_relaxed(GICC_INT_PRI_THRESHOLD, base + GIC_CPU_PRIMASK); gic_cpu_if_up(); } @@ -527,7 +650,8 @@ static void gic_dist_restore(unsigned int gic_nr) writel_relaxed(gic_data[gic_nr].saved_spi_enable[i], dist_base + GIC_DIST_ENABLE_SET + i * 4);

- writel_relaxed(GICD_ENABLE, dist_base + GIC_DIST_CTRL); + writel_relaxed(GICD_ENABLE_GRP1 | GICD_ENABLE, + dist_base + GIC_DIST_CTRL); }

static void gic_cpu_save(unsigned int gic_nr) @@ -655,6 +779,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) { int cpu; unsigned long map = 0; + unsigned long softint;

bl_migration_lock();

@@ -668,8 +793,14 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) */ dmb(ishst);

- /* this always happens on GIC0 */ - writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT); + /* We avoid a readl here by using the shadow copy of IGROUP[0] */ + softint = map << 16 | irq; + if (gic_data[0].igroup0_shadow & BIT(irq)) + softint |= 0x8000; + + /* This always happens on GIC0 */ + writel_relaxed(softint, + gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);

bl_migration_unlock(); } diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h index 71d706d5f169..7690f70049a3 100644 --- a/include/linux/irqchip/arm-gic.h +++ b/include/linux/irqchip/arm-gic.h @@ -22,6 +22,10 @@ #define GIC_CPU_IDENT 0xfc

#define GICC_ENABLE 0x1 +#define GICC_ENABLE_GRP1 0x2 +#define GICC_ACK_CTL 0x4 +#define GICC_FIQ_EN 0x8 +#define GICC_COMMON_BPR 0x10 #define GICC_INT_PRI_THRESHOLD 0xf0 #define GICC_IAR_INT_ID_MASK 0x3ff #define GICC_INT_SPURIOUS 1023 @@ -44,6 +48,7 @@ #define GIC_DIST_SGI_PENDING_SET 0xf20

#define GICD_ENABLE 0x1 +#define GICD_ENABLE_GRP1 0x2 #define GICD_DISABLE 0x0 #define GICD_INT_ACTLOW_LVLTRIG 0x0 #define GICD_INT_EN_CLR_X32 0xffffffff @@ -121,5 +126,8 @@ static inline void __init register_routable_domain_ops { gic_routable_irq_domain_ops = ops; } + +void gic_handle_fiq_ipi(void); + #endif /* __ASSEMBLY */ #endif

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

Port the x86 NMI backtrace to the generic code.

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

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ba397bde7948..f36d3058968e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -138,6 +138,7 @@ config X86 select HAVE_ACPI_APEI_NMI if ACPI select ACPI_LEGACY_TABLES_LOOKUP if ACPI select X86_FEATURE_NAMES if PROC_FS + select ARCH_WANT_NMI_PRINTK if X86_LOCAL_APIC

config INSTRUCTION_DECODER def_bool y diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c index 6873ab925d00..7d4a5c8ac510 100644 --- a/arch/x86/kernel/apic/hw_nmi.c +++ b/arch/x86/kernel/apic/hw_nmi.c @@ -32,26 +32,6 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh) static DECLARE_BITMAP(backtrace_mask, NR_CPUS) __read_mostly; static cpumask_t printtrace_mask;

-#define NMI_BUF_SIZE 4096 - -struct nmi_seq_buf { - unsigned char buffer[NMI_BUF_SIZE]; - struct seq_buf seq; -}; - -/* Safe printing in NMI context */ -static DEFINE_PER_CPU(struct nmi_seq_buf, nmi_print_seq); - -/* "in progress" flag of arch_trigger_all_cpu_backtrace */ -static unsigned long backtrace_flag; - -static void print_seq_line(struct nmi_seq_buf *s, int start, int end) -{ - const char *buf = s->buffer + start; - - printk("%.*s", (end - start) + 1, buf); -} - void arch_trigger_all_cpu_backtrace(bool include_self) { struct nmi_seq_buf *s; @@ -60,28 +40,18 @@ void arch_trigger_all_cpu_backtrace(bool include_self) int i; int this_cpu = get_cpu();

- if (test_and_set_bit(0, &backtrace_flag)) { + if (0 != prepare_nmi_printk(to_cpumask(backtrace_mask))) { /* - * If there is already a trigger_all_cpu_backtrace() in progress - * (backtrace_flag == 1), don't output double cpu dump infos. + * If there is already an nmi printk sequence in + * progress then just give up... */ put_cpu(); return; }

- cpumask_copy(to_cpumask(backtrace_mask), cpu_online_mask); if (!include_self) cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask)); - cpumask_copy(&printtrace_mask, to_cpumask(backtrace_mask)); - /* - * Set up per_cpu seq_buf buffers that the NMIs running on the other - * CPUs will write to. - */ - for_each_cpu(cpu, to_cpumask(backtrace_mask)) { - s = &per_cpu(nmi_print_seq, cpu); - seq_buf_init(&s->seq, s->buffer, NMI_BUF_SIZE); - }

if (!cpumask_empty(to_cpumask(backtrace_mask))) { pr_info("sending NMI to %s CPUs:\n", @@ -97,73 +67,23 @@ void arch_trigger_all_cpu_backtrace(bool include_self) touch_softlockup_watchdog(); }

- /* - * Now that all the NMIs have triggered, we can dump out their - * back traces safely to the console. - */ - for_each_cpu(cpu, &printtrace_mask) { - int last_i = 0; - - s = &per_cpu(nmi_print_seq, cpu); - len = seq_buf_used(&s->seq); - if (!len) - continue; - - /* Print line by line. */ - for (i = 0; i < len; i++) { - if (s->buffer[i] == '\n') { - print_seq_line(s, last_i, i); - last_i = i + 1; - } - } - /* Check if there was a partial line. */ - if (last_i < len) { - print_seq_line(s, last_i, len - 1); - pr_cont("\n"); - } - } - - clear_bit(0, &backtrace_flag); - smp_mb__after_atomic(); + complete_nmi_printk(&printtrace_mask); put_cpu(); }

-/* - * It is not safe to call printk() directly from NMI handlers. - * It may be fine if the NMI detected a lock up and we have no choice - * but to do so, but doing a NMI on all other CPUs to get a back trace - * can be done with a sysrq-l. We don't want that to lock up, which - * can happen if the NMI interrupts a printk in progress. - * - * Instead, we redirect the vprintk() to this nmi_vprintk() that writes - * the content into a per cpu seq_buf buffer. Then when the NMIs are - * all done, we can safely dump the contents of the seq_buf to a printk() - * from a non NMI context. - */ -static int nmi_vprintk(const char *fmt, va_list args) -{ - struct nmi_seq_buf *s = this_cpu_ptr(&nmi_print_seq); - unsigned int len = seq_buf_used(&s->seq); - - seq_buf_vprintf(&s->seq, fmt, args); - return seq_buf_used(&s->seq) - len; -} - static int arch_trigger_all_cpu_backtrace_handler(unsigned int cmd, struct pt_regs *regs) { int cpu; + printk_func_t orig;

cpu = smp_processor_id();

if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) { - printk_func_t printk_func_save = this_cpu_read(printk_func); - - /* Replace printk to write into the NMI seq */ - this_cpu_write(printk_func, nmi_vprintk); + orig = this_cpu_begin_nmi_printk(); printk(KERN_WARNING "NMI backtrace for cpu %d\n", cpu); show_regs(regs); - this_cpu_write(printk_func, printk_func_save); + this_cpu_end_nmi_printk(orig);

cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask)); return NMI_HANDLED;

Currently if arch_trigger_all_cpu_backtrace() is called with interrupts disabled and on a platform the delivers IPI_CPU_BACKTRACE using regular IRQ requests the system will wedge for ten seconds waiting for the current CPU to react to a masked interrupt.

This patch resolves this issue by calling directly into the backtrace dump code instead of generating an IPI.

Signed-off-by: Daniel Thompson daniel.thompson@linaro.org Cc: Steven Rostedt rostedt@goodmis.org --- arch/arm/kernel/smp.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index afb094a1e6d4..cf3b738568b8 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -742,6 +742,16 @@ void arch_trigger_all_cpu_backtrace(bool include_self) cpumask_clear_cpu(this_cpu, to_cpumask(backtrace_mask)); cpumask_copy(&printtrace_mask, to_cpumask(backtrace_mask));

+ /* + * If irqs are disabled on the current processor then, if + * IPI_CPU_BACKTRACE is delivered using IRQ, we will won't be able to + * react to IPI_CPU_BACKTRACE until we leave this function. We avoid + * the potential timeout (not to mention the failure to print useful + * information) by calling the backtrace directly. + */ + if (include_self && irqs_disabled()) + ipi_cpu_backtrace(in_interrupt() ? get_irq_regs() : NULL); + if (!cpumask_empty(to_cpumask(backtrace_mask))) { pr_info("Sending FIQ to %s CPUs:\n", (include_self ? "all" : "other")); @@ -770,7 +780,10 @@ void ipi_cpu_backtrace(struct pt_regs *regs) if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) { orig = this_cpu_begin_nmi_printk(); pr_warn("FIQ backtrace for cpu %d\n", cpu); - show_regs(regs); + if (regs != NULL) + show_regs(regs); + else + dump_stack(); this_cpu_end_nmi_printk(orig);

cpumask_clear_cpu(cpu, to_cpumask(backtrace_mask));