[PATCH v1 01/17] xen/riscv: manage IRQ_DISABLED flag in APLIC irq enable/disable callbacks

Oleksii Kurochko <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <a6d85647d0d726ffb25708829813510421ed5234.1784560663.git.oleksii.kurochko@gmail.com>
desc->status is only set once during setup_irq(), but interrupts can be
enabled/disabled at runtime, so update it in the corresponding callbacks.

For the purposes of the FENCE instruction, CSR read accesses are
classified as device input (I) and CSR write accesses as device output
(O), while the barriers used by spin locks (fence rw,rw) only order
normal memory accesses. An explicit wmb() (fence ow,ow) is therefore
added in aplic_irq_{enable,disable}() to order the desc->status update
with respect to the IMSIC CSR write.

Fixes: d4676a1398bc5 ("xen/riscv: implementation of aplic and imsic operations")
Signed-off-by: Oleksii Kurochko <[email protected]>
---
Changes in v3:
 - Add the comment above wmb() in aplic_irq_enable() and add wmb() also in
   *_disable().
 - Update the commit message: drop info about wmb() and add information why
   it is safe to drop update of desc->status from setup_irq().
---
Changes in v2:
 - New patch
---
---
 xen/arch/riscv/aplic.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/xen/arch/riscv/aplic.c b/xen/arch/riscv/aplic.c
index fe319041cece..3681f0669efb 100644
--- a/xen/arch/riscv/aplic.c
+++ b/xen/arch/riscv/aplic.c
@@ -136,6 +136,15 @@ static void cf_check aplic_irq_enable(struct irq_desc *desc)
 
     spin_lock(&aplic.lock);
 
+    desc->status &= ~IRQ_DISABLED;
+    /*
+     * wmb() (fence ow,ow) orders the ->status memory write (w) before the
+     * CSR write inside imsic_irq_enable() (device output, o on RISC-V).
+     * arch_lock_release_barrier() uses fence rw,rw which does not cover
+     * device output (o), so wmb() is required to close that gap.
+     */
+    wmb();
+
     /* Enable interrupt in IMSIC */
     imsic_irq_enable(desc->irq);
 
@@ -163,6 +172,16 @@ static void cf_check aplic_irq_disable(struct irq_desc *desc)
 
     /* Disable interrupt in IMSIC */
     imsic_irq_disable(desc->irq);
+    /*
+     * wmb() (fence ow,ow) ensures the CSR write (device output, o) inside
+     * imsic_irq_disable() is globally visible before ->status is marked
+     * IRQ_DISABLED. imsic_irq_disable()'s spin_unlock uses fence rw,rw
+     * which does not order device output (o) writes before subsequent
+     * memory writes (w), so an explicit wmb() is needed here.
+     */
+    wmb();
+
+    desc->status |= IRQ_DISABLED;
 
     spin_unlock(&aplic.lock);
 }
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.