[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v4 10/18] xen/arm: IRQ: Require desc.lock be held by callers of hw_irq_controller callbacks
When multiple action are supported, gic_irq_{startup,shutdown} will have to be called in the same critical section as setup/release. Otherwise there is 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 with the IRQ not being enabled. At the same time, modify gic_irq_{enable,disable} to require desc.lock be held. With both of theses changes, ARM's locking requirements is the same as x86's. Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx> --- Changes in v4: - Fix typo in commit message - Don't depend on Stefano's interrupt series (will likely be pushed before) Changes in v3: - Update commit message - Require desc.lock also for gic_irq_{enable,disable} - irqflags is unsigned long not unsigned int 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 | 21 +++++++++++---------- xen/arch/arm/irq.c | 6 ++++-- xen/arch/arm/vgic.c | 10 ++++++++++ 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 77dfecf..29ecb49 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -128,14 +128,14 @@ static void gic_irq_enable(struct irq_desc *desc) int irq = desc->irq; unsigned long flags; - spin_lock_irqsave(&desc->lock, flags); - spin_lock(&gic.lock); + ASSERT(spin_is_locked(&desc->lock)); + + spin_lock_irqsave(&gic.lock, flags); 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); + spin_unlock_irqrestore(&gic.lock, flags); } static void gic_irq_disable(struct irq_desc *desc) @@ -143,18 +143,19 @@ static void gic_irq_disable(struct irq_desc *desc) int irq = desc->irq; unsigned long flags; - spin_lock_irqsave(&desc->lock, flags); - spin_lock(&gic.lock); + ASSERT(spin_is_locked(&desc->lock)); + + spin_lock_irqsave(&gic.lock, flags); /* Disable routing */ GICD[GICD_ICENABLER + irq / 32] = (1u << (irq % 32)); desc->status |= IRQ_DISABLED; - spin_unlock(&gic.lock); - spin_unlock_irqrestore(&desc->lock, flags); + spin_unlock_irqrestore(&gic.lock, flags); } static unsigned int gic_irq_startup(struct irq_desc *desc) { gic_irq_enable(desc); + return 0; } @@ -261,11 +262,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 26574ca..b6dd9de 100644 --- a/xen/arch/arm/irq.c +++ b/xen/arch/arm/irq.c @@ -217,9 +217,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; @@ -254,11 +255,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; } diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 4a7f8c0..1b95003 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -374,6 +374,7 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n) const unsigned long mask = r; struct pending_irq *p; unsigned int irq; + unsigned long flags; int i = 0; while ( (i = find_next_bit(&mask, 32, i)) < 32 ) { @@ -382,7 +383,11 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n) clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status); gic_remove_from_queues(v, irq); if ( p->desc != NULL ) + { + spin_lock_irqsave(&p->desc->lock, flags); p->desc->handler->disable(p->desc); + spin_unlock_irqrestore(&p->desc->lock, flags); + } i++; } } @@ -392,6 +397,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) const unsigned long mask = r; struct pending_irq *p; unsigned int irq; + unsigned long flags; int i = 0; while ( (i = find_next_bit(&mask, 32, i)) < 32 ) { @@ -401,7 +407,11 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) ) gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority); if ( p->desc != NULL ) + { + spin_lock_irqsave(&p->desc->lock, flags); p->desc->handler->enable(p->desc); + spin_unlock_irqrestore(&p->desc->lock, flags); + } i++; } } -- 1.7.10.4 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |