[PATCH 1/2] EFI: Support CAPSULE_FLAGS_INITIATE_RESET for capsule update

[Masami Hiramatsu]

BTW, the original code comes from EDK2 implementation.
It seems that this INITIATE_RESET flag meaning in EDK2 is a bit different
from what we thought here. The flag is only used for resetting system via
UpdateCapsule() EFI call. The capsule update process itself will be done
at the boot time and warm reset soon after that.
(See HandleCapsules()@ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c)

Is it OK to change the INITIATE_RESET flag means? or should we forcibly
reset the system if we succeeded to update capsule on boot time?
(note that capsule on disk must be done at boot time in U-Boot)

Thank you,

2022年1月26日(水) 7:31 Masami Hiramatsu <masami.hiramatsu at linaro.org>:
>
> Hi Takahiro,
>
> 2022年1月25日(火) 21:49 AKASHI Takahiro <takahiro.akashi at linaro.org>:
> >
> > Hi Masami,
> >
> > Thank you for this enhancement.
> >
> > On Tue, Jan 25, 2022 at 08:31:29PM +0900, Masami Hiramatsu wrote:
> > > Support CAPSULE_FLAGS_INITIATE_RESET for rebooting uboot soon after
> > > updating firmware. Note that the machine will reboot soon after
> > > applying the capsule file which has CAPSULE_FLAGS_INITIATE_RESET
> > > flag. If there are multiple capsules and one has this flag, the
> > > machine may reboot while scanning the capsule files.
> > > You can control when the machine reboot by renaming the capsule
> > > file because the capsule files will be applied alphabetically.
> > >
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> > > ---
> > >  lib/efi_loader/efi_capsule.c |   13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > index 4463ae00fd..24a2a026a9 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -407,12 +407,20 @@ static efi_status_t efi_capsule_update_firmware(
> > >       struct efi_firmware_management_protocol *fmp;
> > >       u16 *abort_reason;
> > >       efi_status_t ret = EFI_SUCCESS;
> > > +     bool reset = false;
> > >
> > >       /* sanity check */
> > >       if (capsule_data->header_size < sizeof(*capsule) ||
> > >           capsule_data->header_size >= capsule_data->capsule_image_size)
> > >               return EFI_INVALID_PARAMETER;
> > >
> > > +     if (capsule_data->flags & CAPSULE_FLAGS_INITIATE_RESET) {
> > > +             /* INITIATE_RESET flag requires PERSIST_ACROSS_RESET flag */
> > > +             if (!(capsule_data->flags & CAPSULE_FLAGS_PERSIST_ACROSS_RESET))
> > > +                     return EFI_INVALID_PARAMETER;
> > > +             reset = true;
> > > +     }
> > > +
> > >       capsule = (void *)capsule_data + capsule_data->header_size;
> > >       capsule_size = capsule_data->capsule_image_size
> > >                       - capsule_data->header_size;
> > > @@ -498,6 +506,11 @@ static efi_status_t efi_capsule_update_firmware(
> > >  out:
> > >       efi_free_pool(handles);
> > >
> > > +     if (ret == EFI_SUCCESS && reset) {
> > > +             log_debug("This capsule has CAPSULE_FLAGS_INITIATE_RESET. Reboot machine.\n");
> > > +             do_reset(NULL, 0, 0, NULL);
> >
> > I don't think this is the right place of calling do_reset().
> > Whenever you have processed any capsule file, you have to
> >  - delete the capsule file,
> >  - create/update "CapsuleXXXX" and "CapsuleLast" variables, and
> >  - modify ESRT table
> > before initiating a reset.
>
> Oops, indeed. Let me update the patch.
> Thank you!
>
> >
> > -Takahiro Akashi
> >
> > > +     }
> > > +
> > >       return ret;
> > >  }
> > >  #else
> > >
>
>
>
> --
> Masami Hiramatsu



-- 
Masami Hiramatsu