Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35032 )
Change subject: timestamps: Improve collection for ENV_ROMSTAGE_OR_BEFORE ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35032/3/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/c/coreboot/+/35032/3/src/lib/timestamp.c@51 PS3, Line 51: #define TIMESTAMP_CACHE_IN_BSS (ENV_RAMSTAGE || ENV_POSTCAR)
So... […]
In its current state this commit enables the follow-ups that would simplify car.ld, assuming the parent commit for FSP1.0 CAR migration also works (work-in-progress).
I would delay further changes to this file until we have more details for amd/picasso. They may actually want to have timestamps in .bss; declaring TIMESTAMP_REGION in DRAM seems pointless if nothing prior to romstage will produce timestamps and CBMEM may not be available from the start of romstage. I think I read somewhere AGESA would not give out suitable cbmem_top() until PCIe training/enumeration, I just do not have enough details for the romstage in DRAM approach wrt. how it will be linked.
https://review.coreboot.org/c/coreboot/+/35032/3/src/lib/timestamp.c@94 PS3, Line 94: ts_cache = (void *)_timestamp; Can't use car_get_var_ptr() after _timestamp is moved outside _car_relocatable. Currently FSP1.0 calls timestamp_init() after CAR teardown, maybe easiest to start collection from start of romstage (if parent commit with relative pointer works) instead.