diff mbox series

[U-Boot,v3,3/7] efi_loader: variable: split UEFI variables from U-Boot environment

Message ID 20190604065211.15907-4-takahiro.akashi@linaro.org
State Superseded
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: non-volatile variables support | expand

Commit Message

AKASHI Takahiro June 4, 2019, 6:52 a.m. UTC
UEFI volatile variables are managed in efi_var_htab while UEFI non-volatile
variables are in efi_nv_var_htab. At every SetVariable API, env_efi_save()
will also be called to save data cache (hash table) to persistent storage.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_loader/Kconfig        |  10 +
 lib/efi_loader/efi_variable.c | 342 ++++++++++++++++++++++++++--------
 2 files changed, 275 insertions(+), 77 deletions(-)

Comments

Heinrich Schuchardt June 4, 2019, 9:31 p.m. UTC | #1
On 6/4/19 8:52 AM, AKASHI Takahiro wrote:
> UEFI volatile variables are managed in efi_var_htab while UEFI non-volatile
> variables are in efi_nv_var_htab. At every SetVariable API, env_efi_save()
> will also be called to save data cache (hash table) to persistent storage.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   lib/efi_loader/Kconfig        |  10 +
>   lib/efi_loader/efi_variable.c | 342 ++++++++++++++++++++++++++--------
>   2 files changed, 275 insertions(+), 77 deletions(-)
>
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index cd5436c576b1..8bf4b1754d06 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -18,6 +18,16 @@ config EFI_LOADER
>
>   if EFI_LOADER
>
> +choice
> +	prompt "Select variables storage"
> +	default EFI_VARIABLE_USE_ENV
> +
> +config EFI_VARIABLE_USE_ENV
> +	bool "Same device as U-Boot environment"
> +	select ENV_EFI
> +
> +endchoice
> +
>   config EFI_GET_TIME
>   	bool "GetTime() runtime service"
>   	depends on DM_RTC
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index e56053194dae..d9887be938c2 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -48,6 +48,66 @@
>    * converted to utf16?
>    */
>
> +/*
> + * We will maintain two variable database: one for volatile variables,
> + * the other for non-volatile variables. The former exists only in memory
> + * and will go away at re-boot. The latter is currently backed up by the same
> + * device as U-Boot environment and also works as variables cache.
> + *
> + *	struct hsearch_data efi_var_htab
> + *	struct hsearch_data efi_nv_var_htab
> + */
> +
> +static char *env_efi_get(const char *name, bool is_non_volatile)
> +{
> +	struct hsearch_data *htab;
> +	ENTRY e, *ep;
> +
> +	/* WATCHDOG_RESET(); */
> +
> +	if (is_non_volatile)
> +		htab = &efi_nv_var_htab;
> +	else
> +		htab = &efi_var_htab;
> +
> +	e.key   = name;
> +	e.data  = NULL;
> +	hsearch_r(e, FIND, &ep, htab, 0);
> +
> +	return ep ? ep->data : NULL;
> +}
> +
> +static int env_efi_set(const char *name, const char *value,
> +		       bool is_non_volatile)
> +{
> +	struct hsearch_data *htab;
> +	ENTRY e, *ep;
> +	int ret;
> +
> +	if (is_non_volatile)
> +		htab = &efi_nv_var_htab;
> +	else
> +		htab = &efi_var_htab;
> +
> +	/* delete */
> +	if (!value || *value == '\0') {
> +		ret = hdelete_r(name, htab, H_PROGRAMMATIC);
> +		return !ret;
> +	}
> +
> +	/* set */
> +	e.key   = name;
> +	e.data  = (char *)value;
> +	hsearch_r(e, ENTER, &ep, htab, H_PROGRAMMATIC);
> +	if (!ep) {
> +		printf("## Error inserting \"%s\" variable, errno=%d\n",
> +		       name, errno);
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
>   #define PREFIX_LEN (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx_"))
>
>   /**
> @@ -147,24 +207,12 @@ static const char *parse_attr(const char *str, u32 *attrp)
>   	return str;
>   }
>
> -/**
> - * efi_efi_get_variable() - retrieve value of a UEFI variable
> - *
> - * This function implements the GetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @variable_name:	name of the variable
> - * @vendor:		vendor GUID
> - * @attributes:		attributes of the variable
> - * @data_size:		size of the buffer to which the variable value is copied
> - * @data:		buffer to which the variable value is copied
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> -				     const efi_guid_t *vendor, u32 *attributes,
> -				     efi_uintn_t *data_size, void *data)
> +static
> +efi_status_t EFIAPI efi_get_variable_common(u16 *variable_name,
> +					    const efi_guid_t *vendor,
> +					    u32 *attributes,
> +					    efi_uintn_t *data_size, void *data,
> +					    bool is_non_volatile)
>   {
>   	char *native_name;
>   	efi_status_t ret;
> @@ -172,22 +220,19 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>   	const char *val, *s;
>   	u32 attr;
>
> -	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> -		  data_size, data);
> -
>   	if (!variable_name || !vendor || !data_size)
>   		return EFI_EXIT(EFI_INVALID_PARAMETER);
>
>   	ret = efi_to_native(&native_name, variable_name, vendor);
>   	if (ret)
> -		return EFI_EXIT(ret);
> +		return ret;
>
>   	EFI_PRINT("get '%s'\n", native_name);
>
> -	val = env_get(native_name);
> +	val = env_efi_get(native_name, is_non_volatile);
>   	free(native_name);
>   	if (!val)
> -		return EFI_EXIT(EFI_NOT_FOUND);
> +		return EFI_NOT_FOUND;
>
>   	val = parse_attr(val, &attr);
>
> @@ -198,7 +243,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>
>   		/* number of hexadecimal digits must be even */
>   		if (len & 1)
> -			return EFI_EXIT(EFI_DEVICE_ERROR);
> +			return EFI_DEVICE_ERROR;
>
>   		/* two characters per byte: */
>   		len /= 2;
> @@ -210,10 +255,10 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>   		}
>
>   		if (!data)
> -			return EFI_EXIT(EFI_INVALID_PARAMETER);
> +			return EFI_INVALID_PARAMETER;
>
>   		if (hex2bin(data, s, len))
> -			return EFI_EXIT(EFI_DEVICE_ERROR);
> +			return EFI_DEVICE_ERROR;
>
>   		EFI_PRINT("got value: \"%s\"\n", s);
>   	} else if ((s = prefix(val, "(utf8)"))) {
> @@ -227,7 +272,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>   		}
>
>   		if (!data)
> -			return EFI_EXIT(EFI_INVALID_PARAMETER);
> +			return EFI_INVALID_PARAMETER;
>
>   		memcpy(data, s, len);
>   		((char *)data)[len] = '\0';
> @@ -235,13 +280,67 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
>   		EFI_PRINT("got value: \"%s\"\n", (char *)data);
>   	} else {
>   		EFI_PRINT("invalid value: '%s'\n", val);
> -		return EFI_EXIT(EFI_DEVICE_ERROR);
> +		return EFI_DEVICE_ERROR;
>   	}
>
>   out:
>   	if (attributes)
>   		*attributes = attr & EFI_VARIABLE_MASK;
>
> +	return ret;
> +}
> +
> +static
> +efi_status_t EFIAPI efi_get_volatile_variable(u16 *variable_name,
> +					      const efi_guid_t *vendor,
> +					      u32 *attributes,
> +					      efi_uintn_t *data_size,
> +					      void *data)
> +{
> +	return efi_get_variable_common(variable_name, vendor, attributes,
> +				       data_size, data, false);
> +}
> +
> +efi_status_t EFIAPI efi_get_nonvolatile_variable(u16 *variable_name,
> +						 const efi_guid_t *vendor,
> +						 u32 *attributes,
> +						 efi_uintn_t *data_size,
> +						 void *data)
> +{
> +	return efi_get_variable_common(variable_name, vendor, attributes,
> +				       data_size, data, true);
> +}
> +
> +/**
> + * efi_efi_get_variable() - retrieve value of a UEFI variable
> + *
> + * This function implements the GetVariable runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer to which the variable value is copied
> + * @data:		buffer to which the variable value is copied
> + * Return:		status code
> + */
> +efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> +				     const efi_guid_t *vendor, u32 *attributes,
> +				     efi_uintn_t *data_size, void *data)
> +{
> +	efi_status_t ret;
> +
> +	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> +		  data_size, data);
> +
> +	ret = efi_get_volatile_variable(variable_name, vendor, attributes,
> +					data_size, data);
> +	if (ret == EFI_NOT_FOUND)
> +		ret = efi_get_nonvolatile_variable(variable_name, vendor,
> +						   attributes, data_size, data);
> +
>   	return EFI_EXIT(ret);
>   }
>
> @@ -331,7 +430,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
>   					       u16 *variable_name,
>   					       const efi_guid_t *vendor)
>   {
> -	char *native_name, *variable;
> +	char *native_name, *variable, *tmp_list, *merged_list;
>   	ssize_t name_len, list_len;
>   	char regex[256];
>   	char * const regexlist[] = {regex};
> @@ -387,10 +486,39 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
>   		efi_cur_variable = NULL;
>
>   		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> -		list_len = hexport_r(&env_htab, '\n',
> +		list_len = hexport_r(&efi_var_htab, '\n',
>   				     H_MATCH_REGEX | H_MATCH_KEY,
>   				     &efi_variables_list, 0, 1, regexlist);
> -		/* 1 indicates that no match was found */
> +		/*
> +		 * Note: '1' indicates that nothing is matched
> +		 */
> +		if (list_len <= 1) {
> +			free(efi_variables_list);
> +			efi_variables_list = NULL;
> +			list_len = hexport_r(&efi_nv_var_htab, '\n',
> +					     H_MATCH_REGEX | H_MATCH_KEY,
> +					     &efi_variables_list, 0, 1,
> +					     regexlist);
> +		} else {
> +			tmp_list = NULL;
> +			list_len = hexport_r(&efi_nv_var_htab, '\n',
> +					     H_MATCH_REGEX | H_MATCH_KEY,
> +					     &tmp_list, 0, 1,
> +					     regexlist);
> +			if (list_len <= 1) {
> +				list_len = 2; /* don't care actual number */
> +			} else {
> +				/* merge two variables lists */
> +				merged_list = malloc(strlen(efi_variables_list)
> +							+ strlen(tmp_list) + 1);
> +				strcpy(merged_list, efi_variables_list);
> +				strcat(merged_list, tmp_list);
> +				free(efi_variables_list);
> +				free(tmp_list);
> +				efi_variables_list = merged_list;
> +			}
> +		}
> +
>   		if (list_len <= 1)
>   			return EFI_EXIT(EFI_NOT_FOUND);
>
> @@ -403,77 +531,71 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
>   	return EFI_EXIT(ret);
>   }
>
> -/**
> - * efi_efi_set_variable() - set value of a UEFI variable
> - *
> - * This function implements the SetVariable runtime service.
> - *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> - *
> - * @variable_name:	name of the variable
> - * @vendor:		vendor GUID
> - * @attributes:		attributes of the variable
> - * @data_size:		size of the buffer with the variable value
> - * @data:		buffer with the variable value
> - * Return:		status code
> - */
> -efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> -				     const efi_guid_t *vendor, u32 attributes,
> -				     efi_uintn_t data_size, const void *data)
> +static
> +efi_status_t EFIAPI efi_set_variable_common(u16 *variable_name,
> +					    const efi_guid_t *vendor,
> +					    u32 attributes,
> +					    efi_uintn_t data_size,
> +					    const void *data,
> +					    bool is_non_volatile)
>   {
>   	char *native_name = NULL, *val = NULL, *s;
> -	efi_status_t ret = EFI_SUCCESS;
> +	efi_uintn_t size;
>   	u32 attr;
> -
> -	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> -		  data_size, data);
> +	efi_status_t ret = EFI_SUCCESS;
>
>   	/* TODO: implement APPEND_WRITE */
>   	if (!variable_name || !vendor ||
>   	    (attributes & EFI_VARIABLE_APPEND_WRITE)) {
>   		ret = EFI_INVALID_PARAMETER;
> -		goto out;
> +		goto err;
>   	}
>
>   	ret = efi_to_native(&native_name, variable_name, vendor);
>   	if (ret)
> -		goto out;
> +		goto err;
>
>   #define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
>
> -	if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
> -		/* delete the variable: */
> -		env_set(native_name, NULL);
> -		ret = EFI_SUCCESS;
> -		goto out;
> +	/* check if a variable exists */
> +	size = 0;
> +	ret = EFI_CALL(efi_get_variable(variable_name, vendor, &attr,
> +					&size, NULL));
> +	if (ret == EFI_BUFFER_TOO_SMALL) {
> +		if ((is_non_volatile && !(attr & EFI_VARIABLE_NON_VOLATILE)) ||
> +		    (!is_non_volatile && (attr & EFI_VARIABLE_NON_VOLATILE))) {
> +			ret = EFI_INVALID_PARAMETER;
> +			goto err;
> +		}
>   	}
>
> -	val = env_get(native_name);
> -	if (val) {
> -		parse_attr(val, &attr);
> -
> -		/* We should not free val */
> -		val = NULL;
> -		if (attr & READ_ONLY) {
> -			ret = EFI_WRITE_PROTECTED;
> +	/* delete a variable */
> +	if (data_size == 0 || !(attributes & ACCESS_ATTR)) {
> +		if (size) {
> +			if (attr & READ_ONLY) {
> +				ret = EFI_WRITE_PROTECTED;
> +				goto err;
> +			}
>   			goto out;
>   		}
> +		ret = EFI_SUCCESS;
> +		goto err; /* not error, but nothing to do */
> +	}
>
> +	/* create/modify a variable */
> +	if (size && attr != attributes) {
>   		/*
>   		 * attributes won't be changed
>   		 * TODO: take care of APPEND_WRITE once supported
>   		 */
> -		if (attr != attributes) {
> -			ret = EFI_INVALID_PARAMETER;
> -			goto out;
> -		}
> +		ret = EFI_INVALID_PARAMETER;
> +		goto err;
>   	}
>
>   	val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
>   	if (!val) {
>   		ret = EFI_OUT_OF_RESOURCES;
> -		goto out;
> +		goto err;
>   	}
>
>   	s = val;
> @@ -487,7 +609,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
>   		       EFI_VARIABLE_RUNTIME_ACCESS);
>   	s += sprintf(s, "{");
>   	while (attributes) {
> -		u32 attr = 1 << (ffs(attributes) - 1);
> +		attr = 1 << (ffs(attributes) - 1);
>
>   		if (attr == EFI_VARIABLE_NON_VOLATILE)
>   			s += sprintf(s, "nv");
> @@ -509,12 +631,78 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
>
>   	EFI_PRINT("setting: %s=%s\n", native_name, val);
>
> -	if (env_set(native_name, val))
> +out:
> +	ret = EFI_SUCCESS;
> +	if (env_efi_set(native_name, val, is_non_volatile))
>   		ret = EFI_DEVICE_ERROR;
>
> -out:
> +err:
>   	free(native_name);
>   	free(val);
>
> +	return ret;
> +}
> +
> +static
> +efi_status_t EFIAPI efi_set_volatile_variable(u16 *variable_name,

Once you have implemented this we can make the variable service
available at runtime. So I suggest to define everything here as __runtime.

Why do you any of these three functions (efi_set_volatile_variable(),
efi_set_nonvolatile_variable(), efi_set_nonvolatile_variable()). Just
use an if based on attributes in efi_set_variable() to call
env_efi_save, when a non-volatile function is changed.

What is the advantage of having two separate RAM stores? Can't the save
function sort out what to save and what not to save according to the NV
flag?

> +					      const efi_guid_t *vendor,
> +					      u32 attributes,
> +					      efi_uintn_t data_size,
> +					      const void *data)
> +{
> +	return efi_set_variable_common(variable_name, vendor, attributes,
> +				       data_size, data, false);
> +}
> +
> +efi_status_t EFIAPI efi_set_nonvolatile_variable(u16 *variable_name,
> +						 const efi_guid_t *vendor,
> +						 u32 attributes,
> +						 efi_uintn_t data_size,
> +						 const void *data)
> +{
> +	efi_status_t ret;
> +
> +	ret = efi_set_variable_common(variable_name, vendor, attributes,
> +				      data_size, data, true);
> +	if (ret == EFI_SUCCESS)
> +		/* FIXME: what if save failed? */
> +		if (env_efi_save())

Somewhere we need the ability to switch between different backends. Will
that be in env_efi_save()?

> +			ret = EFI_DEVICE_ERROR;
> +
> +	return ret;
> +}
> +
> +/**
> + * efi_efi_set_variable() - set value of a UEFI variable
> + *
> + * This function implements the SetVariable runtime service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @variable_name:	name of the variable
> + * @vendor:		vendor GUID
> + * @attributes:		attributes of the variable
> + * @data_size:		size of the buffer with the variable value
> + * @data:		buffer with the variable value
> + * Return:		status code
> + */
> +efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> +				     const efi_guid_t *vendor, u32 attributes,
> +				     efi_uintn_t data_size, const void *data)
> +{
> +	efi_status_t ret;
> +
> +	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> +		  data_size, data);
> +
> +	if (attributes & EFI_VARIABLE_NON_VOLATILE)
> +		ret = efi_set_nonvolatile_variable(variable_name, vendor,
> +						   attributes,
> +						   data_size, data);
> +	else
> +		ret = efi_set_volatile_variable(variable_name, vendor,
> +						attributes, data_size, data);
> +
>   	return EFI_EXIT(ret);
>   }
>
AKASHI Takahiro June 5, 2019, 12:48 a.m. UTC | #2
On Tue, Jun 04, 2019 at 11:31:24PM +0200, Heinrich Schuchardt wrote:
> On 6/4/19 8:52 AM, AKASHI Takahiro wrote:
> >UEFI volatile variables are managed in efi_var_htab while UEFI non-volatile
> >variables are in efi_nv_var_htab. At every SetVariable API, env_efi_save()
> >will also be called to save data cache (hash table) to persistent storage.
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >---
> >  lib/efi_loader/Kconfig        |  10 +
> >  lib/efi_loader/efi_variable.c | 342 ++++++++++++++++++++++++++--------
> >  2 files changed, 275 insertions(+), 77 deletions(-)
> >
> >diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> >index cd5436c576b1..8bf4b1754d06 100644
> >--- a/lib/efi_loader/Kconfig
> >+++ b/lib/efi_loader/Kconfig
> >@@ -18,6 +18,16 @@ config EFI_LOADER
> >
> >  if EFI_LOADER
> >
> >+choice
> >+	prompt "Select variables storage"
> >+	default EFI_VARIABLE_USE_ENV
> >+
> >+config EFI_VARIABLE_USE_ENV
> >+	bool "Same device as U-Boot environment"
> >+	select ENV_EFI
> >+
> >+endchoice
> >+
> >  config EFI_GET_TIME
> >  	bool "GetTime() runtime service"
> >  	depends on DM_RTC
> >diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> >index e56053194dae..d9887be938c2 100644
> >--- a/lib/efi_loader/efi_variable.c
> >+++ b/lib/efi_loader/efi_variable.c
> >@@ -48,6 +48,66 @@
> >   * converted to utf16?
> >   */
> >
> >+/*
> >+ * We will maintain two variable database: one for volatile variables,
> >+ * the other for non-volatile variables. The former exists only in memory
> >+ * and will go away at re-boot. The latter is currently backed up by the same
> >+ * device as U-Boot environment and also works as variables cache.
> >+ *
> >+ *	struct hsearch_data efi_var_htab
> >+ *	struct hsearch_data efi_nv_var_htab
> >+ */
> >+
> >+static char *env_efi_get(const char *name, bool is_non_volatile)
> >+{
> >+	struct hsearch_data *htab;
> >+	ENTRY e, *ep;
> >+
> >+	/* WATCHDOG_RESET(); */
> >+
> >+	if (is_non_volatile)
> >+		htab = &efi_nv_var_htab;
> >+	else
> >+		htab = &efi_var_htab;
> >+
> >+	e.key   = name;
> >+	e.data  = NULL;
> >+	hsearch_r(e, FIND, &ep, htab, 0);
> >+
> >+	return ep ? ep->data : NULL;
> >+}
> >+
> >+static int env_efi_set(const char *name, const char *value,
> >+		       bool is_non_volatile)
> >+{
> >+	struct hsearch_data *htab;
> >+	ENTRY e, *ep;
> >+	int ret;
> >+
> >+	if (is_non_volatile)
> >+		htab = &efi_nv_var_htab;
> >+	else
> >+		htab = &efi_var_htab;
> >+
> >+	/* delete */
> >+	if (!value || *value == '\0') {
> >+		ret = hdelete_r(name, htab, H_PROGRAMMATIC);
> >+		return !ret;
> >+	}
> >+
> >+	/* set */
> >+	e.key   = name;
> >+	e.data  = (char *)value;
> >+	hsearch_r(e, ENTER, &ep, htab, H_PROGRAMMATIC);
> >+	if (!ep) {
> >+		printf("## Error inserting \"%s\" variable, errno=%d\n",
> >+		       name, errno);
> >+		return 1;
> >+	}
> >+
> >+	return 0;
> >+}
> >+
> >  #define PREFIX_LEN (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx_"))
> >
> >  /**
> >@@ -147,24 +207,12 @@ static const char *parse_attr(const char *str, u32 *attrp)
> >  	return str;
> >  }
> >
> >-/**
> >- * efi_efi_get_variable() - retrieve value of a UEFI variable
> >- *
> >- * This function implements the GetVariable runtime service.
> >- *
> >- * See the Unified Extensible Firmware Interface (UEFI) specification for
> >- * details.
> >- *
> >- * @variable_name:	name of the variable
> >- * @vendor:		vendor GUID
> >- * @attributes:		attributes of the variable
> >- * @data_size:		size of the buffer to which the variable value is copied
> >- * @data:		buffer to which the variable value is copied
> >- * Return:		status code
> >- */
> >-efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >-				     const efi_guid_t *vendor, u32 *attributes,
> >-				     efi_uintn_t *data_size, void *data)
> >+static
> >+efi_status_t EFIAPI efi_get_variable_common(u16 *variable_name,
> >+					    const efi_guid_t *vendor,
> >+					    u32 *attributes,
> >+					    efi_uintn_t *data_size, void *data,
> >+					    bool is_non_volatile)
> >  {
> >  	char *native_name;
> >  	efi_status_t ret;
> >@@ -172,22 +220,19 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >  	const char *val, *s;
> >  	u32 attr;
> >
> >-	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> >-		  data_size, data);
> >-
> >  	if (!variable_name || !vendor || !data_size)
> >  		return EFI_EXIT(EFI_INVALID_PARAMETER);
> >
> >  	ret = efi_to_native(&native_name, variable_name, vendor);
> >  	if (ret)
> >-		return EFI_EXIT(ret);
> >+		return ret;
> >
> >  	EFI_PRINT("get '%s'\n", native_name);
> >
> >-	val = env_get(native_name);
> >+	val = env_efi_get(native_name, is_non_volatile);
> >  	free(native_name);
> >  	if (!val)
> >-		return EFI_EXIT(EFI_NOT_FOUND);
> >+		return EFI_NOT_FOUND;
> >
> >  	val = parse_attr(val, &attr);
> >
> >@@ -198,7 +243,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >
> >  		/* number of hexadecimal digits must be even */
> >  		if (len & 1)
> >-			return EFI_EXIT(EFI_DEVICE_ERROR);
> >+			return EFI_DEVICE_ERROR;
> >
> >  		/* two characters per byte: */
> >  		len /= 2;
> >@@ -210,10 +255,10 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >  		}
> >
> >  		if (!data)
> >-			return EFI_EXIT(EFI_INVALID_PARAMETER);
> >+			return EFI_INVALID_PARAMETER;
> >
> >  		if (hex2bin(data, s, len))
> >-			return EFI_EXIT(EFI_DEVICE_ERROR);
> >+			return EFI_DEVICE_ERROR;
> >
> >  		EFI_PRINT("got value: \"%s\"\n", s);
> >  	} else if ((s = prefix(val, "(utf8)"))) {
> >@@ -227,7 +272,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >  		}
> >
> >  		if (!data)
> >-			return EFI_EXIT(EFI_INVALID_PARAMETER);
> >+			return EFI_INVALID_PARAMETER;
> >
> >  		memcpy(data, s, len);
> >  		((char *)data)[len] = '\0';
> >@@ -235,13 +280,67 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >  		EFI_PRINT("got value: \"%s\"\n", (char *)data);
> >  	} else {
> >  		EFI_PRINT("invalid value: '%s'\n", val);
> >-		return EFI_EXIT(EFI_DEVICE_ERROR);
> >+		return EFI_DEVICE_ERROR;
> >  	}
> >
> >  out:
> >  	if (attributes)
> >  		*attributes = attr & EFI_VARIABLE_MASK;
> >
> >+	return ret;
> >+}
> >+
> >+static
> >+efi_status_t EFIAPI efi_get_volatile_variable(u16 *variable_name,
> >+					      const efi_guid_t *vendor,
> >+					      u32 *attributes,
> >+					      efi_uintn_t *data_size,
> >+					      void *data)
> >+{
> >+	return efi_get_variable_common(variable_name, vendor, attributes,
> >+				       data_size, data, false);
> >+}
> >+
> >+efi_status_t EFIAPI efi_get_nonvolatile_variable(u16 *variable_name,
> >+						 const efi_guid_t *vendor,
> >+						 u32 *attributes,
> >+						 efi_uintn_t *data_size,
> >+						 void *data)
> >+{
> >+	return efi_get_variable_common(variable_name, vendor, attributes,
> >+				       data_size, data, true);
> >+}
> >+
> >+/**
> >+ * efi_efi_get_variable() - retrieve value of a UEFI variable
> >+ *
> >+ * This function implements the GetVariable runtime service.
> >+ *
> >+ * See the Unified Extensible Firmware Interface (UEFI) specification for
> >+ * details.
> >+ *
> >+ * @variable_name:	name of the variable
> >+ * @vendor:		vendor GUID
> >+ * @attributes:		attributes of the variable
> >+ * @data_size:		size of the buffer to which the variable value is copied
> >+ * @data:		buffer to which the variable value is copied
> >+ * Return:		status code
> >+ */
> >+efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
> >+				     const efi_guid_t *vendor, u32 *attributes,
> >+				     efi_uintn_t *data_size, void *data)
> >+{
> >+	efi_status_t ret;
> >+
> >+	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
> >+		  data_size, data);
> >+
> >+	ret = efi_get_volatile_variable(variable_name, vendor, attributes,
> >+					data_size, data);
> >+	if (ret == EFI_NOT_FOUND)
> >+		ret = efi_get_nonvolatile_variable(variable_name, vendor,
> >+						   attributes, data_size, data);
> >+
> >  	return EFI_EXIT(ret);
> >  }
> >
> >@@ -331,7 +430,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
> >  					       u16 *variable_name,
> >  					       const efi_guid_t *vendor)
> >  {
> >-	char *native_name, *variable;
> >+	char *native_name, *variable, *tmp_list, *merged_list;
> >  	ssize_t name_len, list_len;
> >  	char regex[256];
> >  	char * const regexlist[] = {regex};
> >@@ -387,10 +486,39 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
> >  		efi_cur_variable = NULL;
> >
> >  		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
> >-		list_len = hexport_r(&env_htab, '\n',
> >+		list_len = hexport_r(&efi_var_htab, '\n',
> >  				     H_MATCH_REGEX | H_MATCH_KEY,
> >  				     &efi_variables_list, 0, 1, regexlist);
> >-		/* 1 indicates that no match was found */
> >+		/*
> >+		 * Note: '1' indicates that nothing is matched
> >+		 */
> >+		if (list_len <= 1) {
> >+			free(efi_variables_list);
> >+			efi_variables_list = NULL;
> >+			list_len = hexport_r(&efi_nv_var_htab, '\n',
> >+					     H_MATCH_REGEX | H_MATCH_KEY,
> >+					     &efi_variables_list, 0, 1,
> >+					     regexlist);
> >+		} else {
> >+			tmp_list = NULL;
> >+			list_len = hexport_r(&efi_nv_var_htab, '\n',
> >+					     H_MATCH_REGEX | H_MATCH_KEY,
> >+					     &tmp_list, 0, 1,
> >+					     regexlist);
> >+			if (list_len <= 1) {
> >+				list_len = 2; /* don't care actual number */
> >+			} else {
> >+				/* merge two variables lists */
> >+				merged_list = malloc(strlen(efi_variables_list)
> >+							+ strlen(tmp_list) + 1);
> >+				strcpy(merged_list, efi_variables_list);
> >+				strcat(merged_list, tmp_list);
> >+				free(efi_variables_list);
> >+				free(tmp_list);
> >+				efi_variables_list = merged_list;
> >+			}
> >+		}
> >+
> >  		if (list_len <= 1)
> >  			return EFI_EXIT(EFI_NOT_FOUND);
> >
> >@@ -403,77 +531,71 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
> >  	return EFI_EXIT(ret);
> >  }
> >
> >-/**
> >- * efi_efi_set_variable() - set value of a UEFI variable
> >- *
> >- * This function implements the SetVariable runtime service.
> >- *
> >- * See the Unified Extensible Firmware Interface (UEFI) specification for
> >- * details.
> >- *
> >- * @variable_name:	name of the variable
> >- * @vendor:		vendor GUID
> >- * @attributes:		attributes of the variable
> >- * @data_size:		size of the buffer with the variable value
> >- * @data:		buffer with the variable value
> >- * Return:		status code
> >- */
> >-efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> >-				     const efi_guid_t *vendor, u32 attributes,
> >-				     efi_uintn_t data_size, const void *data)
> >+static
> >+efi_status_t EFIAPI efi_set_variable_common(u16 *variable_name,
> >+					    const efi_guid_t *vendor,
> >+					    u32 attributes,
> >+					    efi_uintn_t data_size,
> >+					    const void *data,
> >+					    bool is_non_volatile)
> >  {
> >  	char *native_name = NULL, *val = NULL, *s;
> >-	efi_status_t ret = EFI_SUCCESS;
> >+	efi_uintn_t size;
> >  	u32 attr;
> >-
> >-	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> >-		  data_size, data);
> >+	efi_status_t ret = EFI_SUCCESS;
> >
> >  	/* TODO: implement APPEND_WRITE */
> >  	if (!variable_name || !vendor ||
> >  	    (attributes & EFI_VARIABLE_APPEND_WRITE)) {
> >  		ret = EFI_INVALID_PARAMETER;
> >-		goto out;
> >+		goto err;
> >  	}
> >
> >  	ret = efi_to_native(&native_name, variable_name, vendor);
> >  	if (ret)
> >-		goto out;
> >+		goto err;
> >
> >  #define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
> >
> >-	if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
> >-		/* delete the variable: */
> >-		env_set(native_name, NULL);
> >-		ret = EFI_SUCCESS;
> >-		goto out;
> >+	/* check if a variable exists */
> >+	size = 0;
> >+	ret = EFI_CALL(efi_get_variable(variable_name, vendor, &attr,
> >+					&size, NULL));
> >+	if (ret == EFI_BUFFER_TOO_SMALL) {
> >+		if ((is_non_volatile && !(attr & EFI_VARIABLE_NON_VOLATILE)) ||
> >+		    (!is_non_volatile && (attr & EFI_VARIABLE_NON_VOLATILE))) {
> >+			ret = EFI_INVALID_PARAMETER;
> >+			goto err;
> >+		}
> >  	}
> >
> >-	val = env_get(native_name);
> >-	if (val) {
> >-		parse_attr(val, &attr);
> >-
> >-		/* We should not free val */
> >-		val = NULL;
> >-		if (attr & READ_ONLY) {
> >-			ret = EFI_WRITE_PROTECTED;
> >+	/* delete a variable */
> >+	if (data_size == 0 || !(attributes & ACCESS_ATTR)) {
> >+		if (size) {
> >+			if (attr & READ_ONLY) {
> >+				ret = EFI_WRITE_PROTECTED;
> >+				goto err;
> >+			}
> >  			goto out;
> >  		}
> >+		ret = EFI_SUCCESS;
> >+		goto err; /* not error, but nothing to do */
> >+	}
> >
> >+	/* create/modify a variable */
> >+	if (size && attr != attributes) {
> >  		/*
> >  		 * attributes won't be changed
> >  		 * TODO: take care of APPEND_WRITE once supported
> >  		 */
> >-		if (attr != attributes) {
> >-			ret = EFI_INVALID_PARAMETER;
> >-			goto out;
> >-		}
> >+		ret = EFI_INVALID_PARAMETER;
> >+		goto err;
> >  	}
> >
> >  	val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
> >  	if (!val) {
> >  		ret = EFI_OUT_OF_RESOURCES;
> >-		goto out;
> >+		goto err;
> >  	}
> >
> >  	s = val;
> >@@ -487,7 +609,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> >  		       EFI_VARIABLE_RUNTIME_ACCESS);
> >  	s += sprintf(s, "{");
> >  	while (attributes) {
> >-		u32 attr = 1 << (ffs(attributes) - 1);
> >+		attr = 1 << (ffs(attributes) - 1);
> >
> >  		if (attr == EFI_VARIABLE_NON_VOLATILE)
> >  			s += sprintf(s, "nv");
> >@@ -509,12 +631,78 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> >
> >  	EFI_PRINT("setting: %s=%s\n", native_name, val);
> >
> >-	if (env_set(native_name, val))
> >+out:
> >+	ret = EFI_SUCCESS;
> >+	if (env_efi_set(native_name, val, is_non_volatile))
> >  		ret = EFI_DEVICE_ERROR;
> >
> >-out:
> >+err:
> >  	free(native_name);
> >  	free(val);
> >
> >+	return ret;
> >+}
> >+
> >+static
> >+efi_status_t EFIAPI efi_set_volatile_variable(u16 *variable_name,
> 
> Once you have implemented this we can make the variable service
> available at runtime. So I suggest to define everything here as __runtime.

Good question! I intended so when I started this work, but
we can't do that partly because bunch of code from U-Boot, which is not
part of RUNTIME_CODE, will be exercised and partly because all the entries
in a hash table are allocated by *malloc*, which are again not part of
RUNTIME_DATA.
It is quite painful to modify the code and data so that they are accessible
at UEFI runtime.

Instead, I implemented "caching" features for runtime access.
I'm going to post the patch set (as RFC) later today.

> Why do you any of these three functions (efi_set_volatile_variable(),
> efi_set_nonvolatile_variable(), efi_set_nonvolatile_variable()). Just
> use an if based on attributes in efi_set_variable() to call
> env_efi_save, when a non-volatile function is changed.

I don't get you point. Please clarify your comment.

> What is the advantage of having two separate RAM stores? Can't the save
> function sort out what to save and what not to save according to the NV
> flag?

Technically, we can do that, but it is nothing but inefficient.
Just FYI, EDK2 also maintains separate buffers.

> >+					      const efi_guid_t *vendor,
> >+					      u32 attributes,
> >+					      efi_uintn_t data_size,
> >+					      const void *data)
> >+{
> >+	return efi_set_variable_common(variable_name, vendor, attributes,
> >+				       data_size, data, false);
> >+}
> >+
> >+efi_status_t EFIAPI efi_set_nonvolatile_variable(u16 *variable_name,
> >+						 const efi_guid_t *vendor,
> >+						 u32 attributes,
> >+						 efi_uintn_t data_size,
> >+						 const void *data)
> >+{
> >+	efi_status_t ret;
> >+
> >+	ret = efi_set_variable_common(variable_name, vendor, attributes,
> >+				      data_size, data, true);
> >+	if (ret == EFI_SUCCESS)
> >+		/* FIXME: what if save failed? */
> >+		if (env_efi_save())
> 
> Somewhere we need the ability to switch between different backends. Will
> that be in env_efi_save()?

That is why I put the related code in "env."

Thanks,
-Takahiro Akashi

> >+			ret = EFI_DEVICE_ERROR;
> >+
> >+	return ret;
> >+}
> >+
> >+/**
> >+ * efi_efi_set_variable() - set value of a UEFI variable
> >+ *
> >+ * This function implements the SetVariable runtime service.
> >+ *
> >+ * See the Unified Extensible Firmware Interface (UEFI) specification for
> >+ * details.
> >+ *
> >+ * @variable_name:	name of the variable
> >+ * @vendor:		vendor GUID
> >+ * @attributes:		attributes of the variable
> >+ * @data_size:		size of the buffer with the variable value
> >+ * @data:		buffer with the variable value
> >+ * Return:		status code
> >+ */
> >+efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
> >+				     const efi_guid_t *vendor, u32 attributes,
> >+				     efi_uintn_t data_size, const void *data)
> >+{
> >+	efi_status_t ret;
> >+
> >+	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
> >+		  data_size, data);
> >+
> >+	if (attributes & EFI_VARIABLE_NON_VOLATILE)
> >+		ret = efi_set_nonvolatile_variable(variable_name, vendor,
> >+						   attributes,
> >+						   data_size, data);
> >+	else
> >+		ret = efi_set_volatile_variable(variable_name, vendor,
> >+						attributes, data_size, data);
> >+
> >  	return EFI_EXIT(ret);
> >  }
> >
>
diff mbox series

