diff mbox series

[RFC,11/14] efi_loader: variable: export variables table for runtime access

Message ID 20200317021247.5849-12-takahiro.akashi@linaro.org
State RFC
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: add capsule update support | expand

Commit Message

AKASHI Takahiro March 17, 2020, 2:12 a.m. UTC 
There are platforms on which OS's won't be able to access UEFI variables
at runtime (i.e. after ExitBootServices).
With this patch, all the UEFI variables are exported as a configuration
table so as to enable retrieving variables' values from the table, and
later modifying them via capsule-on-disk if necessary.

The idea is based on Peter's proposal[1] in boot-arch ML.

[1] https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/efi_loader.h          |   7 +++
 lib/efi_loader/Kconfig        |  10 ++++
 lib/efi_loader/efi_boottime.c |  10 ++++
 lib/efi_loader/efi_capsule.c  |   4 ++
 lib/efi_loader/efi_variable.c | 109 ++++++++++++++++++++++++++++++++++
 5 files changed, 140 insertions(+)

Comments

Heinrich Schuchardt March 17, 2020, 7:37 a.m. UTC | #1 
On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> There are platforms on which OS's won't be able to access UEFI variables
> at runtime (i.e. after ExitBootServices).
> With this patch, all the UEFI variables are exported as a configuration
> table so as to enable retrieving variables' values from the table, and
> later modifying them via capsule-on-disk if necessary.

I do not understand why we should expose our internal memory for holding
UEFI variables to the operating system. This might end up in users
trying to access the variables directly.

I do not understand why we should not keep the pointer in our private
memory.

Best regards

Heinrich

