Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41429 )
Change subject: soc/intel/jasperlake: Add ISST override support ......................................................................
Patch Set 1:
Patch Set 1:
Patch Set 1:
Patch Set 1:
Couple of reasons I added this callback:
- It happens as part of CPU initialization. It definitely happens before mainboard Silicon initialization(as pointed out by Tim). I have to check if mainboard Chip initialization. If this happens before CPU initialization, then we can update the configuration there.
This is the sequence in which different bootstates are executed: https://review.coreboot.org/cgit/coreboot.git/tree/src/include/bootstate.h?i.... IIUC, CPU initialization is being done as part of BS_DEV_INIT, whereas mainboard chip initialization happens in BS_DEV_INIT_CHIPS.
I see that CPUs are getting initialized also at BS_DEV_INIT_CHIPS boot state - https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/common/bloc...
The test also indicates the ISST is getting enabled if moved to mainboard chip init.
From the BIOS logs, I can see that the CPU initialization are done on entry callback to BS_DEV_INIT_CHIPS stage. The mainboard chip init and Silicon Init are done as part of BS_DEV_INIT_CHIPS i.e. run part of BS_DEV_INIT_CHIPS. So the only option to avoid the callback in cpu.c is to add a callback on entry or exit stage of BS_PRE_DEVICE stage. Please find the BIOS logs for reference:
Initializing CPU #0 ... CPU #0 initialized Initializing CPU #2 Initializing CPU #3 Initializing CPU #1 ... CPU #3 initialized ... CPU #1 initialized ... CPU #2 initialized bsp_do_flight_plan done after 248 msecs. CPU: frequency set to 1900 MHz Enabling SMIs. Locking SMM. BS: BS_DEV_INIT_CHIPS entry times (exec / console): 177 / 217 ms ... WEAK: src/soc/intel/jasperlake/fsp_params.c/mainboard_silicon_init_params called Detected 4 core, 4 thread CPU. ... IPC3: 0xffffffff BS: BS_DEV_INIT_CHIPS run times (exec / console): 510 / 134 ms RTC Init ... Disabling Deep S5 BS: BS_DEV_INIT_CHIPS exit times (exec / console): 3 / 19 ms
If the requirement is that this should be done before FSP-S runs, then what Tim suggested makes sense. Basically, add a variant_devtree_update() as part of mainboard_silicon_init_params(). That guarantees the right ordering.