diff mbox series

[13/16] efi_loader: memory buffer for variables

Message ID 20200327052800.11022-14-xypron.glpk@gmx.de
State New
Headers show
Series efi_loader: non-volatile and runtime variables | expand

Commit Message

Heinrich Schuchardt March 27, 2020, 5:27 a.m. UTC
Saving UEFI variable as encoded U-Boot environment variables does not allow
support at runtime.

Provide functions to manage a memory buffer with UEFI variables.

Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
---
 include/efi_variable.h             |  16 ++
 lib/efi_loader/Makefile            |   1 +
 lib/efi_loader/efi_variables_mem.c | 317 +++++++++++++++++++++++++++++
 3 files changed, 334 insertions(+)
 create mode 100644 lib/efi_loader/efi_variables_mem.c

--
2.25.1

Comments

Punit Agrawal March 27, 2020, 8:09 a.m. UTC | #1
Heinrich Schuchardt <xypron.glpk at gmx.de> writes:

> Saving UEFI variable as encoded U-Boot environment variables does not allow
> support at runtime.
>
> Provide functions to manage a memory buffer with UEFI variables.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>  include/efi_variable.h             |  16 ++
>  lib/efi_loader/Makefile            |   1 +
>  lib/efi_loader/efi_variables_mem.c | 317 +++++++++++++++++++++++++++++
>  3 files changed, 334 insertions(+)
>  create mode 100644 lib/efi_loader/efi_variables_mem.c

[...]

> diff --git a/lib/efi_loader/efi_variables_mem.c b/lib/efi_loader/efi_variables_mem.c
> new file mode 100644
> index 0000000000..f70cc65f8b
> --- /dev/null
> +++ b/lib/efi_loader/efi_variables_mem.c
> @@ -0,0 +1,317 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * File interface for UEFI variables
> + *
> + * Copyright (c) 2020, Heinrich Schuchardt
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +#include <efi_variable.h>
> +#include <u-boot/crc.h>
> +
> +static struct efi_var_file __efi_runtime_data *efi_var_buf;
> +static struct efi_var_entry __efi_runtime_data *efi_current_var;
> +
> +/**
> + * memcpy() - copy memory area
> + *
> + * At runtime memcpy() is not available.
> + *
> + * @dest:	destination buffer
> + * @src:	source buffer
> + * @n:		number of bytes to copy
> + * Return:	pointer to destination buffer
> + */
> +void __efi_runtime efi_var_mem_memcpy(void *dest, const void *src, size_t n)
> +{
> +	u8 *d = dest;
> +	const u8 *s = src;
> +
> +	for (; n; --n)
> +		*d++ = *s++;
> +}
> +
> +/**
> + * efi_var_mem_compare() - compare GUID and name with a variable
> + *
> + * @var:	variable to compare
> + * @guid:	GUID to compare
> + * @name:	variable name to compare
> + * @next:	pointer to next variable
> + * Return:	true if match
> + */
> +static bool __efi_runtime
> +efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid,
> +		    const u16 *name, struct efi_var_entry **next)
> +{
> +	int i;
> +	u8 *guid1, *guid2;
> +	const u16 *data, *var_name;
> +	bool match = true;
> +
> +	for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0;
> +	     i < sizeof(efi_guid_t) && match; ++i)
> +		match = (guid1[i] == guid2[i]);
> +
> +	for (data = var->name, var_name = name;; ++data, ++var_name) {
> +		if (match)
> +			match = (*data == *var_name);
> +		if (!*data)
> +			break;
> +	}

Don't roll out two different versions of memcmp() - make it a generic
helper and use it here.

> +
> +	++data;
> +
> +	if (next)
> +		*next = (struct efi_var_entry *)
> +			ALIGN((uintptr_t)data + var->length, 8);

Also, instead of implementing iteration via carrying the iterator
around, the readability of patches would be improved if this was done as
a simple loop outside of this function.

If for some reason, this is considered to be better, please add it as a
comment in your next version.

Thanks,
Punit

