| Message ID | 1384781260-10429-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
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 >
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 > > /*
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 > > /* >
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 > > > > /* > > >
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 /*
Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/platforms/midway.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)