[U-Boot] [PATCH 0/8] PSCI v0.2 framework for ARMv8

[U-Boot] [PATCH 0/8] PSCI v0.2 framework for ARMv8

[Arnab Basu]

This series of patches creates a generic PSCI v0.2 framework for ARMv8.

The first 3 patches refactor existing code so that ARMv7 PSCI,
ARMv8 spin-table and ARMv8 PSCI can coexist.

The next 5 patches create a generic framework for PSCI v0.2 in ARMv8.

The implementation is modelled on the pre-existing PSCI v0.1 support
in ARMv7.

PSCI support patches for the ARMv8 Foundation model will follow shortly.

Arnab Basu (8):
  ARM: PSCI: Update psci.h for psci v0.2
  ARM: PSCI: Alow arch specific DT patching
  ARMv8/fsl-lsch3: Refactor spin-table code
  ARMv8: PSCI: Add linker section to hold PSCI code
  ARMv8: PCSI: Add generic ARMv8 PSCI code
  ARMv8: PSCI: Fixup the device tree for PSCI v0.2
  ARMv8: PSCI: Setup ARMv8 PSCI
  ARMv8: PSCI: Enable SMC

 arch/arm/config.mk                 |    2 +-
 arch/arm/cpu/armv7/virt-dt.c       |    7 +-
 arch/arm/cpu/armv8/Makefile        |    2 +
 arch/arm/cpu/armv8/cpu-dt.c        |  128 ++++++++++++++++++++++++++
 arch/arm/cpu/armv8/cpu.c           |   23 +++++
 arch/arm/cpu/armv8/fsl-lsch3/fdt.c |   46 +++------
 arch/arm/cpu/armv8/psci.S          |  177 ++++++++++++++++++++++++++++++++++++
 arch/arm/cpu/armv8/u-boot.lds      |   30 ++++++
 arch/arm/include/asm/macro.h       |    4 +
 arch/arm/include/asm/psci.h        |   42 ++++++++-
 arch/arm/include/asm/system.h      |    7 ++
 arch/arm/lib/bootm-fdt.c           |   11 ++-
 arch/arm/lib/bootm.c               |    3 +
 13 files changed, 446 insertions(+), 36 deletions(-)
 create mode 100644 arch/arm/cpu/armv8/cpu-dt.c
 create mode 100644 arch/arm/cpu/armv8/psci.S

-- 
1.7.7.4

[U-Boot] [PATCH 1/8] ARM: PSCI: Update psci.h for psci v0.2

[Arnab Basu]

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/psci.h |   42 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
index 704b4b0..68579cd 100644
--- a/arch/arm/include/asm/psci.h
+++ b/arch/arm/include/asm/psci.h
@@ -2,6 +2,10 @@
  * Copyright (C) 2013 - ARM Ltd
  * Author: Marc Zyngier <marc.zyngier@arm.com>
  *
+ * Copyright (C) 2014 - Freescale Semiconductor Ltd
+ * Author: Arnab Basu <arnab.basu@freescale.com>
+ *	   updated file for PSCI v0.2
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
@@ -18,7 +22,7 @@
 #ifndef __ARM_PSCI_H__
 #define __ARM_PSCI_H__
 
-/* PSCI interface */
+/* PSCI v0.1 interface */
 #define ARM_PSCI_FN_BASE		0x95c1ba5e
 #define ARM_PSCI_FN(n)			(ARM_PSCI_FN_BASE + (n))
 
@@ -27,9 +31,45 @@
 #define ARM_PSCI_FN_CPU_ON		ARM_PSCI_FN(2)
 #define ARM_PSCI_FN_MIGRATE		ARM_PSCI_FN(3)
 
+/* PSCI v0.2 interface */
+#define PSCI_0_2_FN_BASE		0x84000000
+#define PSCI_0_2_FN(n)			(PSCI_0_2_FN_BASE + (n))
+#define PSCI_0_2_64BIT			0x40000000
+#define PSCI_0_2_FN64_BASE		\
+				(PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
+#define PSCI_0_2_FN64(n)		(PSCI_0_2_FN64_BASE + (n))
+
+#define PSCI_0_2_FN_PSCI_VERSION	PSCI_0_2_FN(0)
+#define PSCI_0_2_FN_CPU_SUSPEND		PSCI_0_2_FN(1)
+#define PSCI_0_2_FN_CPU_OFF		PSCI_0_2_FN(2)
+#define PSCI_0_2_FN_CPU_ON		PSCI_0_2_FN(3)
+#define PSCI_0_2_FN_AFFINITY_INFO	PSCI_0_2_FN(4)
+#define PSCI_0_2_FN_MIGRATE		PSCI_0_2_FN(5)
+#define PSCI_0_2_FN_MIGRATE_INFO_TYPE	PSCI_0_2_FN(6)
+#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN(7)
+#define PSCI_0_2_FN_SYSTEM_OFF		PSCI_0_2_FN(8)
+#define PSCI_0_2_FN_SYSTEM_RESET	PSCI_0_2_FN(9)
+
+#define PSCI_0_2_FN64_CPU_SUSPEND		PSCI_0_2_FN64(1)
+#define PSCI_0_2_FN64_CPU_ON			PSCI_0_2_FN64(3)
+#define PSCI_0_2_FN64_AFFINITY_INFO		PSCI_0_2_FN64(4)
+#define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
+#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
+
+
+/*
+ * Only PSCI return values such as: SUCCESS, NOT_SUPPORTED,
+ * INVALID_PARAMS, and DENIED defined below are applicable
+ * to PSCI v0.1.
+ */
 #define ARM_PSCI_RET_SUCCESS		0
 #define ARM_PSCI_RET_NI			(-1)
 #define ARM_PSCI_RET_INVAL		(-2)
 #define ARM_PSCI_RET_DENIED		(-3)
+#define PSCI_RET_ALREADY_ON		(-4)
+#define PSCI_RET_ON_PENDING		(-5)
+#define PSCI_RET_INTERNAL_FAILURE	(-6)
+#define PSCI_RET_NOT_PRESENT		(-7)
+#define PSCI_RET_DISABLED		(-8)
 
 #endif /* __ARM_PSCI_H__ */
-- 
1.7.7.4

[U-Boot] [PATCH 1/8] ARM: PSCI: Update psci.h for psci v0.2

[Albert ARIBAUD]

Hello Arnab,

On Thu, 28 Aug 2014 01:59:54 +0530, Arnab Basu
<arnab.basu@freescale.com> wrote:
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/psci.h |   42 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 41 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> index 704b4b0..68579cd 100644
> --- a/arch/arm/include/asm/psci.h
> +++ b/arch/arm/include/asm/psci.h
> @@ -2,6 +2,10 @@
>   * Copyright (C) 2013 - ARM Ltd
>   * Author: Marc Zyngier <marc.zyngier@arm.com>
>   *
> + * Copyright (C) 2014 - Freescale Semiconductor Ltd
> + * Author: Arnab Basu <arnab.basu@freescale.com>
> + *	   updated file for PSCI v0.2
> + *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
> @@ -18,7 +22,7 @@
>  #ifndef __ARM_PSCI_H__
>  #define __ARM_PSCI_H__
>  
> -/* PSCI interface */
> +/* PSCI v0.1 interface */
>  #define ARM_PSCI_FN_BASE		0x95c1ba5e
>  #define ARM_PSCI_FN(n)			(ARM_PSCI_FN_BASE + (n))
>  
> @@ -27,9 +31,45 @@
>  #define ARM_PSCI_FN_CPU_ON		ARM_PSCI_FN(2)
>  #define ARM_PSCI_FN_MIGRATE		ARM_PSCI_FN(3)
>  
> +/* PSCI v0.2 interface */
> +#define PSCI_0_2_FN_BASE		0x84000000
> +#define PSCI_0_2_FN(n)			(PSCI_0_2_FN_BASE + (n))
> +#define PSCI_0_2_64BIT			0x40000000
> +#define PSCI_0_2_FN64_BASE		\
> +				(PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
> +#define PSCI_0_2_FN64(n)		(PSCI_0_2_FN64_BASE + (n))
> +
> +#define PSCI_0_2_FN_PSCI_VERSION	PSCI_0_2_FN(0)
> +#define PSCI_0_2_FN_CPU_SUSPEND		PSCI_0_2_FN(1)
> +#define PSCI_0_2_FN_CPU_OFF		PSCI_0_2_FN(2)
> +#define PSCI_0_2_FN_CPU_ON		PSCI_0_2_FN(3)
> +#define PSCI_0_2_FN_AFFINITY_INFO	PSCI_0_2_FN(4)
> +#define PSCI_0_2_FN_MIGRATE		PSCI_0_2_FN(5)
> +#define PSCI_0_2_FN_MIGRATE_INFO_TYPE	PSCI_0_2_FN(6)
> +#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN(7)
> +#define PSCI_0_2_FN_SYSTEM_OFF		PSCI_0_2_FN(8)
> +#define PSCI_0_2_FN_SYSTEM_RESET	PSCI_0_2_FN(9)
> +
> +#define PSCI_0_2_FN64_CPU_SUSPEND		PSCI_0_2_FN64(1)
> +#define PSCI_0_2_FN64_CPU_ON			PSCI_0_2_FN64(3)
> +#define PSCI_0_2_FN64_AFFINITY_INFO		PSCI_0_2_FN64(4)
> +#define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
> +#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
> +
> +
> +/*
> + * Only PSCI return values such as: SUCCESS, NOT_SUPPORTED,
> + * INVALID_PARAMS, and DENIED defined below are applicable
> + * to PSCI v0.1.
> + */
>  #define ARM_PSCI_RET_SUCCESS		0
>  #define ARM_PSCI_RET_NI			(-1)
>  #define ARM_PSCI_RET_INVAL		(-2)
>  #define ARM_PSCI_RET_DENIED		(-3)
> +#define PSCI_RET_ALREADY_ON		(-4)
> +#define PSCI_RET_ON_PENDING		(-5)
> +#define PSCI_RET_INTERNAL_FAILURE	(-6)
> +#define PSCI_RET_NOT_PRESENT		(-7)
> +#define PSCI_RET_DISABLED		(-8)
>  
>  #endif /* __ARM_PSCI_H__ */
> -- 
> 1.7.7.4

Applied, with apologies for the delay. This patch will appear in
2015.01.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/8] ARM: PSCI: Update psci.h for psci v0.2 -- CORRECTION

[Albert ARIBAUD]

Hello Albert,

On Sat, 8 Nov 2014 08:43:16 +0100, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hello Arnab,
> 
> On Thu, 28 Aug 2014 01:59:54 +0530, Arnab Basu
> <arnab.basu@freescale.com> wrote:
> > Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> > Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/include/asm/psci.h |   42 +++++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 41 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> > index 704b4b0..68579cd 100644
> > --- a/arch/arm/include/asm/psci.h
> > +++ b/arch/arm/include/asm/psci.h
> > @@ -2,6 +2,10 @@
> >   * Copyright (C) 2013 - ARM Ltd
> >   * Author: Marc Zyngier <marc.zyngier@arm.com>
> >   *
> > + * Copyright (C) 2014 - Freescale Semiconductor Ltd
> > + * Author: Arnab Basu <arnab.basu@freescale.com>
> > + *	   updated file for PSCI v0.2
> > + *
> >   * This program is free software; you can redistribute it and/or modify
> >   * it under the terms of the GNU General Public License version 2 as
> >   * published by the Free Software Foundation.
> > @@ -18,7 +22,7 @@
> >  #ifndef __ARM_PSCI_H__
> >  #define __ARM_PSCI_H__
> >  
> > -/* PSCI interface */
> > +/* PSCI v0.1 interface */
> >  #define ARM_PSCI_FN_BASE		0x95c1ba5e
> >  #define ARM_PSCI_FN(n)			(ARM_PSCI_FN_BASE + (n))
> >  
> > @@ -27,9 +31,45 @@
> >  #define ARM_PSCI_FN_CPU_ON		ARM_PSCI_FN(2)
> >  #define ARM_PSCI_FN_MIGRATE		ARM_PSCI_FN(3)
> >  
> > +/* PSCI v0.2 interface */
> > +#define PSCI_0_2_FN_BASE		0x84000000
> > +#define PSCI_0_2_FN(n)			(PSCI_0_2_FN_BASE + (n))
> > +#define PSCI_0_2_64BIT			0x40000000
> > +#define PSCI_0_2_FN64_BASE		\
> > +				(PSCI_0_2_FN_BASE + PSCI_0_2_64BIT)
> > +#define PSCI_0_2_FN64(n)		(PSCI_0_2_FN64_BASE + (n))
> > +
> > +#define PSCI_0_2_FN_PSCI_VERSION	PSCI_0_2_FN(0)
> > +#define PSCI_0_2_FN_CPU_SUSPEND		PSCI_0_2_FN(1)
> > +#define PSCI_0_2_FN_CPU_OFF		PSCI_0_2_FN(2)
> > +#define PSCI_0_2_FN_CPU_ON		PSCI_0_2_FN(3)
> > +#define PSCI_0_2_FN_AFFINITY_INFO	PSCI_0_2_FN(4)
> > +#define PSCI_0_2_FN_MIGRATE		PSCI_0_2_FN(5)
> > +#define PSCI_0_2_FN_MIGRATE_INFO_TYPE	PSCI_0_2_FN(6)
> > +#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN(7)
> > +#define PSCI_0_2_FN_SYSTEM_OFF		PSCI_0_2_FN(8)
> > +#define PSCI_0_2_FN_SYSTEM_RESET	PSCI_0_2_FN(9)
> > +
> > +#define PSCI_0_2_FN64_CPU_SUSPEND		PSCI_0_2_FN64(1)
> > +#define PSCI_0_2_FN64_CPU_ON			PSCI_0_2_FN64(3)
> > +#define PSCI_0_2_FN64_AFFINITY_INFO		PSCI_0_2_FN64(4)
> > +#define PSCI_0_2_FN64_MIGRATE			PSCI_0_2_FN64(5)
> > +#define PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU	PSCI_0_2_FN64(7)
> > +
> > +
> > +/*
> > + * Only PSCI return values such as: SUCCESS, NOT_SUPPORTED,
> > + * INVALID_PARAMS, and DENIED defined below are applicable
> > + * to PSCI v0.1.
> > + */
> >  #define ARM_PSCI_RET_SUCCESS		0
> >  #define ARM_PSCI_RET_NI			(-1)
> >  #define ARM_PSCI_RET_INVAL		(-2)
> >  #define ARM_PSCI_RET_DENIED		(-3)
> > +#define PSCI_RET_ALREADY_ON		(-4)
> > +#define PSCI_RET_ON_PENDING		(-5)
> > +#define PSCI_RET_INTERNAL_FAILURE	(-6)
> > +#define PSCI_RET_NOT_PRESENT		(-7)
> > +#define PSCI_RET_DISABLED		(-8)
> >  
> >  #endif /* __ARM_PSCI_H__ */
> > -- 
> > 1.7.7.4
> 
> Applied, with apologies for the delay. This patch will appear in
> 2015.01.

Drat -- I'd forgotten Arnab had hinted on possibly modifying patches
from the series, and I assumed this one would not. Un-applied for now.

> Amicalement,
> -- 
> Albert.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 2/8] ARM: PSCI: Alow arch specific DT patching

