From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: [PATCH v2 09/16] xen/arm: IRQ: Add lock contrainst for gic_irq_{startup, shutdown} Date: Thu, 3 Apr 2014 21:42:00 +0100 Message-ID: <1396557727-19102-10-git-send-email-julien.grall@linaro.org> References: <1396557727-19102-1-git-send-email-julien.grall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WVoT5-0008D7-W4 for xen-devel@lists.xenproject.org; Thu, 03 Apr 2014 20:42:36 +0000 Received: by mail-we0-f177.google.com with SMTP id u57so2466295wes.36 for ; Thu, 03 Apr 2014 13:42:34 -0700 (PDT) In-Reply-To: <1396557727-19102-1-git-send-email-julien.grall@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xenproject.org Cc: stefano.stabellini@citrix.com, Julien Grall , tim@xen.org, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org 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 --- 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