> +
> +	return match;
> +}
> +
> +/**
> + * efi_var_mem_find() - find a variable in the list
> + *
> + * @guid:	GUID of the variable
> + * @name:	name of the variable
> + * @next:	on exit pointer to the next variable after the found one
> + * Return:	found variable
> + */
> +struct efi_var_entry __efi_runtime
> +*efi_var_mem_find(const efi_guid_t *guid, const u16 *name,
> +		  struct efi_var_entry **next)
> +{
> +	struct efi_var_entry *var, *last;
> +
> +	last = (struct efi_var_entry *)
> +	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
> +
> +	if (!*name) {
> +		if (next) {
> +			*next = efi_var_buf->var;
> +			if (*next >= last)
> +				*next = NULL;
> +		}
> +		return NULL;
> +	}
> +	if (efi_current_var &&
> +	    efi_var_mem_compare(efi_current_var, guid, name, next)) {
> +		if (next && *next >= last)
> +			*next = NULL;
> +		return efi_current_var;
> +	}
> +
> +	var = efi_var_buf->var;
> +	if (var < last) {
> +		for (; var;) {
> +			struct efi_var_entry *pos;
> +			bool match;
> +
> +			match = efi_var_mem_compare(var, guid, name, &pos);
> +			if (pos >= last)
> +				pos = NULL;
> +			if (match) {
> +				if (next)
> +					*next = pos;
> +				return var;
> +			}
> +			var = pos;
> +		}
> +	}
> +	if (next)
> +		*next = NULL;
> +	return NULL;
> +}
> +
> +/**
> + * efi_var_mem_del() - delete a variable from the list of variables
> + *
> + * @var:	variable to delete
> + */
> +void __efi_runtime efi_var_mem_del(struct efi_var_entry *var)
> +{
> +	u16 *data = var->name;
> +	struct efi_var_entry *next, *last;
> +	u64 *from, *to;
> +
> +	if (!var)
> +		return;
> +
> +	last = (struct efi_var_entry *)
> +	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
> +	if (var < efi_current_var)
> +		efi_current_var = NULL;
> +
> +	for (data = var->name; *data; ++data)
> +		;
> +	++data;
> +	next = (struct efi_var_entry *)
> +	       ALIGN((uintptr_t)data + var->length, 8);
> +	efi_var_buf->length -= (uintptr_t)next - (uintptr_t)var;
> +
> +	for (to = (u64 *)var, from = (u64 *)next; from < (u64 *)last;
> +	     ++to, ++from)
> +		*to = *from;
> +	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
> +				   efi_var_buf->length -
> +				   sizeof(struct efi_var_file));
> +}
> +
> +/**
> + * efi_var_mem_ins() - append a variable to the list of variables
> + *
> + * The variable is appended without checking if a variable of the same name
> + * already exists. The two data buffers are concatenated.
> + *
> + * @name:	variable name
> + * @vendor:	GUID
> + * @attributes:	variable attributes
> + * @size1:	size of the first data buffer
> + * @data1:	first data buffer
> + * @size2:	size of the second data field
> + * @data2:	second data buffer
> + * Result:	status code
> + */
> +efi_status_t __efi_runtime efi_var_mem_ins(
> +				u16 *variable_name,
> +				const efi_guid_t *vendor, u32 attributes,
> +				const efi_uintn_t size1, const void *data1,
> +				const efi_uintn_t size2, const void *data2)
> +{
> +	u16 *data;
> +	struct efi_var_entry *var;
> +	u32 var_name_len;
> +
> +	var = (struct efi_var_entry *)
> +	      ((uintptr_t)efi_var_buf + efi_var_buf->length);
> +	for (var_name_len = 0; variable_name[var_name_len]; ++var_name_len)
> +		;
> +	++var_name_len;
> +	data = var->name + var_name_len;
> +
> +	if ((uintptr_t)data - (uintptr_t)efi_var_buf + size1 + size2 >
> +	    EFI_VAR_BUF_SIZE)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	var->attr = attributes;
> +	var->length = size1 + size2;
> +
> +	efi_var_mem_memcpy(&var->guid, vendor, sizeof(efi_guid_t));
> +	efi_var_mem_memcpy(var->name, variable_name,
> +			   sizeof(u16) * var_name_len);
> +	efi_var_mem_memcpy(data, data1, size1);
> +	efi_var_mem_memcpy((u8 *)data + size1, data2, size2);
> +
> +	var = (struct efi_var_entry *)
> +	      ALIGN((uintptr_t)data + var->length, 8);
> +	efi_var_buf->length = (uintptr_t)var - (uintptr_t)efi_var_buf;
> +	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
> +				   efi_var_buf->length -
> +				   sizeof(struct efi_var_file));
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_var_mem_free() - determine free memory for variables
> + *
> + * Return:	maximum data size plus variable name size
> + */
> +u64 __efi_runtime efi_var_mem_free(void)
> +{
> +	return EFI_VAR_BUF_SIZE - efi_var_buf->length -
> +	       sizeof(struct efi_var_entry);
> +}
> +
> +/**
> + * efi_var_mem_bs_del() - delete boot service only variables
> + */
> +static void efi_var_mem_bs_del(void)
> +{
> +	struct efi_var_entry *var = efi_var_buf->var;
> +
> +	for (;;) {
> +		struct efi_var_entry *last;
> +
> +		last = (struct efi_var_entry *)
> +		       ((uintptr_t)efi_var_buf + efi_var_buf->length);
> +		if (var >= last)
> +			break;
> +		if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) {
> +			u16 *data;
> +
> +			/* skip variable */
> +			for (data = var->name; *data; ++data)
> +				;
> +			++data;
> +			var = (struct efi_var_entry *)
> +			      ALIGN((uintptr_t)data + var->length, 8);
> +		} else {
> +			/* delete variable */
> +			efi_var_mem_del(var);
> +		}
> +	}
> +}
> +
> +/**
> + * efi_var_mem_notify_exit_boot_services() - ExitBootService callback
> + *
> + * @event:	callback event
> + * @context:	callback context
> + */
> +static void EFIAPI __efi_runtime
> +efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context)
> +{
> +	/* Delete boot service only variables */
> +	efi_var_mem_bs_del();
> +}
> +
> +/**
> + * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback
> + *
> + * @event:	callback event
> + * @context:	callback context
> + */
> +static void EFIAPI __efi_runtime
> +efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context)
> +{
> +	efi_convert_pointer(0, (void **)&efi_var_buf);
> +}
> +
> +/**
> + * efi_var_mem_init() - set-up variable list
> + *
> + * Return:	status code
> + */
> +efi_status_t efi_var_mem_init(void)
> +{
> +	u64 memory;
> +	efi_status_t ret;
> +	struct efi_event *event;
> +
> +	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> +				 EFI_RUNTIME_SERVICES_DATA,
> +				 efi_size_in_pages(EFI_VAR_BUF_SIZE),
> +				 &memory);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +	efi_var_buf = (struct efi_var_file *)(uintptr_t)memory;
> +	memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE);
> +	efi_var_buf->magic = EFI_VAR_FILE_MAGIC;
> +	efi_var_buf->length = (uintptr_t)efi_var_buf->var -
> +			      (uintptr_t)efi_var_buf;
> +	/* crc32 for 0 bytes = 0 */
> +
> +	ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
> +			       efi_var_mem_notify_exit_boot_services, NULL,
> +			       NULL, &event);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +	ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK,
> +			       efi_var_mem_notify_virtual_address_map, NULL,
> +			       NULL, &event);
> +	if (ret != EFI_SUCCESS)
> +		return ret;
> +	return ret;
> +}
> --
> 2.25.1
Heinrich Schuchardt March 27, 2020, 10:45 a.m. UTC | #2
On 3/27/20 9:09 AM, Punit Agrawal wrote:
> Heinrich Schuchardt <xypron.glpk at gmx.de> writes:
>
>> Saving UEFI variable as encoded U-Boot environment variables does not allow
>> support at runtime.
>>
>> Provide functions to manage a memory buffer with UEFI variables.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>   include/efi_variable.h             |  16 ++
>>   lib/efi_loader/Makefile            |   1 +
>>   lib/efi_loader/efi_variables_mem.c | 317 +++++++++++++++++++++++++++++
>>   3 files changed, 334 insertions(+)
>>   create mode 100644 lib/efi_loader/efi_variables_mem.c
>
> [...]
>
>> diff --git a/lib/efi_loader/efi_variables_mem.c b/lib/efi_loader/efi_variables_mem.c
>> new file mode 100644
>> index 0000000000..f70cc65f8b
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_variables_mem.c
>> @@ -0,0 +1,317 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * File interface for UEFI variables
>> + *
>> + * Copyright (c) 2020, Heinrich Schuchardt
>> + */
>> +
>> +#include <common.h>
>> +#include <efi_loader.h>
>> +#include <efi_variable.h>
>> +#include <u-boot/crc.h>
>> +
>> +static struct efi_var_file __efi_runtime_data *efi_var_buf;
>> +static struct efi_var_entry __efi_runtime_data *efi_current_var;
>> +
>> +/**
>> + * memcpy() - copy memory area
>> + *
>> + * At runtime memcpy() is not available.
>> + *
>> + * @dest:	destination buffer
>> + * @src:	source buffer
>> + * @n:		number of bytes to copy
>> + * Return:	pointer to destination buffer
>> + */
>> +void __efi_runtime efi_var_mem_memcpy(void *dest, const void *src, size_t n)
>> +{
>> +	u8 *d = dest;
>> +	const u8 *s = src;
>> +
>> +	for (; n; --n)
>> +		*d++ = *s++;
>> +}
>> +
>> +/**
>> + * efi_var_mem_compare() - compare GUID and name with a variable
>> + *
>> + * @var:	variable to compare
>> + * @guid:	GUID to compare
>> + * @name:	variable name to compare
>> + * @next:	pointer to next variable
>> + * Return:	true if match
>> + */
>> +static bool __efi_runtime
>> +efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid,
>> +		    const u16 *name, struct efi_var_entry **next)
>> +{
>> +	int i;
>> +	u8 *guid1, *guid2;
>> +	const u16 *data, *var_name;
>> +	bool match = true;
>> +
>> +	for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0;
>> +	     i < sizeof(efi_guid_t) && match; ++i)
>> +		match = (guid1[i] == guid2[i]);
>> +
>> +	for (data = var->name, var_name = name;; ++data, ++var_name) {
>> +		if (match)
>> +			match = (*data == *var_name);
>> +		if (!*data)
>> +			break;
>> +	}
>
> Don't roll out two different versions of memcmp() - make it a generic
> helper and use it here.

