Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1453086 > unrolled thread
| Started by | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| First post | 2016-08-01 12:00 +0200 |
| Last post | 2016-08-01 16:30 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/2] Fix arch timer trigger Marc Zyngier <marc.zyngier@arm.com> - 2016-08-01 12:00 +0200
[PATCH v2 1/2] clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered Marc Zyngier <marc.zyngier@arm.com> - 2016-08-01 12:00 +0200
[tip:timers/urgent] clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered tip-bot for Marc Zyngier <tipbot@zytor.com> - 2016-08-01 15:30 +0200
[tip:timers/urgent] clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered tip-bot for Marc Zyngier <tipbot@zytor.com> - 2016-08-01 16:30 +0200
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-01 12:00 +0200 |
| Subject | [PATCH v2 0/2] Fix arch timer trigger |
| Message-ID | <s1iaR-4Cd-7@gated-at.bofh.it> |
Recent fixes in the IRQ layer (properly propagating the trigger
settings in the interrupt controler instead of silently failing)
uncovered a number of cases where DTs are describing the timer trigger
wrong (edge, while the interrupt is definitely level).
The result is a kernel that doesn't boot anymore (forever waiting for
an interrupt that can't trigger) or behave erratically (missing
interrupts).
This very small series addresses the issue in two ways:
(1) enforcing level triggered in the driver and spitting out a nastygram
so that the user can find out about the issue
(2) fix the existing issue by repainting the broken device trees.
(1) makes sure that a new kernel can run with an old DT, and (2)
addresses the issue properly.
Note that the two patches can be applied independently.
Tested on APM Mustang, which is one of the broken platforms.
Thanks,
M.
* From v1:
- Make GICv3-based platforms use the level triggered setting even if
the HW actually precisly describes high or low settings.
- Rebased on top of linux/master as of today.
Marc Zyngier (2):
clocksource/arm_arch_timer: Force per-CPU interrupt to be
level-triggered
arm64: dts: Fix broken architected timer interrupt trigger
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 8 +++----
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 8 +++----
arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 +++----
arch/arm64/boot/dts/broadcom/ns2.dtsi | 8 +++----
arch/arm64/boot/dts/cavium/thunder-88xx.dtsi | 8 +++----
arch/arm64/boot/dts/exynos/exynos7.dtsi | 8 +++----
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 8 +++----
arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 8 +++----
arch/arm64/boot/dts/marvell/armada-ap806.dtsi | 8 +++----
.../boot/dts/socionext/uniphier-ph1-ld20.dtsi | 8 +++----
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 8 +++----
drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++---
12 files changed, 67 insertions(+), 47 deletions(-)
--
2.1.4
[toc] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2016-08-01 12:00 +0200 |
| Subject | [PATCH v2 1/2] clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered |
| Message-ID | <s1iaS-4Cd-27@gated-at.bofh.it> |
| In reply to | #1453086 |
The ARM architected timer produces level-triggered interrupts (this
is mandated by the architecture). Unfortunately, a number of
device-trees get this wrong, and expose an edge-triggered interrupt.
Until now, this wasn't too much an issue, as the programming of the
trigger would fail (the corresponding PPI cannot be reconfigured),
and the kernel would be happy with this. But we're about to change
this, and trust DT a lot if the driver doesn't provide its own
trigger information. In that context, the timer breaks badly.
While we do need to fix the DTs, there is also some userspace out
there (kvmtool) that generates the same kind of broken DT on the
fly, and that will completely break with newer kernels.
As a safety measure, and to keep buggy software alive as well as
buying us some time to fix DTs all over the place, let's check
what trigger configuration has been given us by the firmware.
If this is not a level configuration, then we know that the
DT/ACPI configuration is bust, and we pick some defaults which
won't be worse than the existing setup.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 28bce3f..5770054 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -8,6 +8,9 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+
+#define pr_fmt(fmt) "arm_arch_timer: " fmt
+
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/device.h>
@@ -370,16 +373,33 @@ static bool arch_timer_has_nonsecure_ppi(void)
arch_timer_ppi[PHYS_NONSECURE_PPI]);
}
+static u32 check_ppi_trigger(int irq)
+{
+ u32 flags = irq_get_trigger_type(irq);
+
+ if (flags != IRQF_TRIGGER_HIGH && flags != IRQF_TRIGGER_LOW) {
+ pr_warn("WARNING: Invalid trigger for IRQ%d, assuming level low\n", irq);
+ pr_warn("WARNING: Please fix your firmware\n");
+ flags = IRQF_TRIGGER_LOW;
+ }
+
+ return flags;
+}
+
static int arch_timer_starting_cpu(unsigned int cpu)
{
struct clock_event_device *clk = this_cpu_ptr(arch_timer_evt);
+ u32 flags;
__arch_timer_setup(ARCH_CP15_TIMER, clk);
- enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], 0);
+ flags = check_ppi_trigger(arch_timer_ppi[arch_timer_uses_ppi]);
+ enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], flags);
- if (arch_timer_has_nonsecure_ppi())
- enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
+ if (arch_timer_has_nonsecure_ppi()) {
+ flags = check_ppi_trigger(arch_timer_ppi[PHYS_NONSECURE_PPI]);
+ enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], flags);
+ }
arch_counter_set_user_access();
if (evtstrm_enable)
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Marc Zyngier <tipbot@zytor.com> |
|---|---|
| Date | 2016-08-01 15:30 +0200 |
| Subject | [tip:timers/urgent] clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered |
| Message-ID | <s1ls5-6Sc-29@gated-at.bofh.it> |
| In reply to | #1453091 |
Commit-ID: 1604dd8aa5827c71b1fdc302c53b678dfa20c6fc
Gitweb: http://git.kernel.org/tip/1604dd8aa5827c71b1fdc302c53b678dfa20c6fc
Author: Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Mon, 1 Aug 2016 10:54:15 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 1 Aug 2016 15:17:45 +0200
clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered
The ARM architected timer produces level-triggered interrupts (this
is mandated by the architecture). Unfortunately, a number of
device-trees get this wrong, and expose an edge-triggered interrupt.
Until now, this wasn't too much an issue, as the programming of the
trigger would fail (the corresponding PPI cannot be reconfigured),
and the kernel would be happy with this. But we're about to change
this, and trust DT a lot if the driver doesn't provide its own
trigger information. In that context, the timer breaks badly.
While we do need to fix the DTs, there is also some userspace out
there (kvmtool) that generates the same kind of broken DT on the
fly, and that will completely break with newer kernels.
As a safety measure, and to keep buggy software alive as well as
buying us some time to fix DTs all over the place, let's check
what trigger configuration has been given us by the firmware.
If this is not a level configuration, then we know that the
DT/ACPI configuration is bust, and we pick some defaults which
won't be worse than the existing setup.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Liu Gang <Gang.Liu@nxp.com>
Cc: Mark Rutland <marc.rutland@arm.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Wenbin Song <Wenbin.Song@freescale.com>
Cc: Mingkai Hu <Mingkai.Hu@freescale.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: arm@kernel.org
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Ray Jui <rjui@broadcom.com>
Cc: "Hou Zhiqiang" <B48286@freescale.com>
Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com>
Cc: linux-samsung-soc@vger.kernel.org
Cc: Yuan Yao <yao.yuan@nxp.com>
Cc: Jan Glauber <jglauber@cavium.com>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: linux-amlogic@lists.infradead.org
Cc: soren.brinkmann@xilinx.com
Cc: Rajesh Bhagat <rajesh.bhagat@freescale.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Duc Dang <dhdang@apm.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Carlo Caione <carlo@caione.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Link: http://lkml.kernel.org/r/1470045256-9032-2-git-send-email-marc.zyngier@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 28bce3f..5770054 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -8,6 +8,9 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+
+#define pr_fmt(fmt) "arm_arch_timer: " fmt
+
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/device.h>
@@ -370,16 +373,33 @@ static bool arch_timer_has_nonsecure_ppi(void)
arch_timer_ppi[PHYS_NONSECURE_PPI]);
}
+static u32 check_ppi_trigger(int irq)
+{
+ u32 flags = irq_get_trigger_type(irq);
+
+ if (flags != IRQF_TRIGGER_HIGH && flags != IRQF_TRIGGER_LOW) {
+ pr_warn("WARNING: Invalid trigger for IRQ%d, assuming level low\n", irq);
+ pr_warn("WARNING: Please fix your firmware\n");
+ flags = IRQF_TRIGGER_LOW;
+ }
+
+ return flags;
+}
+
static int arch_timer_starting_cpu(unsigned int cpu)
{
struct clock_event_device *clk = this_cpu_ptr(arch_timer_evt);
+ u32 flags;
__arch_timer_setup(ARCH_CP15_TIMER, clk);
- enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], 0);
+ flags = check_ppi_trigger(arch_timer_ppi[arch_timer_uses_ppi]);
+ enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], flags);
- if (arch_timer_has_nonsecure_ppi())
- enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
+ if (arch_timer_has_nonsecure_ppi()) {
+ flags = check_ppi_trigger(arch_timer_ppi[PHYS_NONSECURE_PPI]);
+ enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], flags);
+ }
arch_counter_set_user_access();
if (evtstrm_enable)
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Marc Zyngier <tipbot@zytor.com> |
|---|---|
| Date | 2016-08-01 16:30 +0200 |
| Subject | [tip:timers/urgent] clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered |
| Message-ID | <s1moa-7tr-23@gated-at.bofh.it> |
| In reply to | #1453091 |
Commit-ID: f005bd7e3b84a353475a2895e2c7686a66297d87
Gitweb: http://git.kernel.org/tip/f005bd7e3b84a353475a2895e2c7686a66297d87
Author: Marc Zyngier <marc.zyngier@arm.com>
AuthorDate: Mon, 1 Aug 2016 10:54:15 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 1 Aug 2016 16:15:53 +0200
clocksource/arm_arch_timer: Force per-CPU interrupt to be level-triggered
The ARM architected timer produces level-triggered interrupts (this
is mandated by the architecture). Unfortunately, a number of
device-trees get this wrong, and expose an edge-triggered interrupt.
Until now, this wasn't too much an issue, as the programming of the
trigger would fail (the corresponding PPI cannot be reconfigured),
and the kernel would be happy with this. But we're about to change
this, and trust DT a lot if the driver doesn't provide its own
trigger information. In that context, the timer breaks badly.
While we do need to fix the DTs, there is also some userspace out
there (kvmtool) that generates the same kind of broken DT on the
fly, and that will completely break with newer kernels.
As a safety measure, and to keep buggy software alive as well as
buying us some time to fix DTs all over the place, let's check
what trigger configuration has been given us by the firmware.
If this is not a level configuration, then we know that the
DT/ACPI configuration is bust, and we pick some defaults which
won't be worse than the existing setup.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Liu Gang <Gang.Liu@nxp.com>
Cc: Mark Rutland <marc.rutland@arm.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Wenbin Song <Wenbin.Song@freescale.com>
Cc: Mingkai Hu <Mingkai.Hu@freescale.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: arm@kernel.org
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Ray Jui <rjui@broadcom.com>
Cc: "Hou Zhiqiang" <B48286@freescale.com>
Cc: Tirumalesh Chalamarla <tchalamarla@cavium.com>
Cc: linux-samsung-soc@vger.kernel.org
Cc: Yuan Yao <yao.yuan@nxp.com>
Cc: Jan Glauber <jglauber@cavium.com>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: linux-amlogic@lists.infradead.org
Cc: soren.brinkmann@xilinx.com
Cc: Rajesh Bhagat <rajesh.bhagat@freescale.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Duc Dang <dhdang@apm.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Carlo Caione <carlo@caione.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Link: http://lkml.kernel.org/r/1470045256-9032-2-git-send-email-marc.zyngier@arm.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 28bce3f..5770054 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -8,6 +8,9 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+
+#define pr_fmt(fmt) "arm_arch_timer: " fmt
+
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/device.h>
@@ -370,16 +373,33 @@ static bool arch_timer_has_nonsecure_ppi(void)
arch_timer_ppi[PHYS_NONSECURE_PPI]);
}
+static u32 check_ppi_trigger(int irq)
+{
+ u32 flags = irq_get_trigger_type(irq);
+
+ if (flags != IRQF_TRIGGER_HIGH && flags != IRQF_TRIGGER_LOW) {
+ pr_warn("WARNING: Invalid trigger for IRQ%d, assuming level low\n", irq);
+ pr_warn("WARNING: Please fix your firmware\n");
+ flags = IRQF_TRIGGER_LOW;
+ }
+
+ return flags;
+}
+
static int arch_timer_starting_cpu(unsigned int cpu)
{
struct clock_event_device *clk = this_cpu_ptr(arch_timer_evt);
+ u32 flags;
__arch_timer_setup(ARCH_CP15_TIMER, clk);
- enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], 0);
+ flags = check_ppi_trigger(arch_timer_ppi[arch_timer_uses_ppi]);
+ enable_percpu_irq(arch_timer_ppi[arch_timer_uses_ppi], flags);
- if (arch_timer_has_nonsecure_ppi())
- enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], 0);
+ if (arch_timer_has_nonsecure_ppi()) {
+ flags = check_ppi_trigger(arch_timer_ppi[PHYS_NONSECURE_PPI]);
+ enable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI], flags);
+ }
arch_counter_set_user_access();
if (evtstrm_enable)
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web