[Xen-devel] [RFC PATCH 04/10] ARM: vGIC: add struct pending_irq locking

[Andre Przywara <andre.przywara@xxxxxxx>]

Introduce the proper locking sequence for the new pending_irq lock.
This takes the lock around multiple accesses to struct members,
also makes sure we observe the locking order (VGIC VCPU lock first,
then pending_irq lock).

Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
---
 xen/arch/arm/gic.c  | 26 ++++++++++++++++++++++++++
 xen/arch/arm/vgic.c | 12 +++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 67375a2..e175e9b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -351,6 +351,7 @@ void gic_disable_cpu(void)
 static inline void gic_set_lr(int lr, struct pending_irq *p,
                               unsigned int state)
 {
+    ASSERT(spin_is_locked(&p->lock));
     ASSERT(!local_irq_is_enabled());
 
     gic_hw_ops->update_lr(lr, p, state);
@@ -413,6 +414,7 @@ void gic_raise_guest_irq(struct vcpu *v, struct pending_irq 
*p)
     unsigned int nr_lrs = gic_hw_ops->info->nr_lrs;
 
     ASSERT(spin_is_locked(&v->arch.vgic.lock));
+    ASSERT(spin_is_locked(&p->lock));
 
     if ( v == current && list_empty(&v->arch.vgic.lr_pending) )
     {
@@ -439,6 +441,7 @@ static void gic_update_one_lr(struct vcpu *v, int i)
     gic_hw_ops->read_lr(i, &lr_val);
     irq = lr_val.virq;
     p = irq_to_pending(v, irq);
+    spin_lock(&p->lock);
     if ( lr_val.state & GICH_LR_ACTIVE )
     {
         set_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
@@ -495,6 +498,7 @@ static void gic_update_one_lr(struct vcpu *v, int i)
             }
         }
     }
+    spin_unlock(&p->lock);
 }
 
 void gic_clear_lrs(struct vcpu *v)
@@ -545,14 +549,30 @@ static void gic_restore_pending_irqs(struct vcpu *v)
             /* No more free LRs: find a lower priority irq to evict */
             list_for_each_entry_reverse( p_r, inflight_r, inflight )
             {
+                if ( p_r->irq < p->irq )
+                {
+                    spin_lock(&p_r->lock);
+                    spin_lock(&p->lock);
+                }
+                else
+                {
+                    spin_lock(&p->lock);
+                    spin_lock(&p_r->lock);
+                }
                 if ( p_r->priority == p->priority )
+                {
+                    spin_unlock(&p->lock);
+                    spin_unlock(&p_r->lock);
                     goto out;
+                }
                 if ( test_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status) &&
                      !test_bit(GIC_IRQ_GUEST_ACTIVE, &p_r->status) )
                     goto found;
             }
             /* We didn't find a victim this time, and we won't next
              * time, so quit */
+            spin_unlock(&p->lock);
+            spin_unlock(&p_r->lock);
             goto out;
 
 found:
@@ -562,12 +582,18 @@ found:
             clear_bit(GIC_IRQ_GUEST_VISIBLE, &p_r->status);
             gic_add_to_lr_pending(v, p_r);
             inflight_r = &p_r->inflight;
+
+            spin_unlock(&p_r->lock);
         }
+        else
+            spin_lock(&p->lock);
 
         gic_set_lr(lr, p, GICH_LR_PENDING);
         list_del_init(&p->lr_queue);
         set_bit(lr, &this_cpu(lr_mask));
 
+        spin_unlock(&p->lock);
+
         /* We can only evict nr_lrs entries */
         lrs--;
         if ( lrs == 0 )
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index f4ae454..44363bb 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -356,11 +356,16 @@ void vgic_enable_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);
+
+        spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
         p = irq_to_pending(v_target, irq);
+        spin_lock(&p->lock);
+
         set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
-        spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
+
         if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, 
&p->status) )
             gic_raise_guest_irq(v_target, p);
+        spin_unlock(&p->lock);
         spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
         if ( p->desc != NULL )
         {
@@ -482,10 +487,12 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
virq)
         return;
     }
 
+    spin_lock(&n->lock);
     set_bit(GIC_IRQ_GUEST_QUEUED, &n->status);
 
     if ( !list_empty(&n->inflight) )
     {
+        spin_unlock(&n->lock);
         gic_raise_inflight_irq(v, n);
         goto out;
     }
@@ -501,10 +508,13 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int 
virq)
         if ( iter->priority > priority )
         {
             list_add_tail(&n->inflight, &iter->inflight);
+            spin_unlock(&n->lock);
             goto out;
         }
     }
     list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
+    spin_unlock(&n->lock);
+
 out:
     spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
     /* we have a new higher priority irq, inject it into the guest */
-- 
2.9.0


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