memcmp() has many architecture specific implementations. Currently all
of these are gone at UEFI runtime.

The loop above does not only do comparison but also seeks the end of the
u16 string.

>
>> +
>> +	++data;
>> +
>> +	if (next)
>> +		*next = (struct efi_var_entry *)
>> +			ALIGN((uintptr_t)data + var->length, 8);
>
> Also, instead of implementing iteration via carrying the iterator
> around, the readability of patches would be improved if this was done as
> a simple loop outside of this function.
>
> If for some reason, this is considered to be better, please add it as a
> comment in your next version.

I could extract a function for comparing two u16 strings and return the
end of the first string if they match and NULL otherwise.

Would this match your expectations?

Best regards

Heinrich

>
> Thanks,
> Punit
>
>> +
>> +	return match;
>> +}
>> +
>> +/**
>> + * efi_var_mem_find() - find a variable in the list
>> + *
>> + * @guid:	GUID of the variable
>> + * @name:	name of the variable
>> + * @next:	on exit pointer to the next variable after the found one
>> + * Return:	found variable
>> + */
>> +struct efi_var_entry __efi_runtime
>> +*efi_var_mem_find(const efi_guid_t *guid, const u16 *name,
>> +		  struct efi_var_entry **next)
>> +{
>> +	struct efi_var_entry *var, *last;
>> +
>> +	last = (struct efi_var_entry *)
>> +	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
>> +
>> +	if (!*name) {
>> +		if (next) {
>> +			*next = efi_var_buf->var;
>> +			if (*next >= last)
>> +				*next = NULL;
>> +		}
>> +		return NULL;
>> +	}
>> +	if (efi_current_var &&
>> +	    efi_var_mem_compare(efi_current_var, guid, name, next)) {
>> +		if (next && *next >= last)
>> +			*next = NULL;
>> +		return efi_current_var;
>> +	}
>> +
>> +	var = efi_var_buf->var;
>> +	if (var < last) {
>> +		for (; var;) {
>> +			struct efi_var_entry *pos;
>> +			bool match;
>> +
>> +			match = efi_var_mem_compare(var, guid, name, &pos);
>> +			if (pos >= last)
>> +				pos = NULL;
>> +			if (match) {
>> +				if (next)
>> +					*next = pos;
>> +				return var;
>> +			}
>> +			var = pos;
>> +		}
>> +	}
>> +	if (next)
>> +		*next = NULL;
>> +	return NULL;
>> +}
>> +
>> +/**
>> + * efi_var_mem_del() - delete a variable from the list of variables
>> + *
>> + * @var:	variable to delete
>> + */
>> +void __efi_runtime efi_var_mem_del(struct efi_var_entry *var)
>> +{
>> +	u16 *data = var->name;
>> +	struct efi_var_entry *next, *last;
>> +	u64 *from, *to;
>> +
>> +	if (!var)
>> +		return;
>> +
>> +	last = (struct efi_var_entry *)
>> +	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
>> +	if (var < efi_current_var)
>> +		efi_current_var = NULL;
>> +
>> +	for (data = var->name; *data; ++data)
>> +		;
>> +	++data;
>> +	next = (struct efi_var_entry *)
>> +	       ALIGN((uintptr_t)data + var->length, 8);
>> +	efi_var_buf->length -= (uintptr_t)next - (uintptr_t)var;
>> +
>> +	for (to = (u64 *)var, from = (u64 *)next; from < (u64 *)last;
>> +	     ++to, ++from)
>> +		*to = *from;
>> +	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
>> +				   efi_var_buf->length -
>> +				   sizeof(struct efi_var_file));
>> +}
>> +
>> +/**
>> + * efi_var_mem_ins() - append a variable to the list of variables
>> + *
>> + * The variable is appended without checking if a variable of the same name
>> + * already exists. The two data buffers are concatenated.
>> + *
>> + * @name:	variable name
>> + * @vendor:	GUID
>> + * @attributes:	variable attributes
>> + * @size1:	size of the first data buffer
>> + * @data1:	first data buffer
>> + * @size2:	size of the second data field
>> + * @data2:	second data buffer
>> + * Result:	status code
>> + */
>> +efi_status_t __efi_runtime efi_var_mem_ins(
>> +				u16 *variable_name,
>> +				const efi_guid_t *vendor, u32 attributes,
>> +				const efi_uintn_t size1, const void *data1,
>> +				const efi_uintn_t size2, const void *data2)
>> +{
>> +	u16 *data;
>> +	struct efi_var_entry *var;
>> +	u32 var_name_len;
>> +
>> +	var = (struct efi_var_entry *)
>> +	      ((uintptr_t)efi_var_buf + efi_var_buf->length);
>> +	for (var_name_len = 0; variable_name[var_name_len]; ++var_name_len)
>> +		;
>> +	++var_name_len;
>> +	data = var->name + var_name_len;
>> +
>> +	if ((uintptr_t)data - (uintptr_t)efi_var_buf + size1 + size2 >
>> +	    EFI_VAR_BUF_SIZE)
>> +		return EFI_OUT_OF_RESOURCES;
>> +
>> +	var->attr = attributes;
>> +	var->length = size1 + size2;
>> +
>> +	efi_var_mem_memcpy(&var->guid, vendor, sizeof(efi_guid_t));
>> +	efi_var_mem_memcpy(var->name, variable_name,
>> +			   sizeof(u16) * var_name_len);
>> +	efi_var_mem_memcpy(data, data1, size1);
>> +	efi_var_mem_memcpy((u8 *)data + size1, data2, size2);
>> +
>> +	var = (struct efi_var_entry *)
>> +	      ALIGN((uintptr_t)data + var->length, 8);
>> +	efi_var_buf->length = (uintptr_t)var - (uintptr_t)efi_var_buf;
>> +	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
>> +				   efi_var_buf->length -
>> +				   sizeof(struct efi_var_file));
>> +
>> +	return EFI_SUCCESS;
>> +}
>> +
>> +/**
>> + * efi_var_mem_free() - determine free memory for variables
>> + *
>> + * Return:	maximum data size plus variable name size
>> + */
>> +u64 __efi_runtime efi_var_mem_free(void)
>> +{
>> +	return EFI_VAR_BUF_SIZE - efi_var_buf->length -
>> +	       sizeof(struct efi_var_entry);
>> +}
>> +
>> +/**
>> + * efi_var_mem_bs_del() - delete boot service only variables
>> + */
>> +static void efi_var_mem_bs_del(void)
>> +{
>> +	struct efi_var_entry *var = efi_var_buf->var;
>> +
>> +	for (;;) {
>> +		struct efi_var_entry *last;
>> +
>> +		last = (struct efi_var_entry *)
>> +		       ((uintptr_t)efi_var_buf + efi_var_buf->length);
>> +		if (var >= last)
>> +			break;
>> +		if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) {
>> +			u16 *data;
>> +
>> +			/* skip variable */
>> +			for (data = var->name; *data; ++data)
>> +				;
>> +			++data;
>> +			var = (struct efi_var_entry *)
>> +			      ALIGN((uintptr_t)data + var->length, 8);
>> +		} else {
>> +			/* delete variable */
>> +			efi_var_mem_del(var);
>> +		}
>> +	}
>> +}
>> +
>> +/**
>> + * efi_var_mem_notify_exit_boot_services() - ExitBootService callback
>> + *
>> + * @event:	callback event
>> + * @context:	callback context
>> + */
>> +static void EFIAPI __efi_runtime
>> +efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context)
>> +{
>> +	/* Delete boot service only variables */
>> +	efi_var_mem_bs_del();
>> +}
>> +
>> +/**
>> + * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback
>> + *
>> + * @event:	callback event
>> + * @context:	callback context
>> + */
>> +static void EFIAPI __efi_runtime
>> +efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context)
>> +{
>> +	efi_convert_pointer(0, (void **)&efi_var_buf);
>> +}
>> +
>> +/**
>> + * efi_var_mem_init() - set-up variable list
>> + *
>> + * Return:	status code
>> + */
>> +efi_status_t efi_var_mem_init(void)
>> +{
>> +	u64 memory;
>> +	efi_status_t ret;
>> +	struct efi_event *event;
>> +
>> +	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
>> +				 EFI_RUNTIME_SERVICES_DATA,
>> +				 efi_size_in_pages(EFI_VAR_BUF_SIZE),
>> +				 &memory);
>> +	if (ret != EFI_SUCCESS)
>> +		return ret;
>> +	efi_var_buf = (struct efi_var_file *)(uintptr_t)memory;
>> +	memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE);
>> +	efi_var_buf->magic = EFI_VAR_FILE_MAGIC;
>> +	efi_var_buf->length = (uintptr_t)efi_var_buf->var -
>> +			      (uintptr_t)efi_var_buf;
>> +	/* crc32 for 0 bytes = 0 */
>> +
>> +	ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
>> +			       efi_var_mem_notify_exit_boot_services, NULL,
>> +			       NULL, &event);
>> +	if (ret != EFI_SUCCESS)
>> +		return ret;
>> +	ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK,
>> +			       efi_var_mem_notify_virtual_address_map, NULL,
>> +			       NULL, &event);
>> +	if (ret != EFI_SUCCESS)
>> +		return ret;
>> +	return ret;
>> +}
>> --
>> 2.25.1
Punit Agrawal March 30, 2020, 10:50 a.m. UTC | #3
Heinrich Schuchardt <xypron.glpk at gmx.de> writes:

