[Xen-devel] [PATCH 2/2] xen/arm: make accesses to desc->status flags atomic

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

Using *_bit manipulation functions on desc->status is safe on arm64:
status is an unsigned int but is the first field of a struct that
contains pointers, therefore the alignement of the struct is at least 8
bytes.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/arm/gic.c |   10 +++++-----
 xen/arch/arm/irq.c |   29 +++++++++++++++--------------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index a4422fd..2a88998 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -137,7 +137,7 @@ static void gic_irq_enable(struct irq_desc *desc)
     ASSERT(spin_is_locked(&desc->lock));
 
     spin_lock_irqsave(&gic.lock, flags);
-    desc->status &= ~IRQ_DISABLED;
+    clear_bit(_IRQ_DISABLED, &desc->status);
     dsb(sy);
     /* Enable routing */
     GICD[GICD_ISENABLER + irq / 32] = (1u << (irq % 32));
@@ -154,7 +154,7 @@ static void gic_irq_disable(struct irq_desc *desc)
     spin_lock_irqsave(&gic.lock, flags);
     /* Disable routing */
     GICD[GICD_ICENABLER + irq / 32] = (1u << (irq % 32));
-    desc->status |= IRQ_DISABLED;
+    set_bit(_IRQ_DISABLED, &desc->status);
     spin_unlock_irqrestore(&gic.lock, flags);
 }
 
@@ -277,7 +277,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, bool_t 
level,
 {
     ASSERT(priority <= 0xff);     /* Only 8 bits of priority */
     ASSERT(desc->irq < gic.lines);/* Can't route interrupts that don't exist */
-    ASSERT(desc->status & IRQ_DISABLED);
+    ASSERT(test_bit(_IRQ_DISABLED, &desc->status));
     ASSERT(spin_is_locked(&desc->lock));
 
     desc->handler = &gic_host_irq_type;
@@ -296,7 +296,7 @@ void gic_route_irq_to_guest(struct domain *d, struct 
irq_desc *desc,
     ASSERT(spin_is_locked(&desc->lock));
 
     desc->handler = &gic_guest_irq_type;
-    desc->status |= IRQ_GUEST;
+    set_bit(_IRQ_GUEST, &desc->status);
 
     gic_set_irq_properties(desc->irq, level, cpumask_of(smp_processor_id()),
                            GIC_PRI_IRQ);
@@ -685,7 +685,7 @@ static void gic_update_one_lr(struct vcpu *v, int i)
         clear_bit(i, &this_cpu(lr_mask));
 
         if ( p->desc != NULL )
-            p->desc->status &= ~IRQ_INPROGRESS;
+            clear_bit(_IRQ_INPROGRESS, &p->desc->status);
         clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status);
         clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status);
         p->lr = GIC_INVALID_LR;
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 756250c..d0c7962 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -102,7 +102,7 @@ static inline struct domain *irq_get_domain(struct irq_desc 
*desc)
 {
     ASSERT(spin_is_locked(&desc->lock));
 
-    if ( !(desc->status & IRQ_GUEST) )
+    if ( !test_bit(_IRQ_GUEST, &desc->status) )
         return dom_xen;
 
     ASSERT(desc->action != NULL);
@@ -166,40 +166,41 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, 
int is_fiq)
         goto out;
     }
 
-    if ( desc->status & IRQ_GUEST )
+    if ( test_bit(_IRQ_GUEST, &desc->status) )
     {
         struct domain *d = irq_get_domain(desc);
 
         desc->handler->end(desc);
 
-        desc->status |= IRQ_INPROGRESS;
+        set_bit(_IRQ_INPROGRESS, &desc->status);
         desc->arch.eoi_cpu = smp_processor_id();
 
         vgic_vcpu_inject_spi(d, irq);
         goto out_no_end;
     }
 
-    desc->status |= IRQ_PENDING;
+    set_bit(_IRQ_PENDING, &desc->status);
 
     /*
      * Since we set PENDING, if another processor is handling a different
      * instance of this same irq, the other processor will take care of it.
      */
-    if ( desc->status & (IRQ_DISABLED | IRQ_INPROGRESS) )
+    if ( test_bit(_IRQ_DISABLED, &desc->status) ||
+         test_bit(_IRQ_INPROGRESS, &desc->status) )
         goto out;
 
-    desc->status |= IRQ_INPROGRESS;
+    set_bit(_IRQ_INPROGRESS, &desc->status);
 
     action = desc->action;
-    while ( desc->status & IRQ_PENDING )
+    while ( test_bit(_IRQ_PENDING, &desc->status) )
     {
-        desc->status &= ~IRQ_PENDING;
+        clear_bit(_IRQ_PENDING, &desc->status);
         spin_unlock_irq(&desc->lock);
         action->handler(irq, action->dev_id, regs);
         spin_lock_irq(&desc->lock);
     }
 
-    desc->status &= ~IRQ_INPROGRESS;
+    clear_bit(_IRQ_INPROGRESS, &desc->status);
 
 out:
     desc->handler->end(desc);
@@ -222,12 +223,12 @@ void release_irq(unsigned int irq)
 
     action = desc->action;
     desc->action  = NULL;
-    desc->status &= ~IRQ_GUEST;
+    clear_bit(_IRQ_GUEST, &desc->status);
 
     spin_unlock_irqrestore(&desc->lock,flags);
 
     /* Wait to make sure it's not being used on another CPU */
-    do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
+    do { smp_mb(); } while ( test_bit(_IRQ_INPROGRESS, &desc->status) );
 
     if ( action && action->free_on_release )
         xfree(action);
@@ -255,7 +256,7 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction 
*new)
 
     spin_lock_irqsave(&desc->lock, flags);
 
-    if ( desc->status & IRQ_GUEST )
+    if ( test_bit(_IRQ_GUEST, &desc->status) )
     {
         struct domain *d = irq_get_domain(desc);
 
@@ -323,10 +324,10 @@ int route_dt_irq_to_guest(struct domain *d, const struct 
dt_irq *irq,
     {
         struct domain *ad = irq_get_domain(desc);
 
-        if ( (desc->status & IRQ_GUEST) && d == ad )
+        if ( test_bit(_IRQ_GUEST, &desc->status) && d == ad )
             goto out;
 
-        if ( desc->status & IRQ_GUEST )
+        if ( test_bit(_IRQ_GUEST, &desc->status) )
             printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n",
                    irq->irq, ad->domain_id);
         else
-- 
1.7.10.4


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