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

[Simon Glass]

Hi Heinrich,

On Fri, 10 Jan 2025 at 08:13, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 10.01.25 14:40, Simon Glass 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.
>
> What structures are you referring to concerning this patch?

I gave the example of efi_cout_modes[]. I suspect quite a lot of
structures could be relationalised.

>
> >
> > 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.
>
> We have to present the U-Boot DM devices in a way that an EFI binary can
> consume.
>
> Unless U-Boot's DM-layer is re-implemented via EFI structures we will
> end up with two representations.

I believe we can reduce the amount of duplication, in general. I'm not
just thinking about driver model. The EFI loader was originally
designed to avoid dependence on driver model (I can provide the thread
if you like), so even there, I believe improvement is possible.

>
> >
> > 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?
>
> No, we can load both EFI apps and EFI drivers with the bootefi command.
> In the case of EFI drivers we must not remove the handles and protocols
> representing U-Boot DM devices after the binaries return.

My question is whether these can be generated based on the available
objects, with just a small amount of data kept in the dm struct, or
whether they must be in a separate data structure?

Regards,
SImon