[PATCH v2 2/4] bootflow: bootmeth_efi: Don't set bootdev again

[Ilias Apalodimas]

Hi Simon,

Thanks for looping me in

On Sat, 18 Nov 2023 at 19:11, Simon Glass <sjg at chromium.org> wrote:
>
> +Ilias too as this involves a design decision
>
> Hi Shantur,
>
> On Fri, 17 Nov 2023 at 14:22, Shantur Rathore <i at 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.

Yes, we should fine the root cause here instead of papering over the problem

>
> 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.

I'll have a closer look at the efi bootmeth as well and come back with
suggestions

Thanks
/Ilias
>
> >
> > Signed-off-by: Shantur Rathore <i at shantur.com>
> > ---
> >  boot/bootmeth_efi.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> > index 682cf5b23b..5917458dc5 100644
> > --- a/boot/bootmeth_efi.c
> > +++ b/boot/bootmeth_efi.c
> > @@ -360,9 +360,6 @@ static int distro_efi_read_bootflow_net(struct bootflow *bflow)
> >                 return log_msg_ret("sz", -EINVAL);
> >         bflow->size = size;
> >
> > -       /* do the hideous EFI hack */
> > -       efi_set_bootdev("Net", "", bflow->fname, map_sysmem(addr, 0),
> > -                       bflow->size);
> >
> >         /* read the DT file also */
> >         fdt_addr_str = env_get("fdt_addr_r");
> > --
> > 2.40.1
> >
>
> Regards,
> Simon
>
> [1] https://patchwork.ozlabs.org/project/uboot/list/?series=382423