Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30686 )
Change subject: soc/intel/fsp1.1: Implement postcar stage ......................................................................
Patch Set 30:
(1 comment)
https://review.coreboot.org/#/c/30686/30/src/drivers/intel/fsp1_1/car.c File src/drivers/intel/fsp1_1/car.c:
https://review.coreboot.org/#/c/30686/30/src/drivers/intel/fsp1_1/car.c@151 PS30, Line 151: platform_enter_postcar();
Why would we do this here? I don't think I'm following currently of the combinations of things we're still trying to support.
C_ENV_BOOTBLOCK as well as romcc bootblock? Or is romcc bootblock going to be nuked? So when we would we use romstage_c_entry() and cache_as_ram_main() differently? If it's verstage after bootblock then we shouldn't be running postcar from this path. It should be going to romstage.
We might hit this problem on quark with fsp1.1 . I proposed removal on the mailinglist so I might do this first, because it's not possible to break it when it's gone.
This code is a mess. The fsp1.1 platforms are skylake (C_ENVIRONMENT_BOOTBLOCK also working fsp2.0) uses the function below, quark fsp1.1 (C_ENVIRONMENT_BOOTBLOC, it also has fsp2.0 so we might want to drop it), braswell romcc but with a patch for C_ENVIRONMENT_BOOTBLOCK that is not yet ready. braswell and quark use cache_as_ram_main() above, skylake uses romstage_c_entry() below here.
The reason for entering postcar here to be able to detect if romstage smashes stack.