Change in ...coreboot[master]: drivers/intel/fsp2_0: Enable FSP feature to make use of same stack wi...

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32079 )

Change subject: drivers/intel/fsp2_0: Enable FSP feature to make use of same stack with bootloader
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32079/2/src/drivers/intel/fsp2_0/memory_init... 
File src/drivers/intel/fsp2_0/memory_init.c:

https://review.coreboot.org/#/c/32079/2/src/drivers/intel/fsp2_0/memory_init... 
PS2, Line 175: 		arch_upd->StackBase = (void *)_car_stack_start;
> Commit description needs more elaboration. […]

yes, we have discovered a bug with existing implementation where car global variables were getting corrupted after FSP-M call return.

if you see TEST description, it hope its clear that nothing is corrupted now. This was more CB side bug rather FSP.

Also with previous implementation, we were not honoring same stack for CB and FSP, rather we were giving different stack base to both to grow but FSP can actually exist inside same CB stack with new implementation. From FSP team side, this feature been tested across other bootloader and we have also tested the same with coreboot. Should be good for us to enable this feature for ICL and CML going forward.

TEST=Build and boot FSP2.1 enable platform like dragonegg, iclrvp.
No stack variable or car global variable corruption seen after enabling
this feature.