Kangheui Won 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 10:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45059/6/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/45059/6/src/soc/amd/picasso/bootblo... PS6, Line 124: struct transfer_info_struct *info = (struct transfer_info_struct *)
Yeah, I think we do need to worry about when vboot isn't used. […]
Done
https://review.coreboot.org/c/coreboot/+/45059/9/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/45059/9/src/soc/amd/picasso/bootblo... PS9, Line 149: if (info->timestamp_offset != 0) {
Yeah, that's bad.
Oops, my bad. Fixed in latest patchset.
https://review.coreboot.org/c/coreboot/+/45059/9/src/soc/amd/picasso/bootblo... PS9, Line 174: base_timestamp += psp_ts_table->base_time - psp_last_ts;
I should also note that it appears the code is assuming 'A' always have a value of 0. […]
psp_last_ts is absolute so it's completely my mistake to put base_time here.
And I think what you suggested actually simplifies the math here a lot, thanks for the advice. Applied in latest patchset.
About PSP's initial timer value - it's nowhere documented but I think it has value of 0 on reset/resume from my experience.
https://review.coreboot.org/c/coreboot/+/45059/9/src/soc/amd/picasso/bootblo... PS9, Line 175: bootblock_main_with_timestamp(base_timestamp,
FWIW, this manipulation of timestamp table base time moves the basis around that's impossible to mai […]
In latest patchset we're moving basis to starting time of PSP timer, not the time when timestamp_init() is called in the PSP. So we're losing basis in the PSP but I think it's okay since the first entry(5:start of verified boot) will now contain that basis after adjusting and the time between basis in the PSP and the first entry is very small. (<2ms from past measurements)