On Wed, Apr 30, 2025 at 07:39:29PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 30 Apr 2025 at 09:21, Tom Rini <tr...@konsulko.com> wrote:
> >
> > On Wed, Apr 30, 2025 at 08:51:53AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Wed, 30 Apr 2025 at 08:29, Tom Rini <tr...@konsulko.com> wrote:
> > > >
> > > > On Wed, Apr 30, 2025 at 07:55:01AM -0600, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Tue, 29 Apr 2025 at 11:11, Tom Rini <tr...@konsulko.com> wrote:
> > > > > >
> > > > > > On Mon, Apr 28, 2025 at 10:41:25AM -0400, Raymond Mao wrote:
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Thu, 17 Apr 2025 at 14:16, Simon Glass <s...@chromium.org> 
> > > > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > This series adds a standard way of passing information between 
> > > > > > > > different
> > > > > > > > firmware phases. This already exists in U-Boot at a very basic 
> > > > > > > > level, in
> > > > > > > > the form of a bloblist containing an spl_handoff structure, but 
> > > > > > > > the intent
> > > > > > > > here is to define something useful across projects.
> > > > > > > >
> > > > > > > > The need for this is growing as firmware fragments into 
> > > > > > > > multiple binaries
> > > > > > > > each with its own purpose. Without any run-time connection, we 
> > > > > > > > must rely
> > > > > > > > on build-time settings which are brittle and painful to keep in 
> > > > > > > > sync.
> > > > > > > >
> > > > > > > > This feature is named 'standard passage' since the name is more 
> > > > > > > > unique
> > > > > > > > than many others that could be chosen, it is a passage in the 
> > > > > > > > sense that
> > > > > > > > information is flowing from one place to another and it is 
> > > > > > > > standard,
> > > > > > > > because that is what we want to create.
> > > > > > > >
> > > > > > > > The implementation is mostly a pointer to a bloblist in a 
> > > > > > > > register, with
> > > > > > > > an extra register to point to a devicetree, for more complex 
> > > > > > > > data. This
> > > > > > > > should cover all cases (small memory footprint as well as 
> > > > > > > > complex data
> > > > > > > > flow) and be easy enough to implement on all architectures.
> > > > > > > >
> > > > > > > > The emphasis is on enabling open communcation between binaries, 
> > > > > > > > not
> > > > > > > > enabling passage of secret, undocumented data, although this is 
> > > > > > > > possible
> > > > > > > > in a private environment.
> > > > > > > >
> > > > > > > > This series is available at u-boot-dm/pass-working
> > > > > > > >
> > > > > > >
> > > > > > > First of all, can you group those patches which are necessary for
> > > > > > > refactoring the "passage" concept into a smaller series for an 
> > > > > > > easy
> > > > > > > review?
> > > > > > >
> > > > > > > Actually as I mentioned in our previous discussions, I don't 
> > > > > > > agree to
> > > > > > > add a OF_BLOBLIST as this is duplicated with "BLOBLIST +
> > > > > > > BLOBLIST_PASSAGE_MANDATORY (aka PASSAGE_IN in the context of your
> > > > > > > patch - I am not sure the reason you prefer to rename it - do we 
> > > > > > > have
> > > > > > > any other passage approaches other than using bloblist? And do you
> > > > > > > have any considerations to allow a fallback or not in case passage
> > > > > > > failures?)"
> > > > > >
> > > > > > As preamble, I am actively working to push the changes for
> > > > > > vexpress_fvp_bloblist to be available in CI and so provide an easier
> > > > > > testing path.
> > > > > >
> > > > > > At the high level, I don't see what the difference is between 
> > > > > > "BLOBLIST
> > > > > > + BLOBLIST_PASSAGE_MANDATORY" and "BLOBLIST + OF_BLOBLIST" or 
> > > > > > "BLOBLIST
> > > > > > + whatever v4 of this series ends up showing". Everyone seems 
> > > > > > wildly in
> > > > > > agreement that if the prior stage is supposed to give us information
> > > > > > then it must have done so.
> > > > > >
> > > > > > One of the long running "jokes" in computer science about naming 
> > > > > > being
> > > > > > one of the hardest problems applies here, too.
> > > > >
> > > > > I want to use OF_BLOBLIST as it clearly indicates that the devicetree
> > > > > comes from a bloblist. The BLOBLIST_PASSAGE_MANDATORY thing is a
> > > > > work-around to try to avoid having OF_BLOBLIST.
> > > >
> > > > Or:
> > > > Other people want to use BLOBLIST_PASSAGE_MANDATORY as it clearly
> > > > indicates that the standard passage will be present and so include the
> > > > devicetree and any other required logs/blobs/content. The OF_BLOBLIST
> > > > thing is a work-around to try and avoid having
> > > > BLOBLIST_PASSAGE_MANDATORY.
> > > >
> > > > It's a naming problem and I don't care as long as we end up with an
> > > > implementation that's useful. And *that* is where my list of what
> > > > bloblist needs to have, in the end, comes from.
> > >
> > > My approach does the same thing, but allows my x86 boards to work
> > > without all the confusion. It shows fdt coming from 'standard passage'
> > > when it is.
> >
> > It does not matter. Really. Whatever you're doing with OF_BLOBLIST can
> > also be done with BLOBLIST_PASSAGE_MANDATORY as it's just a naming
> > thing. There's nothing confusing here unless you insist on confusing it.
> > Which gets back to what I was saying before about what changes need to
> > happen to bloblist.
> >
> > > > > > > Secondly I prefer to keep the register convention code as a 
> > > > > > > xferlist
> > > > > > > library that can be used for arm arches other than split them into
> > > > > > > different low level assemblies. In case of any firmware handoff
> > > > > > > specification update, we just need to maintain this library.
> > > > > >
> > > > > > Yes, if things can be written in a C file, they should be in a C 
> > > > > > file,
> > > > > > not assembler.
> > > > >
> > > > > In this case I disagree and if you look at the x86 code, this kind of
> > > > > approach is just not sensible. Please take a look at the contortions
> > > > > of xferlist_from_boot_arg(). It was written for one architecture only,
> > > > > yet purports to be a general API. My approach is simpler. Also, if
> > > > > save_boot_args() is implemented by the board, then standard passage
> > > > > will be broken for that board. It's a weak function!
> > > >
> > > > I don't see why this has to be in assembly and not C, no. And it's a
> > > > weak function because it doesn't yet exist on other architectures and
> > > > everyone hates #if.
> > >
> > > We just don't need it, though. If you would like this to work on x86,
> > > it needs to go.
> > >
> > > >
> > > > > More generally, please decide if you are willing to let me maintain 
> > > > > bloblist.
> > > >
> > > > Can you be given substantive feedback? Or is only trivial feedback
> > > > allowed?
> > >
> > > I'm happy to discuss it on a call, but if you insist on
> > > BLOBLIST_PASSAGE_MANDATORY, then it is going to be tricky. The
> > > OF_BLOBLIST thing has consumed far too much time. If you want Linaro
> > > to maintain bloblist, please just say so. I had thought you were OK
> > > with my taking that on again.
> >
> > As I've said numerous times now, BLOBLIST_PASSAGE_MANDATORY and
> > OF_BLOBLIS are just names. Names are one of the hardest problems in
> > computer science. As I've also said numerous times now, you need to
> > show ME that you have a good design. That means you need to implement
> > register passing rather than BLOBLIST_FIXED. You then also likely need
> > to untie "Is there a bloblist?" from "Make a bloblist" as that's what
> > complicates things.
> 
> They are not names, actually. The logic is different. Please look
> through it carefully. Perhaps we can discuss this on a call?

