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...
Well, you can't argue much about what will happen if you set up some rules if you never tried. Sure, the rules could fail us. But is it not worth trying? I'm seeing a lot of bad things on Gerrit where the only argument seems to be that ppl were allowed to do the same with other platforms (just because a single reviewer was lax?). Having some written rules that you can refer to could help a lot, IMHO.
I know I might be generalizing too much about what Intel does. But, AFAICS, for all non-Intel platforms, ppl seem to have done reasonably well enough so that the platforms can already comply with my draft rules. Which also shows that it can't be too hard.
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?
As I can't propose to you to make the homework for multi-billion corporations, plus the problem wouldn't exist if I hadn't been run over on Gerrit:
4. Ditch the code that references the missing UPDs.
Nico
-
Nico Huber