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

[Michal Simek]

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