[Arnab Basu]

Both ARMv7 and ARMv8 need to patch the device tree but the kind
of patching done is different. This creates a function that can be
defined by each architecture to handle the differences

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/cpu/armv7/virt-dt.c |    7 ++++++-
 arch/arm/lib/bootm-fdt.c     |   11 ++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c
index 0b0d6a7..3fbec39 100644
--- a/arch/arm/cpu/armv7/virt-dt.c
+++ b/arch/arm/cpu/armv7/virt-dt.c
@@ -88,7 +88,7 @@ static int fdt_psci(void *fdt)
 	return 0;
 }
 
-int armv7_update_dt(void *fdt)
+static int armv7_update_dt(void *fdt)
 {
 #ifndef CONFIG_ARMV7_SECURE_BASE
 	/* secure code lives in RAM, keep it alive */
@@ -98,3 +98,8 @@ int armv7_update_dt(void *fdt)
 
 	return fdt_psci(fdt);
 }
+
+int cpu_update_dt(void *fdt)
+{
+	return armv7_update_dt(fdt);
+}
diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c
index d4f1578..daabc03 100644
--- a/arch/arm/lib/bootm-fdt.c
+++ b/arch/arm/lib/bootm-fdt.c
@@ -21,6 +21,11 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+__weak int cpu_update_dt(void *fdt)
+{
+	return 0;
+}
+
 int arch_fixup_fdt(void *blob)
 {
 	bd_t *bd = gd->bd;
@@ -34,11 +39,11 @@ int arch_fixup_fdt(void *blob)
 	}
 
 	ret = fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
-#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+
 	if (ret)
 		return ret;
 
-	ret = armv7_update_dt(blob);
-#endif
+	ret = cpu_update_dt(blob);
+
 	return ret;
 }
-- 
1.7.7.4

[U-Boot] [PATCH 2/8] ARM: PSCI: Alow arch specific DT patching

[Mark Rutland]

Hi Arnab,

On Wed, Aug 27, 2014 at 09:29:55PM +0100, Arnab Basu wrote:
> Both ARMv7 and ARMv8 need to patch the device tree but the kind
> of patching done is different. This creates a function that can be
> defined by each architecture to handle the differences

I have no problem with the patch, but what is it that we need to do
differently for ARMv7 and ARMv8?

Mark.

> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/cpu/armv7/virt-dt.c |    7 ++++++-
>  arch/arm/lib/bootm-fdt.c     |   11 ++++++++---
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/virt-dt.c b/arch/arm/cpu/armv7/virt-dt.c
> index 0b0d6a7..3fbec39 100644
> --- a/arch/arm/cpu/armv7/virt-dt.c
> +++ b/arch/arm/cpu/armv7/virt-dt.c
> @@ -88,7 +88,7 @@ static int fdt_psci(void *fdt)
>  	return 0;
>  }
>  
> -int armv7_update_dt(void *fdt)
> +static int armv7_update_dt(void *fdt)
>  {
>  #ifndef CONFIG_ARMV7_SECURE_BASE
>  	/* secure code lives in RAM, keep it alive */
> @@ -98,3 +98,8 @@ int armv7_update_dt(void *fdt)
>  
>  	return fdt_psci(fdt);
>  }
> +
> +int cpu_update_dt(void *fdt)
> +{
> +	return armv7_update_dt(fdt);
> +}
> diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c
> index d4f1578..daabc03 100644
> --- a/arch/arm/lib/bootm-fdt.c
> +++ b/arch/arm/lib/bootm-fdt.c
> @@ -21,6 +21,11 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> +__weak int cpu_update_dt(void *fdt)
> +{
> +	return 0;
> +}
> +
>  int arch_fixup_fdt(void *blob)
>  {
>  	bd_t *bd = gd->bd;
> @@ -34,11 +39,11 @@ int arch_fixup_fdt(void *blob)
>  	}
>  
>  	ret = fdt_fixup_memory_banks(blob, start, size, CONFIG_NR_DRAM_BANKS);
> -#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
> +
>  	if (ret)
>  		return ret;
>  
> -	ret = armv7_update_dt(blob);
> -#endif
> +	ret = cpu_update_dt(blob);
> +
>  	return ret;
>  }
> -- 
> 1.7.7.4
> 
> 

[U-Boot] [PATCH 2/8] ARM: PSCI: Alow arch specific DT patching

[Arnab Basu]

Hi Mark

On 08/28/2014 03:40 PM, Mark Rutland wrote:
> Hi Arnab,
>
> On Wed, Aug 27, 2014 at 09:29:55PM +0100, Arnab Basu wrote:
>> Both ARMv7 and ARMv8 need to patch the device tree but the kind
>> of patching done is different. This creates a function that can be
>> defined by each architecture to handle the differences
>
> I have no problem with the patch, but what is it that we need to do
> differently for ARMv7 and ARMv8?
>

In ARMv7 there does not seem to be any code around to set 
"enable-method" to "spin-table". I guess it is assumed that DTs already 
come with this set.

For ARMv8 we would like to assume that "enable-method" is missing from 
the cpu node and will be set either to "spin-table" or "psci" by the 
boot loader.

So the difference is, for ARMv7 the "enable-method" is only modified in 
case of PSCI, whereas for ARMv8 it is always modified.

Thanks
Arnab

> Mark.
>

[U-Boot] [PATCH 2/8] ARM: PSCI: Alow arch specific DT patching

[Mark Rutland]

On Thu, Aug 28, 2014 at 11:51:08AM +0100, Arnab Basu wrote:
> Hi Mark
> 
> On 08/28/2014 03:40 PM, Mark Rutland wrote:
> > Hi Arnab,
> >
> > On Wed, Aug 27, 2014 at 09:29:55PM +0100, Arnab Basu wrote:
> >> Both ARMv7 and ARMv8 need to patch the device tree but the kind
> >> of patching done is different. This creates a function that can be
> >> defined by each architecture to handle the differences
> >
> > I have no problem with the patch, but what is it that we need to do
> > differently for ARMv7 and ARMv8?
> >
> 
> In ARMv7 there does not seem to be any code around to set 
> "enable-method" to "spin-table". I guess it is assumed that DTs already 
> come with this set.

That'll be a consequence of partial conversion frmo board file on the
32-bit side. For most platforms the SMP boot mechanism is still
implicit.

> For ARMv8 we would like to assume that "enable-method" is missing from 
> the cpu node and will be set either to "spin-table" or "psci" by the 
> boot loader.

Sounds good to me.

> So the difference is, for ARMv7 the "enable-method" is only modified in 
> case of PSCI, whereas for ARMv8 it is always modified.

Ok. Thanks for the description. :)

Mark.

[U-Boot] [PATCH 3/8] ARMv8/fsl-lsch3: Refactor spin-table code

[Arnab Basu]

This creates the function cpu_update_dt for ARMv8 which currently patches
the cpu node in the device table and sets enable-method to spin-table.

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: York Sun <yorksun@freescale.com>
---
 arch/arm/cpu/armv8/Makefile                      |    1 +
 arch/arm/cpu/armv8/{fsl-lsch3/fdt.c => cpu-dt.c} |   37 ++++++++++--------
 arch/arm/cpu/armv8/fsl-lsch3/fdt.c               |   46 ++++++++--------------
 3 files changed, 38 insertions(+), 46 deletions(-)
 copy arch/arm/cpu/armv8/{fsl-lsch3/fdt.c => cpu-dt.c} (62%)

diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
index 7d93f59..4f0ea87 100644
--- a/arch/arm/cpu/armv8/Makefile
+++ b/arch/arm/cpu/armv8/Makefile
@@ -14,3 +14,4 @@ obj-y	+= exceptions.o
 obj-y	+= cache.o
 obj-y	+= tlb.o
 obj-y	+= transition.o
+obj-y	+= cpu-dt.o
diff --git a/arch/arm/cpu/armv8/fsl-lsch3/fdt.c b/arch/arm/cpu/armv8/cpu-dt.c
similarity index 62%
copy from arch/arm/cpu/armv8/fsl-lsch3/fdt.c
copy to arch/arm/cpu/armv8/cpu-dt.c
index 2dbcdcb..9792bc0 100644
--- a/arch/arm/cpu/armv8/fsl-lsch3/fdt.c
+++ b/arch/arm/cpu/armv8/cpu-dt.c
@@ -7,17 +7,24 @@
 #include <common.h>
 #include <libfdt.h>
 #include <fdt_support.h>
-#include "mp.h"
 
 #ifdef CONFIG_MP
-void ft_fixup_cpu(void *blob)
+
+__weak u64 arch_get_release_addr(u64 cpu_id)
+{
+	return 0;
+}
+
+__weak void arch_spin_table_reserve_mem(void *fdt)
+{
+}
+
+static void cpu_update_dt_spin_table(void *blob)
 {
 	int off;
-	__maybe_unused u64 spin_tbl_addr = (u64)get_spin_tbl_addr();
-	fdt32_t *reg;
+	__maybe_unused u64 val;
 	int addr_cells;
-	u64 val;
-	size_t *boot_code_size = &(__secondary_boot_code_size);
+	__maybe_unused fdt32_t *reg;
 
 	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpus", 4);
 	of_bus_default_count_cells(blob, off, &addr_cells, NULL);
@@ -26,31 +33,29 @@ void ft_fixup_cpu(void *blob)
 	while (off != -FDT_ERR_NOTFOUND) {
 		reg = (fdt32_t *)fdt_getprop(blob, off, "reg", 0);
 		if (reg) {
-			val = spin_tbl_addr;
-#ifndef CONFIG_FSL_SMP_RELEASE_ALL
-			val += id_to_core(of_read_number(reg, addr_cells))
-				* SPIN_TABLE_ELEM_SIZE;
-#endif
+			val = arch_get_release_addr(
+					of_read_number(reg, addr_cells));
 			val = cpu_to_fdt64(val);
 			fdt_setprop_string(blob, off, "enable-method",
 					   "spin-table");
 			fdt_setprop(blob, off, "cpu-release-addr",
 				    &val, sizeof(val));
 		} else {
-			puts("cpu NULL\n");
+			puts("Unable to read reg property\n");
 		}
+
 		off = fdt_node_offset_by_prop_value(blob, off, "device_type",
 						    "cpu", 4);
 	}
 
-	fdt_add_mem_rsv(blob, (uintptr_t)&secondary_boot_code,
-			*boot_code_size);
+	arch_spin_table_reserve_mem(blob);
 }
 #endif
 
