[coreboot] FSP 2.0 headers in coreboot

[Youness Alaoui]

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/d88078a708e768c7b6ee5cbc996299d303c3c702/KabylakeFspBinPkg
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 at 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