Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36328 )
Change subject: [RFC, scratch] Documentation/fsp: Start discussion around FSP-S issues ......................................................................
Patch Set 1:
(7 comments)
I spilled some beans of mine, because I felt like it. I am probably mistaken, so don't resist the urge to correct my statements :D
https://review.coreboot.org/c/coreboot/+/36328/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36328/1//COMMIT_MSG@15 PS1, Line 15: Feel free to amend this commit in any way :)
I'm very tempted to talk badly about FSP-T integration since it became optional since 2. […]
I guess a "fsp-t_discussion.md" can also exist! :D
https://review.coreboot.org/c/coreboot/+/36328/1/Documentation/fsp/fsp-s_dis... File Documentation/fsp/fsp-s_discussion.md:
https://review.coreboot.org/c/coreboot/+/36328/1/Documentation/fsp/fsp-s_dis... PS1, Line 42: I wrote this:
The coreboot community uses these pre-built FSP binaries. Several FSP bugs have been found and reported, but they hardly ever get addressed. This means that the community has to awkwardly work around these issues and hope that the bug is fixed in a new FSP release, which may never happen if the platform is too old.
https://review.coreboot.org/c/coreboot/+/36328/1/Documentation/fsp/fsp-s_dis... PS1, Line 56: On at least Skylake, configuring the bus/device/function for the HPET or the IOAPIC (related to VT-d) is a single write to a single register on the PCI config space of the P2SB. However, FSP-M has *four* configuration parameters for the HPET, and FSP-S has another four parameters for the IOAPIC. Just why.
https://review.coreboot.org/c/coreboot/+/36328/1/Documentation/fsp/fsp-s_dis... PS1, Line 102: POSTBOOT_SAI
Please explain briefly what this does/tries to do.
I can talk about it.
Newer Intel chips have an interconnect system called IOSF (Intel On-chip System Interconnect). This is a rather complex architecture. I think it has three "levels" of interconnects: primary (where the gross volume of traffic goes through), secondary (related to the management engine and the P2SB), and tertiary (related to hardware debug capabilities).
The primary level is the most public one. It's what appears on block diagrams, connecting pretty much all the devices. This level seems to belong to the x86 cores.
The secondary/sideband level is wired to the Management Engine as well as other devices, and the P2SB is a gate from primary to sideband. This seems to be the Management Engine's own communication channel, for hell knows what purposes.
AFAIK, Security Attributes of Initiator are some bits that specify the privilege level of a certain "initiator". The one we care about is the host (x86 cores). the default setting is rather privileged, and is needed to do firmware things (init devices). Once this is done, the SAI privilege level should be dropped as a protective measure. This would be the post-boot setting, known as POSTBOOT_SAI.
At least on CoffeeLake, FSP-S sets that when it has finished running. This restricts later code's access to many devices (e.g., the damned P2SB) and forces the use of SMM because, of course, SMM is a special snowflake and is trusted with a higher privilege than non-SMM.
https://review.coreboot.org/c/coreboot/+/36328/1/Documentation/fsp/fsp-s_dis... PS1, Line 108: In the EDS, implications of the SAI switch are _not_ documented. maybe one needs to go deeper when recursively NDA-wrapping oneself? \s
https://review.coreboot.org/c/coreboot/+/36328/1/Documentation/fsp/fsp-s_dis... PS1, Line 128: function pointers
Could you give some examples (mainly console + AP PPI, I guess) here?
mrc.bin also has a function pointer to do console output. I don't really mind a single console callback, but the PPI thing is much more complex...
https://review.coreboot.org/c/coreboot/+/36328/1/Documentation/fsp/fsp-s_dis... PS1, Line 165: at least clear existing documentation for older platforms from NDAs.
I'd add "missing communication & responsibility" […]
Last bullet point: sometimes there's friction when maintenance is done :S