Re: [PATCH RFC 5/7] xen: arm: rewrite start of day page table and cpu bring up

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

On 09/17/2013 05:36 PM, Ian Campbell wrote:
> On Tue, 2013-09-17 at 17:18 +0100, Julien Grall wrote:
>>> +        /* Now we can install the fixmap and dtb mappings, since we
>>> +         * don't need the 1:1 map any more */
>>> +        dsb   sy
>>> +        ldr   r1, =boot_second
>>> +#if defined(EARLY_PRINTK)
>>> +        /* xen_fixmap pagetable */
>>
>> Can you add a comment to explain why we don't need to map the fixmap
>> when early printk is not enabled?
> 
> It's covered by the overall description of the boot tables layout which
> is in mm.c and referenced elsewhere in this file. Is the suficient?

I just saw the comment, I'm fine with it. It took me several minutes to
find where :).

> [..]
>>> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h
>>> index f460e9c..f616807 100644
>>> --- a/xen/include/asm-arm/platform.h
>>> +++ b/xen/include/asm-arm/platform.h
>>> @@ -14,6 +14,11 @@ struct platform_desc {
>>>      /* Platform initialization */
>>>      int (*init)(void);
>>>      int (*init_time)(void);
>>> +#ifdef CONFIG_ARM_32
>>> +    /* SMP */
>>> +    int (*cpu_init)(int cpu);
>>
>> I don't think a cpu_init callback is usefull. An smp_init callback would
>> be better.
>>
>> This will allow you to move the sys_flags check for the versatile
>> express in smp_init.
> 
> I wondered if there might be platforms with differeing mbox addresses
> for different CPU. e.g. the armv8 stuff (which doesn't use this path)
> makes provisions for this.

I believe, it's the case on midway. Andre, can you confirm?
But, I think this code can be merge in cpu_up.

> 
> But I'll make the suggested change -- we can also refactor or add a
> second callback if such a platform shows up.
> 
> You didn't trim your quotes, I hope I didn't miss any comments (I
> trimmed a bunch of the s/xen_/boot_/ ones... Will do a thorough
> sweep...)

You didn't miss any comments, next time I will trim my quotes.

-- 
Julien Grall