Attention is currently required from: Tarun Tuli, Jamie Ryu, Paul Menzel, Kapil Porwal, Arthur Heymans.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73272 )
Change subject: soc/intel/cmn/tom: Cache TOM region early ......................................................................
Patch Set 4:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/73272/comment/46711304_93b5bdb0 PS4, Line 10: non-volatile space (CMOS)
MRC_CACHE would be a better idea. On some cheaper systems a cmos battery is ommited to cut costs.
On latest Intel platform (starting with SKL mostly), CMOS memory is backup by CSE hence, the battery solution is not mandatory. We are anyway storing the platform boot count into the CMOS memory on chromeOS platform (since many generations now).
File src/soc/intel/common/basecode/tom/tom.c:
https://review.coreboot.org/c/coreboot/+/73272/comment/20310144_4c44cf96 PS4, Line 20: #defin
Assert that it does not overlap with other cmos options.
looking at the other files which uses the CMOS offset for storing details previously. Unable to find something which resembles with ur feedback.
Can you please share more details about what/how u wish to do this check of non-overlapping assets ?
https://review.coreboot.org/c/coreboot/+/73272/comment/c00d0306_8d948e01 PS4, Line 113: tom
Does TOM always have a 16M granularity?
Atleast on the client side, I'm seeing the same in last 5 generations.
Also on xeon_sp 16M does not cut it. Maybe use mtrr_ctx like on AMD code to cache the region below TOM as WB? See void early_cache_setup(void) for that.
I believe I'm seeing hang with WB and mostly while postcar is tries to tear-down the CAR.
i hope u r suggesting like this ?
``` set_var_mtrr(&mtrr_ctx.ctx, 0, tom, MTRR_TYPE_WRBACK); ```
-
Subrata Banik (Code Review)