[PATCH v2 2/2] armv8: Disable SYS_DCACHE_OFF & SYS_ICACHE_OFF for ARM64

[Tom Rini]

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 at konsulko.com>
> > > Cc: Simon Glass <sjg at chromium.org>
> > > Cc: Peng Fan <peng.fan at nxp.com>
> > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma at linaro.org>
> > > Reviewed-by: Tom Rini <trini at 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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230831/40482161/attachment.sig>