[3/6,v2] efi_loader: Introduce helper functions for EFI

Message ID 20210313214738.3257922-4-ilias.apalodimas@linaro.org
State New
Headers show
Series
  • Loadfile2 for initrd loading
Related show

Commit Message

Ilias Apalodimas March 13, 2021, 9:47 p.m.
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.apalodimas@linaro.org>

---
 include/efi_helper.h            |  15 ++++
 include/efi_loader.h            |   2 +
 lib/efi_loader/Makefile         |   1 +
 lib/efi_loader/efi_helper.c     | 133 ++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_var_common.c |  33 ++++++++
 5 files changed, 184 insertions(+)
 create mode 100644 include/efi_helper.h
 create mode 100644 lib/efi_loader/efi_helper.c

-- 
2.30.1

Comments

Heinrich Schuchardt March 14, 2021, 7:31 a.m. | #1
On 3/13/21 10:47 PM, 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.apalodimas@linaro.org>

> ---

>   include/efi_helper.h            |  15 ++++

>   include/efi_loader.h            |   2 +

>   lib/efi_loader/Makefile         |   1 +

>   lib/efi_loader/efi_helper.c     | 133 ++++++++++++++++++++++++++++++++

>   lib/efi_loader/efi_var_common.c |  33 ++++++++

>   5 files changed, 184 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..97492c65b6d2

> --- /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>

> +

> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);

> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);

> +

> +#endif

> diff --git a/include/efi_loader.h b/include/efi_loader.h

> index eb11a8c7d4b1..aa812a9a3052 100644

> --- a/include/efi_loader.h

> +++ b/include/efi_loader.h

> @@ -717,6 +717,8 @@ efi_status_t EFIAPI efi_query_variable_info(

>   			u64 *remaining_variable_storage_size,

>   			u64 *maximum_variable_size);

>

> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);

> +

>   /*

>    * See section 3.1.3 in the v2.7 UEFI spec for more details on

>    * the layout of EFI_LOAD_OPTION.  In short it is:

> 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..df5bdc506dbe

> --- /dev/null

> +++ b/lib/efi_loader/efi_helper.c

> @@ -0,0 +1,133 @@

> +// SPDX-License-Identifier: GPL-2.0+

> +/*

> + * Copyright (c) 2020, Linaro Limited

> + */

> +

> +#define LOG_CATEGORY LOGC_EFI

> +#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_create_current_boot_var() - 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 efi_create_current_boot_var(u16 var_name[],

> +						size_t var_name_size)

> +{

> +	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


We could add a paragraph here:

A boot option may contain an array of device paths. We use a VenMedia()
with a specific GUID to identify the usage of the array members. This
function is use to extract a specific device path.

> + *

> + * @guid:	Vendor guid of the VenMedia DP


vendor GUID of the VenMedia() device path node identifying the device path

> + *

> + * 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 = NULL;

> +	struct efi_device_path *tmp;

> +	struct efi_load_option lo;

> +	void *var_value = NULL;

> +	efi_uintn_t size;

> +	efi_status_t ret;

> +	u16 var_name[16];

> +

> +	ret = efi_create_current_boot_var(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)

> +		goto out;

> +

> +	tmp = efi_dp_from_lo(&lo, &size, guid);

> +	if (!tmp)

> +		goto out;

> +

> +	/* efi_dp_dup will just return NULL if efi_dp_next is NULL */

> +	file_path = efi_dp_dup(efi_dp_next(tmp));

> +

> +out:

> +	efi_free_pool(tmp);

> +	free(var_value);

> +

> +	return file_path;

> +}

> +

> +/**

> + * efi_get_file_size() - Get the size of a file using an EFI file handle

> + *

> + * @handle:	EFI file handle

> + * @size:	buffer to fill in the discovered size

> + *

> + * Return:	device path or NULL. Caller must free the returned value

> + */

> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)


Should this function be in lib/efi_loader/efi_file?

Best regards

Heinrich

> +{

> +	struct efi_file_info *info = NULL;

> +	efi_uintn_t bs = 0;

> +	efi_status_t ret;

> +

> +	*size = 0;

> +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> +				   info));

> +	if (ret != EFI_BUFFER_TOO_SMALL) {

> +		ret = EFI_DEVICE_ERROR;

> +		goto out;

> +	}

> +

> +	info = malloc(bs);

> +	if (!info) {

> +		ret = EFI_OUT_OF_RESOURCES;

> +		goto out;

> +	}

> +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> +				   info));

> +	if (ret != EFI_SUCCESS)

> +		goto out;

> +

> +	*size = info->file_size;

> +

> +out:

> +	free(info);

> +	return ret;

> +}

> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c

> index 1c7459266a38..b11ed91a74a4 100644

> --- a/lib/efi_loader/efi_var_common.c

> +++ b/lib/efi_loader/efi_var_common.c

> @@ -9,6 +9,7 @@

>   #include <common.h>

>   #include <efi_loader.h>

>   #include <efi_variable.h>

> +#include <stdlib.h>

>

>   enum efi_secure_mode {

>   	EFI_MODE_SETUP,

> @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)

>   	}

>   	return EFI_AUTH_VAR_NONE;

>   }

> +

> +/**

> + * 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);

> +		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;

> +}

>
Ilias Apalodimas March 14, 2021, 7:34 a.m. | #2
Hi Heinrich, 

On Sun, Mar 14, 2021 at 08:31:20AM +0100, Heinrich Schuchardt wrote:
> On 3/13/21 10:47 PM, Ilias Apalodimas wrote:

> > A following patch introduces a different logic for loading initrd's

> > based on the EFI_LOAD_FILE2_PROTOCOL.

> > +/**

> > + * efi_get_file_size() - Get the size of a file using an EFI file handle

> > + *

> > + * @handle:	EFI file handle

> > + * @size:	buffer to fill in the discovered size

> > + *

> > + * Return:	device path or NULL. Caller must free the returned value

> > + */

> > +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)

> 

> Should this function be in lib/efi_loader/efi_file?


there's already a statically defined efi_get_file_size() in that file.  I can
rename that, just let me know what you prefer.

> 

> Best regards

> 

> Heinrich

> 

> > +{

> > +	struct efi_file_info *info = NULL;

> > +	efi_uintn_t bs = 0;

> > +	efi_status_t ret;

> > +

> > +	*size = 0;

> > +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> > +				   info));

> > +	if (ret != EFI_BUFFER_TOO_SMALL) {

> > +		ret = EFI_DEVICE_ERROR;

> > +		goto out;

> > +	}

> > +

> > +	info = malloc(bs);

> > +	if (!info) {

> > +		ret = EFI_OUT_OF_RESOURCES;

> > +		goto out;

> > +	}

> > +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> > +				   info));

> > +	if (ret != EFI_SUCCESS)

> > +		goto out;

> > +

> > +	*size = info->file_size;

> > +

> > +out:

> > +	free(info);

> > +	return ret;

> > +}

> > diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c

> > index 1c7459266a38..b11ed91a74a4 100644

> > --- a/lib/efi_loader/efi_var_common.c

> > +++ b/lib/efi_loader/efi_var_common.c

> > @@ -9,6 +9,7 @@

> >   #include <common.h>

> >   #include <efi_loader.h>

> >   #include <efi_variable.h>

> > +#include <stdlib.h>

> > 

> >   enum efi_secure_mode {

> >   	EFI_MODE_SETUP,

> > @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)

> >   	}

> >   	return EFI_AUTH_VAR_NONE;

> >   }

> > +

> > +/**

> > + * 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);

> > +		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;

> > +}

> > 

>
Heinrich Schuchardt March 14, 2021, 8:01 a.m. | #3
On 3/14/21 8:34 AM, Ilias Apalodimas wrote:
> Hi Heinrich,

>

> On Sun, Mar 14, 2021 at 08:31:20AM +0100, Heinrich Schuchardt wrote:

>> On 3/13/21 10:47 PM, Ilias Apalodimas wrote:

>>> A following patch introduces a different logic for loading initrd's

>>> based on the EFI_LOAD_FILE2_PROTOCOL.

>>> +/**

>>> + * efi_get_file_size() - Get the size of a file using an EFI file handle

>>> + *

>>> + * @handle:	EFI file handle

>>> + * @size:	buffer to fill in the discovered size

>>> + *

>>> + * Return:	device path or NULL. Caller must free the returned value

>>> + */

>>> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)

>>

>> Should this function be in lib/efi_loader/efi_file?

>

> there's already a statically defined efi_get_file_size() in that file.  I can

> rename that, just let me know what you prefer.


Unfortunately we cannot call the efi_get_file_size() function in
lib/efi_loader/efi_file.c directly because a driver might have installed
a different implementation of a simple file protocol.

We should have different names for the functions, e.g. efi_file_size()
and efi_get_file_size().

But still I think lib/efi_loader/efi_file.c would be a better place for
the new function. We will be able to reuse it in:

* efi_capsule_read_file()
* efi_load_image_from_file()

Best regards

Heinrich

>

>>

>> Best regards

>>

>> Heinrich

>>

