From: Lukas Wunner <lukas@wunner.de> To: Thomas Gleixner <tglx@linutronix.de>, Jason Cooper <jason@lakedaemon.net>, Marc Zyngier <maz@kernel.org> Cc: Florian Fainelli <f.fainelli@gmail.com>, Ray Jui <rjui@broadcom.com>, Scott Branden <sbranden@broadcom.com>, bcm-kernel-feedback-list@broadcom.com, linux-kernel@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, "Nicolas Saenz Julienne" <nsaenzjulienne@suse.de>, Serge Schneider <serge@raspberrypi.org>, Kristina Brooks <notstina@gmail.com>, Stefan Wahren <wahrenst@gmx.net>, Matthias Brugger <mbrugger@suse.com>, Martin Sperl <kernel@martin.sperl.org>, Phil Elwell <phil@raspberrypi.org> Subject: [PATCH v4] irqchip/bcm2835: Quiesce IRQs left enabled by bootloader Date: Tue, 25 Feb 2020 10:50:41 +0100 [thread overview] Message-ID: <f97868ba4e9b86ddad71f44ec9d8b3b7d8daa1ea.1582618537.git.lukas@wunner.de> (raw) In-Reply-To: <1a5735e8-b876-92e4-9f1e-687f5abf8708@i2se.com> Per the spec, the BCM2835's IRQs are all disabled when coming out of power-on reset. Its IRQ driver assumes that's still the case when the kernel boots and does not perform any initialization of the registers. However the Raspberry Pi Foundation's bootloader leaves the USB interrupt enabled when handing over control to the kernel. Quiesce IRQs and the FIQ if they were left enabled and log a message to let users know that they should update the bootloader once a fixed version is released. If the USB interrupt is not quiesced and the USB driver later on claims the FIQ (as it does on the Raspberry Pi Foundation's downstream kernel), interrupt latency for all other peripherals increases and occasional lockups occur. That's because both the FIQ and the normal USB interrupt fire simultaneously: On a multicore Raspberry Pi, if normal interrupts are routed to CPU 0 and the FIQ to CPU 1 (hardcoded in the Foundation's kernel), then a USB interrupt causes CPU 0 to spin in bcm2836_chained_handle_irq() until the FIQ on CPU 1 has cleared it. Other peripherals' interrupts are starved as long. I've seen CPU 0 blocked for up to 2.9 msec. eMMC throughput on a Compute Module 3 irregularly dips to 23.0 MB/s without this commit but remains relatively constant at 23.5 MB/s with this commit. The lockups occur when CPU 0 receives a USB interrupt while holding a lock which CPU 1 is trying to acquire while the FIQ is temporarily disabled on CPU 1. At best users get RCU CPU stall warnings, but most of the time the system just freezes. Fixes: 89214f009c1d ("ARM: bcm2835: add interrupt controller driver") Signed-off-by: Lukas Wunner <lukas@wunner.de> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Cc: stable@vger.kernel.org # v3.7+ Cc: Serge Schneider <serge@raspberrypi.org> Cc: Kristina Brooks <notstina@gmail.com> Cc: Stefan Wahren <wahrenst@gmx.net> --- v4: * Add missing REG_FIQ_ENABLE macro, rename to FIQ_CONTROL_ENABLE (Stefan) v3: (submitted as inline patch) * Shorten commit message (Florian, Marc) v2: * Use "relaxed" MMIO accessors to avoid memory barriers (Marc) * Use u32 instead of int for register access (Marc) * Quiesce FIQ as well (Marc) * Quiesce IRQs after mapping them for better readability * Drop alternative approach from commit message (Marc) drivers/irqchip/irq-bcm2835.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c index 418245d31921..a1e004af23e7 100644 --- a/drivers/irqchip/irq-bcm2835.c +++ b/drivers/irqchip/irq-bcm2835.c @@ -61,6 +61,7 @@ | SHORTCUT1_MASK | SHORTCUT2_MASK) #define REG_FIQ_CONTROL 0x0c +#define FIQ_CONTROL_ENABLE BIT(7) #define NR_BANKS 3 #define IRQS_PER_BANK 32 @@ -135,6 +136,7 @@ static int __init armctrl_of_init(struct device_node *node, { void __iomem *base; int irq, b, i; + u32 reg; base = of_iomap(node, 0); if (!base) @@ -157,6 +159,19 @@ static int __init armctrl_of_init(struct device_node *node, handle_level_irq); irq_set_probe(irq); } + + reg = readl_relaxed(intc.enable[b]); + if (reg) { + writel_relaxed(reg, intc.disable[b]); + pr_err(FW_BUG "Bootloader left irq enabled: " + "bank %d irq %*pbl\n", b, IRQS_PER_BANK, ®); + } + } + + reg = readl_relaxed(base + REG_FIQ_CONTROL); + if (reg & FIQ_CONTROL_ENABLE) { + writel_relaxed(0, base + REG_FIQ_CONTROL); + pr_err(FW_BUG "Bootloader left fiq enabled\n"); } if (is_2836) { -- 2.24.0
WARNING: multiple messages have this Message-ID (diff)
From: Lukas Wunner <lukas@wunner.de> To: Thomas Gleixner <tglx@linutronix.de>, Jason Cooper <jason@lakedaemon.net>, Marc Zyngier <maz@kernel.org> Cc: Florian Fainelli <f.fainelli@gmail.com>, Kristina Brooks <notstina@gmail.com>, Scott Branden <sbranden@broadcom.com>, Ray Jui <rjui@broadcom.com>, Serge Schneider <serge@raspberrypi.org>, linux-kernel@vger.kernel.org, Phil Elwell <phil@raspberrypi.org>, Nicolas Saenz Julienne <nsaenzjulienne@suse.de>, Matthias Brugger <mbrugger@suse.com>, bcm-kernel-feedback-list@broadcom.com, linux-rpi-kernel@lists.infradead.org, Martin Sperl <kernel@martin.sperl.org>, linux-arm-kernel@lists.infradead.org, Stefan Wahren <wahrenst@gmx.net> Subject: [PATCH v4] irqchip/bcm2835: Quiesce IRQs left enabled by bootloader Date: Tue, 25 Feb 2020 10:50:41 +0100 [thread overview] Message-ID: <f97868ba4e9b86ddad71f44ec9d8b3b7d8daa1ea.1582618537.git.lukas@wunner.de> (raw) In-Reply-To: <1a5735e8-b876-92e4-9f1e-687f5abf8708@i2se.com> Per the spec, the BCM2835's IRQs are all disabled when coming out of power-on reset. Its IRQ driver assumes that's still the case when the kernel boots and does not perform any initialization of the registers. However the Raspberry Pi Foundation's bootloader leaves the USB interrupt enabled when handing over control to the kernel. Quiesce IRQs and the FIQ if they were left enabled and log a message to let users know that they should update the bootloader once a fixed version is released. If the USB interrupt is not quiesced and the USB driver later on claims the FIQ (as it does on the Raspberry Pi Foundation's downstream kernel), interrupt latency for all other peripherals increases and occasional lockups occur. That's because both the FIQ and the normal USB interrupt fire simultaneously: On a multicore Raspberry Pi, if normal interrupts are routed to CPU 0 and the FIQ to CPU 1 (hardcoded in the Foundation's kernel), then a USB interrupt causes CPU 0 to spin in bcm2836_chained_handle_irq() until the FIQ on CPU 1 has cleared it. Other peripherals' interrupts are starved as long. I've seen CPU 0 blocked for up to 2.9 msec. eMMC throughput on a Compute Module 3 irregularly dips to 23.0 MB/s without this commit but remains relatively constant at 23.5 MB/s with this commit. The lockups occur when CPU 0 receives a USB interrupt while holding a lock which CPU 1 is trying to acquire while the FIQ is temporarily disabled on CPU 1. At best users get RCU CPU stall warnings, but most of the time the system just freezes. Fixes: 89214f009c1d ("ARM: bcm2835: add interrupt controller driver") Signed-off-by: Lukas Wunner <lukas@wunner.de> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Cc: stable@vger.kernel.org # v3.7+ Cc: Serge Schneider <serge@raspberrypi.org> Cc: Kristina Brooks <notstina@gmail.com> Cc: Stefan Wahren <wahrenst@gmx.net> --- v4: * Add missing REG_FIQ_ENABLE macro, rename to FIQ_CONTROL_ENABLE (Stefan) v3: (submitted as inline patch) * Shorten commit message (Florian, Marc) v2: * Use "relaxed" MMIO accessors to avoid memory barriers (Marc) * Use u32 instead of int for register access (Marc) * Quiesce FIQ as well (Marc) * Quiesce IRQs after mapping them for better readability * Drop alternative approach from commit message (Marc) drivers/irqchip/irq-bcm2835.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c index 418245d31921..a1e004af23e7 100644 --- a/drivers/irqchip/irq-bcm2835.c +++ b/drivers/irqchip/irq-bcm2835.c @@ -61,6 +61,7 @@ | SHORTCUT1_MASK | SHORTCUT2_MASK) #define REG_FIQ_CONTROL 0x0c +#define FIQ_CONTROL_ENABLE BIT(7) #define NR_BANKS 3 #define IRQS_PER_BANK 32 @@ -135,6 +136,7 @@ static int __init armctrl_of_init(struct device_node *node, { void __iomem *base; int irq, b, i; + u32 reg; base = of_iomap(node, 0); if (!base) @@ -157,6 +159,19 @@ static int __init armctrl_of_init(struct device_node *node, handle_level_irq); irq_set_probe(irq); } + + reg = readl_relaxed(intc.enable[b]); + if (reg) { + writel_relaxed(reg, intc.disable[b]); + pr_err(FW_BUG "Bootloader left irq enabled: " + "bank %d irq %*pbl\n", b, IRQS_PER_BANK, ®); + } + } + + reg = readl_relaxed(base + REG_FIQ_CONTROL); + if (reg & FIQ_CONTROL_ENABLE) { + writel_relaxed(0, base + REG_FIQ_CONTROL); + pr_err(FW_BUG "Bootloader left fiq enabled\n"); } if (is_2836) { -- 2.24.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-02-25 9:50 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-07 15:46 [PATCH] irqchip/bcm2835: Quiesce IRQs left enabled by bootloader Lukas Wunner 2020-02-07 15:46 ` Lukas Wunner 2020-02-07 16:11 ` Marc Zyngier 2020-02-07 16:11 ` Marc Zyngier 2020-02-10 9:52 ` [PATCH v2] " Lukas Wunner 2020-02-10 9:52 ` Lukas Wunner 2020-02-12 4:47 ` Florian Fainelli 2020-02-12 4:47 ` Florian Fainelli 2020-02-12 8:13 ` Marc Zyngier 2020-02-12 8:13 ` Marc Zyngier 2020-02-12 12:36 ` Lukas Wunner 2020-02-12 12:55 ` Nicolas Saenz Julienne 2020-02-12 12:55 ` Nicolas Saenz Julienne 2020-02-23 17:59 ` Stefan Wahren 2020-02-23 17:59 ` Stefan Wahren 2020-02-23 18:24 ` Lukas Wunner 2020-02-24 9:21 ` Stefan Wahren 2020-02-24 9:21 ` Stefan Wahren 2020-02-25 9:50 ` Lukas Wunner [this message] 2020-02-25 9:50 ` [PATCH v4] " Lukas Wunner 2020-03-29 20:26 ` [tip: irq/core] " tip-bot2 for Lukas Wunner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=f97868ba4e9b86ddad71f44ec9d8b3b7d8daa1ea.1582618537.git.lukas@wunner.de \ --to=lukas@wunner.de \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=f.fainelli@gmail.com \ --cc=jason@lakedaemon.net \ --cc=kernel@martin.sperl.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-rpi-kernel@lists.infradead.org \ --cc=maz@kernel.org \ --cc=mbrugger@suse.com \ --cc=notstina@gmail.com \ --cc=nsaenzjulienne@suse.de \ --cc=phil@raspberrypi.org \ --cc=rjui@broadcom.com \ --cc=sbranden@broadcom.com \ --cc=serge@raspberrypi.org \ --cc=tglx@linutronix.de \ --cc=wahrenst@gmx.net \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.