Re: [PATCH 2/3] efi_selftest: add tests for QueryVariableInfo at runtime
Hi Heinrich,
On Wed, 24 Apr 2024 at 10:25, Heinrich Schuchardt 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
> > ---
> > 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 ..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
> > + *
> > + * This unit test checks common service across boottime/runtime
> > + */
> > +
> > +#include
> > +
> > +#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,
> > +_storage, _storage,
> > +_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, _storage,
> > +_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,
> > +_storage, NULL,
> > +_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,
> > +_storage, _storage,
> > +NULL);
> > + if (ret != EFI_INVALID_PARAMETER) {
> > + efi_st_error("QueryVariableInfo failed\n");
> > + return EFI_ST_FAILURE;
> > + }
> > +
> > + ret = runtime->query_variable_info(0, _storage, _storage,
> > +_size);
> > + if (ret != EFI_INVALID_PARAMETER) {
> > + efi_st_error("QueryVariableInfo failed\n");
> > + return EFI_ST_FAILURE;
> > + }
>
Re: [PATCH 2/3] efi_selftest: add tests for QueryVariableInfo at runtime
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
---
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 ..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
+ *
+ * This unit test checks common service across boottime/runtime
+ */
+
+#include
+
+#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,
+ _storage, _storage,
+ _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, _storage,
+ _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,
+ _storage, NULL,
+ _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,
+ _storage, _storage,
+ NULL);
+ if (ret != EFI_INVALID_PARAMETER) {
+ efi_st_error("QueryVariableInfo failed\n");
+ return EFI_ST_FAILURE;
+ }
+
+ ret = runtime->query_variable_info(0, _storage, _storage,
+ _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,
+ _storage, _storage,
+ _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, _storage,
+ _storage, _size);
+ if (ret !=
[PATCH 2/3] efi_selftest: add tests for QueryVariableInfo at runtime
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
---
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 ..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
+ *
+ * This unit test checks common service across boottime/runtime
+ */
+
+#include
+
+#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,
+ _storage, _storage,
+ _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, _storage,
+ _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,
+ _storage, NULL,
+ _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,
+ _storage, _storage,
+ NULL);
+ if (ret != EFI_INVALID_PARAMETER) {
+ efi_st_error("QueryVariableInfo failed\n");
+ return EFI_ST_FAILURE;
+ }
+
+ ret = runtime->query_variable_info(0, _storage, _storage,
+ _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,
+ _storage, _storage,
+ _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, _storage,
+ _storage, _size);
+ if (ret != EFI_UNSUPPORTED) {
+ efi_st_error("QueryVariableInfo failed\n");
+ return EFI_ST_FAILURE;
+ }
+
+ ret = runtime->query_variable_info(EFI_VARIABLE_RUNTIME_ACCESS,
_storage, _storage,
+ _size);
+ if (ret != EFI_INVALID_PARAMETER) {
+ efi_st_error("QueryVariableInfo failed\n");
+
