Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43741 )
Change subject: Enable long cr50 ready pulses for Tigerlake systems ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/43741/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43741/2//COMMIT_MSG@15 PS2, Line 15: to FSP. For Volteer (and future Tigerlake boards) we enable mode S0i3.4
all this passing around is only necessary because trying to initialize the Cr50 SPI before FSP-S l […]
The very first proposal, was to add the functionality without adding one more point at which the AP firmware pulls the Cr50 version string, but once I realized that it was not straightforward to pass data from verstage to ramstage, I as a fallback tried adding code to read the version also in early ramstage.
When that turned out to trigger issues which we still understand poorly, that made me want to spend a little more time working on the ideal solution, which adds the minimum amount of boot time, rather than spending the same or more time understanding the FSP issue, allowing me to finish the slower solution.
https://review.coreboot.org/c/coreboot/+/43741/2/src/security/vboot/cr50.c File src/security/vboot/cr50.c:
https://review.coreboot.org/c/coreboot/+/43741/2/src/security/vboot/cr50.c@2... PS2, Line 26: return;
Is all the version checking really necessary, btw? What happens if you try to read the board_cfg reg […]
I asked Namyoon this question, and he told me that prior versions of the cr50 firmware will give random uninitialized data, if trying to read a register it does not recognize. (Anyone remember heartbleed.)
https://review.coreboot.org/c/coreboot/+/43741/2/src/security/vboot/vboot_lo... File src/security/vboot/vboot_logic.c:
https://review.coreboot.org/c/coreboot/+/43741/2/src/security/vboot/vboot_lo... PS2, Line 304: set_cr50_board_cfg();
This has nothing to do with vboot, please don't put this here. […]
This is a good point, I will work on that...