[U-Boot] [PATCH] ARM: socfpga: Clear PL310 early in SPL
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Tue Feb 19 16:31:00 UTC 2019
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 <marex at denx.de
>>>> <mailto:marex at denx.de>> geschrieben:
>>>>
>>>> On 2/19/19 2:28 PM, Simon Goldschmidt wrote:
>>>> > On Tue, Feb 19, 2019 at 1:53 PM Marek Vasut <marex at denx.de
>>>> <mailto:marex at 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 <marex at denx.de
>>>> <mailto:marex at denx.de>>
>>>> >> Cc: Dalon Westergreen <dwesterg at gmail.com
>>>> <mailto:dwesterg at gmail.com>>
>>>> >> Cc: Dinh Nguyen <dinguyen at kernel.org
>>>> <mailto:dinguyen at kernel.org>>
>>>> >
>>>> > We've tested this and it seems to fix the issue, so:
>>>> >
>>>> > Tested-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com
>>>> <mailto:simon.k.r.goldschmidt at 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
More information about the U-Boot
mailing list