[Xen-devel] [PATCH for-4.5 6/8] xen/arm: IRQ: Add lock contrainst for gic_irq_{startup, shutdown}

[Julien Grall <julien.grall@xxxxxxxxxx>]

When multiple action will be supported, gic_irq_{startup,shutdown} will have
to be called in the same critical zone has setup/release.
Otherwise it could have a race condition if at the same time CPU A is calling
release_dt_irq and CPU B is calling setup_dt_irq.

This could end up to the IRQ is not enabled.

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
---
 xen/arch/arm/gic.c |   40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 2643b46..ebd2b5f 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -129,43 +129,53 @@ void gic_restore_state(struct vcpu *v)
     gic_restore_pending_irqs(v);
 }
 
-static void gic_irq_enable(struct irq_desc *desc)
+static unsigned int gic_irq_startup(struct irq_desc *desc)
 {
     int irq = desc->irq;
-    unsigned long flags;
 
-    spin_lock_irqsave(&desc->lock, flags);
+    ASSERT(spin_is_locked(&desc->lock));
+    ASSERT(!local_irq_is_enabled());
+
     spin_lock(&gic.lock);
     desc->status &= ~IRQ_DISABLED;
     dsb();
     /* Enable routing */
     GICD[GICD_ISENABLER + irq / 32] = (1u << (irq % 32));
     spin_unlock(&gic.lock);
-    spin_unlock_irqrestore(&desc->lock, flags);
+
+    return 0;
 }
 
-static void gic_irq_disable(struct irq_desc *desc)
+static void gic_irq_shutdown(struct irq_desc *desc)
 {
     int irq = desc->irq;
 
-    spin_lock(&desc->lock);
+    ASSERT(spin_is_locked(&desc->lock));
+    ASSERT(!local_irq_is_enabled());
+
     spin_lock(&gic.lock);
     /* Disable routing */
     GICD[GICD_ICENABLER + irq / 32] = (1u << (irq % 32));
     desc->status |= IRQ_DISABLED;
     spin_unlock(&gic.lock);
-    spin_unlock(&desc->lock);
 }
 
-static unsigned int gic_irq_startup(struct irq_desc *desc)
+static void gic_irq_enable(struct irq_desc *desc)
 {
-    gic_irq_enable(desc);
-    return 0;
+    unsigned long flags;
+
+    spin_lock_irqsave(&desc->lock, flags);
+    gic_irq_startup(desc);
+    spin_unlock_irqrestore(&desc->lock, flags);
 }
 
-static void gic_irq_shutdown(struct irq_desc *desc)
+static void gic_irq_disable(struct irq_desc *desc)
 {
-    gic_irq_disable(desc);
+    unsigned long flags;
+
+    spin_lock_irqsave(&desc->lock, flags);
+    gic_irq_shutdown(desc);
+    spin_unlock_irqrestore(&desc->lock, flags);
 }
 
 static void gic_irq_ack(struct irq_desc *desc)
@@ -528,9 +538,8 @@ void release_dt_irq(const struct dt_irq *irq, const void 
*dev_id)
 
     desc = irq_to_desc(irq->irq);
 
-    desc->handler->shutdown(desc);
-
     spin_lock_irqsave(&desc->lock,flags);
+    desc->handler->shutdown(desc);
     action = desc->action;
     desc->action  = NULL;
     desc->status &= ~IRQ_GUEST;
@@ -600,11 +609,12 @@ int __init setup_dt_irq(const struct dt_irq *irq, struct 
irqaction *new)
     }
 
     rc = __setup_irq(desc, irq->irq, new);
-    spin_unlock_irqrestore(&desc->lock, flags);
 
     if ( !rc )
         desc->handler->startup(desc);
 
+    spin_unlock_irqrestore(&desc->lock, flags);
+
     return rc;
 }
 
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel