Hi Heinrich,
On Tue, 13 Jun 2023 at 18:00, Heinrich Schuchardt <xypron.g...@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.apalodi...@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;
> > }
> >
>
