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