Re: [PATCH RFC] fix sun6i crash on setup_pagetables

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

On 07/25/2013 04:29 AM, Ian Campbell wrote:
> On Wed, 2013-07-24 at 19:33 +0100, Julien Grall wrote:
>> On 07/24/2013 01:14 PM, Bamvor Jian Zhang wrote:
>>> this patch move set_processor_id in front of setup_pagetables
>>> in order to fix crash when calling this_cpu in that functions.
>>> the HPTIDR is unknown value according to ARM ARM 7AR(DDI0406C),
>>> B4-1598. we should use it before using it.
>>>
>>> but i do not know why other board like arndale board. should
>>> bootloader write this register?
>>
>> On the Arndale, by default the value of HPTIDR is equal to 0. So it
>> works without any issue.
> 
> Right, but strictly the register is undefined reset on reset.
> 
> The bootloader cannot initialise it because the meaning is entirely OS
> specific.
> 
>> During the afternoon, I have noticed this error and send a similar
>> patch few minutes ago. I also moved percpu_init_areas earlier.
> 
> Right, I think this makes sense.
> 
> I suppose I should just take your (Julien's) version of this patch?

If you take my patch, could you slighty modify the commit message to
specify that's in an issue on sun6i?

>> For the next patches, could you add the appropriate people? It will
>> avoid to loose your patch with the other mail of the mailing list :).
> 
> Yes please!
> 
>>
>>>
>>> Signed-off-by: Bamvor Jian Zhang <bjzhang@suse.com>
>>> ---
>>>  xen/arch/arm/setup.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>>> index 2f9fdc8..3bf1b40 100644
>>> --- a/xen/arch/arm/setup.c
>>> +++ b/xen/arch/arm/setup.c
>>> @@ -437,6 +437,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>>>      cpus = smp_get_max_cpus();
>>>      cmdline_parse(device_tree_bootargs(device_tree_flattened));
>>>  
>>> +    set_processor_id(0); /* needed early, for this_cpu() */
>>>      setup_pagetables(boot_phys_offset, get_xen_paddr());
>>>      setup_mm(fdt_paddr, fdt_size);
>>>  
>>> @@ -459,7 +460,6 @@ void __init start_xen(unsigned long boot_phys_offset,
>>>      make_cpus_ready(cpus, boot_phys_offset);
>>>  
>>>      percpu_init_areas();
>>
>> I think you can also move percpu_init_areas earlier.
>>
>>> -    set_processor_id(0); /* needed early, for smp_processor_id() */
>>>      set_current((struct vcpu *)0xfffff000); /* debug sanity */
>>>      idle_vcpu[0] = current;
>>>  
>>>
>>
> 
>