From 4efc851c36e389f7ed432edac0149acc5f94b0c7 Mon Sep 17 00:00:00 2001 From: Daniel Thompson Date: Tue, 3 May 2022 14:45:41 +0100 Subject: irqchip/exiu: Fix acknowledgment of edge triggered interrupts Currently the EXIU uses the fasteoi interrupt flow that is configured by it's parent (irq-gic-v3.c). With this flow the only chance to clear the interrupt request happens during .irq_eoi() and (obviously) this happens after the interrupt handler has run. EXIU requires edge triggered interrupts to be acked prior to interrupt handling. Without this we risk incorrect interrupt dismissal when a new interrupt is delivered after the handler reads and acknowledges the peripheral but before the irq_eoi() takes place. Fix this by clearing the interrupt request from .irq_ack() if we are configured for edge triggered interrupts. This requires adopting the fasteoi-ack flow instead of the fasteoi to ensure the ack gets called. These changes have been tested using the power button on a Developerbox/SC2A11 combined with some hackery in gpio-keys so I can play with the different trigger mode [and an mdelay(500) so I can can check what happens on a double click in both modes]. Fixes: 706cffc1b912 ("irqchip/exiu: Add support for Socionext Synquacer EXIU controller") Signed-off-by: Daniel Thompson Reviewed-by: Ard Biesheuvel Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20220503134541.2566457-1-daniel.thompson@linaro.org --- arch/arm64/Kconfig.platforms | 1 + 1 file changed, 1 insertion(+) (limited to 'arch') diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index 30b123cde02c..aaeaf57c8222 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -253,6 +253,7 @@ config ARCH_INTEL_SOCFPGA config ARCH_SYNQUACER bool "Socionext SynQuacer SoC Family" + select IRQ_FASTEOI_HIERARCHY_HANDLERS config ARCH_TEGRA bool "NVIDIA Tegra SoC Family" -- cgit v1.2.3 From 6efb50923771f392122f5ce69dfc43b08f16e449 Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Fri, 13 May 2022 14:30:37 +0100 Subject: irqchip/gic-v3: Refactor ISB + EOIR at ack time There are cases where a context synchronization event is necessary between an IRQ being raised and being handled, and there are races such that we cannot rely upon the exception entry being subsequent to the interrupt being raised. To fix this, we place an ISB between a read of IAR and the subsequent invocation of an IRQ handler. When EOI mode 1 is in use, we need to EOI an interrupt prior to invoking its handler, and we have a write to EOIR for this. As this write to EOIR requires an ISB, and this is provided by the gic_write_eoir() helper, we omit the usual ISB in this case, with the logic being: | if (static_branch_likely(&supports_deactivate_key)) | gic_write_eoir(irqnr); | else | isb(); This is somewhat opaque, and it would be a little clearer if there were an unconditional ISB, with only the write to EOIR being conditional, e.g. | if (static_branch_likely(&supports_deactivate_key)) | write_gicreg(irqnr, ICC_EOIR1_EL1); | | isb(); This patch rewrites the code that way, with this logic factored into a new helper function with comments explaining what the ISB is for, as were originally laid out in commit: 39a06b67c2c1256b ("irqchip/gic: Ensure we have an ISB between ack and ->handle_irq") Note that since then, we removed the IAR polling in commit: 342677d70ab92142 ("irqchip/gic-v3: Remove acknowledge loop") ... which removed one of the two race conditions. For consistency, other portions of the driver are made to manipulate EOIR using write_gicreg() and explcit ISBs, and the gic_write_eoir() helper function is removed. There should be no functional change as a result of this patch. Signed-off-by: Mark Rutland Cc: Marc Zyngier Cc: Thomas Gleixner Cc: Will Deacon Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20220513133038.226182-3-mark.rutland@arm.com --- arch/arm/include/asm/arch_gicv3.h | 7 +----- arch/arm64/include/asm/arch_gicv3.h | 6 ------ drivers/irqchip/irq-gic-v3.c | 43 ++++++++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 22 deletions(-) (limited to 'arch') diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h index 413abfb42989..f82a819eb0db 100644 --- a/arch/arm/include/asm/arch_gicv3.h +++ b/arch/arm/include/asm/arch_gicv3.h @@ -48,6 +48,7 @@ static inline u32 read_ ## a64(void) \ return read_sysreg(a32); \ } \ +CPUIF_MAP(ICC_EOIR1, ICC_EOIR1_EL1) CPUIF_MAP(ICC_PMR, ICC_PMR_EL1) CPUIF_MAP(ICC_AP0R0, ICC_AP0R0_EL1) CPUIF_MAP(ICC_AP0R1, ICC_AP0R1_EL1) @@ -63,12 +64,6 @@ CPUIF_MAP(ICC_AP1R3, ICC_AP1R3_EL1) /* Low-level accessors */ -static inline void gic_write_eoir(u32 irq) -{ - write_sysreg(irq, ICC_EOIR1); - isb(); -} - static inline void gic_write_dir(u32 val) { write_sysreg(val, ICC_DIR); diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h index 8bd5afc7b692..48d4473e8eee 100644 --- a/arch/arm64/include/asm/arch_gicv3.h +++ b/arch/arm64/include/asm/arch_gicv3.h @@ -26,12 +26,6 @@ * sets the GP register's most significant bits to 0 with an explicit cast. */ -static inline void gic_write_eoir(u32 irq) -{ - write_sysreg_s(irq, SYS_ICC_EOIR1_EL1); - isb(); -} - static __always_inline void gic_write_dir(u32 irq) { write_sysreg_s(irq, SYS_ICC_DIR_EL1); diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 7305d84f2df5..0cbc4e25c48d 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -556,7 +556,8 @@ static void gic_irq_nmi_teardown(struct irq_data *d) static void gic_eoi_irq(struct irq_data *d) { - gic_write_eoir(gic_irq(d)); + write_gicreg(gic_irq(d), ICC_EOIR1_EL1); + isb(); } static void gic_eoimode1_eoi_irq(struct irq_data *d) @@ -640,10 +641,38 @@ static void gic_deactivate_unhandled(u32 irqnr) if (irqnr < 8192) gic_write_dir(irqnr); } else { - gic_write_eoir(irqnr); + write_gicreg(irqnr, ICC_EOIR1_EL1); + isb(); } } +/* + * Follow a read of the IAR with any HW maintenance that needs to happen prior + * to invoking the relevant IRQ handler. We must do two things: + * + * (1) Ensure instruction ordering between a read of IAR and subsequent + * instructions in the IRQ handler using an ISB. + * + * It is possible for the IAR to report an IRQ which was signalled *after* + * the CPU took an IRQ exception as multiple interrupts can race to be + * recognized by the GIC, earlier interrupts could be withdrawn, and/or + * later interrupts could be prioritized by the GIC. + * + * For devices which are tightly coupled to the CPU, such as PMUs, a + * context synchronization event is necessary to ensure that system + * register state is not stale, as these may have been indirectly written + * *after* exception entry. + * + * (2) Deactivate the interrupt when EOI mode 1 is in use. + */ +static inline void gic_complete_ack(u32 irqnr) +{ + if (static_branch_likely(&supports_deactivate_key)) + write_gicreg(irqnr, ICC_EOIR1_EL1); + + isb(); +} + static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs) { bool irqs_enabled = interrupts_enabled(regs); @@ -652,10 +681,7 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs) if (irqs_enabled) nmi_enter(); - if (static_branch_likely(&supports_deactivate_key)) - gic_write_eoir(irqnr); - else - isb() + gic_complete_ack(irqnr); /* * Leave the PSR.I bit set to prevent other NMIs to be @@ -726,10 +752,7 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs gic_arch_enable_irqs(); } - if (static_branch_likely(&supports_deactivate_key)) - gic_write_eoir(irqnr); - else - isb(); + gic_complete_ack(irqnr); if (generic_handle_domain_irq(gic_data.domain, irqnr)) { WARN_ONCE(true, "Unexpected interrupt received!\n"); -- cgit v1.2.3 From d421fd6d1fbf00b6481d836e65bad07d6bad61ed Mon Sep 17 00:00:00 2001 From: Samuel Holland Date: Sun, 8 May 2022 22:49:41 -0500 Subject: irqchip: Add Kconfig symbols for sunxi drivers Not all of these drivers are needed on every ARCH_SUNXI platform. In particular, the ARCH_SUNXI symbol will be reused for the Allwinner D1, a RISC-V SoC which contains none of these irqchips. Introduce Kconfig symbols so we can select only the drivers actually used by a particular set of platforms. This also lets us move the irqchip driver dependencies to a more appropriate location. Signed-off-by: Samuel Holland Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20220509034941.30704-1-samuel@sholland.org --- arch/arm/mach-sunxi/Kconfig | 12 +++++++++--- arch/arm64/Kconfig.platforms | 5 ++--- drivers/irqchip/Kconfig | 12 ++++++++++++ drivers/irqchip/Makefile | 6 +++--- 4 files changed, 26 insertions(+), 9 deletions(-) (limited to 'arch') diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index e5c2fce281cd..abdb99fe1e97 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -4,10 +4,7 @@ menuconfig ARCH_SUNXI depends on ARCH_MULTI_V5 || ARCH_MULTI_V7 select ARCH_HAS_RESET_CONTROLLER select CLKSRC_MMIO - select GENERIC_IRQ_CHIP select GPIOLIB - select IRQ_DOMAIN_HIERARCHY - select IRQ_FASTEOI_HIERARCHY_HANDLERS select PINCTRL select PM_OPP select SUN4I_TIMER @@ -22,10 +19,12 @@ if ARCH_MULTI_V7 config MACH_SUN4I bool "Allwinner A10 (sun4i) SoCs support" default ARCH_SUNXI + select SUN4I_INTC config MACH_SUN5I bool "Allwinner A10s / A13 (sun5i) SoCs support" default ARCH_SUNXI + select SUN4I_INTC select SUN5I_HSTIMER config MACH_SUN6I @@ -34,6 +33,8 @@ config MACH_SUN6I select ARM_GIC select MFD_SUN6I_PRCM select SUN5I_HSTIMER + select SUN6I_R_INTC + select SUNXI_NMI_INTC config MACH_SUN7I bool "Allwinner A20 (sun7i) SoCs support" @@ -43,17 +44,21 @@ config MACH_SUN7I select ARCH_SUPPORTS_BIG_ENDIAN select HAVE_ARM_ARCH_TIMER select SUN5I_HSTIMER + select SUNXI_NMI_INTC config MACH_SUN8I bool "Allwinner sun8i Family SoCs support" default ARCH_SUNXI select ARM_GIC select MFD_SUN6I_PRCM + select SUN6I_R_INTC + select SUNXI_NMI_INTC config MACH_SUN9I bool "Allwinner (sun9i) SoCs support" default ARCH_SUNXI select ARM_GIC + select SUNXI_NMI_INTC config ARCH_SUNXI_MC_SMP bool @@ -69,6 +74,7 @@ if ARCH_MULTI_V5 config MACH_SUNIV bool "Allwinner ARMv5 F-series (suniv) SoCs support" default ARCH_SUNXI + select SUN4I_INTC help Support for Allwinner suniv ARMv5 SoCs. (F1C100A, F1C100s, F1C200s, F1C500, F1C600) diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms index aaeaf57c8222..6a6457fed7b2 100644 --- a/arch/arm64/Kconfig.platforms +++ b/arch/arm64/Kconfig.platforms @@ -11,12 +11,11 @@ config ARCH_ACTIONS config ARCH_SUNXI bool "Allwinner sunxi 64-bit SoC Family" select ARCH_HAS_RESET_CONTROLLER - select GENERIC_IRQ_CHIP - select IRQ_DOMAIN_HIERARCHY - select IRQ_FASTEOI_HIERARCHY_HANDLERS select PINCTRL select RESET_CONTROLLER select SUN4I_TIMER + select SUN6I_R_INTC + select SUNXI_NMI_INTC help This enables support for Allwinner sunxi based SoCs like the A64. diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 15edb9a6fcae..135c156673a7 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -257,6 +257,18 @@ config ST_IRQCHIP help Enables SysCfg Controlled IRQs on STi based platforms. +config SUN4I_INTC + bool + +config SUN6I_R_INTC + bool + select IRQ_DOMAIN_HIERARCHY + select IRQ_FASTEOI_HIERARCHY_HANDLERS + +config SUNXI_NMI_INTC + bool + select GENERIC_IRQ_CHIP + config TB10X_IRQC bool select IRQ_DOMAIN diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index 160a1d8ceaa9..9b1ffb0f98cc 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -23,9 +23,9 @@ obj-$(CONFIG_OMPIC) += irq-ompic.o obj-$(CONFIG_OR1K_PIC) += irq-or1k-pic.o obj-$(CONFIG_ORION_IRQCHIP) += irq-orion.o obj-$(CONFIG_OMAP_IRQCHIP) += irq-omap-intc.o -obj-$(CONFIG_ARCH_SUNXI) += irq-sun4i.o -obj-$(CONFIG_ARCH_SUNXI) += irq-sun6i-r.o -obj-$(CONFIG_ARCH_SUNXI) += irq-sunxi-nmi.o +obj-$(CONFIG_SUN4I_INTC) += irq-sun4i.o +obj-$(CONFIG_SUN6I_R_INTC) += irq-sun6i-r.o +obj-$(CONFIG_SUNXI_NMI_INTC) += irq-sunxi-nmi.o obj-$(CONFIG_ARCH_SPEAR3XX) += spear-shirq.o obj-$(CONFIG_ARM_GIC) += irq-gic.o irq-gic-common.o obj-$(CONFIG_ARM_GIC_PM) += irq-gic-pm.o -- cgit v1.2.3