* Re: [PATCH] xen/arm: midway: implement SMP
2013-11-18 13:27 [PATCH] xen/arm: midway: implement SMP Julien Grall
@ 2013-11-18 15:47 ` Stefano Stabellini
2013-11-19 14:52 ` Ian Campbell
2013-11-19 15:24 ` Andre Przywara
2 siblings, 0 replies; 5+ messages in thread
From: Stefano Stabellini @ 2013-11-18 15:47 UTC (permalink / raw)
To: Julien Grall
Cc: ian.campbell, stefano.stabellini, patches, tim, andre.przywara,
xen-devel
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] xen/arm: midway: implement SMP
2013-11-18 13:27 [PATCH] xen/arm: midway: implement SMP Julien Grall
2013-11-18 15:47 ` Stefano Stabellini
@ 2013-11-19 14:52 ` Ian Campbell
2013-11-19 15:24 ` Andre Przywara
2 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2013-11-19 14:52 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini, tim, andre.przywara, patches
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
>
> /*
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] xen/arm: midway: implement SMP
2013-11-18 13:27 [PATCH] xen/arm: midway: implement SMP Julien Grall
2013-11-18 15:47 ` Stefano Stabellini
2013-11-19 14:52 ` Ian Campbell
@ 2013-11-19 15:24 ` Andre Przywara
2013-11-19 15:46 ` Ian Campbell
2 siblings, 1 reply; 5+ messages in thread
From: Andre Przywara @ 2013-11-19 15:24 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, stefano.stabellini, tim, ian.campbell, patches
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
>
> /*
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] xen/arm: midway: implement SMP
2013-11-19 15:24 ` Andre Przywara
@ 2013-11-19 15:46 ` Ian Campbell
0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2013-11-19 15:46 UTC (permalink / raw)
To: Andre Przywara; +Cc: xen-devel, Julien Grall, tim, stefano.stabellini, patches
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
> >
> > /*
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread