Re: [PATCH v3 3/4] net: tftp: remove explicit efi configuration dependency

[Heinrich Schuchardt]

Am 20. Dezember 2023 05:46:16 MEZ schrieb Simon Glass <sjg at chromium.org>:
>Hi,
>
>On Mon, 18 Dec 2023 at 17:17, AKASHI Takahiro
><takahiro.akashi at linaro.org> wrote:
>>
>> Hi Simon,
>>
>> On Mon, Dec 18, 2023 at 08:01:46AM -0700, Simon Glass wrote:
>> > Hi AKASHI,
>> >
>> > On Sun, 17 Dec 2023 at 19:39, AKASHI Takahiro
>> > <takahiro.akashi at linaro.org> wrote:
>> > >
>> > > Now it is clear that the feature actually depends on efi interfaces,
>> > > not "bootefi" command. efi_set_bootdev() will automatically be nullified
>> > > if necessary efi component is disabled.
>> > >
>> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>> > > ---
>> > >  net/tftp.c | 10 ++++------
>> > >  1 file changed, 4 insertions(+), 6 deletions(-)
>> > >
>> >
>> > I have the same comment here as the 'fs' patch.
>> >
>> > > diff --git a/net/tftp.c b/net/tftp.c
>> > > index 88e71e67de35..2e335413492b 100644
>> > > --- a/net/tftp.c
>> > > +++ b/net/tftp.c
>> > > @@ -302,12 +302,10 @@ static void tftp_complete(void)
>> > >                         time_start * 1000, "/s");
>> > >         }
>> > >         puts("\ndone\n");
>> > > -       if (IS_ENABLED(CONFIG_CMD_BOOTEFI)) {
>> >
>> > Shouldn't this depend on your new CONFIG? What happens if EFI_LOADER
>> > is not enabled?
>>
>> The trick is in efi_loader.h.
>> If EFI_LOADER (more specifically CONFIG_EFI_BINARY_EXEC) is not defined,
>> this function gets voided.  See patch#1 in this version.
>>
>> I took this approach in order not to make users much worried about
>> what config be used as they are not familiar with UEFI implementation.
>
>OK, but we really need to delete this function, so what is the plan
>for that? The info that EFI needs should be passed to the bootefi()
>function, not set in a global.

Hello Simon,

Bootstd is not the only way to boot. Please, do not forget the shell.

The user loads a file with tftpboot. At some later moment the user executes bootefi.

We need a place where we store the device from which the image was loaded.

In future we might have a register of loaded files. But that is beyond the scope of this patch series.

Best regards

Heinrich

>
>
>>
>> -Takahiro Akashi
>>
>> > > -               if (!tftp_put_active)
>> > > -                       efi_set_bootdev("Net", "", tftp_filename,
>> > > -                                       map_sysmem(tftp_load_addr, 0),
>> > > -                                       net_boot_file_size);
>> > > -       }
>> > > +       if (!tftp_put_active)
>> > > +               efi_set_bootdev("Net", "", tftp_filename,
>> > > +                               map_sysmem(tftp_load_addr, 0),
>> > > +                               net_boot_file_size);
>> > >         net_set_state(NETLOOP_SUCCESS);
>> > >  }
>> > >
>> > > --
>> > > 2.34.1
>
>Regards,
>Simon