On Tue, Feb 9, 2016 at 6:19 AM, Ian Campbell <ian.campb...@citrix.com>
wrote:
> On Tue, 2016-02-09 at 04:50 -0800, Suriyan Ramasami wrote:
> >
> >
> > On Tue, Feb 9, 2016 at 1:53 AM, Ian Campbell <ian.campb...@citrix.com>
> > wrote:
> > > On Mon, 2016-02-08 at 21:48 -0800, Suriyan Ramasami wrote:
> > > > The Odroid-XU3/XU4 from hardkernel is an Exynos 5422 based board.
> > > > Code from mcpm-exynos.c and mcpm-platsmp.c from the Linux kernel
> > > > has been used to get all the 8 cores from the 2 clusters powered
> > > > on.
> > > > The Linux DTS for these odroid uses "samsung,exynos5800" as
> > > > the machine compatible string. Hence, the same is used herein.
> > > >
> > > > This change has been tested on the Odroid-XU/XU3/XU4.
> > > >
> > > > Signed-off-by: Suriyan Ramasami <suriya...@gmail.com>
> > >
> > > Thanks, this now looks good to me, with one question about a comment
> > > which
> > > I could resolve upon commit if we agree a wording.
> > >
> > > > ---
> > > > Changes between versions as follows:
> > > >
> > > > v2:
> > > > Try to use common code as much as possible
> > > >
> > > > v1:
> > > > Initial code submission
> > > > ---
> > > > xen/arch/arm/platforms/exynos5.c | 61
> > > > ++++++++++++++++++++++++++++++++++++++--
> > > > 1 file changed, 58 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/xen/arch/arm/platforms/exynos5.c
> > > > b/xen/arch/arm/platforms/exynos5.c
> > > > index bf4964d..12aea31 100644
> > > > --- a/xen/arch/arm/platforms/exynos5.c
> > > > +++ b/xen/arch/arm/platforms/exynos5.c
> > > > @@ -34,9 +34,18 @@ static bool_t secure_firmware;
> > > > #define EXYNOS_ARM_CORE_CONFIG(_nr) (EXYNOS_ARM_CORE0_CONFIG + (0x80
> > > *
> > > > (_nr)))
> > > > #define EXYNOS_ARM_CORE_STATUS(_nr) (EXYNOS_ARM_CORE_CONFIG(_nr) +
> > > 0x4)
> > > > #define S5P_CORE_LOCAL_PWR_EN 0x3
> > > > +#define S5P_PMU_SPARE2 0x908
> > > >
> > > > #define SMC_CMD_CPU1BOOT (-4)
> > > >
> > > > +#define EXYNOS5800_CPUS_PER_CLUSTER 4
> > > > +
> > > > +#define EXYNOS5420_KFC_CORE_RESET0 BIT(8)
> > > > +#define EXYNOS5420_KFC_ETM_RESET0 BIT(20)
> > > > +
> > > > +#define EXYNOS5420_KFC_CORE_RESET(_nr) \
> > > > + ((EXYNOS5420_KFC_CORE_RESET0 | EXYNOS5420_KFC_ETM_RESET0) <<
> > > > (_nr))
> > > > +
> > > > static int exynos5_init_time(void)
> > > > {
> > > > uint32_t reg;
> > > > @@ -166,14 +175,23 @@ static void exynos_cpu_power_up(void __iomem
> > > > *power, int cpu)
> > > > static int exynos5_cpu_power_up(void __iomem *power, int cpu)
> > > > {
> > > > unsigned int timeout;
> > > > + unsigned int mpidr, pcpu, pcluster, cpunr;
> > > > +
> > > > + mpidr = cpu_logical_map(cpu);
> > > > + pcpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> > > > + pcluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> > > >
> > > > - if ( !exynos_cpu_power_state(power, cpu) )
> > > > + cpunr = pcpu + (pcluster * EXYNOS5800_CPUS_PER_CLUSTER);
> > > > + dprintk(XENLOG_DEBUG, "cpu: %d pcpu: %d, cluster: %d cpunr:
> > > %d\n",
> > > > + cpu, pcpu, pcluster, cpunr);
> > > > +
> > > > + if ( !exynos_cpu_power_state(power, cpunr) )
> > > > {
> > > > - exynos_cpu_power_up(power, cpu);
> > > > + exynos_cpu_power_up(power, cpunr);
> > > > timeout = 10;
> > > >
> > > > /* wait max 10 ms until cpu is on */
> > > > - while ( exynos_cpu_power_state(power, cpu) !=
> > > > S5P_CORE_LOCAL_PWR_EN )
> > > > + while ( exynos_cpu_power_state(power, cpunr) !=
> > > > S5P_CORE_LOCAL_PWR_EN )
> > > > {
> > > > if ( timeout-- == 0 )
> > > > break;
> > > > @@ -186,6 +204,42 @@ static int exynos5_cpu_power_up(void __iomem
> > > *power,
> > > > int cpu)
> > > > dprintk(XENLOG_ERR, "CPU%d power enable failed\n", cpu);
> > > > return -ETIMEDOUT;
> > > > }
> > > > +
> > > > + /*
> > > > + * This assumes the cluster number of the big cores(Cortex
> > > A15)
> > > > + * is 0 and the Little cores(Cortex A7) is 1.
> > > > + * When the system was booted from the Little core,
> > > > + * they should be reset during power up cpu.
> > > > + * Note that the below condition is true for Odroid XU3/XU4,
> > > and
> > > > + * false for the XU and the Exynos5800 based boards.
> > >
> > > I think the SoC is more relevant/useful in this context than specific
> > > boards. So I think what it is trying to say here is that for systems
> > > matching samsung,exynos5410 pcluster will always be zero, where as for
> > > ones
> > > matching samsung,exynos5800 it can be non-zero, and for non-zero
> > > clusters
> > > we need to do some extra bringup.
> > >
> > I believe for some SoCs its board based (5422 = pin controlled) and for
> > others its purely SoC based (5420 = A7 boot, 5800 = A15 boot). For
> > example, please look at this post: https://lkml.org/lkml/2015/12/11/107
> > which mentions something along those lines.
> > [...]
> > I agree on the first two paragraphs.
> > For the third paragraph, the rebuttal is that the exynos5800 and
> > exynos5422 based SoCs can have both clusters on at the same time. Hence,
> > the third paragrapah comment will have to be tweaked further. Possibly
> > reading:
> > The exynos5800 and exynos5422 can have both clusters on at the same time.
> > The exynos5800 boots up with cpu0 on cluster0 (A15). The exynos5422 can
> > boot up on either clusters as its pin controlled. In this case the DTS
> > should properly reflect the cpu order.
>
> Does the OS need to be aware of all these combinations though? Is it not
> sufficient to know how to bring up an A15 core and how to bring up an A7
> core and then just do so based on the information in the DTS, without
> needing to worry about which sort of core we happened to have booted on?
>
>
Unfortunately, at least looking at the boot up code for the Exynos5422, the
OS needs to be aware of it. This is what I see in the linux source code. If
it boots up on an A7, then a special reset is needed which is not needed
when booted up otherwise. I do not have much more details on that other
than the Linux code.
Without that reset sequence, I have also verified that the powered on CPU
does not come up.
> > The exynos5410 can have only one cluster on at a time, and it boots up
> > with pcluster == 0.
> > Any tweaks and comments on the above is appreciated.
>
> How much of this is down to physical h/w limitations and how much of it is
> down to firmware or software limitations? Can you flip the to the other
> cluster somehow?
>
>
The 5410 boots up on an A15, and only those 4 A15 CPUs in cluster 0 are
brought up. Hence, no flipping is required. I am wondering if your question
was aimed at the 5422 (Odroid XU3/XU4). For the XU3/XU4, to have only 4
A15s up, its possible by a change in u-boot, where one could power on an
A15 and then turn the A7 boot cpu off. I haven't tried it and I do not know
if there are other issues along the way.
> > > I have one other question -- does this patch result in a Xen system
> > > which
> > > consists of both A7 and A15 pcpus being live at the same time? Does
> > > that
> > > actually work given the subtle differences in things like the cache
> > > line
> > > length? I would expect there to need to be other patches to support a
> > > heterogeneous core configuration.
> > >
> > Yes, it does get both A7 and A15 clusters up at the same time. I am sure
> > that there needs to be patches to support this heterogenous core
> > configuration, but if we keep the doms pinned to the same CPU type then I
> > haven't faced any issues.
> > For example if I had a domU pinned to CPU 3 (A7) and CPU 4 (A15) then I
> > faced hangs.
>
> If that's the case then I'm sorry but I'm rather wary of taking this patch
> as it is, since the defaults are all to allow vCPUs to migrate around, so
> people are just going to get a broken system out of the box.
>
>
I totally agree with you. This does need some work, and thanks for the
suggestions you have provided below.
> Until actual support for heterogeneous cores is in place I'd much prefer to
> only bring up one cluster, I don't mind if it is
>
> Always the A7s
> Always the A15s
> Always whatever the system has booted on
> Somehow configurable at boot time by the user.
>
> Another option, which is a bit more work but not huge, would be to
> automatically partition the system into 2 cpupools at boot, corresponding
> to the two clusters of CPUs and ensure that dom0 is resident in only one
> cpu pool, I think that would be the minimally workable arrangement for a
> heterogeneous system.
>
>
I like this suggestion. I shall pursue this option, and work on a Patch
version 3. Thanks so much for your input. It is much appreciated!
> Ian.
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