I don't know how a call would help. They really are just names for the
same goal. Whatever the symbol is for "board must use standard passage"
can select OF_BLOBLIST, it does not matter. Especially if you do what I
asked in the email you link to later.

> As to BLOBLIST_FIXED, I have a path to do that, and convert x86 also.
> I even have boards in the lab to make sure it all works. I see that
> you have got the bloblist board into CI as well, so that will make
> things easier.

Good, lets get rid of BLOBLIST_FIXED as soon as we can. That'll help
remove some of the overall confusion.

> But that path I'm talking about doesn't run through the xferlist

I cannot understand why:
https://patchwork.ozlabs.org/project/uboot/patch/20250417121601.v3.10.I56d9d06e5d74724c963eaffbbe1c2aa400b60bb8@changeid/
is good
and:
https://source.denx.de/u-boot/u-boot/-/commit/5103e69344d60eb8c8d2cf804ec00d0ef3975124
is bad.

And then what you want will enable x86 to be handled, but what we have
in tree cannot.

> stuff, nor BLOBLIST_PASSAGE_MANDATORY for that matter, so am I not
> willing to do it before this series is in. I do want to focus my
> efforts on things which will bear fruit. For whatever reason, this one
> seems to have generated a lot more heat than light, over a period of
> 18 months.
> 
> Another option you have is to take this series and then the following
> ones for the migration and *then* decide if you want something
> tweaked. That would be more likely to result in a successful
> collaboration.
> 
> The reason I keep asking you about just letting me maintain it is that
> I thought you agreed to this a few months back:
> 
> https://lore.kernel.org/u-boot/20250216154801.GZ1233568@bill-the-cat/
> 
> Please, consider the option of just sending me a 'yes' and I'll sort
> all this out.

So are you, or are you not doing what I outlined in what you linked
above? Because looking briefly, again, at v3, and setting aside the
unintentional breakage, it's not doing much of what I asked. We're
finally (good!) adding a way to pass between U-Boot phases on ARM. We're
also not making use of the existing mechanism for doing so, for unclear
reasons. So yes, before you make a v4 you should explain what's wrong
with the method we have today.

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to