Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36664 )
Change subject: x86/tsc: Only call tsc_freq_mhz once to init clock ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36664/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36664/1//COMMIT_MSG@14 PS1, Line 14: communicate with the TPM on a cold boot causing the system to not boot.
I was able verify the culprit cl and the only change in logic was not storing the result of tsc_freq […]
I think we should assume there are callers outside this tsc/delay_tsc.c that would benefit of having tsc_freq_mhz() result cached in .bss. I kind of like cpu/intel/common/fsb.c:get_fsb_tsc() implementation, but that's probably because I wrote it. I would rather see all soc/intel/x/tsc_freq_mhz.c combined the same way.
At the moment I can think of two failure modes: Source having some invalid register clobbers in cpuid() inline assembly as those are called from intel/common/block/timer/timer.c, or cpuid synchronizing instruction side-effects.
Last, IMHO it should be forbidden for tsc_freq_mhz() to ever return 0.