xen/arm: midway: implement SMP

Message ID 1384781260-10429-1-git-send-email-julien.grall@linaro.org
State Rejected
Headers show

Commit Message

Julien Grall Nov. 18, 2013, 1:27 p.m.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/platforms/midway.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Stefano Stabellini Nov. 18, 2013, 3:47 p.m. | #1
On Mon, 18 Nov 2013, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>


>  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)
> +{
> +    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
>  
>  /*
> -- 
> 1.8.3.1
>
Ian Campbell Nov. 19, 2013, 2:52 p.m. | #2
On Mon, 2013-11-18 at 13:27 +0000, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I'd very much like to see Andre's Ack too.

Is anyone thinking of implementing host PSCI in the near future?

> ---
>  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)
> +{
> +    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
>  
>  /*
Andre Przywara Nov. 19, 2013, 3:24 p.m. | #3
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
>
>   /*
>
Ian Campbell Nov. 19, 2013, 3:46 p.m. | #4
On Tue, 2013-11-19 at 16:24 +0100, Andre Przywara wrote:
> 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?)

This means that we use the core codes CPU enumeration and don't need to
repeat it in smp_init.

> P.S. BTW: Any reason we instantiate cpu_up() for all platforms where 
> they are actually NOPs?

When I initially started building this infrastructure there was stuff
there, but it went away during development and so far it seems to have
turned out to be unnecessary. This is largely because on all the
platforms we supported so far the generic sev suffices to kick acpu out
of either the firmware or Xen's holding pen.

This hook will be needed for PSCI, hopefully sooner rather than later.

>  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.

Could do.

> 
> > +{
> > +    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
> >
> >   /*
> >
>

Patch

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)
+{
+    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
 
 /*