Patch

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index cd5436c576b1..8bf4b1754d06 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -18,6 +18,16 @@  config EFI_LOADER
 
 if EFI_LOADER
 
+choice
+	prompt "Select variables storage"
+	default EFI_VARIABLE_USE_ENV
+
+config EFI_VARIABLE_USE_ENV
+	bool "Same device as U-Boot environment"
+	select ENV_EFI
+
+endchoice
+
 config EFI_GET_TIME
 	bool "GetTime() runtime service"
 	depends on DM_RTC
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index e56053194dae..d9887be938c2 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -48,6 +48,66 @@ 
  * converted to utf16?
  */
 
+/*
+ * We will maintain two variable database: one for volatile variables,
+ * the other for non-volatile variables. The former exists only in memory
+ * and will go away at re-boot. The latter is currently backed up by the same
+ * device as U-Boot environment and also works as variables cache.
+ *
+ *	struct hsearch_data efi_var_htab
+ *	struct hsearch_data efi_nv_var_htab
+ */
+
+static char *env_efi_get(const char *name, bool is_non_volatile)
+{
+	struct hsearch_data *htab;
+	ENTRY e, *ep;
+
+	/* WATCHDOG_RESET(); */
+
+	if (is_non_volatile)
+		htab = &efi_nv_var_htab;
+	else
+		htab = &efi_var_htab;
+
+	e.key   = name;
+	e.data  = NULL;
+	hsearch_r(e, FIND, &ep, htab, 0);
+
+	return ep ? ep->data : NULL;
+}
+
+static int env_efi_set(const char *name, const char *value,
+		       bool is_non_volatile)
+{
+	struct hsearch_data *htab;
+	ENTRY e, *ep;
+	int ret;
+
+	if (is_non_volatile)
+		htab = &efi_nv_var_htab;
+	else
+		htab = &efi_var_htab;
+
+	/* delete */
+	if (!value || *value == '\0') {
+		ret = hdelete_r(name, htab, H_PROGRAMMATIC);
+		return !ret;
+	}
+
+	/* set */
+	e.key   = name;
+	e.data  = (char *)value;
+	hsearch_r(e, ENTER, &ep, htab, H_PROGRAMMATIC);
+	if (!ep) {
+		printf("## Error inserting \"%s\" variable, errno=%d\n",
+		       name, errno);
+		return 1;
+	}
+
+	return 0;
+}
+
 #define PREFIX_LEN (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx_"))
 
 /**
@@ -147,24 +207,12 @@  static const char *parse_attr(const char *str, u32 *attrp)
 	return str;
 }
 
-/**
- * efi_efi_get_variable() - retrieve value of a UEFI variable
- *
- * This function implements the GetVariable runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * @variable_name:	name of the variable
- * @vendor:		vendor GUID
- * @attributes:		attributes of the variable
- * @data_size:		size of the buffer to which the variable value is copied
- * @data:		buffer to which the variable value is copied
- * Return:		status code
- */
-efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
-				     const efi_guid_t *vendor, u32 *attributes,
-				     efi_uintn_t *data_size, void *data)
+static
+efi_status_t EFIAPI efi_get_variable_common(u16 *variable_name,
+					    const efi_guid_t *vendor,
+					    u32 *attributes,
+					    efi_uintn_t *data_size, void *data,
+					    bool is_non_volatile)
 {
 	char *native_name;
 	efi_status_t ret;
@@ -172,22 +220,19 @@  efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
 	const char *val, *s;
 	u32 attr;
 
-	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
-		  data_size, data);
-
 	if (!variable_name || !vendor || !data_size)
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
 	ret = efi_to_native(&native_name, variable_name, vendor);
 	if (ret)
-		return EFI_EXIT(ret);
+		return ret;
 
 	EFI_PRINT("get '%s'\n", native_name);
 
-	val = env_get(native_name);
+	val = env_efi_get(native_name, is_non_volatile);
 	free(native_name);
 	if (!val)
-		return EFI_EXIT(EFI_NOT_FOUND);
+		return EFI_NOT_FOUND;
 
 	val = parse_attr(val, &attr);
 
@@ -198,7 +243,7 @@  efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
 
 		/* number of hexadecimal digits must be even */
 		if (len & 1)
-			return EFI_EXIT(EFI_DEVICE_ERROR);
+			return EFI_DEVICE_ERROR;
 
 		/* two characters per byte: */
 		len /= 2;
@@ -210,10 +255,10 @@  efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
 		}
 
 		if (!data)
-			return EFI_EXIT(EFI_INVALID_PARAMETER);
+			return EFI_INVALID_PARAMETER;
 
 		if (hex2bin(data, s, len))
-			return EFI_EXIT(EFI_DEVICE_ERROR);
+			return EFI_DEVICE_ERROR;
 
 		EFI_PRINT("got value: \"%s\"\n", s);
 	} else if ((s = prefix(val, "(utf8)"))) {
@@ -227,7 +272,7 @@  efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
 		}
 
 		if (!data)
