[PATCH v2 0/2] Disable setting ICACHE and DCACHE off for ARM64 platforms
As suggested by Tom, disable setting ICACHE and DCACHE off for ARM64 platforms. It is also noticed that with CACHEs off u-boot on ARM64 (ARMv8) boots slower.
If some platforms still wish to turn off ICACHE / DCACHE during the SPL stage, support that configuration as well.
While at it, also fix an compilation issue for stm32mp when SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled.
Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Peng Fan peng.fan@nxp.com
Bhupesh Sharma (2): arm: stm32mp: Really fix compilation issue when SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled armv8: Disable SYS_DCACHE_OFF & SYS_ICACHE_OFF for ARM64
arch/arm/Kconfig | 2 ++ arch/arm/cpu/armv8/Makefile | 4 ++-- arch/arm/mach-stm32mp/cpu.c | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-)
While 23e20b2fa6 ("arm: stm32mp: Fix compilation issue when SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled") tried fixing this issue, fix it really by adding #if checks for SYS_ICACHE_OFF and SYS_DCACHE_OFF.
Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org --- arch/arm/mach-stm32mp/cpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index e2f67fc423..8ed065b389 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -90,10 +90,10 @@ static void early_enable_caches(void) if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) return;
- if (!(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))) { +#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) gd->arch.tlb_size = PGTABLE_SIZE; gd->arch.tlb_addr = (unsigned long)&early_tlb; - } +#endif
/* enable MMU (default configuration) */ dcache_enable();
On 8/22/23 09:51, Bhupesh Sharma wrote:
While 23e20b2fa6 ("arm: stm32mp: Fix compilation issue when SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled") tried fixing this issue, fix it really by adding #if checks for SYS_ICACHE_OFF and SYS_DCACHE_OFF.
Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org
arch/arm/mach-stm32mp/cpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index e2f67fc423..8ed065b389 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -90,10 +90,10 @@ static void early_enable_caches(void) if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) return;
- if (!(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))) {
+#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) gd->arch.tlb_size = PGTABLE_SIZE; gd->arch.tlb_addr = (unsigned long)&early_tlb;
- }
+#endif
/* enable MMU (default configuration) */ dcache_enable();
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
On 8/25/23 10:53, Patrice CHOTARD wrote:
On 8/22/23 09:51, Bhupesh Sharma wrote:
While 23e20b2fa6 ("arm: stm32mp: Fix compilation issue when SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled") tried fixing this issue, fix it really by adding #if checks for SYS_ICACHE_OFF and SYS_DCACHE_OFF.
Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org
arch/arm/mach-stm32mp/cpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index e2f67fc423..8ed065b389 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -90,10 +90,10 @@ static void early_enable_caches(void) if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) return;
- if (!(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))) {
+#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) gd->arch.tlb_size = PGTABLE_SIZE; gd->arch.tlb_addr = (unsigned long)&early_tlb;
- }
+#endif
/* enable MMU (default configuration) */ dcache_enable();
Reviewed-by: Patrice Chotard patrice.chotard@foss.st.com
Thanks Patrice
Applied to u-boot-stm32/master
Thanks Patrice
Hi,
On 8/22/23 09:51, Bhupesh Sharma wrote:
While 23e20b2fa6 ("arm: stm32mp: Fix compilation issue when SYS_DCACHE_OFF and/or SYS_DCACHE_SYS are enabled") tried fixing this issue, fix it really by adding #if checks for SYS_ICACHE_OFF and SYS_DCACHE_OFF.
Cc: Patrice Chotard patrice.chotard@foss.st.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org
arch/arm/mach-stm32mp/cpu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c index e2f67fc423..8ed065b389 100644 --- a/arch/arm/mach-stm32mp/cpu.c +++ b/arch/arm/mach-stm32mp/cpu.c @@ -90,10 +90,10 @@ static void early_enable_caches(void) if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) return;
- if (!(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))) {
+#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) gd->arch.tlb_size = PGTABLE_SIZE; gd->arch.tlb_addr = (unsigned long)&early_tlb;
- }
+#endif
/* enable MMU (default configuration) */ dcache_enable();
Reviewed-by: Patrick Delaunay patrick.delaunay@foss.st.com
Thanks Patrick
Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these platforms.
However, if some platforms require ICACHE or DCACHE to be disabled only for the smaller SPL stage, we should support such configurations in u-boot as well.
Compile-tested for: - qemu arm64 - imx8 - stm32
and run-tested on: - Qualcomm RB3 platform
Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Peng Fan peng.fan@nxp.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org --- arch/arm/Kconfig | 2 ++ arch/arm/cpu/armv8/Makefile | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 36ee1e9a3c..92bff715e3 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -141,6 +141,7 @@ config THUMB2_KERNEL
config SYS_ICACHE_OFF bool "Do not enable icache" + depends on !ARM64 help Do not enable instruction cache in U-Boot.
@@ -153,6 +154,7 @@ config SPL_SYS_ICACHE_OFF
config SYS_DCACHE_OFF bool "Do not enable dcache" + depends on !ARM64 help Do not enable data cache in U-Boot.
diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index bba4f570db..0d0c1728e4 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -5,13 +5,13 @@
extra-y := start.o
+obj-y += cache_v8.o obj-y += cpu.o ifndef CONFIG_$(SPL_TPL_)TIMER obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o endif ifndef CONFIG_$(SPL_)SYS_DCACHE_OFF -obj-y += cache_v8.o -obj-y += cache.o +obj-y += cache.o endif ifdef CONFIG_SPL_BUILD obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o
On Tue, Aug 22, 2023 at 01:21:12PM +0530, Bhupesh Sharma wrote:
Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these platforms.
However, if some platforms require ICACHE or DCACHE to be disabled only for the smaller SPL stage, we should support such configurations in u-boot as well.
Compile-tested for:
- qemu arm64
- imx8
- stm32
and run-tested on:
- Qualcomm RB3 platform
Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Peng Fan peng.fan@nxp.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org
Reviewed-by: Tom Rini trini@konsulko.com
On Tue, Aug 22, 2023 at 01:21:12PM +0530, Bhupesh Sharma wrote:
Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these platforms.
However, if some platforms require ICACHE or DCACHE to be disabled only for the smaller SPL stage, we should support such configurations in u-boot as well.
Compile-tested for:
- qemu arm64
- imx8
- stm32
and run-tested on:
- Qualcomm RB3 platform
Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Peng Fan peng.fan@nxp.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org Reviewed-by: Tom Rini trini@konsulko.com
arch/arm/Kconfig | 2 ++ arch/arm/cpu/armv8/Makefile | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 36ee1e9a3c..92bff715e3 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -141,6 +141,7 @@ config THUMB2_KERNEL
config SYS_ICACHE_OFF bool "Do not enable icache"
- depends on !ARM64 help Do not enable instruction cache in U-Boot.
@@ -153,6 +154,7 @@ config SPL_SYS_ICACHE_OFF
config SYS_DCACHE_OFF bool "Do not enable dcache"
- depends on !ARM64 help Do not enable data cache in U-Boot.
diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index bba4f570db..0d0c1728e4 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -5,13 +5,13 @@
extra-y := start.o
+obj-y += cache_v8.o obj-y += cpu.o ifndef CONFIG_$(SPL_TPL_)TIMER obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o endif ifndef CONFIG_$(SPL_)SYS_DCACHE_OFF -obj-y += cache_v8.o -obj-y += cache.o +obj-y += cache.o endif ifdef CONFIG_SPL_BUILD obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o
I'm adding Michal here because this changes the behavior on xilinx platforms that had the icache off. I was under the impression that at the levels U-Boot runs at on aarch64, we just couldn't have icache/dcache off, but I guess that's wrong?
On 8/30/23 20:52, Tom Rini wrote:
On Tue, Aug 22, 2023 at 01:21:12PM +0530, Bhupesh Sharma wrote:
Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these platforms.
However, if some platforms require ICACHE or DCACHE to be disabled only for the smaller SPL stage, we should support such configurations in u-boot as well.
I am missing the reason why this change should be accepted.
Compile-tested for:
- qemu arm64
qemu doesn't model caches that's why I don't think it is relevant here.
- imx8
- stm32
and run-tested on:
- Qualcomm RB3 platform
Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Peng Fan peng.fan@nxp.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org Reviewed-by: Tom Rini trini@konsulko.com
arch/arm/Kconfig | 2 ++ arch/arm/cpu/armv8/Makefile | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 36ee1e9a3c..92bff715e3 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -141,6 +141,7 @@ config THUMB2_KERNEL
config SYS_ICACHE_OFF bool "Do not enable icache"
- depends on !ARM64 help Do not enable instruction cache in U-Boot.
@@ -153,6 +154,7 @@ config SPL_SYS_ICACHE_OFF
config SYS_DCACHE_OFF bool "Do not enable dcache"
- depends on !ARM64 help Do not enable data cache in U-Boot.
diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index bba4f570db..0d0c1728e4 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -5,13 +5,13 @@
extra-y := start.o
+obj-y += cache_v8.o obj-y += cpu.o ifndef CONFIG_$(SPL_TPL_)TIMER obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o endif ifndef CONFIG_$(SPL_)SYS_DCACHE_OFF -obj-y += cache_v8.o -obj-y += cache.o +obj-y += cache.o endif ifdef CONFIG_SPL_BUILD obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o
I'm adding Michal here because this changes the behavior on xilinx platforms that had the icache off. I was under the impression that at the levels U-Boot runs at on aarch64, we just couldn't have icache/dcache off, but I guess that's wrong?
yes. We are using icache off for mini u-boot configurations for quite a long time. I tried to find out any commit why we started to use it but didn't find any record. But will ask team to retest that use cases to see if that is working or not. In our case we are using small full U-Boot not any SPL variant.
My biggest issue with the patch is that it is not clear what issue it is trying to solve as I have mentioned above. It just says that ideally icache/dcache should be on. If you want to enforce it on your platform you can select it when that platform is selected and don't need enforce it for other platforms.
Thanks, Michal
On Thu, Aug 31, 2023 at 10:24:07AM +0200, Michal Simek wrote:
On 8/30/23 20:52, Tom Rini wrote:
On Tue, Aug 22, 2023 at 01:21:12PM +0530, Bhupesh Sharma wrote:
Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these platforms.
However, if some platforms require ICACHE or DCACHE to be disabled only for the smaller SPL stage, we should support such configurations in u-boot as well.
I am missing the reason why this change should be accepted.
Compile-tested for:
- qemu arm64
qemu doesn't model caches that's why I don't think it is relevant here.
- imx8
- stm32
and run-tested on:
- Qualcomm RB3 platform
Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Peng Fan peng.fan@nxp.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org Reviewed-by: Tom Rini trini@konsulko.com
arch/arm/Kconfig | 2 ++ arch/arm/cpu/armv8/Makefile | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 36ee1e9a3c..92bff715e3 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -141,6 +141,7 @@ config THUMB2_KERNEL config SYS_ICACHE_OFF bool "Do not enable icache"
- depends on !ARM64 help Do not enable instruction cache in U-Boot.
@@ -153,6 +154,7 @@ config SPL_SYS_ICACHE_OFF config SYS_DCACHE_OFF bool "Do not enable dcache"
- depends on !ARM64 help Do not enable data cache in U-Boot.
diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index bba4f570db..0d0c1728e4 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -5,13 +5,13 @@ extra-y := start.o +obj-y += cache_v8.o obj-y += cpu.o ifndef CONFIG_$(SPL_TPL_)TIMER obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o endif ifndef CONFIG_$(SPL_)SYS_DCACHE_OFF -obj-y += cache_v8.o -obj-y += cache.o +obj-y += cache.o endif ifdef CONFIG_SPL_BUILD obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o
I'm adding Michal here because this changes the behavior on xilinx platforms that had the icache off. I was under the impression that at the levels U-Boot runs at on aarch64, we just couldn't have icache/dcache off, but I guess that's wrong?
yes. We are using icache off for mini u-boot configurations for quite a long time. I tried to find out any commit why we started to use it but didn't find any record. But will ask team to retest that use cases to see if that is working or not. In our case we are using small full U-Boot not any SPL variant.
My biggest issue with the patch is that it is not clear what issue it is trying to solve as I have mentioned above. It just says that ideally icache/dcache should be on. If you want to enforce it on your platform you can select it when that platform is selected and don't need enforce it for other platforms.
Thanks Michal. Bhupesh, I was wrong in v1, having caches off is allowed, so you just need to fix issues like part one does where code doesn't compile when the options are disabled, and then are there further changes needed for your usecase?
On Thu, 31 Aug 2023 at 19:01, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 31, 2023 at 10:24:07AM +0200, Michal Simek wrote:
On 8/30/23 20:52, Tom Rini wrote:
On Tue, Aug 22, 2023 at 01:21:12PM +0530, Bhupesh Sharma wrote:
Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these platforms.
However, if some platforms require ICACHE or DCACHE to be disabled only for the smaller SPL stage, we should support such configurations in u-boot as well.
I am missing the reason why this change should be accepted.
Compile-tested for:
- qemu arm64
qemu doesn't model caches that's why I don't think it is relevant here.
- imx8
- stm32
and run-tested on:
- Qualcomm RB3 platform
Cc: Tom Rini trini@konsulko.com Cc: Simon Glass sjg@chromium.org Cc: Peng Fan peng.fan@nxp.com Signed-off-by: Bhupesh Sharma bhupesh.sharma@linaro.org Reviewed-by: Tom Rini trini@konsulko.com
arch/arm/Kconfig | 2 ++ arch/arm/cpu/armv8/Makefile | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 36ee1e9a3c..92bff715e3 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -141,6 +141,7 @@ config THUMB2_KERNEL config SYS_ICACHE_OFF bool "Do not enable icache"
- depends on !ARM64 help Do not enable instruction cache in U-Boot.
@@ -153,6 +154,7 @@ config SPL_SYS_ICACHE_OFF config SYS_DCACHE_OFF bool "Do not enable dcache"
- depends on !ARM64 help Do not enable data cache in U-Boot.
diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index bba4f570db..0d0c1728e4 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -5,13 +5,13 @@ extra-y := start.o +obj-y += cache_v8.o obj-y += cpu.o ifndef CONFIG_$(SPL_TPL_)TIMER obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o endif ifndef CONFIG_$(SPL_)SYS_DCACHE_OFF -obj-y += cache_v8.o -obj-y += cache.o +obj-y += cache.o endif ifdef CONFIG_SPL_BUILD obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o
I'm adding Michal here because this changes the behavior on xilinx platforms that had the icache off. I was under the impression that at the levels U-Boot runs at on aarch64, we just couldn't have icache/dcache off, but I guess that's wrong?
yes. We are using icache off for mini u-boot configurations for quite a long time. I tried to find out any commit why we started to use it but didn't find any record. But will ask team to retest that use cases to see if that is working or not. In our case we are using small full U-Boot not any SPL variant.
My biggest issue with the patch is that it is not clear what issue it is trying to solve as I have mentioned above. It just says that ideally icache/dcache should be on. If you want to enforce it on your platform you can select it when that platform is selected and don't need enforce it for other platforms.
Thanks Michal. Bhupesh, I was wrong in v1, having caches off is allowed, so you just need to fix issues like part one does where code doesn't compile when the options are disabled, and then are there further changes needed for your usecase?
Sure Tom. I will go back to sending v1 as v3 with some minor changes.
Thanks, Bhupesh
participants (5)
-
Bhupesh Sharma -
Michal Simek -
Patrice CHOTARD -
Patrick DELAUNAY -
Tom Rini