Kyösti Mälkki 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 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34995/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34995/9//COMMIT_MSG@16 PS9, Line 16: TEST=Verified normal boot time on CML-Hatch with latest coreboot
That's SOC_INTEL_COMETLAKE, building from soc/intel/cannonlake, and this commit does not even touch […]
Ah right.. you were touching common code, that's where cometlake got it from. You should split cometlake TSEG as WB to separate commit, that one should be easy to +2.
https://review.coreboot.org/c/coreboot/+/34995/9//COMMIT_MSG@22 PS9, Line 22: Total Time: 910ms
Huh? You only move the function call from one file to another and you get this improvement??
It was not so obvious at first sight that pretty much every x86 platform gets TSEG as WB with this commit.
Some of the older CPUs are very limited on VAR MTRRs, I am not convinced we can globally add TSEG as WB everywhere, so rest of the API will take some time to discuss and find shape.