On Wed, Jan 26, 2022 at 11:29:10AM +0900, Masami Hiramatsu wrote:
BTW, the original code comes from EDK2 implementation.
What do you mean by "original code"?
It seems that this INITIATE_RESET flag meaning in EDK2 is a bit different from what we thought here.
Yeah,
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.
EDK2's UpdateCapsule() seems to - immediately process *all* the capsules with !PERSIST_ACROSS_RESET - Set "CapsuleUpdateData" variable with a physical address of capsule data - If some of capsules have INITIATE_RESET, kick in warm restart After restart, - process the rest of capsules with PERSIST_ACROSS_RESET using "CapsuleUpdateData" (?)
(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)
Obviously, we are trying to use the flag in a different way; initiating a reset *after* processing a capsule.
While I think that the text in the specification is still ambiguous, we should not give the flag a different meaning.
In 8.5.5 "Delivery of Capsules via file on Mass Storage device", In all cases that a capsule is identified for processing the system is restarted after capsule processing is completed.
So a reset in case of capsule-on-disk seems mandatory. (Personally, I don't like the system to enforce a reset.)
The discussion above also indicates that the current efi_launch_capsules() must be reworked so not to use efi_update_capsule() which is to honor the flags in a capsule header.
-Takahiro Akashi
Thank you,
2022年1月26日(水) 7:31 Masami Hiramatsu masami.hiramatsu@linaro.org:
Hi Takahiro,
2022年1月25日(火) 21:49 AKASHI Takahiro takahiro.akashi@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@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