> On 3/27/20 9:09 AM, Punit Agrawal wrote:
>> Heinrich Schuchardt <xypron.glpk at gmx.de> writes:
>>
>>> Saving UEFI variable as encoded U-Boot environment variables does not allow
>>> support at runtime.
>>>
>>> Provide functions to manage a memory buffer with UEFI variables.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>>   include/efi_variable.h             |  16 ++
>>>   lib/efi_loader/Makefile            |   1 +
>>>   lib/efi_loader/efi_variables_mem.c | 317 +++++++++++++++++++++++++++++
>>>   3 files changed, 334 insertions(+)
>>>   create mode 100644 lib/efi_loader/efi_variables_mem.c
>>
>> [...]
>>
>>> diff --git a/lib/efi_loader/efi_variables_mem.c b/lib/efi_loader/efi_variables_mem.c
>>> new file mode 100644
>>> index 0000000000..f70cc65f8b
>>> --- /dev/null
>>> +++ b/lib/efi_loader/efi_variables_mem.c
>>> @@ -0,0 +1,317 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * File interface for UEFI variables
>>> + *
>>> + * Copyright (c) 2020, Heinrich Schuchardt
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <efi_loader.h>
>>> +#include <efi_variable.h>
>>> +#include <u-boot/crc.h>
>>> +
>>> +static struct efi_var_file __efi_runtime_data *efi_var_buf;
>>> +static struct efi_var_entry __efi_runtime_data *efi_current_var;
>>> +
>>> +/**
>>> + * memcpy() - copy memory area
>>> + *
>>> + * At runtime memcpy() is not available.
>>> + *
>>> + * @dest:	destination buffer
>>> + * @src:	source buffer
>>> + * @n:		number of bytes to copy
>>> + * Return:	pointer to destination buffer
>>> + */
>>> +void __efi_runtime efi_var_mem_memcpy(void *dest, const void *src, size_t n)
>>> +{
>>> +	u8 *d = dest;
>>> +	const u8 *s = src;
>>> +
>>> +	for (; n; --n)
>>> +		*d++ = *s++;
>>> +}
>>> +
>>> +/**
>>> + * efi_var_mem_compare() - compare GUID and name with a variable
>>> + *
>>> + * @var:	variable to compare
>>> + * @guid:	GUID to compare
>>> + * @name:	variable name to compare
>>> + * @next:	pointer to next variable
>>> + * Return:	true if match
>>> + */
>>> +static bool __efi_runtime
>>> +efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid,
>>> +		    const u16 *name, struct efi_var_entry **next)
>>> +{
>>> +	int i;
>>> +	u8 *guid1, *guid2;
>>> +	const u16 *data, *var_name;
>>> +	bool match = true;
>>> +
>>> +	for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0;
>>> +	     i < sizeof(efi_guid_t) && match; ++i)
>>> +		match = (guid1[i] == guid2[i]);
>>> +
>>> +	for (data = var->name, var_name = name;; ++data, ++var_name) {
>>> +		if (match)
>>> +			match = (*data == *var_name);
>>> +		if (!*data)
>>> +			break;
>>> +	}
>>
>> Don't roll out two different versions of memcmp() - make it a generic
>> helper and use it here.
>
> memcmp() has many architecture specific implementations. Currently all
> of these are gone at UEFI runtime.
>
> The loop above does not only do comparison but also seeks the end of the
> u16 string.

