Sorry for being late to answer to my own thread (busy busy busy).
A few notes : The initial check-in of the kabylake FSP was uploaded with a BSD license : https://github.com/IntelFsp/FSP/tree/d88078a708e768c7b6ee5cbc996299d303c3c70... Later commits added Intel's Restricted Use License Agreement (which basically prevents redistribution (and reverse engineering)). That's also why the header commit I submitted didn't update the headers to match the Gold release, but only what was in the initial 'BSD' release, which thankfully is compatible with the Gold release (besides, most of the changes, other than the license header, were to add typos and remove fields, since the header in coreboot is technically 'newer').
Regarding Nico's idea of signing blobs, for old unmaintained blobs, they could still be signed with a coreboot specific "this is a known working, not guaranteed to be malware free but guaranteed to be unmodified for years, or unmodified since its official release" key, and a coreboot maintainer would have the private key to sign old blobs with that key. That would at least resolve the condition of "all blobs must be signed".
I do agree with Julius that headers should remain in coreboot, and not move to the blobs.git, and I agree that removing old boards doesn't make sense. Yes, I think coreboot is meant for enthusiasts and tinkerers who want to build an image for their boards, and that's why I'm suggesting to make the headers match the github version, because it's now impossible to build anything with FSP (whether a google board, or a Purism board for example) without first modifying coreboot because the only FSP available to enthusiasts and tinkerers is the github one.
In general, I like Nico's idea of setting up rules for blobs, but my worry is that no matter how great and logical the rules are, the blob-makers might simply ignore them.. you can ask for signed blobs, but what if they refuse to sign it? Or even better, you can ask for redistributable blobs, but what if they refuse to make them redistributable? Will you simply stop supporting any boards that depend on those blobs? You'd have to eventually compromise (for the sake of the users/project) and break your own rules that you worked so hard to perfect... Since they control the blobs, they have the power, unless you think "integrate in coreboot" is a hard requirement for them, and in that case, if you set up those rules, they would be forced to follow them, that can actually help get things done the right way instead of just asking nicely and never getting a positive response. I don't really know... it might work for Intel/CrOS for example, but maybe not for other vendors who wouldn't need to be integrated in coreboot...
Anyway, to resolve the immediate issue with the header, how about adding a 'variants' system, like we already have with mainboards? Something like : src/vendorcode/intel/fsp/fsp2_0/variants/google/*.h src/vendorcode/intel/fsp/fsp2_0/variants/github/*.h and in the Kconfig, we could just set FSP_VARIANT_DIR="github" That would be (I assume) much less messy than having #ifdefs to define-or-not specific fields in the UPD structures. The only issue would be with regards to fields that coreboot uses that aren't available in a variant, like for example, the PcieRpClkSrcNumber field that doesn't exist in the github header but is used in coreboot, there are a couple of solutions to that : 1 - add #define HAVE_PCIE_RP_CLK_SRC_NUMBER (for example) in the headers and have #ifdefs everywhere in coreboot code. This is not a great idea 2 - have something like "#define PcieRpClkSrcNumber UnusedUpdSpace26", but that would add so much problems that it's a very stupid idea... why am I ever writing this? 3 - manually add the missing fields somewhere in the Upd and only have the changes between two variants where the field offsets change in the structure. 4 - anyone else with better ideas?
Using solution 3 above, The 'google' variant would be the current header, and the 'github' variant would be the header after my patch [1] applied to it (so it's a mix between the google and the real github header, but with field offsets matching what the github blob would return), all the fields are still there, I think there are 3 fields that coreboot uses which aren't in the official header, and they just end up in unused space, but at least the MemInfoHob will be correct.
What do you think ? Youness.
On Wed, May 9, 2018 at 3:49 AM, Nico Huber nico.h@gmx.de wrote:
On 09.05.2018 01:04, Nico Huber wrote:
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.I think this is counter-productive, as is removing any old boards that don't comply. I am okay with creating new rules for future platforms, but there is no reason to throw perfectly good and working boards out just because they weren't written to comply with something we only just made up. You wouldn't really be punishing anyone with that (vendors don't care about outdated chips anyway), you'd just be taking choice away from our users.
Well, that's why I brought older Intel based CrOS devices up. I think they are the only ones that currently can't comply. I agree that we should have an exception for older platforms. But it's not really something "just made up", IMHO. Google made their own rules when bringing blobs in with Sandy Bridge, just to break them later. So the problem was obviously already visible when these platforms were added.
I need to clarify this. There were no written rules, AFAIR. What I meant was that Google broke with a practice they established themselves. So they should have noticed the conflict, IMHO.
I'll stop yelling about the past. Let's focus on some rules for the future.
Nico