Am 19.02.2019 um 17:00 schrieb Marek Vasut:
On 2/19/19 4:55 PM, Simon Goldschmidt wrote:
Am 19.02.2019 um 15:00 schrieb Marek Vasut:
On 2/19/19 2:58 PM, Simon Goldschmidt wrote:
Am Di., 19. Feb. 2019, 14:45 hat Marek Vasut <ma...@denx.de
<mailto:ma...@denx.de>> geschrieben:
On 2/19/19 2:28 PM, Simon Goldschmidt wrote:
> On Tue, Feb 19, 2019 at 1:53 PM Marek Vasut <ma...@denx.de
<mailto:ma...@denx.de>> wrote:
>>
>> On SoCFPGA Gen5 systems, it can rarely happen that a reboot from
Linux
>> will result in stale data in PL310 L2 cache controller. Even if
the L2
>> cache controller is disabled via the CTRL register CTRL_EN
bit, those
>> data can interfere with operation of devices using DMA, like
e.g. the
>> DWMMC controller. This can in turn cause e.g. SPL to fail
reading
data
>> from SD/MMC.
>>
>> The obvious solution here would be to fully reset the L2 cache
controller
>> via the reset manager MPUMODRST L2 bit, however this causes bus
hang even
>> if executed entirely from L1 I-cache to avoid generating any bus
traffic
>> through the L2 cache controller.
>>
>> This patch thus configures and enables the L2 cache controller
very early
>> in the SPL boot process, clears the L2 cache and disables the L2
cache
>> controller again.
>>
>> The reason for doing it in SPL is because we need to avoid
accessing any
>> of the potentially stale data in the L2 cache, and we are
certain
any of
>> the stale data will be below the OCRAM address range. To further
reduce
>> bus traffic during the L2 cache invalidation, we enable L1
I-cache and
>> run the invalidation code entirely out of the L1 I-cache.
>>
>> Signed-off-by: Marek Vasut <ma...@denx.de
<mailto:ma...@denx.de>>
>> Cc: Dalon Westergreen <dwest...@gmail.com
<mailto:dwest...@gmail.com>>
>> Cc: Dinh Nguyen <dingu...@kernel.org
<mailto:dingu...@kernel.org>>
>
> We've tested this and it seems to fix the issue, so:
>
> Tested-by: Simon Goldschmidt <simon.k.r.goldschm...@gmail.com
<mailto:simon.k.r.goldschm...@gmail.com>>
Seems like I was a little fast there: the patch does not apply cleanly
to master. It did apply to our tree, obviously.
It's missing an include to <asm/pl310.h> and 'pl310' is undefined.
Probably a result of rebasing it...
>
> However, I don't really get why clearing the L2 cache later in
U-Boot
> isn't good enough.
Because when U-Boot is running, it is already running from RAM,
which is
on different PL310 master than OCRAM, and you're likely to hit the
corrupted cache lines on the DRAM master which is primed by
prior Linux
operation. Such cacheline can be hit between the code enabling
the PL310
and the code invalidating it, which is a C code, using stack and
calling
functions, thus accessing memory which would likely reside in
different
PL310 cachelines. If one of those cachelines contains
invalid/corrupted
data, they can be provided to the CPU before the cacheline is
invalidated.
To further reduce the likelihood of hitting any such cache line,
the
code which enables the PL310 and invalidates it runs from primed L1
icache lines, thus generating no bus traffic at all.
Ah, right. I somehow didn't realize that invalidating is done after
enabling:-)
Right. If it could be done before (like e.g. by whacking the L2CC
reset), that'd be fantastic.
I haven't yet found a way to cleanly reproduce this on my socrates, so
unfortunately, I currently cannot test any suggestions, right now :-(
Did you ever have this hang happen on SoCrates ?
I think so, yes.
What I did was running some lmb tests loading from mmc, removing the
sdcard to write an updated version of U-Boot, replacing it and then
typing 'reset'. It would then sometimes hang in cache initialization - I
added debug prints throughout cache initialization and was debug prints
from one function and then nothing more after a simple function call...
That would very much fit to your explanation. However, I cannot
reproduce this currently :-(
Wouldn't it be better to place this function in some central position
near the cache initialization code instead of running it from SPL? Or is
it required to run it in SPL?
It's required in SPL as soon as possible, to avoid accessing the DRAM,
see above. The current location is close to the rest of PL310 init in SPL.
OK, I just did not understand why this is necessary before accessing DRAM.
Also, it seems this patch enables the ICACHE in SPL, which wasn't
enabled before. Not a bad thing, but might this have side effects?
Actually, I-Cache is enabled in SPL:
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/start.S;h=0cb6dd39ccfc0584f4e6f01523fef9405c314763;hb=HEAD#l158
Oh, right. Unless disabled via config option.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
