[PATCH v4 4/9] efi: Drop the memset() from efi_alloc()

[Heinrich Schuchardt]

On 11/3/24 01:28, Simon Glass wrote:
>  From my inspection none of the users need the memory to be zeroed. It
> is somewhat unexpected that it does so, since the name gives no clue to
> this.
>
> Drop the memset() so that it effectively becomes a wrapper around the
> normal EFI-pool allocator.
>
> Another option would be to drop this function and call
> efi_allocate_pool() directly, but that increase code size a little.
>
> Move the function comment to the header file like most other exported
> functions in U-Boot.
>
> Comments were made in v3 that another project uses memset() when
> allocating memory, but that is not required by the spec. In any case, as
> above, from inspection, none of the users need the memory to be zeroed,
> as they fill the entire region with their own data.

As already written in response to v3 of the series I want to be sure
that code that runs on EDK II does not fail on U-Boot.

EFI_DEVICE_PATH_UTILITIES_PROTOCOL.CreateDeviceNode() in EDK II zeros
out the returned buffer. We implement this function via efi_alloc().

Here is a code example relying on a zeroed out node:

https://github.com/acidanthera/gfxutil/blob/5284a662181ff4b81dd19d1279aedb68725b827f/edk2misc.c#L215

Please, drop the patch.

I have created issue USWG 0002487
https://mantis.uefi.org/mantis/view.php?id=2487) asking to indicate in
the specification if the returned device path node should be zeroed out
by the firmware.

Best regards

Heinrich

>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> (no changes since v4)
>
> Changes in v4:
> - Expand the commit message
>
> Changes in v3:
> - Add new patch to drop the memset() from efi_alloc()
> - Drop patch to convert device_path allocation to use malloc()
>
>   include/efi_loader.h        | 11 ++++++++++-
>   lib/efi_loader/efi_memory.c |  1 -
>   2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 291eca5c077..f940dc81345 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -758,8 +758,17 @@ efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf,
>    * Return:	size in pages
>    */
>   #define efi_size_in_pages(size) (((size) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT)
> -/* Allocate boot service data pool memory */
> +
> +/**
> + * efi_alloc() - allocate boot services data pool memory
> + *
> + * Allocate memory from pool with type EFI_BOOT_SERVICES_DATA
> + *
> + * @size:	number of bytes to allocate
> + * Return:	pointer to allocated memory, or NULL if out of memory
> + */
>   void *efi_alloc(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 2982c45dc38..8b086b165a7 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -696,7 +696,6 @@ void *efi_alloc(size_t size)
>   		log_err("out of memory\n");
>   		return NULL;
>   	}
> -	memset(buf, 0, size);
>
>   	return buf;
>   }