Hi Tom, On Mon, 5 May 2025 at 22:38, Tom Rini <tr...@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 <s...@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
