Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34897 )
Change subject: arch/x86: Simplified postcar WB MTRR setup ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34897/1/src/soc/intel/skylake/memma... File src/soc/intel/skylake/memmap.c:
https://review.coreboot.org/c/coreboot/+/34897/1/src/soc/intel/skylake/memma... PS1, Line 317: /* Do some magic alignments */
Can you pinpoint me to cases/platforms where such holes are required, between TSEG and cbmem_top(), before we reprogram these MTRRs again? MTRR reprogramming used to happen early in PARALLEL_MP, but I do see it may be delayed until payload entry.
There are certain chipset options which are allocated there. Check out line 169 and onwards in this file which has a pictorial description.
We can argue the merits of why stolen memory actually surfaces in the host memory map... but that's an Intel decision in their implementation.
Many platforms already do this and select "a suitable alignment somewhere higher than cbmem_top()". Aligning wb_end downwards relaxes us from having any alignment requirement on the implementation on cbmem_top(), which is somewhat convoluted for Intel implementations with EBDA located near TOLM.
Yes, I understand the technical desires for this change, but it is ignoring the other selectable options which invalidates the contiguous address range. I think I noted earlier that we could modify the postcar_loader.c code to merge entries if the memory map aligns such that we can optimize the use of variable mtrrs. However, I haven't seen a situation where it's required because of exhausting variable mtrr resources.