-void ft_cpu_setup(void *blob, bd_t *bd)
+int cpu_update_dt(void *fdt)
 {
 #ifdef CONFIG_MP
-	ft_fixup_cpu(blob);
+	cpu_update_dt_spin_table(fdt);
 #endif
+	return 0;
 }
diff --git a/arch/arm/cpu/armv8/fsl-lsch3/fdt.c b/arch/arm/cpu/armv8/fsl-lsch3/fdt.c
index 2dbcdcb..bb35393 100644
--- a/arch/arm/cpu/armv8/fsl-lsch3/fdt.c
+++ b/arch/arm/cpu/armv8/fsl-lsch3/fdt.c
@@ -10,42 +10,28 @@
 #include "mp.h"
 
 #ifdef CONFIG_MP
-void ft_fixup_cpu(void *blob)
+u64 arch_get_release_addr(u64 cpu_id)
 {
-	int off;
-	__maybe_unused u64 spin_tbl_addr = (u64)get_spin_tbl_addr();
-	fdt32_t *reg;
-	int addr_cells;
 	u64 val;
+
+	val = (u64)get_spin_tbl_addr();
+	val += id_to_core(cpu_id) * SPIN_TABLE_ELEM_SIZE;
+
+	return val;
+}
+
+void arch_spin_table_reserve_mem(void *fdt)
+{
 	size_t *boot_code_size = &(__secondary_boot_code_size);
 
-	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpus", 4);
-	of_bus_default_count_cells(blob, off, &addr_cells, NULL);
-
-	off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4);
-	while (off != -FDT_ERR_NOTFOUND) {
-		reg = (fdt32_t *)fdt_getprop(blob, off, "reg", 0);
-		if (reg) {
-			val = spin_tbl_addr;
-#ifndef CONFIG_FSL_SMP_RELEASE_ALL
-			val += id_to_core(of_read_number(reg, addr_cells))
-				* SPIN_TABLE_ELEM_SIZE;
-#endif
-			val = cpu_to_fdt64(val);
-			fdt_setprop_string(blob, off, "enable-method",
-					   "spin-table");
-			fdt_setprop(blob, off, "cpu-release-addr",
-				    &val, sizeof(val));
-		} else {
-			puts("cpu NULL\n");
-		}
-		off = fdt_node_offset_by_prop_value(blob, off, "device_type",
-						    "cpu", 4);
-	}
-
-	fdt_add_mem_rsv(blob, (uintptr_t)&secondary_boot_code,
+	fdt_add_mem_rsv(fdt, (uintptr_t)&secondary_boot_code,
 			*boot_code_size);
 }
+
+static void ft_fixup_cpu(void *blob)
+{
+}
+
 #endif
 
 void ft_cpu_setup(void *blob, bd_t *bd)
-- 
1.7.7.4

[U-Boot] [PATCH 3/8] ARMv8/fsl-lsch3: Refactor spin-table code

[Albert ARIBAUD]

Hello Arnab,

On Thu, 28 Aug 2014 01:59:56 +0530, Arnab Basu
<arnab.basu@freescale.com> wrote:
> This creates the function cpu_update_dt for ARMv8 which currently
> patches the cpu node in the device table and sets enable-method to
> spin-table.
> 
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: York Sun <yorksun@freescale.com>
> ---

This patch fails to apply cleanly. Can you please rebase the series on
top of current u-boot/master?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 3/8] ARMv8/fsl-lsch3: Refactor spin-table code

[arnab.basu at freescale.com]

Hi Albert

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
> Sent: Monday, October 27, 2014 2:36 AM
> To: Basu Arnab-B45036
> Cc: marc.zyngier at arm.com; mark.rutland at arm.com; Sun York-R58495; Yoder
> Stuart-B08248; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 3/8] ARMv8/fsl-lsch3: Refactor spin-table
> code
> 
> Hello Arnab,
> 
> On Thu, 28 Aug 2014 01:59:56 +0530, Arnab Basu <arnab.basu@freescale.com>
> wrote:
> > This creates the function cpu_update_dt for ARMv8 which currently
> > patches the cpu node in the device table and sets enable-method to
> > spin-table.
> >
> > Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> > Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > Cc: York Sun <yorksun@freescale.com>
> > ---
> 
> This patch fails to apply cleanly. Can you please rebase the series on
> top of current u-boot/master?

Yes, in fact I need to rework some of the other patches in the series too.
I got a little side tracked, hence the delay. I will get on this..

Thanks
Arnab

> 
> Amicalement,
> --
> Albert.

[U-Boot] [PATCH 4/8] ARMv8: PSCI: Add linker section to hold PSCI code

[Arnab Basu]

A separate linker section makes it possible to keep this code either
in DDR or in some secure memory location provided specifically for the
purpose.

So far no one is using this section.

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/config.mk            |    2 +-
 arch/arm/cpu/armv8/u-boot.lds |   30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index c339e6d..9272e9c 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -111,7 +111,7 @@ endif
 
 # limit ourselves to the sections we want in the .bin.
 ifdef CONFIG_ARM64
-OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
+OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
 else
 OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
 endif
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index 4c12222..bd95fff 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -8,6 +8,8 @@
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
+#include <config.h>
+
 OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64")
 OUTPUT_ARCH(aarch64)
 ENTRY(_start)
@@ -23,6 +25,34 @@ SECTIONS
 		*(.text*)
 	}
 
+#ifdef CONFIG_ARMV8_PSCI
+
+#ifndef CONFIG_ARMV8_SECURE_BASE
+#define CONFIG_ARMV8_SECURE_BASE
+#endif
+
+	.__secure_start : {
+		. = ALIGN(0x1000);
+		*(.__secure_start)
+	}
+
+	.secure_text CONFIG_ARMV8_SECURE_BASE :
+		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
+	{
+		*(._secure.text)
+	}
+
+	. = LOADADDR(.__secure_start) +
+		SIZEOF(.__secure_start) +
+		SIZEOF(.secure_text);
+
+	__secure_end_lma = .;
+	.__secure_end : AT(__secure_end_lma) {
+		*(.__secure_end)
+		LONG(0x1d1071c);	/* Must output something to reset LMA */
+	}
+#endif
+
 	. = ALIGN(8);
 	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
 
-- 
1.7.7.4

[U-Boot] [PATCH 4/8] ARMv8: PSCI: Add linker section to hold PSCI code

[Albert ARIBAUD]

Hi Arnab,

On Thu, 28 Aug 2014 01:59:57 +0530, Arnab Basu
<arnab.basu@freescale.com> wrote:

> A separate linker section makes it possible to keep this code either
> in DDR or in some secure memory location provided specifically for the
> purpose.
> 
> So far no one is using this section.
> 
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/config.mk            |    2 +-
>  arch/arm/cpu/armv8/u-boot.lds |   30 ++++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index c339e6d..9272e9c 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -111,7 +111,7 @@ endif
>  
>  # limit ourselves to the sections we want in the .bin.
>  ifdef CONFIG_ARM64
> -OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
> +OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
>  else
>  OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
>  endif
> diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
> index 4c12222..bd95fff 100644
> --- a/arch/arm/cpu/armv8/u-boot.lds
> +++ b/arch/arm/cpu/armv8/u-boot.lds
> @@ -8,6 +8,8 @@
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
>  
> +#include <config.h>
> +
>  OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64")
>  OUTPUT_ARCH(aarch64)
>  ENTRY(_start)
> @@ -23,6 +25,34 @@ SECTIONS
>  		*(.text*)
>  	}
>  
> +#ifdef CONFIG_ARMV8_PSCI
> +
> +#ifndef CONFIG_ARMV8_SECURE_BASE
> +#define CONFIG_ARMV8_SECURE_BASE
> +#endif
> +
> +	.__secure_start : {
> +		. = ALIGN(0x1000);
> +		*(.__secure_start)
> +	}
> +
> +	.secure_text CONFIG_ARMV8_SECURE_BASE :
> +		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
> +	{
> +		*(._secure.text)
> +	}
> +
> +	. = LOADADDR(.__secure_start) +
> +		SIZEOF(.__secure_start) +
> +		SIZEOF(.secure_text);
> +
> +	__secure_end_lma = .;
> +	.__secure_end : AT(__secure_end_lma) {
> +		*(.__secure_end)
> +		LONG(0x1d1071c);	/* Must output something to reset LMA */

Can you explain in more detail what issue this fixes?

> +	}
> +#endif
> +
>  	. = ALIGN(8);
>  	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
>  


Amicalement,
-- 
Albert.

[U-Boot] [PATCH 4/8] ARMv8: PSCI: Add linker section to hold PSCI code

[Marc Zyngier]

On Thu, Sep 18 2014 at 10:12:17 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> Hi Arnab,
>
> On Thu, 28 Aug 2014 01:59:57 +0530, Arnab Basu
> <arnab.basu@freescale.com> wrote:
>
>> A separate linker section makes it possible to keep this code either
>> in DDR or in some secure memory location provided specifically for the
>> purpose.
>> 
>> So far no one is using this section.
>> 
>> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
>> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/config.mk            |    2 +-
>>  arch/arm/cpu/armv8/u-boot.lds |   30 ++++++++++++++++++++++++++++++
>>  2 files changed, 31 insertions(+), 1 deletions(-)
>> 
>> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
>> index c339e6d..9272e9c 100644
>> --- a/arch/arm/config.mk
>> +++ b/arch/arm/config.mk
>> @@ -111,7 +111,7 @@ endif
>>  
>>  # limit ourselves to the sections we want in the .bin.
>>  ifdef CONFIG_ARM64
>> -OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
>> +OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
>>  else
>>  OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
>>  endif
>> diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
>> index 4c12222..bd95fff 100644
>> --- a/arch/arm/cpu/armv8/u-boot.lds
>> +++ b/arch/arm/cpu/armv8/u-boot.lds
>> @@ -8,6 +8,8 @@
>>   * SPDX-License-Identifier:	GPL-2.0+
>>   */
>>  
>> +#include <config.h>
>> +
>>  OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64")
>>  OUTPUT_ARCH(aarch64)
>>  ENTRY(_start)
>> @@ -23,6 +25,34 @@ SECTIONS
>>  		*(.text*)
>>  	}
>>  
>> +#ifdef CONFIG_ARMV8_PSCI
>> +
>> +#ifndef CONFIG_ARMV8_SECURE_BASE
>> +#define CONFIG_ARMV8_SECURE_BASE
>> +#endif
>> +
>> +	.__secure_start : {
>> +		. = ALIGN(0x1000);
>> +		*(.__secure_start)
>> +	}
>> +
>> +	.secure_text CONFIG_ARMV8_SECURE_BASE :
>> +		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
>> +	{
>> +		*(._secure.text)
>> +	}
>> +
>> +	. = LOADADDR(.__secure_start) +
>> +		SIZEOF(.__secure_start) +
>> +		SIZEOF(.secure_text);
>> +
>> +	__secure_end_lma = .;
>> +	.__secure_end : AT(__secure_end_lma) {
>> +		*(.__secure_end)
>> +		LONG(0x1d1071c);	/* Must output something to reset LMA */
>
> Can you explain in more detail what issue this fixes?

If you use AT to force a new load address (LMA), you must ensure that
you actually output something at this address. Here, if *(.__secure_end)
ends up being empty, whatever follows would be as if the AT never
happened, ending up at the wrong LMA.

The workaround is to force the output of a dummy value in all
cases, ensuring that the rest of the text is at a sensible LMA. This is
an issue that has been in GNU ld for years, and this workaround is a
copy/paste of the same one in the ARMv7 ld script.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

[U-Boot] [PATCH 4/8] ARMv8: PSCI: Add linker section to hold PSCI code

[Albert ARIBAUD]

Hi Marc,

On Thu, 18 Sep 2014 16:28:52 +0100, Marc Zyngier <marc.zyngier@arm.com>
wrote:

> On Thu, Sep 18 2014 at 10:12:17 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> > Hi Arnab,
> >
> > On Thu, 28 Aug 2014 01:59:57 +0530, Arnab Basu
> > <arnab.basu@freescale.com> wrote:
> >
> >> A separate linker section makes it possible to keep this code either
> >> in DDR or in some secure memory location provided specifically for the
> >> purpose.
> >> 
> >> So far no one is using this section.
> >> 
> >> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> >> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> >> Cc: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >>  arch/arm/config.mk            |    2 +-
> >>  arch/arm/cpu/armv8/u-boot.lds |   30 ++++++++++++++++++++++++++++++
> >>  2 files changed, 31 insertions(+), 1 deletions(-)
> >> 
> >> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> >> index c339e6d..9272e9c 100644
> >> --- a/arch/arm/config.mk
> >> +++ b/arch/arm/config.mk
> >> @@ -111,7 +111,7 @@ endif
> >>  
> >>  # limit ourselves to the sections we want in the .bin.
> >>  ifdef CONFIG_ARM64
> >> -OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
> >> +OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
> >>  else
> >>  OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
> >>  endif
> >> diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
> >> index 4c12222..bd95fff 100644
> >> --- a/arch/arm/cpu/armv8/u-boot.lds
> >> +++ b/arch/arm/cpu/armv8/u-boot.lds
> >> @@ -8,6 +8,8 @@
> >>   * SPDX-License-Identifier:	GPL-2.0+
> >>   */
> >>  
> >> +#include <config.h>
> >> +
> >>  OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64")
> >>  OUTPUT_ARCH(aarch64)
> >>  ENTRY(_start)
> >> @@ -23,6 +25,34 @@ SECTIONS
> >>  		*(.text*)
> >>  	}
> >>  
> >> +#ifdef CONFIG_ARMV8_PSCI
> >> +
> >> +#ifndef CONFIG_ARMV8_SECURE_BASE
> >> +#define CONFIG_ARMV8_SECURE_BASE
> >> +#endif
> >> +
> >> +	.__secure_start : {
> >> +		. = ALIGN(0x1000);
> >> +		*(.__secure_start)
> >> +	}
> >> +
> >> +	.secure_text CONFIG_ARMV8_SECURE_BASE :
> >> +		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
> >> +	{
> >> +		*(._secure.text)
> >> +	}
> >> +
> >> +	. = LOADADDR(.__secure_start) +
> >> +		SIZEOF(.__secure_start) +
> >> +		SIZEOF(.secure_text);
> >> +
> >> +	__secure_end_lma = .;
> >> +	.__secure_end : AT(__secure_end_lma) {
> >> +		*(.__secure_end)
> >> +		LONG(0x1d1071c);	/* Must output something to reset LMA */
> >
> > Can you explain in more detail what issue this fixes?
> 
> If you use AT to force a new load address (LMA), you must ensure that
> you actually output something at this address. Here, if *(.__secure_end)
> ends up being empty, whatever follows would be as if the AT never
> happened, ending up at the wrong LMA.
>
> The workaround is to force the output of a dummy value in all
> cases, ensuring that the rest of the text is at a sensible LMA. This is
> an issue that has been in GNU ld for years, and this workaround is a
> copy/paste of the same one in the ARMv7 ld script.

I see. Does the ld bug have an identifier that we could mention in a
comment in the linker script as a reference?

> Thanks,
> 
> 	M.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 4/8] ARMv8: PSCI: Add linker section to hold PSCI code

