[PATCH v2 01/18] abuf: Add a helper for initing and allocating a buffer

[Simon Glass]

Hi Tom,

On Mon, 5 May 2025 at 22:38, Tom Rini <trini at konsulko.com> wrote:
>
> On Thu, May 01, 2025 at 07:37:01AM -0600, Simon Glass wrote:
>
> > This construct appears in various places. Reduce code size by adding a
> > function for it.
> >
> > It inits the abuf, then allocates it to the requested size.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add new patch with a helper for initing and allocating a buffer
> >
> >  boot/cedit.c          |  3 +--
> >  boot/scene.c          |  3 +--
> >  boot/scene_textline.c |  3 +--
> >  include/abuf.h        | 11 +++++++++++
> >  lib/abuf.c            |  9 +++++++++
> >  lib/of_live.c         |  3 +--
> >  test/lib/abuf.c       | 22 ++++++++++++++++++++++
> >  7 files changed, 46 insertions(+), 8 deletions(-)
>
> This just made me look again at the abuf implementation itself and
> become filled with regret I didn't reject it back in 2021. We're
> introducing wrappers around standard functions and calling conventions /
> patterns with something homegrown (and so not intuitive to others) that
> mainly hides the "sysmem" challenge we also have  and I wish you were
> interested in revisiting how that part of sandbox works instead. And
> even if this is a better design, for the sake of argument, it's not
> something everyone else is used to. And that's important.
>
> If we *really* need something different / new here, I'd rather see us go
> and wrap common/dlmalloc.c with kmalloc/kfree/etc and so give people
> something even more familiar-looking.

The purpose of abuf is actually not about hiding sysmem - you can see
that if you look at lib/abuf.c and the tests. It provides an easy way
to manage buffers, which we do a lot in U-Boot, particularly with
standard boot. It deals with whether the buffer is allocated or not,
so freeing is easy. With expo I want to be able to manage a buffer
containing an autoboot string in high-level code, say, without
worrying whether it has to be realloced.

For sysmem, I don't see a viable alternative, which keeps can reliably
keep addresses consistent across multiple phases (e.g. sandbox_vpl).
Also, I don't believe addresses like 0x7ffff7fbc000 are
human-friendly, yet that is what we would see in tests if we dropped
the mapping. The point of sandbox is to test U-Boot, not expose the
system internals. But we could discuss it if you like?

In general I am sensing that there are a lot of things which bug you
about the direction of U-Boot. and that you feel very differently
about some things than you did in 2021. I have quite a few concerns
too. So how about we make time to discuss these and see if we can get
on the same page, at least somewhat?

Coming back to this patch though, this is really about a code-side
win, rather than anything else.

Regards,
Simon