Nathaniel L Desimone has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35165 )
Change subject: cpu/intel/car: Skip stack integrity check if FSP_USES_CB_STACK is enable ......................................................................
Patch Set 3:
I think the previous comments were detailed enough to explain the situation; stack and heap must be carved out from CAR as two separate allocations. You are currently embedding this "temporary RAM" into the stack allocation, messing up the attempts to have usable stack guard checks.
That was not the intention from a FSP specification standpoint anyway. Purely for backwards compatibility reasons, the FSP's heap is confusingly called "StackBase" and "StackSize" even though it is totally not used for a stack starting with v2.1.
The intention is that during Pre-Memory phase StackBase, and StackSize would be a separate memory space only used for storing HOBs produced by the FSP, and the FSP MemoryInit() function call would run on top of the coreboot stack. The reason we made this change is because memory is at a premium before DRAM is available, and we wanted to share that memory with coreboot better than the previous design of just taking the memory and reserving it for FSP.
I must confess that I don't know a whole lot about coreboot's stack allocation mechanisms, but at a basic level it makes sense to me that if the FSP MemoryInit() runs on the same stack as coreboot then the coreboot stack will need to be larger. For a full memory training FSP uses approximately 128KB, which matches up with the new value for the DCACHE_BSP_STACK_SIZE kcfg option. That said, I have no idea if this is the best way to implement this and defer to others for that feedback.