Hi Takahiro, We forgot a simple fact. The warm reset doesn't load the new firmware from media.
If this reset is for reloading the new firmware, we anyway need the cold reset :-) (and I can't think of any reason other than this) Thank you, 2022年2月3日(木) 13:34 Masami Hiramatsu <masami.hirama...@linaro.org>: > > Hi Takahiro, > > 2022年2月3日(木) 10:24 AKASHI Takahiro <takahiro.aka...@linaro.org>: > > > > On Wed, Feb 02, 2022 at 10:54:43PM +0900, Masami Hiramatsu wrote: > > > Add a config option to reset system soon after processing capsule update > > > on disk. > > > > We no longer have a new config option :) > > Oops, that's my fault. > > > > > > 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.hirama...@linaro.org> > > > --- > > > Changes in v3: > > > - Log succeeded capsule update in info level. > > > - Use sysreset if possible. > > > - Use do_reset() and hang() instead of panic(). > > > 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 | 22 ++++++++++++++++++++-- > > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > > index 1ec7ea29ff..ade9155042 100644 > > > --- a/lib/efi_loader/efi_capsule.c > > > +++ b/lib/efi_loader/efi_capsule.c > > > @@ -14,9 +14,11 @@ > > > #include <env.h> > > > #include <fdtdec.h> > > > #include <fs.h> > > > +#include <hang.h> > > > #include <malloc.h> > > > #include <mapmem.h> > > > #include <sort.h> > > > +#include <sysreset.h> > > > #include <asm/global_data.h> > > > > > > #include <crypto/pkcs7.h> > > > @@ -1120,8 +1122,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 +1147,19 @@ 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. > > > + */ > > > + puts("Reboot after firmware update"); > > > + if (CONFIG_IS_ENABLED(SYSRESET)) { > > > + reset_cpu(); > > > + } else { > > > + do_reset(NULL, 0, 0, NULL); > > > + hang(); > > > + } > > > + /* not reach here */ > > > > Despite the code that I proposed, I have a few concerns: > > 1) warm or cold reset > > Now that we are updating firmware, we may have to initiate > > a cold reset in some cases. > > (That's why I used 'sysreset(WARM)' to raise a question.) > > Indeed. Hm, as far as I can see the EDK2, it also uses cold reset. > (HandleCapsules@ArmPkg/Library/PlatformBootManagerLib/PlatformBM.c) > Since do_reset() calls sysreset_walk_halt(), I think do_reset() is enough. > > > > > From the viewpoint of UEFI specification, > > * A type of reset can be determined per capsule by calling > > QueryCapsuleCapabilities API. > > (The spec said, "Returns if the capsule can be supported via > > UpdateCapsule()" and Capsule-on-disk might be out of scope?) > > I think that is only for UpdateCapsule(), as far as I can read the EDK2 code. > > > * There exists ResetSystem API and it takes a *reset type* > > as a parameter. > > This API is independent from UpdateCapsule(). But while executing > the UpdateCapsule() this API is prohibited. (See Table 8-1) > > > > > 2) ResetSystem at boot time > > So we may want to internally make use of efi_reset_system() following > > capsule-on-disk processing. > > The current implementation, however, does not utilize SYSRESET drivers, > > but call do_reset(). This should be changed (as I suggested above?). > > As I said above, I think it should always be a cold reset and not need to use > efi_reset_system(). For the UpdateCapsule(), there is a reason to use > warm reset, because the capsule images which will be applied after reset, > will be on the memory. In this case the system must be reboot without > resetting the memory. > But after capsule-on-disk process, all capsule images are applied and the > firmware image on the storage is updated. So it is better to reset the > system with cold reset so that the new firmware image can start with > cleaned memory and devices. > > Thank you, > > > > > -Takahiro Akashi > > > > > > > + > > > + return 0; > > > } > > > #endif /* CONFIG_EFI_CAPSULE_ON_DISK */ > > > > > > > -- > Masami Hiramatsu -- Masami Hiramatsu