-			return EFI_EXIT(EFI_INVALID_PARAMETER);
+			return EFI_INVALID_PARAMETER;
 
 		memcpy(data, s, len);
 		((char *)data)[len] = '\0';
@@ -235,13 +280,67 @@  efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
 		EFI_PRINT("got value: \"%s\"\n", (char *)data);
 	} else {
 		EFI_PRINT("invalid value: '%s'\n", val);
-		return EFI_EXIT(EFI_DEVICE_ERROR);
+		return EFI_DEVICE_ERROR;
 	}
 
 out:
 	if (attributes)
 		*attributes = attr & EFI_VARIABLE_MASK;
 
+	return ret;
+}
+
+static
+efi_status_t EFIAPI efi_get_volatile_variable(u16 *variable_name,
+					      const efi_guid_t *vendor,
+					      u32 *attributes,
+					      efi_uintn_t *data_size,
+					      void *data)
+{
+	return efi_get_variable_common(variable_name, vendor, attributes,
+				       data_size, data, false);
+}
+
+efi_status_t EFIAPI efi_get_nonvolatile_variable(u16 *variable_name,
+						 const efi_guid_t *vendor,
+						 u32 *attributes,
+						 efi_uintn_t *data_size,
+						 void *data)
+{
+	return efi_get_variable_common(variable_name, vendor, attributes,
+				       data_size, data, true);
+}
+
+/**
+ * efi_efi_get_variable() - retrieve value of a UEFI variable
+ *
+ * This function implements the GetVariable runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @variable_name:	name of the variable
+ * @vendor:		vendor GUID
+ * @attributes:		attributes of the variable
+ * @data_size:		size of the buffer to which the variable value is copied
+ * @data:		buffer to which the variable value is copied
+ * Return:		status code
+ */
+efi_status_t EFIAPI efi_get_variable(u16 *variable_name,
+				     const efi_guid_t *vendor, u32 *attributes,
+				     efi_uintn_t *data_size, void *data)
+{
+	efi_status_t ret;
+
+	EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes,
+		  data_size, data);
+
+	ret = efi_get_volatile_variable(variable_name, vendor, attributes,
+					data_size, data);
+	if (ret == EFI_NOT_FOUND)
+		ret = efi_get_nonvolatile_variable(variable_name, vendor,
+						   attributes, data_size, data);
+
 	return EFI_EXIT(ret);
 }
 
