| Message ID | 164269266038.39378.1931983995518536620.stgit@localhost |
|---|---|
| State | New |
| Headers | show |
| Series | FWU: Add FWU Multi Bank Update for DeveloerBox | expand |
On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote: > Reboot to the trial state soon after successfully installing > the new firmware to the next bank and updating the active_index. > This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a > recommended option. EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET. See Section "8.5.3 Update Capsule" in the UEFI specification. I think that we'd better implement the feature rather than adding CONFIG_FWU_REBOOT_AFTER_UPDATE. -Takahiro Akashi > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > --- > lib/efi_loader/efi_capsule.c | 10 ++++++++-- > lib/fwu_updates/Kconfig | 9 +++++++++ > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 83c89a0cbb..0928425b5f 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void) > } else { > log_debug("Successfully updated the active_index\n"); > status = fwu_trial_state_ctr_start(); > - if (status < 0) > + if (status < 0) { > ret = EFI_DEVICE_ERROR; > - else > + } else { > ret = EFI_SUCCESS; > + if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) { > + log_info("New firmware is installed in bank#%d. Reboot from that bank.\n", > + update_index); > + do_reset(NULL, 0, 0, NULL); > + } > + } > } > } else if (capsule_update == true && update_status == false) { > log_err("All capsules were not updated. Not updating FWU metadata\n"); > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig > index 6de28e0c9c..0940a90747 100644 > --- a/lib/fwu_updates/Kconfig > +++ b/lib/fwu_updates/Kconfig > @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT > With FWU Multi Bank Update feature enabled, number of times > the platform is allowed to boot in Trial State after an > update. > + > +config FWU_REBOOT_AFTER_UPDATE > + bool "Reboot soon after installing new firmware" > + depends on FWU_MULTI_BANK_UPDATE > + default y > + help > + Reboot the machine soon after installing a new firmware > + and start trial boot. You can disable this option for > + debugging or FWU development, but recommended to enable it. >
Hi Takahiro, 2022年1月21日(金) 10:46 AKASHI Takahiro <takahiro.akashi@linaro.org>: > > On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote: > > Reboot to the trial state soon after successfully installing > > the new firmware to the next bank and updating the active_index. > > This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a > > recommended option. > > EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET. > See Section "8.5.3 Update Capsule" in the UEFI specification. > > I think that we'd better implement the feature rather than adding > CONFIG_FWU_REBOOT_AFTER_UPDATE. Thanks for pointing it! I agree with you, the flag is more useful. Regards, > > -Takahiro Akashi > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > --- > > lib/efi_loader/efi_capsule.c | 10 ++++++++-- > > lib/fwu_updates/Kconfig | 9 +++++++++ > > 2 files changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index 83c89a0cbb..0928425b5f 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void) > > } else { > > log_debug("Successfully updated the active_index\n"); > > status = fwu_trial_state_ctr_start(); > > - if (status < 0) > > + if (status < 0) { > > ret = EFI_DEVICE_ERROR; > > - else > > + } else { > > ret = EFI_SUCCESS; > > + if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) { > > + log_info("New firmware is installed in bank#%d. Reboot from that bank.\n", > > + update_index); > > + do_reset(NULL, 0, 0, NULL); > > + } > > + } > > } > > } else if (capsule_update == true && update_status == false) { > > log_err("All capsules were not updated. Not updating FWU metadata\n"); > > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig > > index 6de28e0c9c..0940a90747 100644 > > --- a/lib/fwu_updates/Kconfig > > +++ b/lib/fwu_updates/Kconfig > > @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT > > With FWU Multi Bank Update feature enabled, number of times > > the platform is allowed to boot in Trial State after an > > update. > > + > > +config FWU_REBOOT_AFTER_UPDATE > > + bool "Reboot soon after installing new firmware" > > + depends on FWU_MULTI_BANK_UPDATE > > + default y > > + help > > + Reboot the machine soon after installing a new firmware > > + and start trial boot. You can disable this option for > > + debugging or FWU development, but recommended to enable it. > >
Hi, 2022年1月21日(金) 13:35 Masami Hiramatsu <masami.hiramatsu@linaro.org>: > > Hi Takahiro, > > 2022年1月21日(金) 10:46 AKASHI Takahiro <takahiro.akashi@linaro.org>: > > > > On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote: > > > Reboot to the trial state soon after successfully installing > > > the new firmware to the next bank and updating the active_index. > > > This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a > > > recommended option. > > > > EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET. > > See Section "8.5.3 Update Capsule" in the UEFI specification. > > > > I think that we'd better implement the feature rather than adding > > CONFIG_FWU_REBOOT_AFTER_UPDATE. > > Thanks for pointing it! I agree with you, the flag is more useful. According to the UEFI spec 2.9, we need to consider implementing some related things. In 8.5.3 Update Capsule ---- A capsule which has the CAPSULE_FLAGS_INITIATE_RESET Flag must have CAPSULE_FLAGS_PERSIST_ACROSS_RESET set in its header as well. [...] If a capsule has the CAPSULE_FLAGS_PERSIST_ACROSS_RESET Flag set in its header, the firmware will process the capsules after system reset. The caller must ensure to reset the system using the required reset value obtained from QueryCapsuleCapabilities. ---- In Table 8-8 Flag Firmware Behavior ---- CAPSULE_FLAGS_PERSIST_ACROSS_RESET + CAPSULE_FLAGS_INITIATE_RESET Firmware will attempt to process or launch the capsule across a reset. The firmware will initiate a reset which is compatible with the passed-in capsule request and will not return back to the caller. If the capsule is not recognized, can expect an error. If the processing requires a reset which is unsupported by the platform, expect an error. ---- So, I have 2 questions; 1) Should we implement CAPSULE_FLAGS_PERSIST_ACROSS_RESET too? Since U-Boot only supports capsule update on disk, it seems the capsule already applied "across a reset". :-) 2) If there are multiple capsule files and only one file (e.g. aaaa.cap) has CAPSULE_FLAGS_INITIATE_RESET flag, should U-Boot resets after applying that capsule, or wait after all capsule files are applied? (current implementation is the latter one) Thank you, > > Regards, > > > > > -Takahiro Akashi > > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > > --- > > > lib/efi_loader/efi_capsule.c | 10 ++++++++-- > > > lib/fwu_updates/Kconfig | 9 +++++++++ > > > 2 files changed, 17 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > > index 83c89a0cbb..0928425b5f 100644 > > > --- a/lib/efi_loader/efi_capsule.c > > > +++ b/lib/efi_loader/efi_capsule.c > > > @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void) > > > } else { > > > log_debug("Successfully updated the active_index\n"); > > > status = fwu_trial_state_ctr_start(); > > > - if (status < 0) > > > + if (status < 0) { > > > ret = EFI_DEVICE_ERROR; > > > - else > > > + } else { > > > ret = EFI_SUCCESS; > > > + if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) { > > > + log_info("New firmware is installed in bank#%d. Reboot from that bank.\n", > > > + update_index); > > > + do_reset(NULL, 0, 0, NULL); > > > + } > > > + } > > > } > > > } else if (capsule_update == true && update_status == false) { > > > log_err("All capsules were not updated. Not updating FWU metadata\n"); > > > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig > > > index 6de28e0c9c..0940a90747 100644 > > > --- a/lib/fwu_updates/Kconfig > > > +++ b/lib/fwu_updates/Kconfig > > > @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT > > > With FWU Multi Bank Update feature enabled, number of times > > > the platform is allowed to boot in Trial State after an > > > update. > > > + > > > +config FWU_REBOOT_AFTER_UPDATE > > > + bool "Reboot soon after installing new firmware" > > > + depends on FWU_MULTI_BANK_UPDATE > > > + default y > > > + help > > > + Reboot the machine soon after installing a new firmware > > > + and start trial boot. You can disable this option for > > > + debugging or FWU development, but recommended to enable it. > > > > > > > -- > Masami Hiramatsu
On Fri, Jan 21, 2022 at 03:54:12PM +0900, Masami Hiramatsu wrote: > Hi, > > 2022年1月21日(金) 13:35 Masami Hiramatsu <masami.hiramatsu@linaro.org>: > > > > Hi Takahiro, > > > > 2022年1月21日(金) 10:46 AKASHI Takahiro <takahiro.akashi@linaro.org>: > > > > > > On Fri, Jan 21, 2022 at 12:31:00AM +0900, Masami Hiramatsu wrote: > > > > Reboot to the trial state soon after successfully installing > > > > the new firmware to the next bank and updating the active_index. > > > > This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a > > > > recommended option. > > > > > > EFI_CAPSULE_HEADER.Flags may have a flag, CAPSULE_FLAGS_INITIATE_RESET. > > > See Section "8.5.3 Update Capsule" in the UEFI specification. > > > > > > I think that we'd better implement the feature rather than adding > > > CONFIG_FWU_REBOOT_AFTER_UPDATE. > > > > Thanks for pointing it! I agree with you, the flag is more useful. > > According to the UEFI spec 2.9, we need to consider implementing some > related things. > > In 8.5.3 Update Capsule > ---- > A capsule which has the CAPSULE_FLAGS_INITIATE_RESET Flag must have > CAPSULE_FLAGS_PERSIST_ACROSS_RESET set in its header as well. > [...] > If a capsule has the CAPSULE_FLAGS_PERSIST_ACROSS_RESET Flag set in its > header, the firmware will process the capsules after system reset. The > caller must > ensure to reset the system using the required reset value obtained from > QueryCapsuleCapabilities. > ---- > In Table 8-8 Flag Firmware Behavior > ---- > CAPSULE_FLAGS_PERSIST_ACROSS_RESET + > CAPSULE_FLAGS_INITIATE_RESET > > Firmware will attempt to process or launch the capsule > across a reset. The firmware will initiate a reset which is > compatible with the passed-in capsule request and will > not return back to the caller. If the capsule is not > recognized, can expect an error. If the processing requires > a reset which is unsupported by the platform, expect an > error. > ---- > > So, I have 2 questions; > > 1) Should we implement CAPSULE_FLAGS_PERSIST_ACROSS_RESET too? > Since U-Boot only supports capsule update on disk, it seems the capsule already > applied "across a reset". :-) Yeah, I suppose that PERSIST_ACROSS_RESET is most for capsules via UpdateCapsule API. > 2) If there are multiple capsule files and only one file (e.g. aaaa.cap) has > CAPSULE_FLAGS_INITIATE_RESET flag, should U-Boot resets after > applying that capsule, or wait after all capsule files are applied? > (current implementation is the latter one) The order of capsules applied can be controlled by their file names. So IIUC, a reset should be initiated immediately after processing a capsule with INITIATE_RESET. The rest of capsules can be processed even after the reboot. I think that this behavior gives us more flexibility. -Takahiro Akashi > Thank you, > > > > > Regards, > > > > > > > > -Takahiro Akashi > > > > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > > > --- > > > > lib/efi_loader/efi_capsule.c | 10 ++++++++-- > > > > lib/fwu_updates/Kconfig | 9 +++++++++ > > > > 2 files changed, 17 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > > > index 83c89a0cbb..0928425b5f 100644 > > > > --- a/lib/efi_loader/efi_capsule.c > > > > +++ b/lib/efi_loader/efi_capsule.c > > > > @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void) > > > > } else { > > > > log_debug("Successfully updated the active_index\n"); > > > > status = fwu_trial_state_ctr_start(); > > > > - if (status < 0) > > > > + if (status < 0) { > > > > ret = EFI_DEVICE_ERROR; > > > > - else > > > > + } else { > > > > ret = EFI_SUCCESS; > > > > + if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) { > > > > + log_info("New firmware is installed in bank#%d. Reboot from that bank.\n", > > > > + update_index); > > > > + do_reset(NULL, 0, 0, NULL); > > > > + } > > > > + } > > > > } > > > > } else if (capsule_update == true && update_status == false) { > > > > log_err("All capsules were not updated. Not updating FWU metadata\n"); > > > > diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig > > > > index 6de28e0c9c..0940a90747 100644 > > > > --- a/lib/fwu_updates/Kconfig > > > > +++ b/lib/fwu_updates/Kconfig > > > > @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT > > > > With FWU Multi Bank Update feature enabled, number of times > > > > the platform is allowed to boot in Trial State after an > > > > update. > > > > + > > > > +config FWU_REBOOT_AFTER_UPDATE > > > > + bool "Reboot soon after installing new firmware" > > > > + depends on FWU_MULTI_BANK_UPDATE > > > > + default y > > > > + help > > > > + Reboot the machine soon after installing a new firmware > > > > + and start trial boot. You can disable this option for > > > > + debugging or FWU development, but recommended to enable it. > > > > > > > > > > > > -- > > Masami Hiramatsu > > > > -- > Masami Hiramatsu
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 83c89a0cbb..0928425b5f 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1355,10 +1355,16 @@ efi_status_t efi_launch_capsules(void) } else { log_debug("Successfully updated the active_index\n"); status = fwu_trial_state_ctr_start(); - if (status < 0) + if (status < 0) { ret = EFI_DEVICE_ERROR; - else + } else { ret = EFI_SUCCESS; + if (IS_ENABLED(CONFIG_FWU_REBOOT_AFTER_UPDATE)) { + log_info("New firmware is installed in bank#%d. Reboot from that bank.\n", + update_index); + do_reset(NULL, 0, 0, NULL); + } + } } } else if (capsule_update == true && update_status == false) { log_err("All capsules were not updated. Not updating FWU metadata\n"); diff --git a/lib/fwu_updates/Kconfig b/lib/fwu_updates/Kconfig index 6de28e0c9c..0940a90747 100644 --- a/lib/fwu_updates/Kconfig +++ b/lib/fwu_updates/Kconfig @@ -29,3 +29,12 @@ config FWU_TRIAL_STATE_CNT With FWU Multi Bank Update feature enabled, number of times the platform is allowed to boot in Trial State after an update. + +config FWU_REBOOT_AFTER_UPDATE + bool "Reboot soon after installing new firmware" + depends on FWU_MULTI_BANK_UPDATE + default y + help + Reboot the machine soon after installing a new firmware + and start trial boot. You can disable this option for + debugging or FWU development, but recommended to enable it.
Reboot to the trial state soon after successfully installing the new firmware to the next bank and updating the active_index. This is enabled by CONFIG_FWU_REBOOT_AFTER_UPDATE and is a recommended option. Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> --- lib/efi_loader/efi_capsule.c | 10 ++++++++-- lib/fwu_updates/Kconfig | 9 +++++++++ 2 files changed, 17 insertions(+), 2 deletions(-)