... which is needed for iteration over the variables. Thanks for
pointing this out.

One way to avoid the complexity would be keep track of the variable
length in efi_var_entry. Same applies for the "value" as well. That ways
you can skip to the next variable without having to increment two bytes
at a time.

>>
>>> +
>>> +	++data;
>>> +
>>> +	if (next)
>>> +		*next = (struct efi_var_entry *)
>>> +			ALIGN((uintptr_t)data + var->length, 8);
>>
>> Also, instead of implementing iteration via carrying the iterator
>> around, the readability of patches would be improved if this was done as
>> a simple loop outside of this function.
>>
>> If for some reason, this is considered to be better, please add it as a
>> comment in your next version.
>
> I could extract a function for comparing two u16 strings and return the
> end of the first string if they match and NULL otherwise.
>
> Would this match your expectations?

Going by the function name, I was expecting something along the lines of
-

    return !memcmp(guid1, guid2, 16) && !memcmp(var->name, name, var->length);
    
depending on whether length is tracking bytes used.

Hope that makes sense.

Punit

[...]
diff mbox series

Patch

diff --git a/include/efi_variable.h b/include/efi_variable.h
index fb8294fc2e..037d268085 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -33,4 +33,20 @@  efi_status_t efi_var_to_file(void);

 efi_status_t efi_var_from_file(void);

