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

Heinrich Schuchardt
2020-04-01T06:37:38Z

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

>
> Then we should have more test cases.

Please, feel free to supply some.

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