[PATCH] arm: stm32mp1: activate data cache in SPL and before relocation

[Marek Vasut]

On 3/30/20 3:49 PM, Patrick DELAUNAY wrote:
> Hi Marek,

Hi,

[...]

>>> -	/* Enable D-cache. I-cache is already enabled in start.S */
>>> +	/* I-cache is already enabled in start.S */
> 
> Not needed for arm V7 (I copy this function from other platfrom ... I don't remember which one)

Maybe this needs to be de-duplicated if it's a copy ?

[...]

>>> diff --git a/arch/arm/mach-stm32mp/dram_init.c
>>> b/arch/arm/mach-stm32mp/dram_init.c
>>> index 3233415eff..15b39ecc03 100644
>>> --- a/arch/arm/mach-stm32mp/dram_init.c
>>> +++ b/arch/arm/mach-stm32mp/dram_init.c
>>> @@ -7,9 +7,29 @@
>>>  #include <dm.h>
>>>  #include <lmb.h>
>>>  #include <ram.h>
>>> +#include <asm/cache.h>
>>>
>>>  DECLARE_GLOBAL_DATA_PTR;
>>>
>>> +static void set_mmu_dcache(u32 addr, u32 size) {
>>> +	int	i;
>>> +	u32 start, end;
>>> +
>>> +	start = addr >> MMU_SECTION_SHIFT;
>>> +	end = ((u64)addr + (u64)size) >> MMU_SECTION_SHIFT;
>>
>> Is this a copy of dram_bank_mmu_setup() in arch/arm/lib/cache-cp15.c ?
>> Why ?
> 
> It is not just a copy...
> 
> set__mmu_dache is only a static helper for  function dram_bank_mmu_setup()
> 
> I override the default implementation of the weak functon dram_bank_mmu_setup()

Can you instead augment the original implementation to cater for this
usecase or would that be too difficult ?

> 1/ mark  SYSRAM cacheable in SPL (embedded RAM used by SPL)
> 
> 2/ mark beginning of DDR cacheable in U-Boot pre-reloc (today all the DDR)
>     => I prepare a possible TF-A limitation: when TF-A is running in DDR,
>          a part of DDR is securized and can't be mapped to avoid speculative access 
> 
> 3/ after relocation, DDR init is performed.... 
>     use the default implementation to map all the DDR (gd->bd->bi_dram[0])
> 
>    PS: in future patches, I will only limit this case for reserved memory part,
>            with "no-map" tag (also for TF-A requirement)

OK, TF-A is just adding more and more problems.

> Now after my explcation I found a issue in my patch...
> SPL also use DDR (at least for CONFIG_SYS_SPL_MALLOC_START	0xC0300000) ,
>  so I need to marked DDR as cacheache also for SPL
> 
>>> +	for (i = start; i < end; i++) {
>>> +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
>>> +		set_section_dcache(i, DCACHE_WRITETHROUGH); #elif
>>> +defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC)
>>> +		set_section_dcache(i, DCACHE_WRITEALLOC); #else
>>> +		set_section_dcache(i, DCACHE_WRITEBACK); #endif
>>> +	}
>>> +}
>>
>> [...]
>>
>>> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
>>> index c34a720e0c..5203fc93ad 100644
>>> --- a/include/configs/stm32mp1.h
>>> +++ b/include/configs/stm32mp1.h
>>> @@ -58,8 +58,8 @@
>>>
>>>  /* limit SYSRAM usage to first 128 KB */
>>>  #define CONFIG_SPL_MAX_SIZE		0x00020000
>>> -#define CONFIG_SPL_STACK		(STM32_SYSRAM_BASE + \
>>> -					 STM32_SYSRAM_SIZE)
>>> +/* stack at STM32_SYSRAM_BASE + STM32_SYSRAM_SIZE -
>> PGTABLE_SIZE (=16kB) */
>>> +#define CONFIG_SPL_STACK		0x2FFFC000
>>
>> Can't you memalign() the pagetable area instead of this hacking around?
>> Or use something around board_init_f_alloc_reserve().
> 
> It was my first idea: use malloc
> 
> But as I try to activate the data cache as soon as possible.
> So before spl_early_init call (for spl in board_init_f) and malloc is not yet accessible.
> 
> And board_init_f_alloc_reserve  is only called in U-Boot board-f.c..... 
> after the MMU configuration for pre-relocation / not called in SPL.
> 
> I don't see this address as hack but a memory configuration of SYSRAM:
> 
> SYRAM content (board_f)
> - SPL code
> - SPL data
> - SPL stack (reversed order)
> - TTB 
> 
> But I can move it in BSS as global apl variable, I need to think about it....
> It is probably more clean.

Please do :)