>>> +{

>>> +	struct efi_file_info *info = NULL;

>>> +	efi_uintn_t bs = 0;

>>> +	efi_status_t ret;

>>> +

>>> +	*size = 0;

>>> +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

>>> +				   info));

>>> +	if (ret != EFI_BUFFER_TOO_SMALL) {

>>> +		ret = EFI_DEVICE_ERROR;

>>> +		goto out;

>>> +	}

>>> +

>>> +	info = malloc(bs);

>>> +	if (!info) {

>>> +		ret = EFI_OUT_OF_RESOURCES;

>>> +		goto out;

>>> +	}

>>> +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

>>> +				   info));

>>> +	if (ret != EFI_SUCCESS)

>>> +		goto out;

>>> +

>>> +	*size = info->file_size;

>>> +

>>> +out:

>>> +	free(info);

>>> +	return ret;

>>> +}

>>> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c

>>> index 1c7459266a38..b11ed91a74a4 100644

>>> --- a/lib/efi_loader/efi_var_common.c

>>> +++ b/lib/efi_loader/efi_var_common.c

>>> @@ -9,6 +9,7 @@

>>>    #include <common.h>

>>>    #include <efi_loader.h>

>>>    #include <efi_variable.h>

>>> +#include <stdlib.h>

>>>

>>>    enum efi_secure_mode {

>>>    	EFI_MODE_SETUP,

>>> @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)

>>>    	}

>>>    	return EFI_AUTH_VAR_NONE;

>>>    }

>>> +

>>> +/**

>>> + * 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);

>>> +		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;

>>> +}

>>>

>>
Ilias Apalodimas March 14, 2021, 8:07 a.m. | #4
On Sun, Mar 14, 2021 at 09:01:48AM +0100, Heinrich Schuchardt wrote:
> On 3/14/21 8:34 AM, Ilias Apalodimas wrote:

> > Hi Heinrich,

> > 

> > On Sun, Mar 14, 2021 at 08:31:20AM +0100, Heinrich Schuchardt wrote:

> > > On 3/13/21 10:47 PM, Ilias Apalodimas wrote:

> > > > A following patch introduces a different logic for loading initrd's

> > > > based on the EFI_LOAD_FILE2_PROTOCOL.

> > > > +/**

> > > > + * efi_get_file_size() - Get the size of a file using an EFI file handle

> > > > + *

> > > > + * @handle:	EFI file handle

> > > > + * @size:	buffer to fill in the discovered size

> > > > + *

> > > > + * Return:	device path or NULL. Caller must free the returned value

> > > > + */

> > > > +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)

> > > 

> > > Should this function be in lib/efi_loader/efi_file?

> > 

> > there's already a statically defined efi_get_file_size() in that file.  I can

> > rename that, just let me know what you prefer.

> 

> Unfortunately we cannot call the efi_get_file_size() function in

> lib/efi_loader/efi_file.c directly because a driver might have installed

> a different implementation of a simple file protocol.

> 

> We should have different names for the functions, e.g. efi_file_size()

> and efi_get_file_size().

> 

> But still I think lib/efi_loader/efi_file.c would be a better place for

> the new function. We will be able to reuse it in:

> 

> * efi_capsule_read_file()

> * efi_load_image_from_file()


Sure, I'll rename this to efi_file_size() and move it in lib/efi_loader/efi_file.c

> 

> Best regards

> 

> Heinrich

> 

> > 

> > > 

> > > Best regards

> > > 

> > > Heinrich

> > > 

> > > > +{

> > > > +	struct efi_file_info *info = NULL;

> > > > +	efi_uintn_t bs = 0;

> > > > +	efi_status_t ret;

> > > > +

> > > > +	*size = 0;

> > > > +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> > > > +				   info));

> > > > +	if (ret != EFI_BUFFER_TOO_SMALL) {

> > > > +		ret = EFI_DEVICE_ERROR;

> > > > +		goto out;

> > > > +	}

> > > > +

> > > > +	info = malloc(bs);

> > > > +	if (!info) {

> > > > +		ret = EFI_OUT_OF_RESOURCES;

> > > > +		goto out;

> > > > +	}

> > > > +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> > > > +				   info));

> > > > +	if (ret != EFI_SUCCESS)

> > > > +		goto out;

> > > > +

> > > > +	*size = info->file_size;

> > > > +

> > > > +out:

> > > > +	free(info);

> > > > +	return ret;

> > > > +}

> > > > diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c

> > > > index 1c7459266a38..b11ed91a74a4 100644

> > > > --- a/lib/efi_loader/efi_var_common.c

> > > > +++ b/lib/efi_loader/efi_var_common.c

