On 11.05.2018 21:18, Youness Alaoui wrote:
I feel like this discussion is getting slightly out of hand, so let's try to regroup a bit and move the discussion back to the original topic : how to handle the FSP headers in coreboot. I fully understand and agree with Nico's frustration about the blobs situation and I think that's a bigger conversation than needed right now. I do encourage discussing it and finding a good solution to avoid having a similar mess in the future (or now, but in a different thread to avoid going off-topic). I think Nico's idea of setting up rules for how/what to merge in the future is a good idea and needs to be further discussed until everyone is happy with the rules defined.
Yep, sorry. I'll try again in another thread, I guess.
Now, back to the discussion about the "incompatible FSP headers", here are the possibilities :
- should we merge my patch that makes the FSP header compatible with
the public blob, and let the CrOs coreboot clone have its own commit that reverts it,
I have to admit, I don't like your patch. While it gets the job done, it brings `MemInfoHob.h` and `FspsUpd.h` out of sync, so the state in coreboot as a whole would match neither version.
- or abandon my patch (which means I can never send a working
board-status for the librems without having a dirty tree or a version commit hash that doesn't match upstream)
- or have the possibility to choose between the two (either via
#ifdefs or via a variants method).
If we can't get a new, fitting binary out of Intel, I would prefer this, or, more bluntly, just copy the GitHub version and revert the changes that need the additional UPD.
In other words, whatever happens, I think we should have the headers of the latest public release in the tree.
- or, if Intel people are reading this right now (or someone here can
poke them directly), have the public release updated so this matter can be dropped entirely (the public update would need to be released *very* soon though).
Even if you (Purism) poke them about it, that might help. But I would like to see that Google does that too (IMHO, they profit most from the mess). Any of you should have more leverage than the customer of a customer of a customer of a potential customer of Intel has, that I work for ;)
Should we put this to a vote now, or should we discuss the possibilities/alternatives some more, if anyone has ideas on how to tackle this specific issue ?
Well, my vote, in order of preference:
1. Poke Intel. 2. Get a verbatim copy of the GitHub headers in (in a way of effort for the community). Maybe in a month from now? no matter the outcome from 1.
Nico
On 18.05.2018 20:59, Nico Huber wrote:
Well, my vote, in order of preference:
- Poke Intel.
- Get a verbatim copy of the GitHub headers in (in a way of effort for
* least effort
the community). Maybe in a month from now? no matter the outcome from 1.
Nico
On Fri, May 18, 2018 at 2:59 PM, Nico Huber nico.h@gmx.de wrote:
I have to admit, I don't like your patch. While it gets the job done, it brings `MemInfoHob.h` and `FspsUpd.h` out of sync, so the state in coreboot as a whole would match neither version.
Good point. It is a Frankenstein, but it was either that or have #ifdefs in the fsp vendorcode itself to determine if attribute X is available or not, etc..
- or abandon my patch (which means I can never send a working
board-status for the librems without having a dirty tree or a version commit hash that doesn't match upstream)
- or have the possibility to choose between the two (either via
#ifdefs or via a variants method).
If we can't get a new, fitting binary out of Intel, I would prefer this, or, more bluntly, just copy the GitHub version and revert the changes that need the additional UPD.
In other words, whatever happens, I think we should have the headers of the latest public release in the tree.
I agree, that's probably the cleanest solution, but I didn't suggest it because I figured people would be yelling about removing functionality that might need to be re-added eventually if Intel end up doing an updated release.
- or, if Intel people are reading this right now (or someone here can
poke them directly), have the public release updated so this matter can be dropped entirely (the public update would need to be released *very* soon though).
Even if you (Purism) poke them about it, that might help. But I would like to see that Google does that too (IMHO, they profit most from the mess). Any of you should have more leverage than the customer of a customer of a customer of a potential customer of Intel has, that I work for ;)
I know no-one at Intel to poke them about. I'll ask if Todd has contacts that might be able to help. I'm hoping that some of the people that are working for Intel are following this email thread but if they are, they haven't raised their hands. Anyone knows or can suggest the name of someone that we might poke ? I'm guessing those working for Google who actually received the newer FSP from Intel might know who to ask, and since they are the ones that would be affected by their code/features being removed if we revert to github version of header, it might help put some pressure on this issue (so far, I haven't seen any incentive to make them do that).
Should we put this to a vote now, or should we discuss the possibilities/alternatives some more, if anyone has ideas on how to tackle this specific issue ?
Well, my vote, in order of preference:
- Poke Intel.
- Get a verbatim copy of the GitHub headers in (in a way of [least] effort for the community). Maybe in a month from now? no matter the outcome from 1.
Nico
I agree, and I give my 100% vote for that as well. It will be much better than an #ifdef mess and would be a good incentive for the people from Intel not saying "no, too much hassle to update the file, we got nothing to lose anyway if we just ignore this". So let's see who can poke who from Intel, let's give a deadline (1 or 2 months? if bureaucracy means it will take longer, we might revisit if we at least got a confirmation that the issue is being looked at on Intel's side), then once deadline is reached, we revert to public headers and remove functionality that prevents building (access to nonexistent fields in UPD) and in the future, reject patches on gerrit for non public blobs.
Thanks!