Attention is currently required from: Tarun Tuli, Jamie Ryu, Subrata Banik, Paul Menzel, Kapil Porwal.
Arthur Heymans 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/45e4bf77_3b2f3f78 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).
Thanks! I was not aware of modern implementations.
File src/soc/intel/common/basecode/tom/tom.c:
https://review.coreboot.org/c/coreboot/+/73272/comment/873d1f22_3e824a85 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 ?
soc/intel/xeon_sp/cpx/romstage.c has a good example
https://review.coreboot.org/c/coreboot/+/73272/comment/c167fa69_eb8a166c 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.
Ok. Maybe add this as a comment?
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.
Right. https://review.coreboot.org/c/coreboot/+/37196 should help.
i hope u r suggesting like this ?
set_var_mtrr(&mtrr_ctx.ctx, 0, tom, MTRR_TYPE_WRBACK);
yes