Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36216 )
Change subject: soc/intel: common,skl,cnl,icl: drop reserved mmio memory size calculation ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36216/5/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/36216/5/src/soc/intel/common/block/... PS5, Line 187: - reserved_mmio_size;
I just went through all the affected memmap implementations again, still […]
going by the ascii art memory map those carve outs should be taken into account. I think that if we tested all the potential combinations that affect the memory map, I bet the cbmem_top() implementations are wrong. What we have now is a snapshot in coreboot of the things we've collectively seen/tested. As more people are retrofitting coreboot onto windows devices that have a larger set of unique combinations I bet we'll run into issues.
Marking areas as RAM is orthogonal to how those areas are actually used. My understanding was that the intent of this patchset is to remove the tight coupling to FSP implementation based on the various options (either passed into FSP or statically configured in CSE/soft straps). I personally like knowing the full memory map, but if it makes things easier for people to honor what is returned from FSP I'm not going to battle against it.
In short, I'm fine with the current patchset as long as we keep the region types marked consistently from how it was previously.