Hello Aaron Durbin, Patrick Rudolph, Arthur Heymans, Michael Niewöhner, Duncan Laurie, David Wu, Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/36328
to review the following change.
Change subject: [RFC, scratch] Documentation/fsp: Start discussion around FSP-S issues ......................................................................
[RFC, scratch] Documentation/fsp: Start discussion around FSP-S issues
To get some discussion started, list some heavy issues with FSP(-S). I've copy pasted some things together, and added a few words on top and bottom. Not sure yet, where this should lead. To convince Intel to contribute open-source code on their own, we might want to add a list of initialization steps, that should be open.
Feel free to amend this commit in any way :)
Change-Id: Ied0d8ad60fa487272a05fc73b16a7c343cc4d993 Signed-off-by: Nico Huber nico.huber@secunet.com --- A Documentation/fsp/fsp-s_discussion.md 1 file changed, 165 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/28/36328/1
diff --git a/Documentation/fsp/fsp-s_discussion.md b/Documentation/fsp/fsp-s_discussion.md new file mode 100644 index 0000000..7ac649a --- /dev/null +++ b/Documentation/fsp/fsp-s_discussion.md @@ -0,0 +1,165 @@ +Introduction +============ + +*FSP* is an ABI that allows silicon vendors to provide pre-compiled +binaries of their silicon-initialization code for the integration +into firmware frameworks such as coreboot. Current, 2.x versions +of *FSP* describe three binaries with rough responsibilities: + +* `FSP-T`: Sets up a temporary RAM environment (usually some sort + of Cache as RAM, CAR) among other things. + +* `FSP-M`: Initializes the memory (DRAM) controller, among other + things. + +* `FSP-S`: Performs various silicon-initialization steps left. + +However, what operations these binaries perform exactly and what +operations are left to the firmware framework, is neither specified +nor documented. + +Historically, all these responsibilities were left to coreboot. +Later, pre-compiled reference code for the memory-controller +initialization was introduced, then for other silicon init, and +finally there are attempts to replace coreboot's CAR setup with +FSP-T. + +While the FSP-M integration saves coreboot developers time, because +of the complex nature of the memory-controller initialization, the +integration of FSP-S turns out to be harder than implementing the +necessary code natively in coreboot. We'll discuss why that is the +case and provide an incomplete list of issues with current FSP-S +binaries. + + +Bug Fixing +---------- + +Many companies rely on the pre-built FSP binaries. Yet, there is +no common procedure to get bugs fixed or even acknowledged. This +leads to an increase of workarounds in firmware frameworks, +especially for older hardware generations. + + +Lack of Documentation +--------------------- + +Many of the initialization steps performed by FSP-S are merely +translating mainboard-specific configuration into register settings. +This is reflected by a list of hundreds of configuration options of +FSP-S. While these steps are quite simple, the additional translation +from FSP options to register values heavily increases the integration +costs. Not only are the register settings usually much better documented +than their FSP counterparts, but FSP also uses different terms so that +the existing hardware documentation often can't assist developers when +dealing with FSP. + +FSP-S also performs locking of several registers so that they can't +be written anymore afterwards. It is not documented what locks are +set by FSP. This creates heavy security issues for multiple reasons: + +* One either has to assume that FSP doesn't perform any locking + and do it redundantly in the firmware framework, or expensively + test what is locked and keep doing so for every new FSP revision. + +* Some registers may be locked before they are configured in a + secure manner. This may result in workarounds in the firmware + framework to configure things early. Sometimes it's even + impossible to do so, because relevant information isn't + available yet. + +This shows that no matter the many options that are implemented in +FSP-S, there will always be things that must be controlled by the +firmware framework. Locking should either be a separate, last step +in FSP, or be left entirely to the firmware framework. And most of +all, the locking has to be documented. + + +Arbitrary Interface Changes +--------------------------- + +Within coreboot, we try to keep code-compatibility with older hardware +generations. The FSP ABI isn't stable, however. Even when the hardware +doesn't change between minor platform updates, the configuration +settings of FSP are sometimes adapted without backwards compatibility +in mind. This increases costs, just to translate configuration data +from one representation to another only to let FSP translate it again. +In such cases, directly configuring registers from coreboot, would +save two layers of indirection. + +Another problem is the disappearance of options in newer FSP releases. +See [SpiFlashCfgLockDown UPD vanished after KBL] +(https://github.com/IntelFsp/FSP/issues/25), for instance. Current +coreboot code seems to still rely on this option. It boots, though, +only security assumptions are broken. Which again shows, locking +shouldn't be part of FSP, or at least optional. + + +FSP Switches SAI +---------------- + +A problem that multiplies with the lack of documentation is that FSP-S +switches to `POSTBOOT_SAI` on newer platforms. While it probably tries +to achieve the opposite, this undermines firmware quality and security. + +* In some firmware frameworks FSP may run long before End-of-Post, this + means most of the framework runs with `POSTBOOT_SAI`. + +* In the EDS, implications of the SAI switch are _not_ documented. + Virtually for every register, it is unknown if it is still accessible + after the SAI switch. This seems to be a problem for developers both + inside and outside of Intel. + +* The SAI switch seems to force the firmware framework to do some things + in SMM. However, the use of SMM as a more privileged execution mode is + generally discouraged. This means FSP forces firmware development to + go backwards. + +Especially the lack of documentation leads to many bugs in firmware. +Even Intel developers working on coreboot don't seem to know the +implications of the SAI switch or lack the resources to re-evaluate +all code wrt. the SAI switch. + + +FSP Creates Legal Uncertainty +----------------------------- + +Many companies sell coreboot together with FSP as part of their products. +Especially, the function pointers to GPL code that were introduced with +FSP 2.1 seem unprecedented. However, they could be compared with runtime +linking which seems troubling. + + +Mitigations +----------- + +None of the listed issues would exist if FSP were open-source and the +initialization steps could be integrated into coreboot, like they were +before. An open-source approach would not only cut costs heavily but +might also decrease time-to-market if the whole coreboot community +could work together on new platforms. + +However, in the absence of such a firmware-development utopia, there +may be more realistic approaches. As mentioned earlier, not all of FSP +has a negative impact on development performance. FSP-M, for instance, +rarely causes trouble. And due to its complex nature, it is no surprise +if memory-controller initialization is kept secret. The configuration +done by FSP-S, on the other hand, overlaps with many of coreboot's +responsibilities. The few lines of code in FSP-S that help to get a +mainboard running, don't outweigh the damage it does. However, one +can't have the cherries without taking the whole menu, which takes +years to digest. + +One approach could be to split FSP-S further up and identify parts +that are necessarily closed source and parts that could as well be +reimplemented in coreboot. However, if parts that are left closed +source change the state of the execution environment (e.g. switch +SAI), finalize any settings or raise legal concerns, the ABI would +have to change to solve all aforementioned issues. + +Another approach could be to align older platforms with open-source +coreboot, step by step. Currently, everything from Skylake on relies +on FSP-S. However, it seems Intel has already abandoned development +for this platform (while the coreboot integration was never finished). +It would seem reasonable to release abandoned code to the public, or +at least clear existing documentation for older platforms from NDAs.