Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38700 )
Change subject: drivers/intel/fsp2_0: Add callback after non-XIP FSP-M load ......................................................................
Patch Set 7:
Patch Set 6:
After talking to Nico on IRC it finally became clear what his point is here: The memory reservation should be done in the FSP driver and not the Picasso SoC code, since it's not specific to Picasso, but to the (non-XIP & non-CAR) FSP-M case. Not sure if it would be better to rework this now or fix it later, but before the next platform will land
So the proposal is to just make FSP-M call bootmem_add_range() instead of adding the callback? I'm fine with that.
It's a little more. `bootmem` is a ramstage feature. We'd have to cache the address somewhere (some structure on the stack, or global, doesn't matter) then add it to CBMEM once it's up (to pass the info to ramstage) and in ramstage call finally bootmem_add_range().
We could also implement checks that nothing else collides with the FSP-M location (don't know how it's location is decided), e.g. CBMEM.
Correct. The broader issue is trying to mark memory reserved to the OS (and keeping its locations sorted out) to carry into ramstage for S3 resume purposes. I think I'll propose an alternative to work around the funkiness of picasso boot flow.