[PATCH] efi_selftests: fix controllers repeated selftesting

[Ilias Apalodimas]

Hi Heinrich,

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