Re: [PATCH for-4.5 6/8] xen/arm: IRQ: Add lock contrainst for gic_irq_{startup, shutdown}

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

Hi Ian,

On 02/19/2014 11:51 AM, Ian Campbell wrote:
> On Fri, 2014-01-24 at 16:43 +0000, Julien Grall wrote:
>> When multiple action will be supported, gic_irq_{startup,shutdown} will have
>> to be called in the same critical zone has setup/release.
> 
> s/has/as/?

Yes.

>> Otherwise it could have a race condition if at the same time CPU A is calling
>> release_dt_irq and CPU B is calling setup_dt_irq.
>>
>> This could end up to the IRQ is not enabled.
> 
> "This could end up with the IRQ not being enabled."
> 
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/arch/arm/gic.c |   40 +++++++++++++++++++++++++---------------
>>  1 file changed, 25 insertions(+), 15 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>> index 2643b46..ebd2b5f 100644
>> --- a/xen/arch/arm/gic.c
>> +++ b/xen/arch/arm/gic.c
>> @@ -129,43 +129,53 @@ void gic_restore_state(struct vcpu *v)
>>      gic_restore_pending_irqs(v);
>>  }
>>  
>> -static void gic_irq_enable(struct irq_desc *desc)
>> +static unsigned int gic_irq_startup(struct irq_desc *desc)
> 
> unsigned? What are the error codes here going to be?

This is the return type requested by hw_interrupt_type.startup.

It seems that the return is never checked (even in x86 code). Maybe we
should change the prototype of hw_interrupt_type.startup.

Cheers,

-- 
Julien Grall