Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45059 )
Change subject: soc/amd/picasso: pass verstage timestamps to x86 ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45059/3/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/45059/3/src/soc/amd/picasso/bootblo... PS3, Line 146: tse->entry_stamp = abs_stamp * tick_freq_mhz; I don't think this math is correct (despite my objection of doing this at all). You essentially have two time domains and trying to align the units. 2 multiplications won't scale that since abs_stamp is just in the time unit from psp (in this case 1 us but nothing guarantees that). The timestamp_table object has tick_freq_mhz in it. The values need to be scaled up or down based on timestamp_tick_freq_mhz() and the one in the transfer table. It seems you are assuming everything is 1us and not commenting on that assumption.
COLLECT_TIMESTAMPS_NO_TSC uses the monotonic timer as a timestamp source which by definition is in microseconds. Therefore, when that is used nothing is needed for scaling (though there's still an issue of monotonically increasing values which may need a relative offset).
https://review.coreboot.org/c/coreboot/+/45059/3/src/soc/amd/picasso/bootblo... PS3, Line 157: bootblock_main_with_basetime(base_timestamp); As I noted before line 154 and this should be collapsed as well as removing #if conditionals.