[U-Boot] [PATCH 0/8] PSCI v0.2 framework for ARMv8
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
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__ */
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,
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,
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; }
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
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.
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.
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)
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,
Hi Albert
-----Original Message----- From: Albert ARIBAUD [mailto:albert.u.boot@aribaud.net] Sent: Monday, October 27, 2014 2:36 AM To: Basu Arnab-B45036 Cc: marc.zyngier@arm.com; mark.rutland@arm.com; Sun York-R58495; Yoder Stuart-B08248; u-boot@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.
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*))) }
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,
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.
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,
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,
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.
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
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
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);
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.
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.
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.
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.
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
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.
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); }
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
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,
-----Original Message----- From: Albert ARIBAUD [mailto:albert.u.boot@aribaud.net] Sent: Thursday, September 18, 2014 2:48 PM To: Basu Arnab-B45036 Cc: marc.zyngier@arm.com; mark.rutland@arm.com; Sun York-R58495; Yoder Stuart-B08248; u-boot@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.
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
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
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.h...
participants (7)
-
Albert ARIBAUD -
Arnab Basu -
arnab.basuļ¼ freescale.com
-
Jan Kiszka -
Marc Zyngier -
Mark Rutland -
Stuart Yoder