Hi Simon, On Sat, Nov 18, 2023 at 5:11 PM Simon Glass <s...@chromium.org> wrote: > > +Ilias too as this involves a design decision > > Hi Shantur, > > On Fri, 17 Nov 2023 at 14:22, Shantur Rathore <i...@shantur.com> wrote: > > > > efi_set_bootdev is already called as part of tftp while doing dhcp_run() > > Is that in tftp_complete() ? > > > Doing this again crashes U-boot and we don't need to call again. > > U-Boot > > The problem is that we might scan for 4 bootflows, then later decide > which one to boot. So we cannot know which one it will be until that > point. > > The hack is needed so that it actually tells EFI about the right one. > > The addition of EFI hacks when reading files (i.e. the code in > tftp_complete()) is a real problem, unfortunately. I understand that > it was a convenient hack, but with standard boot we really don't want > it to do that. We end up calling it multiple times for bootflows that > won't be used. > > So I believe the fix is to be able to disable those calls, leaving it > to bootstd. > > I'm not sure of the best way to do that. Perhaps we should have a > function like bootstd_scanning() which returns a bool indicating that > bootstd is scanning? If that is true then no efi_set_bootdev() calls > are made? We could have a flag in 'struct bootstd_priv' which is set > before bootflow_check() is called and cleared afterwards. > > As to how this should be done with standard boot, we should create a > function like efi_start_app() and pass it the information we need. > That function should do the equivalent of the 'bootefi' command, > except programmatically, with no prior state hanging around. > > As to when we might be able to do that, we need more refactoring of > the bootm code. There is a start on this [1] but it likely needs 2-3 > more series before it might be possible. > > So for now, I think bootstd_scanning() is the best approach. If you > are not sure about that, I can send a patch. >
My apologies, the fix I submitted is incorrect. After reading your explanation, I started thinking on why dhcp can call this multiple times but bootmethod can't. There it was clear that the issue bflow->fname is null when we call this function. The correct fix should be below + /* bootfile should be setup by dhcp by now*/ + bootfile_name = env_get("bootfile"); + if (!bootfile_name) + return log_msg_ret("bootfile_name", ret); + bflow->fname = strdup(bootfile_name); + /* do the hideous EFI hack */ efi_set_bootdev("Net", "", bflow->fname, map_sysmem(addr, 0), bflow->size); I have a patch ready for this which I will send but I have some queries regarding that. As I sent this a series of patches out of which 2 were reviewed, should I send a new version for the whole series even though 2 are not going to be changed? Another thing, should I send them as a reply to the original email or a new thread? Kind regards, Shantur