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

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

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