| Message ID | 20170526173540.10066-5-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@arm.com> > --- > 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,
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)); if ( !list_empty(&p->lr_queue) ) 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);
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@arm.com> --- 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(-)