* [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
@ 2016-02-09 5:48 Suriyan Ramasami
2016-02-09 9:53 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Suriyan Ramasami @ 2016-02-09 5:48 UTC (permalink / raw)
To: xen-devel; +Cc: Suriyan Ramasami, stefano.stabellini, ian.campbell
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 <suriyan.r@gmail.com>
---
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.
+ */
+ if ( pcluster &&
+ pcluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1) ) {
+ /*
+ * Before we reset the Little cores, we should wait
+ * the SPARE2 register is set to 1 because the init
+ * codes of the iROM will set the register after
+ * initialization.
+ */
+
+ /* wait max 10ms for the spare register to be set to 1 */
+ timeout = 10;
+ while ( !__raw_readl(power + S5P_PMU_SPARE2) )
+ {
+ if ( timeout-- == 0 )
+ break;
+
+ mdelay(1);
+ }
+
+ if ( timeout == 0 )
+ {
+ dprintk(XENLOG_ERR, "CPU%d SPARE2 register wait failed\n", cpu);
+ return -ETIMEDOUT;
+ }
+ __raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
+ power + EXYNOS5_SWRESET);
+ }
}
return 0;
}
@@ -298,6 +352,7 @@ static const char * const exynos5250_dt_compat[] __initconst =
static const char * const exynos5_dt_compat[] __initconst =
{
"samsung,exynos5410",
+ "samsung,exynos5800",
NULL
};
--
2.5.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-09 5:48 [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support Suriyan Ramasami
@ 2016-02-09 9:53 ` Ian Campbell
2016-02-09 12:50 ` Suriyan Ramasami
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2016-02-09 9:53 UTC (permalink / raw)
To: Suriyan Ramasami, xen-devel; +Cc: stefano.stabellini
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 <suriyan.r@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 think the comment should therefore focus on the SoC (maybe giving some
examples of systems, but such a list cannot be exhaustive and could be
omitted IMHO). How about:
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 only exynos5800 based SoCs have a pcluster==1 of little cores,
for exynos5410 there is only pcluster==0.
?
I would also consider perhaps moving the comment up to where pcluster is
calculated, and in particular near the:
cpunr = pcpu + (pcluster * EXYNOS5800_CPUS_PER_CLUSTER);
Since this also relies on pcluster==0 for !5800 systems.
What do you think? I could adjust the comment wording and placement on
commit if you are happy.
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.
Hopefully I've just misunderstood and the effect of this patch is to switch
to just running the A15 processors even after booting on the A7s.
Ian.
> + */
> + if ( pcluster &&
> + pcluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1) ) {
> + /*
> + * Before we reset the Little cores, we should wait
> + * the SPARE2 register is set to 1 because the init
> + * codes of the iROM will set the register after
> + * initialization.
> + */
> +
> + /* wait max 10ms for the spare register to be set to 1 */
> + timeout = 10;
> + while ( !__raw_readl(power + S5P_PMU_SPARE2) )
> + {
> + if ( timeout-- == 0 )
> + break;
> +
> + mdelay(1);
> + }
> +
> + if ( timeout == 0 )
> + {
> + dprintk(XENLOG_ERR, "CPU%d SPARE2 register wait
> failed\n", cpu);
> + return -ETIMEDOUT;
> + }
> + __raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
> + power + EXYNOS5_SWRESET);
> + }
> }
> return 0;
> }
> @@ -298,6 +352,7 @@ static const char * const exynos5250_dt_compat[]
> __initconst =
> static const char * const exynos5_dt_compat[] __initconst =
> {
> "samsung,exynos5410",
> + "samsung,exynos5800",
> NULL
> };
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-09 9:53 ` Ian Campbell
@ 2016-02-09 12:50 ` Suriyan Ramasami
2016-02-09 14:19 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Suriyan Ramasami @ 2016-02-09 12:50 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 8159 bytes --]
On Tue, Feb 9, 2016 at 1:53 AM, Ian Campbell <ian.campbell@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 <suriyan.r@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 think the comment should therefore focus on the SoC (maybe giving some
> examples of systems, but such a list cannot be exhaustive and could be
> omitted IMHO). How about:
>
> 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 only exynos5800 based SoCs have a pcluster==1 of little
> cores,
> for exynos5410 there is only pcluster==0.
>
> ?
>
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. 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.
> I would also consider perhaps moving the comment up to where pcluster is
> calculated, and in particular near the:
>
> cpunr = pcpu + (pcluster * EXYNOS5800_CPUS_PER_CLUSTER);
>
> Since this also relies on pcluster==0 for !5800 systems.
>
> What do you think? I could adjust the comment wording and placement on
> commit if you are happy.
>
>
I am definitely OK with moving the comment to where you suggest.
> 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.
> Hopefully I've just misunderstood and the effect of this patch is to switch
> to just running the A15 processors even after booting on the A7s.
>
> Ian.
>
>
> > + */
> > + if ( pcluster &&
> > + pcluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1) ) {
> > + /*
> > + * Before we reset the Little cores, we should wait
> > + * the SPARE2 register is set to 1 because the init
> > + * codes of the iROM will set the register after
> > + * initialization.
> > + */
> > +
> > + /* wait max 10ms for the spare register to be set to 1 */
> > + timeout = 10;
> > + while ( !__raw_readl(power + S5P_PMU_SPARE2) )
> > + {
> > + if ( timeout-- == 0 )
> > + break;
> > +
> > + mdelay(1);
> > + }
> > +
> > + if ( timeout == 0 )
> > + {
> > + dprintk(XENLOG_ERR, "CPU%d SPARE2 register wait
> > failed\n", cpu);
> > + return -ETIMEDOUT;
> > + }
> > + __raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
> > + power + EXYNOS5_SWRESET);
> > + }
> > }
> > return 0;
> > }
> > @@ -298,6 +352,7 @@ static const char * const exynos5250_dt_compat[]
> > __initconst =
> > static const char * const exynos5_dt_compat[] __initconst =
> > {
> > "samsung,exynos5410",
> > + "samsung,exynos5800",
> > NULL
> > };
> >
>
[-- Attachment #1.2: Type: text/html, Size: 11339 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-09 12:50 ` Suriyan Ramasami
@ 2016-02-09 14:19 ` Ian Campbell
2016-02-09 18:20 ` Suriyan Ramasami
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2016-02-09 14:19 UTC (permalink / raw)
To: Suriyan Ramasami; +Cc: stefano.stabellini, xen-devel
On Tue, 2016-02-09 at 04:50 -0800, Suriyan Ramasami wrote:
>
>
> On Tue, Feb 9, 2016 at 1:53 AM, Ian Campbell <ian.campbell@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 <suriyan.r@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?
> 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?
> > 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.
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.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-09 14:19 ` Ian Campbell
@ 2016-02-09 18:20 ` Suriyan Ramasami
2016-02-10 10:03 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Suriyan Ramasami @ 2016-02-09 18:20 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 8821 bytes --]
On Tue, Feb 9, 2016 at 6:19 AM, Ian Campbell <ian.campbell@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.campbell@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 <suriyan.r@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.
>
[-- Attachment #1.2: Type: text/html, Size: 12220 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-09 18:20 ` Suriyan Ramasami
@ 2016-02-10 10:03 ` Ian Campbell
2016-02-11 1:47 ` Suriyan Ramasami
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2016-02-10 10:03 UTC (permalink / raw)
To: Suriyan Ramasami; +Cc: stefano.stabellini, xen-devel
On Tue, 2016-02-09 at 10:20 -0800, Suriyan Ramasami wrote:
> 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.
Are we able to say that if we are booted on cluster 1 (always the A7s) then
we always need this magic reset? i.e. is true for all SoCs which have an A7
cluster and can boot from it? (it's tautologically true for SocS which
either have no A7's or cannot boot from them).
Maybe I'm looking for similarities between different exynos variants which
doesn't exist though. If we are going to talk about specific SoCs in the
comments then I would rather that the code was also explicit rather than
assuming cluster 1 will only be found on the 5800, that might be as simple
as mapping the compatible string to a max_cluster (default 0 for unknown
SoC) and warning if pcluster > max_cluster.
>
> > > 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.
What I meant was, given that the 5410 has a cluster of A15 and a cluster of
A7s (right?) and you can only have one on at a time, how does an OS make
use of the A7s if it wants to? As you say it boots from the A15, so how can
the A7s be used?
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-10 10:03 ` Ian Campbell
@ 2016-02-11 1:47 ` Suriyan Ramasami
2016-02-11 9:40 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Suriyan Ramasami @ 2016-02-11 1:47 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 3909 bytes --]
On Wed, Feb 10, 2016 at 2:03 AM, Ian Campbell <ian.campbell@citrix.com>
wrote:
> On Tue, 2016-02-09 at 10:20 -0800, Suriyan Ramasami wrote:
> > 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.
>
> Are we able to say that if we are booted on cluster 1 (always the A7s) then
> we always need this magic reset? i.e. is true for all SoCs which have an A7
> cluster and can boot from it? (it's tautologically true for SocS which
> either have no A7's or cannot boot from them).
>
>
I do not have the information to answer the question. I am limited to what
I know (albeit a little bit) wrt the hardkernel related boards - Exynos
5410 (odroid-XU) and the Exynos 5422 (Odoird XU3/XU4). With my limited
knowledge, I am only aware of Exynos 5410 which is capable of booting off
of an A7 or an A15.
> Maybe I'm looking for similarities between different exynos variants which
> doesn't exist though. If we are going to talk about specific SoCs in the
> comments then I would rather that the code was also explicit rather than
> assuming cluster 1 will only be found on the 5800, that might be as simple
> as mapping the compatible string to a max_cluster (default 0 for unknown
> SoC) and warning if pcluster > max_cluster.
>
Can you please elaborate on the mapping that you talk about above. I am
lost here :-(
>
> >
> > > > 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.
>
> What I meant was, given that the 5410 has a cluster of A15 and a cluster of
> A7s (right?) and you can only have one on at a time, how does an OS make
> use of the A7s if it wants to? As you say it boots from the A15, so how can
> the A7s be used?
>
> The Linux OS has a bL (big - little) switcher module/code which handles
that. It maps one big core to one little core, and when the load is not
high, it switches off the big cluster, and turns on the small cluster -
AFAICT.
Also, are we still on wrt the two cpu pool suggestion and to have 4 cores
from cluster 0 in cpupool0 and 4 cores from cluster 1 in cpupool1. It would
be great if you can point me to some code as well. I have been looking at
cpupool.c and also on the system call interface that it provides.
> Ian.
>
[-- Attachment #1.2: Type: text/html, Size: 5236 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-11 1:47 ` Suriyan Ramasami
@ 2016-02-11 9:40 ` Ian Campbell
2016-02-15 6:32 ` Suriyan Ramasami
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2016-02-11 9:40 UTC (permalink / raw)
To: Suriyan Ramasami; +Cc: stefano.stabellini, xen-devel
On Wed, 2016-02-10 at 17:47 -0800, Suriyan Ramasami wrote:
>
>
> On Wed, Feb 10, 2016 at 2:03 AM, Ian Campbell <ian.campbell@citrix.com>
> wrote:
> > On Tue, 2016-02-09 at 10:20 -0800, Suriyan Ramasami wrote:
> > > 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.
> >
> > Are we able to say that if we are booted on cluster 1 (always the A7s)
> > then
> > we always need this magic reset? i.e. is true for all SoCs which have
> > an A7
> > cluster and can boot from it? (it's tautologically true for SocS which
> > either have no A7's or cannot boot from them).
> >
> I do not have the information to answer the question. I am limited to
> what I know (albeit a little bit) wrt the hardkernel related boards -
> Exynos 5410 (odroid-XU) and the Exynos 5422 (Odoird XU3/XU4). With my
> limited knowledge, I am only aware of Exynos 5410 which is capable of
> booting off of an A7 or an A15.
>
> > Maybe I'm looking for similarities between different exynos variants
> > which
> > doesn't exist though. If we are going to talk about specific SoCs in
> > the
> > comments then I would rather that the code was also explicit rather
> > than
> > assuming cluster 1 will only be found on the 5800, that might be as
> > simple
> > as mapping the compatible string to a max_cluster (default 0 for
> > unknown
> > SoC) and warning if pcluster > max_cluster.
> Can you please elaborate on the mapping that you talk about above. I am
> lost here :-(
What I mean is can we say:
exynos 1234 => Two clusters (max_cluster == 1)
exynos 5678 => One cluster (max_cluster == 0)
exynos ABCD => Two clusters (max_cluster == 1)
Unknown => Assume one cluster
and can we also assume that cluster 0 always consists of A15s and cluster 1
(if it exists) always consists of A7s?
If so then we can say:
max_cluster = look_up_by_compat(compat)
pcluster = figure out from midr
pcpu = figure it out
if (pcluster >= max_cluster)
error
do bringup
if (pluster == 1)
do special handling for cluster 1 == a7
The difference compared with what you have is that it adds a check that we
expect a second cluster for the SoC before it goes poking at stuff.
What I'm trying to avoid is coming across some other SoC variant which has
2 clusters but has something different to the A7s or which requires some
different handling.
If we were confident that all exynosXXXX SoCs always require the same
special handling for cluster 1 then we wouldn't really need this, but I
don't think we know that?
>
> >
> > >
> > > > > 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.
> >
> > What I meant was, given that the 5410 has a cluster of A15 and a
> > cluster of
> > A7s (right?) and you can only have one on at a time, how does an OS
> > make
> > use of the A7s if it wants to? As you say it boots from the A15, so how
> > can
> > the A7s be used?
> >
> >
> The Linux OS has a bL (big - little) switcher module/code which handles
> that. It maps one big core to one little core, and when the load is not
> high, it switches off the big cluster, and turns on the small cluster -
> AFAICT.
So this is an OS limitation, not a h/w one? What's to stop an OS from
brninging up the A15s and the A7s at the same time?
> Also, are we still on wrt the two cpu pool suggestion and to have 4 cores
> from cluster 0 in cpupool0 and 4 cores from cluster 1 in cpupool1. It
> would be great if you can point me to some code as well. I have been
> looking at cpupool.c and also on the system call interface that it
> provides.
I'm afraid I'm not really very familiar with this side of things myself :-(
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-11 9:40 ` Ian Campbell
@ 2016-02-15 6:32 ` Suriyan Ramasami
2016-02-16 10:03 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Suriyan Ramasami @ 2016-02-15 6:32 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 9803 bytes --]
On Thu, Feb 11, 2016 at 1:40 AM, Ian Campbell <ian.campbell@citrix.com>
wrote:
> On Wed, 2016-02-10 at 17:47 -0800, Suriyan Ramasami wrote:
> >
> >
> > On Wed, Feb 10, 2016 at 2:03 AM, Ian Campbell <ian.campbell@citrix.com>
> > wrote:
> > > On Tue, 2016-02-09 at 10:20 -0800, Suriyan Ramasami wrote:
> > > > 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.
> > >
> > > Are we able to say that if we are booted on cluster 1 (always the A7s)
> > > then
> > > we always need this magic reset? i.e. is true for all SoCs which have
> > > an A7
> > > cluster and can boot from it? (it's tautologically true for SocS which
> > > either have no A7's or cannot boot from them).
> > >
> > I do not have the information to answer the question. I am limited to
> > what I know (albeit a little bit) wrt the hardkernel related boards -
> > Exynos 5410 (odroid-XU) and the Exynos 5422 (Odoird XU3/XU4). With my
> > limited knowledge, I am only aware of Exynos 5410 which is capable of
> > booting off of an A7 or an A15.
> >
> > > Maybe I'm looking for similarities between different exynos variants
> > > which
> > > doesn't exist though. If we are going to talk about specific SoCs in
> > > the
> > > comments then I would rather that the code was also explicit rather
> > > than
> > > assuming cluster 1 will only be found on the 5800, that might be as
> > > simple
> > > as mapping the compatible string to a max_cluster (default 0 for
> > > unknown
> > > SoC) and warning if pcluster > max_cluster.
> > Can you please elaborate on the mapping that you talk about above. I am
> > lost here :-(
>
> What I mean is can we say:
> exynos 1234 => Two clusters (max_cluster == 1)
> exynos 5678 => One cluster (max_cluster == 0)
> exynos ABCD => Two clusters (max_cluster == 1)
> Unknown => Assume one cluster
>
> and can we also assume that cluster 0 always consists of A15s and cluster 1
> (if it exists) always consists of A7s?
>
> If so then we can say:
>
> max_cluster = look_up_by_compat(compat)
> pcluster = figure out from midr
> pcpu = figure it out
>
> if (pcluster >= max_cluster)
> error
>
> do bringup
>
> if (pluster == 1)
> do special handling for cluster 1 == a7
>
> The difference compared with what you have is that it adds a check that we
> expect a second cluster for the SoC before it goes poking at stuff.
>
> What I'm trying to avoid is coming across some other SoC variant which has
> 2 clusters but has something different to the A7s or which requires some
> different handling.
>
> If we were confident that all exynosXXXX SoCs always require the same
> special handling for cluster 1 then we wouldn't really need this, but I
> don't think we know that?
>
>
I did some looking at the linux 4.4.y dts for exynos, and this is what I
see:
exynos3250: cpu0, cpu1 = A7 (1 cluster)
exynos4210: cpu0, cpu1 = A9 (1 cluster)
exynos4212: cpu0, cpu1 = A9 (1 cluster)
exynos4412: cpu0, cpu1, cpu2, cpu3 = A9 (1 cluster)
exynos4415: cpu0, cpu1, cpu2, cpu3 = A9 (1 cluster)
exynos5250: cpu0, cpu1 = A15 (1 cluster)
exynos5260: cpu0, cpu1 = A15. cpu2, cpu3, cpu4, cpu5 = A7 (2 clusters)
exynos5410: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster - even thougg it has 2
clusters, but not both can be on at same time)
exynos5420: cpu0, cpu1, cpu2, cpu3 = A15. cpu4, cpu5, cpu6, cpu7 = A7 (2
clusters)
exynos5422: cpu0, cpu1, cpu2, cpu3 = A7. cpu4, cpu5, cpu6, cpu7 = A15 (2
clusters)
exynos5440: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster)
If we look at the ones that can potentially support hardware
virtualization, limits us to the A7s and the A15s. That gives us:
exynos3250: cpu0, cpu1 = A7 (1 cluster)
exynos5250: cpu0, cpu1 = A15 (1 cluster)
exynos5260: cpu0, cpu1 = A15. cpu2, cpu3, cpu4, cpu5 = A7 (2 clusters)
exynos5410: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster)
exynos5420: cpu0, cpu1, cpu2, cpu3 = A15. cpu4, cpu5, cpu6, cpu7 = A7 (2
clusters)
exynos5422: cpu0, cpu1, cpu2, cpu3 = A7. cpu4, cpu5, cpu6, cpu7 = A15 (2
clusters)
exynos5440: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster)
Of these the only ones which has A7 as the 1st cluster are:
exynos3250: cpu0, cpu1 = A7
exynos5420: cpu0, cpu1, cpu2, cpu3 = A15. cpu4, cpu5, cpu6, cpu7 = A7 (2
clusters)
Note that the exynos5800 has the same cpu config as the exysno5420.
I was looking at the cpu bring up code, and notice that if the secondary
cpu being brought up is an A7, then it invariably does the below:
For the exynos3250: in platsmp.c
void exynos_core_restart(u32 core_id)
{
u32 val;
if (!of_machine_is_compatible("samsung,exynos3250"))
return;
while (!pmu_raw_readl(S5P_PMU_SPARE2))
udelay(10);
udelay(10);
val = pmu_raw_readl(EXYNOS_ARM_CORE_STATUS(core_id));
val |= S5P_CORE_WAKEUP_FROM_LOCAL_CFG;
pmu_raw_writel(val, EXYNOS_ARM_CORE_STATUS(core_id));
pmu_raw_writel(EXYNOS_CORE_PO_RESET(core_id), EXYNOS_SWRESET);
}
And for the exynos5800/exynos5422: mcpm-exynos.c
static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
{
unsigned int cpunr = cpu + (cluster * EXYNOS5420_CPUS_PER_CLUSTER);
pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
if (cpu >= EXYNOS5420_CPUS_PER_CLUSTER ||
cluster >= EXYNOS5420_NR_CLUSTERS)
return -EINVAL;
if (!exynos_cpu_power_state(cpunr)) {
exynos_cpu_power_up(cpunr);
/*
* 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.
*/
if (cluster &&
cluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1))
{
/*
* Before we reset the Little cores, we should wait
* the SPARE2 register is set to 1 because the init
* codes of the iROM will set the register after
* initialization.
*/
while (!pmu_raw_readl(S5P_PMU_SPARE2))
udelay(10);
pmu_raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
EXYNOS_SWRESET);
}
}
return 0;
}
This pretty much indicates that when bringing up the A7s, the special reset
sequence is deemed essential. Probably, we could generalize that it might
be true for future exynos's having A7s.
> >
> > >
> > > >
> > > > > > 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.
> > >
> > > What I meant was, given that the 5410 has a cluster of A15 and a
> > > cluster of
> > > A7s (right?) and you can only have one on at a time, how does an OS
> > > make
> > > use of the A7s if it wants to? As you say it boots from the A15, so how
> > > can
> > > the A7s be used?
> > >
> > >
> > The Linux OS has a bL (big - little) switcher module/code which handles
> > that. It maps one big core to one little core, and when the load is not
> > high, it switches off the big cluster, and turns on the small cluster -
> > AFAICT.
>
> So this is an OS limitation, not a h/w one? What's to stop an OS from
> brninging up the A15s and the A7s at the same time?
>
> > Also, are we still on wrt the two cpu pool suggestion and to have 4 cores
> > from cluster 0 in cpupool0 and 4 cores from cluster 1 in cpupool1. It
> > would be great if you can point me to some code as well. I have been
> > looking at cpupool.c and also on the system call interface that it
> > provides.
>
> I'm afraid I'm not really very familiar with this side of things myself :-(
>
> Thanks, I shall dig into this some more.
> Ian.
>
[-- Attachment #1.2: Type: text/html, Size: 13553 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-15 6:32 ` Suriyan Ramasami
@ 2016-02-16 10:03 ` Ian Campbell
2016-02-17 2:24 ` Suriyan Ramasami
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2016-02-16 10:03 UTC (permalink / raw)
To: Suriyan Ramasami; +Cc: stefano.stabellini, xen-devel
On Sun, 2016-02-14 at 22:32 -0800, Suriyan Ramasami wrote:
>
>
> On Thu, Feb 11, 2016 at 1:40 AM, Ian Campbell <
> ian.campbell@citrix.com> wrote:
> > On Wed, 2016-02-10 at 17:47 -0800, Suriyan Ramasami wrote:
> > >
> > >
> > > On Wed, Feb 10, 2016 at 2:03 AM, Ian Campbell <
> > ian.campbell@citrix.com>
> > > wrote:
> > > > On Tue, 2016-02-09 at 10:20 -0800, Suriyan Ramasami wrote:
> > > > > 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.
> > > >
> > > > Are we able to say that if we are booted on cluster 1 (always
> > the A7s)
> > > > then
> > > > we always need this magic reset? i.e. is true for all SoCs
> > which have
> > > > an A7
> > > > cluster and can boot from it? (it's tautologically true for
> > SocS which
> > > > either have no A7's or cannot boot from them).
> > > >
> > > I do not have the information to answer the question. I am
> > limited to
> > > what I know (albeit a little bit) wrt the hardkernel related
> > boards -
> > > Exynos 5410 (odroid-XU) and the Exynos 5422 (Odoird XU3/XU4).
> > With my
> > > limited knowledge, I am only aware of Exynos 5410 which is
> > capable of
> > > booting off of an A7 or an A15.
> > >
> > > > Maybe I'm looking for similarities between different exynos
> > variants
> > > > which
> > > > doesn't exist though. If we are going to talk about specific
> > SoCs in
> > > > the
> > > > comments then I would rather that the code was also explicit
> > rather
> > > > than
> > > > assuming cluster 1 will only be found on the 5800, that might
> > be as
> > > > simple
> > > > as mapping the compatible string to a max_cluster (default 0
> > for
> > > > unknown
> > > > SoC) and warning if pcluster > max_cluster.
> > > Can you please elaborate on the mapping that you talk about
> > above. I am
> > > lost here :-(
> >
> > What I mean is can we say:
> > exynos 1234 => Two clusters (max_cluster == 1)
> > exynos 5678 => One cluster (max_cluster == 0)
> > exynos ABCD => Two clusters (max_cluster == 1)
> > Unknown => Assume one cluster
> >
> > and can we also assume that cluster 0 always consists of A15s and
> > cluster 1
> > (if it exists) always consists of A7s?
> >
> > If so then we can say:
> >
> > max_cluster = look_up_by_compat(compat)
> > pcluster = figure out from midr
> > pcpu = figure it out
> >
> > if (pcluster >= max_cluster)
> > error
> >
> > do bringup
> >
> > if (pluster == 1)
> > do special handling for cluster 1 == a7
> >
> > The difference compared with what you have is that it adds a check
> > that we
> > expect a second cluster for the SoC before it goes poking at stuff.
> >
> > What I'm trying to avoid is coming across some other SoC variant
> > which has
> > 2 clusters but has something different to the A7s or which requires
> > some
> > different handling.
> >
> > If we were confident that all exynosXXXX SoCs always require the
> > same
> > special handling for cluster 1 then we wouldn't really need this,
> > but I
> > don't think we know that?
> >
> >
> I did some looking at the linux 4.4.y dts for exynos, and this is
> what I see:
>
[...]
> If we look at the ones that can potentially support hardware
> virtualization, limits us to the A7s and the A15s. That gives us:
> exynos3250: cpu0, cpu1 = A7 (1 cluster)
> exynos5250: cpu0, cpu1 = A15 (1 cluster)
> exynos5260: cpu0, cpu1 = A15. cpu2, cpu3, cpu4, cpu5 = A7 (2
> clusters)
> exynos5410: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster)
> exynos5420: cpu0, cpu1, cpu2, cpu3 = A15. cpu4, cpu5, cpu6, cpu7 = A7
> (2 clusters)
> exynos5422: cpu0, cpu1, cpu2, cpu3 = A7. cpu4, cpu5, cpu6, cpu7 = A15
> (2 clusters)
> exynos5440: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster)
>
> Of these the only ones which has A7 as the 1st cluster are:
> exynos3250: cpu0, cpu1 = A7
> exynos5420: cpu0, cpu1, cpu2, cpu3 = A15. cpu4, cpu5, cpu6, cpu7 = A7
> (2 clusters)
Did you mean 5422 for this second one i.e.
exynos5422: cpu0, cpu1, cpu2, cpu3 = A7. cpu4, cpu5, cpu6, cpu7 = A15 (2 clusters)
? (I'm assuming you did)
In such configurations does the second cluster (A15s in this case) need
the same magic dance as you were adding for the A7s in the other cases?
> Note that the exynos5800 has the same cpu config as the exysno5420.
>
> I was looking at the cpu bring up code, and notice that if the secondary cpu being brought up is an A7, then it invariably does the below:
> For the exynos3250: in platsmp.c
> void exynos_core_restart(u32 core_id)
> {
> u32 val;
>
> if (!of_machine_is_compatible("samsung,exynos3250"))
> return;
>
> while (!pmu_raw_readl(S5P_PMU_SPARE2))
> udelay(10);
> udelay(10);
>
> val = pmu_raw_readl(EXYNOS_ARM_CORE_STATUS(core_id));
> val |= S5P_CORE_WAKEUP_FROM_LOCAL_CFG;
> pmu_raw_writel(val, EXYNOS_ARM_CORE_STATUS(core_id));
>
> pmu_raw_writel(EXYNOS_CORE_PO_RESET(core_id), EXYNOS_SWRESET);
> }
>
> And for the exynos5800/exynos5422: mcpm-exynos.c
> static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
> {
> unsigned int cpunr = cpu + (cluster * EXYNOS5420_CPUS_PER_CLUSTER);
>
> pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> if (cpu >= EXYNOS5420_CPUS_PER_CLUSTER ||
> cluster >= EXYNOS5420_NR_CLUSTERS)
> return -EINVAL;
>
> if (!exynos_cpu_power_state(cpunr)) {
> exynos_cpu_power_up(cpunr);
>
> /*
> * 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.
> */
> if (cluster &&
> cluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1)) {
> /*
> * Before we reset the Little cores, we should wait
> * the SPARE2 register is set to 1 because the init
> * codes of the iROM will set the register after
> * initialization.
> */
> while (!pmu_raw_readl(S5P_PMU_SPARE2))
> udelay(10);
>
> pmu_raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
> EXYNOS_SWRESET);
> }
> }
>
> return 0;
> }
>
> This pretty much indicates that when bringing up the A7s, the special
> reset sequence is deemed essential.
If (and only if) they are in the second cluster?
> Probably, we could generalize that it might be true for future
> exynos's having A7s.
This whole thing has turned into a bit of a tarpit, sorry :-/
I think I'd be happiest with code which explicitly checked for SoC
configurations which we know about (and ideally can test) over code
which tries to predict what future SoC variants will do -- we can
always patch them in later.
Essentially I think that just means adding some machine_is_compatible
type checks to the code you already have rather than relying on the
overall compatibility list only have a couple of entries right now and
implicitly relying on the differences between them (the
presence/absence of a second cluster in this case).
Ian.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support
2016-02-16 10:03 ` Ian Campbell
@ 2016-02-17 2:24 ` Suriyan Ramasami
0 siblings, 0 replies; 11+ messages in thread
From: Suriyan Ramasami @ 2016-02-17 2:24 UTC (permalink / raw)
To: Ian Campbell; +Cc: stefano.stabellini, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 9774 bytes --]
On Tue, Feb 16, 2016 at 2:03 AM, Ian Campbell <ian.campbell@citrix.com>
wrote:
> On Sun, 2016-02-14 at 22:32 -0800, Suriyan Ramasami wrote:
> >
> >
> > On Thu, Feb 11, 2016 at 1:40 AM, Ian Campbell <
> > ian.campbell@citrix.com> wrote:
> > > On Wed, 2016-02-10 at 17:47 -0800, Suriyan Ramasami wrote:
> > > >
> > > >
> > > > On Wed, Feb 10, 2016 at 2:03 AM, Ian Campbell <
> > > ian.campbell@citrix.com>
> > > > wrote:
> > > > > On Tue, 2016-02-09 at 10:20 -0800, Suriyan Ramasami wrote:
> > > > > > 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.
> > > > >
> > > > > Are we able to say that if we are booted on cluster 1 (always
> > > the A7s)
> > > > > then
> > > > > we always need this magic reset? i.e. is true for all SoCs
> > > which have
> > > > > an A7
> > > > > cluster and can boot from it? (it's tautologically true for
> > > SocS which
> > > > > either have no A7's or cannot boot from them).
> > > > >
> > > > I do not have the information to answer the question. I am
> > > limited to
> > > > what I know (albeit a little bit) wrt the hardkernel related
> > > boards -
> > > > Exynos 5410 (odroid-XU) and the Exynos 5422 (Odoird XU3/XU4).
> > > With my
> > > > limited knowledge, I am only aware of Exynos 5410 which is
> > > capable of
> > > > booting off of an A7 or an A15.
> > > >
> > > > > Maybe I'm looking for similarities between different exynos
> > > variants
> > > > > which
> > > > > doesn't exist though. If we are going to talk about specific
> > > SoCs in
> > > > > the
> > > > > comments then I would rather that the code was also explicit
> > > rather
> > > > > than
> > > > > assuming cluster 1 will only be found on the 5800, that might
> > > be as
> > > > > simple
> > > > > as mapping the compatible string to a max_cluster (default 0
> > > for
> > > > > unknown
> > > > > SoC) and warning if pcluster > max_cluster.
> > > > Can you please elaborate on the mapping that you talk about
> > > above. I am
> > > > lost here :-(
> > >
> > > What I mean is can we say:
> > > exynos 1234 => Two clusters (max_cluster == 1)
> > > exynos 5678 => One cluster (max_cluster == 0)
> > > exynos ABCD => Two clusters (max_cluster == 1)
> > > Unknown => Assume one cluster
> > >
> > > and can we also assume that cluster 0 always consists of A15s and
> > > cluster 1
> > > (if it exists) always consists of A7s?
> > >
> > > If so then we can say:
> > >
> > > max_cluster = look_up_by_compat(compat)
> > > pcluster = figure out from midr
> > > pcpu = figure it out
> > >
> > > if (pcluster >= max_cluster)
> > > error
> > >
> > > do bringup
> > >
> > > if (pluster == 1)
> > > do special handling for cluster 1 == a7
> > >
> > > The difference compared with what you have is that it adds a check
> > > that we
> > > expect a second cluster for the SoC before it goes poking at stuff.
> > >
> > > What I'm trying to avoid is coming across some other SoC variant
> > > which has
> > > 2 clusters but has something different to the A7s or which requires
> > > some
> > > different handling.
> > >
> > > If we were confident that all exynosXXXX SoCs always require the
> > > same
> > > special handling for cluster 1 then we wouldn't really need this,
> > > but I
> > > don't think we know that?
> > >
> > >
> > I did some looking at the linux 4.4.y dts for exynos, and this is
> > what I see:
> >
> [...]
> > If we look at the ones that can potentially support hardware
> > virtualization, limits us to the A7s and the A15s. That gives us:
> > exynos3250: cpu0, cpu1 = A7 (1 cluster)
> > exynos5250: cpu0, cpu1 = A15 (1 cluster)
> > exynos5260: cpu0, cpu1 = A15. cpu2, cpu3, cpu4, cpu5 = A7 (2
> > clusters)
> > exynos5410: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster)
> > exynos5420: cpu0, cpu1, cpu2, cpu3 = A15. cpu4, cpu5, cpu6, cpu7 = A7
> > (2 clusters)
> > exynos5422: cpu0, cpu1, cpu2, cpu3 = A7. cpu4, cpu5, cpu6, cpu7 = A15
> > (2 clusters)
> > exynos5440: cpu0, cpu1, cpu2, cpu3 = A15 (1 cluster)
> >
> > Of these the only ones which has A7 as the 1st cluster are:
> > exynos3250: cpu0, cpu1 = A7
> > exynos5420: cpu0, cpu1, cpu2, cpu3 = A15. cpu4, cpu5, cpu6, cpu7 = A7
> > (2 clusters)
>
> Did you mean 5422 for this second one i.e.
> exynos5422: cpu0, cpu1, cpu2, cpu3 = A7. cpu4, cpu5, cpu6, cpu7 = A15 (2
> clusters)
> ? (I'm assuming you did)
>
> In such configurations does the second cluster (A15s in this case) need
> the same magic dance as you were adding for the A7s in the other cases?
>
> > Note that the exynos5800 has the same cpu config as the exysno5420.
> >
> > I was looking at the cpu bring up code, and notice that if the secondary
> cpu being brought up is an A7, then it invariably does the below:
> > For the exynos3250: in platsmp.c
> > void exynos_core_restart(u32 core_id)
> > {
> > u32 val;
> >
> > if (!of_machine_is_compatible("samsung,exynos3250"))
> > return;
> >
> > while (!pmu_raw_readl(S5P_PMU_SPARE2))
> > udelay(10);
> > udelay(10);
> >
> > val = pmu_raw_readl(EXYNOS_ARM_CORE_STATUS(core_id));
> > val |= S5P_CORE_WAKEUP_FROM_LOCAL_CFG;
> > pmu_raw_writel(val, EXYNOS_ARM_CORE_STATUS(core_id));
> >
> > pmu_raw_writel(EXYNOS_CORE_PO_RESET(core_id), EXYNOS_SWRESET);
> > }
> >
> > And for the exynos5800/exynos5422: mcpm-exynos.c
> > static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
> > {
> > unsigned int cpunr = cpu + (cluster *
> EXYNOS5420_CPUS_PER_CLUSTER);
> >
> > pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> > if (cpu >= EXYNOS5420_CPUS_PER_CLUSTER ||
> > cluster >= EXYNOS5420_NR_CLUSTERS)
> > return -EINVAL;
> >
> > if (!exynos_cpu_power_state(cpunr)) {
> > exynos_cpu_power_up(cpunr);
> >
> > /*
> > * 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.
> > */
> > if (cluster &&
> > cluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0),
> 1)) {
> > /*
> > * Before we reset the Little cores, we should
> wait
> > * the SPARE2 register is set to 1 because the
> init
> > * codes of the iROM will set the register after
> > * initialization.
> > */
> > while (!pmu_raw_readl(S5P_PMU_SPARE2))
> > udelay(10);
> >
> > pmu_raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
> > EXYNOS_SWRESET);
> > }
> > }
> >
> > return 0;
> > }
> >
> > This pretty much indicates that when bringing up the A7s, the special
> > reset sequence is deemed essential.
>
> If (and only if) they are in the second cluster?
>
> > Probably, we could generalize that it might be true for future
> > exynos's having A7s.
>
> This whole thing has turned into a bit of a tarpit, sorry :-/
>
> I think I'd be happiest with code which explicitly checked for SoC
> configurations which we know about (and ideally can test) over code
> which tries to predict what future SoC variants will do -- we can
> always patch them in later.
>
> Essentially I think that just means adding some machine_is_compatible
> type checks to the code you already have rather than relying on the
> overall compatibility list only have a couple of entries right now and
> implicitly relying on the differences between them (the
> presence/absence of a second cluster in this case).
>
>
I agree. Instead of generalizing (which this patch did), I will do a
function call for the A7 powering up sequence, in case it's
machine_is_compatible with exynos5800.
I am still moping over the cpupool suggestion :-)
> Ian.
>
[-- Attachment #1.2: Type: text/html, Size: 13448 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-02-17 2:24 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09 5:48 [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support Suriyan Ramasami
2016-02-09 9:53 ` Ian Campbell
2016-02-09 12:50 ` Suriyan Ramasami
2016-02-09 14:19 ` Ian Campbell
2016-02-09 18:20 ` Suriyan Ramasami
2016-02-10 10:03 ` Ian Campbell
2016-02-11 1:47 ` Suriyan Ramasami
2016-02-11 9:40 ` Ian Campbell
2016-02-15 6:32 ` Suriyan Ramasami
2016-02-16 10:03 ` Ian Campbell
2016-02-17 2:24 ` Suriyan Ramasami
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.