Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34995 )
Change subject: arch/x86: Cache the TSEG region at the top of ram ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34995/9/src/arch/x86/postcar_loader... File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/c/coreboot/+/34995/9/src/arch/x86/postcar_loader... PS9, Line 203: return;
The control can be done by open coding the call at all the call sites that are needed or utilize a K […]
i have spitted this CL in former approach. Kindly take a look
https://review.coreboot.org/c/coreboot/+/34995/9/src/arch/x86/postcar_loader... PS9, Line 210: MTRR_TYPE_WRBACK);
Minor correection: For S3 resume path, code does not run from TSEG, stage_cache_load_stage() is the reverse memcpy() of stage_cache_add().
I really would like to do just the following and always have TSEG as WB, but your words 'almost all platforms' makes it difficult to have the call in common code.
if (CONFIG(SMM_TSEG)) enable_tseg_cache();
did the same from platform code now
I tried something similar with 0..CACHE_TMP_RAMTOP as WB and got feedback all recent soc/intel failed. See CB:35187, maybe soc/intel just has buggy CAR teardown? There is still that unexplained behaviour around timestamp 1100 in previously provided logs, where marking TSEG as WB in postcar has triggered 10 ms delay late in payload.
surprisingly i don't see 1100 is taking the extra 10ms now when marked TSEG as WB with latest chromium code
At least intel/cannonlake has this:
* Skip Pre MP init MTRR programming as MTRRs are mirrored from BSP, * that are set prior to ramstage.Since memcpy() for SMI handler does not happen in SMM mode, this relies on variable MTRRs. Not sure what was the motivation to delay the final MTRRs here?