[Xen-devel,PATCH-4.5,v2,07/10] xen/arm: don't protect GICH and lr_queue accesses with gic.lock

Message ID 1392393098-7351-7-git-send-email-stefano.stabellini@eu.citrix.com
State New
Headers show

Commit Message

Stefano Stabellini Feb. 14, 2014, 3:51 p.m.
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@eu.citrix.com>
---
 xen/arch/arm/gic.c  |   22 +++-------------------
 xen/arch/arm/vgic.c |   12 ++++++++++--
 2 files changed, 13 insertions(+), 21 deletions(-)

Comments

Julien Grall Feb. 14, 2014, 5:37 p.m. | #1
Hi Stefano,

On 02/14/2014 03:51 PM, Stefano Stabellini wrote:
> 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@eu.citrix.com>
> ---
>  xen/arch/arm/gic.c  |   22 +++-------------------
>  xen/arch/arm/vgic.c |   12 ++++++++++--
>  2 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 0955d48..6386ccb 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -667,19 +667,14 @@ void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
>  {
>      struct pending_irq *p = irq_to_pending(v, virtual_irq);
>  
> -    spin_lock(&gic.lock);
>      if ( !list_empty(&p->lr_queue) )
>          list_del_init(&p->lr_queue);
> -    spin_unlock(&gic.lock);
>  }

This patch doesn't apply correctly on the latest master. The commit
0ddaeff has replace spin_lock by spin_lock_irqsave.

You need to rebase your patches series at least on top of this patch.

Regards,

Patch

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 0955d48..6386ccb 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -667,19 +667,14 @@  void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
 {
     struct pending_irq *p = irq_to_pending(v, virtual_irq);
 
-    spin_lock(&gic.lock);
     if ( !list_empty(&p->lr_queue) )
         list_del_init(&p->lr_queue);
-    spin_unlock(&gic.lock);
 }
 
 void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
         unsigned int state, unsigned int priority)
 {
     int i;
-    unsigned long flags;
-
-    spin_lock_irqsave(&gic.lock, flags);
 
     if ( v == current && list_empty(&v->arch.vgic.lr_pending) )
     {
@@ -687,15 +682,11 @@  void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
         if (i < nr_lrs) {
             set_bit(i, &this_cpu(lr_mask));
             gic_set_lr(v, i, irq, state, priority);
-            goto out;
+            return;
         }
     }
 
     gic_add_to_lr_pending(v, irq, priority);
-
-out:
-    spin_unlock_irqrestore(&gic.lock, flags);
-    return;
 }
 
 static void _gic_clear_lr(struct vcpu *v, int i)
@@ -717,8 +708,6 @@  static void _gic_clear_lr(struct vcpu *v, int i)
     } else if ( lr & GICH_LR_PENDING ) {
         clear_bit(GIC_IRQ_GUEST_PENDING, &p->status);
     } else {
-        spin_lock(&gic.lock);
-
         GICH[GICH_LR + i] = 0;
         clear_bit(i, &this_cpu(lr_mask));
 
@@ -732,8 +721,6 @@  static void _gic_clear_lr(struct vcpu *v, int i)
             gic_add_to_lr_pending(v, irq, p->priority);
         } else
             list_del_init(&p->inflight);
-
-        spin_unlock(&gic.lock);
     }
 }
 
@@ -767,11 +754,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(v, i, p->irq, GICH_LR_PENDING, p->priority);
         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);
     }
 
 }
@@ -779,13 +766,10 @@  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);
     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);
 }
 
 static void gic_inject_irq_start(void)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 210ac39..4bfab26 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -365,12 +365,15 @@  static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
     struct pending_irq *p;
     unsigned int irq;
     int i = 0;
+    unsigned long flags;
 
     while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 ) {
         irq = i + (32 * n);
         p = irq_to_pending(v, irq);
+        spin_lock_irqsave(&v->arch.vgic.lock, flags);
         clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
         gic_remove_from_queues(v, irq);
+        spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
         if ( p->desc != NULL )
             p->desc->handler->disable(p->desc);
         i++;
@@ -391,8 +394,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_set_guest_irq(v, irq, GICH_LR_PENDING, 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_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
+            spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
+        }
         if ( p->desc != NULL )
             p->desc->handler->enable(p->desc);
         i++;