From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@citrix.com>
Subject: Re: [PATCH V2 1/2] xen/arm: gic_disable_cpu must be
 called with interrupts disabled
Date: Wed, 1 May 2013 13:25:58 +0100
Message-ID: <518109D6.3070801@citrix.com>
References: <1366119508-9227-1-git-send-email-julien.grall@citrix.com>
	<1366119508-9227-2-git-send-email-julien.grall@citrix.com>
	<1367409334.3142.718.camel@zakaz.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1367409334.3142.718.camel@zakaz.uk.xensource.com>
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: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

On 05/01/2013 12:55 PM, Ian Campbell wrote:

> On Tue, 2013-04-16 at 14:38 +0100, Julien Grall wrote:
>> gic_disable_cpu is only called with interrupt disabled.
>> Use spin_lock instead of spin_lock_irq and check the function is
>> called with interrupts disabled.
> 
> Is this patch still relevant after this change which is now in master?


Yes. gic_disable is only called by __cpu_disable, and the irq is disabled.

local_irq_disable();
gic_disable_cpu();

> 
> commit b4aae03ce525e8e2364814d0fed8f982aefc4958
> Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Date:   Mon Apr 29 18:04:25 2013 +0100
> 
>     xen/arm: do not call __cpu_disable on machine_halt
>     
>     __cpu_disable shouldn't be called on machine_halt, in fact it cannot
>     succeed: cpu_disable_scheduler won't be able to migrate away vcpus to
>     others pcpus.
>     
>     Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>     Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
>>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>> ---
>>  xen/arch/arm/gic.c |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>> index aa179a9..0ef994e 100644
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -419,10 +419,12 @@ void __cpuinit gic_init_secondary_cpu(void)
>>  /* Shut down the per-CPU GIC interface */
>>  void gic_disable_cpu(void)
>>  {
>> -    spin_lock_irq(&gic.lock);
>> +    ASSERT(!local_irq_is_enabled());
>> +
>> +    spin_lock(&gic.lock);
>>      gic_cpu_disable();
>>      gic_hyp_disable();
>> -    spin_unlock_irq(&gic.lock);
>> +    spin_unlock(&gic.lock);
>>  }
>>  
>>  void gic_route_ppis(void)
> 
>