[RFC PATCH 1/3] efi_loader: Introduce helper functions for EFI

[Heinrich Schuchardt]

Am 14. Januar 2021 05:48:28 MEZ schrieb AKASHI Takahiro <takahiro.akashi at linaro.org>:
>On Wed, Jan 13, 2021 at 01:11:47PM +0200, 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),
>
>In this respect,
>
>> let's add some helper functions which will retrieve and
>> parse file paths stored in EFI load options.
>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>> ---
>>  include/efi_helper.h        |  23 ++++++
>>  lib/efi_loader/efi_helper.c | 146
>++++++++++++++++++++++++++++++++++++
>>  2 files changed, 169 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..4e6bd2f036df
>> --- /dev/null
>> +++ b/include/efi_helper.h
>> @@ -0,0 +1,23 @@
>> +/* 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>
>> +
>> +enum load_option_dp_type {
>> +	BOOT_IMAGE_DP,
>> +	INITRD_DP,
>> +	DTB_DP,
>> +};
>> +
>> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t
>*size);
>> +struct efi_device_path *efi_get_fp_from_boot(int idx);
>> +struct efi_device_path *efi_dp_instance_by_idx(struct
>efi_device_path *dp,
>> +					       efi_uintn_t *size, int idx);
>> +
>> +#endif
>> diff --git a/lib/efi_loader/efi_helper.c
>b/lib/efi_loader/efi_helper.c
>> new file mode 100644
>> index 000000000000..e2437a4f698b
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_helper.c
>> @@ -0,0 +1,146 @@
>> +// 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_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)
>> +{
>> +	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);
>> +		ret = efi_get_variable_int(name, vendor, NULL, size, buf, NULL);
>> +	}
>> +
>> +	if (ret != EFI_SUCCESS) {
>> +		free(buf);
>> +		*size = 0;
>> +		return NULL;
>> +	}
>> +
>> +	return buf;
>> +}
>> +
>> +/**
>> + * efi_dp_instance_by_idx() - Get a file path with a specific index
>> + *
>> + * @name:	device path array
>> + * @size:	size of the discovered device path
>> + * @idx:	index of the device path
>> + *
>> + * Return:	device path or NULL. Caller must free the returned value
>> + */
>> +
>> +struct
>> +efi_device_path *efi_dp_instance_by_idx(struct efi_device_path *dp,
>> +					efi_uintn_t *size, int idx)
>
>The type of "idx" should be 'enum load_option_dp_type'.

There is nothing boot manager specific in this function. You can use it to access any multiple device path.

An enum or #define can be used on the caller side.

Should this function be moved to efi_devicepath.c?

Best regards

Heinrich

>
>Currently, "idx" is used as an index into the array of device paths,
>but given each device path is set to have its own guid, "idx" should be
>unlinked from the 'order' within the array.
>
>Even if you don't want so, this function should at least take care of
>some special cases like
><execution image> + (no initrd) + <dtb>
>Alternatively, "END device path" can be put at the second place.
>
>I expect that handling those corner cases should be described
>explicitly.
>
>> +{
>> +	struct efi_device_path *instance = NULL;
>> +	efi_uintn_t instance_size = 0;
>> +
>> +	if (!efi_dp_is_multi_instance(dp))
>> +		return NULL;
>> +
>> +	while (idx >= 0 && dp) {
>> +		instance = efi_dp_get_next_instance(&dp, &instance_size);
>> +		if (idx && instance) {
>> +			efi_free_pool(instance);
>> +			instance_size = 0;
>> +			instance = NULL;
>> +		}
>> +		idx--;
>> +	}
>> +	*size = instance_size;
>> +
>> +	return instance;
>> +}
>> +
>> +/**
>> + * 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)
>> +{
>> +	efi_uintn_t boot_order_size;
>> +	efi_status_t ret;
>> +	u16 boot_order;
>> +	u16 *pos;
>> +
>> +	boot_order_size = sizeof(boot_order);
>> +	ret = efi_get_variable_int(L"BootCurrent",
>> +				   &efi_global_variable_guid, NULL,
>> +				   &boot_order_size, &boot_order, NULL);
>> +	if (ret != EFI_SUCCESS)
>> +		goto out;
>> +
>> +	pos = efi_create_indexed_name(var_name, var_name_size, "Boot",
>> +				      boot_order);
>> +	if (!pos) {
>> +		ret = EFI_OUT_OF_RESOURCES;
>> +		goto out;
>> +	}
>> +
>> +out:
>> +	return ret;
>> +}
>> +
>> +/**
>> + * efi_get_fp_from_var() - Retrieve and return a device path from an
>EFI
>> + *			   Boot### variable
>> + *
>> + * @idx:	index of the instance to retrieve
>> + *
>> + * Return:	device path of NULL. Caller must free the returned value
>> + */
>> +struct efi_device_path *efi_get_fp_from_boot(int idx)
>
>Are you sure that this function is called only when "BootCurrent"
>is appropriately set?
>
>> +{
>> +	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];
>
>The size of var_name: 16 -> 8
>
>-Takahiro Akashi
>
>> +	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;
>> +
>> +	efi_deserialize_load_option(&lo, var_value, &size);
>> +	file_path = efi_dp_instance_by_idx(lo.file_path, &size, idx);
>> +	if (!file_path) {
>> +		printf("Instance not found\n");
>> +		return NULL;
>> +	}
>> +
>> +	return file_path;
>> +}
>> -- 
>> 2.30.0.rc2
>>