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

[Julien Grall <julien.grall@xxxxxxxxxx>]

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?

Regards,

-- 
Julien Grall

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