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

Re: [Xen-devel] [RFC PATCH 04/10] ARM: vGIC: add struct pending_irq locking



On Thu, 4 May 2017, Julien Grall wrote:
> On 04/05/17 16:31, Andre Przywara wrote:
> > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > index f4ae454..44363bb 100644
> > --- a/xen/arch/arm/vgic.c
> > +++ b/xen/arch/arm/vgic.c
> > @@ -356,11 +356,16 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int
> > n)
> >      while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
> >          irq = i + (32 * n);
> >          v_target = vgic_get_target_vcpu(v, irq);
> > +
> > +        spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
> >          p = irq_to_pending(v_target, irq);
> > +        spin_lock(&p->lock);
> > +
> >          set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> > -        spin_lock_irqsave(&v_target->arch.vgic.lock, flags);
> > +
> >          if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE,
> > &p->status) )
> >              gic_raise_guest_irq(v_target, p);
> > +        spin_unlock(&p->lock);
> 
> Why does the lock not cover p->desc below?

Indeed. The main problem with this patch is that it doesn't say what
this lock is supposed to cover. It is OK for the lock not to cover
everything pending_irq related as long as it is clear.

As it stands, it is not clear.

For example, why the lock is not added to following functions?

- gic_route_irq_to_guest
- gic_remove_irq_from_guest
- gic_remove_from_queues
- gic_raise_inflight_irq
- vgic_migrate_irq
- arch_move_irqs
- vgic_disable_irqs

I came up with this list by grepping for irq_to_pending and listing the
function where fields of the pending_irq struct are accessed.



> >          spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
> >          if ( p->desc != NULL )
> >          {

_______________________________________________
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®.