Despite my frustration here, I’m optimistic that this discussion can serve to close out the thread “Copy-first platform additions (was: Re: Re: Proposal to add teeth to our Gerrit guidelines)” https://mail.coreboot.org/hyperkitty/list/coreboot@coreboot.org/thread/JHXCP... which I felt went somewhat unanswered. There were good arguments on both sides of that, many that I agree with, but I don’t think we came away with any solid policies for future development.
Before addressing the main topic, I think a little context of how corporate contributors must develop and add functionality is important.
To paraphrase one very large coreboot consumer, “The fastest time any company has ever demonstrated development from scratch to ship-worthy was eighteen months”. Pick your favorite fraction of this for when code is therefore push-worthy (9 months? The full 18 months?), and consider how much you’ve personally observed the project evolve in that same length of time. I’m sure you can foresee the amount of rebasing/rewriting that would be required. Many years ago, I observed an Asian team attempting to complete 100% of their development internally. When it was time to push, they were so out of date it was a disaster, requiring additional work for both developers and reviewers to point out what was newly wrong.
Predating the discussion of the community preference for development/contribution practices, we had already chosen the path of copy/modify approach for stoneyridge-to-picasso. Despite any detractors’ opinions on the code itself, stoneyridge was (1) the sole AMD product supporting modern coreboot features and directory structure, (2) extensively tested and verified. Therefore, the first order of business was moving as much stoneyridge functionality into a common directory as I was comfortable with.
And yes, there is currently board design and bringup, power measurements, etc. all being conducted on an internal fork of the work-in-progress stack out of the coreboot repo. (You’ll forgive me if I don’t discuss specifics…) I assure you there are many more stakeholders than are visible to the coreboot community. The reality of corporate contributors, is they essentially must seek revenue either by working on a specific product or seeding a roadmap; either pursuing near-term revenue or laying the groundwork for the more distant future. Either way, they must have a reason to believe there will be some reward for the heavy development expenses they shoulder up front. AMD is not unique in this regard. In my judgement of the timeline, i.e. as an outsider looking in at that time, AMD began picasso development much later than any of us would have wanted.
That brings me back to the patch submit effort that would be required if there’s not an acceptable middle ground. Ballparking the number of picasso patches that have already landed, plus what’s on the fork, we could easily be looking at 250-300 individual patches or possibly more. Even if properly sized for easy review, that seems to me to be too many to dump on the community at once. And you can imagine what happens if a revision is requested on the earliest one, i.e. how many others might need to be fixed up, and the unnoticed errors that could result from that process.
I am also sympathetic to the idea that the work-in-progress has not been updated as rapidly as it should have been. YOU”RE RIGHT. I agree, and I’m frustrated too. This has simply been a problem of resource constraints. My responsibility; I’m not blaming anybody. The stakeholders mentioned above simply must be considered too, and it frustrates me that I can’t tip that hand for community people also looking for more rapid changes to picasso.
Let me respond this way, though. Before being asked to return to AMD, I asked very direct questions to my future management trying to gauge how serious AMD _really_ is about being a top-notch contributor and member of the coreboot community, regarding how big do we expect to grow our coreboot team(s), about how they see me as pivotal to growing the team. I don’t need to tell you that AMD has been in a place of “not getting it” for a very long time. Believe me, it would’ve been very easy and very lucrative not returning to AMD but I went back with the personal goal of turning them into a first-class contributor to coreboot. Sure, ambitious. And to that end, surely you understand the time and effort it requires to either hire or create talented coreboot engineers.
I’ll also add that I sense many people are still sore over many of AMD’s sins of the past, especially of creating complete new duplicated directories and then maintaining them. Not unjustified…[*]
Apologies for such a long screed before addressing what got us here, “soc/amd/picasso: Drop forked copy of SMBus source“. I respectfully request that features not be stripped whole-cloth from an soc port actively in development. I fully acknowledge there is no build testing in place, which applies not only to this one feature but the entire amd/picasso directory itself. (Hopefully nobody would seriously advocate for removing the entire picasso directory.) Instead, please give me the opportunity to review any of your changes that touch the picasso directory. If I have any concerns of building, I can test and run it locally, recommend solutions, etc. We can expect a few problems to slip through that process, which I will immediately discover prior to repushing my subsequent work. I will fix it, and allow you to review that work. I am also committed to converting anything resembling a fork to amd/common where it makes sense, and when the development priorities allow. The instant that mb/amd/mandolin lands, it will no longer be work-in-progress and it will undoubtedly build successfully.
As is frequently the case, there is substantial effort, resources and money invested, negotiations, and so on, that the coreboot community would never be able to see. I’m not sure how to best convey to you that AMD is serious about making picasso and subsequent “Zen” processors very viable options for running coreboot. I’ll mention that several of us have already purchased picasso-based laptops, however, with the intent of eventually adding mainboard ports for them. On the outside chance no picasso-coreboot product ever ships, I’ll definitely accept deleting the picasso directory out of the source.
Ironically, I could have commonized the SMBus feature several times over within the time we’ve had this discussion. I feel this is an important topic, however, so thank you for indulging me. Although I still won’t reprioritize this work ahead of what I promised to my stakeholders, I believe the time discussing this was valuable.
Finally a word about blobs. Not that I think it belongs in the context of this discussion, but it seems to have been a source of bias against picasso. This is a long-standing complaint, and for legitimate reasons. I also get a strong sense that the industry (and I’m thinking of OCP in particular) is coming to accept blobs as a necessary downside to using modern x86 processors, but that blobs must be redistributable (and Nico, no ideas either on Intel’s redistribution policies or expectations). I have no knowledge on how discussions within AMD stack up against discussions within Intel, of course. I realize many of you are aware of past broken promises to reopen AGESA, the reverse engineering research done on the PSP, and many other things. However, there are very powerful and influential people in AMD who want both AGESA and PSP open sourced sooner vs. later. If we see the day come when this begins to happen, an inherently slow process will ensue. I think too many people assume source can simply be dumped onto github, but they don’t understand the extensive legal and other processes required leading up to that point.
[*] In my opinion, the entire project and its standards & expectations have matured since those days. And, surely I don’t need to remind anybody of AMD’s financial situation in the not-so-distant past. By “financial situation”, I especially mean the ability to fund development and support efforts. AMD was all but broke, so there was simply no chance they were going to fund the upkeep of those older solutions. And regarding comments about Sage, a preferred AMD partner at the time, the main reason we saw any sanctioned work in the project was due to Sage’s attempting to remain profitable while doing so. Sage also worked on both AMD and Intel products, and supplying to coreboot consumers (names that might surprise you), and yet we saw that profitability ultimately was a real problem.
Thanks, Marshall
On Mon, Jan 27, 2020 at 11:56 AM Nico Huber nico.h@gmx.de wrote:
Hi Jonathan,
thanks for your email. It's become very rare that developers take part in mailing-list discussions when they are asked to. So it's really appreciated.
On 27.01.20 17:21, Jonathan Zhang (Infra) wrote:
On 1/26/20, 11:32 AM, "Nico Huber" nico.h@gmx.de wrote:
Hi David,On 26.01.20 20:15, David Hendricks wrote:
On Sat, Jan 25, 2020 at 4:44 PM Nico Huber nico.h@gmx.de wrote:
There are currently two new platforms in development that seem to have trouble with public binaries (which would be necessary to make the code useful to the coreboot community). Namely, AMD/Picasso and Intel/Skylake-SP. Support for the former is already partially rotting on our master branch. Shouldn't we discuss their fate before more resources are wasted?
I happen to know that for the latter the whole point of uploading it in its current state was to get some feedback. The authors gave a live demo of it last fall at the OCP Summit in Europe and wanted to finally get some code published, which itself was quite a feat.
As for their fate, I think we need to look forward and not just backward. The code was pushed upstream with the intent of being used in real products and not just for the fun of putting a bunch of unusable code on display and making peoples' lives difficult. It also serves as a starting point for future work.
That said, it's fair to say that if nothing uses that code then perhaps it should be removed from the master branch. In Picasso's case, there is a mainboard in progress (CB:33772), and given the timeline I suspect there was a previous board that got cancelled (stuff doesn't always go as planned...). In Skylake-SP and Tioga Pass case, the hardware already exists and is in production but the blob situation might prevent it from being usable by the community, but the code is already being used as a starting point for the next generation platform.
sounds like good progress. Though, you make it look like SKL-SP support is just a code drop. If there is no intention to get it into shape and working with upstream coreboot, together with the community, should we merge it? Jonathan seems to work hard to clean the patches "formally into shape" (i.e. fixing checkpatch issues), but that's not all that matters, is it?
It is NOT just a code drop.
Don't worry. Maybe that came out wrong. What I meant is that /if/ the SKL-SP code will not be usable by anyone else, there'll probably be little interest in the community to work on it. Hence, the code would likely just stay as is. So what upstream it for? (that's not a retho- rical question, I'm really asking for expectations)
We had this before: Something that nobody really cared about was upstreamed. And then later, it was copied for newer platforms and people were confused by the feedback that they didn't get for the original platform's code (they expected that what was acceptable for the platform that nobody cared about would always be accepted). I'm not saying that this is the wrong way. Just that from my point of view, we had bad experience with it.
It is backed up by a huge commitment, multi-company collaboration and a long term roadmap.
Sounds nice. If it's really long term, i.e. covering more than a few months, maybe it's even worth to add that roadmap to coreboot's Documentation/ folder? Generally, in my experience, the more infor- mation is available before a review, the better the review will be.
For some context, please refer to [1] and [2]. The intention of this upstream is to get reviews from the
community, and
in turn to enable the community to work on coreboot support for Xeon
Scalable
Processors based servers, with this patch set as a start point.
Yeah, but what I don't understand so far: Where is one supposed to get the required blob? And who produced it anyway? Does an NDA with Intel suffice? or does it need a three-party NDA with Intel and Facebook? For which (future?) platform can we expect a public binary if any?
This seems critical to me. With little documentation (if any at all) about the silicon initialization, no documentation about the blob (I assume?) and no binaries to at least test it, what do you expect from the review?
[1] https://www.youtube.com/watch?v=eVmx9n5FyDI [2] https://www.youtube.com/watch?v=lvAnj0k54Jw
Thanks. Um, do you have anything that is not on Youtube? One of the nice things of the mailing list is that it's archived. But that applies only to information inline in the e-mail, of course. Also, personally, I would prefer something with less java-script, less commercials, and less tracking.
Nico _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org