[PATCH v2 0/6] efi_loader: run bootdev_hunt() to find ESP
When initializing the EFI sub-system we need all block devices and the NIC to be probed. Extensions should have been applied to the device-tree.
Our current implementation only guarantees that bound block devices are probed. But this misses out devices where binding requires specific routines to be executed, e.g. NVMe (nvme scan), SCSI (scsi start).
Invoking bootdev_hunt() allows to find all block devices, probe the network nic, and enable extensions.
Reviewers complained that PXE boot was executed unexpectedly with a patch that only added the bootdev_hunt() call to the EFI sub-system initialization.
The EFI sub-system does not require the IP address to be set up. We can reduce the boottime by not executing dchp_run() in eth_bootdev_hunt().
Furthermore on the legacy network stack with autostart=yes dhcp_run() tries to download the boot file via TFTP and execute it which seems to be a problem with non-observance of autoload=no.
To fix Gitlab CI issues the following changes are necessary:
In test_extension.py do not assume that extensions have not been loaded by bootdev hunter in a previous test.
Remove CONFIG_AMIGA_PARTITION from sandbox_deconfig to avoid a timeout.
Remove CONFIG_USB_DWC3 on the CI from xilinx_versal_virt_defconfig to avoid a boot failure.
Add a parameter '-e' to select if UEFI boot options shall be shown by the bootmenu command.
v2: disable USB_DWC3 only in CI update commit message for eth_bootdev_hunt() change
Heinrich Schuchardt (6): test: fix test_extension.py configs: sandbox_deconfig: remove CONFIG_AMIGA_PARTITION CI: xilinx_versal_virt: disable USB_DWC3 net: eth_bootdev_hunt() must not try to boot cmd: bootmenu: add parameter -e for UEFI boot options efi_loader: run bootdev_hunt() to find ESP
.azure-pipelines.yml | 1 + .gitlab-ci.yml | 1 + cmd/bootmenu.c | 39 +++++++++++++++++++++++++-------- configs/sandbox_defconfig | 1 - doc/usage/cmd/bootmenu.rst | 13 ++++++++--- lib/efi_loader/efi_setup.c | 8 +++++++ net/eth_bootdev.c | 30 +++++++++++++++---------- test/py/tests/test_extension.py | 4 +++- 8 files changed, 71 insertions(+), 26 deletions(-)
test_extension.py assumes that no extension is known at test start. This assumption is wrong because we do not come out of reboot. A prior test may have already hunted for the extension bootdev.
Remove the invalid assert.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: no change --- test/py/tests/test_extension.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/test/py/tests/test_extension.py b/test/py/tests/test_extension.py index 267cf2ff27c..2a3c5116171 100644 --- a/test/py/tests/test_extension.py +++ b/test/py/tests/test_extension.py @@ -26,7 +26,9 @@ def test_extension(u_boot_console): load_dtb(u_boot_console)
output = u_boot_console.run_command('extension list') - assert('No extension' in output) + # extension_bootdev_hunt may have already run. + # Without reboot we cannot make any assumption here. + # assert('No extension' in output)
output = u_boot_console.run_command('extension scan') assert output == 'Found 2 extension board(s).'
We do not actually test the code. Scanning for Amiga partitions of the sandbox is extremely slow, especially on the partially implemented USB device.
For build testing the other sandbox defconfigs are good enough.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: no change --- configs/sandbox_defconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 718e4a8283c..683888f238f 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -142,7 +142,6 @@ CONFIG_CMD_SQUASHFS=y CONFIG_CMD_MTDPARTS=y CONFIG_CMD_STACKPROTECTOR_TEST=y CONFIG_MAC_PARTITION=y -CONFIG_AMIGA_PARTITION=y CONFIG_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_ENV_IS_NOWHERE=y
On Wed, 27 Nov 2024 at 09:06, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
We do not actually test the code. Scanning for Amiga partitions of the sandbox is extremely slow, especially on the partially implemented USB device.
For build testing the other sandbox defconfigs are good enough.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: no change
configs/sandbox_defconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 718e4a8283c..683888f238f 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -142,7 +142,6 @@ CONFIG_CMD_SQUASHFS=y CONFIG_CMD_MTDPARTS=y CONFIG_CMD_STACKPROTECTOR_TEST=y CONFIG_MAC_PARTITION=y -CONFIG_AMIGA_PARTITION=y CONFIG_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_ENV_IS_NOWHERE=y -- 2.45.2
Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org
The CI uses the following command to launch xilinx_versal_virt_defconfig:
qemu-system-aarch64 -M xlnx-versal-virt \ -display none -m 4G -serial mon:stdio \ -device loader,file=u-boot,cpu-num=0
'usb start' or invoking eth_bootdev_hunt leads to a crash when function dwc3_core_init() tries to access a register at offset 0xc704 (DWC3_DCTL) relative to the register start address 0xfe20c100.
Disable CONFIG_USB_DWC3 in the CI until the driver problem is fixed.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: Only disable CONFIG_USB_DWC3 in CI as actual hardware uses it. --- .azure-pipelines.yml | 1 + .gitlab-ci.yml | 1 + 2 files changed, 2 insertions(+)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 4ecf76eaa0b..bac577ec008 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -511,6 +511,7 @@ stages: TEST_PY_BD: "xilinx_versal_virt" TEST_PY_ID: "--id qemu" TEST_PY_TEST_SPEC: "not sleep" + OVERRIDE: "-a ~CONFIG_USB_DWC3" xtfpga: TEST_PY_BD: "xtfpga" TEST_PY_ID: "--id qemu" diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2164ad79a72..714284a56df 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -508,6 +508,7 @@ xilinx_versal_virt test.py: TEST_PY_BD: "xilinx_versal_virt" TEST_PY_TEST_SPEC: "not sleep" TEST_PY_ID: "--id qemu" + OVERRIDE: "-a ~CONFIG_USB_DWC3" <<: *buildman_and_testpy_dfn
xtfpga test.py:
On 11/27/24 08:06, Heinrich Schuchardt wrote:
The CI uses the following command to launch xilinx_versal_virt_defconfig:
qemu-system-aarch64 -M xlnx-versal-virt \ -display none -m 4G -serial mon:stdio \ -device loader,file=u-boot,cpu-num=0'usb start' or invoking eth_bootdev_hunt leads to a crash when function dwc3_core_init() tries to access a register at offset 0xc704 (DWC3_DCTL) relative to the register start address 0xfe20c100.
Disable CONFIG_USB_DWC3 in the CI until the driver problem is fixed.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: Only disable CONFIG_USB_DWC3 in CI as actual hardware uses it.
.azure-pipelines.yml | 1 + .gitlab-ci.yml | 1 + 2 files changed, 2 insertions(+)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 4ecf76eaa0b..bac577ec008 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -511,6 +511,7 @@ stages: TEST_PY_BD: "xilinx_versal_virt" TEST_PY_ID: "--id qemu" TEST_PY_TEST_SPEC: "not sleep"
OVERRIDE: "-a ~CONFIG_USB_DWC3" xtfpga: TEST_PY_BD: "xtfpga" TEST_PY_ID: "--id qemu"diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2164ad79a72..714284a56df 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -508,6 +508,7 @@ xilinx_versal_virt test.py: TEST_PY_BD: "xilinx_versal_virt" TEST_PY_TEST_SPEC: "not sleep" TEST_PY_ID: "--id qemu"
OVERRIDE: "-a ~CONFIG_USB_DWC3" <<: *buildman_and_testpy_dfn
xtfpga test.py:
If it is affecting only qemu CI then it is fine. If this is related also to other HW lab then I think limitation should be done only on that particular qemu instance via u-boot-test-hook repo.
Thanks, Michal
Currently when booting dhcp_run() may be executed multiple times: once in eth_bootdev_hunt() and once in the network booting bootmeth.
We need to call eth_bootdev_hunt() when setting up the EFI sub-system to supply the simple network protocol. We don't need an IP address set up.
We can reduce the bootime by not executing dhcp_run() in eth_bootdev_hunt().
Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy network statck leads to downloading a file via TFTP and to booting the downloaded file.
Instead of running dchp_run() just check that there is a network device in eth_bootdev_hunt().
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: reword the commit message --- net/eth_bootdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c index 6ee54e3c790..b0fca6e8313 100644 --- a/net/eth_bootdev.c +++ b/net/eth_bootdev.c @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev) return 0; }
+/** + * eth_bootdev_hunt() - probe all network devices + * + * Network devices can also come from USB, but that is a higher + * priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been + * enumerated already. If something like 'bootflow scan dhcp' is used, + * then the user will need to run 'usb start' first. + * + * @info: info structure describing this hunter + * @show: true to show information from the hunter + * + * Return: 0 if device found, -EINVAL otherwise + */ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) { int ret; + struct udevice *dev = NULL;
if (!test_eth_enabled()) return 0; @@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) log_warning("Failed to init PCI (%dE)\n", ret); }
- /* - * Ethernet devices can also come from USB, but that is a higher - * priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should have been - * enumerated already. If something like 'bootflow scan dhcp' is used - * then the user will need to run 'usb start' first. - */ - if (IS_ENABLED(CONFIG_CMD_DHCP)) { - ret = dhcp_run(0, NULL, false); - if (ret) - return -EINVAL; - } + ret = -EINVAL; + uclass_foreach_dev_probe(UCLASS_ETH, dev) + ret = 0;
- return 0; + return ret; }
struct bootdev_ops eth_bootdev_ops = {
Hi Heinrich,
On Wed, 27 Nov 2024 at 00:07, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently when booting dhcp_run() may be executed multiple times: once in eth_bootdev_hunt() and once in the network booting bootmeth.
We need to call eth_bootdev_hunt() when setting up the EFI sub-system to supply the simple network protocol. We don't need an IP address set up.
We can reduce the bootime by not executing dhcp_run() in eth_bootdev_hunt().
Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy network statck leads to downloading a file via TFTP and to booting the
spelling
downloaded file.
I have now found the feature you're referring to - the calls to env_get_autostart(). Perhaps we can remove this feature? It seems pretty old and I don't see boards using it.
That feature stops bootstd working properly.
But if we don't remove it, we need to disable autostart in dhcp_run(), since in that case it does not behave correctly. I can do a patch for that if needed.
Instead of running dchp_run() just check that there is a network device in eth_bootdev_hunt().
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: reword the commit message
net/eth_bootdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c index 6ee54e3c790..b0fca6e8313 100644 --- a/net/eth_bootdev.c +++ b/net/eth_bootdev.c @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev) return 0; }
+/**
- eth_bootdev_hunt() - probe all network devices
- Network devices can also come from USB, but that is a higher
- priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been
- enumerated already. If something like 'bootflow scan dhcp' is used,
- then the user will need to run 'usb start' first.
- @info: info structure describing this hunter
- @show: true to show information from the hunter
- Return: 0 if device found, -EINVAL otherwise
- */
static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) { int ret;
struct udevice *dev = NULL; if (!test_eth_enabled()) return 0;@@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) log_warning("Failed to init PCI (%dE)\n", ret); }
/** Ethernet devices can also come from USB, but that is a higher* priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should have been* enumerated already. If something like 'bootflow scan dhcp' is used* then the user will need to run 'usb start' first.*/
Please keep this comment
if (IS_ENABLED(CONFIG_CMD_DHCP)) {ret = dhcp_run(0, NULL, false);if (ret)return -EINVAL;}
ret = -EINVAL;uclass_foreach_dev_probe(UCLASS_ETH, dev)ret = 0;
There is a uclass_probe_all() function.
My suggestion from the original series was to just do the dhcp once.
How does probing the ethernet devices actually help EFI? It is not needed for bootstd, since it probes any devices it uses.
return 0;
return ret;}
struct bootdev_ops eth_bootdev_ops = {
2.45.2
Regards, Simon
On 27.11.24 14:07, Simon Glass wrote:
Hi Heinrich,
On Wed, 27 Nov 2024 at 00:07, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently when booting dhcp_run() may be executed multiple times: once in eth_bootdev_hunt() and once in the network booting bootmeth.
We need to call eth_bootdev_hunt() when setting up the EFI sub-system to supply the simple network protocol. We don't need an IP address set up.
We can reduce the bootime by not executing dhcp_run() in eth_bootdev_hunt().
Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy network statck leads to downloading a file via TFTP and to booting the
spelling
downloaded file.
I have now found the feature you're referring to - the calls to env_get_autostart(). Perhaps we can remove this feature? It seems pretty old and I don't see boards using it.
That feature stops bootstd working properly.
But if we don't remove it, we need to disable autostart in dhcp_run(), since in that case it does not behave correctly. I can do a patch for that if needed.
Instead of running dchp_run() just check that there is a network device in eth_bootdev_hunt().
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: reword the commit message
net/eth_bootdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c index 6ee54e3c790..b0fca6e8313 100644 --- a/net/eth_bootdev.c +++ b/net/eth_bootdev.c @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev) return 0; }
+/**
- eth_bootdev_hunt() - probe all network devices
- Network devices can also come from USB, but that is a higher
- priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been
- enumerated already. If something like 'bootflow scan dhcp' is used,
- then the user will need to run 'usb start' first.
- @info: info structure describing this hunter
- @show: true to show information from the hunter
- Return: 0 if device found, -EINVAL otherwise
*/ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) { int ret;
struct udevice *dev = NULL; if (!test_eth_enabled()) return 0;@@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) log_warning("Failed to init PCI (%dE)\n", ret); }
/** Ethernet devices can also come from USB, but that is a higher* priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should have been* enumerated already. If something like 'bootflow scan dhcp' is used* then the user will need to run 'usb start' first.*/Please keep this comment
I have moved the comment to the function description. See above. I think that is the adequate place.
if (IS_ENABLED(CONFIG_CMD_DHCP)) {ret = dhcp_run(0, NULL, false);if (ret)return -EINVAL;}
ret = -EINVAL;uclass_foreach_dev_probe(UCLASS_ETH, dev)ret = 0;There is a uclass_probe_all() function.
My suggestion from the original series was to just do the dhcp once.
How does probing the ethernet devices actually help EFI? It is not needed for bootstd, since it probes any devices it uses.
bootstd is not the only way to start an EFI application, you could use bootefi or bootm.
Best regards
Heinrich
return 0;
return ret;}
struct bootdev_ops eth_bootdev_ops = {
-- 2.45.2
Regards, Simon
Hi Heinrich,
On Wed, 27 Nov 2024 at 07:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 27.11.24 14:07, Simon Glass wrote:
Hi Heinrich,
On Wed, 27 Nov 2024 at 00:07, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently when booting dhcp_run() may be executed multiple times: once in eth_bootdev_hunt() and once in the network booting bootmeth.
We need to call eth_bootdev_hunt() when setting up the EFI sub-system to supply the simple network protocol. We don't need an IP address set up.
We can reduce the bootime by not executing dhcp_run() in eth_bootdev_hunt().
Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy network statck leads to downloading a file via TFTP and to booting the
spelling
downloaded file.
I have now found the feature you're referring to - the calls to env_get_autostart(). Perhaps we can remove this feature? It seems pretty old and I don't see boards using it.
That feature stops bootstd working properly.
But if we don't remove it, we need to disable autostart in dhcp_run(), since in that case it does not behave correctly. I can do a patch for that if needed.
Instead of running dchp_run() just check that there is a network device in eth_bootdev_hunt().
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: reword the commit message
net/eth_bootdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c index 6ee54e3c790..b0fca6e8313 100644 --- a/net/eth_bootdev.c +++ b/net/eth_bootdev.c @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev) return 0; }
+/**
- eth_bootdev_hunt() - probe all network devices
- Network devices can also come from USB, but that is a higher
- priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been
- enumerated already. If something like 'bootflow scan dhcp' is used,
- then the user will need to run 'usb start' first.
- @info: info structure describing this hunter
- @show: true to show information from the hunter
- Return: 0 if device found, -EINVAL otherwise
*/ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) { int ret;
struct udevice *dev = NULL; if (!test_eth_enabled()) return 0;@@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) log_warning("Failed to init PCI (%dE)\n", ret); }
/** Ethernet devices can also come from USB, but that is a higher* priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should have been* enumerated already. If something like 'bootflow scan dhcp' is used* then the user will need to run 'usb start' first.*/Please keep this comment
I have moved the comment to the function description. See above. I think that is the adequate place.
if (IS_ENABLED(CONFIG_CMD_DHCP)) {ret = dhcp_run(0, NULL, false);if (ret)return -EINVAL;}
ret = -EINVAL;uclass_foreach_dev_probe(UCLASS_ETH, dev)ret = 0;There is a uclass_probe_all() function.
My suggestion from the original series was to just do the dhcp once.
How does probing the ethernet devices actually help EFI? It is not needed for bootstd, since it probes any devices it uses.
bootstd is not the only way to start an EFI application, you could use bootefi or bootm.
That's fine, but I'm just trying to understand what probing does, in this particular case. Doesn't EFI probe a device before it uses it? I think I am just missing some context.
Regards, Simon
On 27.11.24 15:14, Simon Glass wrote:
Hi Heinrich,
On Wed, 27 Nov 2024 at 07:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 27.11.24 14:07, Simon Glass wrote:
Hi Heinrich,
On Wed, 27 Nov 2024 at 00:07, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently when booting dhcp_run() may be executed multiple times: once in eth_bootdev_hunt() and once in the network booting bootmeth.
We need to call eth_bootdev_hunt() when setting up the EFI sub-system to supply the simple network protocol. We don't need an IP address set up.
We can reduce the bootime by not executing dhcp_run() in eth_bootdev_hunt().
Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy network statck leads to downloading a file via TFTP and to booting the
spelling
I will handle that in my pull request.
downloaded file.
I have now found the feature you're referring to - the calls to env_get_autostart(). Perhaps we can remove this feature? It seems pretty old and I don't see boards using it.
Commands controlled by autostart include bootelf, dhcp, tftboot.
For tftpboot autostart=no makes sense if you may want to download multiple files (kernel, initrd, dtb) before booting or the downloaded file is a firmware upgrade.
But for PXE booting you will need autostart=yes. Many routers allow recovery via TFTP.
For bootelf autostart=no leads to only checking the file in memory but not actually booting.
I don't think that we can simply remove the setting looking at the diverse use cases.
That feature stops bootstd working properly.
But if we don't remove it, we need to disable autostart in dhcp_run(), since in that case it does not behave correctly. I can do a patch for that if needed.
That change can be implemented separately from this patch series.
Instead of running dchp_run() just check that there is a network device in eth_bootdev_hunt().
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: reword the commit message
net/eth_bootdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c index 6ee54e3c790..b0fca6e8313 100644 --- a/net/eth_bootdev.c +++ b/net/eth_bootdev.c @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev) return 0; }
+/**
- eth_bootdev_hunt() - probe all network devices
- Network devices can also come from USB, but that is a higher
- priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been
- enumerated already. If something like 'bootflow scan dhcp' is used,
- then the user will need to run 'usb start' first.
- @info: info structure describing this hunter
- @show: true to show information from the hunter
- Return: 0 if device found, -EINVAL otherwise
*/ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) { int ret;
struct udevice *dev = NULL; if (!test_eth_enabled()) return 0;@@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) log_warning("Failed to init PCI (%dE)\n", ret); }
/** Ethernet devices can also come from USB, but that is a higher* priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should have been* enumerated already. If something like 'bootflow scan dhcp' is used* then the user will need to run 'usb start' first.*/Please keep this comment
I have moved the comment to the function description. See above. I think that is the adequate place.
if (IS_ENABLED(CONFIG_CMD_DHCP)) {ret = dhcp_run(0, NULL, false);if (ret)return -EINVAL;}
ret = -EINVAL;uclass_foreach_dev_probe(UCLASS_ETH, dev)ret = 0;There is a uclass_probe_all() function.
My suggestion from the original series was to just do the dhcp once.
How does probing the ethernet devices actually help EFI? It is not needed for bootstd, since it probes any devices it uses.
bootstd is not the only way to start an EFI application, you could use bootefi or bootm.
That's fine, but I'm just trying to understand what probing does, in this particular case. Doesn't EFI probe a device before it uses it? I think I am just missing some context.
EFI block devices will not even exist if not probed. See function efi_disk_probe().
For network devices I would like to change the code in a similar fashion in future to make all network devices available in the EFI sub-system.
Best regards
Heinrich
Hi Heinrich,
On Wed, 4 Dec 2024 at 02:10, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 27.11.24 15:14, Simon Glass wrote:
Hi Heinrich,
On Wed, 27 Nov 2024 at 07:00, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 27.11.24 14:07, Simon Glass wrote:
Hi Heinrich,
On Wed, 27 Nov 2024 at 00:07, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Currently when booting dhcp_run() may be executed multiple times: once in eth_bootdev_hunt() and once in the network booting bootmeth.
We need to call eth_bootdev_hunt() when setting up the EFI sub-system to supply the simple network protocol. We don't need an IP address set up.
We can reduce the bootime by not executing dhcp_run() in eth_bootdev_hunt().
Furthermore eth_bootdev_hunt() with autostart=yes leads on the legacy network statck leads to downloading a file via TFTP and to booting the
spelling
I will handle that in my pull request.
downloaded file.
I have now found the feature you're referring to - the calls to env_get_autostart(). Perhaps we can remove this feature? It seems pretty old and I don't see boards using it.
Commands controlled by autostart include bootelf, dhcp, tftboot.
For tftpboot autostart=no makes sense if you may want to download multiple files (kernel, initrd, dtb) before booting or the downloaded file is a firmware upgrade.
But for PXE booting you will need autostart=yes. Many routers allow recovery via TFTP.
For bootelf autostart=no leads to only checking the file in memory but not actually booting.
I don't think that we can simply remove the setting looking at the diverse use cases.
That feature stops bootstd working properly.
But if we don't remove it, we need to disable autostart in dhcp_run(), since in that case it does not behave correctly. I can do a patch for that if needed.
That change can be implemented separately from this patch series.
Instead of running dchp_run() just check that there is a network device in eth_bootdev_hunt().
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
v2: reword the commit message
net/eth_bootdev.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/eth_bootdev.c b/net/eth_bootdev.c index 6ee54e3c790..b0fca6e8313 100644 --- a/net/eth_bootdev.c +++ b/net/eth_bootdev.c @@ -64,9 +64,23 @@ static int eth_bootdev_bind(struct udevice *dev) return 0; }
+/**
- eth_bootdev_hunt() - probe all network devices
- Network devices can also come from USB, but that is a higher
- priority (BOOTDEVP_5_SCAN_SLOW) than network, so it should have been
- enumerated already. If something like 'bootflow scan dhcp' is used,
- then the user will need to run 'usb start' first.
- @info: info structure describing this hunter
- @show: true to show information from the hunter
- Return: 0 if device found, -EINVAL otherwise
*/ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) { int ret;
struct udevice *dev = NULL; if (!test_eth_enabled()) return 0;@@ -78,19 +92,11 @@ static int eth_bootdev_hunt(struct bootdev_hunter *info, bool show) log_warning("Failed to init PCI (%dE)\n", ret); }
/** Ethernet devices can also come from USB, but that is a higher* priority (BOOTDEVP_5_SCAN_SLOW) than ethernet, so it should have been* enumerated already. If something like 'bootflow scan dhcp' is used* then the user will need to run 'usb start' first.*/Please keep this comment
I have moved the comment to the function description. See above. I think that is the adequate place.
if (IS_ENABLED(CONFIG_CMD_DHCP)) {ret = dhcp_run(0, NULL, false);if (ret)return -EINVAL;}
ret = -EINVAL;uclass_foreach_dev_probe(UCLASS_ETH, dev)ret = 0;There is a uclass_probe_all() function.
My suggestion from the original series was to just do the dhcp once.
How does probing the ethernet devices actually help EFI? It is not needed for bootstd, since it probes any devices it uses.
bootstd is not the only way to start an EFI application, you could use bootefi or bootm.
That's fine, but I'm just trying to understand what probing does, in this particular case. Doesn't EFI probe a device before it uses it? I think I am just missing some context.
EFI block devices will not even exist if not probed. See function efi_disk_probe().
For network devices I would like to change the code in a similar fashion in future to make all network devices available in the EFI sub-system.
OK, thanks for the info. This patch seems OK since it fixes your problem,
Reviewed-by: Simon Glass sjg@chromium.org
Here's my understanding of what could be next: - land my PXE series (which unfortunately needs another spin) - update dhcp_run() so that it controls autostart, perhaps put autostart in struct bootm_info? - update bootstd so it only runs DHCP once on a device
Regards, Simon
The bootmenu command can display
* menu entries defined by environment variables * menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: no change --- cmd/bootmenu.c | 39 +++++++++++++++++++++++++++++--------- doc/usage/cmd/bootmenu.rst | 13 ++++++++++--- 2 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index ffa63a4628d..90f4f3d583c 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -330,7 +330,13 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, } #endif
-static struct bootmenu_data *bootmenu_create(int delay) +/** + * bootmenu_create() - create boot menu entries + * + * @uefi: consider UEFI boot options + * @delay: autostart delay in seconds + */ +static struct bootmenu_data *bootmenu_create(int uefi, int delay) { int ret; unsigned short int i = 0; @@ -357,7 +363,7 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup;
#if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) && (IS_ENABLED(CONFIG_CMD_EFICONFIG)) - if (i < MAX_COUNT - 1) { + if (uefi && i < MAX_COUNT - 1) { efi_status_t efi_ret;
/* @@ -481,7 +487,13 @@ static void handle_uefi_bootnext(void) run_command("bootefi bootmgr", 0); }
-static enum bootmenu_ret bootmenu_show(int delay) +/** + * bootmenu_show - display boot menu + * + * @uefi: generated entries for UEFI boot options + * @delay: autoboot delay in seconds + */ +static enum bootmenu_ret bootmenu_show(int uefi, int delay) { int cmd_ret; int init = 0; @@ -495,7 +507,7 @@ static enum bootmenu_ret bootmenu_show(int delay) efi_status_t efi_ret = EFI_SUCCESS; char *option, *sep;
- if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) + if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR) && uefi) handle_uefi_bootnext();
/* If delay is 0 do not create menu, just run first entry */ @@ -514,7 +526,7 @@ static enum bootmenu_ret bootmenu_show(int delay) return (cmd_ret == CMD_RET_SUCCESS ? BOOTMENU_RET_SUCCESS : BOOTMENU_RET_FAIL); }
- bootmenu = bootmenu_create(delay); + bootmenu = bootmenu_create(uefi, delay); if (!bootmenu) return BOOTMENU_RET_FAIL;
@@ -609,7 +621,7 @@ int menu_show(int bootdelay) int ret;
while (1) { - ret = bootmenu_show(bootdelay); + ret = bootmenu_show(1, bootdelay); bootdelay = -1; if (ret == BOOTMENU_RET_UPDATED) continue; @@ -635,11 +647,19 @@ int do_bootmenu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { char *delay_str = NULL; int delay = 10; + int uefi = 0;
#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) delay = CONFIG_BOOTDELAY; #endif
+ if (argc >= 2) { + if (!strcmp("-e", argv[1])) { + uefi = 1; + --argc; + ++argv; + } + } if (argc >= 2) delay_str = argv[1];
@@ -649,13 +669,14 @@ int do_bootmenu(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (delay_str) delay = (int)simple_strtol(delay_str, NULL, 10);
- bootmenu_show(delay); + bootmenu_show(uefi, delay); return 0; }
U_BOOT_CMD( bootmenu, 2, 1, do_bootmenu, "ANSI terminal bootmenu", - "[delay]\n" - " - show ANSI terminal bootmenu with autoboot delay" + "[-e] [delay]\n" + "-e - show UEFI entries\n" + "delay - show ANSI terminal bootmenu with autoboot delay" ); diff --git a/doc/usage/cmd/bootmenu.rst b/doc/usage/cmd/bootmenu.rst index 294cc02b17a..cd5597bc646 100644 --- a/doc/usage/cmd/bootmenu.rst +++ b/doc/usage/cmd/bootmenu.rst @@ -11,7 +11,7 @@ Synopsis -------- ::
- bootmenu [delay] + bootmenu [-e] [delay]
Description ----------- @@ -28,6 +28,14 @@ The "bootmenu" command interprets ANSI escape sequences, so an ANSI terminal is required for proper menu rendering and item selection.
+-e + show menu entries based on UEFI boot options + +delay + is the autoboot delay in seconds, after which the first + menu entry will be selected automatically + + The assembling of the menu is done via a set of environment variables "bootmenu_<num>" and "bootmenu_delay", i.e.::
@@ -35,8 +43,7 @@ The assembling of the menu is done via a set of environment variables bootmenu_<num>="<title>=<commands>"
<delay> - is the autoboot delay in seconds, after which the first - menu entry will be selected automatically + autostart delay in seconds
<num> is the boot menu entry number, starting from zero
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
The bootmenu command can display
- menu entries defined by environment variables
- menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Very strangely, this leads to the EFI selftest "Executing 'random number generator'" test failing on Pi 4, where was was passing previously (and Pi 4 has an RNG, unlike Pi 3), and this is on rpi_4_defconfig with: CONFIG_CMD_BOOTEFI_HELLO=y CONFIG_CMD_BOOTEFI_SELFTEST=y CONFIG_IPV6=y CONFIG_IPV6_ROUTER_DISCOVERY=y CONFIG_CMD_TFTPPUT=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_CMD_BOOTMENU=y CONFIG_CMD_LOG=y CONFIG_UNIT_TEST=y # CONFIG_CMD_EFIDEBUG is not set CONFIG_BOOTSTAGE_STASH_ADDR=0x02400000 CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_STASH=y CONFIG_CMD_BOOTSTAGE=y
added.
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
The bootmenu command can display
- menu entries defined by environment variables
- menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Very strangely, this leads to the EFI selftest "Executing 'random number generator'" test failing on Pi 4, where was was passing previously (and Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Best regards
Heinrich
on rpi_4_defconfig with:
CONFIG_CMD_BOOTEFI_HELLO=y CONFIG_CMD_BOOTEFI_SELFTEST=y CONFIG_IPV6=y CONFIG_IPV6_ROUTER_DISCOVERY=y CONFIG_CMD_TFTPPUT=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_CMD_BOOTMENU=y CONFIG_CMD_LOG=y CONFIG_UNIT_TEST=y # CONFIG_CMD_EFIDEBUG is not set CONFIG_BOOTSTAGE_STASH_ADDR=0x02400000 CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_STASH=y CONFIG_CMD_BOOTSTAGE=y
added.
-- Tom
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote:
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
The bootmenu command can display
- menu entries defined by environment variables
- menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Very strangely, this leads to the EFI selftest "Executing 'random number generator'" test failing on Pi 4, where was was passing previously (and Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
Hi,
On Sun, 12 Jan 2025 at 18:51, Tom Rini trini@konsulko.com wrote:
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote:
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
The bootmenu command can display
- menu entries defined by environment variables
- menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Very strangely, this leads to the EFI selftest "Executing 'random number generator'" test failing on Pi 4, where was was passing previously (and Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
This might help make the log more intelligible.
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-s...
Regards, Simon
Simon Glass sjg@chromium.org schrieb am Mo., 13. Jan. 2025, 20:02:
Hi,
On Sun, 12 Jan 2025 at 18:51, Tom Rini trini@konsulko.com wrote:
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote:
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
The bootmenu command can display
- menu entries defined by environment variables
- menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot
options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt <
heinrich.schuchardt@canonical.com>
Very strangely, this leads to the EFI selftest "Executing 'random
number
generator'" test failing on Pi 4, where was was passing previously
(and
Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
This might help make the log more intelligible.
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-s...
No clue which problem you have to read the log.
Best regards
Heinrich
Regards, Simon
On Mon, Jan 13, 2025 at 12:01:45PM -0700, Simon Glass wrote:
Hi,
On Sun, 12 Jan 2025 at 18:51, Tom Rini trini@konsulko.com wrote:
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote:
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
The bootmenu command can display
- menu entries defined by environment variables
- menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Very strangely, this leads to the EFI selftest "Executing 'random number generator'" test failing on Pi 4, where was was passing previously (and Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
This might help make the log more intelligible.
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-s...
Shrug, it's the age old problem of sending raw stdout to a file. It looks fine on console as it happens, I was just grabbing from an existing log.
Hi Tom,
On Mon, 13 Jan 2025 at 12:24, Tom Rini trini@konsulko.com wrote:
On Mon, Jan 13, 2025 at 12:01:45PM -0700, Simon Glass wrote:
Hi,
On Sun, 12 Jan 2025 at 18:51, Tom Rini trini@konsulko.com wrote:
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote:
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
The bootmenu command can display
- menu entries defined by environment variables
- menu entries defined by UEFI boot options
Not in all cases showing the UEFI boot options is desired. Provide a new parameter '-e' to select the display of UEFI boot options.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Very strangely, this leads to the EFI selftest "Executing 'random number generator'" test failing on Pi 4, where was was passing previously (and Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
This might help make the log more intelligible.
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-s...
Shrug, it's the age old problem of sending raw stdout to a file. It looks fine on console as it happens, I was just grabbing from an existing log.
Oh well. I would probably suggest only writing this junk to log files if we actually need it in there (i.e. something is programmatically checking), but it seems like this is a different situation from the one my patch fixes.
Regards, Simon
On Mon, Jan 13, 2025 at 01:09:16PM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 13 Jan 2025 at 12:24, Tom Rini trini@konsulko.com wrote:
On Mon, Jan 13, 2025 at 12:01:45PM -0700, Simon Glass wrote:
Hi,
On Sun, 12 Jan 2025 at 18:51, Tom Rini trini@konsulko.com wrote:
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote:
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote:
> The bootmenu command can display > > * menu entries defined by environment variables > * menu entries defined by UEFI boot options > > Not in all cases showing the UEFI boot options is desired. > Provide a new parameter '-e' to select the display of UEFI boot options. > > Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org > Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
Very strangely, this leads to the EFI selftest "Executing 'random number generator'" test failing on Pi 4, where was was passing previously (and Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
This might help make the log more intelligible.
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-s...
Shrug, it's the age old problem of sending raw stdout to a file. It looks fine on console as it happens, I was just grabbing from an existing log.
Oh well. I would probably suggest only writing this junk to log files if we actually need it in there (i.e. something is programmatically checking), but it seems like this is a different situation from the one my patch fixes.
I mean, yes? The problem here is that the random number generator test now fails, when it didn't before. As far as I know (and I think the rest of us understood?) your problem was that the ANSI sequences exist.
Hi Tom,
On Mon, 13 Jan 2025 at 13:35, Tom Rini trini@konsulko.com wrote:
On Mon, Jan 13, 2025 at 01:09:16PM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 13 Jan 2025 at 12:24, Tom Rini trini@konsulko.com wrote:
On Mon, Jan 13, 2025 at 12:01:45PM -0700, Simon Glass wrote:
Hi,
On Sun, 12 Jan 2025 at 18:51, Tom Rini trini@konsulko.com wrote:
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote:
Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46:
> On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote: > > > The bootmenu command can display > > > > * menu entries defined by environment variables > > * menu entries defined by UEFI boot options > > > > Not in all cases showing the UEFI boot options is desired. > > Provide a new parameter '-e' to select the display of UEFI boot options. > > > > Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org > > Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com > > Very strangely, this leads to the EFI selftest "Executing 'random number > generator'" test failing on Pi 4, where was was passing previously (and > Pi 4 has an RNG, unlike Pi 3), and this is
Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
This might help make the log more intelligible.
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-s...
Shrug, it's the age old problem of sending raw stdout to a file. It looks fine on console as it happens, I was just grabbing from an existing log.
Oh well. I would probably suggest only writing this junk to log files if we actually need it in there (i.e. something is programmatically checking), but it seems like this is a different situation from the one my patch fixes.
I mean, yes? The problem here is that the random number generator test now fails, when it didn't before. As far as I know (and I think the rest of us understood?) your problem was that the ANSI sequences exist.
Yes, I want them to be controlled, so we can have them when needed (e.g. for user benefit, getting the terminal size) but not when not needed (CI output, log files, testing that ANSI works, etc.)
Regards, Simon
On Mon, Jan 13, 2025 at 05:15:08PM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 13 Jan 2025 at 13:35, Tom Rini trini@konsulko.com wrote:
On Mon, Jan 13, 2025 at 01:09:16PM -0700, Simon Glass wrote:
Hi Tom,
On Mon, 13 Jan 2025 at 12:24, Tom Rini trini@konsulko.com wrote:
On Mon, Jan 13, 2025 at 12:01:45PM -0700, Simon Glass wrote:
Hi,
On Sun, 12 Jan 2025 at 18:51, Tom Rini trini@konsulko.com wrote:
On Sun, Jan 12, 2025 at 08:08:13PM +0100, Heinrich Schuchardt wrote: > Tom Rini trini@konsulko.com schrieb am Fr., 10. Jan. 2025, 23:46: > > > On Wed, Nov 27, 2024 at 08:06:30AM +0100, Heinrich Schuchardt wrote: > > > > > The bootmenu command can display > > > > > > * menu entries defined by environment variables > > > * menu entries defined by UEFI boot options > > > > > > Not in all cases showing the UEFI boot options is desired. > > > Provide a new parameter '-e' to select the display of UEFI boot options. > > > > > > Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org > > > Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com > > > > Very strangely, this leads to the EFI selftest "Executing 'random number > > generator'" test failing on Pi 4, where was was passing previously (and > > Pi 4 has an RNG, unlike Pi 3), and this is > > Do you have a link to the log?
Here's the failing part: Setting up 'random number generator' ESC[0;37;40mESC[1;32;40mSetting up 'random number generator' succeeded ESC[0;37;40mESC[1;34;40m Executing 'random number generator' ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest_rng.c(52): ERROR: ESC[0;37;40mESC[1;31;40mRandom number generator protocol not available ESC[0;37;40mESC[1;31;40mlib/efi_selftest/efi_selftest.c(114): ERROR: ESC[0;37;40mESC[1;31;40mExecuting 'random number generator' failed ESC[0;37;40mESC[1;34;40m
This might help make the log more intelligible.
https://patchwork.ozlabs.org/project/uboot/patch/20240811145209.4191404-37-s...
Shrug, it's the age old problem of sending raw stdout to a file. It looks fine on console as it happens, I was just grabbing from an existing log.
Oh well. I would probably suggest only writing this junk to log files if we actually need it in there (i.e. something is programmatically checking), but it seems like this is a different situation from the one my patch fixes.
I mean, yes? The problem here is that the random number generator test now fails, when it didn't before. As far as I know (and I think the rest of us understood?) your problem was that the ANSI sequences exist.
Yes, I want them to be controlled, so we can have them when needed (e.g. for user benefit, getting the terminal size) but not when not needed (CI output, log files, testing that ANSI works, etc.)
Yes, and Heinrich said for the test he wrote he finds having color sequences to them (and not just the color sequence tests) helpful. That is literally the only place we have them when not strictly needed AFAIK. So using one of the countless "strip ansi color sequences from my log" tool seems the best option here, when bothered by them.
Some hard devices need specific routines to scan for block devices, e.g. NVMe (nvme scan), SCSI (scsi start).
Invoke bootdev_hunt() to find all block devices.
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: no change --- cmd/bootmenu.c | 39 +++++++++++++++++++++++++++++--------- doc/usage/cmd/bootmenu.rst | 13 ++++++++++--- 2 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c index ffa63a4628d..90f4f3d583c 100644 --- a/cmd/bootmenu.c +++ b/cmd/bootmenu.c @@ -330,7 +330,13 @@ static int prepare_uefi_bootorder_entry(struct bootmenu_data *menu, } #endif
-static struct bootmenu_data *bootmenu_create(int delay) +/** + * bootmenu_create() - create boot menu entries + * + * @uefi: consider UEFI boot options + * @delay: autostart delay in seconds + */ +static struct bootmenu_data *bootmenu_create(int uefi, int delay) { int ret; unsigned short int i = 0; @@ -357,7 +363,7 @@ static struct bootmenu_data *bootmenu_create(int delay) goto cleanup;
#if (IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR)) && (IS_ENABLED(CONFIG_CMD_EFICONFIG)) - if (i < MAX_COUNT - 1) { + if (uefi && i < MAX_COUNT - 1) { efi_status_t efi_ret; lib/efi_loader/efi_setup.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index aa59bc7779d..8e0ff16f3eb 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -7,6 +7,7 @@
#define LOG_CATEGORY LOGC_EFI
+#include <bootdev.h> #include <efi_loader.h> #include <efi_variable.h> #include <log.h> @@ -228,6 +229,13 @@ efi_status_t efi_init_obj_list(void) * Probe block devices to find the ESP. * efi_disks_register() must be called before efi_init_variables(). */ + if (CONFIG_IS_ENABLED(BOOTSTD)) { + int r; + + r = bootdev_hunt(NULL, 0); + if (r) + log_debug("No boot device available\n"); + } ret = efi_disks_register(); if (ret != EFI_SUCCESS) goto out;
participants (5)
-
Heinrich Schuchardt -
Ilias Apalodimas -
Michal Simek -
Simon Glass -
Tom Rini