[Xen-devel] [PATCH v7 10/12] xen/arm: don't protect GICH and lr_queue accesses with gic.lock

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

GICH is banked, protect accesses by disabling interrupts.
Protect lr_queue accesses with the vgic.lock only.
gic.lock only protects accesses to GICD now.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

---

Changes in v5:
- gic_remove_from_queues need to be protected with a vgic lock;
- introduce ASSERTs to check the vgic is locked and interrupts are
disabled.

Changes in v4:
- improved in code comments.
---
 xen/arch/arm/gic.c           |   35 ++++++++++++++++-------------------
 xen/arch/arm/vgic.c          |    9 +++++++--
 xen/include/asm-arm/domain.h |    5 ++++-
 3 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 13ce703..e6e6f1a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -119,6 +119,7 @@ void gic_save_state(struct vcpu *v)
 void gic_restore_state(struct vcpu *v)
 {
     int i;
+    ASSERT(!local_irq_is_enabled());
 
     if ( is_idle_vcpu(v) )
         return;
@@ -630,6 +631,7 @@ static inline void gic_set_lr(int lr, struct pending_irq *p,
 {
     uint32_t lr_reg;
 
+    ASSERT(!local_irq_is_enabled());
     BUG_ON(lr >= nr_lrs);
     BUG_ON(lr < 0);
     BUG_ON(state & ~(GICH_LR_STATE_MASK<<GICH_LR_STATE_SHIFT));
@@ -650,6 +652,8 @@ static inline void gic_add_to_lr_pending(struct vcpu *v, 
struct pending_irq *n)
 {
     struct pending_irq *iter;
 
+    ASSERT(spin_is_locked(&v->arch.vgic.lock));
+
     if ( !list_empty(&n->lr_queue) )
         return;
 
@@ -669,19 +673,20 @@ 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_irqsave(&gic.lock, flags);
+    spin_lock_irqsave(&v->arch.vgic.lock, flags);
     if ( !list_empty(&p->lr_queue) )
         list_del_init(&p->lr_queue);
-    spin_unlock_irqrestore(&gic.lock, flags);
+    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
 }
 
 void gic_raise_guest_irq(struct vcpu *v, unsigned int virtual_irq,
         unsigned int priority)
 {
     int i;
-    unsigned long flags;
     struct pending_irq *n = irq_to_pending(v, virtual_irq);
 
+    ASSERT(spin_is_locked(&v->arch.vgic.lock));
+
     if ( test_bit(GIC_IRQ_GUEST_VISIBLE, &n->status))
     {
         if ( v == current )
@@ -689,23 +694,17 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int 
virtual_irq,
         return;
     }
 
-    spin_lock_irqsave(&gic.lock, flags);
-
     if ( v == current && list_empty(&v->arch.vgic.lr_pending) )
     {
         i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
         if (i < nr_lrs) {
             set_bit(i, &this_cpu(lr_mask));
             gic_set_lr(i, irq_to_pending(v, virtual_irq), GICH_LR_PENDING);
-            goto out;
+            return;
         }
     }
 
     gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq));
-
-out:
-    spin_unlock_irqrestore(&gic.lock, flags);
-    return;
 }
 
 static void gic_update_one_lr(struct vcpu *v, int i)
@@ -715,6 +714,7 @@ static void gic_update_one_lr(struct vcpu *v, int i)
     int irq;
 
     ASSERT(spin_is_locked(&v->arch.vgic.lock));
+    ASSERT(!local_irq_is_enabled());
 
     lr = GICH[GICH_LR + i];
     irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK;
@@ -729,8 +729,6 @@ static void gic_update_one_lr(struct vcpu *v, int i)
     } else if ( lr & GICH_LR_PENDING ) {
         clear_bit(GIC_IRQ_GUEST_QUEUED, &p->status);
     } else {
-        spin_lock(&gic.lock);
-
         GICH[GICH_LR + i] = 0;
         clear_bit(i, &this_cpu(lr_mask));
 
@@ -744,8 +742,6 @@ static void gic_update_one_lr(struct vcpu *v, int i)
             gic_raise_guest_irq(v, irq, p->priority);
         } else
             list_del_init(&p->inflight);
-
-        spin_unlock(&gic.lock);
     }
 }
 
@@ -776,11 +772,11 @@ static void gic_restore_pending_irqs(struct vcpu *v)
         i = find_first_zero_bit(&this_cpu(lr_mask), nr_lrs);
         if ( i >= nr_lrs ) return;
 
-        spin_lock_irqsave(&gic.lock, flags);
+        spin_lock_irqsave(&v->arch.vgic.lock, flags);
         gic_set_lr(i, p, GICH_LR_PENDING);
         list_del_init(&p->lr_queue);
         set_bit(i, &this_cpu(lr_mask));
-        spin_unlock_irqrestore(&gic.lock, flags);
+        spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
     }
 
 }
@@ -788,13 +784,12 @@ static void gic_restore_pending_irqs(struct vcpu *v)
 void gic_clear_pending_irqs(struct vcpu *v)
 {
     struct pending_irq *p, *t;
-    unsigned long flags;
 
-    spin_lock_irqsave(&gic.lock, flags);
+    ASSERT(spin_is_locked(&v->arch.vgic.lock));
+
     v->arch.lr_mask = 0;
     list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue )
         list_del_init(&p->lr_queue);
-    spin_unlock_irqrestore(&gic.lock, flags);
 }
 
 int gic_events_need_delivery(void)
@@ -805,6 +800,8 @@ int gic_events_need_delivery(void)
 
 void gic_inject(void)
 {
+    ASSERT(!local_irq_is_enabled());
+
     gic_restore_pending_irqs(current);
 
     if ( !list_empty(&current->arch.vgic.lr_pending) && lr_all_full() )
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 435a8d7..c5e17e0 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -393,8 +393,13 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, 
int n)
              vcpu_info(current, evtchn_upcall_pending) &&
              list_empty(&p->inflight) )
             vgic_vcpu_inject_irq(v, irq);
-        else if ( !list_empty(&p->inflight) && 
!test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
-            gic_raise_guest_irq(v, irq, p->priority);
+        else {
+            unsigned long flags;
+            spin_lock_irqsave(&v->arch.vgic.lock, flags);
+            if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, 
&p->status) )
+                gic_raise_guest_irq(v, irq, p->priority);
+            spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+        }
         if ( p->desc != NULL )
             p->desc->handler->enable(p->desc);
         i++;
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index f96dc12..71f563f 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -68,7 +68,10 @@ struct pending_irq
      * vgic.inflight_irqs */
     struct list_head inflight;
     /* lr_queue is used to append instances of pending_irq to
-     * gic.lr_pending */
+     * lr_pending. lr_pending is a per vcpu queue, therefore lr_queue
+     * accesses are protected with the vgic lock.
+     * TODO: when implementing irq migration, taking only the current
+     * vgic lock is not going to be enough. */
     struct list_head lr_queue;
 };
 
-- 
1.7.10.4


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