[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v10 04/32] ARM: vGIC: rework gic_remove_from_queues()



Hi Andre,

On 26/05/17 18:35, Andre Przywara wrote:
The function name gic_remove_from_queues() was a bit of a misnomer,
since it just removes an IRQ from the pending queue, not both queues.
Rename the function to make this more clear, also give it a pointer to
a struct pending_irq directly and rely on the VGIC VCPU lock to be
already taken, so this can be used in more places.
Replace the list removal in gic_clear_pending_irqs() with a call to
this function.

Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
---
 xen/arch/arm/gic.c        | 12 +++---------
 xen/arch/arm/vgic.c       |  2 +-
 xen/include/asm-arm/gic.h |  2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index dcb1783..9dde146 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -400,18 +400,12 @@ static inline void gic_add_to_lr_pending(struct vcpu *v, 
struct pending_irq *n)
     list_add_tail(&n->lr_queue, &v->arch.vgic.lr_pending);
 }

-void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq)
+void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p)
 {
-    struct pending_irq *p;
-    unsigned long flags;
-
-    spin_lock_irqsave(&v->arch.vgic.lock, flags);
-
-    p = irq_to_pending(v, virtual_irq);
+    ASSERT(spin_is_locked(&v->arch.vgic.lock));

Likely something before was wrong if you replace the lock by an ASSERT without even adding lock in the current callers. Such as the deadlock introduced in the previous patch...


     if ( !list_empty(&p->lr_queue) )

Whilst you rework gic_remove_from_queues, you could probably remove this check.

         list_del_init(&p->lr_queue);
-    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
 }

 void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq)
@@ -612,7 +606,7 @@ void gic_clear_pending_irqs(struct vcpu *v)

     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);
+        gic_remove_from_lr_pending(v, p);
 }

 int gic_events_need_delivery(void)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 69d732b..3993965 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -325,7 +325,7 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
         spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
         p = irq_to_pending(v_target, irq);
         clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
-        gic_remove_from_queues(v_target, irq);
+        gic_remove_from_lr_pending(v_target, p);
         desc = p->desc;
         spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);

diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 836a103..3130634 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -243,7 +243,7 @@ extern void init_maintenance_interrupt(void);
 extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
         unsigned int priority);
 extern void gic_raise_inflight_irq(struct vcpu *v, unsigned int virtual_irq);
-extern void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq);
+extern void gic_remove_from_lr_pending(struct vcpu *v, struct pending_irq *p);

 /* Accept an interrupt from the GIC and dispatch its handler */
 extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq);


Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.