>
> The idea is based on Peter's proposal[1] in boot-arch ML.
>
> [1] https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   include/efi_loader.h          |   7 +++
>   lib/efi_loader/Kconfig        |  10 ++++
>   lib/efi_loader/efi_boottime.c |  10 ++++
>   lib/efi_loader/efi_capsule.c  |   4 ++
>   lib/efi_loader/efi_variable.c | 109 ++++++++++++++++++++++++++++++++++
>   5 files changed, 140 insertions(+)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 79bdf9586d24..93ed5502821c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -183,6 +183,8 @@ extern const efi_guid_t efi_guid_hii_string_protocol;
>   extern const efi_guid_t efi_guid_capsule_report;
>   /* GUID of firmware management protocol */
>   extern const efi_guid_t efi_guid_firmware_management_protocol;
> +/* GUID of runtime variable access */
> +extern const efi_guid_t efi_guid_variable_storage;
>
>   /* GUID of RNG protocol */
>   extern const efi_guid_t efi_guid_rng_protocol;
> @@ -659,6 +661,10 @@ efi_status_t EFIAPI efi_query_variable_info(
>   			u64 *remaining_variable_storage_size,
>   			u64 *maximum_variable_size);
>
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +efi_status_t efi_install_variables_table(void);
> +#endif
> +
>   /*
>    * 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:
> @@ -683,6 +689,7 @@ struct efi_load_option {
>   void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
>   unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
>   efi_status_t efi_bootmgr_load(efi_handle_t *handle);
> +efi_status_t efi_install_variables_table(void);
>
>   /* Capsule update */
>   efi_status_t EFIAPI efi_update_capsule(
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 616e2acbe102..edb8d19059ea 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -41,6 +41,16 @@ config EFI_SET_TIME
>   	  Provide the SetTime() runtime service at boottime. This service
>   	  can be used by an EFI application to adjust the real time clock.
>
> +config EFI_VARIABLE_EXPORT
> +	bool "Export variables table as configuration table"
> +	depends on !EFI_VARIABLE_RUNTIME_ACCESS
> +	depends on  EFI_CAPSULE_UPDATE_VARIABLE
> +	default y
> +	help
> +	  Select this option if you want to export UEFI variables as
> +	  configuration table so that OS can still get UEFI variable's
> +	  value.
> +
>   config EFI_DEVICE_PATH_TO_TEXT
>   	bool "Device path to text protocol"
>   	default y
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index c2a789b4f910..406644e4da67 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2898,6 +2898,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
>   	if (ret != EFI_SUCCESS)
>   		return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +	/* Install variables database as a configuration table */
> +	ret = efi_install_variables_table();
> +	if (ret != EFI_SUCCESS)
> +		return EFI_EXIT(ret);
> +#endif
> +	/*
> +	 * TODO: remove a table after image is terminated.
> +	 */
> +
>   	image_obj->exit_data_size = exit_data_size;
>   	image_obj->exit_data = exit_data;
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 1293270aea95..6b737aec1b28 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -18,7 +18,11 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
>   		EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>   const efi_guid_t efi_guid_firmware_management_protocol =
>   		EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
> +#else
>   static const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
> +#endif
>
>   /* for file system access */
>   static struct efi_file_handle *bootdev_root;
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index c316bdfec0e4..270e5211f633 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -641,3 +641,112 @@ efi_status_t efi_init_variables(void)
>   {
>   	return EFI_SUCCESS;
>   }
> +
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +/**
> + * efi_install_variables_table() - install variables table
> + *
> + * This function adds a configuration table that contains all the cache
> + * data of UEFI variables for runtime access.
> + *
> + * Return:	status code
> + */
> +efi_status_t efi_install_variables_table(void)
> +{
> +	u16 var_name16[64];
> +	efi_uintn_t var_name16_len;
> +	efi_guid_t guid;
> +	u32 attributes;
> +	struct efi_capsule_header capsule;
> +	u8 *data = NULL, *p;
> +	efi_uintn_t capsule_size;
> +	struct efi_ebbr_variable variable;
> +	u8 *var_buf;
> +	efi_uintn_t var_size, var_max_size;
> +	efi_status_t ret;
> +
> +	capsule.capsule_guid = efi_guid_variable_storage;
> +	capsule.header_size = sizeof(capsule);
> +	capsule.flags = CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE;
> +	capsule_size = sizeof(capsule);
> +
> +	/* Determine size of capsule */
> +	var_name16[0] = 0;
> +	var_name16_len = 64;
> +	var_buf = NULL;
> +	var_max_size = 0;
> +	for (;;) {
> +		ret = EFI_CALL(efi_get_next_variable_name(&var_name16_len,
> +							  var_name16,
> +							  &guid));
> +		if (ret == EFI_NOT_FOUND)
> +			break;
> +		else if (ret != EFI_SUCCESS)
> +			goto out;
> +
> +		var_size = 0;
> +		ret = EFI_CALL(efi_get_variable(var_name16, &guid, &attributes,
> +						&var_size, var_buf));
> +		if (ret != EFI_BUFFER_TOO_SMALL)
> +			goto out;
> +
> +		capsule_size += sizeof(variable) + var_size;
> +		if (var_size > var_max_size)
> +			var_max_size = var_size;
> +	}
> +	capsule.capsule_image_size = capsule_size;
> +
> +	ret = efi_allocate_pool(EFI_RUNTIME_SERVICES_DATA,
> +				capsule_size, (void **)&data);
> +	if (ret != EFI_SUCCESS)
> +		return EFI_OUT_OF_RESOURCES;
> +
> +	var_buf = malloc(var_max_size);
> +	if (!var_buf) {
> +		ret = EFI_OUT_OF_RESOURCES;
> +		goto out;
> +	}
> +
> +	/* Copy data */
> +	p = data;
> +	memcpy(p, &capsule, sizeof(capsule));
> +	p += sizeof(capsule);
> +
> +	var_name16[0] = 0;
> +	for (;;) {
> +		ret = EFI_CALL(efi_get_next_variable_name(&var_name16_len,
> +							  var_name16, &guid));
> +		if (ret == EFI_NOT_FOUND)
> +			break;
> +		else if (ret != EFI_SUCCESS)
> +			goto out;
> +
> +		var_size = var_max_size;
> +		ret = EFI_CALL(efi_get_variable(var_name16, &guid, &attributes,
> +						&var_size, var_buf));
> +		if (ret != EFI_SUCCESS)
> +			/* should not happen */
> +			goto out;
> +
> +		u16_strcpy(variable.variable_name, var_name16);
> +		variable.vendor_guid = guid;
> +		variable.attributes = attributes;
> +		variable.data_size = var_size;
> +
> +		memcpy(p, &variable, sizeof(variable));
> +		p += sizeof(variable);
> +		memcpy(p, var_buf, var_size);
> +		p += var_size;
> +	}
> +
> +	ret = EFI_CALL(efi_install_configuration_table(
> +				&efi_guid_variable_storage, data));
> +
> +out:
> +	free(var_buf);
> +	if (ret != EFI_SUCCESS && data)
> +		efi_free_pool(data);
> +
> +	return ret;
> +}
> +#endif /* CONFIG_EFI_VARIABLE_EXPORT */
>
AKASHI Takahiro March 18, 2020, 1:53 a.m. UTC | #2 
On Tue, Mar 17, 2020 at 08:37:47AM +0100, Heinrich Schuchardt wrote:
> On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> > There are platforms on which OS's won't be able to access UEFI variables
> > at runtime (i.e. after ExitBootServices).
> > With this patch, all the UEFI variables are exported as a configuration
> > table so as to enable retrieving variables' values from the table, and
> > later modifying them via capsule-on-disk if necessary.
> 
> I do not understand why we should expose our internal memory for holding
> UEFI variables to the operating system. This might end up in users
> trying to access the variables directly.

I think that you somehow misunderstand my code as it never exposes
any "internal memory," although I don't know what it exactly means in
this context.
This configuration table is nothing but a list of data that represent
all the UEFI variables in implementation-agnostic format.

> I do not understand why we should not keep the pointer in our private
> memory.

Anyway, this patch naively implements Peter's proposal while I also
submitted another patch[1] that allows HL-OS to use GetVariable
interface directly via *caching*.

Since how we should enable accessing UEFI variables at runtime is
one of key issues, I'd rather discuss in boot-arch ML as I suggested
in the cover letter.
I have already re-activated the discussion there[2].
Please make your comments there for wider audience.

[1] https://lists.denx.de/pipermail/u-boot/2019-June/371769.html
[2] https://lists.linaro.org/pipermail/boot-architecture/2020-March/001149.html

Thanks,
-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> > 
> > The idea is based on Peter's proposal[1] in boot-arch ML.
> > 
> > [1] https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >   include/efi_loader.h          |   7 +++
> >   lib/efi_loader/Kconfig        |  10 ++++
> >   lib/efi_loader/efi_boottime.c |  10 ++++
> >   lib/efi_loader/efi_capsule.c  |   4 ++
> >   lib/efi_loader/efi_variable.c | 109 ++++++++++++++++++++++++++++++++++
> >   5 files changed, 140 insertions(+)
> > 
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 79bdf9586d24..93ed5502821c 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -183,6 +183,8 @@ extern const efi_guid_t efi_guid_hii_string_protocol;
> >   extern const efi_guid_t efi_guid_capsule_report;
> >   /* GUID of firmware management protocol */
> >   extern const efi_guid_t efi_guid_firmware_management_protocol;
> > +/* GUID of runtime variable access */
> > +extern const efi_guid_t efi_guid_variable_storage;
> > 
> >   /* GUID of RNG protocol */
> >   extern const efi_guid_t efi_guid_rng_protocol;
> > @@ -659,6 +661,10 @@ efi_status_t EFIAPI efi_query_variable_info(
> >   			u64 *remaining_variable_storage_size,
> >   			u64 *maximum_variable_size);
> > 
> > +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> > +efi_status_t efi_install_variables_table(void);
> > +#endif
> > +
> >   /*
> >    * 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:
> > @@ -683,6 +689,7 @@ struct efi_load_option {
> >   void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
> >   unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
> >   efi_status_t efi_bootmgr_load(efi_handle_t *handle);
> > +efi_status_t efi_install_variables_table(void);
> > 
> >   /* Capsule update */
> >   efi_status_t EFIAPI efi_update_capsule(
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 616e2acbe102..edb8d19059ea 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -41,6 +41,16 @@ config EFI_SET_TIME
> >   	  Provide the SetTime() runtime service at boottime. This service
> >   	  can be used by an EFI application to adjust the real time clock.
> > 
> > +config EFI_VARIABLE_EXPORT
> > +	bool "Export variables table as configuration table"
> > +	depends on !EFI_VARIABLE_RUNTIME_ACCESS
> > +	depends on  EFI_CAPSULE_UPDATE_VARIABLE
> > +	default y
> > +	help
> > +	  Select this option if you want to export UEFI variables as
> > +	  configuration table so that OS can still get UEFI variable's
> > +	  value.
> > +
> >   config EFI_DEVICE_PATH_TO_TEXT
> >   	bool "Device path to text protocol"
> >   	default y
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index c2a789b4f910..406644e4da67 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -2898,6 +2898,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
> >   	if (ret != EFI_SUCCESS)
> >   		return EFI_EXIT(EFI_INVALID_PARAMETER);
> > 
> > +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> > +	/* Install variables database as a configuration table */
> > +	ret = efi_install_variables_table();
> > +	if (ret != EFI_SUCCESS)
> > +		return EFI_EXIT(ret);
> > +#endif
> > +	/*
> > +	 * TODO: remove a table after image is terminated.
> > +	 */
> > +
> >   	image_obj->exit_data_size = exit_data_size;
> >   	image_obj->exit_data = exit_data;
> > 
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 1293270aea95..6b737aec1b28 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -18,7 +18,11 @@ static const efi_guid_t efi_guid_firmware_management_capsule_id =
> >   		EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> >   const efi_guid_t efi_guid_firmware_management_protocol =
> >   		EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> > +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> > +const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
> > +#else
> >   static const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
> > +#endif
> > 
> >   /* for file system access */
> >   static struct efi_file_handle *bootdev_root;
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index c316bdfec0e4..270e5211f633 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -641,3 +641,112 @@ efi_status_t efi_init_variables(void)
> >   {
> >   	return EFI_SUCCESS;
> >   }
> > +
> > +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> > +/**
> > + * efi_install_variables_table() - install variables table
> > + *
> > + * This function adds a configuration table that contains all the cache
> > + * data of UEFI variables for runtime access.
> > + *
> > + * Return:	status code
> > + */
> > +efi_status_t efi_install_variables_table(void)
> > +{
> > +	u16 var_name16[64];
> > +	efi_uintn_t var_name16_len;
> > +	efi_guid_t guid;
> > +	u32 attributes;
> > +	struct efi_capsule_header capsule;
> > +	u8 *data = NULL, *p;
> > +	efi_uintn_t capsule_size;
> > +	struct efi_ebbr_variable variable;
> > +	u8 *var_buf;
> > +	efi_uintn_t var_size, var_max_size;
> > +	efi_status_t ret;
> > +
> > +	capsule.capsule_guid = efi_guid_variable_storage;
> > +	capsule.header_size = sizeof(capsule);
> > +	capsule.flags = CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE;
> > +	capsule_size = sizeof(capsule);
> > +
> > +	/* Determine size of capsule */
> > +	var_name16[0] = 0;
> > +	var_name16_len = 64;
> > +	var_buf = NULL;
> > +	var_max_size = 0;
> > +	for (;;) {
> > +		ret = EFI_CALL(efi_get_next_variable_name(&var_name16_len,
> > +							  var_name16,
> > +							  &guid));
> > +		if (ret == EFI_NOT_FOUND)
> > +			break;
> > +		else if (ret != EFI_SUCCESS)
> > +			goto out;
> > +
> > +		var_size = 0;
> > +		ret = EFI_CALL(efi_get_variable(var_name16, &guid, &attributes,
> > +						&var_size, var_buf));
> > +		if (ret != EFI_BUFFER_TOO_SMALL)
> > +			goto out;
> > +
> > +		capsule_size += sizeof(variable) + var_size;
> > +		if (var_size > var_max_size)
> > +			var_max_size = var_size;
> > +	}
> > +	capsule.capsule_image_size = capsule_size;
> > +
> > +	ret = efi_allocate_pool(EFI_RUNTIME_SERVICES_DATA,
> > +				capsule_size, (void **)&data);
> > +	if (ret != EFI_SUCCESS)
> > +		return EFI_OUT_OF_RESOURCES;
> > +
> > +	var_buf = malloc(var_max_size);
> > +	if (!var_buf) {
> > +		ret = EFI_OUT_OF_RESOURCES;
> > +		goto out;
> > +	}
> > +
> > +	/* Copy data */
> > +	p = data;
> > +	memcpy(p, &capsule, sizeof(capsule));
> > +	p += sizeof(capsule);
> > +
> > +	var_name16[0] = 0;
> > +	for (;;) {
> > +		ret = EFI_CALL(efi_get_next_variable_name(&var_name16_len,
> > +							  var_name16, &guid));
> > +		if (ret == EFI_NOT_FOUND)
> > +			break;
> > +		else if (ret != EFI_SUCCESS)
> > +			goto out;
> > +
> > +		var_size = var_max_size;
> > +		ret = EFI_CALL(efi_get_variable(var_name16, &guid, &attributes,
> > +						&var_size, var_buf));
> > +		if (ret != EFI_SUCCESS)
> > +			/* should not happen */
> > +			goto out;
> > +
> > +		u16_strcpy(variable.variable_name, var_name16);
> > +		variable.vendor_guid = guid;
> > +		variable.attributes = attributes;
> > +		variable.data_size = var_size;
> > +
> > +		memcpy(p, &variable, sizeof(variable));
> > +		p += sizeof(variable);
> > +		memcpy(p, var_buf, var_size);
> > +		p += var_size;
> > +	}
> > +
> > +	ret = EFI_CALL(efi_install_configuration_table(
> > +				&efi_guid_variable_storage, data));
> > +
> > +out:
> > +	free(var_buf);
> > +	if (ret != EFI_SUCCESS && data)
> > +		efi_free_pool(data);
> > +
> > +	return ret;
> > +}
> > +#endif /* CONFIG_EFI_VARIABLE_EXPORT */
> > 
>
Sughosh Ganu March 18, 2020, 1:54 p.m. UTC | #3 
On Tue, 17 Mar 2020 at 07:42, AKASHI Takahiro <takahiro.akashi@linaro.org>
wrote:

> There are platforms on which OS's won't be able to access UEFI variables
> at runtime (i.e. after ExitBootServices).
> With this patch, all the UEFI variables are exported as a configuration
> table so as to enable retrieving variables' values from the table, and
> later modifying them via capsule-on-disk if necessary.
>
> The idea is based on Peter's proposal[1] in boot-arch ML.
>
> [1]
> https://lists.linaro.org/pipermail/boot-architecture/2018-October/000883.html
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  include/efi_loader.h          |   7 +++
>  lib/efi_loader/Kconfig        |  10 ++++
>  lib/efi_loader/efi_boottime.c |  10 ++++
>  lib/efi_loader/efi_capsule.c  |   4 ++
>  lib/efi_loader/efi_variable.c | 109 ++++++++++++++++++++++++++++++++++
>  5 files changed, 140 insertions(+)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 79bdf9586d24..93ed5502821c 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -183,6 +183,8 @@ extern const efi_guid_t efi_guid_hii_string_protocol;
>  extern const efi_guid_t efi_guid_capsule_report;
>  /* GUID of firmware management protocol */
>  extern const efi_guid_t efi_guid_firmware_management_protocol;
> +/* GUID of runtime variable access */
> +extern const efi_guid_t efi_guid_variable_storage;
>
>  /* GUID of RNG protocol */
>  extern const efi_guid_t efi_guid_rng_protocol;
> @@ -659,6 +661,10 @@ efi_status_t EFIAPI efi_query_variable_info(
>                         u64 *remaining_variable_storage_size,
>                         u64 *maximum_variable_size);
>
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +efi_status_t efi_install_variables_table(void);
> +#endif
> +
>  /*
>   * 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:
> @@ -683,6 +689,7 @@ struct efi_load_option {
>  void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
>  unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8
> **data);
>  efi_status_t efi_bootmgr_load(efi_handle_t *handle);
> +efi_status_t efi_install_variables_table(void);
>
>  /* Capsule update */
>  efi_status_t EFIAPI efi_update_capsule(
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 616e2acbe102..edb8d19059ea 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -41,6 +41,16 @@ config EFI_SET_TIME
>           Provide the SetTime() runtime service at boottime. This service
>           can be used by an EFI application to adjust the real time clock.
>
> +config EFI_VARIABLE_EXPORT
> +       bool "Export variables table as configuration table"
> +       depends on !EFI_VARIABLE_RUNTIME_ACCESS
> +       depends on  EFI_CAPSULE_UPDATE_VARIABLE
> +       default y
> +       help
> +         Select this option if you want to export UEFI variables as
> +         configuration table so that OS can still get UEFI variable's
> +         value.
> +
>  config EFI_DEVICE_PATH_TO_TEXT
>         bool "Device path to text protocol"
>         default y
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index c2a789b4f910..406644e4da67 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2898,6 +2898,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t
> image_handle,
>         if (ret != EFI_SUCCESS)
>                 return EFI_EXIT(EFI_INVALID_PARAMETER);
>
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +       /* Install variables database as a configuration table */
> +       ret = efi_install_variables_table();
> +       if (ret != EFI_SUCCESS)
> +               return EFI_EXIT(ret);
> +#endif
> +       /*
> +        * TODO: remove a table after image is terminated.
> +        */
> +
>         image_obj->exit_data_size = exit_data_size;
>         image_obj->exit_data = exit_data;
>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 1293270aea95..6b737aec1b28 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -18,7 +18,11 @@ static const efi_guid_t
> efi_guid_firmware_management_capsule_id =
>                 EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>  const efi_guid_t efi_guid_firmware_management_protocol =
>                 EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
> +#ifdef CONFIG_EFI_VARIABLE_EXPORT
> +const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
> +#else
>  static const efi_guid_t efi_guid_variable_storage =
> EFI_VARIABLE_STORAGE_GUID;
> +#endif
>

Need to remove the static keyword here. The build fails when
CONFIG_EFI_VARIABLE_EXPORT is not defined, with the extern declaration now
being done in efi_loader.h

-sughosh
Ilias Apalodimas March 19, 2020, 9:30 a.m. UTC | #4 
Akashi-san,

On Wed, Mar 18, 2020 at 10:53:45AM +0900, AKASHI Takahiro wrote:
> On Tue, Mar 17, 2020 at 08:37:47AM +0100, Heinrich Schuchardt wrote:
> > On 3/17/20 3:12 AM, AKASHI Takahiro wrote:
> > > There are platforms on which OS's won't be able to access UEFI variables
> > > at runtime (i.e. after ExitBootServices).
> > > With this patch, all the UEFI variables are exported as a configuration
> > > table so as to enable retrieving variables' values from the table, and
> > > later modifying them via capsule-on-disk if necessary.
> > 
> > I do not understand why we should expose our internal memory for holding
> > UEFI variables to the operating system. This might end up in users
> > trying to access the variables directly.
> 
> I think that you somehow misunderstand my code as it never exposes
> any "internal memory," although I don't know what it exactly means in
> this context.
> This configuration table is nothing but a list of data that represent
> all the UEFI variables in implementation-agnostic format.
> 
> > I do not understand why we should not keep the pointer in our private
> > memory.
> 
> Anyway, this patch naively implements Peter's proposal while I also
> submitted another patch[1] that allows HL-OS to use GetVariable
> interface directly via *caching*.

How are the two approaches going to affect existig tools (i.e efivar --list) to
read the variables?

> 
> Since how we should enable accessing UEFI variables at runtime is
> one of key issues, I'd rather discuss in boot-arch ML as I suggested
> in the cover letter.
> I have already re-activated the discussion there[2].
> Please make your comments there for wider audience.
> 
> [1] https://lists.denx.de/pipermail/u-boot/2019-June/371769.html
> [2] https://lists.linaro.org/pipermail/boot-architecture/2020-March/001149.html
> 
Will do

Regards
/Ilias
diff mbox series

Patch

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 79bdf9586d24..93ed5502821c 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -183,6 +183,8 @@  extern const efi_guid_t efi_guid_hii_string_protocol;
 extern const efi_guid_t efi_guid_capsule_report;
 /* GUID of firmware management protocol */
 extern const efi_guid_t efi_guid_firmware_management_protocol;
+/* GUID of runtime variable access */
+extern const efi_guid_t efi_guid_variable_storage;
 
 /* GUID of RNG protocol */
 extern const efi_guid_t efi_guid_rng_protocol;
@@ -659,6 +661,10 @@  efi_status_t EFIAPI efi_query_variable_info(
 			u64 *remaining_variable_storage_size,
 			u64 *maximum_variable_size);
 
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+efi_status_t efi_install_variables_table(void);
+#endif
+
 /*
  * 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:
@@ -683,6 +689,7 @@  struct efi_load_option {
 void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data);
 unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data);
 efi_status_t efi_bootmgr_load(efi_handle_t *handle);
+efi_status_t efi_install_variables_table(void);
 
 /* Capsule update */
 efi_status_t EFIAPI efi_update_capsule(
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 616e2acbe102..edb8d19059ea 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -41,6 +41,16 @@  config EFI_SET_TIME
 	  Provide the SetTime() runtime service at boottime. This service
 	  can be used by an EFI application to adjust the real time clock.
 
+config EFI_VARIABLE_EXPORT
+	bool "Export variables table as configuration table"
+	depends on !EFI_VARIABLE_RUNTIME_ACCESS
+	depends on  EFI_CAPSULE_UPDATE_VARIABLE
+	default y
+	help
+	  Select this option if you want to export UEFI variables as
+	  configuration table so that OS can still get UEFI variable's
+	  value.
+
 config EFI_DEVICE_PATH_TO_TEXT
 	bool "Device path to text protocol"
 	default y
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index c2a789b4f910..406644e4da67 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2898,6 +2898,16 @@  efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
 	if (ret != EFI_SUCCESS)
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+	/* Install variables database as a configuration table */
+	ret = efi_install_variables_table();
+	if (ret != EFI_SUCCESS)
+		return EFI_EXIT(ret);
+#endif
+	/*
+	 * TODO: remove a table after image is terminated.
+	 */
+
 	image_obj->exit_data_size = exit_data_size;
 	image_obj->exit_data = exit_data;
 
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 1293270aea95..6b737aec1b28 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -18,7 +18,11 @@  static const efi_guid_t efi_guid_firmware_management_capsule_id =
 		EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
 const efi_guid_t efi_guid_firmware_management_protocol =
 		EFI_FIRMWARE_MANAGEMENT_PROTOCOL_GUID;
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
+#else
 static const efi_guid_t efi_guid_variable_storage = EFI_VARIABLE_STORAGE_GUID;
+#endif
 
 /* for file system access */
 static struct efi_file_handle *bootdev_root;
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index c316bdfec0e4..270e5211f633 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -641,3 +641,112 @@  efi_status_t efi_init_variables(void)
 {
 	return EFI_SUCCESS;
 }
+
+#ifdef CONFIG_EFI_VARIABLE_EXPORT
+/**
+ * efi_install_variables_table() - install variables table
+ *
+ * This function adds a configuration table that contains all the cache
+ * data of UEFI variables for runtime access.
+ *
+ * Return:	status code
+ */
+efi_status_t efi_install_variables_table(void)
+{
+	u16 var_name16[64];
+	efi_uintn_t var_name16_len;
+	efi_guid_t guid;
+	u32 attributes;
+	struct efi_capsule_header capsule;
+	u8 *data = NULL, *p;
+	efi_uintn_t capsule_size;
+	struct efi_ebbr_variable variable;
+	u8 *var_buf;
+	efi_uintn_t var_size, var_max_size;
+	efi_status_t ret;
+
+	capsule.capsule_guid = efi_guid_variable_storage;
+	capsule.header_size = sizeof(capsule);
+	capsule.flags = CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE;
+	capsule_size = sizeof(capsule);
+
+	/* Determine size of capsule */
+	var_name16[0] = 0;
+	var_name16_len = 64;
+	var_buf = NULL;
+	var_max_size = 0;
+	for (;;) {
+		ret = EFI_CALL(efi_get_next_variable_name(&var_name16_len,
+							  var_name16,
+							  &guid));
+		if (ret == EFI_NOT_FOUND)
+			break;
+		else if (ret != EFI_SUCCESS)
+			goto out;
+
+		var_size = 0;
+		ret = EFI_CALL(efi_get_variable(var_name16, &guid, &attributes,
+						&var_size, var_buf));
+		if (ret != EFI_BUFFER_TOO_SMALL)
+			goto out;
+
+		capsule_size += sizeof(variable) + var_size;
+		if (var_size > var_max_size)
+			var_max_size = var_size;
+	}
+	capsule.capsule_image_size = capsule_size;
+
+	ret = efi_allocate_pool(EFI_RUNTIME_SERVICES_DATA,
+				capsule_size, (void **)&data);
+	if (ret != EFI_SUCCESS)
+		return EFI_OUT_OF_RESOURCES;
+
+	var_buf = malloc(var_max_size);
+	if (!var_buf) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+
+	/* Copy data */
+	p = data;
+	memcpy(p, &capsule, sizeof(capsule));
+	p += sizeof(capsule);
+
+	var_name16[0] = 0;
+	for (;;) {
+		ret = EFI_CALL(efi_get_next_variable_name(&var_name16_len,
+							  var_name16, &guid));
+		if (ret == EFI_NOT_FOUND)
+			break;
+		else if (ret != EFI_SUCCESS)
+			goto out;
+
+		var_size = var_max_size;
+		ret = EFI_CALL(efi_get_variable(var_name16, &guid, &attributes,
+						&var_size, var_buf));
+		if (ret != EFI_SUCCESS)
+			/* should not happen */
+			goto out;
+
+		u16_strcpy(variable.variable_name, var_name16);
+		variable.vendor_guid = guid;
+		variable.attributes = attributes;
+		variable.data_size = var_size;
+
+		memcpy(p, &variable, sizeof(variable));
+		p += sizeof(variable);
+		memcpy(p, var_buf, var_size);
+		p += var_size;
+	}
+
+	ret = EFI_CALL(efi_install_configuration_table(
+				&efi_guid_variable_storage, data));
+
+out:
+	free(var_buf);
+	if (ret != EFI_SUCCESS && data)
+		efi_free_pool(data);
+
+	return ret;
+}
+#endif /* CONFIG_EFI_VARIABLE_EXPORT */