Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37196 )
Change subject: cpu/x86/cache: CLFLUSH programs to memory before running ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c File src/cpu/x86/cache/cache.c:
https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 27: int clflush_supported(void) This is not sufficient. When CAR is active it's microarchitecturally defined how a clflush is handled in CAR mode. You need a way for the chipset to indicate this is valid sequence that can be performed.
https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@2... PS7, Line 29: 19 Aren't there defines for these bits instead of open coding them?
https://review.coreboot.org/c/coreboot/+/37196/7/src/cpu/x86/cache/cache.c@6... PS7, Line 60: if (!cbmem_ready()) Why is a cbmem check in here? It doesn't immediately make sense why this would be the case. Likewise, this function is also active when not loading postcar, e.g. FSP. What about that? Or are you using cbmem to be an indirect proxy for postcar loading?