[PATCH v2 2/2] efi_loader: Reset system after CapsuleUpdate on disk

[Masami Hiramatsu]

Hi Takahiro,

2022年2月1日(火) 20:38 AKASHI Takahiro <takahiro.akashi at linaro.org>:

>
> On Tue, Feb 01, 2022 at 05:33:09PM +0900, Masami Hiramatsu wrote:
> > Add a config option to reset system soon after processing capsule update
> > on disk. This is required in UEFI specification 2.9 Section 8.5.5
> >  "Delivery of Capsules via file on Mass Storage device" as;
> >
> >     In all cases that a capsule is identified for processing the system is
> >     restarted after capsule processing is completed.
> >
> > This also reports the result of each capsule update so that the user can
> > notice that the capsule update has been succeeded or not from console log.
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> > ---
> >  Changes in v2:
> >   - Remove kconfig option to disable this feature.
> >   - Use panic() instead of do_reset() so that if the reset fails,
> >     the machine halt.
> >   - Log the result of each capsule update always.
> > ---
> >  lib/efi_loader/efi_capsule.c |   12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > index 1ec7ea29ff..39bce714f7 100644
> > --- a/lib/efi_loader/efi_capsule.c
> > +++ b/lib/efi_loader/efi_capsule.c
> > @@ -1119,9 +1119,9 @@ efi_status_t efi_launch_capsules(void)
> >               ret = efi_capsule_read_file(files[i], &capsule);
> >               if (ret == EFI_SUCCESS) {
> >                       ret = efi_capsule_update_firmware(capsule);
> > -                     if (ret != EFI_SUCCESS)
> > -                             log_err("Applying capsule %ls failed\n",
> > -                                     files[i]);
> > +                     log_err("Applying capsule %ls %s\n",
> > +                             files[i],
> > +                             ret == EFI_SUCCESS ? "succeeded" : "failed");
>
> log_err()? log_info() is better, I think.

Hmm, would you think to use log_info() even if it is failed? Or should
we have log_err(failure) and log_info(success)?

>
> >
> >                       /* create CapsuleXXXX */
> >                       set_capsule_result(index, capsule, ret);
> > @@ -1142,6 +1142,12 @@ efi_status_t efi_launch_capsules(void)
> >               free(files[i]);
> >       free(files);
> >
> > +     /*
> > +      * UEFI spec requires to reset system after complete processing capsule
> > +      * update on the storage.
> > +      */
> > +     panic("Reboot after firmware update");
>
> If CONFIG_PANIC_HANG is enabled, the system won't restart.
> It's not what we want here.

Indeed.
Heinrich, what would you think if do_reset() doesn't work?
(I think it is OK to get it back here, but needs a warning)

Thank you,

>
> -Takahiro Akashi
>
> > +
> >       return ret;
> >  }
> >  #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
> >



--
Masami Hiramatsu