Re: [PATCH v3 3/6] xen/arm: gic: Use the correct CPU ID

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

On 09/20/2013 02:36 PM, Ian Campbell wrote:
> On Fri, 2013-09-20 at 13:44 +0100, Julien Grall wrote:
>>> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
>>> index b969d23..4061691 100644
>>> --- a/xen/arch/arm/gic.c
>>> +++ b/xen/arch/arm/gic.c
>>> @@ -57,6 +57,29 @@ static DEFINE_PER_CPU(uint64_t, lr_mask);
>>>  
>>>  static unsigned nr_lrs;
>>>  
>>> +/* The GIC mapping of CPU interfaces does not necessarily match the
>>> + * logical CPU numbering. Let's use mapping as returned by the GIC
>>> + * itself
>>> + */
>>> +static DEFINE_PER_CPU(u8, gic_cpu_id);
>>> +
>>
>> Following the discussion on another thread
>> (http://lists.xen.org/archives/html/xen-devel/2013-09/msg02072.html), we
>> need to initialize each CPU ID to 0xff by default, to be able to bring
>> up secondary CPUs with an SGI (send_SGI_mask).
> 
> We may as well use the send_SGI_allbutself mechanism, which doesn't rely
> on knowing and target ids.
>> But as I understand, per_cpu area for a give area is initialized in
>> cpu_up and there is no way to give a default value. So user could call
>> send_SGI_mask to early function and use an invalid CPU area.
> 
> If we need to go down this path then I would write something like
> 	cpu_online(cpu) ? this_cpu(thing, cpu) :  0xff
> but I don't think we need to because we can easily avoid the problem.
> 
>> IMHO, this solution is too fragile and I would like to use the same
>> solution as my first version (ie with an array of 8 cells).
> 
> I think cpu bring up should use allbutself and the other callers can
> reasonable be restricted to only operate on cpus which are up and
> therefore have a valid mask already allocated and initialised.


Ok, so I will, at least, add a comment in the gic code to explain this
restriction.

-- 
Julien Grall