[Xen-devel] [PATCH v2 09/16] 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 as 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 not being enabled.

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

---
    Changes in v2:
        - Fix typoes in commit message
        - Move this patch earlier in the series => move shutdown() in
        release_irq and gic_route_irq
---
 xen/arch/arm/gic.c |   39 ++++++++++++++++++++++++---------------
 xen/arch/arm/irq.c |    6 ++++--
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 82e0316..8c53e52 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -123,44 +123,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(sy);
     /* 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;
-    unsigned long flags;
 
-    spin_lock_irqsave(&desc->lock, flags);
+    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_irqrestore(&desc->lock, flags);
 }
 
-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)
@@ -261,11 +270,11 @@ static int gic_route_irq(unsigned int irq, bool_t level,
     if ( desc->action != NULL )
         return -EBUSY;
 
+    spin_lock_irqsave(&desc->lock, flags);
+
     /* Disable interrupt */
     desc->handler->shutdown(desc);
 
-    spin_lock_irqsave(&desc->lock, flags);
-
     desc->handler = &gic_host_irq_type;
 
     gic_set_irq_properties(irq, level, cpu_mask, priority);
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 5111b90..798353b 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -214,9 +214,10 @@ void release_irq(unsigned int irq)
 
     desc = irq_to_desc(irq);
 
+    spin_lock_irqsave(&desc->lock,flags);
+
     desc->handler->shutdown(desc);
 
-    spin_lock_irqsave(&desc->lock,flags);
     action = desc->action;
     desc->action  = NULL;
     desc->status &= ~IRQ_GUEST;
@@ -251,11 +252,12 @@ int setup_dt_irq(const struct dt_irq *irq, struct 
irqaction *new)
 
     spin_lock_irqsave(&desc->lock, flags);
     rc = __setup_irq(desc, 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