> > > > @@ -9,6 +9,7 @@

> > > >    #include <common.h>

> > > >    #include <efi_loader.h>

> > > >    #include <efi_variable.h>

> > > > +#include <stdlib.h>

> > > > 

> > > >    enum efi_secure_mode {

> > > >    	EFI_MODE_SETUP,

> > > > @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)

> > > >    	}

> > > >    	return EFI_AUTH_VAR_NONE;

> > > >    }

> > > > +

> > > > +/**

> > > > + * 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);

> > > > +		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;

> > > > +}

> > > > 

> > > 

>
Heinrich Schuchardt March 14, 2021, 8:49 a.m. | #5
On 3/13/21 10:47 PM, 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.apalodimas@linaro.org>

> ---

>   include/efi_helper.h            |  15 ++++

>   include/efi_loader.h            |   2 +

>   lib/efi_loader/Makefile         |   1 +

>   lib/efi_loader/efi_helper.c     | 133 ++++++++++++++++++++++++++++++++

>   lib/efi_loader/efi_var_common.c |  33 ++++++++

>   5 files changed, 184 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..97492c65b6d2

> --- /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


%s/_EFI_HELPER_H/_EFI_HELPER_H_/

Best regards

Heirnich
Heinrich Schuchardt March 14, 2021, 9:24 a.m. | #6
On 3/13/21 10:47 PM, 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.apalodimas@linaro.org>

> ---

>   include/efi_helper.h            |  15 ++++

>   include/efi_loader.h            |   2 +

>   lib/efi_loader/Makefile         |   1 +

>   lib/efi_loader/efi_helper.c     | 133 ++++++++++++++++++++++++++++++++

>   lib/efi_loader/efi_var_common.c |  33 ++++++++

>   5 files changed, 184 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..97492c65b6d2

> --- /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>

> +

> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);

> +struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);

> +

> +#endif

> diff --git a/include/efi_loader.h b/include/efi_loader.h

> index eb11a8c7d4b1..aa812a9a3052 100644

> --- a/include/efi_loader.h

> +++ b/include/efi_loader.h

> @@ -717,6 +717,8 @@ efi_status_t EFIAPI efi_query_variable_info(

>   			u64 *remaining_variable_storage_size,

>   			u64 *maximum_variable_size);

>

> +void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);

> +

>   /*

>    * See section 3.1.3 in the v2.7 UEFI spec for more details on

>    * the layout of EFI_LOAD_OPTION.  In short it is:

> 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..df5bdc506dbe

> --- /dev/null

> +++ b/lib/efi_loader/efi_helper.c

> @@ -0,0 +1,133 @@

> +// SPDX-License-Identifier: GPL-2.0+

> +/*

> + * Copyright (c) 2020, Linaro Limited

> + */

> +

> +#define LOG_CATEGORY LOGC_EFI

> +#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_create_current_boot_var() - 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 efi_create_current_boot_var(u16 var_name[],

> +						size_t var_name_size)

> +{

> +	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 = NULL;

> +	struct efi_device_path *tmp;



struct efi_device_path *tmp = NULL;

+lib/efi_loader/efi_helper.c:79:6: error: variable 'tmp' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
+        if (ret != EFI_SUCCESS)
+            ^~~~~~~~~~~~~~~~~~
+lib/efi_loader/efi_helper.c:90:16: note: uninitialized use occurs here
+        efi_free_pool(tmp);
+                      ^~~

> +	struct efi_load_option lo;

> +	void *var_value = NULL;

> +	efi_uintn_t size;

> +	efi_status_t ret;

> +	u16 var_name[16];

> +

> +	ret = efi_create_current_boot_var(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)

> +		goto out;

> +

> +	tmp = efi_dp_from_lo(&lo, &size, guid);

> +	if (!tmp)

> +		goto out;

> +

> +	/* efi_dp_dup will just return NULL if efi_dp_next is NULL */

> +	file_path = efi_dp_dup(efi_dp_next(tmp));

> +

> +out:

> +	efi_free_pool(tmp);

> +	free(var_value);

> +

> +	return file_path;

> +}

> +

> +/**

> + * efi_get_file_size() - Get the size of a file using an EFI file handle

> + *

> + * @handle:	EFI file handle

> + * @size:	buffer to fill in the discovered size

> + *

> + * Return:	device path or NULL. Caller must free the returned value

> + */

> +efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)

> +{

> +	struct efi_file_info *info = NULL;

> +	efi_uintn_t bs = 0;

> +	efi_status_t ret;

> +

> +	*size = 0;

> +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> +				   info));

