[RFC PATCH] irqchip/gic, gic-v3: Ensure data visibility in peripheral
From: Leo Yan
Date: Wed Sep 01 2021 - 02:31:55 EST
When an interrupt line is assered, GIC handles interrupt with the flow
(with EOImode == 1):
gic_handle_irq()
`> do_read_iar() => Change int state to active
`> gic_write_eoir() => Drop int priority
`> handle_domain_irq()
`> generic_handle_irq_desc()
`> handle_fasteoi_irq()
`> handle_irq_event() => Peripheral handler and
de-assert int line
`> cond_unmask_eoi_irq()
`> chip->irq_eoi()
`> gic_eoimode1_eoi_irq() => Change int state to inactive
In this flow, it has no explicit memory barrier between the functions
handle_irq_event() and chip->irq_eoi(), it's possible that the
outstanding data has not reached device in handle_irq_event() but the
callback chip->irq_eoi() is invoked, this can lead to state transition
for level triggered interrupt:
Flow | Interrupt state in GIC
---------------------------------+-------------------------------------
Interrupt line is asserted | 'inactive' -> 'pending'
do_read_iar() | 'pending' -> 'pending & active'
handle_irq_event() | Write peripheral register but it's
| not visible for device, so the
| interrupt line is still asserted
chip->irq_eoi() | 'pending & active' -> 'pending'
...
Produce spurious interrupt |
with interrupt ID: 1024 |
| Finally the peripheral reigster is
| updated and the interrupt line is
| deasserted: 'pending' -> 'inactive'
To avoid this potential issue, this patch adds wmb() barrier prior to
invoke EOI operation, this can make sure the interrupt line is
de-asserted in peripheral before deactivating interrupt in GIC. At the
end, this can avoid spurious interrupt.
Reported-by: Orito Takao <orito.takao@xxxxxxxxxxxxx>
Signed-off-by: Leo Yan <leo.yan@xxxxxxxxxx>
---
drivers/irqchip/irq-gic-v3.c | 4 ++++
drivers/irqchip/irq-gic.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e0f4debe64e1..fcd1477e2274 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -530,6 +530,8 @@ static void gic_irq_nmi_teardown(struct irq_data *d)
static void gic_eoi_irq(struct irq_data *d)
{
+ /* Ensure the data is written to peripheral */
+ wmb();
gic_write_eoir(gic_irq(d));
}
@@ -541,6 +543,8 @@ static void gic_eoimode1_eoi_irq(struct irq_data *d)
*/
if (gic_irq(d) >= 8192 || irqd_is_forwarded_to_vcpu(d))
return;
+ /* Ensure the data is written to peripheral */
+ wmb();
gic_write_dir(gic_irq(d));
}
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index d329ec3d64d8..3e9d4b6dc31b 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -222,6 +222,8 @@ static void gic_eoi_irq(struct irq_data *d)
if (hwirq < 16)
hwirq = this_cpu_read(sgi_intid);
+ /* Ensure the data is written to peripheral */
+ wmb();
writel_relaxed(hwirq, gic_cpu_base(d) + GIC_CPU_EOI);
}
@@ -236,6 +238,8 @@ static void gic_eoimode1_eoi_irq(struct irq_data *d)
if (hwirq < 16)
hwirq = this_cpu_read(sgi_intid);
+ /* Ensure the data is written to peripheral */
+ wmb();
writel_relaxed(hwirq, gic_cpu_base(d) + GIC_CPU_DEACTIVATE);
}
--
2.25.1