On Mon, Jun 13, 2016 at 05:51:23PM +0200, Ard Biesheuvel wrote: > On 13 June 2016 at 17:45, Mark Rutland <mark.rutl...@arm.com> wrote: > > On Mon, Jun 13, 2016 at 05:26:07PM +0200, Ard Biesheuvel wrote: > >> On some platforms, performing cache maintenance on regions that are backed > >> by NOR flash result in SErrors. Since cache maintenance is unnecessary in > >> that case, create a PEIM specific version that only performs said cache > >> maintenance in its constructor if the module is shadowed in RAM. To avoid > >> performing the cache maintenance if the MMU code is not used to begin with, > >> check that explicitly in the constructor. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > >> --- > >> > >> As discussed in the thread dedicated to this subject, the preferred way of > >> addressing this to split off the MMU manipulation code from ArmLib into a > >> separate ArmMmuLib, but this affects other packages and platforms. So in > >> the > >> mean time, let's merge this patch so that D02 can use the upstream ArmLib > >> unmodified. > > > > I'm missing something here (and couldn't figure out which thread covered > > this earlier), and this feels pretty suspect. > > > > Why does cache maintenance to these regions result in SError in the > > first place? > > > > Is the SRAM not accepting some writeback or fetch that occurs as a > > result? How are we protected against natural evictions and/or fetches? > > > > Does the SRAM respond badly to a CMO itself for some reason? > > > > It's NOR flash, not SRAM, and it is basically a quirk of the D02 > non-DRAM memory bus implementation.
Ok. I take it that means the CMO itself is the issue (i.e. it gets forwarded to the memory controller endpoint, which barfs), rather than asynchronous writebacks or fetches being a potential issue. If Heyi can confirm that, then this looks fine to me. For the sake of future reference, it would be nice to note the specific issue in the commit message. > I will let Heyi respond with more details, but this patch is basically > a stop gap solution until we split off the MMU code from ArmLib, which > will allow us to deal with this more elegantly (Currently, the only > PEI phase module that manipulates the page tables with the MMU on, and > is likely to split entries is DxeIpl, which maps the DXE phase stack > non-executable. The only other user is the module that creates the > page tables in the first place, and so does not require the extra > caution when splitting block entries) Understood! My only concern was whether this was masking an issue with asynchronous behaviour. Thanks, Mark. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
