[openwrt/openwrt] realtek: avoid wrong interrupt routing
LEDE Commits
lede-commits at lists.infradead.org
Sun Sep 18 11:39:35 PDT 2022
svanheule pushed a commit to openwrt/openwrt.git, branch master:
https://git.openwrt.org/bcb5d6b21ba574db1d02385b494eb01fe46bf8aa
commit bcb5d6b21ba574db1d02385b494eb01fe46bf8aa
Author: Markus Stockhausen <markus.stockhausen at gmx.de>
AuthorDate: Wed Sep 7 17:02:35 2022 +0200
realtek: avoid wrong interrupt routing
The interrupt controller depends on two control registers. GIMR enables
or disables interrupts and IRRx routes these to MIPS CPU interrupts 2-7.
Wiki currently states "A value of '0' (in IRRx) disconnects this input from
the output line, independent of the line's setting in GIMR."
Contrary to normal intuition this statement DOES NOT mean, that interrupts
can be disabled by IRRx alone. The sad truth was discovered by enabling
SMP for an Zyxel XGS1010 on the 930x target. It shows that driver and
interrupts behave as follows:
- Timer 0 interrupt 7 has active routing to CPU0 and no routing to CPU1
- Timer 1 interrupt 8 has no routing to CPU0 and active routing to CPU1
- Unmasking (enabling) interrupts writes 1 bits to all GIMR registers
- Masking (disabling) interrupts writes 0 bits to both GIMR registers
During operation we can encounter a situation like
- GIMR bit for a interrupt/CPU combination is set to enabed (=1)
- IRRx routing bits for a interrupt/CPU combination are set to disabed (=0)
This setting already allows the hardware to fire interrupts to the target
CPU/VPE if the other CPU/VPE is currently busy. Especially for CPU bound
timer interrupts this is lethal. If timer interrupt 7 arrives at CPU1 and
vice versa for interrupt 8 the restart trigger gets lost. The timer dies
and a msleep() operation in the kernel will halt endlessly.
Fix this by tracking the IRRx active routing setting in a new bitfield with
0="routing active" and 1="no routing". Enable interrupts in GIMR only
for a interrupt & CPU if routing is active. Thus we have
- GIMR = 0 / IRRx = 0 -> everything disabled
- GIMR = 1 / IRRx > 0 -> active and normal routing
- GIMR = 0 / IRRx > 0 -> masked (disabled) with normal routing
- GIMR = 1 / IRRx = 0 -> no longer possible
Signed-off-by: Markus Stockhausen <markus.stockhausen at gmx.de>
---
...-irqchip-irq-realtek-rtl-fix-VPE-affinity.patch | 145 +++++++++++++++++++++
1 file changed, 145 insertions(+)
diff --git a/target/linux/realtek/patches-5.10/319-irqchip-irq-realtek-rtl-fix-VPE-affinity.patch b/target/linux/realtek/patches-5.10/319-irqchip-irq-realtek-rtl-fix-VPE-affinity.patch
new file mode 100644
index 0000000000..0f4ab38d4d
--- /dev/null
+++ b/target/linux/realtek/patches-5.10/319-irqchip-irq-realtek-rtl-fix-VPE-affinity.patch
@@ -0,0 +1,145 @@
+--- a/drivers/irqchip/irq-realtek-rtl.c
++++ b/drivers/irqchip/irq-realtek-rtl.c
+@@ -28,6 +28,7 @@ static DEFINE_RAW_SPINLOCK(irq_lock);
+
+ #define REG(offset, cpu) (realtek_ictl_base[cpu] + offset)
+
++static u32 realtek_ictl_unmask[NR_CPUS];
+ static void __iomem *realtek_ictl_base[NR_CPUS];
+ static cpumask_t realtek_ictl_cpu_configurable;
+
+@@ -41,11 +42,29 @@ struct realtek_ictl_output {
+ };
+
+ /*
+- * IRR0-IRR3 store 4 bits per interrupt, but Realtek uses inverted numbering,
+- * placing IRQ 31 in the first four bits. A routing value of '0' means the
+- * interrupt is left disconnected. Routing values {1..15} connect to output
+- * lines {0..14}.
++ * Per CPU we have a set of 5 registers that determine interrupt handling for
++ * 32 external interrupts. GIMR (enable/disable interrupt) plus IRR0-IRR3 that
++ * contain "routing" or "priority" values. GIMR uses one bit for each interrupt
++ * and IRRx store 4 bits per interrupt. Realtek uses inverted numbering,
++ * placing IRQ 31 in the first four bits. The register combinations give the
++ * following results for a single interrupt in the wild:
++ *
++ * a) GIMR = 0 / IRRx > 0 -> no interrupts
++ * b) GIMR = 0 / IRRx = 0 -> no interrupts
++ * c) GIMR = 1 / IRRx > 0 -> interrupts
++ * d) GIMR = 1 / IRRx = 0 -> rare interrupts in SMP environment
++ *
++ * Combination d) seems to trigger interrupts only on a VPE if the other VPE
++ * has GIMR = 0 and IRRx > 0. E.g. busy without interrupts allowed. To provide
++ * IRQ balancing features in SMP this driver will handle the registers as
++ * follows:
++ *
++ * 1) set IRRx > 0 for VPE where the interrupt is desired
++ * 2) set IRRx = 0 for VPE where the interrupt is not desired
++ * 3) set both GIMR = 0 to mask (disabled) interrupt
++ * 4) set GIMR = 1 to unmask (enable) interrupt but only for VPE where IRRx > 0
+ */
++
+ #define IRR_OFFSET(idx) (4 * (3 - (idx * 4) / 32))
+ #define IRR_SHIFT(idx) ((idx * 4) % 32)
+
+@@ -65,19 +84,33 @@ static inline void write_irr(void __iomem *irr0, int idx, u32 value)
+ writel(irr, irr0 + offset);
+ }
+
++static inline void enable_gimr(int hwirq, int cpu)
++{
++ u32 value;
++
++ value = readl(REG(RTL_ICTL_GIMR, cpu));
++ value |= (BIT(hwirq) & realtek_ictl_unmask[cpu]);
++ writel(value, REG(RTL_ICTL_GIMR, cpu));
++}
++
++static inline void disable_gimr(int hwirq, int cpu)
++{
++ u32 value;
++
++ value = readl(REG(RTL_ICTL_GIMR, cpu));
++ value &= ~BIT(hwirq);
++ writel(value, REG(RTL_ICTL_GIMR, cpu));
++}
++
+ static void realtek_ictl_unmask_irq(struct irq_data *i)
+ {
+ unsigned long flags;
+- u32 value;
+ int cpu;
+
+ raw_spin_lock_irqsave(&irq_lock, flags);
+
+- for_each_cpu(cpu, &realtek_ictl_cpu_configurable) {
+- value = readl(REG(RTL_ICTL_GIMR, cpu));
+- value |= BIT(i->hwirq);
+- writel(value, REG(RTL_ICTL_GIMR, cpu));
+- }
++ for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
++ enable_gimr(i->hwirq, cpu);
+
+ raw_spin_unlock_irqrestore(&irq_lock, flags);
+ }
+@@ -85,16 +118,12 @@ static void realtek_ictl_unmask_irq(struct irq_data *i)
+ static void realtek_ictl_mask_irq(struct irq_data *i)
+ {
+ unsigned long flags;
+- u32 value;
+ int cpu;
+
+ raw_spin_lock_irqsave(&irq_lock, flags);
+
+- for_each_cpu(cpu, &realtek_ictl_cpu_configurable) {
+- value = readl(REG(RTL_ICTL_GIMR, cpu));
+- value &= ~BIT(i->hwirq);
+- writel(value, REG(RTL_ICTL_GIMR, cpu));
+- }
++ for_each_cpu(cpu, &realtek_ictl_cpu_configurable)
++ disable_gimr(i->hwirq, cpu);
+
+ raw_spin_unlock_irqrestore(&irq_lock, flags);
+ }
+@@ -116,11 +145,17 @@ static int __maybe_unused realtek_ictl_irq_affinity(struct irq_data *i,
+ cpumask_and(&cpu_enable, &cpu_configure, dest);
+ cpumask_andnot(&cpu_disable, &cpu_configure, dest);
+
+- for_each_cpu(cpu, &cpu_disable)
++ for_each_cpu(cpu, &cpu_disable) {
+ write_irr(REG(RTL_ICTL_IRR0, cpu), i->hwirq, 0);
++ realtek_ictl_unmask[cpu] &= ~BIT(i->hwirq);
++ disable_gimr(i->hwirq, cpu);
++ }
+
+- for_each_cpu(cpu, &cpu_enable)
++ for_each_cpu(cpu, &cpu_enable) {
+ write_irr(REG(RTL_ICTL_IRR0, cpu), i->hwirq, output->output_index + 1);
++ realtek_ictl_unmask[cpu] |= BIT(i->hwirq);
++ enable_gimr(i->hwirq, cpu);
++ }
+
+ irq_data_update_effective_affinity(i, &cpu_enable);
+
+@@ -149,6 +184,7 @@ static int intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
+
+ output->child_mask |= BIT(hw);
+ write_irr(REG(RTL_ICTL_IRR0, 0), hw, output->output_index + 1);
++ realtek_ictl_unmask[0] |= BIT(hw);
+
+ raw_spin_unlock_irqrestore(&irq_lock, flags);
+
+@@ -279,9 +315,11 @@ static int __init realtek_rtl_of_init(struct device_node *node, struct device_no
+ cpumask_set_cpu(cpu, &realtek_ictl_cpu_configurable);
+
+ /* Disable all cascaded interrupts and clear routing */
+- writel(0, REG(RTL_ICTL_GIMR, cpu));
+- for (soc_irq = 0; soc_irq < RTL_ICTL_NUM_INPUTS; soc_irq++)
++ for (soc_irq = 0; soc_irq < RTL_ICTL_NUM_INPUTS; soc_irq++) {
+ write_irr(REG(RTL_ICTL_IRR0, cpu), soc_irq, 0);
++ realtek_ictl_unmask[cpu] &= ~BIT(soc_irq);
++ disable_gimr(soc_irq, cpu);
++ }
+ }
+ }
+
More information about the lede-commits
mailing list