[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1] xen/arm: Fix deadlock in gic_set_guest_irq()



On 02/04/2014 03:52 PM, Julien Grall wrote:
Forget to add George.

On 02/03/2014 05:51 PM, Julien Grall wrote:
(+ Xen ARM maintainers)

Hello Oleksandr,

Thanks for the patch. For next time, can you add the Xen ARM maintainers
in cc? With the amount of mail in the mailing list, your mail could be
lost easily. :)

On 02/03/2014 05:33 PM, Oleksandr Tyshchenko wrote:
The possible deadlock scenario is explained below:

non interrupt context:    interrupt contex        interrupt context
                           (CPU0):                 (CPU1):
vgic_distr_mmio_write()   do_trap_irq()           do_softirq()
   |                         |                       |
   vgic_disable_irqs()       ...                     ...
     |                         |                       |
     gic_remove_from_queues()  vgic_vcpu_inject_irq()  vgic_vcpu_inject_irq()
     |  ...                      |                       |
     |  spin_lock(...)           gic_set_guest_irq()     gic_set_guest_irq()
     |  ...                        ...                     ...
     |  ... <----------------.---- spin_lock_irqsave(...)  ...
     |  ... 
<----------------.-.---------------------------spin_lock_irqsave(...)
     |  ...                  . .       Oops! The lock has already taken.
     |  spin_unlock(...)     . .
     |  ...                  . .
     gic_irq_disable()       . .
        ...                  . .
        spin_lock(...)       . .
        ...                  . .
        ... <----------------. .
        ... <------------------.
        ...
        spin_unlock(...)

Since the gic_remove_from_queues() and gic_irq_disable() called from
non interrupt context and they acquire the same lock as gic_set_guest_irq()
which called from interrupt context we must disable interrupts in these
functions to avoid possible deadlocks.

Change-Id: Ia354d87bb44418956e30cd7e49cc76616c359cc9
Signed-off-by: Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx>
Acked-by: Julien Grall <julien.grall@xxxxxxxxxx>

I think this patch should have a release exception for Xen 4.4. It's fix
a race condition in the interrupt management.

Release-acked-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>


---
  xen/arch/arm/gic.c |   10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index c44a4d0..7d83b0c 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -147,14 +147,15 @@ static void gic_irq_enable(struct irq_desc *desc)
  static void gic_irq_disable(struct irq_desc *desc)
  {
      int irq = desc->irq;
+    unsigned long flags;
- spin_lock(&desc->lock);
+    spin_lock_irqsave(&desc->lock, flags);
      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);
+    spin_unlock_irqrestore(&desc->lock, flags);
  }
static unsigned int gic_irq_startup(struct irq_desc *desc)
@@ -658,11 +659,12 @@ static inline void gic_add_to_lr_pending(struct vcpu *v, 
unsigned int irq,
  void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
  {
      struct pending_irq *p = irq_to_pending(v, virtual_irq);
+    unsigned long flags;
- spin_lock(&gic.lock);
+    spin_lock_irqsave(&gic.lock, flags);
      if ( !list_empty(&p->lr_queue) )
          list_del_init(&p->lr_queue);
-    spin_unlock(&gic.lock);
+    spin_unlock_irqrestore(&gic.lock, flags);
  }
void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,





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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.