[PATCH v3 22/32] efi: Update EFI_LOADER to depend on DM_ETH

[Simon Glass]

Hi Peter,

On Thu, 19 Oct 2023 at 04:40, Peter Robinson <pbrobinson at gmail.com> wrote:
>
> On Wed, Oct 18, 2023 at 4:29 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Tom,
> >
> > On Wed, 18 Oct 2023 at 07:37, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Mon, Oct 16, 2023 at 04:28:13PM -0600, Simon Glass wrote:
> > >
> > > > Since efi_device_path.c calls eth_get_dev() and assumes that Ethernet is
> > > > available, add it as an explicit dependency.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > ---
> > > >
> > > > (no changes since v2)
> > > >
> > > > Changes in v2:
> > > > - Add new patch to update EFI_LOADER to depend on DM_ETH
> > > >
> > > >  lib/efi_loader/Kconfig | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > index 13cad6342c36..fca4b3eef270 100644
> > > > --- a/lib/efi_loader/Kconfig
> > > > +++ b/lib/efi_loader/Kconfig
> > > > @@ -11,6 +11,7 @@ config EFI_LOADER
> > > >       # We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
> > > >       depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
> > > >       depends on BLK
> > > > +     depends on DM_ETH
> > > >       depends on !EFI_APP
> > > >       default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> > > >       select CHARSET
> > >
> > > I don't think this is needed.  After reconfiguring qemu_arm64 to be able
> > > to disable networking entirely, we still are able to build with
> > > EFI_LOADER enabled, and no warning / link errors.
> >
> > Fair enough.
> >
> > As of this patch the following errors are generated with -a ~CMDLINE :
> >
> > +lib/efi_loader/efi_device_path.c:985:(.text+0xca4): undefined
> > reference to `eth_get_dev'
> > +/bin/ld: lib/efi_loader/efi_device_path.c:987:(.text+0xca9):
> > undefined reference to `eth_get_dev'
> > +/bin/ld: lib/efi_loader/efi_device_path.c:993:(.text+0xcc9):
> > undefined reference to `eth_get_dev'
> > +/bin/ld: lib/efi_loader/efi_device_path.o: in function `efi_dp_from_name':
> > +lib/efi_loader/efi_device_path.c:1095:(.text+0xe00): undefined
> > reference to `efi_get_image_parameters'
> >
> > which is why I added this patch. A later patch disables networking
> > entirely with ~CMDLINE so that is why this problem goes away.
> >
> > This series was basically built up by disabling CMDLINE and then
> > fixing the errors one by one. I didn't go back and check whether (at
> > the end) all the patches were needed.
>
> Was this just tested on sandbox? How do we know it's not going to
> regress a vast amount of actual devices?

This was tested with buildman to check for config changes. The purpose
of this series.is to get back to what worked a few years ago (CMDLINE
could be disabled). If you don't disable CMDLINE, then it doesn't
really change anything.

Perhaps we should add a build test for non-network EFI_LOADER if
people are going to use it? I don't see any boards that do:

$ ./tools/qconfig.py -f EFI_LOADER ~NET
0 matches

That is a great tool for checking what is used in the code base.

Regards,
Simon