[PATCH] arm/xilinx_zynq: Disable the MMU and the data and instruction cache on boot.

[Pavel Pisa]

Hello Chris,

On Tuesday 09 of August 2016 09:19:51 Chris Johns wrote:
> The u-boot loader can enable the MMU plus the data and instruction caches.
> Disable them and if the data cache is enabled clear it before turn the
> caches off.
>
> Closes #2774.
> ---
>  c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h   | 4 ++++
>  c/src/lib/libbsp/arm/xilinx-zynq/startup/bspstartmmu.c | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h
> b/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h index
> 01fdbb3..7734ddc 100644
> --- a/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h
> +++ b/c/src/lib/libbsp/arm/shared/include/arm-cp15-start.h
> @@ -166,6 +166,10 @@ arm_cp15_start_setup_mmu_and_cache(uint32_t
> ctrl_clear, uint32_t ctrl_set) {
>    uint32_t ctrl = arm_cp15_get_control();
>
> +  if ((ctrl & ARM_CP15_CTRL_C) != 0) {
> +    arm_cp15_data_cache_clean_all_levels();
> +  }
> +

I am not sure if this should be combined in arm_cp15_start_setup_mmu_and_cache
function. In the fact, the cache clean and disable can be required
for operations (for example interrupt vectors setup) between
bsp_start_hook_0 and bsp_start_hook_1 functions.
So I would incline to leave this code in the board specific
bsp_start_hook_0.

There can be even some problems with arm_cp15_data_cache_clean_all_levels
on on different ARM architecture variants. 

The arm_cp15_start_setup_mmu_and_cache is used on all variants (ARM920T
for example) but arm_cp15_data_cache_clean_all_levels is ARMv7 specific
for now.

There is even problem on SMP. You need to setup MMU on the secondary CPUs
but arm_cp15_data_cache_clean_all_levels is highly problematic to be called
when other CPU is already running in MMU enabled state. You can corrupt
L2 cache content there because clean by set and way is not (according to my
ARMv7A+R manual reading) guaranteed to lead to snoop based caches 
synchronization. Only L1 should be clean on the secondary CPUs.

So I have kept the code in bsp_start_hook_0 for RaspberryPi

https://git.rtems.org/rtems/tree/c/src/lib/libbsp/arm/raspberrypi/startup/bspstarthooks.c#n39

I use there RTEMS cache manager functions wrapping CP15 as well
because these are implemented such way that they are at least compatible
with ARMv& and ARMv6. Direct calling arm_cp15_data_cache_clean_all_levels
could be problematic on ARMv6. On the other hand, Zynq is ARMv7 only
so direct call should not be a problem

rtems_cache_flush_entire_data();
rtems_cache_invalidate_entire_data();

I call rtems_cache_invalidate_entire_data() later
even if the cache is disabled to ensure that there
are no stalled data in the cache from the loader which
enables and disables cache temporarily. If I remember
well, I have some problems with such stale content
in HYP mode startup case.

Sebastian, what is your opinion there?
I am not sure about consequences regarding SMP.

Best wishes,

                Pavel

>    ctrl &= ~ctrl_clear;
>    ctrl |= ctrl_set;
>
> diff --git a/c/src/lib/libbsp/arm/xilinx-zynq/startup/bspstartmmu.c
> b/c/src/lib/libbsp/arm/xilinx-zynq/startup/bspstartmmu.c index
> c7a1089..0918588 100644
> --- a/c/src/lib/libbsp/arm/xilinx-zynq/startup/bspstartmmu.c
> +++ b/c/src/lib/libbsp/arm/xilinx-zynq/startup/bspstartmmu.c
> @@ -41,7 +41,7 @@ BSP_START_TEXT_SECTION void
> zynq_setup_mmu_and_cache(void) __attribute__ ((weak) BSP_START_TEXT_SECTION
> void zynq_setup_mmu_and_cache(void)
>  {
>    uint32_t ctrl = arm_cp15_start_setup_mmu_and_cache(
> -    ARM_CP15_CTRL_A,
> +    ARM_CP15_CTRL_A | ARM_CP15_CTRL_C | ARM_CP15_CTRL_I | ARM_CP15_CTRL_M,
>      ARM_CP15_CTRL_AFE | ARM_CP15_CTRL_Z
>    );