On Fri, 23 May 2025 at 14:44, Simon Glass <s...@chromium.org> wrote:
> Hi,
>
> On Thu, 22 May 2025 at 03:18, Ying-Chun Liu (PaulLiu)
> <paul...@debian.org> wrote:
> >
> > From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org>
> >
> > Add efi_realloc() for realloc memory that previously alloc by
> efi_alloc().
> >
> > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul....@linaro.org>
> > Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
> > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > Cc: Peter Robinson <pbrobin...@gmail.com>
> > ---
> > include/efi_loader.h | 2 ++
> > lib/efi_loader/efi_memory.c | 52 +++++++++++++++++++++++++++++++++++++
> > 2 files changed, 54 insertions(+)
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index d0c72d0bc58..ff7ca5a0480 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -878,6 +878,8 @@ efi_status_t efi_next_variable_name(efi_uintn_t
> *size, u16 **buf,
> > #define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >>
> EFI_PAGE_SHIFT)
> > /* Allocate boot service data pool memory */
> > void *efi_alloc(size_t len);
> > +/* Reallocate boot service data pool memory */
> > +void *efi_realloc(void *ptr, size_t len);
> > /* Allocate pages on the specified alignment */
> > void *efi_alloc_aligned_pages(u64 len, int memory_type, size_t align);
> > /* More specific EFI memory allocator, called by EFI payloads */
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 0abb1f6159a..4c76df642dc 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -666,6 +666,58 @@ void *efi_alloc(size_t size)
> > return buf;
> > }
> >
> > +/**
> > + * efi_realloc() - reallocate boot services data pool memory
> > + *
> > + * Reallocate memory from pool for a new size and copy the data from
> old one.
> > + *
> > + * @ptr: pointer to old buffer
> > + * @size: number of bytes to allocate
> > + * Return: pointer to allocated memory or NULL
> > + */
>
> Should go in header file
>
Hi Simon,
I look into the code. It seems other functions in the same .c file put the
comments in the .c file instead of the header file.
Should I move the comments to the header file? I'm a bit confused.
Yours,
Paul
>
> > +void *efi_realloc(void *ptr, size_t size)
> > +{
> > + efi_status_t ret;
> > + void *new_ptr;
> > + struct efi_pool_allocation *alloc;
> > + u64 num_pages = efi_size_in_pages(size +
> > + sizeof(struct
> efi_pool_allocation));
> > + size_t old_size;
> > +
> > + if (!ptr)
> > + return efi_alloc(size);
> > +
> > + ret = efi_check_allocated((uintptr_t)ptr, true);
> > + if (ret != EFI_SUCCESS)
> > + return ptr;
> > +
> > + alloc = container_of(ptr, struct efi_pool_allocation, data);
> > +
> > + /* Check that this memory was allocated by efi_allocate_pool() */
> > + if (((uintptr_t)alloc & EFI_PAGE_MASK) ||
> > + alloc->checksum != checksum(alloc)) {
> > + printf("%s: illegal realloc 0x%p\n", __func__, ptr);
> > + return ptr;
> > + }
> > +
> > + /* Don't realloc. The actual size in pages is the same. */
> > + if (alloc->num_pages == num_pages)
> > + return ptr;
> > +
> > + old_size = alloc->num_pages * EFI_PAGE_SIZE -
> > + sizeof(struct efi_pool_allocation);
> > +
> > + new_ptr = efi_alloc(size);
> > +
> > + /* copy old data to new alloced buffer */
> > + memcpy(new_ptr, ptr, min(size, old_size));
> > +
> > + /* free the old buffer */
> > + efi_free_pool(ptr);
> > +
> > + return new_ptr;
> > +}
> > +
> > /**
> > * efi_free_pool() - free memory from pool
> > *
> > --
> > 2.39.5
> >
>
> Could you add a bit of detail as to why this patch is needed? It is
> hard to tell which code will actually use it. So long as it cannot be
> used outside lib/efi_loader then it should be OK.
>
> I would like to remove most/all use of efi_alloc() so that the EFI app
> can use the standard U-Boot malloc(). I am not 100% sure about this,
> but so far it seems like the best way to support EFI booting from the
> EFI app.
>
> So if this patch is accepted, it may make it harder to reconcile
> memory allocation in U-Boot across EFI loader and app.
>
> Regards,
> Simon
>
