Proposal: U-Boot memory management

[Tom Rini]

On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt wrote:
> 
> 
> Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini <trini at konsulko.com>:
> >On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich Schuchardt wrote:
> >> 
> >> 
> >> Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini <trini at konsulko.com>:
> >> >On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich Schuchardt wrote:
> >> >> 
> >> >> 
> >> >> Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom Rini <trini at konsulko.com>:
> >> >> >On Mon, Dec 18, 2023 at 11:34:16PM +0100, Heinrich Schuchardt wrote:
> >> >> >
> >> >> >[snip]
> >> >> >> Or take:
> >> >> >> 
> >> >> >> load host 0:1 $c kernel.efi
> >> >> >> load host 0:1 $d initrd.img
> >> >> >> 
> >> >> >> How could we ensure that initrd.img is not overwriting a part of kernel.efi without memory allocation?
> >> >> >
> >> >> >Today, invalid checksum as part of some part of the kernel fails. But
> >> >> >how do we do this tomorrow, are you suggesting that "load" perform
> >> >> >malloc() in some predefined size? If $c is below $d and $c + kernel.efi
> >> >> >is now above $d we can throw an error before trying to load, yes. But
> >> >> >what about:
> >> >> >load host 0:1 $d initrd.img
> >> >> >load host 0:1 $c kernel.efi
> >> >> >
> >> >> >In that case (which is only marginally contrived, the more real case is
> >> >> >loading device tree in to unexpectedly large ramdisk because someone
> >> >> >didn't understand the general advice on why device tree is lower than
> >> >> >ramdisk address) I'm fine with an error that amounts to "you just
> >> >> >corrupted another allocation" and then "fail, reset the board" or so.
> >> >> >
> >> >> 
> >> >> Our current malloc library cannot manage the complete memory. We need a library like lmb which should also cover the memory management that we currently have in lib/efi/efi_memory.c. This must include a memory type attribute for usage in the GetMemoryMap() service. A management on page level seems sufficient.
> >> >> 
> >> >> The load command should permanently allocate memory in that lmb+ library.
> >> >> 
> >> >> We need an unload command to free the memory if we want to reuse the memory or we might let the load comand free the memory if exactly the same start address is reused.
> >> >
> >> >Our current way of loading things in to memory does not handle the case
> >> >I described, yes. How would what you're proposing handle it?
> >> 
> >> If the load command has to allocate memory for the image and that allocation is kept, any attempt to allocate overlapping memory would fail.
> >
> >So you're saying that the load command has to pre-allocate memory? Or as
> >it goes? If the latter, in what size chunks? This starts to get at what
> >Simon was talking about with respect to memory fragmentation. Which to
> >be clear is a problem we have today, we just let things overlap and hope
> >something later catches an incorrect checksum.
> >
> 
> I don't want to replace the malloc library which handles large numbets of allocations.

I'm confused. The normal malloc library is not involved with current
image loading, it's direct to memory (with some attempts at sanity
checking by lmb).  Are you proposing a different allocator with
malloc/free like behavior? If so, please outline how it will determine
pool size, and how we'll use it to load thing to memory.

> Closing the eyes when the user loads multiple files does not solve the fragmentation problem.

Yes. I'm only noting that today we just ignore the problem and sometimes
catch it via checksums.

> Fragmentation only happens if we have many concurrent allocations.  In EFI we are allocating top down. The number of concurrent allocations is low. Typically a few dozen at most. After terminating an application these should be freed again.

OK, so are you saying that we would no longer be loading _to_ a location
in memory and instead just be saying "load this thing" and picking where
dynamically?

> When loading a file from a file system we know the filesize beforehand. So allocation is trivial.
> 
> The loady command currently does not use the  offered size information but could do so.

We should be using that information to make sure we don't overwrite
U-Boot itself, but I don't recall how exactly we handle it today
off-hand.

> TFTP is problematic because it does not transfer the filesize. We would probably try to allocate a large chunk of memory and then downsize the allocation after reading the whole file.

Reading from non-filesystem flash also has this problem, but we at least
specify the amount to read too. But yes, it gets back to what I was
asking about on how you're proposing to handle network load cases.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231218/6e00d1dc/attachment.sig>