@@ -331,7 +430,7 @@  efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 					       u16 *variable_name,
 					       const efi_guid_t *vendor)
 {
-	char *native_name, *variable;
+	char *native_name, *variable, *tmp_list, *merged_list;
 	ssize_t name_len, list_len;
 	char regex[256];
 	char * const regexlist[] = {regex};
@@ -387,10 +486,39 @@  efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 		efi_cur_variable = NULL;
 
 		snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*");
-		list_len = hexport_r(&env_htab, '\n',
+		list_len = hexport_r(&efi_var_htab, '\n',
 				     H_MATCH_REGEX | H_MATCH_KEY,
 				     &efi_variables_list, 0, 1, regexlist);
-		/* 1 indicates that no match was found */
+		/*
+		 * Note: '1' indicates that nothing is matched
+		 */
+		if (list_len <= 1) {
+			free(efi_variables_list);
+			efi_variables_list = NULL;
+			list_len = hexport_r(&efi_nv_var_htab, '\n',
+					     H_MATCH_REGEX | H_MATCH_KEY,
+					     &efi_variables_list, 0, 1,
+					     regexlist);
+		} else {
+			tmp_list = NULL;
+			list_len = hexport_r(&efi_nv_var_htab, '\n',
+					     H_MATCH_REGEX | H_MATCH_KEY,
+					     &tmp_list, 0, 1,
+					     regexlist);
+			if (list_len <= 1) {
+				list_len = 2; /* don't care actual number */
+			} else {
+				/* merge two variables lists */
+				merged_list = malloc(strlen(efi_variables_list)
+							+ strlen(tmp_list) + 1);
+				strcpy(merged_list, efi_variables_list);
+				strcat(merged_list, tmp_list);
+				free(efi_variables_list);
+				free(tmp_list);
+				efi_variables_list = merged_list;
+			}
+		}
+
 		if (list_len <= 1)
 			return EFI_EXIT(EFI_NOT_FOUND);
 
@@ -403,77 +531,71 @@  efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size,
 	return EFI_EXIT(ret);
 }
 
-/**
- * efi_efi_set_variable() - set value of a UEFI variable
- *
- * This function implements the SetVariable runtime service.
- *
- * See the Unified Extensible Firmware Interface (UEFI) specification for
- * details.
- *
- * @variable_name:	name of the variable
- * @vendor:		vendor GUID
- * @attributes:		attributes of the variable
- * @data_size:		size of the buffer with the variable value
- * @data:		buffer with the variable value
- * Return:		status code
- */
-efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
-				     const efi_guid_t *vendor, u32 attributes,
-				     efi_uintn_t data_size, const void *data)
+static
+efi_status_t EFIAPI efi_set_variable_common(u16 *variable_name,
+					    const efi_guid_t *vendor,
+					    u32 attributes,
+					    efi_uintn_t data_size,
+					    const void *data,
+					    bool is_non_volatile)
 {
 	char *native_name = NULL, *val = NULL, *s;
-	efi_status_t ret = EFI_SUCCESS;
+	efi_uintn_t size;
 	u32 attr;
-
-	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
-		  data_size, data);
+	efi_status_t ret = EFI_SUCCESS;
 
 	/* TODO: implement APPEND_WRITE */
 	if (!variable_name || !vendor ||
 	    (attributes & EFI_VARIABLE_APPEND_WRITE)) {
 		ret = EFI_INVALID_PARAMETER;
-		goto out;
+		goto err;
 	}
 
 	ret = efi_to_native(&native_name, variable_name, vendor);
 	if (ret)
