Nathaniel L Desimone has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36328 )
Change subject: [RFC] Documentation/fsp: Discuss FSP-S issues ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/36328/4/Documentation/fsp/fsp-s_dis... File Documentation/fsp/fsp-s_discussion.md:
https://review.coreboot.org/c/coreboot/+/36328/4/Documentation/fsp/fsp-s_dis... PS4, Line 89: * 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.
@Nathaniel Well, why not default to locking an leave the possibility to disable it?
Having a UPD to disable the SAI lock is not something that I will be able to get approved.
As for the "when" part, from a security standpoint the latest it can be done is EnumInitPhaseAfterPciEnumeration. After that, on a UEFI firmware you will start executing 3rd party OpROMs. It would be a security issue if an OpROM had unrestricted write access to SPI.
There is no doubt in my mind that the documentation around this could be better and I will endeavor to help improve that on my side.
https://review.coreboot.org/c/coreboot/+/36328/4/Documentation/fsp/fsp-s_dis... PS4, Line 126: FSP Switches SAI : ----------------
Though, wouldn't you agree that the first FSP-S stage is a little to […]
Keep in mind the wording on this stuff was written by silicon architects. In their minds, anything that happens after MRC is done is "post boot."
https://review.coreboot.org/c/coreboot/+/36328/4/Documentation/fsp/fsp-s_dis... PS4, Line 155: Missing Communication and Responsibility
Many people don't want professional level service but simply run their hardware they bought without any proprietary blobs.
I am not aware of a single modern performance competitive chipset/SoC that does not require software/firmware provided by the silicon vendor in order to function. Once upon a time these blobs were written in ROM fuses, but that has severe consequences. The infamous Pentium FDIV bug could have been resolved by a microcode update, but the original Pentium did not have updatable microcode. Having in-field re-programmability is essential for the modern world. Spectre/Meltdown was able to be mitigated with in-field updates instead of a recall because of Intel's focus on making as much of the SoC behavior updatable through firmware as possible.
All of Intel's competitors use blobs. Even the Raspberry Pi uses blobs. When you buy silicon regardless of vendor, you are buying a bundle that includes hardware, firmware, and software.
It feels like Intel tries to sell a license for using a CPU/SoC instead of selling the hardware itself...
We have zero interest in license revenue for our firmware. The cost of developing the firmware for the SoC is included in the price of the SoC itself.
However, we do not have any obligation to provide some sort of FSP help desk. I try to help as I am able, but keep in mind that I have a regular day job as well working on getting the firmware for our upcoming chips ready to ship. It is unreasonable to expect me to be able to answer every question in my very limited spare time, and management is unwilling to formally assign people to something that won't directly impact chip sales.
https://review.coreboot.org/c/coreboot/+/36328/4/Documentation/fsp/fsp-s_dis... PS4, Line 194: or raise legal concerns
Please let's not mix this up with the removed paragraph that was talking about FSP2.1. All that this sentence says is that "if something would raise legal concerns, it should change". Not targeting anything in particular.
If you focus on just the "legal" part, it would be reasonable to read this sentence as follows:
"However, if parts that are left closed source raise legal concerns, the ABI would have to change to solve aforementioned issues."
The reference to the ABI causing a potential legal concern brings to mind the MP_SERVICES example, that is the only one I can think of that would meet the parameters of this sentence.
Regardless, I would like to respectfully request that we remove it out of an abundance of caution. If the lawyers get involved, I think progress here will become impossible.
Thanks for pointing this out. As mentioned somewhere above, there is a lack of documentation. It's a shame that information like this needs to be "leaked".
Apologize this was not clear. I thought it was mentioned in the integration guide but it appears to not be. Can you file a issue on IntelFsp to remind me to go fix that?