irqchip/mips: Fix RCU violation when using irqdomain lookup on interrupt entry

Since d4a45c68dc81 ("irqdomain: Protect the linear revmap with RCU"), any irqdomain lookup requires the RCU read lock to be held.

This assumes that the architecture code will be structured such as irq_enter() will be called *before* the interrupt is looked up in the irq domain. However, this isn't the case for MIPS, and a number of drivers are structured to do it the other way around when handling an interrupt in their root irqchip (secondary irqchips are OK by construction).

This results in a RCU splat on a lockdep-enabled kernel when the kernel takes an interrupt from idle, as reported by Guenter Roeck.

Note that this could have fired previously if any driver had used tree-based irqdomain, which always had the RCU requirement.

To solve this, provide a MIPS-specific helper (do_domain_IRQ()) as the pendent of do_IRQ() that will do thing in the right order (and maybe save some cycles in the process).

Ideally, MIPS would be moved over to using handle_domain_irq(), but that's much more ambitious.

Reported-by Guenter Roeck <[email protected]>

Tested-by Guenter Roeck <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

Cc Thomas Bogendoerfer <[email protected]>

Cc Serge Semin <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

---
 arch/mips/include/asm/irq.h      |  3 +++
 arch/mips/kernel/irq.c           | 14 ++++++++++++++
 drivers/irqchip/irq-mips-cpu.c   | 10 ++++++----
 drivers/irqchip/irq-mips-gic.c   |  8 ++++----
 drivers/irqchip/irq-pic32-evic.c |  5 ++---
 5 files changed, 29 insertions(+), 11 deletions(-)

arch/mips/include/asm/irq.h +3 -0

index d1477ecb1af9..57561e0e6e8d 100644
@@ -57,6 +57,9 @@ asmlinkage void plat_irq_dispatch(void);
 
 extern void do_IRQ(unsigned int irq);
 
+struct irq_domain;
+extern void do_domain_IRQ(struct irq_domain *domain, unsigned int irq);
+
 extern void arch_init_irq(void);
 extern void spurious_interrupt(void);
 

arch/mips/kernel/irq.c +14 -0

index 85b6c60f285d..c76005cd3b79 100644
@@ -21,6 +21,7 @@
 #include <linux/kallsyms.h>
 #include <linux/kgdb.h>
 #include <linux/ftrace.h>
+#include <linux/irqdomain.h>
 
 #include <linux/atomic.h>
 #include <linux/uaccess.h>

@@ -107,3 +108,16 @@ void __irq_entry do_IRQ(unsigned int irq)
 	irq_exit();
 }
 
+void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq)
+{
+	struct irq_desc *desc;
+
+	irq_enter();
+	check_stack_overflow();
+
+	desc = irq_resolve_mapping(domain, hwirq);
+	if (likely(desc))
+		handle_irq_desc(desc);
+
+	irq_exit();
+}

drivers/irqchip/irq-mips-cpu.c +6 -4

index 0bbb0b2d0dd5..0c7ae71a0af0 100644
@@ -127,7 +127,6 @@ static struct irq_chip mips_mt_cpu_irq_controller = {
 asmlinkage void __weak plat_irq_dispatch(void)
 {
 	unsigned long pending = read_c0_cause() & read_c0_status() & ST0_IM;
-	unsigned int virq;
 	int irq;
 
 	if (!pending) {

@@ -137,12 +136,15 @@ asmlinkage void __weak plat_irq_dispatch(void)
 
 	pending >>= CAUSEB_IP;
 	while (pending) {
+		struct irq_domain *d;
+
 		irq = fls(pending) - 1;
 		if (IS_ENABLED(CONFIG_GENERIC_IRQ_IPI) && irq < 2)
-			virq = irq_linear_revmap(ipi_domain, irq);
+			d = ipi_domain;
 		else
-			virq = irq_linear_revmap(irq_domain, irq);
-		do_IRQ(virq);
+			d = irq_domain;
+
+		do_domain_IRQ(d, irq);
 		pending &= ~BIT(irq);
 	}
 }

drivers/irqchip/irq-mips-gic.c +4 -4

index b146e069bf5b..54c7092cc61d 100644
@@ -169,8 +169,8 @@ static void gic_handle_shared_int(bool chained)
 			generic_handle_domain_irq(gic_irq_domain,
 						  GIC_SHARED_TO_HWIRQ(intr));
 		else
-			do_IRQ(irq_find_mapping(gic_irq_domain,
-						GIC_SHARED_TO_HWIRQ(intr)));
+			do_domain_IRQ(gic_irq_domain,
+				      GIC_SHARED_TO_HWIRQ(intr));
 	}
 }
 

