Re: [PATCH] xen: arm: wait 1000ms for a CPU to come up, instead of forever

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

On 14/01/15 14:56, Ian Campbell wrote:
>>> Where do you mean exactly?
>>
>> See for instance exynos5_cpu_power_up.
> 
> Appears to be waiting the h/w to acknowledge that the CPU power is on,
> which is no guarantee that it is going to actually boot, or even make it
> to Xen code.

Right. This is what I was trying to say.

>>
>>>>
>>>>> +
>>>>> +    while ( !cpu_online(cpu) && NOW() < deadline )
>>>>>      {
>>>>>          cpu_relax();
>>>>>          process_pending_softirqs();
>>>>>      }
>>>>>  
>>>>> +    /*
>>>>> +     * Nuke start of day info before checking one last time if the CPU
>>>>> +     * actually came online.
>>>>> +     *
>>>>> +     * Doesn't completely avoid the posibility of it trying to
>>>>> +     * progress with another CPUs stack etc, but better than nothing,
>>>>> +     * hopefully.
>>>>> +     */
>>>>> +    init_data.stack = NULL;
>>>>> +    init_data.cpuid = ~0;
>>>>> +    smp_up_cpu = MPIDR_INVALID;
>>>>> +    clean_dcache(smp_up_cpu);
>>>>
>>>> I don't understand why you need to do this. Is it for pure clean up? If
>>>> so, please explain it in the commit message.
>>>
>>> Is the comment right above it not sufficient explanation? I can insert
>>> at the end of the first paragraph "If it is not online it may still be
>>> trying and may show up later" is that would help.
>>
>> It's more clear for me with "If it is not online ...".
>>
>> For the second paragraph, I would say "It doesn't completely avoid ...".
> 
> OK, I'll make both changes.
> 
>> Also I was wondering if there is any possibility to turn off the cpu if
>> it doesn't come online?
> 
> PSCI has a cpu_off, and there will be arch specific mechanisms. Whether
> they will work under the circumstances is hard to say. In any case that
> is out of scope for this patch.

True. I will give a look later when I will have time.

With the change in the comment:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall