[U-Boot] [PATCH 1/2] armv8: Enable CPUECTLR.SMPEN for coherency

From: Mingkai Hu mingkai.hu@nxp.com
For A53, data coherency is enabled only when the CPUECTLR.SMPEN bit is set. The SMPEN bit should be set before enabling the data cache. If not enabled, the cache is not coherent with other cores and data corruption could occur.
For A57/A72, SMPEN bit enables the processor to receive instruction cache and TLB maintenance operations broadcast from other processors in the cluster. This bit should be set before enabling the caches and MMU, or performing any cache and TLB maintenance operations.
Signed-off-by: Mingkai Hu mingkai.hu@nxp.com Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com Signed-off-by: Hou Zhiqiang Zhiqiang.Hou@nxp.com --- arch/arm/cpu/armv8/Kconfig | 12 ++++++++++++ arch/arm/cpu/armv8/start.S | 11 +++++++++++ 2 files changed, 23 insertions(+)
diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 965a8d1..ce749f2 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -3,6 +3,18 @@ if ARM64 config ARMV8_MULTIENTRY bool "Enable multiple CPUs to enter into U-Boot"
+config ARMV8_SET_SMPEN + bool "Enable data coherency with other cores in cluster" + help + Cortex A53/57/72 cores require CPUECTLR_EL1.SMPEN set even + for single core systems. Unfortunately write access to this + register may be controlled by EL3/EL2 firmware. To be more + precise, by default (if there is EL2/EL3 firmware running) + this register is RO for NS EL1. + This switch can be used to avoid writing to CPUECTLR_EL1, + it can be safely enabled when El2/EL3 initialized SMPEN bit + or when CPU implementation doesn't include that register. + config ARMV8_SPIN_TABLE bool "Support spin-table enable method" depends on ARMV8_MULTIENTRY && OF_LIBFDT diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 4f5f6d8..5308702 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -86,6 +86,17 @@ save_boot_params_ret: msr cpacr_el1, x0 /* Enable FP/SIMD */ 0:
+ /* + * Enalbe SMPEN bit for coherency. + * This register is not architectural but at the moment + * this bit should be set for A53/A57/A72. + */ +#ifdef CONFIG_ARMV8_SET_SMPEN + mrs x0, S3_1_c15_c2_1 /* cpuactlr_el1 */ + orr x0, x0, #0x40 + msr S3_1_c15_c2_1, x0 +#endif + /* Apply ARM core specific erratas */ bl apply_core_errata

From: Hou Zhiqiang Zhiqiang.Hou@nxp.com
Signed-off-by: Hou Zhiqiang Zhiqiang.Hou@nxp.com --- arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig index 6772584..28d474b 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig @@ -1,5 +1,6 @@ config ARCH_LS1012A bool + select ARMV8_SET_SMPEN select FSL_LSCH2 select SYS_FSL_DDR_BE select SYS_FSL_MMDC @@ -7,6 +8,7 @@ config ARCH_LS1012A
config ARCH_LS1043A bool + select ARMV8_SET_SMPEN select FSL_LSCH2 select SYS_FSL_DDR_BE select SYS_FSL_DDR_VER_50 @@ -15,6 +17,7 @@ config ARCH_LS1043A
config ARCH_LS1046A bool + select ARMV8_SET_SMPEN select FSL_LSCH2 select SYS_FSL_DDR_BE select SYS_FSL_DDR4 @@ -24,6 +27,7 @@ config ARCH_LS1046A
config ARCH_LS2080A bool + select ARMV8_SET_SMPEN select FSL_LSCH3 select SYS_FSL_DDR4 select SYS_FSL_DDR_LE

On 12/14/2016 10:22 PM, Zhiqiang Hou wrote:
From: Mingkai Hu mingkai.hu@nxp.com
For A53, data coherency is enabled only when the CPUECTLR.SMPEN bit is set. The SMPEN bit should be set before enabling the data cache. If not enabled, the cache is not coherent with other cores and data corruption could occur.
For A57/A72, SMPEN bit enables the processor to receive instruction cache and TLB maintenance operations broadcast from other processors in the cluster. This bit should be set before enabling the caches and MMU, or performing any cache and TLB maintenance operations.
Signed-off-by: Mingkai Hu mingkai.hu@nxp.com Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com Signed-off-by: Hou Zhiqiang Zhiqiang.Hou@nxp.com
arch/arm/cpu/armv8/Kconfig | 12 ++++++++++++ arch/arm/cpu/armv8/start.S | 11 +++++++++++ 2 files changed, 23 insertions(+)
diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 965a8d1..ce749f2 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -3,6 +3,18 @@ if ARM64 config ARMV8_MULTIENTRY bool "Enable multiple CPUs to enter into U-Boot"
+config ARMV8_SET_SMPEN
bool "Enable data coherency with other cores in cluster"
help
Cortex A53/57/72 cores require CPUECTLR_EL1.SMPEN set even
for single core systems. Unfortunately write access to this
register may be controlled by EL3/EL2 firmware. To be more
precise, by default (if there is EL2/EL3 firmware running)
this register is RO for NS EL1.
This switch can be used to avoid writing to CPUECTLR_EL1,
it can be safely enabled when El2/EL3 initialized SMPEN bit
or when CPU implementation doesn't include that register.
config ARMV8_SPIN_TABLE bool "Support spin-table enable method" depends on ARMV8_MULTIENTRY && OF_LIBFDT diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 4f5f6d8..5308702 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -86,6 +86,17 @@ save_boot_params_ret: msr cpacr_el1, x0 /* Enable FP/SIMD */ 0:
- /*
* Enalbe SMPEN bit for coherency.
* This register is not architectural but at the moment
* this bit should be set for A53/A57/A72.
*/
+#ifdef CONFIG_ARMV8_SET_SMPEN
- mrs x0, S3_1_c15_c2_1 /* cpuactlr_el1 */
- orr x0, x0, #0x40
- msr S3_1_c15_c2_1, x0
+#endif
- /* Apply ARM core specific erratas */ bl apply_core_errata
Would it be a good idea to detect the core and apply this setting at run time, instead of using Kconfig option? Check the macro branch_if_a53_core, branch_if_a57_core for example.
York

On Thu, Dec 15, 2016 at 04:55:44PM +0000, york sun wrote:
On 12/14/2016 10:22 PM, Zhiqiang Hou wrote:
From: Mingkai Hu mingkai.hu@nxp.com
For A53, data coherency is enabled only when the CPUECTLR.SMPEN bit is set. The SMPEN bit should be set before enabling the data cache. If not enabled, the cache is not coherent with other cores and data corruption could occur.
For A57/A72, SMPEN bit enables the processor to receive instruction cache and TLB maintenance operations broadcast from other processors in the cluster. This bit should be set before enabling the caches and MMU, or performing any cache and TLB maintenance operations.
Signed-off-by: Mingkai Hu mingkai.hu@nxp.com Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com Signed-off-by: Hou Zhiqiang Zhiqiang.Hou@nxp.com
arch/arm/cpu/armv8/Kconfig | 12 ++++++++++++ arch/arm/cpu/armv8/start.S | 11 +++++++++++ 2 files changed, 23 insertions(+)
diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 965a8d1..ce749f2 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -3,6 +3,18 @@ if ARM64 config ARMV8_MULTIENTRY bool "Enable multiple CPUs to enter into U-Boot"
+config ARMV8_SET_SMPEN
bool "Enable data coherency with other cores in cluster"
help
Cortex A53/57/72 cores require CPUECTLR_EL1.SMPEN set even
for single core systems. Unfortunately write access to this
register may be controlled by EL3/EL2 firmware. To be more
precise, by default (if there is EL2/EL3 firmware running)
this register is RO for NS EL1.
This switch can be used to avoid writing to CPUECTLR_EL1,
it can be safely enabled when El2/EL3 initialized SMPEN bit
or when CPU implementation doesn't include that register.
config ARMV8_SPIN_TABLE bool "Support spin-table enable method" depends on ARMV8_MULTIENTRY && OF_LIBFDT diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 4f5f6d8..5308702 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -86,6 +86,17 @@ save_boot_params_ret: msr cpacr_el1, x0 /* Enable FP/SIMD */ 0:
- /*
* Enalbe SMPEN bit for coherency.
* This register is not architectural but at the moment
* this bit should be set for A53/A57/A72.
*/
+#ifdef CONFIG_ARMV8_SET_SMPEN
- mrs x0, S3_1_c15_c2_1 /* cpuactlr_el1 */
- orr x0, x0, #0x40
- msr S3_1_c15_c2_1, x0
+#endif
- /* Apply ARM core specific erratas */ bl apply_core_errata
Would it be a good idea to detect the core and apply this setting at run time, instead of using Kconfig option? Check the macro branch_if_a53_core, branch_if_a57_core for example.
I think we need to look back at the discussion from the last time we made a change around here, and make sure to fully understand what platforms do and do not need / want to touch that. See d73718f3236c520a92efa401084c658e6cc067f3 and then 3a592a1349ac3961b0f4f2db0a8d9f128225d897.

On 12/15/2016 09:17 AM, Tom Rini wrote:
On Thu, Dec 15, 2016 at 04:55:44PM +0000, york sun wrote:
Would it be a good idea to detect the core and apply this setting at run time, instead of using Kconfig option? Check the macro branch_if_a53_core, branch_if_a57_core for example.
I think we need to look back at the discussion from the last time we made a change around here, and make sure to fully understand what platforms do and do not need / want to touch that. See d73718f3236c520a92efa401084c658e6cc067f3 and then 3a592a1349ac3961b0f4f2db0a8d9f128225d897.
Tom,
Thanks for the reminder. Yes we need to understand which platform really needs this change. The author suggested using Kconfig option. It can surely avoid applying this change blindly.
York

On Thu, Dec 15, 2016 at 05:22:38PM +0000, york sun wrote:
On 12/15/2016 09:17 AM, Tom Rini wrote:
On Thu, Dec 15, 2016 at 04:55:44PM +0000, york sun wrote:
Would it be a good idea to detect the core and apply this setting at run time, instead of using Kconfig option? Check the macro branch_if_a53_core, branch_if_a57_core for example.
I think we need to look back at the discussion from the last time we made a change around here, and make sure to fully understand what platforms do and do not need / want to touch that. See d73718f3236c520a92efa401084c658e6cc067f3 and then 3a592a1349ac3961b0f4f2db0a8d9f128225d897.
Tom,
Thanks for the reminder. Yes we need to understand which platform really needs this change. The author suggested using Kconfig option. It can surely avoid applying this change blindly.
Yes. I think this series as-is will be safe in that it's only being enabled on NXP platforms that have (I will assume) all been tested with the change. But even for this series I want to make sure it's clear when/why it would be needed and the Kconfig help should reflect that information. Thanks!

2016-12-15 15:08 GMT+09:00 Zhiqiang Hou Zhiqiang.Hou@nxp.com:
From: Mingkai Hu mingkai.hu@nxp.com
For A53, data coherency is enabled only when the CPUECTLR.SMPEN bit is set. The SMPEN bit should be set before enabling the data cache. If not enabled, the cache is not coherent with other cores and data corruption could occur.
For A57/A72, SMPEN bit enables the processor to receive instruction cache and TLB maintenance operations broadcast from other processors in the cluster. This bit should be set before enabling the caches and MMU, or performing any cache and TLB maintenance operations.
Signed-off-by: Mingkai Hu mingkai.hu@nxp.com Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com Signed-off-by: Hou Zhiqiang Zhiqiang.Hou@nxp.com
arch/arm/cpu/armv8/Kconfig | 12 ++++++++++++ arch/arm/cpu/armv8/start.S | 11 +++++++++++ 2 files changed, 23 insertions(+)
diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 965a8d1..ce749f2 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -3,6 +3,18 @@ if ARM64 config ARMV8_MULTIENTRY bool "Enable multiple CPUs to enter into U-Boot"
+config ARMV8_SET_SMPEN
bool "Enable data coherency with other cores in cluster"
help
Cortex A53/57/72 cores require CPUECTLR_EL1.SMPEN set even
for single core systems. Unfortunately write access to this
register may be controlled by EL3/EL2 firmware. To be more
precise, by default (if there is EL2/EL3 firmware running)
this register is RO for NS EL1.
This switch can be used to avoid writing to CPUECTLR_EL1,
it can be safely enabled when El2/EL3 initialized SMPEN bit
or when CPU implementation doesn't include that register.
If you run ARM Trusted Firmware, this bit has already been set correctly. (or if you implement your own trusted firmware, this bit should be set there.) In those cases, there is no need to touch it in U-Boot.
The motivation for this commit is to boot the system without any firmware before U-Boot?

Hi Masahiro Yamada,
Thanks a lot for your comments!
-----Original Message----- From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com] Sent: 2016年12月19日 0:49 To: Z.Q. Hou zhiqiang.hou@nxp.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Albert ARIBAUD albert.u.boot@aribaud.net; Simon Glass sjg@chromium.org; Mingkai Hu mingkai.hu@nxp.com; york sun york.sun@nxp.com; Ashish Kumar ashish.kumar@nxp.com; Mateusz Kulikowski mateusz.kulikowski@gmail.com; Tom Rini trini@konsulko.com Subject: Re: [U-Boot] [PATCH 1/2] armv8: Enable CPUECTLR.SMPEN for coherency
2016-12-15 15:08 GMT+09:00 Zhiqiang Hou Zhiqiang.Hou@nxp.com:
From: Mingkai Hu mingkai.hu@nxp.com
For A53, data coherency is enabled only when the CPUECTLR.SMPEN bit is set. The SMPEN bit should be set before enabling the data cache. If not enabled, the cache is not coherent with other cores and data corruption could occur.
For A57/A72, SMPEN bit enables the processor to receive instruction cache and TLB maintenance operations broadcast from other processors in the cluster. This bit should be set before enabling the caches and MMU, or performing any cache and TLB maintenance operations.
Signed-off-by: Mingkai Hu mingkai.hu@nxp.com Signed-off-by: Gong Qianyu Qianyu.Gong@nxp.com Signed-off-by: Mateusz Kulikowski mateusz.kulikowski@gmail.com Signed-off-by: Hou Zhiqiang Zhiqiang.Hou@nxp.com
arch/arm/cpu/armv8/Kconfig | 12 ++++++++++++ arch/arm/cpu/armv8/start.S | 11 +++++++++++ 2 files changed, 23 insertions(+)
diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig index 965a8d1..ce749f2 100644 --- a/arch/arm/cpu/armv8/Kconfig +++ b/arch/arm/cpu/armv8/Kconfig @@ -3,6 +3,18 @@ if ARM64 config ARMV8_MULTIENTRY bool "Enable multiple CPUs to enter into U-Boot"
+config ARMV8_SET_SMPEN
bool "Enable data coherency with other cores in cluster"
help
Cortex A53/57/72 cores require CPUECTLR_EL1.SMPEN set
even
for single core systems. Unfortunately write access to this
register may be controlled by EL3/EL2 firmware. To be more
precise, by default (if there is EL2/EL3 firmware running)
this register is RO for NS EL1.
This switch can be used to avoid writing to CPUECTLR_EL1,
it can be safely enabled when El2/EL3 initialized SMPEN bit
or when CPU implementation doesn't include that register.
If you run ARM Trusted Firmware, this bit has already been set correctly. (or if you implement your own trusted firmware, this bit should be set there.) In those cases, there is no need to touch it in U-Boot.
The motivation for this commit is to boot the system without any firmware before U-Boot?
Yes
Thanks, Zhiqiang
participants (5)
-
Masahiro Yamada
-
Tom Rini
-
york sun
-
Z.Q. Hou
-
Zhiqiang Hou