[Xen-devel] [PATCH] arm/acpi: Fix the deadlock in function vgic_lock_rank()

[Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>]

Commit 9d77b3c01d1261c (Configure SPI interrupt type and route to
Dom0 dynamically) causing dead loop inside the spinlock function.
Note that spinlocks in XEN are not recursive. Re-acquiring a spinlock
that has already held by calling CPU leads to deadlock. This happens
whenever dom0 does writes to GICD regs ISENABLER/ICENABLER.

The following call trace explains the problem.

DOM0 writes GICD_ISENABLER/GICD_ICENABLER
  vgic_v3_distr_common_mmio_write()
    vgic_lock_rank()  -->  acquiring first time
      vgic_enable_irqs()
        route_irq_to_guest()
          gic_route_irq_to_guest()
            vgic_get_target_vcpu()
              vgic_lock_rank()  -->  attemping acquired lock

The simple fix release spinlock before calling vgic_enable_irqs()
and vgic_disable_irqs().

Signed-off-by: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>
---
 xen/arch/arm/vgic-v2.c | 10 +++++++---
 xen/arch/arm/vgic-v3.c | 10 +++++++---
 xen/arch/arm/vgic.c    |  4 ++--
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 9adb4a9..44cd834 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -415,7 +415,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,
     struct hsr_dabt dabt = info->dabt;
     struct vgic_irq_rank *rank;
     int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
-    uint32_t tr;
+    uint32_t tr, index;
     unsigned long flags;
 
     perfc_incr(vgicd_writes);
@@ -457,8 +457,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
         vgic_reg32_setbits(&rank->ienable, r, info);
-        vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
+        index = rank->index;
+        tr = rank->ienable & (~tr);
         vgic_unlock_rank(v, rank, flags);
+        vgic_enable_irqs(v, tr, index);
         return 1;
 
     case VRANGE32(GICD_ICENABLER, GICD_ICENABLERN):
@@ -468,8 +470,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
         vgic_reg32_clearbits(&rank->ienable, r, info);
-        vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
+        index = rank->index;
+        tr = (~rank->ienable) & tr;
         vgic_unlock_rank(v, rank, flags);
+        vgic_disable_irqs(v, tr, index);
         return 1;
 
     case VRANGE32(GICD_ISPENDR, GICD_ISPENDRN):
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index b37a7c0..e04e180 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -568,7 +568,7 @@ static int __vgic_v3_distr_common_mmio_write(const char 
*name, struct vcpu *v,
 {
     struct hsr_dabt dabt = info->dabt;
     struct vgic_irq_rank *rank;
-    uint32_t tr;
+    uint32_t tr, index;
     unsigned long flags;
 
     switch ( reg )
@@ -584,8 +584,10 @@ static int __vgic_v3_distr_common_mmio_write(const char 
*name, struct vcpu *v,
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
         vgic_reg32_setbits(&rank->ienable, r, info);
-        vgic_enable_irqs(v, (rank->ienable) & (~tr), rank->index);
+        index = rank->index;
+        tr = rank->ienable & (~tr);
         vgic_unlock_rank(v, rank, flags);
+        vgic_enable_irqs(v, tr, index);
         return 1;
 
     case VRANGE32(GICD_ICENABLER, GICD_ICENABLERN):
@@ -595,8 +597,10 @@ static int __vgic_v3_distr_common_mmio_write(const char 
*name, struct vcpu *v,
         vgic_lock_rank(v, rank, flags);
         tr = rank->ienable;
         vgic_reg32_clearbits(&rank->ienable, r, info);
-        vgic_disable_irqs(v, (~rank->ienable) & tr, rank->index);
+        index = rank->index;
+        tr = (~rank->ienable) & tr;
         vgic_unlock_rank(v, rank, flags);
+        vgic_disable_irqs(v, tr, index);
         return 1;
 
     case VRANGE32(GICD_ISPENDR, GICD_ISPENDRN):
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index aa420bb..82758d2 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -322,7 +322,7 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
         irq = i + (32 * n);
-        v_target = __vgic_get_target_vcpu(v, irq);
+        v_target = vgic_get_target_vcpu(v, irq);
         p = irq_to_pending(v_target, irq);
         clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
         gic_remove_from_queues(v_target, irq);
@@ -377,7 +377,7 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
                 gprintk(XENLOG_ERR, "Unable to route IRQ %u to domain %u\n",
                         irq, d->domain_id);
         }
-        v_target = __vgic_get_target_vcpu(v, irq);
+        v_target = vgic_get_target_vcpu(v, irq);
         p = irq_to_pending(v_target, irq);
         set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
         spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
-- 
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project


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