Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35187 )
Change subject: [NOTFORMERGE] soc/intel exit_car.S ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35187/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/car/exit_car.S:
https://review.coreboot.org/c/coreboot/+/35187/2/src/soc/intel/common/block/... PS2, Line 48: /* Disable cache ??? */
Perhaps so, see comment on arch/x86/exit_car.S line 36. […]
Patrick Rudolph CB:34791 Aug 08 20:26
My tests on Sandy Bridge showed that MTRRs are only updated on CR0.CD=1 to CR0.CD=0 transitions. All of AMD and early Intel CPUs follow this scheme and update MTRRs with cache disabled. Isn't that transition required any more starting with APL?
From that commit comments, I did not find reference to Intel docs describing NEM at a satisfactory level of details. Instead I read about random lockups in ramstage (MP init) and consistent delay being triggered inside vboot payload after we had only added WB MTRRs in postcar frame. With access to BWG you probably can find exact paragraphs showing that it is no longer required to toggle CR0.CD to correcly exit non-evict mode??
The sequence for CONFIG_INTEL_CAR_NEM below is the same for sandy/ivy, except that the implementation Google provided does set CR0.CD here first. Having peeked into some random FSP-M disassembly, CR0.CD was definetly getting set at several locations, but with a 5 minute effort it's hard to see code that is on TempRamExit() execution path. That's the closed-source CAR teardown implementation.