[PATCH] efi_selftests: fix controllers repeated selftesting

Running the controller selftest more than one times fails with
=> setenv efi_selftest 'controllers' && bootefi selftest Testing EFI API implementation Selected test: 'controllers' Setting up 'controllers' Setting up 'controllers' succeeded Executing 'controllers' Executing 'controllers' succeeded Summary: 0 failures
=> bootefi selftest Testing EFI API implementation Selected test: 'controllers' Setting up 'controllers' lib/efi_selftest/efi_selftest_controllers.c(280): ERROR: InstallProtocolInterface failed lib/efi_selftest/efi_selftest.c(89): ERROR: Setting up 'controllers' failed Summary: 1 failures
There are multiple reason for this. We don't uninstall the binding interface from the controller handle and we don't reset the handle pointers either. So let's uninstall all the protocols properly and reset the handles to NULL on setup().
While at it add a forgotten check when uninstalling protocols from the handle_controller and make sure the number of child controllers is 0
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_selftest/efi_selftest_controllers.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/lib/efi_selftest/efi_selftest_controllers.c b/lib/efi_selftest/efi_selftest_controllers.c index d2bbd1c4f65b..07a17ed866a9 100644 --- a/lib/efi_selftest/efi_selftest_controllers.c +++ b/lib/efi_selftest/efi_selftest_controllers.c @@ -271,6 +271,8 @@ static int setup(const efi_handle_t img_handle, efi_status_t ret;
boottime = systable->boottime; + handle_controller = NULL; + handle_driver = NULL;
/* Create controller handle */ ret = boottime->install_protocol_interface( @@ -402,8 +404,18 @@ static int execute(void) /* Check number of child controllers */ ret = count_child_controllers(handle_controller, &guid_controller, &count); - if (ret == EFI_SUCCESS) + if (ret == EFI_SUCCESS || count != 0) efi_st_error("Uninstall failed\n"); + + /* Uninstall binding protocol */ + ret = boottime->uninstall_protocol_interface(handle_driver, + &guid_driver_binding_protocol, + &binding_interface); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to uninstall protocols\n"); + return EFI_ST_FAILURE; + } + return EFI_ST_SUCCESS; }

Running the protocols selftest more than one times fails with
=> setenv efi_selftest 'manage protocols' && bootefi selftest Testing EFI API implementation Selected test: 'manage protocols' Setting up 'manage protocols' Setting up 'manage protocols' succeeded Executing 'manage protocols' Executing 'manage protocols' succeeded Tearing down 'manage protocols' Tearing down 'manage protocols' succeeded Summary: 0 failures
=> bootefi selftest Testing EFI API implementation Selected test: 'manage protocols' Setting up 'manage protocols' lib/efi_selftest/efi_selftest_manageprotocols.c(88): ERROR: InstallProtocolInterface failed lib/efi_selftest/efi_selftest.c(89): ERROR: Setting up 'manage protocols' failed Tearing down 'manage protocols' Tearing down 'manage protocols' succeeded Summary: 1 failures
The reason is that we don't set the handles to NULL after deleting and freeing them. As a result the subsequent protocol installation will try to use an existing handle which we just removed that from our object list.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- lib/efi_selftest/efi_selftest_manageprotocols.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_manageprotocols.c b/lib/efi_selftest/efi_selftest_manageprotocols.c index 8edb1e4d4671..097b2ae35456 100644 --- a/lib/efi_selftest/efi_selftest_manageprotocols.c +++ b/lib/efi_selftest/efi_selftest_manageprotocols.c @@ -79,6 +79,8 @@ static int setup(const efi_handle_t img_handle, efi_status_t ret; efi_handle_t handle;
+ handle1 = NULL; + handle2 = NULL; boottime = systable->boottime;
ret = boottime->install_protocol_interface(&handle1, &guid3,

On 6/13/23 15:23, Ilias Apalodimas wrote:
Running the protocols selftest more than one times fails with
=> setenv efi_selftest 'manage protocols' && bootefi selftest Testing EFI API implementation Selected test: 'manage protocols' Setting up 'manage protocols' Setting up 'manage protocols' succeeded Executing 'manage protocols' Executing 'manage protocols' succeeded Tearing down 'manage protocols' Tearing down 'manage protocols' succeeded Summary: 0 failures
=> bootefi selftest Testing EFI API implementation Selected test: 'manage protocols' Setting up 'manage protocols' lib/efi_selftest/efi_selftest_manageprotocols.c(88): ERROR: InstallProtocolInterface failed lib/efi_selftest/efi_selftest.c(89): ERROR: Setting up 'manage protocols' failed Tearing down 'manage protocols' Tearing down 'manage protocols' succeeded Summary: 1 failures
The reason is that we don't set the handles to NULL after deleting and freeing them. As a result the subsequent protocol installation will try to use an existing handle which we just removed that from our object list.
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de

On 6/13/23 15:23, Ilias Apalodimas wrote:
Running the controller selftest more than one times fails with
=> setenv efi_selftest 'controllers' && bootefi selftest Testing EFI API implementation Selected test: 'controllers' Setting up 'controllers' Setting up 'controllers' succeeded Executing 'controllers' Executing 'controllers' succeeded Summary: 0 failures
=> bootefi selftest Testing EFI API implementation Selected test: 'controllers' Setting up 'controllers' lib/efi_selftest/efi_selftest_controllers.c(280): ERROR: InstallProtocolInterface failed lib/efi_selftest/efi_selftest.c(89): ERROR: Setting up 'controllers' failed Summary: 1 failures
There are multiple reason for this. We don't uninstall the binding interface from the controller handle and we don't reset the handle pointers either. So let's uninstall all the protocols properly and reset the handles to NULL on setup().
While at it add a forgotten check when uninstalling protocols from the handle_controller and make sure the number of child controllers is 0
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_selftest/efi_selftest_controllers.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/lib/efi_selftest/efi_selftest_controllers.c b/lib/efi_selftest/efi_selftest_controllers.c index d2bbd1c4f65b..07a17ed866a9 100644 --- a/lib/efi_selftest/efi_selftest_controllers.c +++ b/lib/efi_selftest/efi_selftest_controllers.c @@ -271,6 +271,8 @@ static int setup(const efi_handle_t img_handle, efi_status_t ret;
boottime = systable->boottime;
- handle_controller = NULL;
- handle_driver = NULL;
Other instances of the same problem could be in
lib/efi_selftest/efi_selftest_open_protocol.c lib/efi_selftest/efi_selftest_loadimage.c lib/efi_selftest/efi_selftest_manageprotocols.c
We should probably try for each test if it can be repeated.
/* Create controller handle */ ret = boottime->install_protocol_interface( @@ -402,8 +404,18 @@ static int execute(void) /* Check number of child controllers */ ret = count_child_controllers(handle_controller, &guid_controller, &count);
- if (ret == EFI_SUCCESS)
- if (ret == EFI_SUCCESS || count != 0) efi_st_error("Uninstall failed\n");
- /* Uninstall binding protocol */
- ret = boottime->uninstall_protocol_interface(handle_driver,
&guid_driver_binding_protocol,
&binding_interface);
- if (ret != EFI_SUCCESS) {
efi_st_error("Failed to uninstall protocols\n");
return EFI_ST_FAILURE;
- }
Could this easily be moved to teardown()?
Best regards
Heinrich
return EFI_ST_SUCCESS; }

Hi Heinrich,
On Tue, 13 Jun 2023 at 18:00, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 6/13/23 15:23, Ilias Apalodimas wrote:
Running the controller selftest more than one times fails with
=> setenv efi_selftest 'controllers' && bootefi selftest Testing EFI API implementation Selected test: 'controllers' Setting up 'controllers' Setting up 'controllers' succeeded Executing 'controllers' Executing 'controllers' succeeded Summary: 0 failures
=> bootefi selftest Testing EFI API implementation Selected test: 'controllers' Setting up 'controllers' lib/efi_selftest/efi_selftest_controllers.c(280): ERROR: InstallProtocolInterface failed lib/efi_selftest/efi_selftest.c(89): ERROR: Setting up 'controllers' failed Summary: 1 failures
There are multiple reason for this. We don't uninstall the binding interface from the controller handle and we don't reset the handle pointers either. So let's uninstall all the protocols properly and reset the handles to NULL on setup().
While at it add a forgotten check when uninstalling protocols from the handle_controller and make sure the number of child controllers is 0
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
lib/efi_selftest/efi_selftest_controllers.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/lib/efi_selftest/efi_selftest_controllers.c b/lib/efi_selftest/efi_selftest_controllers.c index d2bbd1c4f65b..07a17ed866a9 100644 --- a/lib/efi_selftest/efi_selftest_controllers.c +++ b/lib/efi_selftest/efi_selftest_controllers.c @@ -271,6 +271,8 @@ static int setup(const efi_handle_t img_handle, efi_status_t ret;
boottime = systable->boottime;
handle_controller = NULL;
handle_driver = NULL;
Other instances of the same problem could be in
lib/efi_selftest/efi_selftest_open_protocol.c lib/efi_selftest/efi_selftest_loadimage.c lib/efi_selftest/efi_selftest_manageprotocols.c
We should probably try for each test if it can be repeated.
I've fixed the manageprotocols in a subsequent patch. The rest are probably problematic as well, but I was planning to deal with them after I am done with the protocol refactoring.
/* Create controller handle */ ret = boottime->install_protocol_interface(
@@ -402,8 +404,18 @@ static int execute(void) /* Check number of child controllers */ ret = count_child_controllers(handle_controller, &guid_controller, &count);
if (ret == EFI_SUCCESS)
if (ret == EFI_SUCCESS || count != 0) efi_st_error("Uninstall failed\n");
/* Uninstall binding protocol */
ret = boottime->uninstall_protocol_interface(handle_driver,
&guid_driver_binding_protocol,
&binding_interface);
if (ret != EFI_SUCCESS) {
efi_st_error("Failed to uninstall protocols\n");
return EFI_ST_FAILURE;
}
Could this easily be moved to teardown()?
Yea, I don't think this will be a problem
Thanks /Ilias
Best regards
Heinrich
return EFI_ST_SUCCESS;
}
participants (2)
-
Heinrich Schuchardt
-
Ilias Apalodimas