Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36137 )
Change subject: soc/intel: skl,cnl,icl: consolidate ebda and memmap and move to common code ......................................................................
Patch Set 19: Code-Review+1
(7 comments)
I'm not sure about the block/ location. However, this wouldn't be the first software feature there... or maybe put it into block/systemagent/?
https://review.coreboot.org/c/coreboot/+/36137/19//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36137/19//COMMIT_MSG@7 PS19, Line 7: and move to common code `and ...` not needed and exceeds line length
https://review.coreboot.org/c/coreboot/+/36137/19/src/soc/intel/common/block... File src/soc/intel/common/block/memmap/memmap.c:
https://review.coreboot.org/c/coreboot/+/36137/19/src/soc/intel/common/block... PS19, Line 40: also maps into MC address space Does anybody know what this means?
https://review.coreboot.org/c/coreboot/+/36137/19/src/soc/intel/common/block... PS19, Line 54: * +---------------------------+ TOLUM / top_of_ram (aligned) This is the same as cbmem_top
https://review.coreboot.org/c/coreboot/+/36137/19/src/soc/intel/common/block... PS19, Line 54: aligned Aligned to what?
https://review.coreboot.org/c/coreboot/+/36137/19/src/soc/intel/common/block... PS19, Line 56: * +---------------------------+ cbmem_top Not cbmem_top, as CBMEM root is part of CBMEM
https://review.coreboot.org/c/coreboot/+/36137/19/src/soc/intel/common/block... PS19, Line 62: FSP TOLUM FSP makes no sense here. If anything, the above TOLUM would be the FSP one.
https://review.coreboot.org/c/coreboot/+/36137/19/src/soc/intel/common/block... PS19, Line 66: * Some of the base registers above can be equal making the size of those : * regions 0. The reason is because the memory controller internally subtracts : * the base registers from each other to determine sizes of the regions. In : * other words, the memory map is in a fixed order no matter what. This confuses me. Does it tell us anything non-obvious?