[Albert ARIBAUD]

Hi Albert,

On Fri, 19 Sep 2014 18:04:14 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> Hi Marc,
> 
> On Thu, 18 Sep 2014 16:28:52 +0100, Marc Zyngier <marc.zyngier@arm.com>
> wrote:
> 
> > On Thu, Sep 18 2014 at 10:12:17 AM, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote:
> > > Hi Arnab,
> > >
> > > On Thu, 28 Aug 2014 01:59:57 +0530, Arnab Basu
> > > <arnab.basu@freescale.com> wrote:
> > >
> > >> A separate linker section makes it possible to keep this code either
> > >> in DDR or in some secure memory location provided specifically for the
> > >> purpose.
> > >> 
> > >> So far no one is using this section.
> > >> 
> > >> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> > >> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > >> Cc: Marc Zyngier <marc.zyngier@arm.com>
> > >> ---
> > >>  arch/arm/config.mk            |    2 +-
> > >>  arch/arm/cpu/armv8/u-boot.lds |   30 ++++++++++++++++++++++++++++++
> > >>  2 files changed, 31 insertions(+), 1 deletions(-)
> > >> 
> > >> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> > >> index c339e6d..9272e9c 100644
> > >> --- a/arch/arm/config.mk
> > >> +++ b/arch/arm/config.mk
> > >> @@ -111,7 +111,7 @@ endif
> > >>  
> > >>  # limit ourselves to the sections we want in the .bin.
> > >>  ifdef CONFIG_ARM64
> > >> -OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
> > >> +OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .data -j .u_boot_list -j .rela.dyn
> > >>  else
> > >>  OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
> > >>  endif
> > >> diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
> > >> index 4c12222..bd95fff 100644
> > >> --- a/arch/arm/cpu/armv8/u-boot.lds
> > >> +++ b/arch/arm/cpu/armv8/u-boot.lds
> > >> @@ -8,6 +8,8 @@
> > >>   * SPDX-License-Identifier:	GPL-2.0+
> > >>   */
> > >>  
> > >> +#include <config.h>
> > >> +
> > >>  OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64")
> > >>  OUTPUT_ARCH(aarch64)
> > >>  ENTRY(_start)
> > >> @@ -23,6 +25,34 @@ SECTIONS
> > >>  		*(.text*)
> > >>  	}
> > >>  
> > >> +#ifdef CONFIG_ARMV8_PSCI
> > >> +
> > >> +#ifndef CONFIG_ARMV8_SECURE_BASE
> > >> +#define CONFIG_ARMV8_SECURE_BASE
> > >> +#endif
> > >> +
> > >> +	.__secure_start : {
> > >> +		. = ALIGN(0x1000);
> > >> +		*(.__secure_start)
> > >> +	}
> > >> +
> > >> +	.secure_text CONFIG_ARMV8_SECURE_BASE :
> > >> +		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
> > >> +	{
> > >> +		*(._secure.text)
> > >> +	}
> > >> +
> > >> +	. = LOADADDR(.__secure_start) +
> > >> +		SIZEOF(.__secure_start) +
> > >> +		SIZEOF(.secure_text);
> > >> +
> > >> +	__secure_end_lma = .;
> > >> +	.__secure_end : AT(__secure_end_lma) {
> > >> +		*(.__secure_end)
> > >> +		LONG(0x1d1071c);	/* Must output something to reset LMA */
> > >
> > > Can you explain in more detail what issue this fixes?
> > 
> > If you use AT to force a new load address (LMA), you must ensure that
> > you actually output something at this address. Here, if *(.__secure_end)
> > ends up being empty, whatever follows would be as if the AT never
> > happened, ending up at the wrong LMA.
> >
> > The workaround is to force the output of a dummy value in all
> > cases, ensuring that the rest of the text is at a sensible LMA. This is
> > an issue that has been in GNU ld for years, and this workaround is a
> > copy/paste of the same one in the ARMv7 ld script.
> 
> I see. Does the ld bug have an identifier that we could mention in a
> comment in the linker script as a reference?

Ping.

> > Thanks,
> > 
> > 	M.
> 
> Amicalement,

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 4/8] ARMv8: PSCI: Add linker section to hold PSCI code

[Marc Zyngier]

