From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [PATCH for-4.6 2/4] xen/arm: vgic: Keep track of
 vIRQ used by a domain
Date: Wed, 17 Dec 2014 15:23:27 +0000
Message-ID: <54919FEF.1020308@linaro.org>
References: <1418395392-30460-1-git-send-email-julien.grall@linaro.org>
	<1418395392-30460-3-git-send-email-julien.grall@linaro.org>
	<alpine.DEB.2.02.1412151527310.30971@kaball.uk.xensource.com>
	<548F0733.2020108@linaro.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <julien.grall@linaro.org>) id 1Y1GS4-0001fT-26
	for xen-devel@lists.xenproject.org; Wed, 17 Dec 2014 15:23:48 +0000
Received: by mail-wi0-f172.google.com with SMTP id n3so16088266wiv.11
	for <xen-devel@lists.xenproject.org>;
	Wed, 17 Dec 2014 07:23:44 -0800 (PST)
In-Reply-To: <548F0733.2020108@linaro.org>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: ian.campbell@citrix.com, tim@xen.org, stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org, parth.dixit@linaro.org, christoffer.dall@linaro.org
List-Id: xen-devel@lists.xenproject.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