@@ -320,8 +320,8 @@ static void gic_handle_local_int(bool chained)
 			generic_handle_domain_irq(gic_irq_domain,
 						  GIC_LOCAL_TO_HWIRQ(intr));
 		else
-			do_IRQ(irq_find_mapping(gic_irq_domain,
-						GIC_LOCAL_TO_HWIRQ(intr)));
+			do_domain_IRQ(gic_irq_domain,
+				      GIC_LOCAL_TO_HWIRQ(intr));
 	}
 }
 

drivers/irqchip/irq-pic32-evic.c +2 -3

index 34c4b4ffacd1..1d9bb28d13e5 100644
@@ -42,11 +42,10 @@ static void __iomem *evic_base;
 
 asmlinkage void __weak plat_irq_dispatch(void)
 {
-	unsigned int irq, hwirq;
+	unsigned int hwirq;
 
 	hwirq = readl(evic_base + REG_INTSTAT) & 0xFF;
-	irq = irq_linear_revmap(evic_irq_domain, hwirq);
-	do_IRQ(irq);
+	do_domain_IRQ(evic_irq_domain, hwirq);
 }
 
 static struct evic_chip_data *irqd_to_priv(struct irq_data *data)

Signature · 1 line

2.30.2

Hi Marc,
Thanks for the fix.

Quoted 10 lines · Marc Zyngier

Since d4a45c68dc81 ("irqdomain: Protect the linear revmap with RCU"), any irqdomain lookup requires the RCU read lock to be held.

This assumes that the architecture code will be structured such as irq_enter() will be called *before* the interrupt is looked up in the irq domain. However, this isn't the case for MIPS, and a number of drivers are structured to do it the other way around when handling an interrupt in their root irqchip (secondary irqchips are OK by construction).

Quoted 2 lines

This results in a RCU splat on a lockdep-enabled kernel when the kernel takes an interrupt from idle, as reported by Guenter Roeck.

Alas I am still on 5.12-rc4, so can't test it out at the moment. Soon after getting further on the modern kernel version I'll give this patch a try on my hw and send a report.

Hi Sergey,

Quoted 18 lines · Serge Semin

Hi Marc,
Thanks for the fix.

Quoted 12 lines · Marc Zyngier

Since d4a45c68dc81 ("irqdomain: Protect the linear revmap with RCU"), any irqdomain lookup requires the RCU read lock to be held.

This assumes that the architecture code will be structured such as irq_enter() will be called *before* the interrupt is looked up in the irq domain. However, this isn't the case for MIPS, and a number of drivers are structured to do it the other way around when handling an interrupt in their root irqchip (secondary irqchips are OK by construction).

This results in a RCU splat on a lockdep-enabled kernel when the kernel takes an interrupt from idle, as reported by Guenter Roeck.

Quoted 3 lines

Alas I am still on 5.12-rc4, so can't test it out at the moment. Soon after getting further on the modern kernel version I'll give this patch a try on my hw and send a report.

It is likely that I'll send a pull request to Thomas with this shortly, given that it affects existing systems and that this patch does address the issue (see Guenter's report). We can always amend things once you've had the time to upgrade your kernel to the latest.

Thanks,

	M.

Signature · 1 line

Without deviation from the norm, progress is not possible.