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

[Simon Glass]

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

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.

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