Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34883 )
Change subject: [WIP] amd/stoneyridge: Fix CAR stack location ......................................................................
Patch Set 1:
Patch Set 1:
Why do code that has always worked are now marked as problem? In particular, why are globals prohibited in romstage?
Placement of valuables must agree with the linker script. Otherwise it only works by luck, not by design. In other words, the codebase is volatile; increase pre-ram cbmem console or timestamps stash and it will crash runtime instead of failing already at buildtime.
Prohibited globals in romstage goes back to days with CAR_GLOBAL_MIGRATION=y, did you come across some stale comments about it or why did you bring it up here?
That said, what would happen if an AP executed the code you introduced? Would it be easier to modify the macro AMD_ENABLE_STACK instead (though more files)? The macro already detect BSP when setting the stack. I believe you might avoid the global issue by modifying the macro itself.
This code without fix? AP would overwrite BSP stack.
AMD_ENABLE_STACK is a mammoth. IMO, preferably assembly would reference the stack locations by the symbol names from car.ld to avoid the mismatch. Linker script would have authority. But car.ld does not even account for the CAR region APs use...