-		goto out;
+		goto err;
 
 #define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS)
 
-	if ((data_size == 0) || !(attributes & ACCESS_ATTR)) {
-		/* delete the variable: */
-		env_set(native_name, NULL);
-		ret = EFI_SUCCESS;
-		goto out;
+	/* check if a variable exists */
+	size = 0;
+	ret = EFI_CALL(efi_get_variable(variable_name, vendor, &attr,
+					&size, NULL));
+	if (ret == EFI_BUFFER_TOO_SMALL) {
+		if ((is_non_volatile && !(attr & EFI_VARIABLE_NON_VOLATILE)) ||
+		    (!is_non_volatile && (attr & EFI_VARIABLE_NON_VOLATILE))) {
+			ret = EFI_INVALID_PARAMETER;
+			goto err;
+		}
 	}
 
-	val = env_get(native_name);
-	if (val) {
-		parse_attr(val, &attr);
-
-		/* We should not free val */
-		val = NULL;
-		if (attr & READ_ONLY) {
-			ret = EFI_WRITE_PROTECTED;
+	/* delete a variable */
+	if (data_size == 0 || !(attributes & ACCESS_ATTR)) {
+		if (size) {
+			if (attr & READ_ONLY) {
+				ret = EFI_WRITE_PROTECTED;
+				goto err;
+			}
 			goto out;
 		}
+		ret = EFI_SUCCESS;
+		goto err; /* not error, but nothing to do */
+	}
 
+	/* create/modify a variable */
+	if (size && attr != attributes) {
 		/*
 		 * attributes won't be changed
 		 * TODO: take care of APPEND_WRITE once supported
 		 */
-		if (attr != attributes) {
-			ret = EFI_INVALID_PARAMETER;
-			goto out;
-		}
+		ret = EFI_INVALID_PARAMETER;
+		goto err;
 	}
 
 	val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1);
 	if (!val) {
 		ret = EFI_OUT_OF_RESOURCES;
-		goto out;
+		goto err;
 	}
 
 	s = val;
@@ -487,7 +609,7 @@  efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
 		       EFI_VARIABLE_RUNTIME_ACCESS);
 	s += sprintf(s, "{");
 	while (attributes) {
-		u32 attr = 1 << (ffs(attributes) - 1);
+		attr = 1 << (ffs(attributes) - 1);
 
 		if (attr == EFI_VARIABLE_NON_VOLATILE)
 			s += sprintf(s, "nv");
@@ -509,12 +631,78 @@  efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
 
 	EFI_PRINT("setting: %s=%s\n", native_name, val);
 
-	if (env_set(native_name, val))
+out:
+	ret = EFI_SUCCESS;
+	if (env_efi_set(native_name, val, is_non_volatile))
 		ret = EFI_DEVICE_ERROR;
 
-out:
+err:
 	free(native_name);
 	free(val);
 
+	return ret;
+}
+
+static
+efi_status_t EFIAPI efi_set_volatile_variable(u16 *variable_name,
+					      const efi_guid_t *vendor,
+					      u32 attributes,
+					      efi_uintn_t data_size,
+					      const void *data)
+{
+	return efi_set_variable_common(variable_name, vendor, attributes,
+				       data_size, data, false);
+}
+
+efi_status_t EFIAPI efi_set_nonvolatile_variable(u16 *variable_name,
+						 const efi_guid_t *vendor,
+						 u32 attributes,
+						 efi_uintn_t data_size,
+						 const void *data)
+{
+	efi_status_t ret;
+
+	ret = efi_set_variable_common(variable_name, vendor, attributes,
+				      data_size, data, true);
+	if (ret == EFI_SUCCESS)
+		/* FIXME: what if save failed? */
+		if (env_efi_save())
+			ret = EFI_DEVICE_ERROR;
+
+	return ret;
+}
+
+/**
+ * efi_efi_set_variable() - set value of a UEFI variable
+ *
+ * This function implements the SetVariable runtime service.
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @variable_name:	name of the variable
+ * @vendor:		vendor GUID
+ * @attributes:		attributes of the variable
+ * @data_size:		size of the buffer with the variable value
+ * @data:		buffer with the variable value
+ * Return:		status code
+ */
+efi_status_t EFIAPI efi_set_variable(u16 *variable_name,
+				     const efi_guid_t *vendor, u32 attributes,
+				     efi_uintn_t data_size, const void *data)
+{
+	efi_status_t ret;
+
+	EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes,
+		  data_size, data);
+
+	if (attributes & EFI_VARIABLE_NON_VOLATILE)
+		ret = efi_set_nonvolatile_variable(variable_name, vendor,
+						   attributes,
+						   data_size, data);
+	else
+		ret = efi_set_volatile_variable(variable_name, vendor,
+						attributes, data_size, data);
+
 	return EFI_EXIT(ret);
 }