[PATCH 16/16] efi_selftest: adjust runtime test for variables

AKASHI Takahiro
2020-04-01T07:27:16Z

On Wed, Apr 01, 2020 at 08:37:38AM +0200, Heinrich Schuchardt wrote:
> On 4/1/20 3:05 AM, AKASHI Takahiro wrote:
> > On Tue, Mar 31, 2020 at 08:07:39AM +0200, Heinrich Schuchardt wrote:
> >> As variable services are available at runtime we have to expect EFI_SUCCESS
> >> when calling the services.
> >
> > Why not run variables test *after* ExitBootServices as well as
> > *before* that event?
>
> We have a separate test for before ExitBootServices
> (lib/efi_selftest/efi_selftest_variables.c).

Ah, I misundersttood the file to be patched.
(EXECUTE_BEFORE_BOOTTIME_EXIT and SETUP_BEFORE_BOOTTIME_EXIT
are quite confusing.)

> >
> > Then we should have more test cases.
>
> Please, feel free to supply some.

?
You re-invented UEFI variable implementation almost from the scratch,
then you should cover as many corner cases as possible in general.

-Takahiro Akashi

> Unfortunately SCT does not provide runtime testing. I have been running
> my code successfully against Ubuntu's firmware test suite
> (https://git.launchpad.net/fwts).
>
> >
> >> Check the SetVariable() only succeeds with EFI_VARIABLE_RUNTIME_ACCESS and
> >> EFI_VARIABLE_NON_VOLATILE set.
> >
> > I'm not sure if this claim(check) is true.
> > At least, you need provide more description about specific test conditions.
>
> At runtime non-volatile variables and variables without runtime access
> cannot be changed according to the UEFI spec.

> Best regards
>
> Heinrich
>
> >
> > -Takahiro Akashi
> >
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >> .../efi_selftest_variables_runtime.c | 47 +++++++++++++++----
> >> 1 file changed, 39 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c
> >> index b3b40ad2cf..c6005eeeaf 100644
> >> --- a/lib/efi_selftest/efi_selftest_variables_runtime.c
> >> +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c
> >> @@ -20,8 +20,8 @@ static const efi_guid_t guid_vendor0 =
> >> EFI_GUID(0x67029eb5, 0x0af2, 0xf6b1,
> >> 0xda, 0x53, 0xfc, 0xb5, 0x66, 0xdd, 0x1c, 0xe6);
> >>
> >> -/*
> >> - * Setup unit test.
> >> +/**
> >> + * setup() - set up unit test.
> >> *
> >> * @handle handle of the loaded image
> >> * @systable system table
> >> @@ -38,7 +38,7 @@ static int setup(const efi_handle_t img_handle,
> >> /**
> >> * execute() - execute unit test
> >> *
> >> - * As runtime support is not implmented expect EFI_UNSUPPORTED to be returned.
> >> + * Test variable services at runtime.
> >> */
> >> static int execute(void)
> >> {
> >> @@ -52,37 +52,68 @@ static int execute(void)
> >> efi_guid_t guid;
> >> u64 max_storage, rem_storage, max_size;
> >>
> >> - ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS,
> >> + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >> + EFI_VARIABLE_RUNTIME_ACCESS |
> >> + EFI_VARIABLE_NON_VOLATILE,
> >> &max_storage, &rem_storage,
> >> &max_size);
> >> - if (ret != EFI_UNSUPPORTED) {
> >> + if (ret != EFI_SUCCESS) {
> >> efi_st_error("QueryVariableInfo failed\n");
> >> return EFI_ST_FAILURE;
> >> }
> >>
> >> + ret = runtime->set_variable(L"efi_st_var0", &guid_vendor0,
> >> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >> + EFI_VARIABLE_NON_VOLATILE,
> >> + 3, v + 4);
> >> + if (ret != EFI_INVALID_PARAMETER) {
> >> + efi_st_error("SetVariable succeeded w/o EFI_VARIABLE_RUNTIME_ACCESS\n");
> >> + return EFI_ST_FAILURE;
> >> + }
> >> +
> >> ret = runtime->set_variable(L"efi_st_var0", &guid_vendor0,
> >> EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >> EFI_VARIABLE_RUNTIME_ACCESS,
> >> 3, v + 4);
> >> - if (ret != EFI_UNSUPPORTED) {
> >> + if (ret != EFI_INVALID_PARAMETER) {
> >> + efi_st_error("SetVariable succeeded w/o EFI_VARIABLE_NON_VOLATILE\n");
> >> + return EFI_ST_FAILURE;
> >> + }
> >> +
> >> + ret = runtime->set_variable(L"efi_st_var0", &guid_vendor0,
> >> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
> >> + EFI_VARIABLE_RUNTIME_ACCESS |
> >> + EFI_VARIABLE_NON_VOLATILE,
> >> + 3, v + 4);
> >> + if (ret != EFI_SUCCESS) {
> >> efi_st_error("SetVariable failed\n");
> >> return EFI_ST_FAILURE;
> >> }
> >> +
> >> len = 3;
> >> ret = runtime->get_variable(L"efi_st_var0", &guid_vendor0,
> >> &attr, &len, data);
> >> - if (ret != EFI_UNSUPPORTED) {
> >> + if (ret != EFI_SUCCESS) {
> >> efi_st_error("GetVariable failed\n");
> >> return EFI_ST_FAILURE;
> >> }
> >> +
> >> memset(&guid, 0, 16);
> >> *varname = 0;
> >> + len = EFI_ST_MAX_VARNAME_SIZE * sizeof(u16);
> >> ret = runtime->get_next_variable_name(&len, varname, &guid);
> >> - if (ret != EFI_UNSUPPORTED) {
> >> + if (ret != EFI_SUCCESS) {
> >> efi_st_error("GetNextVariableName failed\n");
> >> return EFI_ST_FAILURE;
> >> }
> >>
> >> + ret = runtime->set_variable(L"efi_st_var0", &guid_vendor0, 0,
> >> + 3, v + 4);
> >> + if (ret != EFI_SUCCESS) {
> >> + efi_st_error("Variable deletion failed\n");
> >> + return EFI_ST_FAILURE;
> >> + }
> >> +
> >> return EFI_ST_SUCCESS;
> >> }
> >>
> >> --
> >> 2.25.1
> >>
>