[PATCH 22/26 v6] spl: spl_legacy: Add cache flush after reading U-Boot image

[Stefan Roese]

On 09.04.20 18:47, Daniel Schwierzeck wrote:
> 
> 
> Am 09.04.20 um 09:43 schrieb Stefan Roese:
>> On 09.04.20 09:29, Simon Goldschmidt wrote:
>>> Am 08.04.2020 um 10:09 schrieb Stefan Roese:
>>>> From: Weijie Gao <weijie.gao at mediatek.com>
>>>>
>>>> Flush the cache after reading of the U-Boot proper into SDRAM so that
>>>> it can be started.
>>>>
>>>> This is needed on some platforms, e.g. MT76x8.
>>>>
>>>> Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
>>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>> Cc: Weijie Gao <weijie.gao at mediatek.com>
>>>> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>>>> ---
>>>> Changes in v6:
>>>> - New patch
>>>>
>>>>    common/spl/spl_legacy.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
>>>> index 2cd2a74a4c..e320206098 100644
>>>> --- a/common/spl/spl_legacy.c
>>>> +++ b/common/spl/spl_legacy.c
>>>> @@ -4,6 +4,7 @@
>>>>     */
>>>>      #include <common.h>
>>>> +#include <cpu_func.h>
>>>>    #include <malloc.h>
>>>>    #include <spl.h>
>>>>    @@ -108,5 +109,8 @@ int spl_load_legacy_img(struct spl_image_info
>>>> *spl_image,
>>>>            return -EINVAL;
>>>>        }
>>>>    +    /* Flush cache of loaded U-Boot image */
>>>> +    flush_cache((unsigned long)spl_image->load_addr, spl_image->size);
>>>> +
>>>
>>> I failed to find the mail, but haven't we discussed moving this cache
>>> flush to your arch before starting a binary?
>>
>> I don't remember such an agreement. But I don't object in general.
>>
>>> I cannot see this being required or implemented for non-legacy images,
>>> and it still seems wrong here.
>>
>> Its pretty common when an OS image is loaded and booted, that the cache
>> is flushed before running code from it.
>>
>> But I can rework this to add some empty weak function (I don't see an
>> easy better way) to do this platform specific image handling before its
>> booted. Or do you have a better idea on how to handle this?
>>
>> Thanks,
>> Stefan
> 
> actually all MIPS platforms with non-coherent cache need that flush
> before jumping to another U-Boot or OS. Thus currently random crashes
> can occur on all MIPS boards with generic SPL not just mtmips.
> 
> But jump_to_image_no_args() in spl.c is already declared as weak, so we
> should implement that unconditionally for MIPS similar to your patch for
> do_go_exec(). It would be great if you could prepare a patch to replace
> this one, thanks.

Sure, good idea. I'll drop this patch and will add the cache flush to
the newly created MIPS specfic version of jump_to_image_no_args().

Thanks,
Stefan