On 06.05.2018 00:03, Aaron Durbin wrote:
On Sat, May 5, 2018 at 5:36 AM, Nico Huber nico.h@gmx.de wrote:
On 04.05.2018 23:41, Aaron Durbin via coreboot wrote:
On Fri, May 4, 2018 at 3:20 PM, Youness Alaoui kakaroto@kakaroto.homelinux.net wrote:
Hi,
I've just pushed a commit for review on gerrit (https://review.coreboot.org/#/c/coreboot/+/26108/) and I'm hoping to open the discussion here on whether the public coreboot code should have the FSP headers that match the public FSP headers or if they should match the 'google fsp' headers.
I tried myself before [1] and ran against a wall of lies and broken promises.
Roughly:
- July to September/October: I warned Intel about the situation over all available channels (Gerrit, different corporate support channels, wrote to Vincent who uploaded the first Kabylake FSP to GitHub, several laments about the situation on this mailing list I guess).
- After the first Kaby Lake FSP drop on GitHub, Intel commented on Gerrit that the headers there are outdated and internal ones have to be used.
- December 11th, Intel commented on Gerrit that everything is sorted out.
- December 13th, Intel painted Kabylake FSP "Gold" with 6 months outdated UPDs.
My understanding of the situation is that chromebooks ship with a google-modified FSP image and the fsp headers in coreboot have been added by google employees even before the FSP was officially made public by Intel. I also somewhat understood that the fsp headers in coreboot would not be updated to match the public release because it would break everyone building coreboot for their chromebooks or something like that.
They aren't modified necessarily. It's more of different views on the same timeline of a development. Granted, the releases are definitely not coordinated. I'm not even sure how/who releases the github blobs/headers on Intel's side.
I have more a feeling that different branches exist.
I'm a bit tired of always having that commit that fixes the header applied locally in all my branches, and I think that it makes much more sense for coreboot to have the fsp headers that match the public release and for the google/chromebook coreboot repository to have the "non-standard header" committed to it instead. Worst case scenario, we could add #ifdefs to determine what the structure contents should be depending on the target platform.
Maybe everyone will say "of course, that makes sense" or maybe everyone will say "nope, I disagree", but hopefully we can have the discussion here (or on gerrit) and decide how to handle this use case. Note: I only pushed for skylake/kabylake, but apollolake/canonlake are probably also affected by this mismatch of headers.
I think we should have both that can be selected through a Kconfig such that we can bind to the headers correctly. We already had to do that for edk2 issues as well. I can't think of better alternative at the moment that supports both.
You can not just switch headers. As Youness pointed out in his commit, UPDs used by coreboot are missing in the public release. We'd clutter up the tree with #ifdefs. And once that started, I'm sure, somebody will use that as excuse to practice it further. OTOH, we could just revert the past 10 months of soc/intel/. The major stakeholders had been warned early enough. If somebody gets that through Jenkins, I'll +2.
The technical challenges of managing different sets of headers doesn't seem too hard. Things can be isolated fairly easily w.r.t. to the field mismatches. Code can be co-located to handle such scenarios and keep it isolated.
And who is going to do the work? And who can even tell which sets of header files should be supported by upstream coreboot? Of course we can just fix this instance of the problem and wait for it to happen again...
For Youness' request, the Kaby Lake FSP headers, who can tell if Intel isn't going to release another public version with coreboot-matching headers one day after we finished incorporating the old version? I don't think we can solve this by running around and fixing issues that should not exist. We should figure out how to prevent such issues, first.
Um, a little more serious: I believe the only solution is to enforce some sane rules about code changes that depend on blob headers and blobbed platforms in general, e.g. (just a draft; but I wouldn't mind if we take it verbatim):
(meaning of *shall*, *should* and *recommended* as specified in [2], "changes" refer to new commits for the coreboot master branch)
- Generally:
All platforms should be blob-free. All blobs should be redistributable. Providers of blobs should sign them and take responsibility that the signed blobs were unaltered after compilation (e.g. do not contain malware). It is *recommended* that the public key needed to verify the signature is obtainable through a specific channel (e.g. single responsible contact person) _independently_ from the blob's distribution path.
After the coreboot 4.8 release: All changes to a binary-specific header file (including the addition of new files) *shall* be refused, unless a matching binary is publicly available.
For each platform that relies on a blob, the coreboot tree *should* point (e.g. git submodule) to a commit containing both the header files and the matching binary.
After the coreboot 4.9 release: For each platform that relies on a blob, the coreboot tree *shall* point (e.g. git submodule) to a commit containing both the header files and the matching binary. If the plat- form code within coreboot supports multiple variants of that platform, that commit shall contain all necessary binaries for all variants supported by coreboot, all compatible with the same header files.
Unless a pointer as described above exists for a given plat- form that relies on a blob, all changes* to that platform *shall* be refused.
All platforms that rely on a blob that is not redistribu- table and whose distribution as an individual file has been stopped or has never happened *shall* be immediately removed from the master branch.
*excluding changes to maintain tree-wide compatibility
After the coreboot 4.10 release: Unless a pointer as described above exists for a given plat- form that relies on a blob, that platform *shall* be imme- diately removed from the master branch.
Unless a signature as described above exists for a given blob, all changes* to platforms that have been launched to market and rely on that blob *shall* be refused.
Amendment: "for a given blob" that was released after coreboot 4.10
This should give everybody enough time to adapt and the refusal to merge new patches should constantly remind them so they won't forget the matter and panic right before a release.
One reason for not writing this up earlier is that it potentially hits a lot of CrOS devices. Though, in the proposed scheme there'd still be 6 months for Google to figure things out.
It's just not Google who can sign up for such things. There are legal agreements about some of the points above that may make it completely infeasible to meet. As you pointed out there is 6 months, but contracts would have to be renegotiated.
I assume we are talking about the CrOS devices with mrc.bin (Haswell, Broadwell, BayTrail)? I know the situation must be awkward for those. OTOH, what's the benefit of maintaining them in the master branch if nobody else can do a port with the platform code?
NB: Funny legal interpretation, IANAL: If I wanted to make a product based on Broadwell with upstream coreboot, it would be illegal here in Germany _without_ reverse engineering the blobs first (unless I can get the sources for the blob in a legal way and get them to com- pile, which I estimate would be more work).
I'm curious how you want things to play out from above. Could you provide how you think things will look over time?
I think mostly the process of adding and developing new platforms would have to change. Instead of pushing useless headers to coreboot.git, one would have to push headers+binaries to another repo instead (can be anywhere as long as it's public) and change a pointer in coreboot.git.
Some older unmaintained platforms would vanish, I guess. Though, I doubt there is much to do to fix them (beside CrOS devices where no public blobs exist). To save an older platform, headers and blobs would have to move to a common location. I have to admit I didn't think the demanding of signed blobs through. There would have to be an exception for older unmaintained blobs.
This isn't just impacting recent and new Intel platforms. There are other vendors beside Intel.
I was told often that we have the blobs from other vendors in our 3rdparty/blobs repo. It's just the headers that would have to move.
I'm trying to understand what platforms would be supported in practice on coreboot.
All of them, I hope. AFAICS, public binaries already exist for all but the Intel+CrOS combination. Why should that be so hard to change? Everything else seems to be either redistributable, already in our 3rdparty/blobs repo (so I was told) or (in case of FSP) sits in a GitHub repo where commits with headers+binaries already exist.
Nico
[1] https://review.coreboot.org/#/c/coreboot/+/20477/ [2] http://www.ietf.org/rfc/rfc2119.txt