[PATCH v2 1/3] efi_loader: enable QueryVariableInfo at runtime for file backed variables
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Apr 25 10:32:39 CEST 2024
On 25.04.24 07:18, Ilias Apalodimas wrote:
> Since commit c28d32f946f0 ("efi_loader: conditionally enable SetvariableRT")
> we are enabling the last bits of missing runtime services.
> Add support for QueryVariableInfo which we already support at boottime
> and we just need to mark some fucntions available at runtime and move
> some checks around.
>
> It's worth noting that pointer checks for maxmimum and remaining
> variable storage aren't when we store variables on the RPMB, since the
> Secure World backend is already performing them.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> Changes since v1:
> - require EFI_VARIABLE_RUNTIME_ACCESS to be set at runtime
> - return EFI_UNSUPPORTED for auth variables
> lib/efi_loader/efi_runtime.c | 4 +++
> lib/efi_loader/efi_var_common.c | 6 -----
> lib/efi_loader/efi_variable.c | 25 ++++++++++++++-----
> lib/efi_loader/efi_variable_tee.c | 5 ++++
> .../efi_selftest_variables_runtime.c | 14 ++++++++---
> 5 files changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 73831c527e00..011bcd04836d 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -129,6 +129,10 @@ efi_status_t efi_init_runtime_supported(void)
> EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP |
> EFI_RT_SUPPORTED_CONVERT_POINTER;
>
> + if (IS_ENABLED(CONFIG_EFI_VARIABLE_FILE_STORE))
> + rt_table->runtime_services_supported |=
> + EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO;
> +
> if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> u8 s = 0;
>
> diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> index 961139f005af..ea8d2a4cf98c 100644
> --- a/lib/efi_loader/efi_var_common.c
> +++ b/lib/efi_loader/efi_var_common.c
> @@ -1,4 +1,3 @@
> -// SPDX-License-Identifier: GPL-2.0+
> /*
> * UEFI runtime variable services
> *
> @@ -163,11 +162,6 @@ efi_status_t EFIAPI efi_query_variable_info(
> EFI_ENTRY("%x %p %p %p", attributes, maximum_variable_storage_size,
> remaining_variable_storage_size, maximum_variable_size);
>
> - if (!maximum_variable_storage_size ||
> - !remaining_variable_storage_size ||
> - !maximum_variable_size)
> - return EFI_EXIT(EFI_INVALID_PARAMETER);
> -
> ret = efi_query_variable_info_int(attributes,
> maximum_variable_storage_size,
> remaining_variable_storage_size,
> diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> index 0cbed53d1dbf..1cc02acb3b26 100644
> --- a/lib/efi_loader/efi_variable.c
> +++ b/lib/efi_loader/efi_variable.c
> @@ -406,12 +406,15 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
> return EFI_SUCCESS;
> }
>
> -efi_status_t efi_query_variable_info_int(u32 attributes,
> - u64 *maximum_variable_storage_size,
> - u64 *remaining_variable_storage_size,
> - u64 *maximum_variable_size)
> +efi_status_t __efi_runtime
> +efi_query_variable_info_int(u32 attributes,
> + u64 *maximum_variable_storage_size,
> + u64 *remaining_variable_storage_size,
> + u64 *maximum_variable_size)
> {
> - if (attributes == 0)
> + if (!maximum_variable_storage_size ||
> + !remaining_variable_storage_size ||
> + !maximum_variable_size || !attributes)
> return EFI_INVALID_PARAMETER;
>
> /* EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated */
> @@ -460,7 +463,17 @@ static efi_status_t __efi_runtime EFIAPI efi_query_variable_info_runtime(
> u64 *remaining_variable_storage_size,
> u64 *maximum_variable_size)
> {
> - return EFI_UNSUPPORTED;
According to the UEFI 2.10 specification EFI_UNSUPPORTED must be returned if
"The attribute is not supported on this platform, and the
MaximumVariableStorageSize, RemainingVariableStorageSize,
MaximumVariableSize are undefined."
I wonder if in the spec the 'and' should be replaced by 'or'.
I guess we should return EFI_UNSUPPORTED if
CONFIG_EFI_VARIABLE_RUNTIME_ACCESS=n and change the corresponding test
accordingly. This check should be placed first.
Best regards
Heinrich
> + if (!(attributes & EFI_VARIABLE_RUNTIME_ACCESS))
> + return EFI_INVALID_PARAMETER;
> + if ((attributes & (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
> + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS |
> + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS)))
> + return EFI_UNSUPPORTED;
> +
> + return efi_query_variable_info_int(attributes,
> + maximum_variable_storage_size,
> + remaining_variable_storage_size,
> + maximum_variable_size);
> }
>
> /**
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index 4f1aa298da13..8b6b0a390869 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -873,6 +873,11 @@ efi_status_t efi_query_variable_info_int(u32 attributes,
> efi_status_t ret;
> u8 *comm_buf;
>
> + if (!max_variable_storage_size ||
> + !remain_variable_storage_size ||
> + !max_variable_size || !attributes)
> + return EFI_INVALID_PARAMETER;
> +
> payload_size = sizeof(*mm_query_info);
> comm_buf = setup_mm_hdr((void **)&mm_query_info, payload_size,
> SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO,
> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> index afa91be62c85..5794a7b2d405 100644
> --- a/lib/efi_selftest/efi_selftest_variables_runtime.c
> +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
> @@ -60,9 +60,17 @@ static int execute(void)
> ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS,
> &max_storage, &rem_storage,
> &max_size);
> - if (ret != EFI_UNSUPPORTED) {
> - efi_st_error("QueryVariableInfo failed\n");
> - return EFI_ST_FAILURE;
> +
> + if (IS_ENABLED(CONFIG_EFI_VARIABLE_FILE_STORE)) {
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("QueryVariableInfo failed\n");
> + return EFI_ST_FAILURE;
> + }
> + } else {
> + if (ret != EFI_UNSUPPORTED) {
> + efi_st_error("QueryVariableInfo failed\n");
> + return EFI_ST_FAILURE;
> + }
> }
>
> ret = runtime->set_variable(u"efi_st_var0", &guid_vendor0,
> --
> 2.40.1
>
More information about the U-Boot
mailing list