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

AKASHI Takahiro
2020-03-18T01:53:45Z

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