Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35902 )
Change subject: soc/intel/apl,skl,cnl,icl: implement a PRMRR size selection algorithm ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35902/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35902/7//COMMIT_MSG@34 PS7, Line 34: Todo: needs testing
I think all we have to do is to give FSP-M a size setting that the CPU supports. Then we can be sure that it will use exactly that setting.
Yes, this is what I expect, as this is exactly what we are doing currently with that ugly devicetree setting.
Then, the only thing we have to know about FSP-M is how it will align PRMRR. (And only if we keep mimicking the whole FSP memmap.
As we use the EBDA to cache TOLUD now, I assume we could also get rid of that and store the address returned by FSP instead.)
If I am not mistaken, this would even allow us to get rid of the user limit I implemented, as we simply set the Kconfig value, the user selected, and then don't need to do any calculation here.
Subrata, in the coreboot code we currently have 128MiB alignment for the big cores and no alignment but 2x the configured size for GLK. Can you confirm the alignment, please? I don't have access to all the sources. A grep for `MAX_PRMRR` should help to find the respective code.
I'm rather sure the alignment is 256MiB for newer SoCs. Maybe Kaby Lake is the exception, though, and really aligns to 128MiB.
For me it seems the maximum possible value is used for alignedment. KBL does not support 256, so aligment happens to 128. Not sure for the previous platforms, though.