| Message ID | 20200317021247.5849-12-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | efi_loader: add capsule update support | expand |
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 at 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 */ >
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 at 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 */ > > >
On Tue, 17 Mar 2020 at 07:42, AKASHI Takahiro <takahiro.akashi at 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 at 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
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 --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 */
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 at 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(+)