| Message ID | 20240110011637.174710-3-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cmd: bootefi: refactor the code for bootmgr | expand |
On 1/10/24 02:16, AKASHI Takahiro wrote: > At this point, EFI boot manager interfaces is fully independent from > bootefi command. So just rename the configuration parameter. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > Reviewed-by: Simon Glass <sjg@chromium.org> This patch breaks the 'bootefi hello' command in qemu-riscv64_smode_defconfig and other QEMU defconfigs. Best regards Heinrich
On Sun, Jan 14, 2024 at 01:46:45PM +0100, Heinrich Schuchardt wrote: > On 1/10/24 02:16, AKASHI Takahiro wrote: > > At this point, EFI boot manager interfaces is fully independent from > > bootefi command. So just rename the configuration parameter. > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > This patch breaks the 'bootefi hello' command in > qemu-riscv64_smode_defconfig and other QEMU defconfigs. What happened? Please elaborate details so that I can trace your issue. On my side, I didn't see any problem with "bootefi hello" on qemu-arm64 and moreover CI check (github pull request) didn't complain anything. -Takahiro Akashi > Best regards > > Heinrich
On 1/15/24 01:58, AKASHI Takahiro wrote: > On Sun, Jan 14, 2024 at 01:46:45PM +0100, Heinrich Schuchardt wrote: >> On 1/10/24 02:16, AKASHI Takahiro wrote: >>> At this point, EFI boot manager interfaces is fully independent from >>> bootefi command. So just rename the configuration parameter. >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> This patch breaks the 'bootefi hello' command in >> qemu-riscv64_smode_defconfig and other QEMU defconfigs. > > What happened? Please elaborate details so that I can trace your issue. > > On my side, I didn't see any problem with "bootefi hello" on qemu-arm64 > and moreover CI check (github pull request) didn't complain anything. The failures are logged in https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19302 I have pushed the tag failed_20240114 to https://source.denx.de/u-boot/custodians/u-boot-efi.git for analysis. Best regards Heinrich
On Mon, Jan 15, 2024 at 09:34:51AM +0100, Heinrich Schuchardt wrote: > On 1/15/24 01:58, AKASHI Takahiro wrote: > > On Sun, Jan 14, 2024 at 01:46:45PM +0100, Heinrich Schuchardt wrote: > > > On 1/10/24 02:16, AKASHI Takahiro wrote: > > > > At this point, EFI boot manager interfaces is fully independent from > > > > bootefi command. So just rename the configuration parameter. > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > This patch breaks the 'bootefi hello' command in > > > qemu-riscv64_smode_defconfig and other QEMU defconfigs. > > > > What happened? Please elaborate details so that I can trace your issue. > > > > On my side, I didn't see any problem with "bootefi hello" on qemu-arm64 > > and moreover CI check (github pull request) didn't complain anything. > > The failures are logged in > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19302 In the logs for qemu-arm64, qemu-risc64 and maybe others as well, I see the same error: --- --- ______________________________ test_efi_grub_net _______________________________ test/py/tests/test_efi_loader.py:188: in test_efi_grub_net addr = fetch_tftp_file(u_boot_console, 'env__efi_loader_grub_file') test/py/tests/test_efi_loader.py:136: in fetch_tftp_file assert expected_text in output E assert 'Bytes transferred = 520192' in "*** ERROR: `serverip' not set" --- --- This seems to be the root cause and my commit will have nothing to do with the problem. Please check the test environment. -Takahiro Akashi > I have pushed the tag failed_20240114 to > https://source.denx.de/u-boot/custodians/u-boot-efi.git for analysis. > > Best regards > > Heinrich
On 15.01.24 11:12, AKASHI Takahiro wrote: > On Mon, Jan 15, 2024 at 09:34:51AM +0100, Heinrich Schuchardt wrote: >> On 1/15/24 01:58, AKASHI Takahiro wrote: >>> On Sun, Jan 14, 2024 at 01:46:45PM +0100, Heinrich Schuchardt wrote: >>>> On 1/10/24 02:16, AKASHI Takahiro wrote: >>>>> At this point, EFI boot manager interfaces is fully independent from >>>>> bootefi command. So just rename the configuration parameter. >>>>> >>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>> >>>> This patch breaks the 'bootefi hello' command in >>>> qemu-riscv64_smode_defconfig and other QEMU defconfigs. >>> >>> What happened? Please elaborate details so that I can trace your issue. >>> >>> On my side, I didn't see any problem with "bootefi hello" on qemu-arm64 >>> and moreover CI check (github pull request) didn't complain anything. >> >> The failures are logged in >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19302 > > In the logs for qemu-arm64, qemu-risc64 and maybe others as well, > I see the same error: > --- --- > ______________________________ test_efi_grub_net _______________________________ > test/py/tests/test_efi_loader.py:188: in test_efi_grub_net > addr = fetch_tftp_file(u_boot_console, 'env__efi_loader_grub_file') > test/py/tests/test_efi_loader.py:136: in fetch_tftp_file > assert expected_text in output > E assert 'Bytes transferred = 520192' in "*** ERROR: `serverip' not set" > --- --- > > This seems to be the root cause and my commit will have nothing to do > with the problem. > Please check the test environment. You are looking at test_efi_grub_net. I referred to 'bootefi hello', i.e. test_efi_helloworld_builtin: ----------------------------- Captured stdout call ----------------------------- => bootefi hello => The command does not provide any output. Best regards Heinrich > > -Takahiro Akashi > >> I have pushed the tag failed_20240114 to >> https://source.denx.de/u-boot/custodians/u-boot-efi.git for analysis. >> >> Best regards >> >> Heinrich
On Mon, Jan 15, 2024 at 03:16:22PM +0100, Heinrich Schuchardt wrote: > On 15.01.24 11:12, AKASHI Takahiro wrote: > > On Mon, Jan 15, 2024 at 09:34:51AM +0100, Heinrich Schuchardt wrote: > > > On 1/15/24 01:58, AKASHI Takahiro wrote: > > > > On Sun, Jan 14, 2024 at 01:46:45PM +0100, Heinrich Schuchardt wrote: > > > > > On 1/10/24 02:16, AKASHI Takahiro wrote: > > > > > > At this point, EFI boot manager interfaces is fully independent from > > > > > > bootefi command. So just rename the configuration parameter. > > > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > This patch breaks the 'bootefi hello' command in > > > > > qemu-riscv64_smode_defconfig and other QEMU defconfigs. > > > > > > > > What happened? Please elaborate details so that I can trace your issue. > > > > > > > > On my side, I didn't see any problem with "bootefi hello" on qemu-arm64 > > > > and moreover CI check (github pull request) didn't complain anything. > > > > > > The failures are logged in > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19302 > > > > In the logs for qemu-arm64, qemu-risc64 and maybe others as well, > > I see the same error: > > --- --- > > ______________________________ test_efi_grub_net _______________________________ > > test/py/tests/test_efi_loader.py:188: in test_efi_grub_net > > addr = fetch_tftp_file(u_boot_console, 'env__efi_loader_grub_file') > > test/py/tests/test_efi_loader.py:136: in fetch_tftp_file > > assert expected_text in output > > E assert 'Bytes transferred = 520192' in "*** ERROR: `serverip' not set" > > --- --- > > > > This seems to be the root cause and my commit will have nothing to do > > with the problem. > > Please check the test environment. > > You are looking at test_efi_grub_net. I referred to 'bootefi hello', > i.e. test_efi_helloworld_builtin: > > > ----------------------------- Captured stdout call > ----------------------------- > => bootefi hello > > => > > > The command does not provide any output. Ah, yeah, but > Best regards > > Heinrich > > > > > > -Takahiro Akashi > > > > > I have pushed the tag failed_20240114 to > > > https://source.denx.de/u-boot/custodians/u-boot-efi.git for analysis. While I compiled this code and tried to run "bootefi hello", I could not reproduce this issue, always seeing the right output either on qemu-arm64 or sandbox64. (I also invoked test_efi_loader/efi_helloworld_builtin locally, but the test passed.) So no clue. -Takahiro Akashi > > > Best regards > > > > > > Heinrich >
On 1/16/24 02:43, AKASHI Takahiro wrote: > On Mon, Jan 15, 2024 at 03:16:22PM +0100, Heinrich Schuchardt wrote: >> On 15.01.24 11:12, AKASHI Takahiro wrote: >>> On Mon, Jan 15, 2024 at 09:34:51AM +0100, Heinrich Schuchardt wrote: >>>> On 1/15/24 01:58, AKASHI Takahiro wrote: >>>>> On Sun, Jan 14, 2024 at 01:46:45PM +0100, Heinrich Schuchardt wrote: >>>>>> On 1/10/24 02:16, AKASHI Takahiro wrote: >>>>>>> At this point, EFI boot manager interfaces is fully independent from >>>>>>> bootefi command. So just rename the configuration parameter. >>>>>>> >>>>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>>>>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>>>> >>>>>> This patch breaks the 'bootefi hello' command in >>>>>> qemu-riscv64_smode_defconfig and other QEMU defconfigs. >>>>> >>>>> What happened? Please elaborate details so that I can trace your issue. >>>>> >>>>> On my side, I didn't see any problem with "bootefi hello" on qemu-arm64 >>>>> and moreover CI check (github pull request) didn't complain anything. >>>> >>>> The failures are logged in >>>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19302 >>> >>> In the logs for qemu-arm64, qemu-risc64 and maybe others as well, >>> I see the same error: >>> --- --- >>> ______________________________ test_efi_grub_net _______________________________ >>> test/py/tests/test_efi_loader.py:188: in test_efi_grub_net >>> addr = fetch_tftp_file(u_boot_console, 'env__efi_loader_grub_file') >>> test/py/tests/test_efi_loader.py:136: in fetch_tftp_file >>> assert expected_text in output >>> E assert 'Bytes transferred = 520192' in "*** ERROR: `serverip' not set" >>> --- --- >>> >>> This seems to be the root cause and my commit will have nothing to do >>> with the problem. >>> Please check the test environment. >> >> You are looking at test_efi_grub_net. I referred to 'bootefi hello', >> i.e. test_efi_helloworld_builtin: >> >> >> ----------------------------- Captured stdout call >> ----------------------------- >> => bootefi hello >> >> => >> >> >> The command does not provide any output. > > Ah, yeah, but > >>> >>> -Takahiro Akashi >>> >>>> I have pushed the tag failed_20240114 to >>>> https://source.denx.de/u-boot/custodians/u-boot-efi.git for analysis. > > While I compiled this code and tried to run "bootefi hello", > I could not reproduce this issue, always seeing the right output > either on qemu-arm64 or sandbox64. > (I also invoked test_efi_loader/efi_helloworld_builtin locally, > but the test passed.) > So no clue. Please, rebase your patch set upon origin/master and retry the tests. Best regards Heinrich
On Tue, Jan 16, 2024 at 07:44:22PM +0100, Heinrich Schuchardt wrote: > On 1/16/24 02:43, AKASHI Takahiro wrote: > > On Mon, Jan 15, 2024 at 03:16:22PM +0100, Heinrich Schuchardt wrote: > > > On 15.01.24 11:12, AKASHI Takahiro wrote: > > > > On Mon, Jan 15, 2024 at 09:34:51AM +0100, Heinrich Schuchardt wrote: > > > > > On 1/15/24 01:58, AKASHI Takahiro wrote: > > > > > > On Sun, Jan 14, 2024 at 01:46:45PM +0100, Heinrich Schuchardt wrote: > > > > > > > On 1/10/24 02:16, AKASHI Takahiro wrote: > > > > > > > > At this point, EFI boot manager interfaces is fully independent from > > > > > > > > bootefi command. So just rename the configuration parameter. > > > > > > > > > > > > > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > > > > > > > > > > > This patch breaks the 'bootefi hello' command in > > > > > > > qemu-riscv64_smode_defconfig and other QEMU defconfigs. > > > > > > > > > > > > What happened? Please elaborate details so that I can trace your issue. > > > > > > > > > > > > On my side, I didn't see any problem with "bootefi hello" on qemu-arm64 > > > > > > and moreover CI check (github pull request) didn't complain anything. > > > > > > > > > > The failures are logged in > > > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/19302 > > > > > > > > In the logs for qemu-arm64, qemu-risc64 and maybe others as well, > > > > I see the same error: > > > > --- --- > > > > ______________________________ test_efi_grub_net _______________________________ > > > > test/py/tests/test_efi_loader.py:188: in test_efi_grub_net > > > > addr = fetch_tftp_file(u_boot_console, 'env__efi_loader_grub_file') > > > > test/py/tests/test_efi_loader.py:136: in fetch_tftp_file > > > > assert expected_text in output > > > > E assert 'Bytes transferred = 520192' in "*** ERROR: `serverip' not set" > > > > --- --- > > > > > > > > This seems to be the root cause and my commit will have nothing to do > > > > with the problem. > > > > Please check the test environment. > > > > > > You are looking at test_efi_grub_net. I referred to 'bootefi hello', > > > i.e. test_efi_helloworld_builtin: > > > > > > > > > ----------------------------- Captured stdout call > > > ----------------------------- > > > => bootefi hello > > > > > > => > > > > > > > > > The command does not provide any output. > > > > Ah, yeah, but > > > > > > > > > > -Takahiro Akashi > > > > > > > > > I have pushed the tag failed_20240114 to > > > > > https://source.denx.de/u-boot/custodians/u-boot-efi.git for analysis. > > > > While I compiled this code and tried to run "bootefi hello", > > I could not reproduce this issue, always seeing the right output > > either on qemu-arm64 or sandbox64. > > (I also invoked test_efi_loader/efi_helloworld_builtin locally, > > but the test passed.) > > So no clue. > > Please, rebase your patch set upon origin/master and retry the tests. After rebasing my commits to the latest master (please note that my v4 has already been rebased on the master at my submission time), I tried the test again but anyhow the test passed as before on my side. I'm not able to reproduce your issue. The only conflicting commit was f19171c919e0 ("efi_loader: Clean up efi_dp_append and efi_dp_concat"). I don't think that it would make any difference. -Takahiro Akashi > Best regards > > Heinrich >
diff --git a/boot/Makefile b/boot/Makefile index a90ebea5a867..bcd01cfc890c 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -34,7 +34,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_CROS) += bootm.o bootm_os.o bootmeth_cros.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SANDBOX) += bootmeth_sandbox.o obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_SCRIPT) += bootmeth_script.o ifdef CONFIG_$(SPL_TPL_)BOOTSTD_FULL -obj-$(CONFIG_BOOTEFI_BOOTMGR) += bootmeth_efi_mgr.o +obj-$(CONFIG_EFI_BOOTMGR) += bootmeth_efi_mgr.o obj-$(CONFIG_$(SPL_TPL_)EXPO) += bootflow_menu.o obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += bootflow_menu.o obj-$(CONFIG_$(SPL_TPL_)CEDIT) += cedit.o diff --git a/cmd/Kconfig b/cmd/Kconfig index 150fa37a50a7..62d2ae3d3f1b 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -395,7 +395,7 @@ config CMD_BOOTEFI_BINARY config CMD_BOOTEFI_BOOTMGR bool "UEFI Boot Manager command" - depends on BOOTEFI_BOOTMGR + depends on EFI_BOOTMGR default y help Select this option to enable the 'bootmgr' subcommand of 'bootefi'. @@ -2153,7 +2153,7 @@ config CMD_EFIDEBUG config CMD_EFICONFIG bool "eficonfig - provide menu-driven uefi variables maintenance interface" default y if !HAS_BOARD_SIZE_LIMIT - depends on BOOTEFI_BOOTMGR + depends on EFI_BOOTMGR select MENU help Enable the 'eficonfig' command which provides the menu-driven UEFI diff --git a/cmd/efidebug.c b/cmd/efidebug.c index e10fbf891a42..b4954258eeba 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -1410,7 +1410,7 @@ static __maybe_unused int do_efi_test_bootmgr(struct cmd_tbl *cmdtp, int flag, } static struct cmd_tbl cmd_efidebug_test_sub[] = { -#ifdef CONFIG_BOOTEFI_BOOTMGR +#ifdef CONFIG_EFI_BOOTMGR U_BOOT_CMD_MKENT(bootmgr, CONFIG_SYS_MAXARGS, 1, do_efi_test_bootmgr, "", ""), #endif @@ -1604,7 +1604,7 @@ U_BOOT_LONGHELP(efidebug, " - show UEFI memory map\n" "efidebug tables\n" " - show UEFI configuration tables\n" -#ifdef CONFIG_BOOTEFI_BOOTMGR +#ifdef CONFIG_EFI_BOOTMGR "efidebug test bootmgr\n" " - run simple bootmgr for test\n" #endif diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 64f2f1cdd161..db5571de1d95 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -41,7 +41,7 @@ config EFI_BINARY_EXEC You may enable CMD_BOOTEFI_BINARY so that you can use bootefi command to do that. -config BOOTEFI_BOOTMGR +config EFI_BOOTMGR bool "UEFI Boot Manager" default y select BOOTMETH_GLOBAL if BOOTSTD diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile index f85d26d9724c..56e5ce220a3e 100644 --- a/lib/efi_loader/Makefile +++ b/lib/efi_loader/Makefile @@ -42,7 +42,7 @@ targets += initrddump.o endif obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o -obj-$(CONFIG_BOOTEFI_BOOTMGR) += efi_bootmgr.o +obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o obj-$(CONFIG_EFI_BINARY_EXEC) += efi_bootbin.o obj-y += efi_boottime.o obj-y += efi_helper.o diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c index 104f49deef2a..fa54dde661c8 100644 --- a/test/boot/bootflow.c +++ b/test/boot/bootflow.c @@ -374,7 +374,7 @@ static int bootflow_system(struct unit_test_state *uts) { struct udevice *bootstd, *dev; - if (!IS_ENABLED(CONFIG_BOOTEFI_BOOTMGR)) + if (!IS_ENABLED(CONFIG_EFI_BOOTMGR)) return -EAGAIN; ut_assertok(uclass_first_device_err(UCLASS_BOOTSTD, &bootstd)); ut_assertok(device_bind(bootstd, DM_DRIVER_GET(bootmeth_efi_mgr),