On 05.03.21 23:22, Ilias Apalodimas wrote:
> A following patch introduces a different logic for loading initrd's
> based on the EFI_LOAD_FILE2_PROTOCOL.
> Since similar logic can be applied in the future for other system files
> (i.e DTBs), let's add some helper functions which will retrieve and
> parse file paths stored in EFI variables.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> ---
> include/efi_helper.h | 15 +++++
> lib/efi_loader/Makefile | 1 +
> lib/efi_loader/efi_helper.c | 118 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 134 insertions(+)
> create mode 100644 include/efi_helper.h
> create mode 100644 lib/efi_loader/efi_helper.c
>
> diff --git a/include/efi_helper.h b/include/efi_helper.h
> new file mode 100644
> index 000000000000..4980a1bb35d7
> --- /dev/null
> +++ b/include/efi_helper.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#if !defined _EFI_HELPER_H_
> +#define _EFI_HELPER_H
> +
> +#include <efi.h>
> +#include <efi_api.h>
> +
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
> +
> +#endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 10b42e8847bf..da2741adecfa 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -23,6 +23,7 @@ endif
> obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> obj-$(CONFIG_CMD_BOOTEFI_BOOTMGR) += efi_bootmgr.o
> obj-y += efi_boottime.o
> +obj-y += efi_helper.o
> obj-$(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) += efi_capsule.o
> obj-$(CONFIG_EFI_CAPSULE_FIRMWARE) += efi_firmware.o
> obj-y += efi_console.o
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> new file mode 100644
> index 000000000000..5437faa3ec49
> --- /dev/null
> +++ b/lib/efi_loader/efi_helper.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2020, Linaro Limited
> + */
> +
> +#include <common.h>
> +#include <env.h>
> +#include <malloc.h>
> +#include <dm.h>
> +#include <fs.h>
> +#include <efi_helper.h>
> +#include <efi_load_initrd.h>
> +#include <efi_loader.h>
> +#include <efi_variable.h>
> +
> +/**
> + * efi_get_var() - read value of an EFI variable
> + *
> + * @name: variable name
> + * @start: vendor GUID
> + * @size: size of allocated buffer
> + *
> + * Return: buffer with variable data or NULL
> + */
> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
Please, move the function to lib/efi_loader/efi_var_common.c.
> +{
> + efi_status_t ret;
> + void *buf = NULL;
> +
> + *size = 0;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL) {
> + buf = malloc(*size);
> + if (!buf)
> + return NULL;
> + ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
> + }
> +
> + if (ret != EFI_SUCCESS) {
> + free(buf);
> + *size = 0;
> + return NULL;
> + }
> +
> + return buf;
> +}
> +
> +/**
> + * create_boot_var_indexed() - Return Boot#### name were #### is replaced by
> + * the value of BootCurrent
> + *
> + * @var_name: variable name
> + * @var_name_size: size of var_name
> + *
> + * Return: Status code
> + */
> +static efi_status_t create_boot_var_indexed(u16 var_name[], size_t
> var_name_size)
The function name does not convey that it is related to BootCurrent.
How about efi_create_current_boot_var()?
> +{
> + efi_uintn_t boot_current_size;
> + efi_status_t ret;
> + u16 boot_current;
> + u16 *pos;
> +
> + boot_current_size = sizeof(boot_current);
> + ret = efi_get_variable_int(L"BootCurrent",
> + &efi_global_variable_guid, NULL,
> + &boot_current_size, &boot_current, NULL);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
> + boot_current);
> + if (!pos) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +/**
> + * efi_get_dp_from_boot() - Retrieve and return a device path from an EFI
> + * Boot### variable
> + *
> + * @guid: Vendor guid of the VenMedia DP
> + *
> + * Return: device path or NULL. Caller must free the returned value
> + */
> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid)
> +{
> + struct efi_device_path *file_path;
> + struct efi_load_option lo;
> + void *var_value = NULL;
> + efi_uintn_t size;
> + efi_status_t ret;
> + u16 var_name[16];
> +
> + ret = create_boot_var_indexed(var_name, sizeof(var_name));
> + if (ret != EFI_SUCCESS)
> + return NULL;
> +
> + var_value = efi_get_var(var_name, &efi_global_variable_guid, &size);
> + if (!var_value)
> + return NULL;
> +
> + ret = efi_deserialize_load_option(&lo, var_value, &size);
> + if (ret != EFI_SUCCESS)
> + return NULL;
> +
> + file_path = efi_dp_from_lo(&lo, &size, guid);
> + if (!file_path) {
> + log_debug("Missing file path in %ls", var_name);
Missing LF. Knowing the GUID would help. I suggest:
log_debug("VenMedia(%pUl) not found in %ls\n", guid, var_name);
Better move the message to efi_dp_from_lo().
Please, add
#define LOG_CATEGORY LOGC_EFI
before the first #include to allow filtering for EFI messages.
Best regards
Heinrich
> + return NULL;
> + }
> +
> + return file_path;
> +}
>
