On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt wrote: > > > Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini <[email protected]>: > >On Tue, Dec 19, 2023 at 12:29:19AM +0100, Heinrich Schuchardt wrote: > >> > >> > >> Am 19. Dezember 2023 00:16:40 MEZ schrieb Tom Rini <[email protected]>: > >> >On Tue, Dec 19, 2023 at 12:08:31AM +0100, Heinrich Schuchardt wrote: > >> >> > >> >> > >> >> Am 18. Dezember 2023 23:41:08 MEZ schrieb Tom Rini <[email protected]>: > >> >> >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
signature.asc
Description: PGP signature
