Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31762 )
Change subject: arch/x86/postcar: Add separate timestamp for postcar stage ......................................................................
Patch Set 1:
(2 comments)
I noticed the previous CL addresses some of my comments but not all.
https://review.coreboot.org/#/c/31762/1/src/arch/x86/postcar.c File src/arch/x86/postcar.c:
https://review.coreboot.org/#/c/31762/1/src/arch/x86/postcar.c@44 PS1, Line 44: run_ramstage(); There is the following in run_ramstage():
timestamp_add_now(TS_END_ROMSTAGE);
That addition should be conditionalized on ENV_POSTCAR and ENV_ROMSTAGE.
Similarly, TS_END_ROMSTAGE should reside in run_postcar_phase() where you put TS_START_POSTCAR.
That would be more consistent and not get a weirder sequence of timestamps.
https://review.coreboot.org/#/c/31762/1/src/arch/x86/postcar_loader.c File src/arch/x86/postcar_loader.c:
https://review.coreboot.org/#/c/31762/1/src/arch/x86/postcar_loader.c@178 PS1, Line 178: timestamp_add_now(TS_START_POSTCAR); It seems to me for romstage and ramstage the start timestamp is in the stage itself. Why is postcar's start timestamp not in the beginning of the stage?