From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: [PATCH for-4.5 6/8] xen/arm: IRQ: Add lock contrainst for gic_irq_{startup, shutdown} Date: Fri, 24 Jan 2014 16:43:40 +0000 Message-ID: <1390581822-32624-7-git-send-email-julien.grall@linaro.org> References: <1390581822-32624-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.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1W6jrL-0001y1-0I for xen-devel@lists.xenproject.org; Fri, 24 Jan 2014 16:43:59 +0000 Received: by mail-ee0-f41.google.com with SMTP id e49so1084971eek.0 for ; Fri, 24 Jan 2014 08:43:57 -0800 (PST) In-Reply-To: <1390581822-32624-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, patches@linaro.org 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 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 --- 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