Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46855 )
Change subject: mrc_cache: Move force memory retraining code into mrc_cache ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46855/1/src/drivers/mrc_cache/mrc_c... File src/drivers/mrc_cache/mrc_cache.c:
https://review.coreboot.org/c/coreboot/+/46855/1/src/drivers/mrc_cache/mrc_c... PS1, Line 264: if (!CONFIG(HAS_RECOVERY_MRC_CACHE)) I don't think we can add this here unless we want to change policy. I think the way this driver is currently written is that by default, recovery mode will also use the normal MRC cache if HAS_RECOVERY_MRC_CACHE is not set. That's why the normal_training region has both NORMAL_FLAG and RECOVERY_FLAG set, and the cache_regions[] array is carefully ordered so the explicit recovery cache (if it exists) comes first.
So either this check will have to stay in the platform code, or we need to turn it into a generic Kconfig (e.g. CONFIG_MRC_ALWAYS_RETRAIN_IN_RECOVERY or something) so platforms can select which behavior they want (and then, that Kconfig would probably be best implemented by just making it decide whether normal_training has RECOVERY_FLAG set or not).