Hi Heinrich,
2022年1月31日(月) 21:17 Heinrich Schuchardt <xypron.g...@gmx.de>:
>
> On 1/31/22 12:19, Grant Likely wrote:
> > On 31/01/2022 09:19, 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.
> >>
> >> Signed-off-by: Masami Hiramatsu <masami.hirama...@linaro.org>
> >
> > Is there known use cases for making this an option? Feels a bit like
> > option creep that is too easy to choose the wrong setting.
> >
> > Otherwise, this looks good to me.
> >
> > g.
> >
> >> ---
> >> lib/efi_loader/Kconfig | 10 ++++++++++
> >> lib/efi_loader/efi_capsule.c | 9 +++++++++
> >> 2 files changed, 19 insertions(+)
> >>
> >> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> >> index 24f9a2bb75..db05c3ad90 100644
> >> --- a/lib/efi_loader/Kconfig
> >> +++ b/lib/efi_loader/Kconfig
> >> @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS
> >> without setting the
> >> EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED
> >> flag in variable OsIndications.
> >>
> >> +config EFI_RESET_AFTER_CAPSULE_ON_DISK
> >> + bool "Reset right after CapsuleUpdate on-disk"
> >> + depends on EFI_CAPSULE_ON_DISK
> >> + default y
> >> + help
> >> + UEFI specification requests the system to be restarted after
> >> capsule
> >> + processing is complete. This implements that, but for some
> >> reason,
> >> + if you want to keep the (old) system running after the capsule
> >> update
> >> + on-disk, you can say 'n' here.
> >> +
> >> config EFI_CAPSULE_ON_DISK_EARLY
> >> bool "Initiate capsule-on-disk at U-Boot boottime"
> >> depends on EFI_CAPSULE_ON_DISK
> >> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> >> index 98dab1c6f5..44d4fa2f82 100644
> >> --- a/lib/efi_loader/efi_capsule.c
> >> +++ b/lib/efi_loader/efi_capsule.c
> >> @@ -1142,6 +1142,15 @@ 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.
> >> + */
> >> + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) {
>
> We should not allow configuring a non-compliant behavior.
OK, let me remove this option then.
>
> >> + log_info("Restarting the system to boot the updated
> >> firmware.\n");
>
> I am missing a success message for each installed capsule.
Indeed, but this reboot will be done unconditionally.
let me add a message when the capsule update is successfully completed.
Thank you,
>
> >> + do_reset(NULL, 0, 0, NULL);
>
> do_reset() may return. How about:
>
> panic("Reboot after firmware update");
>
> This will hang if do_reset() returns.
>
> >> + }
> >> +
> >> if (IS_ENABLED(CONFIG_EFI_ESRT)) {
>
> We don't need this code anymore if we call panic() above.
>
> Best regards
>
> Heinrich
>
> >> /* Rebuild the ESRT to reflect any updated FW images. */
> >> ret = efi_esrt_populate();
> >>
> >
--
Masami Hiramatsu
