diff mbox

[Xen-devel,v8,08/10] xen/arm: take the rank lock before accessing ipriority

Message ID 1405016003-19131-8-git-send-email-stefano.stabellini@eu.citrix.com
State New
Headers show

Commit Message

Stefano Stabellini July 10, 2014, 6:13 p.m. UTC
Currently we read ipriority from vgic_vcpu_inject_irq without taking the
rank lock. Fix that by taking the rank lock and reading ipriority at the
beginning of the function.

As vgic_vcpu_inject_irq is called from the irq.c upon receiving an
interrupt, we need to change the implementation of vgic_lock/unlock_rank
to spin_lock_irqsave to make it safe in irq context.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

Changes in v2:
- rebased on ab78724fc5628318b172b4344f7280621a151e1b;
- remove warning on changing priority of active irqs.
---
 xen/arch/arm/vgic-v2.c     |    2 ++
 xen/arch/arm/vgic.c        |    6 +++++-
 xen/include/asm-arm/vgic.h |    4 ++--
 3 files changed, 9 insertions(+), 3 deletions(-)

Comments

Ian Campbell July 17, 2014, 12:51 p.m. UTC | #1
On Thu, 2014-07-10 at 19:13 +0100, Stefano Stabellini wrote:

> -#define vgic_lock_rank(v, r) spin_lock(&(r)->lock)
> -#define vgic_unlock_rank(v, r) spin_unlock(&(r)->lock)
> +#define vgic_lock_rank(v, r) spin_lock_irqsave(&(r)->lock, flags)
> +#define vgic_unlock_rank(v, r) spin_unlock_irqrestore(&(r)->lock, flags)

Please make flags an explicit macro parameter.

Ian.
Stefano Stabellini July 23, 2014, 2:57 p.m. UTC | #2
On Thu, 17 Jul 2014, Ian Campbell wrote:
> On Thu, 2014-07-10 at 19:13 +0100, Stefano Stabellini wrote:
> 
> > -#define vgic_lock_rank(v, r) spin_lock(&(r)->lock)
> > -#define vgic_unlock_rank(v, r) spin_unlock(&(r)->lock)
> > +#define vgic_lock_rank(v, r) spin_lock_irqsave(&(r)->lock, flags)
> > +#define vgic_unlock_rank(v, r) spin_unlock_irqrestore(&(r)->lock, flags)
> 
> Please make flags an explicit macro parameter.

OK
diff mbox

Patch

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 5116a99..ae31dbf 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -39,6 +39,7 @@  static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
     register_t *r = select_user_reg(regs, dabt.reg);
     struct vgic_irq_rank *rank;
     int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
+    unsigned long flags;
 
     switch ( gicd_reg )
     {
@@ -269,6 +270,7 @@  static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
     struct vgic_irq_rank *rank;
     int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase);
     uint32_t tr;
+    unsigned long flags;
 
     switch ( gicd_reg )
     {
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 569a859..fc51e87 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -175,6 +175,7 @@  struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int irq)
 {
     struct vcpu *v_target;
     struct vgic_irq_rank *rank = vgic_rank_irq(v, irq);
+    unsigned long flags;
 
     vgic_lock_rank(v, rank);
     v_target = _vgic_get_target_vcpu(v, irq);
@@ -386,6 +387,10 @@  void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
     bool_t running;
     struct vcpu *vcpu_migrate_from;
 
+    vgic_lock_rank(v, rank);
+    priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8, irq, DABT_WORD)], 0, irq & 0x3);
+    vgic_unlock_rank(v, rank);
+
     spin_lock_irqsave(&v->arch.vgic.lock, flags);
 
     /* vcpu offline */
@@ -418,7 +423,6 @@  void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq)
         goto out;
     }
  
-    priority = vgic_byte_read(rank->ipriority[REG_RANK_INDEX(8, irq, DABT_WORD)], 0, irq & 0x3);
 
     n->irq = irq;
     n->priority = priority;
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index e961780..5d6a8ad 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -106,8 +106,8 @@  struct vgic_ops {
 #define vgic_lock(v)   spin_lock_irq(&(v)->domain->arch.vgic.lock)
 #define vgic_unlock(v) spin_unlock_irq(&(v)->domain->arch.vgic.lock)
 
-#define vgic_lock_rank(v, r) spin_lock(&(r)->lock)
-#define vgic_unlock_rank(v, r) spin_unlock(&(r)->lock)
+#define vgic_lock_rank(v, r) spin_lock_irqsave(&(r)->lock, flags)
+#define vgic_unlock_rank(v, r) spin_unlock_irqrestore(&(r)->lock, flags)
 
 /*
  * Rank containing GICD_<FOO><n> for GICD_<FOO> with