Attention is currently required from: Christian Walter, Werner Zeh, Yu-Ping Wu. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59682 )
Change subject: cbfs: Remove deprecated APIs ......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/59682/comment/cc6f004c_95120a40 PS2, Line 10: new
Sorry I was not following this closely. What is the new API? Providing a CL link would be nice.
I just mean the whole CBFS stack rewrite I've been doing (on and off) for the last two years, replacing the commonlib/cbfs.c code with commonlib/bsd/cbfs_private.c, and replacing all the top-level CBFS APIs with cbfs_load()/cbfs_map() and their variations. Added a few CL links here to clarify.
Patchset:
PS2:
I did one additional comparison. Same code base but now with TPM_MEASURED_BOOT disabled. […]
Thanks for testing Werner! These results are indeed very strange, and I get the feeling that there might be something fundamentally wrong on your platform that is only exacerbated by subtle changes in behavior here. Loading a payload should probably not take over two seconds, unless it is ridiculously huge... how big is the payload you use? And do you happen to know the SPI bus speed for your platform?
The only guess I have what can cause this is that the tpm_measure_region() code would load data in 1KB chunks onto the stack before hashing, while the new flow here runs the hash algorithm directly on the memory-mapped buffer (ironically, I asked Philipp to do that to make it work on Arm platforms back when he added that code, originally he was also just mapping the whole thing and hashing that directly because he was only testing on x86). My (very limited) understanding of the whole x86 flash-mapping thing is that all of this gets magically cached to the point where the access pattern differences shouldn't matter and the memory-mapped regions can basically be treated like (read-only) RAM. I guess that may not be true on your platform. Is it possible that something (e.g. some MTRR thing) is misconfigured to disable this caching?
Anyway, it should be pretty simple to figure out if this is it: I've uploaded a test CL CB:59767 which just loads the whole payload into RAM before doing anything else with it. Can you try that out and see what difference it makes for your boot times? (If that's not it, could you try narrowing down a bit further where exactly all this time is spent for you? E.g. just sprinkle some timestamp_add_now(1234) with increasing ID numbers throughout the code to see where the big delay comes from.)
File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/59682/comment/93ced10c_3c77abe7 PS2, Line 254: CBFS will look for a file in the RO : (COREBOOT) region
This reads a little bit weird to me, as the filesystem doesn't do it by itself. How about […]
Changed to "the CBFS code".
-
Julius Werner (Code Review)