efi_loader: use event callback for initrd deregistration

Currently efi_initrd_deregister() is called in bootefi.c
when the image started from bootefi command returns.
Since efi_guid_event_group_return_to_efibootmgr event is
implemented, so let's use this event for invoking
initrd deregistration.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Note that this patch can be applied to u-boot/next.
This patch requires the patch e0d1a1ea68("efi_loader: add return to efibootmgr event group")

 cmd/bootefi.c                    |  5 --
 lib/efi_loader/efi_load_initrd.c | 82 +++++++++++++++++++++-----------
 2 files changed, 55 insertions(+), 32 deletions(-)


base-commit: 4a363dd516856344558034027e40d903868f67b4

Thanks for cleaning this up!

On Mon, 4 Dec 2023 at 06:31, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
>
> Currently efi_initrd_deregister() is called in bootefi.c
> when the image started from bootefi command returns.
> Since efi_guid_event_group_return_to_efibootmgr event is
> implemented, so let's use this event for invoking
> initrd deregistration.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Note that this patch can be applied to u-boot/next.
> This patch requires the patch e0d1a1ea68("efi_loader: add return to efibootmgr event group")
>
>  cmd/bootefi.c                    |  5 --
>  lib/efi_loader/efi_load_initrd.c | 82 +++++++++++++++++++++-----------
>  2 files changed, 55 insertions(+), 32 deletions(-)

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Tested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 4d74969ad6..e52bd3f743 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -390,11 +390,6 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
>  out:
>         free(load_options);
>
> -       if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> -               if (efi_initrd_deregister() != EFI_SUCCESS)
> -                       log_err("Failed to remove loadfile2 for initrd\n");
> -       }
> -
>         /* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
>         list_for_each_entry(evt, &efi_events, link) {
>                 if (evt->group &&
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index 193433782c..7ca7785c04 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -184,6 +184,50 @@ out:
>         return ret;
>  }
>
> +/**
> + * efi_initrd_deregister() - delete the handle for loading initial RAM disk
> + *
> + * This will delete the handle containing the Linux specific vendor device
> + * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> + *
> + * Return:     status code
> + */
> +efi_status_t efi_initrd_deregister(void)
> +{
> +       efi_status_t ret;
> +
> +       if (!efi_initrd_handle)
> +               return EFI_SUCCESS;
> +
> +       ret = efi_uninstall_multiple_protocol_interfaces(efi_initrd_handle,
> +                                                        /* initramfs */
> +                                                        &efi_guid_device_path,
> +                                                        &dp_lf2_handle,
> +                                                        /* LOAD_FILE2 */
> +                                                        &efi_guid_load_file2_protocol,
> +                                                        &efi_lf2_protocol,
> +                                                        NULL);
> +       efi_initrd_handle = NULL;
> +
> +       return ret;
> +}
> +
> +/**
> + * efi_initrd_return_notify() - return to efibootmgr callback
> + *
> + * @event:     the event for which this notification function is registered
> + * @context:   event context
> + */
> +static void EFIAPI efi_initrd_return_notify(struct efi_event *event,
> +                                                 void *context)
> +{
> +       efi_status_t ret;
> +
> +       EFI_ENTRY("%p, %p", event, context);
> +       ret = efi_initrd_deregister();
> +       EFI_EXIT(ret);
> +}
> +
>  /**
>   * efi_initrd_register() - create handle for loading initial RAM disk
>   *
> @@ -197,6 +241,7 @@ out:
>  efi_status_t efi_initrd_register(void)
>  {
>         efi_status_t ret;
> +       struct efi_event *event;
>
>         /*
>          * Allow the user to continue if Boot#### file path is not set for
> @@ -215,34 +260,17 @@ efi_status_t efi_initrd_register(void)
>                                                        &efi_guid_load_file2_protocol,
>                                                        &efi_lf2_protocol,
>                                                        NULL);
> +       if (ret != EFI_SUCCESS) {
> +               log_err("installing EFI_LOAD_FILE2_PROTOCOL failed\n");
> +               return ret;
> +       }
>
> -       return ret;
> -}
> -
> -/**
> - * efi_initrd_deregister() - delete the handle for loading initial RAM disk
> - *
> - * This will delete the handle containing the Linux specific vendor device
> - * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> - *
> - * Return:     status code
> - */
> -efi_status_t efi_initrd_deregister(void)
> -{
> -       efi_status_t ret;
> -
> -       if (!efi_initrd_handle)
> -               return EFI_SUCCESS;
> -
> -       ret = efi_uninstall_multiple_protocol_interfaces(efi_initrd_handle,
> -                                                        /* initramfs */
> -                                                        &efi_guid_device_path,
> -                                                        &dp_lf2_handle,
> -                                                        /* LOAD_FILE2 */
> -                                                        &efi_guid_load_file2_protocol,
> -                                                        &efi_lf2_protocol,
> -                                                        NULL);
> -       efi_initrd_handle = NULL;
> +       ret = efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> +                              efi_initrd_return_notify, NULL,
> +                              &efi_guid_event_group_return_to_efibootmgr,
> +                              &event);
> +       if (ret != EFI_SUCCESS)
> +               log_err("Creating event failed\n");
>
>         return ret;
>  }
>
> base-commit: 4a363dd516856344558034027e40d903868f67b4
> --
> 2.34.1
>

Kojima-san

Apologies but for some reason we failed to pull this patch....
In meantime some parts of the bootefi.c code -- specifically theinitrd
cleanup, have been moved to efi_helper.c

Any chance you can rebase this? If not I'll try to find some time to do it

