On Tue, Dec 19, 2023 at 09:15:21PM -0700, Simon Glass wrote: > Hi, > > On Tue, 19 Dec 2023 at 05:46, Tom Rini <tr...@konsulko.com> wrote: > > > > On Tue, Dec 19, 2023 at 03:15:38AM +0100, Heinrich Schuchardt wrote: > > > > > > > > > Am 19. Dezember 2023 02:26:00 MEZ schrieb Tom Rini <tr...@konsulko.com>: > > > >On Tue, Dec 19, 2023 at 01:01:51AM +0100, Heinrich Schuchardt wrote: > > > >> > > > >> > > > >> Am 19. Dezember 2023 00:31:30 MEZ schrieb Tom Rini > > > >> <tr...@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 > > > >> >> <tr...@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 > > > >> >> >> <tr...@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. > > > > > > All memory below the stack needs to be managed. Malloc uses a small > > > memory area (a few MiB) above the stack. > > > > That's a rather huge change for how U-Boot works. > > > > > >> 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? > > > > > > Both preassigned and allocator assigned adresses are compatible with > > > memory management. > > > > > > Architectures and binaries have different requirements. On riscv64 you > > > can load Linux kernel, initrd, fdt anywhere. We don't need predefined > > > addresses there. Other architectures have restrictions. > > > > Yes, 64 bit architecture tend to only have alignment requirements while > > 32bit architectures have both alignment requirements and some memory > > window requirement. Whatever we implement here needs to handle both > > cases. > > > > > >> 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. > > > > > > If the user issues multiple load commands, he can overwrite previous > > > files. > > > > Then it sounds like we lost one benefit of all of this overhead. > > > > > During boot command execution I guess the different allocations respect > > > each other. > > > > > > > > > > >> 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. > > > > > > > > > > It depends on the protocol. Http conveys the size before the data. Tftp > > > does not. > > > > > > If you don't know the size, you must preallocate a big chunk, check that > > > the download does not exceed it, and downsize the allocation afterwards. > > > This is not a new problem but exists already with current lmb usage. > > > > Yes, and what I'm trying to find out is if what you're suggesting would > > do anything about it, since previous statements you made implied to me > > that we would prevent it. > > > > To me, at this point it sounds like what we need is more like persistent > > memory blocks and a hook that can be called in to for both "give me all > > known memory blocks" and "add this memory block to the list", so that > > EFI can do whatever it needs to do upon starting an application and then > > upon return to U-Boot. Both malloc/free allocations and "load this blob > > to memory from whatever" allocations would call the appropriate hook for > > tracking. > > In my mind the solution to this entire problem is fairly minor changes > to how memory is allocated and only for EFI. > > I tried to map out what that would look like and we have IMO got lost > in the weeds a bit. > > I am not trying to solve the problem of the 'load' command doing an > allocation and throwing it away. To be that is WAI, at least until we > come up with another type of command. This is one of the reasons for > standard boot, allowing a more cohesive approach to booting. > > I will think about this some more...
OK, but please keep in mind that lmb not being at all persistent is a problem for everyone, not just EFI. That really needs to be addressed, maybe with some flags for dis-allowing overwrites to the area. For example, the apple-m1 code to use lmb to find locations for the kernel/etc can be written to more than once (allocate the address, then write to it to start with, even) but the range that covers U-Boot itself (malloc pool and so forth) need to be stopped. -- Tom
signature.asc
Description: PGP signature
