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

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.hiramatsu@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 */