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 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35165/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35165/1//COMMIT_MSG@12 PS1, Line 12: Don't need to run code logic to check the integrity of stack if FSP and : coreboot both likely to make use of common stack.
If FSP needs a memory area to use as a "heap", can it require one from the bootloader instead of tre […]
update the latest patchset which should resolve such question, i hope
https://review.coreboot.org/c/coreboot/+/35165/1/src/cpu/intel/car/romstage.... File src/cpu/intel/car/romstage.c:
https://review.coreboot.org/c/coreboot/+/35165/1/src/cpu/intel/car/romstage.... PS1, Line 63: if (!CONFIG(FSP_USES_CB_STACK)) {
If we do need to go this route, I think we can make this a little cleaner with some helper functions […]
we don't need this now and FSP won't touch stack area for heap allocation hence we won't see "Smash stack" issue now
https://review.coreboot.org/c/coreboot/+/35165/2/src/cpu/intel/car/romstage.... File src/cpu/intel/car/romstage.c:
https://review.coreboot.org/c/coreboot/+/35165/2/src/cpu/intel/car/romstage.... PS2, Line 63: if (!CONFIG(FSP_USES_CB_STACK)) {
So by this logic, the stack_guard should not be created when FSP_USES_CB_STACK is enabled either. […]
Ack
https://review.coreboot.org/c/coreboot/+/35165/2/src/cpu/intel/car/romstage.... PS2, Line 67: printk(BIOS_DEBUG, "Smashed stack detected in" : "romstage!\n");
thanks noted. […]
Ack