Re: [Xen-devel] [PATCH v6 1/5] xen/arm: observe itargets setting in vgic_enable_irqs and vgic_disable_irqs

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 24 Jun 2014, Julien Grall wrote:
> On 24/06/14 12:38, Stefano Stabellini wrote:
> > > Sorry for not having catch this earlier. I don't really like the idea to
> > > extend the rank lock to vgic_{enable/disable}_irqs. The 2 functions
> > > can be long to execute as it may touch the GIC distributor.
> > > 
> > > In another way, the rank lock is only taken in the distributor emulation.
> > > 
> > > Assuming we consider that distributor access may be slow:
> > 
> > We could end up enabling or disabling the wrong set of interrupts
> > without this change. I think it is necessary.
> 
> AFAIU, this lock only protects the rank when we retrieve the target VCPU, the
> other part of the function will still work without it.
> 
> What I meant is to call vgic_get_target_vcpu, so the lock will protect only
> what is necessary and not more.

I don't think so (unless I misunderstood your suggestion): setting
ienable and enabling the interrupts need to be atomic.  Otherwise this
can happen:

    VCPU0                                       VCPU1
- rank_lock
- write icenabler
- get target vcpus
- rank_unlock
                                         - rank_lock
                                         - write icenable
                                         - get target vcpus (some are the same)
                                         - rank_unlock

                                         - vgic_enable_irqs

- vgic_enable_irqs


we now have an inconsistent state: we enabled the irqs written from
vcpu0 but icenable reflects what was written from vcpu1.

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