[PATCH 1/2] efi_loader: efi_setup: Add efi_start_obj_list() to efi_setup.c

[Simon Glass]

Hi Ilias,

On Fri, 10 Jan 2025 at 07:15, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> On Fri, 10 Jan 2025 at 15:41, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Heinrich,
> >
> > On Fri, 3 Jan 2025 at 12:55, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >
> > > On 06.12.24 18:30, Adriano Cordova wrote:
> > > > The command bootefi calls efi_init_obj_list to do the efi set up
> > > > before launching an .efi payload, but efi_init_obj_list is called
> > > > only once. There are some initializations which depend on the
> > > > environment and should be done each time a payload gets launched and
> > > > not only once and can now be done in efi_start_obj_list(). A motivation
> > > > for this change is the following order of events:
> > > >
> > > > 1. Launch an EFI application (e.g. bootefi hello)
> > > > 2. Change the ip address
> > > > 3. Launch another application which uses the pxe protocol
> > >
> > > Hello Adriano,
> > >
> > > Thanks for addressing this issue.
> > >
> > > There are a number of events that can lead to updating the IP address:
> > >
> > > * EFI_PXE_BASE_CODE_PROTOCOL.Dhcp()
> > > * EFI_PXE_BASE_CODE_PROTOCOL.SetStationIp()
> > > * EFI_HTTP_PROTOCOL.Configure()
> > > * command set env ethaddr#
> > > * command dhcp
> > >
> > > All of these events require:
> > >
> > > * Updating the relevant ethaddr* variable.
> > > * Updating any other references to the IP address.
> > >
> > > When not only considering EFI applications but also EFI drivers
> > > installed via the bootefi command all of these events can occur during
> > > the runtime of EFI binaries.
> > >
> > > Function efi_init_obj_list() is used for setups that are needed only
> > > once before accessing the UEFI sub-system. It is not able to handle events.
> > >
> > > If we need to update any internal UEFI data structures, we should do
> > > this instantaneously when the event occurs and not when executing a UEFI
> > > related shell command.
> > >
> > > U-Boot already has callbacks invoked when variables are changed. These
> > > are set up with U_BOOT_ENV_CALLBACK(). There is even a command 'env
> > > callbacks' to list these callbacks.
> > >
> > > You could define a new function to handle changes of the IP address and
> > > invoke it via U_BOOT_ENV_CALLBACK(ipaddr, <function name>) or
> > > U_BOOT_ENV_CALLBACK(netmask, <function name>).
> > >
> > > With lwIP we can have multiple network interfaces. Each has a different
> > > variable ipaddr# assigned to it. We will have to extend the concept of
> > > U_BOOT_ENV_CALLBACK() to allow for wildcard matches by adjusting
> > > find_env_callback(). With CONFIG_REGEX=y regular expressions could be
> > > used for this purpose (cf. include/slre.h) but we should better avoid
> > > the code size increase.
> > >
> >
> > I have been saying for some time that EFI_LOADER should make use of
> > U-Boot's existing tables, rather than creating its own duplicate ones
> > with extra info.
> >
> > This seems to have been understood as 'hang the EFI tables onto
> > existing data structures', e.g. efi_disk_create_part(). But this leads
> > to duplication.
> >
> > Perhaps the EFI_LOADER init should be quite small, and be done each
> > time an app starts, to deal with the 'current' state of U-Boot?
>
> That would break all the command line tools that use the EFI subsystem
> -- e.g printenv -e, efidebug, eficonfig etc

Obviously we would not do it in such a way as to break anything! I'm
not quite sure if that is was a serious comment, or just a reminder
that we should not break things?

As a very simple example, somewhat unrelated to this patch, rather
than maintaining its own efi_cout_modes[], it could make use of
U-Boot's information.

There is quite a bit of effort involved in maintaining two separate
states. It started from a design to run independently of driver model
(which was a mistake), but has since become common in many areas. It
means that a special, heavyweight 'init' has to be done and we need to
pick when that is done and how to update it later. We should chart a
slightly different course and chip away at this problem.

Regards,
Simon