| Message ID | 164741778621.309780.13612431059777970603.stgit@localhost |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] efi_loader: Use sysreset instead of reset command | expand |
On 3/16/22 09:03, Masami Hiramatsu wrote: > Use sysreset_walk_halt() directly from reset-after-capsule-on-disk > feature to reboot (cold reset) machine instead of using reset command > interface, since this is not a command. > Note that this will make CONFIG_EFI_CAPSULE_ON_DISK depending on > the CONFIG_SYSRESET. > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > Changes in v2: > - Add CONFIG_SYSRESET dependency. > - Fix to add #include <sysreset.h> > --- > lib/efi_loader/Kconfig | 1 + > lib/efi_loader/efi_capsule.c | 5 +++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index e5e35fe51f..33138237cc 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -130,6 +130,7 @@ config EFI_RUNTIME_UPDATE_CAPSULE > > config EFI_CAPSULE_ON_DISK > bool "Enable capsule-on-disk support" > + depends on SYSRESET > select EFI_HAVE_CAPSULE_SUPPORT > help > Select this option if you want to use capsule-on-disk feature, > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > index 613b531b82..a3d1d7e95a 100644 > --- a/lib/efi_loader/efi_capsule.c > +++ b/lib/efi_loader/efi_capsule.c > @@ -18,6 +18,7 @@ > #include <malloc.h> > #include <mapmem.h> > #include <sort.h> > +#include <sysreset.h> > #include <asm/global_data.h> > > #include <crypto/pkcs7.h> > @@ -1150,9 +1151,9 @@ efi_status_t efi_launch_capsules(void) > * UEFI spec requires to reset system after complete processing capsule > * update on the storage. > */ > - log_info("Reboot after firmware update"); > + log_info("Reboot after firmware update.\n"); > /* Cold reset is required for loading the new firmware. */ > - do_reset(NULL, 0, 0, NULL); > + sysreset_walk_halt(SYSRESET_COLD); > hang(); > /* not reach here */ > >
On Wed, 16 Mar 2022 at 06:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 3/16/22 09:03, Masami Hiramatsu wrote: > > Use sysreset_walk_halt() directly from reset-after-capsule-on-disk > > feature to reboot (cold reset) machine instead of using reset command > > interface, since this is not a command. > > Note that this will make CONFIG_EFI_CAPSULE_ON_DISK depending on > > the CONFIG_SYSRESET. > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > > > --- > > Changes in v2: > > - Add CONFIG_SYSRESET dependency. > > - Fix to add #include <sysreset.h> > > --- > > lib/efi_loader/Kconfig | 1 + > > lib/efi_loader/efi_capsule.c | 5 +++-- > > 2 files changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index e5e35fe51f..33138237cc 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -130,6 +130,7 @@ config EFI_RUNTIME_UPDATE_CAPSULE > > > > config EFI_CAPSULE_ON_DISK > > bool "Enable capsule-on-disk support" > > + depends on SYSRESET > > select EFI_HAVE_CAPSULE_SUPPORT > > help > > Select this option if you want to use capsule-on-disk feature, > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index 613b531b82..a3d1d7e95a 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -18,6 +18,7 @@ > > #include <malloc.h> > > #include <mapmem.h> > > #include <sort.h> > > +#include <sysreset.h> > > #include <asm/global_data.h> > > > > #include <crypto/pkcs7.h> > > @@ -1150,9 +1151,9 @@ efi_status_t efi_launch_capsules(void) > > * UEFI spec requires to reset system after complete processing capsule > > * update on the storage. > > */ > > - log_info("Reboot after firmware update"); > > + log_info("Reboot after firmware update.\n"); > > /* Cold reset is required for loading the new firmware. */ > > - do_reset(NULL, 0, 0, NULL); > > + sysreset_walk_halt(SYSRESET_COLD); > > hang(); > > /* not reach here */ > > > > >
On 3/16/22 20:23, Simon Glass wrote: > On Wed, 16 Mar 2022 at 06:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 3/16/22 09:03, Masami Hiramatsu wrote: >>> Use sysreset_walk_halt() directly from reset-after-capsule-on-disk >>> feature to reboot (cold reset) machine instead of using reset command >>> interface, since this is not a command. >>> Note that this will make CONFIG_EFI_CAPSULE_ON_DISK depending on >>> the CONFIG_SYSRESET. >>> >>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> >> >> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> >>> --- >>> Changes in v2: >>> - Add CONFIG_SYSRESET dependency. >>> - Fix to add #include <sysreset.h> >>> --- >>> lib/efi_loader/Kconfig | 1 + >>> lib/efi_loader/efi_capsule.c | 5 +++-- >>> 2 files changed, 4 insertions(+), 2 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > >>> >>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >>> index e5e35fe51f..33138237cc 100644 >>> --- a/lib/efi_loader/Kconfig >>> +++ b/lib/efi_loader/Kconfig >>> @@ -130,6 +130,7 @@ config EFI_RUNTIME_UPDATE_CAPSULE >>> >>> config EFI_CAPSULE_ON_DISK >>> bool "Enable capsule-on-disk support" >>> + depends on SYSRESET With this change xilinx_zynq_virt_defconfig fails to build: arm-linux-gnueabi-ld.bfd: cmd/efidebug.o: in function `do_efi_capsule_on_disk_update': /home/zfsdt/workspace/u-boot-build/denx/cmd/efidebug.c:92: undefined reference to `efi_launch_capsules' Command 'efi capsule disk-update' should only be enabled if CONFIG_EFI_CAPSULE_ON_DISK=y. arm-linux-gnueabi-ld.bfd: lib/efi_loader/efi_setup.o: in function `efi_init_obj_list': /home/zfsdt/workspace/u-boot-build/denx/lib/efi_loader/efi_setup.c:272: undefined reference to `efi_load_capsule_drivers' make: *** [Makefile:1801: u-boot] Error 1 efi_load_capsule_drivers() must be available even if EFI_CAPSULE_ON_DISK is not enabled to make CapsuleUpdate() work. Please, add the missing changes. Best regards Heinrich >>> select EFI_HAVE_CAPSULE_SUPPORT >>> help >>> Select this option if you want to use capsule-on-disk feature, >>> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c >>> index 613b531b82..a3d1d7e95a 100644 >>> --- a/lib/efi_loader/efi_capsule.c >>> +++ b/lib/efi_loader/efi_capsule.c >>> @@ -18,6 +18,7 @@ >>> #include <malloc.h> >>> #include <mapmem.h> >>> #include <sort.h> >>> +#include <sysreset.h> >>> #include <asm/global_data.h> >>> >>> #include <crypto/pkcs7.h> >>> @@ -1150,9 +1151,9 @@ efi_status_t efi_launch_capsules(void) >>> * UEFI spec requires to reset system after complete processing capsule >>> * update on the storage. >>> */ >>> - log_info("Reboot after firmware update"); >>> + log_info("Reboot after firmware update.\n"); >>> /* Cold reset is required for loading the new firmware. */ >>> - do_reset(NULL, 0, 0, NULL); >>> + sysreset_walk_halt(SYSRESET_COLD); >>> hang(); >>> /* not reach here */ >>> >>> >>
Hi Heinrich, Thanks for finding the issues. 2022年3月19日(土) 18:02 Heinrich Schuchardt <xypron.glpk@gmx.de>: > > On 3/16/22 20:23, Simon Glass wrote: > > On Wed, 16 Mar 2022 at 06:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> On 3/16/22 09:03, Masami Hiramatsu wrote: > >>> Use sysreset_walk_halt() directly from reset-after-capsule-on-disk > >>> feature to reboot (cold reset) machine instead of using reset command > >>> interface, since this is not a command. > >>> Note that this will make CONFIG_EFI_CAPSULE_ON_DISK depending on > >>> the CONFIG_SYSRESET. > >>> > >>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > >> > >> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> > >>> --- > >>> Changes in v2: > >>> - Add CONFIG_SYSRESET dependency. > >>> - Fix to add #include <sysreset.h> > >>> --- > >>> lib/efi_loader/Kconfig | 1 + > >>> lib/efi_loader/efi_capsule.c | 5 +++-- > >>> 2 files changed, 4 insertions(+), 2 deletions(-) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > >>> > >>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > >>> index e5e35fe51f..33138237cc 100644 > >>> --- a/lib/efi_loader/Kconfig > >>> +++ b/lib/efi_loader/Kconfig > >>> @@ -130,6 +130,7 @@ config EFI_RUNTIME_UPDATE_CAPSULE > >>> > >>> config EFI_CAPSULE_ON_DISK > >>> bool "Enable capsule-on-disk support" > >>> + depends on SYSRESET > > With this change xilinx_zynq_virt_defconfig fails to build: > > arm-linux-gnueabi-ld.bfd: cmd/efidebug.o: in function > `do_efi_capsule_on_disk_update': > /home/zfsdt/workspace/u-boot-build/denx/cmd/efidebug.c:92: undefined > reference to `efi_launch_capsules' > > Command 'efi capsule disk-update' should only be enabled if > CONFIG_EFI_CAPSULE_ON_DISK=y. OK. > arm-linux-gnueabi-ld.bfd: lib/efi_loader/efi_setup.o: in function > `efi_init_obj_list': > /home/zfsdt/workspace/u-boot-build/denx/lib/efi_loader/efi_setup.c:272: > undefined reference to `efi_load_capsule_drivers' > make: *** [Makefile:1801: u-boot] Error 1 > > efi_load_capsule_drivers() must be available even if EFI_CAPSULE_ON_DISK > is not enabled to make CapsuleUpdate() work. Yes, we have CONFIG_EFI_HAVE_CAPSULE_SUPPORT for such case. > > Please, add the missing changes. OK, let me update the series. Thank you! > > Best regards > > Heinrich > > >>> select EFI_HAVE_CAPSULE_SUPPORT > >>> help > >>> Select this option if you want to use capsule-on-disk feature, > >>> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > >>> index 613b531b82..a3d1d7e95a 100644 > >>> --- a/lib/efi_loader/efi_capsule.c > >>> +++ b/lib/efi_loader/efi_capsule.c > >>> @@ -18,6 +18,7 @@ > >>> #include <malloc.h> > >>> #include <mapmem.h> > >>> #include <sort.h> > >>> +#include <sysreset.h> > >>> #include <asm/global_data.h> > >>> > >>> #include <crypto/pkcs7.h> > >>> @@ -1150,9 +1151,9 @@ efi_status_t efi_launch_capsules(void) > >>> * UEFI spec requires to reset system after complete processing capsule > >>> * update on the storage. > >>> */ > >>> - log_info("Reboot after firmware update"); > >>> + log_info("Reboot after firmware update.\n"); > >>> /* Cold reset is required for loading the new firmware. */ > >>> - do_reset(NULL, 0, 0, NULL); > >>> + sysreset_walk_halt(SYSRESET_COLD); > >>> hang(); > >>> /* not reach here */ > >>> > >>> > >> >
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index e5e35fe51f..33138237cc 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -130,6 +130,7 @@ config EFI_RUNTIME_UPDATE_CAPSULE config EFI_CAPSULE_ON_DISK bool "Enable capsule-on-disk support" + depends on SYSRESET select EFI_HAVE_CAPSULE_SUPPORT help Select this option if you want to use capsule-on-disk feature, diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 613b531b82..a3d1d7e95a 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -18,6 +18,7 @@ #include <malloc.h> #include <mapmem.h> #include <sort.h> +#include <sysreset.h> #include <asm/global_data.h> #include <crypto/pkcs7.h> @@ -1150,9 +1151,9 @@ efi_status_t efi_launch_capsules(void) * UEFI spec requires to reset system after complete processing capsule * update on the storage. */ - log_info("Reboot after firmware update"); + log_info("Reboot after firmware update.\n"); /* Cold reset is required for loading the new firmware. */ - do_reset(NULL, 0, 0, NULL); + sysreset_walk_halt(SYSRESET_COLD); hang(); /* not reach here */
Use sysreset_walk_halt() directly from reset-after-capsule-on-disk feature to reboot (cold reset) machine instead of using reset command interface, since this is not a command. Note that this will make CONFIG_EFI_CAPSULE_ON_DISK depending on the CONFIG_SYSRESET. Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> --- Changes in v2: - Add CONFIG_SYSRESET dependency. - Fix to add #include <sysreset.h> --- lib/efi_loader/Kconfig | 1 + lib/efi_loader/efi_capsule.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-)