Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35029 )
Change subject: soc/intel/{cnl, dnv, icl, skl}: Make top_of_ram align ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35029/1/src/soc/intel/cannonlake/ro... File src/soc/intel/cannonlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/35029/1/src/soc/intel/cannonlake/ro... PS1, Line 162: top_of_ram = ALIGN_DOWN((uintptr_t)cbmem_top(), top_of_ram_size); So.. if cbmem_top() was at 31 MiB, your WB cache region now covers 0 MiB to 16MiB ?
So you need to align cbmem_top(). One more reason to put everything about postcar_frame into memmap.c files instead. CB:34894
I was hoping we could evolve from CB:34897 and have the platform code just tell the absolute upper boundary for WB (often end of TSEG with alignment no smaller than 8 MiB) and a floating lower boundary that is guaranteed to cover all of CBMEM. But I believe Aaron commented UC holes would be required. I just don't see why anyone would access those (claimed UC-only) regions before MP init where reprogramming of all the MTRRs happens.