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

[Simon Glass]

Hi Masami,

On Mon, 28 Feb 2022 at 00:53, Masami Hiramatsu
<masami.hiramatsu at linaro.org> wrote:
>
> Hi Simon,
>
> BTW, I saw the below code in the sysreset-uclass.c. It seems if I pass
> 0 to argc, it seems to do SYSRESET_COLD, isn't it?

Yes, but we should use the driver interface to do things, not the
command-line interface :-)


- Simon

>
> int do_reset(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> {
>         enum sysreset_t reset_type = SYSRESET_COLD;
>
>         if (argc > 2)
>                 return CMD_RET_USAGE;
>
>         if (argc == 2 && argv[1][0] == '-' && argv[1][1] == 'w') {
>                 reset_type = SYSRESET_WARM;
>         }
>
>         printf("resetting ...\n");
>         mdelay(100);
>
>         sysreset_walk_halt(reset_type);
>
>         return 0;
> }
>
> 2022年2月28日(月) 13:19 Masami Hiramatsu <masami.hiramatsu at linaro.org>:
> >
> > Hi Simon,
> >
> > 2022年2月27日(日) 3:37 Simon Glass <sjg at chromium.org>:
> > >
> > > Hi Masami,
> > >
> > > On Thu, 3 Feb 2022 at 02:23, Masami Hiramatsu
> > > <masami.hiramatsu at linaro.org> wrote:
> > > >
> > > > Add a cold reset 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 v4:
> > > >   - Do not use sysreset because that is a warm reset.
> > >
> > > I don't get that.
> > >
> > > You should use sysreset and the SYSRESET_COLD type.
> >
> > Thanks, yeah, I found that parameter.
> > Let's fix that.
> >
> > Thank you,
> >
> > >
> > > >   - Fix patch description.
> > > > ---
> > > >  lib/efi_loader/efi_capsule.c |   18 ++++++++++++++++--
> > > >  1 file changed, 16 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > > index 1ec7ea29ff..20d9490dbd 100644
> > > > --- a/lib/efi_loader/efi_capsule.c
> > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > @@ -14,6 +14,7 @@
> > > >  #include <env.h>
> > > >  #include <fdtdec.h>
> > > >  #include <fs.h>
> > > > +#include <hang.h>
> > > >  #include <malloc.h>
> > > >  #include <mapmem.h>
> > > >  #include <sort.h>
> > > > @@ -1120,8 +1121,11 @@ efi_status_t efi_launch_capsules(void)
> > > >                 if (ret == EFI_SUCCESS) {
> > > >                         ret = efi_capsule_update_firmware(capsule);
> > > >                         if (ret != EFI_SUCCESS)
> > > > -                               log_err("Applying capsule %ls failed\n",
> > > > +                               log_err("Applying capsule %ls failed.\n",
> > > >                                         files[i]);
> > > > +                       else
> > > > +                               log_info("Applying capsule %ls succeeded.\n",
> > > > +                                        files[i]);
> > > >
> > > >                         /* create CapsuleXXXX */
> > > >                         set_capsule_result(index, capsule, ret);
> > > > @@ -1142,6 +1146,16 @@ efi_status_t efi_launch_capsules(void)
> > > >                 free(files[i]);
> > > >         free(files);
> > > >
> > > > -       return ret;
> > > > +       /*
> > > > +        * UEFI spec requires to reset system after complete processing capsule
> > > > +        * update on the storage.
> > > > +        */
> > > > +       log_info("Reboot after firmware update");
> > > > +       /* Cold reset is required for loading the new firmware. */
> > > > +       do_reset(NULL, 0, 0, NULL);
> > >
> > > No! This should use sysreset.
> > >
> > > > +       hang();
> > > > +       /* not reach here */
> > > > +
> > > > +       return 0;
> > > >  }
> > > >  #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
> > > >
> > >
> > > Regards,
> > > Simon
> >
> >
> >
> > --
> > Masami Hiramatsu
>
>
>
> --
> Masami Hiramatsu