Julius Werner has posted comments on this change. ( https://review.coreboot.org/19785 )
Change subject: arm64: Align cache maintenance code with libpayload and ARM32 ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/19785/1/src/arch/arm64/armv8/cache.c File src/arch/arm64/armv8/cache.c:
Line 126: else if (sctlr & SCTLR_I)
Are _C and _I mutually exclusive?
No. If _C is set, we want to run clean_all() (regardless of whether _I is set as well... usually it would be).
The reason ARM32 has it this way goes back to a weird edge case discussed in https://chromium-review.googlesource.com/c/236422/ : if you have the instruction cache enabled (_I) but don't have the data cache enabled (_C), instruction fetches will still fetch instructions into the unified L2 cache. Then if you have a situation like OVERLAP_VERSTAGE_ROMSTAGE where you load different code to the same addresses, running a clean_all() will just flush those old instructions from L2 back to RAM and overwrite the new instructions you just loaded.
They figured out that they needed this to get that Qualcomm board to boot reliably. For all our current boards this should make no difference since they will always run with _C and _I right from the bootblock. With their data caches enabled they will just write the new stage code to the L2 cache at load time and a flush without invalidate is enough to get back in sync with RAM.