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 5:
(6 comments)
https://review.coreboot.org/c/coreboot/+/45059/2/src/soc/amd/picasso/bootblo... File src/soc/amd/picasso/bootblock/bootblock.c:
https://review.coreboot.org/c/coreboot/+/45059/2/src/soc/amd/picasso/bootblo... PS2, Line 125: CONFIG_PSP_SHAREDMEM_BASE;
CONFIG_PSP_SHAREDMEM is a #define that is an int to the type system. […]
Done
https://review.coreboot.org/c/coreboot/+/45059/2/src/soc/amd/picasso/bootblo... PS2, Line 130: uint32_t tick_freq_mhz = timestamp_tick_freq_mhz();
We should normalize to use 1us tick regardless of environment. Then there's no funny math required.
I think this conversation continued on another comment on patchset 3, if it's different question please reopen this.
https://review.coreboot.org/c/coreboot/+/45059/2/src/soc/amd/picasso/bootblo... PS2, Line 149: base_timestamp = psp_ts_table->base_time * tick_freq_mhz;
None of this ensures the timestamp from x86 is monotonically increasing relative to psp. […]
I specifically asked AMD about this and haven't got answer yet: b/167148121
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 125: CONFIG_PSP_SHAREDMEM_BASE
Would it be better to use the '_transfer_buffer' symbol here? It's based on the value in CONFIG_PSP […]
Done
https://review.coreboot.org/c/coreboot/+/45059/3/src/soc/amd/picasso/bootblo... PS3, Line 132: CONFIG_PSP_SHAREDMEM_BASE
Same idea here, but maybe use the symbol _psp_sharedmem_dram?
Changed to _transfer_buffer. Is there any reason to use two different names?
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). […]
If we use COLLECT_TIMESTAMPS_NO_TSC we need to supply monotonic timer instead of TSC. Are you suggesting that we should use PTSC for timestamps like what we did on stoneyridge?