+void efi_var_mem_memcpy(void *dest, const void *src, size_t n);
+
+efi_status_t efi_var_mem_init(void);
+
+struct efi_var_entry *efi_var_mem_find(const efi_guid_t *guid, const u16 *name,
+				       struct efi_var_entry **next);
+
+void efi_var_mem_del(struct efi_var_entry *var);
+
+efi_status_t efi_var_mem_ins(u16 *variable_name,
+			     const efi_guid_t *vendor, u32 attributes,
+			     const efi_uintn_t size1, const void *data1,
+			     const efi_uintn_t size2, const void *data2);
+
+u64 efi_var_mem_free(void);
+
 #endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 621a767ab3..14b210e189 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -36,6 +36,7 @@  obj-y += efi_setup.o
 obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
 obj-y += efi_variable.o
 obj-y += efi_variables_file.o
+obj-y += efi_variables_mem.o
 obj-y += efi_watchdog.o
 obj-$(CONFIG_LCD) += efi_gop.o
 obj-$(CONFIG_DM_VIDEO) += efi_gop.o
diff --git a/lib/efi_loader/efi_variables_mem.c b/lib/efi_loader/efi_variables_mem.c
new file mode 100644
index 0000000000..f70cc65f8b
--- /dev/null
+++ b/lib/efi_loader/efi_variables_mem.c
@@ -0,0 +1,317 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * File interface for UEFI variables
+ *
+ * Copyright (c) 2020, Heinrich Schuchardt
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+#include <efi_variable.h>
+#include <u-boot/crc.h>
+
+static struct efi_var_file __efi_runtime_data *efi_var_buf;
+static struct efi_var_entry __efi_runtime_data *efi_current_var;
+
+/**
+ * memcpy() - copy memory area
+ *
+ * At runtime memcpy() is not available.
+ *
+ * @dest:	destination buffer
+ * @src:	source buffer
+ * @n:		number of bytes to copy
+ * Return:	pointer to destination buffer
+ */
+void __efi_runtime efi_var_mem_memcpy(void *dest, const void *src, size_t n)
+{
+	u8 *d = dest;
+	const u8 *s = src;
+
+	for (; n; --n)
+		*d++ = *s++;
+}
+
+/**
+ * efi_var_mem_compare() - compare GUID and name with a variable
+ *
+ * @var:	variable to compare
+ * @guid:	GUID to compare
+ * @name:	variable name to compare
+ * @next:	pointer to next variable
+ * Return:	true if match
+ */
+static bool __efi_runtime
+efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid,
+		    const u16 *name, struct efi_var_entry **next)
+{
+	int i;
+	u8 *guid1, *guid2;
+	const u16 *data, *var_name;
+	bool match = true;
+
+	for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0;
+	     i < sizeof(efi_guid_t) && match; ++i)
+		match = (guid1[i] == guid2[i]);
+
+	for (data = var->name, var_name = name;; ++data, ++var_name) {
+		if (match)
+			match = (*data == *var_name);
+		if (!*data)
+			break;
+	}
+
+	++data;
+
+	if (next)
+		*next = (struct efi_var_entry *)
+			ALIGN((uintptr_t)data + var->length, 8);
+
+	return match;
+}
+
+/**
+ * efi_var_mem_find() - find a variable in the list
+ *
+ * @guid:	GUID of the variable
+ * @name:	name of the variable
+ * @next:	on exit pointer to the next variable after the found one
+ * Return:	found variable
+ */
+struct efi_var_entry __efi_runtime
+*efi_var_mem_find(const efi_guid_t *guid, const u16 *name,
+		  struct efi_var_entry **next)
+{
+	struct efi_var_entry *var, *last;
+
+	last = (struct efi_var_entry *)
+	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
+
+	if (!*name) {
+		if (next) {
+			*next = efi_var_buf->var;
+			if (*next >= last)
+				*next = NULL;
+		}
+		return NULL;
+	}
+	if (efi_current_var &&
+	    efi_var_mem_compare(efi_current_var, guid, name, next)) {
+		if (next && *next >= last)
+			*next = NULL;
+		return efi_current_var;
+	}
+
+	var = efi_var_buf->var;
+	if (var < last) {
+		for (; var;) {
+			struct efi_var_entry *pos;
+			bool match;
+
+			match = efi_var_mem_compare(var, guid, name, &pos);
+			if (pos >= last)
+				pos = NULL;
+			if (match) {
+				if (next)
+					*next = pos;
+				return var;
+			}
+			var = pos;
+		}
+	}
+	if (next)
+		*next = NULL;
+	return NULL;
+}
+
+/**
+ * efi_var_mem_del() - delete a variable from the list of variables
+ *
+ * @var:	variable to delete
+ */
+void __efi_runtime efi_var_mem_del(struct efi_var_entry *var)
+{
+	u16 *data = var->name;
+	struct efi_var_entry *next, *last;
+	u64 *from, *to;
+
+	if (!var)
+		return;
+
+	last = (struct efi_var_entry *)
+	       ((uintptr_t)efi_var_buf + efi_var_buf->length);
+	if (var < efi_current_var)
+		efi_current_var = NULL;
+
+	for (data = var->name; *data; ++data)
+		;
+	++data;
+	next = (struct efi_var_entry *)
+	       ALIGN((uintptr_t)data + var->length, 8);
+	efi_var_buf->length -= (uintptr_t)next - (uintptr_t)var;
+
+	for (to = (u64 *)var, from = (u64 *)next; from < (u64 *)last;
+	     ++to, ++from)
+		*to = *from;
+	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
+				   efi_var_buf->length -
+				   sizeof(struct efi_var_file));
+}
+
+/**
+ * efi_var_mem_ins() - append a variable to the list of variables
+ *
+ * The variable is appended without checking if a variable of the same name
+ * already exists. The two data buffers are concatenated.
+ *
+ * @name:	variable name
+ * @vendor:	GUID
+ * @attributes:	variable attributes
+ * @size1:	size of the first data buffer
+ * @data1:	first data buffer
+ * @size2:	size of the second data field
+ * @data2:	second data buffer
+ * Result:	status code
+ */
+efi_status_t __efi_runtime efi_var_mem_ins(
+				u16 *variable_name,
+				const efi_guid_t *vendor, u32 attributes,
+				const efi_uintn_t size1, const void *data1,
+				const efi_uintn_t size2, const void *data2)
+{
+	u16 *data;
+	struct efi_var_entry *var;
+	u32 var_name_len;
+
+	var = (struct efi_var_entry *)
+	      ((uintptr_t)efi_var_buf + efi_var_buf->length);
+	for (var_name_len = 0; variable_name[var_name_len]; ++var_name_len)
+		;
+	++var_name_len;
+	data = var->name + var_name_len;
+
+	if ((uintptr_t)data - (uintptr_t)efi_var_buf + size1 + size2 >
+	    EFI_VAR_BUF_SIZE)
+		return EFI_OUT_OF_RESOURCES;
+
+	var->attr = attributes;
+	var->length = size1 + size2;
+
+	efi_var_mem_memcpy(&var->guid, vendor, sizeof(efi_guid_t));
+	efi_var_mem_memcpy(var->name, variable_name,
+			   sizeof(u16) * var_name_len);
+	efi_var_mem_memcpy(data, data1, size1);
+	efi_var_mem_memcpy((u8 *)data + size1, data2, size2);
+
+	var = (struct efi_var_entry *)
+	      ALIGN((uintptr_t)data + var->length, 8);
+	efi_var_buf->length = (uintptr_t)var - (uintptr_t)efi_var_buf;
+	efi_var_buf->crc32 = crc32(0, (u8 *)efi_var_buf->var,
+				   efi_var_buf->length -
+				   sizeof(struct efi_var_file));
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * efi_var_mem_free() - determine free memory for variables
+ *
+ * Return:	maximum data size plus variable name size
+ */
+u64 __efi_runtime efi_var_mem_free(void)
+{
+	return EFI_VAR_BUF_SIZE - efi_var_buf->length -
+	       sizeof(struct efi_var_entry);
+}
+
+/**
+ * efi_var_mem_bs_del() - delete boot service only variables
+ */
+static void efi_var_mem_bs_del(void)
+{
+	struct efi_var_entry *var = efi_var_buf->var;
+
+	for (;;) {
+		struct efi_var_entry *last;
+
+		last = (struct efi_var_entry *)
+		       ((uintptr_t)efi_var_buf + efi_var_buf->length);
+		if (var >= last)
+			break;
+		if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) {
+			u16 *data;
+
+			/* skip variable */
+			for (data = var->name; *data; ++data)
+				;
+			++data;
+			var = (struct efi_var_entry *)
+			      ALIGN((uintptr_t)data + var->length, 8);
+		} else {
+			/* delete variable */
+			efi_var_mem_del(var);
+		}
+	}
+}
+
+/**
+ * efi_var_mem_notify_exit_boot_services() - ExitBootService callback
+ *
+ * @event:	callback event
+ * @context:	callback context
+ */
+static void EFIAPI __efi_runtime
+efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context)
+{
+	/* Delete boot service only variables */
+	efi_var_mem_bs_del();
+}
+
+/**
+ * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback
+ *
+ * @event:	callback event
+ * @context:	callback context
+ */
+static void EFIAPI __efi_runtime
+efi_var_mem_notify_virtual_address_map(struct efi_event *event, void *context)
+{
+	efi_convert_pointer(0, (void **)&efi_var_buf);
+}
+
+/**
+ * efi_var_mem_init() - set-up variable list
+ *
+ * Return:	status code
+ */
+efi_status_t efi_var_mem_init(void)
+{
+	u64 memory;
+	efi_status_t ret;
+	struct efi_event *event;
+
+	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+				 EFI_RUNTIME_SERVICES_DATA,
+				 efi_size_in_pages(EFI_VAR_BUF_SIZE),
+				 &memory);
+	if (ret != EFI_SUCCESS)
+		return ret;
+	efi_var_buf = (struct efi_var_file *)(uintptr_t)memory;
+	memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE);
+	efi_var_buf->magic = EFI_VAR_FILE_MAGIC;
+	efi_var_buf->length = (uintptr_t)efi_var_buf->var -
+			      (uintptr_t)efi_var_buf;
+	/* crc32 for 0 bytes = 0 */
+
+	ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+			       efi_var_mem_notify_exit_boot_services, NULL,
+			       NULL, &event);
+	if (ret != EFI_SUCCESS)
+		return ret;
+	ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK,
+			       efi_var_mem_notify_virtual_address_map, NULL,
+			       NULL, &event);
+	if (ret != EFI_SUCCESS)
+		return ret;
+	return ret;
+}