hi Simon, On Mon, 10 Jun 2024 at 19:25, Simon Glass <s...@chromium.org> wrote: > > Hi Sughosh, > > On Thu, 6 Jun 2024 at 13:18, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > hi Simon, > > > > On Wed, 29 May 2024 at 22:00, Simon Glass <s...@chromium.org> wrote: > > > > > > +Sughosh Ganu for reference > > > > > > > > > On Sun, 31 Dec 2023 at 09:16, Tom Rini <tr...@konsulko.com> wrote: > > > > > > > > On Sun, Dec 31, 2023 at 04:40:06PM +0100, Heinrich Schuchardt wrote: > > > > > > > > > > > > > > > Am 31. Dezember 2023 16:11:44 MEZ schrieb Tom Rini > > > > > <tr...@konsulko.com>: > > > > > >On Sun, Dec 31, 2023 at 07:22:10AM -0700, Simon Glass wrote: > > > > > >> Hi Tom, > > > > > >> > > > > > >> On Sun, Dec 31, 2023 at 6:54 AM Tom Rini <tr...@konsulko.com> > > > > > >> wrote: > > > > > >> > > > > > > >> > On Sun, Dec 31, 2023 at 05:48:23AM -0700, Simon Glass wrote: > > > > > >> > > Hi, > > > > > >> > > > > > > > >> > > On Fri, Dec 29, 2023 at 10:52 AM Tom Rini <tr...@konsulko.com> > > > > > >> > > wrote: > > > > > >> > > > > > > > > >> > > > On Fri, Dec 29, 2023 at 06:44:15PM +0100, Mark Kettenis > > > > > >> > > > wrote: > > > > > >> > > > > > Date: Fri, 29 Dec 2023 11:17:44 -0500 > > > > > >> > > > > > From: Tom Rini <tr...@konsulko.com> > > > > > >> > > > > > > > > > > >> > > > > > On Fri, Dec 29, 2023 at 05:05:17PM +0100, Heinrich > > > > > >> > > > > > Schuchardt wrote: > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > Am 29. Dezember 2023 16:43:07 MEZ schrieb Tom Rini > > > > > >> > > > > > > <tr...@konsulko.com>: > > > > > >> > > > > > > >On Fri, Dec 29, 2023 at 05:36:09AM +0000, Simon Glass > > > > > >> > > > > > > >wrote: > > > > > >> > > > > > > >> Hi, > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> On Sat, Dec 16, 2023 at 6:01 PM Simon Glass > > > > > >> > > > > > > >> <s...@chromium.org> wrote: > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Hi, > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > This records my thoughts after a discussion with > > > > > >> > > > > > > >> > Ilias & Heinrich re > > > > > >> > > > > > > >> > memory allocation in U-Boot. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 1. malloc() > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > malloc() is used for programmatic memory > > > > > >> > > > > > > >> > allocation. It allows memory > > > > > >> > > > > > > >> > to be freed. It is not designed for very large > > > > > >> > > > > > > >> > allocations (e.g. a > > > > > >> > > > > > > >> > 10MB kernel or 100MB ramdisk). > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 2. lmb > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > lmb is used for large blocks of memory, such as > > > > > >> > > > > > > >> > those needed for a > > > > > >> > > > > > > >> > kernel or ramdisk. Allocation is only transitory, > > > > > >> > > > > > > >> > for the purposes of > > > > > >> > > > > > > >> > loading some images and booting. If the boot > > > > > >> > > > > > > >> > fails, then all lmb > > > > > >> > > > > > > >> > allocations go away. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > lmb is set up by getting all available memory and > > > > > >> > > > > > > >> > then removing what > > > > > >> > > > > > > >> > is used by U-Boot (code, data, malloc() space, > > > > > >> > > > > > > >> > etc.) > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > lmb reservations have a few flags so that areas > > > > > >> > > > > > > >> > of memory can be > > > > > >> > > > > > > >> > provided with attributes > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > There are some corner cases...e.g. loading a file > > > > > >> > > > > > > >> > does an lmb > > > > > >> > > > > > > >> > allocation but only for the purpose of avoiding a > > > > > >> > > > > > > >> > file being loaded > > > > > >> > > > > > > >> > over U-Boot code/data. The allocation is dropped > > > > > >> > > > > > > >> > immediately after the > > > > > >> > > > > > > >> > file is loaded. Within the bootm command, or when > > > > > >> > > > > > > >> > using standard boot, > > > > > >> > > > > > > >> > this would be fairly easy to solve. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Linux has renamed lmb to memblock. We should > > > > > >> > > > > > > >> > consider doing the same. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 3. EFI > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > EFI has its own memory-allocation tables. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Like lmb, EFI is able to deal with large > > > > > >> > > > > > > >> > allocations. But via a 'pool' > > > > > >> > > > > > > >> > function it can also do smaller allocations > > > > > >> > > > > > > >> > similar to malloc(), > > > > > >> > > > > > > >> > although each one uses at least 4KB at present. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > EFI allocations do not go away when a boot fails. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > With EFI it is possible to add allocations post > > > > > >> > > > > > > >> > facto, in which case > > > > > >> > > > > > > >> > they are added to the allocation table just as if > > > > > >> > > > > > > >> > the memory was > > > > > >> > > > > > > >> > allocated with EFI to begin with. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > The EFI allocations and the lmb allocations use > > > > > >> > > > > > > >> > the same memory, so in > > > > > >> > > > > > > >> > principle could conflict. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > EFI allocations are sometimes used to allocate > > > > > >> > > > > > > >> > internal U-Boot data as > > > > > >> > > > > > > >> > well, if needed by the EFI app. For example, > > > > > >> > > > > > > >> > while efi_image_parse() > > > > > >> > > > > > > >> > uses malloc(), efi_var_mem.c uses EFI allocations > > > > > >> > > > > > > >> > since the code runs > > > > > >> > > > > > > >> > in the app context and may need to access the > > > > > >> > > > > > > >> > memory after U-Boot has > > > > > >> > > > > > > >> > exited. Also efi_smbios.c uses allocate_pages() > > > > > >> > > > > > > >> > and then adds a new > > > > > >> > > > > > > >> > mapping as well. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > EFI memory has attributes, including what the > > > > > >> > > > > > > >> > memory is used for (to > > > > > >> > > > > > > >> > some degree of granularity). See enum > > > > > >> > > > > > > >> > efi_memory_type and struct > > > > > >> > > > > > > >> > efi_mem_desc. In the latter there are also > > > > > >> > > > > > > >> > attribute flags - whether > > > > > >> > > > > > > >> > memory is cacheable, etc. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > EFI also has the x86 idea of 'conventional' > > > > > >> > > > > > > >> > memory, meaning (I > > > > > >> > > > > > > >> > believe) that below 4GB that isn't reserved for > > > > > >> > > > > > > >> > the hardware/system. > > > > > >> > > > > > > >> > This is meaningless, or at least confusing, on > > > > > >> > > > > > > >> > ARM systems. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 4. reservations > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > It is perhaps worth mentioning a fourth method of > > > > > >> > > > > > > >> > memory management, > > > > > >> > > > > > > >> > where U-Boot reserves chunks of memory before > > > > > >> > > > > > > >> > relocation (in > > > > > >> > > > > > > >> > board_init_f.c), e.g. for the framebuffer, U-Boot > > > > > >> > > > > > > >> > code, the malloc() > > > > > >> > > > > > > >> > region, etc. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Problems > > > > > >> > > > > > > >> > —------- > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > There are no urgent problems, but here are some > > > > > >> > > > > > > >> > things that could be improved: > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 1. EFI should attach most of its data structures > > > > > >> > > > > > > >> > to driver model. This > > > > > >> > > > > > > >> > work has started, with the partition support, but > > > > > >> > > > > > > >> > more effort would > > > > > >> > > > > > > >> > help. This would make it easier to see which > > > > > >> > > > > > > >> > memory is related to > > > > > >> > > > > > > >> > devices and which is separate. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 2. Some drivers do EFI reservations today, > > > > > >> > > > > > > >> > whether EFI is used for > > > > > >> > > > > > > >> > booting or not (e.g. rockchip video > > > > > >> > > > > > > >> > rk_vop_probe()). > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 3. U-Boot doesn't really map arch-specific memory > > > > > >> > > > > > > >> > attributes (e.g. > > > > > >> > > > > > > >> > armv8's struct mm_region) to EFI ones. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 4. EFI duplicates some code from bootm, some of > > > > > >> > > > > > > >> > which relates to > > > > > >> > > > > > > >> > memory allocation (e.g. FDT fixup). > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 5. EFI code is used even if EFI is never used to > > > > > >> > > > > > > >> > boot > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 6. EFI allocations can result in the same memory > > > > > >> > > > > > > >> > being used as has > > > > > >> > > > > > > >> > already been allocated by lmb. Users may load > > > > > >> > > > > > > >> > files which overwrite > > > > > >> > > > > > > >> > memory allocated by EFI. > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> 7. We need to support doing an allocation when a > > > > > >> > > > > > > >> file is loaded (to > > > > > >> > > > > > > >> ensure files do not overlap), without making it too > > > > > >> > > > > > > >> difficult to load > > > > > >> > > > > > > >> multiple files to the same place, if desired. > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Lifetime > > > > > >> > > > > > > >> > -------- > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > We have three different memory allocators with > > > > > >> > > > > > > >> > different purposes. Can > > > > > >> > > > > > > >> > we unify them a little? > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Within U-Boot: > > > > > >> > > > > > > >> > - malloc() space lives forever > > > > > >> > > > > > > >> > - lmb lives while setting out images for booting > > > > > >> > > > > > > >> > - EFI (mostly) lives while booting an EFI app > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > In practice, EFI is set up early in U-Boot. Some > > > > > >> > > > > > > >> > of this is necessary, > > > > > >> > > > > > > >> > some not. EFI allocations stay around forever. > > > > > >> > > > > > > >> > This works OK since > > > > > >> > > > > > > >> > large allocations are normally not done in EFI, > > > > > >> > > > > > > >> > so memory isn't really > > > > > >> > > > > > > >> > consumed to any great degree by the boot process. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > What happens to EFI allocations if the app > > > > > >> > > > > > > >> > returns? They are still > > > > > >> > > > > > > >> > present, in case another app is run. This seems > > > > > >> > > > > > > >> > fine. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > API > > > > > >> > > > > > > >> > –-- > > > > > >> > > > > > > >> > Can we unify some APIs? > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > It should be possible to use lmb for large EFI > > > > > >> > > > > > > >> > memory allocations, so > > > > > >> > > > > > > >> > long as they are only needed for booting. We > > > > > >> > > > > > > >> > effectively do this > > > > > >> > > > > > > >> > today, since EFI does not manage the arrangement > > > > > >> > > > > > > >> > of loaded images in > > > > > >> > > > > > > >> > memory. for the most part. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > It would not make sense to use EFI allocation to > > > > > >> > > > > > > >> > replace lmb and > > > > > >> > > > > > > >> > malloc(), of course. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Could we use a common (lower-level) API for > > > > > >> > > > > > > >> > allocation, used by both > > > > > >> > > > > > > >> > lmb and EFI? They do have some similarities. > > > > > >> > > > > > > >> > However they have > > > > > >> > > > > > > >> > different lifetime constraints (EFI allocations > > > > > >> > > > > > > >> > are never dropped, > > > > > >> > > > > > > >> > unlikely lmb). > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > ** Overall, it seems that the existence of memory > > > > > >> > > > > > > >> > allocation in > > > > > >> > > > > > > >> > boot-time services has created confusion. Memory > > > > > >> > > > > > > >> > allocation is > > > > > >> > > > > > > >> > muddled, with both U-Boot code and boot-time > > > > > >> > > > > > > >> > services calling the same > > > > > >> > > > > > > >> > memory allocator. This just has not been clearly > > > > > >> > > > > > > >> > thought out. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Proposal > > > > > >> > > > > > > >> > —------- > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > Here are some ideas: > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 1. For video, use the driver model API to locate > > > > > >> > > > > > > >> > the video regions, or > > > > > >> > > > > > > >> > block off the entire framebuffer memory, for all > > > > > >> > > > > > > >> > devices as a whole. > > > > > >> > > > > > > >> > Use efi_add_memory_map() > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 2. Add memory attributes to UCLASS_RAM and use > > > > > >> > > > > > > >> > them in EFI, mapping to > > > > > >> > > > > > > >> > the EFI_MEMORY_... attributes in struct > > > > > >> > > > > > > >> > efi_mem_desc. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 3. Add all EFI reservations just before booting > > > > > >> > > > > > > >> > the app, as we do with > > > > > >> > > > > > > >> > devicetree fixup. With this model, malloc() and > > > > > >> > > > > > > >> > lmb are used for all > > > > > >> > > > > > > >> > allocation. Then efi_add_memory_map() is called > > > > > >> > > > > > > >> > for each region in > > > > > >> > > > > > > >> > turn just before booting. Memory attributes are > > > > > >> > > > > > > >> > dealt with above. The > > > > > >> > > > > > > >> > type (enum efi_memory_type) can be determined > > > > > >> > > > > > > >> > simply by the data > > > > > >> > > > > > > >> > structure stored in it, as is done today. For > > > > > >> > > > > > > >> > example, SMBIOS tables > > > > > >> > > > > > > >> > can use EFI_ACPI_RECLAIM_MEMORY. Very few types > > > > > >> > > > > > > >> > are used and EFI code > > > > > >> > > > > > > >> > understands the meaning of each. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 4. Avoid setting up EFI memory at the start of > > > > > >> > > > > > > >> > U-Boot. Do it only when > > > > > >> > > > > > > >> > booting. This looks to require very little effort. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 5. Avoid calling efi_allocate_pages() and > > > > > >> > > > > > > >> > efi_allocate_pool() outside > > > > > >> > > > > > > >> > boot-time services. This solves the problem 6. If > > > > > >> > > > > > > >> > memory is needed by > > > > > >> > > > > > > >> > an app, allocate it with malloc() and see 3. > > > > > >> > > > > > > >> > There are only two > > > > > >> > > > > > > >> > efi_allocate_pages() (smbios and efi_runtime). > > > > > >> > > > > > > >> > There are more calls of > > > > > >> > > > > > > >> > efi_allocate_pool(), but most of these seem easy > > > > > >> > > > > > > >> > to fix up. For > > > > > >> > > > > > > >> > example, efi_init_event_log() allocates a buffer, > > > > > >> > > > > > > >> > but this can be > > > > > >> > > > > > > >> > allocated in normal malloc() space or in a > > > > > >> > > > > > > >> > bloblist. > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > 6. Don't worry too much about whether EFI will be > > > > > >> > > > > > > >> > used for booting. > > > > > >> > > > > > > >> > The cost is likely not that great: use bootstage > > > > > >> > > > > > > >> > to measure it as is > > > > > >> > > > > > > >> > done for driver model. Try to minmise the cost of > > > > > >> > > > > > > >> > its tables, > > > > > >> > > > > > > >> > particularly for execution time, but otherwise > > > > > >> > > > > > > >> > just rely on the > > > > > >> > > > > > > >> > ability to disable EFI_LOADER. > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> 7. Add a flag to the 'load' command: > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> -m <type> - make an lmb allocation for the file > > > > > >> > > > > > > >> <type> is the image type to use (kernel, > > > > > >> > > > > > > >> ramdisk, flat_dt) > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> any existing allocation for that type will be > > > > > >> > > > > > > >> automatically freed > > > > > >> > > > > > > >> first. If <type> is "none" then no freeing is > > > > > >> > > > > > > >> possible: any loaded > > > > > >> > > > > > > >> images just stack up in lmb. > > > > > >> > > > > > > >> > > > > > >> > > > > > > >> Add an 'lmb' (or memblock) command to allow listing > > > > > >> > > > > > > >> and clearing allocations. > > > > > >> > > > > > > > > > > > > >> > > > > > > >I would really not like to change the user interface > > > > > >> > > > > > > >and instead simply > > > > > >> > > > > > > >handle this with flags to whatever mark/allocation > > > > > >> > > > > > > >function is called. > > > > > >> > > > > > > >You can always overwrite things that are brought in > > > > > >> > > > > > > >to memory, you > > > > > >> > > > > > > >cannot overwrite U-Boot or our internals. Optionally > > > > > >> > > > > > > >noting that some > > > > > >> > > > > > > >previous load to memory has been at least partially > > > > > >> > > > > > > >overwritten could be > > > > > >> > > > > > > >helpful, if it's not too much extra logic. > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > In most use cases users load exactly one file at each > > > > > >> > > > > > > address. An > > > > > >> > > > > > > unload command would be the cleanest way for a user to > > > > > >> > > > > > > indicate that > > > > > >> > > > > > > he wants to reuse the memory. > > > > > >> > > > > > > > > > > >> > > > > > I very much do not want to change the API. There's > > > > > >> > > > > > untold numbers of > > > > > >> > > > > > scripts out there and they should continue to work. I > > > > > >> > > > > > mentioned to Ilias > > > > > >> > > > > > off list just now that I'm not against adding a command > > > > > >> > > > > > to add flags to > > > > > >> > > > > > these areas, but I don't think it's worthwhile to > > > > > >> > > > > > prevent overwrites the > > > > > >> > > > > > user did early. The biggest long running problem in this > > > > > >> > > > > > space was that > > > > > >> > > > > > for 32bit ARM we couldn't know where the kernel BSS was > > > > > >> > > > > > going to be and > > > > > >> > > > > > so would have ramdisk at the wrong spot and get > > > > > >> > > > > > partially eaten, and > > > > > >> > > > > > this was hard to figure out. The current example is > > > > > >> > > > > > "ooops, > > > > > >> > > > > > decompression buffer for Image.gz/etc is too close to > > > > > >> > > > > > other things" > > > > > >> > > > > > which ends up failing nice and loudly, and in the future > > > > > >> > > > > > once this > > > > > >> > > > > > proposal is done we can just dynamically find and use a > > > > > >> > > > > > spot, since > > > > > >> > > > > > we'll have that ability finally. > > > > > >> > > > > > > > > > >> > > > > In order to keep the existing interfaces we need lmb to > > > > > >> > > > > keep track of > > > > > >> > > > > (at least ) three different states. I think of those as > > > > > >> > > > > "free", > > > > > >> > > > > "allocated" and "reserved". The load command would > > > > > >> > > > > "reserve" memory > > > > > >> > > > > insteaf "allocate". And it would pass a flag to lmb when > > > > > >> > > > > reserving > > > > > >> > > > > memory to indicate that reserving memory that is already > > > > > >> > > > > reserved is > > > > > >> > > > > ok. Both "reserved" and "allocated" memory should show up > > > > > >> > > > > as not free > > > > > >> > > > > in the EFI memory map (probably as EfiLoaderData). > > > > > >> > > > > > > > > >> > > > Yes, something like this is what I was getting at, thanks. > > > > > >> > > > > > > > >> > > Yes, that is a good way of putting it. There is definitely a > > > > > >> > > distinction there. > > > > > >> > > > > > > > >> > > If we don't want this flag, we could make U-Boot always do a > > > > > >> > > reservation on load, with a '-f' command to force loading over > > > > > >> > > an > > > > > >> > > existing reservation / releasing it first? > > > > > >> > > > > > > >> > Again, this is an API change and I don't want to change the API. > > > > > >> > > > > > >> The flag is only needed to drop a reservation, since we apparently > > > > > >> want the 'load' command to create a permanent reservation. It > > > > > >> should > > > > > >> not affect existing boot scripts since they won't load overlapping > > > > > >> images. > > > > > >> > > > > > >> Anyway, what do you suggest? > > > > > > > > > > > >That "load" (and sf read and nand read and tftp and wget and ...) > > > > > >"reserve" memory but not "allocate" memory and "reserve" means > > > > > >something > > > > > >is there and "allocate" means that it can't be modified again. For > > > > > >example, running U-Boot and our malloc pool are "allocated" but just > > > > > >loading a file to memory is "reserved". And then yes, I can see use > > > > > >for > > > > > >the command where some cases might want to "reserve" memory to fiddle > > > > > >with it and then "allocate" it so something else can't change it. > > > > > > > > > > > >This is one of those cases where english is terrible to discuss > > > > > >things > > > > > >in as both reserve and allocate can mean similar things. > > > > > > > > > > > > > > > > I have no clue what the semantics of the mentioned "reserved" state > > > > > might be. Up to now memory reservations designated address ranges > > > > > that U-Boot must not use at all, e.g. the memory used by OpenSBI or > > > > > TF-A. > > > > > > > > > > Who can and who cannot write into "reserved" memory? > > > > > > > > > > What is wrong about allocating memory for files to forbid any other > > > > > use until you are done with the file and free the memory? > > > > > > > > So, historically (lets say mid 2010s), you could use "load" to bring a > > > > file in to memory, anywhere, and it could even overwrite part of U-Boot > > > > (running, or malloc pool or whatever). You could also use "load" to > > > > bring a file in to memory and then bring another file in to that same > > > > location in memory for whatever reason (assorted development cases). > > > > > > > > Then later someone noted that using "load" to overwrite U-Boot should > > > > get a CVE and instead of ignoring it we decided to use "lmb" to try and > > > > make sure that we couldn't use "load" to overwrite U-Boot itself, and > > > > that that's a pre-check. This still allows overwriting a previously > > > > loaded file in memory. > > > > > > > > Overwriting something the user put in memory is part of the ABI and has > > > > some use cases. > > > > > > > > So for whatever future system we setup, a memory location can be: > > > > - Free. > > > > - Readable but not Writable. > > > > - Readable and Writable. > > > > > > > > Something like $loadaddr starts as Free. If someone then uses "load" to > > > > bring in an OS image, it's now "Readable and Writable". But if someone > > > > does an API call to ask for a new region memory, it wouldn't return > > > > $loadaddr because it's not Free. On the other hand, $relocaddr where > > > > U-Boot is, at least in terms of the API is that it's "Readable but not > > > > Writable". > > > > > > > > Does that help? > > > > Thanks for sharing this with me. I have been working on a solution for > > this issue [1], and plan to send a rfc series by the end of this week. > > My primary motivation behind this work is to have some kind of > > synchronisation between the allocations done by the LMB module and the > > EFI subsystem. As you are aware, the way things stand today, either of > > the two can make memory allocations overwriting any earlier allocated > > memory. The primary reasons behind this is 1) the LMB allocations are > > private to their respective callers, so the LMB memory map is not > > global and persistent, and 2) the EFI allocation routines do not have > > any view of the allocations that would have been made by the LMB > > module. > > > > To fix these issues, I am working on a solution which 1) makes LMB > > memory map persistent and global, and 2) have notifications between > > the two modules when either of the two has made any changes to it's > > memory map. This way, it would be possible to prohibit one module from > > allocating memory which is in use. > > > > I am aware that we have a whole bunch of tests and user scripts which > > work with an assumption that the memory can be re-used. The solution > > that I would be proposing takes care of this assumption. Hopefully I > > should be in a position to send a rfc series this week. We can discuss > > on the patches. Thanks. > > I actually am not sure what we are on the same page here. I described > the problems in my original email but your series seems to solve > somewhat different problems, while IMO introducing a whole new class > of problems.
Can you please describe in a little more detail what "whole new class of problems" are being introduced here. If there are problems being introduced, it is better if they are enumerated, so that I can try to fix them. > > Making LMB persistent and global is a solution looking for a problem, > IMO. Can we go up a level and figure out exactly what is broken here? > That is what I attempted to do in my original email. Perhaps you could > reply to that and suggest where you agree / disagree? I have gone through this thread multiple times, and my understanding was that there was no conclusive agreement that was arrived at. I saw a reply from Tom where he broadly mentioned what kind of a solution he is looking for. I am not sure if you have gone through the cover-letter of my RFC series [1], which clearly states the problem my patch series is trying to fix. Like I said above, it would help if you can describe in a little more detail the kind of issues you see with the RFC series. Thanks. -sughosh > > Regards, > Simon > > > > > -sughosh > > > > [1] - > > https://docs.google.com/document/d/1LJj4f1oBaxPIALnMlIy8Bf-3pESjecTyKhrQoeCfyrk/edit?usp=sharing > > > > > > > > > > -- > > > > Tom
