Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34740 )
Change subject: intel/smm/gen1: Use smm_subregion() ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... File src/cpu/intel/smm/gen1/smmrelocate.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/cpu/intel/smm/gen1/smmr... PS1, Line 129: I'd not remove this check. If this is not the case, I recall there were some weird issues. On older systems it could happen with some gttsize/gfxsize and tseg size settings that the TSEG base is not aligned. Currently a nice value is typically chosen in the raminit for this to not happen.
https://review.coreboot.org/c/coreboot/+/34740/1/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.c:
https://review.coreboot.org/c/coreboot/+/34740/1/src/northbridge/intel/gm45/... PS1, Line 140: sub_size ASSERT(sub_size > cache_size) somewhere?