Martin Roth 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 9:
(4 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 132: CONFIG_PSP_SHAREDMEM_BASE
Changed to _transfer_buffer. […]
It's not really a big deal, since one is based on the other, but it's probably a good idea to use the symbol from the linker table instead of the Kconfig option. Maybe this is just a personal preference.
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 *)
_transfer_buffer is defined in src/soc/amd/picasso/memlayout_x86.ld and guarded with CONFIG(VBOOT). […]
Yeah, I think we do need to worry about when vboot isn't used. Not for chromeos, but typically the chromeos platforms will also be ported to a non-chromeos firmware version. Check out https://mrchromebox.tech/ to see the ports.
Maybe put the struct and all the code inside the if into a separate function, then run the old bootblock main in this func if that one didn't set the timestamps? In there, you can just check if verstage is enabled and starts before bootblock and return immediately if it isn't.
https://review.coreboot.org/c/coreboot/+/45059/6/src/soc/amd/picasso/bootblo... PS6, Line 138: (uint64_t)info->timestamp
I stated that in line 131.
Ack
https://review.coreboot.org/c/coreboot/+/45059/6/src/soc/amd/picasso/bootblo... PS6, Line 157: * Since bootblock_main_with_timestamp calls
What flag do you mean?
Rob, are you saying to put the code to adjust all the timestamps based on the base time into bootblock_main_with_timestamp, then pass in a flag to do that?