/Ilias
On Mon, Dec 04, 2023 at 01:38:17PM +0200, Ilias Apalodimas wrote:
> Thanks for cleaning this up!
>
> On Mon, 4 Dec 2023 at 06:31, Masahisa Kojima <masahisa.kojima@linaro.org> wrote:
> >
> > Currently efi_initrd_deregister() is called in bootefi.c
> > when the image started from bootefi command returns.
> > Since efi_guid_event_group_return_to_efibootmgr event is
> > implemented, so let's use this event for invoking
> > initrd deregistration.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Note that this patch can be applied to u-boot/next.
> > This patch requires the patch e0d1a1ea68("efi_loader: add return to efibootmgr event group")
> >
> >  cmd/bootefi.c                    |  5 --
> >  lib/efi_loader/efi_load_initrd.c | 82 +++++++++++++++++++++-----------
> >  2 files changed, 55 insertions(+), 32 deletions(-)
>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Tested-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 4d74969ad6..e52bd3f743 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -390,11 +390,6 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options)
> >  out:
> >         free(load_options);
> >
> > -       if (IS_ENABLED(CONFIG_EFI_LOAD_FILE2_INITRD)) {
> > -               if (efi_initrd_deregister() != EFI_SUCCESS)
> > -                       log_err("Failed to remove loadfile2 for initrd\n");
> > -       }
> > -
> >         /* Notify EFI_EVENT_GROUP_RETURN_TO_EFIBOOTMGR event group. */
> >         list_for_each_entry(evt, &efi_events, link) {
> >                 if (evt->group &&
> > diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> > index 193433782c..7ca7785c04 100644
> > --- a/lib/efi_loader/efi_load_initrd.c
> > +++ b/lib/efi_loader/efi_load_initrd.c
> > @@ -184,6 +184,50 @@ out:
> >         return ret;
> >  }
> >
> > +/**
> > + * efi_initrd_deregister() - delete the handle for loading initial RAM disk
> > + *
> > + * This will delete the handle containing the Linux specific vendor device
> > + * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> > + *
> > + * Return:     status code
> > + */
> > +efi_status_t efi_initrd_deregister(void)
> > +{
> > +       efi_status_t ret;
> > +
> > +       if (!efi_initrd_handle)
> > +               return EFI_SUCCESS;
> > +
> > +       ret = efi_uninstall_multiple_protocol_interfaces(efi_initrd_handle,
> > +                                                        /* initramfs */
> > +                                                        &efi_guid_device_path,
> > +                                                        &dp_lf2_handle,
> > +                                                        /* LOAD_FILE2 */
> > +                                                        &efi_guid_load_file2_protocol,
> > +                                                        &efi_lf2_protocol,
> > +                                                        NULL);
> > +       efi_initrd_handle = NULL;
> > +
> > +       return ret;
> > +}
> > +
> > +/**
> > + * efi_initrd_return_notify() - return to efibootmgr callback
> > + *
> > + * @event:     the event for which this notification function is registered
> > + * @context:   event context
> > + */
> > +static void EFIAPI efi_initrd_return_notify(struct efi_event *event,
> > +                                                 void *context)
> > +{
> > +       efi_status_t ret;
> > +
> > +       EFI_ENTRY("%p, %p", event, context);
> > +       ret = efi_initrd_deregister();
> > +       EFI_EXIT(ret);
> > +}
> > +
> >  /**
> >   * efi_initrd_register() - create handle for loading initial RAM disk
> >   *
> > @@ -197,6 +241,7 @@ out:
> >  efi_status_t efi_initrd_register(void)
> >  {
> >         efi_status_t ret;
> > +       struct efi_event *event;
> >
> >         /*
> >          * Allow the user to continue if Boot#### file path is not set for
> > @@ -215,34 +260,17 @@ efi_status_t efi_initrd_register(void)
> >                                                        &efi_guid_load_file2_protocol,
> >                                                        &efi_lf2_protocol,
> >                                                        NULL);
> > +       if (ret != EFI_SUCCESS) {
> > +               log_err("installing EFI_LOAD_FILE2_PROTOCOL failed\n");
> > +               return ret;
> > +       }
> >
> > -       return ret;
> > -}
> > -
> > -/**
> > - * efi_initrd_deregister() - delete the handle for loading initial RAM disk
> > - *
> > - * This will delete the handle containing the Linux specific vendor device
> > - * path and EFI_LOAD_FILE2_PROTOCOL for loading an initrd
> > - *
> > - * Return:     status code
> > - */
> > -efi_status_t efi_initrd_deregister(void)
> > -{
> > -       efi_status_t ret;
> > -
> > -       if (!efi_initrd_handle)
> > -               return EFI_SUCCESS;
> > -
> > -       ret = efi_uninstall_multiple_protocol_interfaces(efi_initrd_handle,
> > -                                                        /* initramfs */
> > -                                                        &efi_guid_device_path,
> > -                                                        &dp_lf2_handle,
> > -                                                        /* LOAD_FILE2 */
> > -                                                        &efi_guid_load_file2_protocol,
> > -                                                        &efi_lf2_protocol,
> > -                                                        NULL);
> > -       efi_initrd_handle = NULL;
> > +       ret = efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> > +                              efi_initrd_return_notify, NULL,
> > +                              &efi_guid_event_group_return_to_efibootmgr,
> > +                              &event);
> > +       if (ret != EFI_SUCCESS)
> > +               log_err("Creating event failed\n");
> >
> >         return ret;
> >  }
> >
> > base-commit: 4a363dd516856344558034027e40d903868f67b4
> > --
> > 2.34.1
> >