Message ID | 20240424050351.318724-2-ilias.apalodimas@linaro.org |
---|---|
State | New |
Headers | show |
Series | [1/3] efi_loader: enable QueryVariableInfo at runtime for file backed variables | expand |
On 24.04.24 07:03, Ilias Apalodimas wrote: > Since we support QueryVariableInfo at runtime now add the relevant > tests. Since we want those to be reusable at bootime, add them > in a separate file > > Add tests for > - Test QueryVariableInfo returns EFI_SUCCESS > - Test null pointers for the function arguments > - Test invalid combination of attributes > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > include/efi_selftest.h | 9 ++ > lib/efi_selftest/Makefile | 1 + > .../efi_selftest_variables_common.c | 102 ++++++++++++++++++ > .../efi_selftest_variables_runtime.c | 10 +- > 4 files changed, 118 insertions(+), 4 deletions(-) > create mode 100644 lib/efi_selftest/efi_selftest_variables_common.c > > diff --git a/include/efi_selftest.h b/include/efi_selftest.h > index 5bcebb368287..ca7ae948663e 100644 > --- a/include/efi_selftest.h > +++ b/include/efi_selftest.h > @@ -147,6 +147,15 @@ void *efi_st_get_config_table(const efi_guid_t *guid); > */ > u16 efi_st_get_key(void); > > +/** > + * efi_st_query_variable_common - Common variable tests for boottime/runtime > + * > + * @runtime: Pointer to services table > + * > + * Return: EFI_ST_SUCCESS/FAILURE > + */ > +int efi_st_query_variable_common(struct efi_runtime_services *runtime); > + > /** > * struct efi_unit_test - EFI unit test > * > diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile > index e4d75420bff6..414701893f65 100644 > --- a/lib/efi_selftest/Makefile > +++ b/lib/efi_selftest/Makefile > @@ -45,6 +45,7 @@ efi_selftest_textinputex.o \ > efi_selftest_textoutput.o \ > efi_selftest_tpl.o \ > efi_selftest_util.o \ > +efi_selftest_variables_common.o \ > efi_selftest_variables.o \ > efi_selftest_variables_runtime.o \ > efi_selftest_watchdog.o > diff --git a/lib/efi_selftest/efi_selftest_variables_common.c b/lib/efi_selftest/efi_selftest_variables_common.c > new file mode 100644 > index 000000000000..36bdfe6ff9c3 > --- /dev/null > +++ b/lib/efi_selftest/efi_selftest_variables_common.c > @@ -0,0 +1,102 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * efi_selftest_variables_runtime > + * > + * Copyright (c) 2024 Ilias Apalodimas <ilias.apalodimas@linaro.org> > + * > + * This unit test checks common service across boottime/runtime > + */ > + > +#include <efi_selftest.h> > + > +#define EFI_INVALID_ATTR BIT(30) > + > +int efi_st_query_variable_common(struct efi_runtime_services *runtime) > +{ EDK2's VariableServiceQueryVariableInfo() in MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c has different attribute requirements at runtime than at boot services time. I guess QueryVariables() should return EFI_INVALID_PARAMETER whenever SetVariables() does. Best regards Heinrich Best regards Heinrich > + efi_status_t ret; > + u64 max_storage, rem_storage, max_size; > + > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > + &max_storage, &rem_storage, > + &max_size); > + if (ret != EFI_SUCCESS) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > + NULL, &rem_storage, > + &max_size); > + if (ret != EFI_INVALID_PARAMETER) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > + &max_storage, NULL, > + &max_size); > + if (ret != EFI_INVALID_PARAMETER) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > + &max_storage, &rem_storage, > + NULL); > + if (ret != EFI_INVALID_PARAMETER) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + ret = runtime->query_variable_info(0, &max_storage, &rem_storage, > + &max_size); > + if (ret != EFI_INVALID_PARAMETER) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + ret = runtime->query_variable_info(EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | > + EFI_VARIABLE_NON_VOLATILE, > + &max_storage, &rem_storage, > + &max_size); > + if (ret != EFI_UNSUPPORTED) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + /* Use an attribute bit not described in the EFI spec */ > + ret = runtime->query_variable_info(EFI_INVALID_ATTR, &max_storage, > + &rem_storage, &max_size); > + if (ret != EFI_UNSUPPORTED) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + ret = runtime->query_variable_info(EFI_VARIABLE_RUNTIME_ACCESS, &max_storage, &rem_storage, > + &max_size); > + if (ret != EFI_INVALID_PARAMETER) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + ret = runtime->query_variable_info(EFI_VARIABLE_NON_VOLATILE, &max_storage, &rem_storage, > + &max_size); > + if (ret != EFI_INVALID_PARAMETER) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + /* > + * Use a mix existing/non-existing attribute bits from the > + * UEFI spec > + */ > + ret = runtime->query_variable_info(EFI_INVALID_ATTR | EFI_VARIABLE_NON_VOLATILE, > + &max_storage, &rem_storage, > + &max_size); > + if (ret != EFI_INVALID_PARAMETER) { > + efi_st_error("QueryVariableInfo failed\n"); > + return EFI_ST_FAILURE; > + } > + > + return EFI_ST_SUCCESS; > +} > diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c > index 5794a7b2d405..6cb86771fe78 100644 > --- a/lib/efi_selftest/efi_selftest_variables_runtime.c > +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c > @@ -55,18 +55,20 @@ static int execute(void) > u16 varname[EFI_ST_MAX_VARNAME_SIZE]; > efi_guid_t guid; > u64 max_storage, rem_storage, max_size; > + int test_ret; > > memset(v2, 0x1, sizeof(v2)); > - ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > - &max_storage, &rem_storage, > - &max_size); > > if (IS_ENABLED(CONFIG_EFI_VARIABLE_FILE_STORE)) { > - if (ret != EFI_SUCCESS) { > + test_ret = efi_st_query_variable_common(runtime); > + if (test_ret != EFI_ST_SUCCESS) { > efi_st_error("QueryVariableInfo failed\n"); > return EFI_ST_FAILURE; > } > } else { > + 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;
Hi Heinrich, On Wed, 24 Apr 2024 at 10:25, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 24.04.24 07:03, Ilias Apalodimas wrote: > > Since we support QueryVariableInfo at runtime now add the relevant > > tests. Since we want those to be reusable at bootime, add them > > in a separate file > > > > Add tests for > > - Test QueryVariableInfo returns EFI_SUCCESS > > - Test null pointers for the function arguments > > - Test invalid combination of attributes > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > include/efi_selftest.h | 9 ++ > > lib/efi_selftest/Makefile | 1 + > > .../efi_selftest_variables_common.c | 102 ++++++++++++++++++ > > .../efi_selftest_variables_runtime.c | 10 +- > > 4 files changed, 118 insertions(+), 4 deletions(-) > > create mode 100644 lib/efi_selftest/efi_selftest_variables_common.c > > > > diff --git a/include/efi_selftest.h b/include/efi_selftest.h > > index 5bcebb368287..ca7ae948663e 100644 > > --- a/include/efi_selftest.h > > +++ b/include/efi_selftest.h > > @@ -147,6 +147,15 @@ void *efi_st_get_config_table(const efi_guid_t *guid); > > */ > > u16 efi_st_get_key(void); > > > > +/** > > + * efi_st_query_variable_common - Common variable tests for boottime/runtime > > + * > > + * @runtime: Pointer to services table > > + * > > + * Return: EFI_ST_SUCCESS/FAILURE > > + */ > > +int efi_st_query_variable_common(struct efi_runtime_services *runtime); > > + > > /** > > * struct efi_unit_test - EFI unit test > > * > > diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile > > index e4d75420bff6..414701893f65 100644 > > --- a/lib/efi_selftest/Makefile > > +++ b/lib/efi_selftest/Makefile > > @@ -45,6 +45,7 @@ efi_selftest_textinputex.o \ > > efi_selftest_textoutput.o \ > > efi_selftest_tpl.o \ > > efi_selftest_util.o \ > > +efi_selftest_variables_common.o \ > > efi_selftest_variables.o \ > > efi_selftest_variables_runtime.o \ > > efi_selftest_watchdog.o > > diff --git a/lib/efi_selftest/efi_selftest_variables_common.c b/lib/efi_selftest/efi_selftest_variables_common.c > > new file mode 100644 > > index 000000000000..36bdfe6ff9c3 > > --- /dev/null > > +++ b/lib/efi_selftest/efi_selftest_variables_common.c > > @@ -0,0 +1,102 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * efi_selftest_variables_runtime > > + * > > + * Copyright (c) 2024 Ilias Apalodimas <ilias.apalodimas@linaro.org> > > + * > > + * This unit test checks common service across boottime/runtime > > + */ > > + > > +#include <efi_selftest.h> > > + > > +#define EFI_INVALID_ATTR BIT(30) > > + > > +int efi_st_query_variable_common(struct efi_runtime_services *runtime) > > +{ > > EDK2's VariableServiceQueryVariableInfo() in > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c has different > attribute requirements at runtime than at boot services time. > > I guess QueryVariables() should return EFI_INVALID_PARAMETER whenever > SetVariables() does. I think that's an easy fix. We can still keep a common file, but also pass the attributes as a function argument depending on bootime/runtime I'll send a v2. Thanks /Ilias > > Best regards > > Heinrich > > Best regards > > Heinrich > > > + efi_status_t ret; > > + u64 max_storage, rem_storage, max_size; > > + > > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > > + &max_storage, &rem_storage, > > + &max_size); > > + if (ret != EFI_SUCCESS) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > > + NULL, &rem_storage, > > + &max_size); > > + if (ret != EFI_INVALID_PARAMETER) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > > + &max_storage, NULL, > > + &max_size); > > + if (ret != EFI_INVALID_PARAMETER) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > > + &max_storage, &rem_storage, > > + NULL); > > + if (ret != EFI_INVALID_PARAMETER) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + ret = runtime->query_variable_info(0, &max_storage, &rem_storage, > > + &max_size); > > + if (ret != EFI_INVALID_PARAMETER) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + ret = runtime->query_variable_info(EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | > > + EFI_VARIABLE_NON_VOLATILE, > > + &max_storage, &rem_storage, > > + &max_size); > > + if (ret != EFI_UNSUPPORTED) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + /* Use an attribute bit not described in the EFI spec */ > > + ret = runtime->query_variable_info(EFI_INVALID_ATTR, &max_storage, > > + &rem_storage, &max_size); > > + if (ret != EFI_UNSUPPORTED) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + ret = runtime->query_variable_info(EFI_VARIABLE_RUNTIME_ACCESS, &max_storage, &rem_storage, > > + &max_size); > > + if (ret != EFI_INVALID_PARAMETER) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + ret = runtime->query_variable_info(EFI_VARIABLE_NON_VOLATILE, &max_storage, &rem_storage, > > + &max_size); > > + if (ret != EFI_INVALID_PARAMETER) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + /* > > + * Use a mix existing/non-existing attribute bits from the > > + * UEFI spec > > + */ > > + ret = runtime->query_variable_info(EFI_INVALID_ATTR | EFI_VARIABLE_NON_VOLATILE, > > + &max_storage, &rem_storage, > > + &max_size); > > + if (ret != EFI_INVALID_PARAMETER) { > > + efi_st_error("QueryVariableInfo failed\n"); > > + return EFI_ST_FAILURE; > > + } > > + > > + return EFI_ST_SUCCESS; > > +} > > diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c > > index 5794a7b2d405..6cb86771fe78 100644 > > --- a/lib/efi_selftest/efi_selftest_variables_runtime.c > > +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c > > @@ -55,18 +55,20 @@ static int execute(void) > > u16 varname[EFI_ST_MAX_VARNAME_SIZE]; > > efi_guid_t guid; > > u64 max_storage, rem_storage, max_size; > > + int test_ret; > > > > memset(v2, 0x1, sizeof(v2)); > > - ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, > > - &max_storage, &rem_storage, > > - &max_size); > > > > if (IS_ENABLED(CONFIG_EFI_VARIABLE_FILE_STORE)) { > > - if (ret != EFI_SUCCESS) { > > + test_ret = efi_st_query_variable_common(runtime); > > + if (test_ret != EFI_ST_SUCCESS) { > > efi_st_error("QueryVariableInfo failed\n"); > > return EFI_ST_FAILURE; > > } > > } else { > > + 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; >
diff --git a/include/efi_selftest.h b/include/efi_selftest.h index 5bcebb368287..ca7ae948663e 100644 --- a/include/efi_selftest.h +++ b/include/efi_selftest.h @@ -147,6 +147,15 @@ void *efi_st_get_config_table(const efi_guid_t *guid); */ u16 efi_st_get_key(void); +/** + * efi_st_query_variable_common - Common variable tests for boottime/runtime + * + * @runtime: Pointer to services table + * + * Return: EFI_ST_SUCCESS/FAILURE + */ +int efi_st_query_variable_common(struct efi_runtime_services *runtime); + /** * struct efi_unit_test - EFI unit test * diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index e4d75420bff6..414701893f65 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -45,6 +45,7 @@ efi_selftest_textinputex.o \ efi_selftest_textoutput.o \ efi_selftest_tpl.o \ efi_selftest_util.o \ +efi_selftest_variables_common.o \ efi_selftest_variables.o \ efi_selftest_variables_runtime.o \ efi_selftest_watchdog.o diff --git a/lib/efi_selftest/efi_selftest_variables_common.c b/lib/efi_selftest/efi_selftest_variables_common.c new file mode 100644 index 000000000000..36bdfe6ff9c3 --- /dev/null +++ b/lib/efi_selftest/efi_selftest_variables_common.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * efi_selftest_variables_runtime + * + * Copyright (c) 2024 Ilias Apalodimas <ilias.apalodimas@linaro.org> + * + * This unit test checks common service across boottime/runtime + */ + +#include <efi_selftest.h> + +#define EFI_INVALID_ATTR BIT(30) + +int efi_st_query_variable_common(struct efi_runtime_services *runtime) +{ + efi_status_t ret; + u64 max_storage, rem_storage, max_size; + + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, + &max_storage, &rem_storage, + &max_size); + if (ret != EFI_SUCCESS) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, + NULL, &rem_storage, + &max_size); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, + &max_storage, NULL, + &max_size); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, + &max_storage, &rem_storage, + NULL); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + ret = runtime->query_variable_info(0, &max_storage, &rem_storage, + &max_size); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + ret = runtime->query_variable_info(EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | + EFI_VARIABLE_NON_VOLATILE, + &max_storage, &rem_storage, + &max_size); + if (ret != EFI_UNSUPPORTED) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + /* Use an attribute bit not described in the EFI spec */ + ret = runtime->query_variable_info(EFI_INVALID_ATTR, &max_storage, + &rem_storage, &max_size); + if (ret != EFI_UNSUPPORTED) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + ret = runtime->query_variable_info(EFI_VARIABLE_RUNTIME_ACCESS, &max_storage, &rem_storage, + &max_size); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + ret = runtime->query_variable_info(EFI_VARIABLE_NON_VOLATILE, &max_storage, &rem_storage, + &max_size); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + /* + * Use a mix existing/non-existing attribute bits from the + * UEFI spec + */ + ret = runtime->query_variable_info(EFI_INVALID_ATTR | EFI_VARIABLE_NON_VOLATILE, + &max_storage, &rem_storage, + &max_size); + if (ret != EFI_INVALID_PARAMETER) { + efi_st_error("QueryVariableInfo failed\n"); + return EFI_ST_FAILURE; + } + + return EFI_ST_SUCCESS; +} diff --git a/lib/efi_selftest/efi_selftest_variables_runtime.c b/lib/efi_selftest/efi_selftest_variables_runtime.c index 5794a7b2d405..6cb86771fe78 100644 --- a/lib/efi_selftest/efi_selftest_variables_runtime.c +++ b/lib/efi_selftest/efi_selftest_variables_runtime.c @@ -55,18 +55,20 @@ static int execute(void) u16 varname[EFI_ST_MAX_VARNAME_SIZE]; efi_guid_t guid; u64 max_storage, rem_storage, max_size; + int test_ret; memset(v2, 0x1, sizeof(v2)); - ret = runtime->query_variable_info(EFI_VARIABLE_BOOTSERVICE_ACCESS, - &max_storage, &rem_storage, - &max_size); if (IS_ENABLED(CONFIG_EFI_VARIABLE_FILE_STORE)) { - if (ret != EFI_SUCCESS) { + test_ret = efi_st_query_variable_common(runtime); + if (test_ret != EFI_ST_SUCCESS) { efi_st_error("QueryVariableInfo failed\n"); return EFI_ST_FAILURE; } } else { + 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;
Since we support QueryVariableInfo at runtime now add the relevant tests. Since we want those to be reusable at bootime, add them in a separate file Add tests for - Test QueryVariableInfo returns EFI_SUCCESS - Test null pointers for the function arguments - Test invalid combination of attributes Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- include/efi_selftest.h | 9 ++ lib/efi_selftest/Makefile | 1 + .../efi_selftest_variables_common.c | 102 ++++++++++++++++++ .../efi_selftest_variables_runtime.c | 10 +- 4 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 lib/efi_selftest/efi_selftest_variables_common.c