Re: [Xen-devel] [PATCH-4.5 v3 10/12] xen/arm: don't protect GICH and lr_queue accesses with gic.lock

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 18 Mar 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 03/18/2014 06:52 PM, Stefano Stabellini wrote:
> > On Tue, 18 Mar 2014, Julien Grall wrote:
> >> Hi Stefano,
> >>
> >> On 03/18/2014 02:59 PM, Stefano Stabellini wrote:
> >>> On Wed, 26 Feb 2014, Julien Grall wrote:
> >>>> On 26/02/14 18:39, Stefano Stabellini wrote:
> >>>>> GICH is banked, protect accesses by disabling interrupts.
> >>>>> Protect lr_queue accesses with the vgic.lock only.
> >>>>
> >>>> When the interrupt is an SPI, the lr_queue is shared between every VCPU. 
> >>>> Using
> >>>> only vgic.lock seems wrong to me.
> >>>
> >>> Even though lr_queue is a field in a struct that can be per domain for
> >>> irq > 32, the lr_pending queue is always per vcpu, in fact it keeps
> >>> track of the irqs waiting to go into lr registers, that are banked.
> >>>
> >>
> >> It might have a race between adding and removing lr_queue. For now it's
> >> safe because the SPIs are always routed to VCPU0 (even if the guest ask
> >> to route on VCPU1).
> >>
> >> Just by reading the code, nothing protect correctly SPIs between VCPUs.
> >> We are only take vgic.lock, which is wrong.
> > 
> > Like you said, today SPIs are always routed to the same cpu.
> > When we'll implement IRQ migration we'll have to make sure that we take
> > the appropriate locks during the operation but I don't expect that this
> > patch is going to make things more difficult.
> 
> I'm fine with that. Can you create a TODO somewhere to not forget this
> possible locking issue later?

Sure

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