Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35165 )
Change subject: arch/x86: Reserve some CAR memory for FSP single stack requirement ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35165/9/src/arch/x86/car.ld File src/arch/x86/car.ld:
https://review.coreboot.org/c/coreboot/+/35165/9/src/arch/x86/car.ld@94 PS9, Line 94: #if CONFIG(FSP_USES_CB_STACK)
Yes, it should be guarded to be more correct. However, what matters here is the ordering. […]
yes, agree the ordering is important hence i have added the layout diagram to call out where FSP heap should exist. i hope that helps. Now as you said, will guard with ENV_ROMSTAGE to make it explicit. but i need to check in case of FSP-T usecase do FSP need to have heap region reversed as well (i hope that would be the case as well), right not chrome doesn't use FSP_t but there are some user for FSp-T as well.
https://review.coreboot.org/c/coreboot/+/35165/9/src/drivers/intel/fsp2_0/Kc... File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/35165/9/src/drivers/intel/fsp2_0/Kc... PS9, Line 209: default 0x10000
we had a sync with FSP team and finalize heap = 0x10000 and stack = 0x20000 for now on all FSP2. […]
seems like this always the case with single stack assumption where FSP need to have bigger stack size and heap requirement is something that personally i have noticed with this issue but it was there and got resolved by our arch->StacKBase UPD wrongly assumed as stack base for FSP to provide additional 128KB for heap as well.
Now hopefully with CML integration guide documentation, FSP stack requirement ~128KB (mostly getting used in FSP-M training considering both debug and release mode) and heap requirement ~64KB (for all hobs)