Re: [PATCH] xen/arm: midway: implement SMP

[Andre Przywara <andre.przywara@linaro.org>]

On 11/18/2013 02:27 PM, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>   xen/arch/arm/platforms/midway.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
>
> diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c
> index 399056b..3b9fcfc 100644
> --- a/xen/arch/arm/platforms/midway.c
> +++ b/xen/arch/arm/platforms/midway.c
> @@ -47,6 +47,27 @@ static uint32_t midway_quirks(void)
>       return PLATFORM_QUIRK_DOM0_MAPPING_11;
>   }
>
> +static int __init midway_cpu_up(int cpu)

Wouldn't it make more sense to do this initialization in smp_init() 
instead of here per CPU? init_secondary() is fixed and thus we will not 
write different values for each core.
I guess it does not really matter, I was just wondering whether this 
would be a saner approach (and I think your first version was this way, 
right?)

Thanks,
Andre.

P.S. BTW: Any reason we instantiate cpu_up() for all platforms where 
they are actually NOPs? I think the platform code only calls a function 
if it is non-NULL, so we could just skip all of this: cpu_up() {return 
0;} definition.

> +{
> +    void __iomem *pens;
> +
> +    pens = ioremap_nocache(0, PAGE_SIZE);
> +    if ( !pens )
> +    {
> +        dprintk(XENLOG_ERR, "Unable to map midway pens MMIO\n");
> +        return -EFAULT;
> +    }
> +
> +    printk("Set cpu pen %u to %"PRIpaddr" (%p)\n",
> +           cpu, __pa(init_secondary), init_secondary);
> +
> +    writel(__pa(init_secondary), pens + 0x40 + cpu * 0x10);
> +
> +    iounmap(pens);
> +
> +    return 0;
> +}
> +
>   static const char * const midway_dt_compat[] __initconst =
>   {
>       "calxeda,ecx-2000",
> @@ -57,6 +78,7 @@ PLATFORM_START(midway, "CALXEDA MIDWAY")
>       .compatible = midway_dt_compat,
>       .reset = midway_reset,
>       .quirks = midway_quirks,
> +    .cpu_up = midway_cpu_up,
>   PLATFORM_END
>
>   /*
>