* [PATCH 0/2] EFI: Reset system after capsule-on-disk
@ 2022-01-31 9:18 Masami Hiramatsu
2022-01-31 9:18 ` [PATCH 1/2] efi_loader: Avoid using efi_update_capsule() from update capsule on disk Masami Hiramatsu
2022-01-31 9:19 ` [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate " Masami Hiramatsu
0 siblings, 2 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2022-01-31 9:18 UTC (permalink / raw)
To: u-boot
Cc: Masami Hiramatsu, Patrick Delaunay, Patrice Chotard,
Heinrich Schuchardt, Alexander Graf, AKASHI Takahiro, Simon Glass,
Bin Meng, Ilias Apalodimas, Jose Marinho, Grant Likely, Tom Rini,
Etienne Carriere, Sughosh Ganu, Paul Liu
Hi,
According to Takahiro's suggestion and discussion, I made a patchset to
update the EFI capsule-on-disk, so that it does not use UpdateCapsule()
EFI API and reset after completing the capsule-on-disk.
The reset after completing the capsule-on-disk is stated in the UEFI
specification 2.9, section 8.5.5 "Delivery of Capsules via file on Mass
Storage device" as below,
In all cases that a capsule is identified for processing the system is
restarted after capsule processing is completed.
Note that this feature is enabled by CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK.
If you want to keep the current firmware runs after capsule-on-disk (e.g.
for debugging), you can configure it disabled.
Thank you,
---
Masami Hiramatsu (2):
efi_loader: Avoid using efi_update_capsule() from update capsule on disk
efi_loader: Reset system after CapsuleUpdate on disk
lib/efi_loader/Kconfig | 10 ++++++++++
lib/efi_loader/efi_capsule.c | 18 +++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletion(-)
--
Masami Hiramatsu <masami.hiramatsu@linaro.org>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] efi_loader: Avoid using efi_update_capsule() from update capsule on disk 2022-01-31 9:18 [PATCH 0/2] EFI: Reset system after capsule-on-disk Masami Hiramatsu @ 2022-01-31 9:18 ` Masami Hiramatsu 2022-01-31 19:26 ` Heinrich Schuchardt 2022-01-31 9:19 ` [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate " Masami Hiramatsu 1 sibling, 1 reply; 10+ messages in thread From: Masami Hiramatsu @ 2022-01-31 9:18 UTC (permalink / raw) To: u-boot Cc: Masami Hiramatsu, Patrick Delaunay, Patrice Chotard, Heinrich Schuchardt, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Grant Likely, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu The efi_update_capsule() may have to handle the capsule flags as an UEFI runtime and boottime service, but the capsule-on-disk process doesn't. Thus, the capsule-on-disk should use the efi_capsule_update_firmware() directly instead of efi_update_capsule(). To keep the consistency ESRT also will be updated after all capsule updates are completed. Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> --- lib/efi_loader/efi_capsule.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 4463ae00fd..98dab1c6f5 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) index = 0; ret = efi_capsule_read_file(files[i], &capsule); if (ret == EFI_SUCCESS) { - ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0)); + ret = efi_capsule_update_firmware(&capsule); if (ret != EFI_SUCCESS) log_err("Applying capsule %ls failed\n", files[i]); @@ -1142,6 +1142,13 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files); + if (IS_ENABLED(CONFIG_EFI_ESRT)) { + /* Rebuild the ESRT to reflect any updated FW images. */ + ret = efi_esrt_populate(); + if (ret != EFI_SUCCESS) + log_warning("ESRT update failed\n"); + } + return ret; } #endif /* CONFIG_EFI_CAPSULE_ON_DISK */ ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] efi_loader: Avoid using efi_update_capsule() from update capsule on disk 2022-01-31 9:18 ` [PATCH 1/2] efi_loader: Avoid using efi_update_capsule() from update capsule on disk Masami Hiramatsu @ 2022-01-31 19:26 ` Heinrich Schuchardt 2022-02-01 2:21 ` Masami Hiramatsu 0 siblings, 1 reply; 10+ messages in thread From: Heinrich Schuchardt @ 2022-01-31 19:26 UTC (permalink / raw) To: Masami Hiramatsu Cc: Patrick Delaunay, Patrice Chotard, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Grant Likely, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu, u-boot On 1/31/22 10:18, Masami Hiramatsu wrote: > The efi_update_capsule() may have to handle the capsule flags as an UEFI > runtime and boottime service, but the capsule-on-disk process doesn't. > Thus, the capsule-on-disk should use the efi_capsule_update_firmware() > directly instead of efi_update_capsule(). To keep the consistency > ESRT also will be updated after all capsule updates are completed. > > Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > --- > lib/efi_loader/efi_capsule.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 4463ae00fd..98dab1c6f5 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) > index = 0; > ret = efi_capsule_read_file(files[i], &capsule); > if (ret == EFI_SUCCESS) { > - ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0)); > + ret = efi_capsule_update_firmware(&capsule); Thanks for the patch. This look better. > if (ret != EFI_SUCCESS) > log_err("Applying capsule %ls failed\n", > files[i]); > @@ -1142,6 +1142,13 @@ efi_status_t efi_launch_capsules(void) > free(files[i]); > free(files); > > + if (IS_ENABLED(CONFIG_EFI_ESRT)) { > + /* Rebuild the ESRT to reflect any updated FW images. */ > + ret = efi_esrt_populate(); > + if (ret != EFI_SUCCESS) > + log_warning("ESRT update failed\n"); > + } > + This is not needed as you want to reboot. Best regards Heinrich > return ret; > } > #endif /* CONFIG_EFI_CAPSULE_ON_DISK */ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] efi_loader: Avoid using efi_update_capsule() from update capsule on disk 2022-01-31 19:26 ` Heinrich Schuchardt @ 2022-02-01 2:21 ` Masami Hiramatsu 0 siblings, 0 replies; 10+ messages in thread From: Masami Hiramatsu @ 2022-02-01 2:21 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Patrick Delaunay, Patrice Chotard, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Grant Likely, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu, u-boot Hi Heinrich, Thanks for review. 2022年2月1日(火) 4:26 Heinrich Schuchardt <xypron.glpk@gmx.de>: > > On 1/31/22 10:18, Masami Hiramatsu wrote: > > The efi_update_capsule() may have to handle the capsule flags as an UEFI > > runtime and boottime service, but the capsule-on-disk process doesn't. > > Thus, the capsule-on-disk should use the efi_capsule_update_firmware() > > directly instead of efi_update_capsule(). To keep the consistency > > ESRT also will be updated after all capsule updates are completed. > > > > Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > --- > > lib/efi_loader/efi_capsule.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index 4463ae00fd..98dab1c6f5 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -1118,7 +1118,7 @@ efi_status_t efi_launch_capsules(void) > > index = 0; > > ret = efi_capsule_read_file(files[i], &capsule); > > if (ret == EFI_SUCCESS) { > > - ret = EFI_CALL(efi_update_capsule(&capsule, 1, 0)); > > + ret = efi_capsule_update_firmware(&capsule); > > Thanks for the patch. This look better. > > > if (ret != EFI_SUCCESS) > > log_err("Applying capsule %ls failed\n", > > files[i]); > > @@ -1142,6 +1142,13 @@ efi_status_t efi_launch_capsules(void) > > free(files[i]); > > free(files); > > > > + if (IS_ENABLED(CONFIG_EFI_ESRT)) { > > + /* Rebuild the ESRT to reflect any updated FW images. */ > > + ret = efi_esrt_populate(); > > + if (ret != EFI_SUCCESS) > > + log_warning("ESRT update failed\n"); > > + } > > + > > This is not needed as you want to reboot. Yes, OK. I'll drop this part. Thank you, > > > Best regards > > Heinrich > > > return ret; > > } > > #endif /* CONFIG_EFI_CAPSULE_ON_DISK */ > > > -- Masami Hiramatsu ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate on disk 2022-01-31 9:18 [PATCH 0/2] EFI: Reset system after capsule-on-disk Masami Hiramatsu 2022-01-31 9:18 ` [PATCH 1/2] efi_loader: Avoid using efi_update_capsule() from update capsule on disk Masami Hiramatsu @ 2022-01-31 9:19 ` Masami Hiramatsu 2022-01-31 11:19 ` Grant Likely 1 sibling, 1 reply; 10+ messages in thread From: Masami Hiramatsu @ 2022-01-31 9:19 UTC (permalink / raw) To: u-boot Cc: Masami Hiramatsu, Patrick Delaunay, Patrice Chotard, Heinrich Schuchardt, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Grant Likely, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu Add a config option to reset system soon after processing capsule update on disk. 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. Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> --- lib/efi_loader/Kconfig | 10 ++++++++++ lib/efi_loader/efi_capsule.c | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 24f9a2bb75..db05c3ad90 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS without setting the EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED flag in variable OsIndications. +config EFI_RESET_AFTER_CAPSULE_ON_DISK + bool "Reset right after CapsuleUpdate on-disk" + depends on EFI_CAPSULE_ON_DISK + default y + help + UEFI specification requests the system to be restarted after capsule + processing is complete. This implements that, but for some reason, + if you want to keep the (old) system running after the capsule update + on-disk, you can say 'n' here. + config EFI_CAPSULE_ON_DISK_EARLY bool "Initiate capsule-on-disk at U-Boot boottime" depends on EFI_CAPSULE_ON_DISK diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 98dab1c6f5..44d4fa2f82 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1142,6 +1142,15 @@ efi_status_t efi_launch_capsules(void) free(files[i]); free(files); + /* + * UEFI spec requires to reset system after complete processing capsule + * update on the storage. + */ + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) { + log_info("Restarting the system to boot the updated firmware.\n"); + do_reset(NULL, 0, 0, NULL); + } + if (IS_ENABLED(CONFIG_EFI_ESRT)) { /* Rebuild the ESRT to reflect any updated FW images. */ ret = efi_esrt_populate(); ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate on disk 2022-01-31 9:19 ` [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate " Masami Hiramatsu @ 2022-01-31 11:19 ` Grant Likely 2022-01-31 12:17 ` Heinrich Schuchardt 2022-01-31 12:38 ` Masami Hiramatsu 0 siblings, 2 replies; 10+ messages in thread From: Grant Likely @ 2022-01-31 11:19 UTC (permalink / raw) To: Masami Hiramatsu, u-boot Cc: Patrick Delaunay, Patrice Chotard, Heinrich Schuchardt, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu On 31/01/2022 09:19, Masami Hiramatsu wrote: > Add a config option to reset system soon after processing capsule update > on disk. 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. > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> Is there known use cases for making this an option? Feels a bit like option creep that is too easy to choose the wrong setting. Otherwise, this looks good to me. g. > --- > lib/efi_loader/Kconfig | 10 ++++++++++ > lib/efi_loader/efi_capsule.c | 9 +++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index 24f9a2bb75..db05c3ad90 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS > without setting the EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED > flag in variable OsIndications. > > +config EFI_RESET_AFTER_CAPSULE_ON_DISK > + bool "Reset right after CapsuleUpdate on-disk" > + depends on EFI_CAPSULE_ON_DISK > + default y > + help > + UEFI specification requests the system to be restarted after capsule > + processing is complete. This implements that, but for some reason, > + if you want to keep the (old) system running after the capsule update > + on-disk, you can say 'n' here. > + > config EFI_CAPSULE_ON_DISK_EARLY > bool "Initiate capsule-on-disk at U-Boot boottime" > depends on EFI_CAPSULE_ON_DISK > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 98dab1c6f5..44d4fa2f82 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -1142,6 +1142,15 @@ efi_status_t efi_launch_capsules(void) > free(files[i]); > free(files); > > + /* > + * UEFI spec requires to reset system after complete processing capsule > + * update on the storage. > + */ > + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) { > + log_info("Restarting the system to boot the updated firmware.\n"); > + do_reset(NULL, 0, 0, NULL); > + } > + > if (IS_ENABLED(CONFIG_EFI_ESRT)) { > /* Rebuild the ESRT to reflect any updated FW images. */ > ret = efi_esrt_populate(); > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate on disk 2022-01-31 11:19 ` Grant Likely @ 2022-01-31 12:17 ` Heinrich Schuchardt 2022-02-01 2:26 ` Masami Hiramatsu 2022-01-31 12:38 ` Masami Hiramatsu 1 sibling, 1 reply; 10+ messages in thread From: Heinrich Schuchardt @ 2022-01-31 12:17 UTC (permalink / raw) To: Masami Hiramatsu Cc: Patrick Delaunay, Patrice Chotard, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu, Grant Likely, u-boot On 1/31/22 12:19, Grant Likely wrote: > On 31/01/2022 09:19, Masami Hiramatsu wrote: >> Add a config option to reset system soon after processing capsule update >> on disk. 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. >> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > Is there known use cases for making this an option? Feels a bit like > option creep that is too easy to choose the wrong setting. > > Otherwise, this looks good to me. > > g. > >> --- >> lib/efi_loader/Kconfig | 10 ++++++++++ >> lib/efi_loader/efi_capsule.c | 9 +++++++++ >> 2 files changed, 19 insertions(+) >> >> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >> index 24f9a2bb75..db05c3ad90 100644 >> --- a/lib/efi_loader/Kconfig >> +++ b/lib/efi_loader/Kconfig >> @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS >> without setting the >> EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED >> flag in variable OsIndications. >> >> +config EFI_RESET_AFTER_CAPSULE_ON_DISK >> + bool "Reset right after CapsuleUpdate on-disk" >> + depends on EFI_CAPSULE_ON_DISK >> + default y >> + help >> + UEFI specification requests the system to be restarted after >> capsule >> + processing is complete. This implements that, but for some >> reason, >> + if you want to keep the (old) system running after the capsule >> update >> + on-disk, you can say 'n' here. >> + >> config EFI_CAPSULE_ON_DISK_EARLY >> bool "Initiate capsule-on-disk at U-Boot boottime" >> depends on EFI_CAPSULE_ON_DISK >> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c >> index 98dab1c6f5..44d4fa2f82 100644 >> --- a/lib/efi_loader/efi_capsule.c >> +++ b/lib/efi_loader/efi_capsule.c >> @@ -1142,6 +1142,15 @@ efi_status_t efi_launch_capsules(void) >> free(files[i]); >> free(files); >> >> + /* >> + * UEFI spec requires to reset system after complete processing >> capsule >> + * update on the storage. >> + */ >> + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) { We should not allow configuring a non-compliant behavior. >> + log_info("Restarting the system to boot the updated >> firmware.\n"); I am missing a success message for each installed capsule. >> + do_reset(NULL, 0, 0, NULL); do_reset() may return. How about: panic("Reboot after firmware update"); This will hang if do_reset() returns. >> + } >> + >> if (IS_ENABLED(CONFIG_EFI_ESRT)) { We don't need this code anymore if we call panic() above. Best regards Heinrich >> /* Rebuild the ESRT to reflect any updated FW images. */ >> ret = efi_esrt_populate(); >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate on disk 2022-01-31 12:17 ` Heinrich Schuchardt @ 2022-02-01 2:26 ` Masami Hiramatsu 2022-02-01 3:16 ` AKASHI Takahiro 0 siblings, 1 reply; 10+ messages in thread From: Masami Hiramatsu @ 2022-02-01 2:26 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Patrick Delaunay, Patrice Chotard, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu, Grant Likely, u-boot Hi Heinrich, 2022年1月31日(月) 21:17 Heinrich Schuchardt <xypron.glpk@gmx.de>: > > On 1/31/22 12:19, Grant Likely wrote: > > On 31/01/2022 09:19, Masami Hiramatsu wrote: > >> Add a config option to reset system soon after processing capsule update > >> on disk. 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. > >> > >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > > > Is there known use cases for making this an option? Feels a bit like > > option creep that is too easy to choose the wrong setting. > > > > Otherwise, this looks good to me. > > > > g. > > > >> --- > >> lib/efi_loader/Kconfig | 10 ++++++++++ > >> lib/efi_loader/efi_capsule.c | 9 +++++++++ > >> 2 files changed, 19 insertions(+) > >> > >> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > >> index 24f9a2bb75..db05c3ad90 100644 > >> --- a/lib/efi_loader/Kconfig > >> +++ b/lib/efi_loader/Kconfig > >> @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS > >> without setting the > >> EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED > >> flag in variable OsIndications. > >> > >> +config EFI_RESET_AFTER_CAPSULE_ON_DISK > >> + bool "Reset right after CapsuleUpdate on-disk" > >> + depends on EFI_CAPSULE_ON_DISK > >> + default y > >> + help > >> + UEFI specification requests the system to be restarted after > >> capsule > >> + processing is complete. This implements that, but for some > >> reason, > >> + if you want to keep the (old) system running after the capsule > >> update > >> + on-disk, you can say 'n' here. > >> + > >> config EFI_CAPSULE_ON_DISK_EARLY > >> bool "Initiate capsule-on-disk at U-Boot boottime" > >> depends on EFI_CAPSULE_ON_DISK > >> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > >> index 98dab1c6f5..44d4fa2f82 100644 > >> --- a/lib/efi_loader/efi_capsule.c > >> +++ b/lib/efi_loader/efi_capsule.c > >> @@ -1142,6 +1142,15 @@ efi_status_t efi_launch_capsules(void) > >> free(files[i]); > >> free(files); > >> > >> + /* > >> + * UEFI spec requires to reset system after complete processing > >> capsule > >> + * update on the storage. > >> + */ > >> + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) { > > We should not allow configuring a non-compliant behavior. OK, let me remove this option then. > > >> + log_info("Restarting the system to boot the updated > >> firmware.\n"); > > I am missing a success message for each installed capsule. Indeed, but this reboot will be done unconditionally. let me add a message when the capsule update is successfully completed. Thank you, > > >> + do_reset(NULL, 0, 0, NULL); > > do_reset() may return. How about: > > panic("Reboot after firmware update"); > > This will hang if do_reset() returns. > > >> + } > >> + > >> if (IS_ENABLED(CONFIG_EFI_ESRT)) { > > We don't need this code anymore if we call panic() above. > > Best regards > > Heinrich > > >> /* Rebuild the ESRT to reflect any updated FW images. */ > >> ret = efi_esrt_populate(); > >> > > -- Masami Hiramatsu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate on disk 2022-02-01 2:26 ` Masami Hiramatsu @ 2022-02-01 3:16 ` AKASHI Takahiro 0 siblings, 0 replies; 10+ messages in thread From: AKASHI Takahiro @ 2022-02-01 3:16 UTC (permalink / raw) To: Masami Hiramatsu Cc: Heinrich Schuchardt, Patrick Delaunay, Patrice Chotard, Alexander Graf, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu, Grant Likely, u-boot On Tue, Feb 01, 2022 at 11:26:38AM +0900, Masami Hiramatsu wrote: > Hi Heinrich, > > 2022年1月31日(月) 21:17 Heinrich Schuchardt <xypron.glpk@gmx.de>: > > > > On 1/31/22 12:19, Grant Likely wrote: > > > On 31/01/2022 09:19, Masami Hiramatsu wrote: > > >> Add a config option to reset system soon after processing capsule update > > >> on disk. 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. > > >> > > >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > > > > > Is there known use cases for making this an option? Feels a bit like > > > option creep that is too easy to choose the wrong setting. > > > > > > Otherwise, this looks good to me. > > > > > > g. > > > > > >> --- > > >> lib/efi_loader/Kconfig | 10 ++++++++++ > > >> lib/efi_loader/efi_capsule.c | 9 +++++++++ > > >> 2 files changed, 19 insertions(+) > > >> > > >> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > >> index 24f9a2bb75..db05c3ad90 100644 > > >> --- a/lib/efi_loader/Kconfig > > >> +++ b/lib/efi_loader/Kconfig > > >> @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS > > >> without setting the > > >> EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED > > >> flag in variable OsIndications. > > >> > > >> +config EFI_RESET_AFTER_CAPSULE_ON_DISK > > >> + bool "Reset right after CapsuleUpdate on-disk" > > >> + depends on EFI_CAPSULE_ON_DISK > > >> + default y > > >> + help > > >> + UEFI specification requests the system to be restarted after > > >> capsule > > >> + processing is complete. This implements that, but for some > > >> reason, > > >> + if you want to keep the (old) system running after the capsule > > >> update > > >> + on-disk, you can say 'n' here. > > >> + > > >> config EFI_CAPSULE_ON_DISK_EARLY > > >> bool "Initiate capsule-on-disk at U-Boot boottime" > > >> depends on EFI_CAPSULE_ON_DISK > > >> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > >> index 98dab1c6f5..44d4fa2f82 100644 > > >> --- a/lib/efi_loader/efi_capsule.c > > >> +++ b/lib/efi_loader/efi_capsule.c > > >> @@ -1142,6 +1142,15 @@ efi_status_t efi_launch_capsules(void) > > >> free(files[i]); > > >> free(files); > > >> > > >> + /* > > >> + * UEFI spec requires to reset system after complete processing > > >> capsule > > >> + * update on the storage. > > >> + */ > > >> + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) { > > > > We should not allow configuring a non-compliant behavior. > > OK, let me remove this option then. > > > > > >> + log_info("Restarting the system to boot the updated > > >> firmware.\n"); > > > > I am missing a success message for each installed capsule. > > Indeed, but this reboot will be done unconditionally. > let me add a message when the capsule update is successfully completed. > Thank you, While I don't object to adding a message, I'd rather see that a sub-command, like "efidebug capsule show-result," be added to efidebug command. Please note the result for each capsule will be recorded in "CapsuleNNNN" variable which may contain additional information. (I haven't implemented efi_variable_result_fmp though.) -Takahiro Akashi > > > > > >> + do_reset(NULL, 0, 0, NULL); > > > > do_reset() may return. How about: > > > > panic("Reboot after firmware update"); > > > > This will hang if do_reset() returns. > > > > >> + } > > >> + > > >> if (IS_ENABLED(CONFIG_EFI_ESRT)) { > > > > We don't need this code anymore if we call panic() above. > > > > Best regards > > > > Heinrich > > > > >> /* Rebuild the ESRT to reflect any updated FW images. */ > > >> ret = efi_esrt_populate(); > > >> > > > > > > > -- > Masami Hiramatsu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate on disk 2022-01-31 11:19 ` Grant Likely 2022-01-31 12:17 ` Heinrich Schuchardt @ 2022-01-31 12:38 ` Masami Hiramatsu 1 sibling, 0 replies; 10+ messages in thread From: Masami Hiramatsu @ 2022-01-31 12:38 UTC (permalink / raw) To: Grant Likely Cc: u-boot, Patrick Delaunay, Patrice Chotard, Heinrich Schuchardt, Alexander Graf, AKASHI Takahiro, Simon Glass, Bin Meng, Ilias Apalodimas, Jose Marinho, Tom Rini, Etienne Carriere, Sughosh Ganu, Paul Liu Hi Grant, 2022年1月31日(月) 20:20 Grant Likely <grant.likely@arm.com>: > > On 31/01/2022 09:19, Masami Hiramatsu wrote: > > Add a config option to reset system soon after processing capsule update > > on disk. 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. > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > Is there known use cases for making this an option? Feels a bit like > option creep that is too easy to choose the wrong setting. No, I just guessed that it may be helpful for porting FWU, because we can check whether the metadata is updated correctly before reboot. (anyway, while developing we can just comment out the code too.) Thank you, > > Otherwise, this looks good to me. > > g. > > > --- > > lib/efi_loader/Kconfig | 10 ++++++++++ > > lib/efi_loader/efi_capsule.c | 9 +++++++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 24f9a2bb75..db05c3ad90 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -146,6 +146,16 @@ config EFI_IGNORE_OSINDICATIONS > > without setting the EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED > > flag in variable OsIndications. > > > > +config EFI_RESET_AFTER_CAPSULE_ON_DISK > > + bool "Reset right after CapsuleUpdate on-disk" > > + depends on EFI_CAPSULE_ON_DISK > > + default y > > + help > > + UEFI specification requests the system to be restarted after capsule > > + processing is complete. This implements that, but for some reason, > > + if you want to keep the (old) system running after the capsule update > > + on-disk, you can say 'n' here. > > + > > config EFI_CAPSULE_ON_DISK_EARLY > > bool "Initiate capsule-on-disk at U-Boot boottime" > > depends on EFI_CAPSULE_ON_DISK > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index 98dab1c6f5..44d4fa2f82 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -1142,6 +1142,15 @@ efi_status_t efi_launch_capsules(void) > > free(files[i]); > > free(files); > > > > + /* > > + * UEFI spec requires to reset system after complete processing capsule > > + * update on the storage. > > + */ > > + if (IS_ENABLED(CONFIG_EFI_RESET_AFTER_CAPSULE_ON_DISK)) { > > + log_info("Restarting the system to boot the updated firmware.\n"); > > + do_reset(NULL, 0, 0, NULL); > > + } > > + > > if (IS_ENABLED(CONFIG_EFI_ESRT)) { > > /* Rebuild the ESRT to reflect any updated FW images. */ > > ret = efi_esrt_populate(); > > > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -- Masami Hiramatsu ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-02-01 3:17 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-31 9:18 [PATCH 0/2] EFI: Reset system after capsule-on-disk Masami Hiramatsu 2022-01-31 9:18 ` [PATCH 1/2] efi_loader: Avoid using efi_update_capsule() from update capsule on disk Masami Hiramatsu 2022-01-31 19:26 ` Heinrich Schuchardt 2022-02-01 2:21 ` Masami Hiramatsu 2022-01-31 9:19 ` [PATCH 2/2] efi_loader: Reset system after CapsuleUpdate " Masami Hiramatsu 2022-01-31 11:19 ` Grant Likely 2022-01-31 12:17 ` Heinrich Schuchardt 2022-02-01 2:26 ` Masami Hiramatsu 2022-02-01 3:16 ` AKASHI Takahiro 2022-01-31 12:38 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox