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 :)
> 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.)
>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?)
* There exists ResetSystem API and it takes a *reset type*
as a parameter.
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?).
-Takahiro Akashi
> +
> + return 0;
> }
> #endif /* CONFIG_EFI_CAPSULE_ON_DISK */
>
