Re: [PATCH for-4.6 2/4] xen/arm: vgic: Keep track of vIRQ used by a domain

[Julien Grall <julien.grall@linaro.org>]

On 15/12/14 16:07, Julien Grall wrote:
> On 15/12/14 15:32, Stefano Stabellini wrote:
>> On Fri, 12 Dec 2014, Julien Grall wrote:
>>> +    spin_lock_init(&d->arch.vgic.lock);
>>
>> you should probably explain in the commit message the reason why you are
>> making changes to the vgic lock
> 
> Actually the domain vgic lock was never used. Only the per-vcpu vgic
> lock was in used.
> 
> So I don't make any change to the vgic lock. If I don't use it, I will
> add a patch to drop it.
> 
>>> @@ -441,6 +453,70 @@ int vgic_emulate(struct cpu_user_regs *regs, union hsr hsr)
>>>      return v->domain->arch.vgic.handler->emulate_sysreg(regs, hsr);
>>>  }
>>>  
>>> +bool_t vgic_reserve_virq(struct domain *d, unsigned int virq)
>>> +{
>>> +    bool_t reserved;
>>> +
>>> +    if ( virq >= vgic_num_irqs(d) )
>>> +        return 0;
>>> +
>>> +    spin_lock(&d->arch.vgic.lock);
>>> +    reserved = !test_and_set_bit(virq, d->arch.vgic.allocated_irqs);
>>> +    spin_unlock(&d->arch.vgic.lock);
>>
>> test_and_set_bit is atomic, why do you need to take the lock?
> 
> To avoid race condition with vgic_allocate_virq.
> 
> Anyway, I will dropped it with your suggestion to implement
> vgic_allocate_virq without lock.

Hmmm ... I was wrong on this one. The domain vgic lock is used in the
macro vgic_lock.

But it has never been initialized. I will send a separate patch for
correctly initialize it.

Regards,

-- 
Julien Grall