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 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45059/2/src/lib/bootblock.c File src/lib/bootblock.c:
https://review.coreboot.org/c/coreboot/+/45059/2/src/lib/bootblock.c@28 PS2, Line 28: void bootblock_main_with_timestamp(uint64_t base_timestamp, These bootblock changes should reside in their own patch.
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 123: #if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) Why is this guarded by a macro and not a C if() ?
if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) bootblock_main_with_timestamp(...); else bootblock_main_with_basetime(...);
https://review.coreboot.org/c/coreboot/+/45059/2/src/soc/amd/picasso/bootblo... PS2, Line 125: CONFIG_PSP_SHAREDMEM_BASE; You should cast ints through uintptr_t for establishing pointer values.
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(); This is the tsc frequency since we aren't selecting COLLECT_TIMESTAMPS_NO_TSC. How are we aligning the base times across x86 and psp? I would expect us to normalize on a common clock.