> +	if (ret != EFI_BUFFER_TOO_SMALL) {

> +		ret = EFI_DEVICE_ERROR;

> +		goto out;

> +	}

> +

> +	info = malloc(bs);

> +	if (!info) {

> +		ret = EFI_OUT_OF_RESOURCES;

> +		goto out;

> +	}

> +	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,

> +				   info));

> +	if (ret != EFI_SUCCESS)

> +		goto out;

> +

> +	*size = info->file_size;

> +

> +out:

> +	free(info);

> +	return ret;

> +}

> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c

> index 1c7459266a38..b11ed91a74a4 100644

> --- a/lib/efi_loader/efi_var_common.c

> +++ b/lib/efi_loader/efi_var_common.c

> @@ -9,6 +9,7 @@

>   #include <common.h>

>   #include <efi_loader.h>

>   #include <efi_variable.h>

> +#include <stdlib.h>

>

>   enum efi_secure_mode {

>   	EFI_MODE_SETUP,

> @@ -343,3 +344,35 @@ enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)

>   	}

>   	return EFI_AUTH_VAR_NONE;

>   }

> +

> +/**

> + * 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);

> +		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;

> +}

>

Patch

diff --git a/include/efi_helper.h b/include/efi_helper.h
new file mode 100644
index 000000000000..97492c65b6d2
--- /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>
+
+efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size);
+struct efi_device_path *efi_get_dp_from_boot(const efi_guid_t guid);
+
+#endif
diff --git a/include/efi_loader.h b/include/efi_loader.h
index eb11a8c7d4b1..aa812a9a3052 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -717,6 +717,8 @@  efi_status_t EFIAPI efi_query_variable_info(
 			u64 *remaining_variable_storage_size,
 			u64 *maximum_variable_size);
 
+void *efi_get_var(u16 *name, const efi_guid_t *vendor, efi_uintn_t *size);
+
 /*
  * See section 3.1.3 in the v2.7 UEFI spec for more details on
  * the layout of EFI_LOAD_OPTION.  In short it is:
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..df5bdc506dbe
--- /dev/null
+++ b/lib/efi_loader/efi_helper.c
@@ -0,0 +1,133 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2020, Linaro Limited
+ */
+
+#define LOG_CATEGORY LOGC_EFI
+#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_create_current_boot_var() - 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 efi_create_current_boot_var(u16 var_name[],
+						size_t var_name_size)
+{
+	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 = NULL;
+	struct efi_device_path *tmp;
+	struct efi_load_option lo;
+	void *var_value = NULL;
+	efi_uintn_t size;
+	efi_status_t ret;
+	u16 var_name[16];
+
+	ret = efi_create_current_boot_var(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)
+		goto out;
+
+	tmp = efi_dp_from_lo(&lo, &size, guid);
+	if (!tmp)
+		goto out;
+
+	/* efi_dp_dup will just return NULL if efi_dp_next is NULL */
+	file_path = efi_dp_dup(efi_dp_next(tmp));
+
+out:
+	efi_free_pool(tmp);
+	free(var_value);
+
+	return file_path;
+}
+
+/**
+ * efi_get_file_size() - Get the size of a file using an EFI file handle
+ *
+ * @handle:	EFI file handle
+ * @size:	buffer to fill in the discovered size
+ *
+ * Return:	device path or NULL. Caller must free the returned value
+ */
+efi_status_t efi_get_file_size(struct efi_file_handle *fh, efi_uintn_t *size)
+{
+	struct efi_file_info *info = NULL;
+	efi_uintn_t bs = 0;
+	efi_status_t ret;
+
+	*size = 0;
+	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
+				   info));
+	if (ret != EFI_BUFFER_TOO_SMALL) {
+		ret = EFI_DEVICE_ERROR;
+		goto out;
+	}
+
+	info = malloc(bs);
+	if (!info) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+	ret = EFI_CALL(fh->getinfo(fh, (efi_guid_t *)&efi_file_info_guid, &bs,
+				   info));
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	*size = info->file_size;
+
+out:
+	free(info);
+	return ret;
+}
diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
index 1c7459266a38..b11ed91a74a4 100644
--- a/lib/efi_loader/efi_var_common.c
+++ b/lib/efi_loader/efi_var_common.c
@@ -9,6 +9,7 @@ 
 #include <common.h>
 #include <efi_loader.h>
 #include <efi_variable.h>
+#include <stdlib.h>
 
 enum efi_secure_mode {
 	EFI_MODE_SETUP,
@@ -343,3 +344,35 @@  enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t *guid)
 	}
 	return EFI_AUTH_VAR_NONE;
 }
+
+/**
+ * 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);
+		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;
+}