[PATCH 07/12] efi: selftest: add runtime variable tests with non-volatile storage

[Simon Glass]

Hi Harsimran,

On 2026-04-24T17:31:50, Harsimran Singh Tungal
<harsimransingh.tungal at arm.com> wrote:
> efi: selftest: add runtime variable tests with non-volatile storage
>
> Extend runtime variable tests for persistent storage
>
> Previously, EFI selftesting of runtime variables was only supported
> under CONFIG_EFI_RT_VOLATILE_STORE, which uses VarToFile to simulate
> non-volatile variable storage. This commit adds new test cases that
> exercise runtime variable operations for persistent storage.
>
> Features tested:
> - Creation of runtime-accessible variables (set)
> - Retrieval of runtime variables (get)
> - Deletion using SetVariable() with size = 0
> - Append operation using EFI_VARIABLE_APPEND_WRITE
>
> This improves EFI compliance validation for non-volatile runtime storage
> scenarios and ensures proper attribute enforcement and variable
> management.
>
> Signed-off-by: Harsimran Singh Tungal <harsimransingh.tungal at arm.com>
>
> lib/efi_selftest/efi_selftest_variables_runtime.c | 106 +++++++++++++++++++++-
>  1 file changed, 104 insertions(+), 2 deletions(-)

> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> @@ -39,6 +46,80 @@ static int execute(void)
> +     /* Compare the value of EFI variable if it already exists in non volatile storage */
> +     if (!IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
> +             len = sizeof(v) / 2;
> +             ret = st_runtime->get_variable(u'efi_st_var0', &guid_vendor0,
> +                             &attr, &len, data);
> +             if (ret == EFI_SUCCESS) {

The two-phase split worries me. As written, phase 1 creates a
non-volatile variable and immediately returns EFI_ST_SUCCESS, so a
single 'bootefi selftest' run reports a green test even though none of
the new assertions (append, delete, re-get) actually ran. It also
leaves 'efi_st_var0' behind in NV storage if the user never reboots
(or if phase 2 fails on an early assertion before the final delete).
Could you split this into two distinct EFI_UNIT_TEST entries - say
'variables at runtime (setup)' and 'variables at runtime (verify)'.
Then the user (and CI) explicitly select each phase, and the
persistent state across the two is part of the contract rather than a
hidden side-effect. At minimum, phase 1 needs to print a clear 'test
incomplete, please reboot and rerun' rather than EFI_ST_SUCCESS.

> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> @@ -39,6 +46,80 @@ static int execute(void)
> +                     return EFI_ST_SUCCESS;
> +             } else {
> +                     if (ret == EFI_NOT_FOUND) {
> +                             efi_st_printf("EFI Variable efi_st_var0 not found. "
> +                                     "Executing First Phase\n");
> +                     }
> +             }
> +     }

Any return value other than EFI_SUCCESS or EFI_NOT_FOUND
(EFI_DEVICE_ERROR, EFI_UNSUPPORTED on a platform without an FF-A
backend, etc.) silently falls through to the volatile-store path below
and mis-diagnoses the failure. Please fail explicitly when ret is not
one of the two expected values.

> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> @@ -279,7 +360,28 @@ static int execute(void)
>       } else {
> -             if (ret != EFI_UNSUPPORTED) {
> +             if (ret != EFI_SUCCESS) {

This flips the contract: previously the !CONFIG_EFI_RT_VOLATILE_STORE
branch asserted that runtime SetVariable() returns EFI_UNSUPPORTED,
which is what every board without a runtime variable backend still
does. After this patch any such board fails 'variables at runtime'.
Please gate the new behaviour on something like CONFIG_EFI_MM_COMM_TEE
(or whatever indicates a working runtime backend) and keep the
EFI_UNSUPPORTED expectation otherwise.

> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> @@ -39,6 +46,80 @@ static int execute(void)
> +                     /* Append an existing variable */
> +                     append_len = sizeof(v) - len;
> +                     ret = st_runtime->set_variable(u'efi_st_var0', &guid_vendor0,
> +                                                 EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +                                                 EFI_VARIABLE_RUNTIME_ACCESS |
> +                                                 EFI_VARIABLE_APPEND_WRITE |
> +                                                 EFI_VARIABLE_NON_VOLATILE,
> +                                                 append_len, (v + len));

Not sure if it is possible to align these lines properly? Not related
to this patch, but the EFI_VARIABLE_... prefix is really too long.

Drop redundant parens around v + len

> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> @@ -39,6 +46,80 @@ static int execute(void)
> +                     if (memcmp(data, v, len)) {
> +                             efi_st_error("GetVariable failed\n");
> +                             return EFI_ST_FAILURE;
> +                     }

Quite a lot of separate sites in this hunk print 'GetVariable failed'
or 'SetVariable failed' with no further context - when one trips in CI
it's impossible to tell which assertion fired without rebuilding with
prints. Please make each message identify the step (e.g. 'phase 2:
append SetVariable failed', 'phase 2: post-delete GetVariable did not
return EFI_NOT_FOUND') so a log line is enough to localise the
failure.

> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> @@ -3,6 +3,7 @@
> + * Copyright (c) 2026 Arm Limited and/or its affiliates <open-source-office at arm.com>
> + *
>   * This unit test checks the runtime services for variables after
>   * ExitBootServices():
> @@ -22,7 +23,13 @@ static const efi_guid_t efi_rt_var_guid = U_BOOT_EFI_RT_VAR_FILE_GUID;
> + * For EFI variables in non-volatile storage, these tests have to be executed in two phases

Please use 'non-volatile' (hyphenated) consistently in the doc comment
and in the inline comment a few lines down ('non volatile storage');
the rest of the file already spells it that way. Same nit for the
commit message body. The commit message also opens with 'Extend
runtime variable tests for persistent storage' that just paraphrases
the subject - please just drop it. 'Previously, EFI selftesting … was
only supported …' should be present tense per U-Boot conventions ('EFI
selftesting of runtime variables is currently only supported …').

Regards,
Simon