Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35266 )
Change subject: _WIP_ drivers/intel/fsp2_0: Use generic defines for early storage ......................................................................
Patch Set 1:
BTW, I really disliked this particular PS and I assumed it would be required to change due to whatever changes come before it.
Good, I dislike this too :) I will rebase and update this, there should not be much left to deal with.
I see "intel/fsp2_0: Move temporary RAM to .bss with FSP_USES_CB_STACK" and will include that in my next push. However, I have a new problem now so don't be alarmed when this patch is still in the stack. I'm not yet certain how I want to fix it, so I'm open to suggestions.
Right now we're still creating a 64K romstage.elf, and the .bss falls within _program (after the bulk of the functionality but below the reset vector). The large amount of space used for temp_ram in memory_init.c blows up the linking.
(a) Keeping it within _program has a nice side effect of zeroing it at build/compress time, so I'd need to expand my hybrid romstage for the flexibility of being >64K.
or (b) Theoretically my .bss could be relocated to the DRAM I'm burning for early temp storage. So far that hasn't worked the way I want. If it goes there, the BSP will need to explicitly clear that region, as the DRAM comes up with garbage in it. (Possibly data used during training; not sure.)
BTW, AFAIK the FSP_USES_CB_STACK symbol is for FSP 2.1 and we're building 2.0 images currently. We want FSP_USES_CB_STACK, and it's also our intent to change to 2.1 before production, but we're trying to keep as much to established specs as possible.