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

[Bhupesh Sharma]

On Thu, 31 Aug 2023 at 19:01, Tom Rini <trini at 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 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?

Sure Tom. I will go back to sending v1 as v3 with some minor changes.

Thanks,
Bhupesh