Proposal: U-Boot memory management

Mon Jun 10 15:55:44 CEST 2024

Hi Sughosh,

On Thu, 6 Jun 2024 at 13:18, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Simon,
>
> On Wed, 29 May 2024 at 22:00, Simon Glass <sjg at chromium.org> wrote:
> >
> > +Sughosh Ganu for reference
> >
> >
> > On Sun, 31 Dec 2023 at 09:16, Tom Rini <trini at 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 <trini at 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 <trini at 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 <trini at 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 <trini at 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 <trini at 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 <sjg at 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.

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?

Regards,
Simon

>
> -sughosh
>
> [1] - https://docs.google.com/document/d/1LJj4f1oBaxPIALnMlIy8Bf-3pESjecTyKhrQoeCfyrk/edit?usp=sharing
>
> > >
> > > --
> > > Tom