Re: [PATCH 1/2] EFI: Support CAPSULE_FLAGS_INITIATE_RESET for capsule update

[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.

-Takahiro Akashi

> +	}
> +
>  	return ret;
>  }
>  #else
>