On 2014-10-11 12:27, Albert ARIBAUD wrote:
> Hi Albert,
>
> On Fri, 19 Sep 2014 18:04:14 +0200, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
>
>> Hi Marc,
>>
>> On Thu, 18 Sep 2014 16:28:52 +0100, Marc Zyngier 
>> <marc.zyngier@arm.com>
>> wrote:
>>
>> > On Thu, Sep 18 2014 at 10:12:17 AM, Albert ARIBAUD 
>> <albert.u.boot@aribaud.net> wrote:
>> > > Hi Arnab,
>> > >
>> > > On Thu, 28 Aug 2014 01:59:57 +0530, Arnab Basu
>> > > <arnab.basu@freescale.com> wrote:
>> > >
>> > >> A separate linker section makes it possible to keep this code 
>> either
>> > >> in DDR or in some secure memory location provided specifically 
>> for the
>> > >> purpose.
>> > >>
>> > >> So far no one is using this section.
>> > >>
>> > >> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
>> > >> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
>> > >> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> > >> ---
>> > >>  arch/arm/config.mk            |    2 +-
>> > >>  arch/arm/cpu/armv8/u-boot.lds |   30 
>> ++++++++++++++++++++++++++++++
>> > >>  2 files changed, 31 insertions(+), 1 deletions(-)
>> > >>
>> > >> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
>> > >> index c339e6d..9272e9c 100644
>> > >> --- a/arch/arm/config.mk
>> > >> +++ b/arch/arm/config.mk
>> > >> @@ -111,7 +111,7 @@ endif
>> > >>
>> > >>  # limit ourselves to the sections we want in the .bin.
>> > >>  ifdef CONFIG_ARM64
>> > >> -OBJCOPYFLAGS += -j .text -j .rodata -j .data -j .u_boot_list 
>> -j .rela.dyn
>> > >> +OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .data 
>> -j .u_boot_list -j .rela.dyn
>> > >>  else
>> > >>  OBJCOPYFLAGS += -j .text -j .secure_text -j .rodata -j .hash 
>> -j .data -j .got.plt -j .u_boot_list -j .rel.dyn
>> > >>  endif
>> > >> diff --git a/arch/arm/cpu/armv8/u-boot.lds 
>> b/arch/arm/cpu/armv8/u-boot.lds
>> > >> index 4c12222..bd95fff 100644
>> > >> --- a/arch/arm/cpu/armv8/u-boot.lds
>> > >> +++ b/arch/arm/cpu/armv8/u-boot.lds
>> > >> @@ -8,6 +8,8 @@
>> > >>   * SPDX-License-Identifier:	GPL-2.0+
>> > >>   */
>> > >>
>> > >> +#include <config.h>
>> > >> +
>> > >>  OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", 
>> "elf64-littleaarch64")
>> > >>  OUTPUT_ARCH(aarch64)
>> > >>  ENTRY(_start)
>> > >> @@ -23,6 +25,34 @@ SECTIONS
>> > >>  		*(.text*)
>> > >>  	}
>> > >>
>> > >> +#ifdef CONFIG_ARMV8_PSCI
>> > >> +
>> > >> +#ifndef CONFIG_ARMV8_SECURE_BASE
>> > >> +#define CONFIG_ARMV8_SECURE_BASE
>> > >> +#endif
>> > >> +
>> > >> +	.__secure_start : {
>> > >> +		. = ALIGN(0x1000);
>> > >> +		*(.__secure_start)
>> > >> +	}
>> > >> +
>> > >> +	.secure_text CONFIG_ARMV8_SECURE_BASE :
>> > >> +		AT(ADDR(.__secure_start) + SIZEOF(.__secure_start))
>> > >> +	{
>> > >> +		*(._secure.text)
>> > >> +	}
>> > >> +
>> > >> +	. = LOADADDR(.__secure_start) +
>> > >> +		SIZEOF(.__secure_start) +
>> > >> +		SIZEOF(.secure_text);
>> > >> +
>> > >> +	__secure_end_lma = .;
>> > >> +	.__secure_end : AT(__secure_end_lma) {
>> > >> +		*(.__secure_end)
>> > >> +		LONG(0x1d1071c);	/* Must output something to reset LMA */
>> > >
>> > > Can you explain in more detail what issue this fixes?
>> >
>> > If you use AT to force a new load address (LMA), you must ensure 
>> that
>> > you actually output something at this address. Here, if 
>> *(.__secure_end)
>> > ends up being empty, whatever follows would be as if the AT never
>> > happened, ending up at the wrong LMA.
>> >
>> > The workaround is to force the output of a dummy value in all
>> > cases, ensuring that the rest of the text is at a sensible LMA. 
>> This is
>> > an issue that has been in GNU ld for years, and this workaround is 
>> a
>> > copy/paste of the same one in the ARMv7 ld script.
>>
>> I see. Does the ld bug have an identifier that we could mention in a
>> comment in the linker script as a reference?

The only report I'm aware of is this one:
https://sourceware.org/bugzilla/show_bug.cgi?id=948

There may be other ways to work around this, but my linker-foo is 
admittedly pretty limited.

Thanks,

         M.
-- 
Fast, cheap, reliable. Pick two.

[U-Boot] [PATCH 5/8] ARMv8: PCSI: Add generic ARMv8 PSCI code

[Arnab Basu]

Implement core support for PSCI. As this is generic code, it doesn't
implement anything really useful (all the functions are returning
Not Implemented).

This is largely ported from the similar code that exists for ARMv7

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/cpu/armv8/Makefile |    1 +
 arch/arm/cpu/armv8/psci.S   |  171 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 172 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/cpu/armv8/psci.S

diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
index 4f0ea87..8f6988d 100644
--- a/arch/arm/cpu/armv8/Makefile
+++ b/arch/arm/cpu/armv8/Makefile
@@ -15,3 +15,4 @@ obj-y	+= cache.o
 obj-y	+= tlb.o
 obj-y	+= transition.o
 obj-y	+= cpu-dt.o
+obj-$(CONFIG_ARMV8_PSCI)	+= psci.o
diff --git a/arch/arm/cpu/armv8/psci.S b/arch/arm/cpu/armv8/psci.S
new file mode 100644
index 0000000..5f4e3b2
--- /dev/null
+++ b/arch/arm/cpu/armv8/psci.S
@@ -0,0 +1,171 @@
+/*
+ * (C) Copyright 2014
+ * Arnab Basu <arnab.basu@freescale.com>
+ *
+ * Based on arch/arm/cpu/armv7/psci.S
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <linux/linkage.h>
+#include <asm/psci.h>
+
+.pushsection ._secure.text, "ax"
+
+ENTRY(psci_0_2_cpu_suspend_64)
+ENTRY(psci_0_2_cpu_on_64)
+ENTRY(psci_0_2_affinity_info_64)
+ENTRY(psci_0_2_migrate_64)
+ENTRY(psci_0_2_migrate_info_up_cpu_64)
+	mov	x0, #ARM_PSCI_RET_NI	/* Return -1 (Not Implemented) */
+	ret
+ENDPROC(psci_0_2_cpu_suspend_64)
+ENDPROC(psci_0_2_cpu_on_64)
+ENDPROC(psci_0_2_affinity_info_64)
+ENDPROC(psci_0_2_migrate_64)
+ENDPROC(psci_0_2_migrate_info_up_cpu_64)
+.weak psci_0_2_cpu_suspend_64
+.weak psci_0_2_cpu_on_64
+.weak psci_0_2_affinity_info_64
+.weak psci_0_2_migrate_64
+.weak psci_0_2_migrate_info_up_cpu_64
+
+ENTRY(psci_0_2_psci_version)
+	mov	x0, #2			/* Return Major = 0, Minor = 2*/
+	ret
+ENDPROC(psci_0_2_psci_version)
+
+.align 4
+_psci_0_2_table:
+	.quad	PSCI_0_2_FN_PSCI_VERSION
+	.quad	psci_0_2_psci_version
+	.quad	PSCI_0_2_FN64_CPU_SUSPEND
+	.quad	psci_0_2_cpu_suspend_64
+	.quad	PSCI_0_2_FN64_CPU_ON
+	.quad	psci_0_2_cpu_on_64
+	.quad	PSCI_0_2_FN64_AFFINITY_INFO
+	.quad	psci_0_2_affinity_info_64
+	.quad	PSCI_0_2_FN64_MIGRATE
+	.quad	psci_0_2_migrate_64
+	.quad	PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU
+	.quad	psci_0_2_migrate_info_up_cpu_64
+	.quad	0
+	.quad	0
+
+.macro	psci_enter
+	stp	x29, x30, [sp, #-16]!
+	stp	x27, x28, [sp, #-16]!
+	stp	x25, x26, [sp, #-16]!
+	stp	x23, x24, [sp, #-16]!
+	stp	x21, x22, [sp, #-16]!
+	stp	x19, x20, [sp, #-16]!
+	stp	x17, x18, [sp, #-16]!
+	stp	x15, x16, [sp, #-16]!
+	stp	x13, x14, [sp, #-16]!
+	stp	x11, x12, [sp, #-16]!
+	stp	x9, x10, [sp, #-16]!
+	stp	x7, x8, [sp, #-16]!
+	stp	x5, x6, [sp, #-16]!
+	mrs	x5, elr_el3
+	stp	x5, x4, [sp, #-16]!
+
+	/* EL0 and El1 will execute in secure */
+	mrs	x4, scr_el3
+	bic	x4, x4, #1
+	msr	scr_el3, x4
+.endm
+
+.macro	psci_return
+	/* EL0 and El1 will execute in non-secure */
+	mrs	x4, scr_el3
+	orr	x4, x4, #1
+	msr	scr_el3, x4
+
+	ldp	x5, x4, [sp], #16
+	msr	elr_el3, x5
+	ldp	x5, x6, [sp], #16
+	ldp	x7, x8, [sp], #16
+	ldp	x9, x10, [sp], #16
+	ldp	x11, x12, [sp], #16
+	ldp	x13, x14, [sp], #16
+	ldp	x15, x16, [sp], #16
+	ldp	x17, x18, [sp], #16
+	ldp	x19, x20, [sp], #16
+	ldp	x21, x22, [sp], #16
+	ldp	x23, x24, [sp], #16
+	ldp	x25, x26, [sp], #16
+	ldp	x27, x28, [sp], #16
+	ldp	x29, x30, [sp], #16
+	eret
+.endm
+
+ENTRY(_smc_psci)
+	psci_enter
+	adr	x4, _psci_0_2_table
+1:	ldr	x5, [x4]		/* Load PSCI function ID */
+	ldr	x6, [x4, #8]		/* Load target PC */
+	cmp	x5, #0			/* If reach the end, bail out */
+	b.eq	fn_not_found
+	cmp	x0, x5			/* If not matching, try next entry */
+	b.eq	fn_call
+	add	x4, x4, #16
+	b	1b
+
+fn_call:
+	blr	x6
+	psci_return
+
+fn_not_found:
+	mov	x0, #ARM_PSCI_RET_INVAL  /* Return -2 (Invalid) */
+	psci_return
+ENDPROC(_smc_psci)
+
+ENTRY(default_psci_vector)
+	eret
+ENDPROC(default_psci_vector)
+
+.align 2
+__handle_sync:
+	str 	x4, [sp, #-8]!
+	mrs	x4, esr_el3
+	ubfx	x4, x4, #26, #6
+	cmp	x4, #0x17
+	b.eq	smc_found
+	ldr	x4, [sp], #8
+	b	default_psci_vector
+smc_found:
+	ldr     x4, [sp], #8
+	b	_smc_psci
+
+/*
+ * PSCI Exception vectors.
+ */
+	.align	11
+	.globl	psci_vectors
+psci_vectors:
+	.align	7
+	b	default_psci_vector	/* Current EL Synchronous Thread */
+	.align	7
+	b	default_psci_vector	/* Current EL IRQ Thread */
+	.align	7
+	b	default_psci_vector	/* Current EL FIQ Thread */
+	.align	7
+	b	default_psci_vector	/* Current EL Error Thread */
+	.align	7
+	b	default_psci_vector	/* Current EL Synchronous Handler */
+	.align	7
+	b	default_psci_vector	/* Current EL IRQ Handler */
+	.align	7
+	b	default_psci_vector	/* Current EL FIQ Handler */
+	.align	7
+	b	default_psci_vector	/* Current EL Error Handler */
+	.align	7
+	b	__handle_sync		/* Lower EL Synchronous (64b) */
+	.align	7
+	b	default_psci_vector	/* Lower EL IRQ (64b) */
+	.align	7
+	b	default_psci_vector	/* Lower EL FIQ (64b) */
+	.align	7
+	b	default_psci_vector	/* Lower EL Error (64b) */
+
+.popsection
-- 
1.7.7.4

[U-Boot] [PATCH 5/8] ARMv8: PCSI: Add generic ARMv8 PSCI code

[Mark Rutland]

Hi Arnab,

On Wed, Aug 27, 2014 at 09:29:58PM +0100, Arnab Basu wrote:
> Implement core support for PSCI. As this is generic code, it doesn't
> implement anything really useful (all the functions are returning
> Not Implemented).

This is really nice to see! Thanks for working on this.

Some functions which return NOT_IMPLEMENTED below are requried to be
implemented per the PSCI 0.2 spec.

I hope that the plan is to implement all the required functions before
we turn this on?

Otherwise, comments below.

> This is largely ported from the similar code that exists for ARMv7
> 
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/cpu/armv8/Makefile |    1 +
>  arch/arm/cpu/armv8/psci.S   |  171 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 172 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/cpu/armv8/psci.S
> 
> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> index 4f0ea87..8f6988d 100644
> --- a/arch/arm/cpu/armv8/Makefile
> +++ b/arch/arm/cpu/armv8/Makefile
> @@ -15,3 +15,4 @@ obj-y	+= cache.o
>  obj-y	+= tlb.o
>  obj-y	+= transition.o
>  obj-y	+= cpu-dt.o
> +obj-$(CONFIG_ARMV8_PSCI)	+= psci.o
> diff --git a/arch/arm/cpu/armv8/psci.S b/arch/arm/cpu/armv8/psci.S
> new file mode 100644
> index 0000000..5f4e3b2
> --- /dev/null
> +++ b/arch/arm/cpu/armv8/psci.S
> @@ -0,0 +1,171 @@
> +/*
> + * (C) Copyright 2014
> + * Arnab Basu <arnab.basu@freescale.com>
> + *
> + * Based on arch/arm/cpu/armv7/psci.S
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/psci.h>
> +
> +.pushsection ._secure.text, "ax"
> +
> +ENTRY(psci_0_2_cpu_suspend_64)
> +ENTRY(psci_0_2_cpu_on_64)
> +ENTRY(psci_0_2_affinity_info_64)
> +ENTRY(psci_0_2_migrate_64)
> +ENTRY(psci_0_2_migrate_info_up_cpu_64)
> +	mov	x0, #ARM_PSCI_RET_NI	/* Return -1 (Not Implemented) */
> +	ret
> +ENDPROC(psci_0_2_cpu_suspend_64)
> +ENDPROC(psci_0_2_cpu_on_64)
> +ENDPROC(psci_0_2_affinity_info_64)
> +ENDPROC(psci_0_2_migrate_64)
> +ENDPROC(psci_0_2_migrate_info_up_cpu_64)
> +.weak psci_0_2_cpu_suspend_64
> +.weak psci_0_2_cpu_on_64
> +.weak psci_0_2_affinity_info_64
> +.weak psci_0_2_migrate_64
> +.weak psci_0_2_migrate_info_up_cpu_64
> +
> +ENTRY(psci_0_2_psci_version)
> +	mov	x0, #2			/* Return Major = 0, Minor = 2*/
> +	ret
> +ENDPROC(psci_0_2_psci_version)
> +
> +.align 4
> +_psci_0_2_table:
> +	.quad	PSCI_0_2_FN_PSCI_VERSION
> +	.quad	psci_0_2_psci_version
> +	.quad	PSCI_0_2_FN64_CPU_SUSPEND
> +	.quad	psci_0_2_cpu_suspend_64
> +	.quad	PSCI_0_2_FN64_CPU_ON
> +	.quad	psci_0_2_cpu_on_64
> +	.quad	PSCI_0_2_FN64_AFFINITY_INFO
> +	.quad	psci_0_2_affinity_info_64
> +	.quad	PSCI_0_2_FN64_MIGRATE
> +	.quad	psci_0_2_migrate_64
> +	.quad	PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU
> +	.quad	psci_0_2_migrate_info_up_cpu_64
> +	.quad	0
> +	.quad	0

It would be nice if we could reorganise this something like:

	.quad PSCI_0_2_FN_PSCI_VERSION,		psci_0_2_psci_version
	.quad PSCI_0_2_FN64_CPU_SUSPEND,	psci_0_2_cpu_suspend_64
	.quad PSCI_0_2_FN64_CPU_ON,		psci_0_2_cpu_on_64
	.quad PSCI_0_2_FN64_AFFINITY_INFO,	psci_0_2_affinity_info_64
	.quad PSCI_0_2_FN64_MIGRATE,		psci_0_2_migrate_64
	.quad PSCI_0_2_FN64_MIGRATE_INFO_UP_CPU,	psci_0_2_migrate_info_up_cpu_64
	.quad 0, 				0

As that would make the relationship between IDs and functions clearer
(at least to me). Maybe a macro could make this less painful.

> +.macro	psci_enter
> +	stp	x29, x30, [sp, #-16]!
> +	stp	x27, x28, [sp, #-16]!
> +	stp	x25, x26, [sp, #-16]!
> +	stp	x23, x24, [sp, #-16]!
> +	stp	x21, x22, [sp, #-16]!
> +	stp	x19, x20, [sp, #-16]!
> +	stp	x17, x18, [sp, #-16]!
> +	stp	x15, x16, [sp, #-16]!
> +	stp	x13, x14, [sp, #-16]!
> +	stp	x11, x12, [sp, #-16]!
> +	stp	x9, x10, [sp, #-16]!
> +	stp	x7, x8, [sp, #-16]!
> +	stp	x5, x6, [sp, #-16]!
> +	mrs	x5, elr_el3
> +	stp	x5, x4, [sp, #-16]!
> +
> +	/* EL0 and El1 will execute in secure */

I think this would be better as:

	/* U-Boot will run on the secure side */

> +	mrs	x4, scr_el3
> +	bic	x4, x4, #1
> +	msr	scr_el3, x4
> +.endm
> +
> +.macro	psci_return
> +	/* EL0 and El1 will execute in non-secure */

Similarly:

	/* The OS will run on the non-secure side */

> +	mrs	x4, scr_el3
> +	orr	x4, x4, #1
> +	msr	scr_el3, x4
> +
> +	ldp	x5, x4, [sp], #16
> +	msr	elr_el3, x5
> +	ldp	x5, x6, [sp], #16
> +	ldp	x7, x8, [sp], #16
> +	ldp	x9, x10, [sp], #16
> +	ldp	x11, x12, [sp], #16
> +	ldp	x13, x14, [sp], #16
> +	ldp	x15, x16, [sp], #16
> +	ldp	x17, x18, [sp], #16
> +	ldp	x19, x20, [sp], #16
> +	ldp	x21, x22, [sp], #16
> +	ldp	x23, x24, [sp], #16
> +	ldp	x25, x26, [sp], #16
> +	ldp	x27, x28, [sp], #16
> +	ldp	x29, x30, [sp], #16
> +	eret
> +.endm

PSCI 0.2 follows the SMC calling convention, where x0-x17 are
caller-saved as with the usual AArch64 calling conventions, so we don't
need to save/restore them here for AArch64 callers.

We do need to ensure we don't clobber x18-x30, sp_elx and sp_el0. The
calling convention is ambiguous as to whether we can clobber sp_el1, but
for the moment I'd assume we shouldn't.

For AArch32 callers, only x0-x3 are caller-saved.

> +ENTRY(_smc_psci)
> +	psci_enter
> +	adr	x4, _psci_0_2_table
> +1:	ldr	x5, [x4]		/* Load PSCI function ID */
> +	ldr	x6, [x4, #8]		/* Load target PC */

1:	ldp	x5, x6, [x4]		/* Load function ID and handler addr */

> +	cmp	x5, #0			/* If reach the end, bail out */
> +	b.eq	fn_not_found

cbz	x5, fn_not_found		/* If reach the end, bail out */

> +	cmp	x0, x5			/* If not matching, try next entry */
> +	b.eq	fn_call
> +	add	x4, x4, #16
> +	b	1b
> +
> +fn_call:
> +	blr	x6
> +	psci_return
> +
> +fn_not_found:
> +	mov	x0, #ARM_PSCI_RET_INVAL  /* Return -2 (Invalid) */

I believe this should be NOT_SUPPORTED (-1), which states that the PSCI
implementation doesn't implement this function rather than the arguments
to the function were invalid.

> +	psci_return
> +ENDPROC(_smc_psci)
> +
> +ENTRY(default_psci_vector)
> +	eret
> +ENDPROC(default_psci_vector)

This looks misnamed. This is the vector for exceptions triggered by
something other than PSCI.

That said I think we need to do something else here. If we haven't
handled the exception we're likely to just return to whatever caused it
and take the same exception again.

A failstop behaviour would seem better if the only other exceptions we
expect to take are fatal anyway.

> +.align 2

I don't think this is necessary; we had code immediately before.

> +__handle_sync:
> +	str 	x4, [sp, #-8]!
> +	mrs	x4, esr_el3
> +	ubfx	x4, x4, #26, #6
> +	cmp	x4, #0x17

Could we not have a macro like ESR_EC_SMC64 instead of the #17?

Thanks,
Mark.

> +	b.eq	smc_found
> +	ldr	x4, [sp], #8
> +	b	default_psci_vector
> +smc_found:
> +	ldr     x4, [sp], #8
> +	b	_smc_psci
> +
> +/*
> + * PSCI Exception vectors.
> + */
> +	.align	11
> +	.globl	psci_vectors
> +psci_vectors:
> +	.align	7
> +	b	default_psci_vector	/* Current EL Synchronous Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL IRQ Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL FIQ Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL Error Thread */
> +	.align	7
> +	b	default_psci_vector	/* Current EL Synchronous Handler */
> +	.align	7
> +	b	default_psci_vector	/* Current EL IRQ Handler */
> +	.align	7
> +	b	default_psci_vector	/* Current EL FIQ Handler */
> +	.align	7
> +	b	default_psci_vector	/* Current EL Error Handler */
> +	.align	7
> +	b	__handle_sync		/* Lower EL Synchronous (64b) */
> +	.align	7
> +	b	default_psci_vector	/* Lower EL IRQ (64b) */
> +	.align	7
> +	b	default_psci_vector	/* Lower EL FIQ (64b) */
> +	.align	7
> +	b	default_psci_vector	/* Lower EL Error (64b) */
> +
> +.popsection
> -- 
> 1.7.7.4
> 
> 

[U-Boot] [PATCH 6/8] ARMv8: PSCI: Fixup the device tree for PSCI v0.2

[Arnab Basu]

Set the enable-method in the cpu node to psci, create the psci
device tree node and also add a reserve section for the psci code
that lives in in normal RAM, so that the kernel leaves it alone

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/cpu/armv8/cpu-dt.c   |   67 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/system.h |    4 ++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c
index 9792bc0..c2c8fe7 100644
--- a/arch/arm/cpu/armv8/cpu-dt.c
+++ b/arch/arm/cpu/armv8/cpu-dt.c
@@ -9,7 +9,69 @@
 #include <fdt_support.h>
 
 #ifdef CONFIG_MP
+#ifdef CONFIG_ARMV8_PSCI
 
+static void psci_reserve_mem(void *fdt)
+{
+#ifndef CONFIG_ARMV8_SECURE_BASE
+	/* secure code lives in RAM, keep it alive */
+	fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
+			__secure_end - __secure_start);
+#endif
+}
+
+static int cpu_update_dt_psci(void *fdt)
+{
+	int nodeoff;
+	int tmp;
+
+	psci_reserve_mem(fdt);
+
+	nodeoff = fdt_path_offset(fdt, "/cpus");
+	if (nodeoff < 0) {
+		printf("couldn't find /cpus\n");
+		return nodeoff;
+	}
+
+	/* add 'enable-method = "psci"' to each cpu node */
+	for (tmp = fdt_first_subnode(fdt, nodeoff);
+	     tmp >= 0;
+	     tmp = fdt_next_subnode(fdt, tmp)) {
+		const struct fdt_property *prop;
+		int len;
+
+		prop = fdt_get_property(fdt, tmp, "device_type", &len);
+		if (!prop)
+			continue;
+		if (len < 4)
+			continue;
+		if (strcmp(prop->data, "cpu"))
+			continue;
+
+		fdt_setprop_string(fdt, tmp, "enable-method", "psci");
+	}
+
+	nodeoff = fdt_path_offset(fdt, "/psci");
+	if (nodeoff < 0) {
+		nodeoff = fdt_path_offset(fdt, "/");
+		if (nodeoff < 0)
+			return nodeoff;
+
+		nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
+		if (nodeoff < 0)
+			return nodeoff;
+	}
+
+	tmp = fdt_setprop_string(fdt, nodeoff, "compatible", "arm,psci-0.2");
+	if (tmp)
+		return tmp;
+	tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
+	if (tmp)
+		return tmp;
+
+	return 0;
+}
+#else
 __weak u64 arch_get_release_addr(u64 cpu_id)
 {
 	return 0;
@@ -51,11 +113,16 @@ static void cpu_update_dt_spin_table(void *blob)
 	arch_spin_table_reserve_mem(blob);
 }
 #endif
+#endif
 
 int cpu_update_dt(void *fdt)
 {
 #ifdef CONFIG_MP
+#ifdef CONFIG_ARMV8_PSCI
+	cpu_update_dt_psci(fdt);
+#else
 	cpu_update_dt_spin_table(fdt);
 #endif
+#endif
 	return 0;
 }
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index d51ba66..a1066d4 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -77,6 +77,10 @@ void gic_init(void);
 void gic_send_sgi(unsigned long sgino);
 void wait_for_wakeup(void);
 void smp_kick_all_cpus(void);
+#ifdef CONFIG_ARMV8_PSCI
+extern char __secure_start[];
+extern char __secure_end[];
+#endif
 
 void flush_l3_cache(void);
 
-- 
1.7.7.4

[U-Boot] [PATCH 6/8] ARMv8: PSCI: Fixup the device tree for PSCI v0.2

[Mark Rutland]

Hi,

On Wed, Aug 27, 2014 at 09:29:59PM +0100, Arnab Basu wrote:
> Set the enable-method in the cpu node to psci, create the psci
> device tree node and also add a reserve section for the psci code
> that lives in in normal RAM, so that the kernel leaves it alone
> 
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/cpu/armv8/cpu-dt.c   |   67 +++++++++++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/system.h |    4 ++
>  2 files changed, 71 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c
> index 9792bc0..c2c8fe7 100644
> --- a/arch/arm/cpu/armv8/cpu-dt.c
> +++ b/arch/arm/cpu/armv8/cpu-dt.c
> @@ -9,7 +9,69 @@
>  #include <fdt_support.h>
>  
>  #ifdef CONFIG_MP
> +#ifdef CONFIG_ARMV8_PSCI
>  
> +static void psci_reserve_mem(void *fdt)
> +{
> +#ifndef CONFIG_ARMV8_SECURE_BASE
> +	/* secure code lives in RAM, keep it alive */
> +	fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
> +			__secure_end - __secure_start);
> +#endif
> +}

With PSCI I'd be worried about telling the OS about this memory at all.

If the OS maps the memory we could encounter issues with mismatched
aliases and/or unexpected hits in the D-cache, which can result in a
loss of ordering and/or visbility guarantees, which could break the PSCI
implementation.

With the KVM or trusted firmware PSCI implementations the (guest) OS
cannot map the implementation's memory, preventing such problems. The
arm64 Linux boot-wrapper is dodgy in this regard currently.

> +static int cpu_update_dt_psci(void *fdt)
> +{
> +	int nodeoff;
> +	int tmp;
> +
> +	psci_reserve_mem(fdt);
> +
> +	nodeoff = fdt_path_offset(fdt, "/cpus");
> +	if (nodeoff < 0) {
> +		printf("couldn't find /cpus\n");
> +		return nodeoff;
> +	}
> +
> +	/* add 'enable-method = "psci"' to each cpu node */
> +	for (tmp = fdt_first_subnode(fdt, nodeoff);
> +	     tmp >= 0;
> +	     tmp = fdt_next_subnode(fdt, tmp)) {
> +		const struct fdt_property *prop;
> +		int len;
> +
> +		prop = fdt_get_property(fdt, tmp, "device_type", &len);
> +		if (!prop)
> +			continue;
> +		if (len < 4)
> +			continue;
> +		if (strcmp(prop->data, "cpu"))
> +			continue;
> +
> +		fdt_setprop_string(fdt, tmp, "enable-method", "psci");

Do we need to check the return code here, as we do when setting up the
psci node?

> +	}
> +
> +	nodeoff = fdt_path_offset(fdt, "/psci");

We might need to search by compatible string. All psci nodes so far have
been called /psci, but that's not guaranteed. Linux looks for nodes
compatible with "arm,psci" and/or "arm,psci-0.2".

> +	if (nodeoff < 0) {
> +		nodeoff = fdt_path_offset(fdt, "/");
> +		if (nodeoff < 0)
> +			return nodeoff;
> +
> +		nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
> +		if (nodeoff < 0)
> +			return nodeoff;
> +	}
> +
> +	tmp = fdt_setprop_string(fdt, nodeoff, "compatible", "arm,psci-0.2");
> +	if (tmp)
> +		return tmp;
> +	tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
> +	if (tmp)
> +		return tmp;
> +
> +	return 0;
> +}

Otherwise this looks fine.

Mark.

[U-Boot] [PATCH 6/8] ARMv8: PSCI: Fixup the device tree for PSCI v0.2

[Arnab Basu]

Hi Mark

On 08/28/2014 06:14 PM, Mark Rutland wrote:
> Hi,
>
> On Wed, Aug 27, 2014 at 09:29:59PM +0100, Arnab Basu wrote:
>> Set the enable-method in the cpu node to psci, create the psci
>> device tree node and also add a reserve section for the psci code
>> that lives in in normal RAM, so that the kernel leaves it alone
>>
>> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
>> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
>> Cc: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>   arch/arm/cpu/armv8/cpu-dt.c   |   67 +++++++++++++++++++++++++++++++++++++++++
>>   arch/arm/include/asm/system.h |    4 ++
>>   2 files changed, 71 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c
>> index 9792bc0..c2c8fe7 100644
>> --- a/arch/arm/cpu/armv8/cpu-dt.c
>> +++ b/arch/arm/cpu/armv8/cpu-dt.c
>> @@ -9,7 +9,69 @@
>>   #include <fdt_support.h>
>>
>>   #ifdef CONFIG_MP
>> +#ifdef CONFIG_ARMV8_PSCI
>>
>> +static void psci_reserve_mem(void *fdt)
>> +{
>> +#ifndef CONFIG_ARMV8_SECURE_BASE
>> +	/* secure code lives in RAM, keep it alive */
>> +	fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
>> +			__secure_end - __secure_start);
>> +#endif
>> +}
>
> With PSCI I'd be worried about telling the OS about this memory at all.
>
> If the OS maps the memory we could encounter issues with mismatched
> aliases and/or unexpected hits in the D-cache, which can result in a
> loss of ordering and/or visbility guarantees, which could break the PSCI
> implementation.
>
> With the KVM or trusted firmware PSCI implementations the (guest) OS
> cannot map the implementation's memory, preventing such problems. The
> arm64 Linux boot-wrapper is dodgy in this regard currently.
>

The idea here is that if there is no PSCI specific (most likely secure) 
memory allocated in the system, the macro "CONFIG_ARMV8_SECURE_BASE" 
will not be defined. In this case the PSCI vector table and its support 
code will be in DDR and will be protected from Linux using memreserve.

If this macro is defined the assumption is that it points to some 
non-ddr location, say secure OCRAM. In this case U-Boot will copy the 
PSCI vector table and its support code to that region and we are hoping 
that this address space is not visible to the OS in the first place.

This is my understanding of the code, maybe Marc would like to comment 
on if this was the thinking in ARMv7.

I realise that this probably needs to be documented.

>> +static int cpu_update_dt_psci(void *fdt)
>> +{
>> +	int nodeoff;
>> +	int tmp;
>> +
>> +	psci_reserve_mem(fdt);
>> +
>> +	nodeoff = fdt_path_offset(fdt, "/cpus");
>> +	if (nodeoff < 0) {
>> +		printf("couldn't find /cpus\n");
>> +		return nodeoff;
>> +	}
>> +
>> +	/* add 'enable-method = "psci"' to each cpu node */
>> +	for (tmp = fdt_first_subnode(fdt, nodeoff);
>> +	     tmp >= 0;
>> +	     tmp = fdt_next_subnode(fdt, tmp)) {
>> +		const struct fdt_property *prop;
>> +		int len;
>> +
>> +		prop = fdt_get_property(fdt, tmp, "device_type", &len);
>> +		if (!prop)
>> +			continue;
>> +		if (len < 4)
>> +			continue;
>> +		if (strcmp(prop->data, "cpu"))
>> +			continue;
>> +
>> +		fdt_setprop_string(fdt, tmp, "enable-method", "psci");
>
> Do we need to check the return code here, as we do when setting up the
> psci node?
>

Probably, I'll add it.

>> +	}
>> +
>> +	nodeoff = fdt_path_offset(fdt, "/psci");
>
> We might need to search by compatible string. All psci nodes so far have
> been called /psci, but that's not guaranteed. Linux looks for nodes
> compatible with "arm,psci" and/or "arm,psci-0.2".
>

I see that it is called "Main node" in the kernel documentation. Any 
reason it's name has not been fixed to "psci"? Is it too late to do that 
and save myself some work here? :)

>> +	if (nodeoff < 0) {
>> +		nodeoff = fdt_path_offset(fdt, "/");
>> +		if (nodeoff < 0)
>> +			return nodeoff;
>> +
>> +		nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
>> +		if (nodeoff < 0)
>> +			return nodeoff;
>> +	}
>> +
>> +	tmp = fdt_setprop_string(fdt, nodeoff, "compatible", "arm,psci-0.2");
>> +	if (tmp)
>> +		return tmp;
>> +	tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
>> +	if (tmp)
>> +		return tmp;
>> +
>> +	return 0;
>> +}
>
> Otherwise this looks fine.

Thanks

>
> Mark.
>

[U-Boot] [PATCH 6/8] ARMv8: PSCI: Fixup the device tree for PSCI v0.2

[Mark Rutland]

Hi,

> >> diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c
> >> index 9792bc0..c2c8fe7 100644
> >> --- a/arch/arm/cpu/armv8/cpu-dt.c
> >> +++ b/arch/arm/cpu/armv8/cpu-dt.c
> >> @@ -9,7 +9,69 @@
> >>   #include <fdt_support.h>
> >>
> >>   #ifdef CONFIG_MP
> >> +#ifdef CONFIG_ARMV8_PSCI
> >>
> >> +static void psci_reserve_mem(void *fdt)
> >> +{
> >> +#ifndef CONFIG_ARMV8_SECURE_BASE
> >> +	/* secure code lives in RAM, keep it alive */
> >> +	fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
> >> +			__secure_end - __secure_start);
> >> +#endif
> >> +}
> >
> > With PSCI I'd be worried about telling the OS about this memory at all.
> >
> > If the OS maps the memory we could encounter issues with mismatched
> > aliases and/or unexpected hits in the D-cache, which can result in a
> > loss of ordering and/or visbility guarantees, which could break the PSCI
> > implementation.
> >
> > With the KVM or trusted firmware PSCI implementations the (guest) OS
> > cannot map the implementation's memory, preventing such problems. The
> > arm64 Linux boot-wrapper is dodgy in this regard currently.
> >
> 
> The idea here is that if there is no PSCI specific (most likely secure) 
> memory allocated in the system, the macro "CONFIG_ARMV8_SECURE_BASE" 
> will not be defined. In this case the PSCI vector table and its support 
> code will be in DDR and will be protected from Linux using memreserve.

Sure, this will prevent the OS from explicitly modifying this memory.

However, the OS will still map the memory. This renders the protection
incomplete due to the possibility of mismatched attributes and/or
unexpected cache hits resulting in nasty coherency problems. We are
likely to get away with this most of the time (if the kernel and U-Boot
use the same attributes), but it would be very easy to blow things up
accidentally.

The only way to prevent that is to completely remove a portion of the
memory from the view of the OS, such that it doesn't map the memory at
all.

> If this macro is defined the assumption is that it points to some 
> non-ddr location, say secure OCRAM. In this case U-Boot will copy the 
> PSCI vector table and its support code to that region and we are hoping 
> that this address space is not visible to the OS in the first place.

This makes sense, but was not the issue I was referring to.

> This is my understanding of the code, maybe Marc would like to comment 
> on if this was the thinking in ARMv7.

If we're doing this on ARMv7 then it is dodgy there too.

Marc, thoughts?

[...]

> >> +	}
> >> +
> >> +	nodeoff = fdt_path_offset(fdt, "/psci");
> >
> > We might need to search by compatible string. All psci nodes so far have
> > been called /psci, but that's not guaranteed. Linux looks for nodes
> > compatible with "arm,psci" and/or "arm,psci-0.2".
> >
> 
> I see that it is called "Main node" in the kernel documentation. Any 
> reason it's name has not been fixed to "psci"? Is it too late to do that 
> and save myself some work here? :)

Unfortunately the canonical way to find the PSCI node is by compatible
string, and that's what Linux does. While we might be able to ensure all
in-tree dts follow this convention, it's not something that should be
relied upon.

Sorry :(

Cheers,
Mark.

[U-Boot] [PATCH 6/8] ARMv8: PSCI: Fixup the device tree for PSCI v0.2

[Mark Rutland]

On Mon, Sep 01, 2014 at 07:43:18PM +0100, Mark Rutland wrote:
> Hi,
> 
> > >> diff --git a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c
> > >> index 9792bc0..c2c8fe7 100644
> > >> --- a/arch/arm/cpu/armv8/cpu-dt.c
> > >> +++ b/arch/arm/cpu/armv8/cpu-dt.c
> > >> @@ -9,7 +9,69 @@
> > >>   #include <fdt_support.h>
> > >>
> > >>   #ifdef CONFIG_MP
> > >> +#ifdef CONFIG_ARMV8_PSCI
> > >>
> > >> +static void psci_reserve_mem(void *fdt)
> > >> +{
> > >> +#ifndef CONFIG_ARMV8_SECURE_BASE
> > >> +	/* secure code lives in RAM, keep it alive */
> > >> +	fdt_add_mem_rsv(fdt, (unsigned long)__secure_start,
> > >> +			__secure_end - __secure_start);
> > >> +#endif
> > >> +}
> > >
> > > With PSCI I'd be worried about telling the OS about this memory at all.
> > >
> > > If the OS maps the memory we could encounter issues with mismatched
> > > aliases and/or unexpected hits in the D-cache, which can result in a
> > > loss of ordering and/or visbility guarantees, which could break the PSCI
> > > implementation.
> > >
> > > With the KVM or trusted firmware PSCI implementations the (guest) OS
> > > cannot map the implementation's memory, preventing such problems. The
> > > arm64 Linux boot-wrapper is dodgy in this regard currently.
> > >
> > 
> > The idea here is that if there is no PSCI specific (most likely secure) 
> > memory allocated in the system, the macro "CONFIG_ARMV8_SECURE_BASE" 
> > will not be defined. In this case the PSCI vector table and its support 
> > code will be in DDR and will be protected from Linux using memreserve.
> 
> Sure, this will prevent the OS from explicitly modifying this memory.
> 
> However, the OS will still map the memory. This renders the protection
> incomplete due to the possibility of mismatched attributes and/or
> unexpected cache hits resulting in nasty coherency problems. We are
> likely to get away with this most of the time (if the kernel and U-Boot
> use the same attributes), but it would be very easy to blow things up
> accidentally.
> 
> The only way to prevent that is to completely remove a portion of the
> memory from the view of the OS, such that it doesn't map the memory at
> all.

To clarify:

If the PSCI implementation uses some memory not described to the OS
there is no problem. Ideally this would be some secure SRAM somwhere,
which the OS can never map. So if you are using some secure RAM then
there is no issue.

If the memory is described to the non-secure OS, then there can be
coherency issues unless either:

 * The caches are not in use at EL3. This necessitates something like
   bakery locks for synchronization.

 * The memory is mapped at EL3 as secure, and the core makes a
   distinction between secure and non-secure memory (see
   ID_AA64MMFR0_EL1.SNSMem). Otherwise misatched attributes can cause
   coherency issues (see B2.9 "Mismatched memory attributes" in the ARM
   ARM).

Thanks,
Mark.

[U-Boot] [PATCH 6/8] ARMv8: PSCI: Fixup the device tree for PSCI v0.2

[Stuart Yoder]

> > The idea here is that if there is no PSCI specific (most likely secure)
> > memory allocated in the system, the macro "CONFIG_ARMV8_SECURE_BASE"
> > will not be defined. In this case the PSCI vector table and its support
> > code will be in DDR and will be protected from Linux using memreserve.
> 
> Sure, this will prevent the OS from explicitly modifying this memory.
> 
> However, the OS will still map the memory. This renders the protection
> incomplete due to the possibility of mismatched attributes and/or
> unexpected cache hits resulting in nasty coherency problems. We are
> likely to get away with this most of the time (if the kernel and U-Boot
> use the same attributes), but it would be very easy to blow things up
> accidentally.
> 
> The only way to prevent that is to completely remove a portion of the
> memory from the view of the OS, such that it doesn't map the memory at
> all.

Can't this be done by simply removing that secure portion of memory
from the memory advertised in the memory node of the device tree passed
to the non-secure OS?  ...should prevent the OS from mapping the memory.

Stuart

[U-Boot] [PATCH 6/8] ARMv8: PSCI: Fixup the device tree for PSCI v0.2

[Mark Rutland]

On Tue, Sep 02, 2014 at 04:21:24PM +0100, Stuart Yoder wrote:
> > > The idea here is that if there is no PSCI specific (most likely secure)
> > > memory allocated in the system, the macro "CONFIG_ARMV8_SECURE_BASE"
> > > will not be defined. In this case the PSCI vector table and its support
> > > code will be in DDR and will be protected from Linux using memreserve.
> > 
> > Sure, this will prevent the OS from explicitly modifying this memory.
> > 
> > However, the OS will still map the memory. This renders the protection
> > incomplete due to the possibility of mismatched attributes and/or
> > unexpected cache hits resulting in nasty coherency problems. We are
> > likely to get away with this most of the time (if the kernel and U-Boot
> > use the same attributes), but it would be very easy to blow things up
> > accidentally.
> > 
> > The only way to prevent that is to completely remove a portion of the
> > memory from the view of the OS, such that it doesn't map the memory at
> > all.
> 
> Can't this be done by simply removing that secure portion of memory
> from the memory advertised in the memory node of the device tree passed
> to the non-secure OS?  ...should prevent the OS from mapping the memory.

Yes, removing such memory entirely from the memory nodes would work.

The only caveat (I believe) is that it would be necessary to remove such
memory in 2MB naturally-aligned chunks due to the way Linux maps memory.

I intend to at some point decouple the Linux linear mapping from the
text mapping, so that Linux can address meemory below it. So it's vital
to remove the memory enitrely from the view of the kernel rather than
just loading the kernel 2MB higher.

Mark.

[U-Boot] [PATCH 7/8] ARMv8: PSCI: Setup ARMv8 PSCI

[Arnab Basu]

Setup the ARMv8 PSCI code just before switching to EL2 and jumping
to the kernel.

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/cpu/armv8/cpu.c      |   23 +++++++++++++++++++++++
 arch/arm/cpu/armv8/psci.S     |    6 ++++++
 arch/arm/include/asm/system.h |    3 +++
 arch/arm/lib/bootm.c          |    3 +++
 4 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv8/cpu.c b/arch/arm/cpu/armv8/cpu.c
index e06c3cc..55d2654 100644
--- a/arch/arm/cpu/armv8/cpu.c
+++ b/arch/arm/cpu/armv8/cpu.c
@@ -41,3 +41,26 @@ int cleanup_before_linux(void)
 
 	return 0;
 }
+
+#ifdef CONFIG_ARMV8_PSCI
+
+static void relocate_secure_section(void)
+{
+#ifdef CONFIG_ARMV8_SECURE_BASE
+	size_t sz = __secure_end - __secure_start;
+
+	memcpy((void *)CONFIG_ARMV8_SECURE_BASE, __secure_start, sz);
+	flush_dcache_range(CONFIG_ARMV8_SECURE_BASE,
+			   CONFIG_ARMV8_SECURE_BASE + sz + 1);
+	invalidate_icache_all();
+#endif
+}
+
+void setup_psci(void)
+{
+	relocate_secure_section();
+	fixup_vectors();
+	psci_arch_init();
+}
+
+#endif
diff --git a/arch/arm/cpu/armv8/psci.S b/arch/arm/cpu/armv8/psci.S
index 5f4e3b2..9eaad3a 100644
--- a/arch/arm/cpu/armv8/psci.S
+++ b/arch/arm/cpu/armv8/psci.S
@@ -169,3 +169,9 @@ psci_vectors:
 	b	default_psci_vector	/* Lower EL Error (64b) */
 
 .popsection
+
+ENTRY(fixup_vectors)
+        adr     x0, psci_vectors
+        msr     vbar_el3, x0
+        ret
+ENDPROC(fixup_vectors)
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index a1066d4..214fed3 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -78,6 +78,9 @@ void gic_send_sgi(unsigned long sgino);
 void wait_for_wakeup(void);
 void smp_kick_all_cpus(void);
 #ifdef CONFIG_ARMV8_PSCI
+void setup_psci(void);
+void fixup_vectors(void);
+void psci_arch_init(void);
 extern char __secure_start[];
 extern char __secure_end[];
 #endif
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 178e8fb..576c7d5 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -251,6 +251,9 @@ static void boot_jump_linux(bootm_headers_t *images, int flag)
 	announce_and_cleanup(fake);
 
 	if (!fake) {
+#ifdef CONFIG_ARMV8_PSCI
+		setup_psci();
+#endif
 		do_nonsec_virt_switch();
 		kernel_entry(images->ft_addr);
 	}
-- 
1.7.7.4

[U-Boot] [PATCH 8/8] ARMv8: PSCI: Enable SMC

[Arnab Basu]

Enable the SMC instruction so that the kernel can use the psci code

Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/include/asm/macro.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h
index 0009c28..94a1e68 100644
--- a/arch/arm/include/asm/macro.h
+++ b/arch/arm/include/asm/macro.h
@@ -106,7 +106,11 @@ lr	.req	x30
 .endm
 
 .macro armv8_switch_to_el2_m, xreg1
+#ifdef CONFIG_ARMV8_PSCI
+	mov	\xreg1, #0x531	/* Non-secure EL0/EL1 | HVC | 64bit EL2 */
+#else
 	mov	\xreg1, #0x5b1	/* Non-secure EL0/EL1 | HVC | 64bit EL2 */
+#endif
 	msr	scr_el3, \xreg1
 	msr	cptr_el3, xzr	/* Disable coprocessor traps to EL3 */
 	mov	\xreg1, #0x33ff
-- 
1.7.7.4

[U-Boot] [PATCH 8/8] ARMv8: PSCI: Enable SMC

[Albert ARIBAUD]

Hi Arnab,

On Thu, 28 Aug 2014 02:00:01 +0530, Arnab Basu
<arnab.basu@freescale.com> wrote:

> Enable the SMC instruction so that the kernel can use the psci code
> 
> Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/include/asm/macro.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/macro.h b/arch/arm/include/asm/macro.h
> index 0009c28..94a1e68 100644
> --- a/arch/arm/include/asm/macro.h
> +++ b/arch/arm/include/asm/macro.h
> @@ -106,7 +106,11 @@ lr	.req	x30
>  .endm
>  
>  .macro armv8_switch_to_el2_m, xreg1
> +#ifdef CONFIG_ARMV8_PSCI
> +	mov	\xreg1, #0x531	/* Non-secure EL0/EL1 | HVC | 64bit EL2 */
> +#else
>  	mov	\xreg1, #0x5b1	/* Non-secure EL0/EL1 | HVC | 64bit EL2 */

The 'mov' lines have different constant arguments in the instruction;
their explanatory comments should not be the same.

> +#endif
>  	msr	scr_el3, \xreg1
>  	msr	cptr_el3, xzr	/* Disable coprocessor traps to EL3 */
>  	mov	\xreg1, #0x33ff


Amicalement,
-- 
Albert.

[U-Boot] [PATCH 8/8] ARMv8: PSCI: Enable SMC

[arnab.basu at freescale.com]

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
> Sent: Thursday, September 18, 2014 2:48 PM
> To: Basu Arnab-B45036
> Cc: marc.zyngier at arm.com; mark.rutland at arm.com; Sun York-R58495; Yoder
> Stuart-B08248; u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH 8/8] ARMv8: PSCI: Enable SMC
> 
> Hi Arnab,
> 
> On Thu, 28 Aug 2014 02:00:01 +0530, Arnab Basu <arnab.basu@freescale.com>
> wrote:
> 
> > Enable the SMC instruction so that the kernel can use the psci code
> >
> > Signed-off-by: Arnab Basu <arnab.basu@freescale.com>
> > Reviewed-by: Bhupesh Sharma <bhupesh.sharma@freescale.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  arch/arm/include/asm/macro.h |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/macro.h
> > b/arch/arm/include/asm/macro.h index 0009c28..94a1e68 100644
> > --- a/arch/arm/include/asm/macro.h
> > +++ b/arch/arm/include/asm/macro.h
> > @@ -106,7 +106,11 @@ lr	.req	x30
> >  .endm
> >
> >  .macro armv8_switch_to_el2_m, xreg1
> > +#ifdef CONFIG_ARMV8_PSCI
> > +	mov	\xreg1, #0x531	/* Non-secure EL0/EL1 | HVC | 64bit EL2 */
> > +#else
> >  	mov	\xreg1, #0x5b1	/* Non-secure EL0/EL1 | HVC | 64bit EL2 */
> 
> The 'mov' lines have different constant arguments in the instruction;
> their explanatory comments should not be the same.
> 

Right, I'll fix the comment.

Thanks
Arnab

> > +#endif
> >  	msr	scr_el3, \xreg1
> >  	msr	cptr_el3, xzr	/* Disable coprocessor traps to EL3 */
> >  	mov	\xreg1, #0x33ff
> 
> 
> Amicalement,
> --
> Albert.

[U-Boot] [PATCH 0/8] PSCI v0.2 framework for ARMv8

[Arnab Basu]

On 08/28/2014 01:59 AM, Arnab Basu wrote:
> This series of patches creates a generic PSCI v0.2 framework for ARMv8.
>
> The first 3 patches refactor existing code so that ARMv7 PSCI,
> ARMv8 spin-table and ARMv8 PSCI can coexist.
>
> The next 5 patches create a generic framework for PSCI v0.2 in ARMv8.
>
> The implementation is modelled on the pre-existing PSCI v0.1 support
> in ARMv7.
>
> PSCI support patches for the ARMv8 Foundation model will follow shortly.
>

I forgot to mention that this patch set requires the following patches:

http://patchwork.ozlabs.org/patch/381479/
http://patchwork.ozlabs.org/patch/381478/
http://patchwork.ozlabs.org/patch/381480/
http://patchwork.ozlabs.org/patch/381481/
http://patchwork.ozlabs.org/patch/381482/

> Arnab Basu (8):
>    ARM: PSCI: Update psci.h for psci v0.2
>    ARM: PSCI: Alow arch specific DT patching
>    ARMv8/fsl-lsch3: Refactor spin-table code
>    ARMv8: PSCI: Add linker section to hold PSCI code
>    ARMv8: PCSI: Add generic ARMv8 PSCI code
>    ARMv8: PSCI: Fixup the device tree for PSCI v0.2
>    ARMv8: PSCI: Setup ARMv8 PSCI
>    ARMv8: PSCI: Enable SMC
>
>   arch/arm/config.mk                 |    2 +-
>   arch/arm/cpu/armv7/virt-dt.c       |    7 +-
>   arch/arm/cpu/armv8/Makefile        |    2 +
>   arch/arm/cpu/armv8/cpu-dt.c        |  128 ++++++++++++++++++++++++++
>   arch/arm/cpu/armv8/cpu.c           |   23 +++++
>   arch/arm/cpu/armv8/fsl-lsch3/fdt.c |   46 +++------
>   arch/arm/cpu/armv8/psci.S          |  177 ++++++++++++++++++++++++++++++++++++
>   arch/arm/cpu/armv8/u-boot.lds      |   30 ++++++
>   arch/arm/include/asm/macro.h       |    4 +
>   arch/arm/include/asm/psci.h        |   42 ++++++++-
>   arch/arm/include/asm/system.h      |    7 ++
>   arch/arm/lib/bootm-fdt.c           |   11 ++-
>   arch/arm/lib/bootm.c               |    3 +
>   13 files changed, 446 insertions(+), 36 deletions(-)
>   create mode 100644 arch/arm/cpu/armv8/cpu-dt.c
>   create mode 100644 arch/arm/cpu/armv8/psci.S
>

[U-Boot] [PATCH 0/8] PSCI v0.2 framework for ARMv8

[Jan Kiszka]

On 2014-08-27 22:29, Arnab Basu wrote:
> This series of patches creates a generic PSCI v0.2 framework for ARMv8.
> 
> The first 3 patches refactor existing code so that ARMv7 PSCI,
> ARMv8 spin-table and ARMv8 PSCI can coexist.
> 
> The next 5 patches create a generic framework for PSCI v0.2 in ARMv8.
> 
> The implementation is modelled on the pre-existing PSCI v0.1 support
> in ARMv7.
> 
> PSCI support patches for the ARMv8 Foundation model will follow shortly.
> 

What's the status of this effort? I'll look into v0.2 support for v7
soon, so I was wondering if there is something recent to possibly build
upon / derive from.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

[U-Boot] [PATCH 0/8] PSCI v0.2 framework for ARMv8

[Mark Rutland]

On Wed, Nov 26, 2014 at 12:52:11PM +0000, Jan Kiszka wrote:
> On 2014-08-27 22:29, Arnab Basu wrote:
> > This series of patches creates a generic PSCI v0.2 framework for ARMv8.
> > 
> > The first 3 patches refactor existing code so that ARMv7 PSCI,
> > ARMv8 spin-table and ARMv8 PSCI can coexist.
> > 
> > The next 5 patches create a generic framework for PSCI v0.2 in ARMv8.
> > 
> > The implementation is modelled on the pre-existing PSCI v0.1 support
> > in ARMv7.
> > 
> > PSCI support patches for the ARMv8 Foundation model will follow shortly.
> > 
> 
> What's the status of this effort? I'll look into v0.2 support for v7
> soon, so I was wondering if there is something recent to possibly build
> upon / derive from.

When I asked on linux-arm-kernel [1], I was told that Arnab Basu has
left Freescale, but intends to continue with this series. I don't know
if we're likely to see anything newer at any point soon.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/307820.html