On 2/19/19 5:31 PM, Simon Goldschmidt wrote: > 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 :-(
So basically a board which uses DMA (like the DWMMC DMA) is likely to be affected. Surely, the DMA does trigger PL310 operations. >>> 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. Yes -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
