On 10/22/23 23:55, Simon Glass wrote:
Hi,
On Sun, 22 Oct 2023 at 07:59, Tom Rini <tr...@konsulko.com> wrote:
On Sun, Oct 22, 2023 at 10:29:22AM -0400, Tom Rini wrote:
On Sun, Oct 22, 2023 at 08:08:11AM +0200, Heinrich Schuchardt wrote:
On 10/21/23 20:26, Tom Rini wrote:
On Sat, Oct 21, 2023 at 08:43:08AM -0700, Simon Glass wrote:
Hi,
On Thu, 19 Oct 2023 at 17:30, AKASHI Takahiro
<takahiro.aka...@linaro.org> wrote:
On Thu, Oct 19, 2023 at 08:01:11AM -0600, Simon Glass wrote:
Hi Heinrich,
On Wed, 18 Oct 2023 at 06:55, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
On 10/17/23 16:09, Tom Rini 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 <s...@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
Does this work for you Heinrich, or do you want to clarify the
dependencies (and re-organize the code as needed) around networking?
We should be able to boot via EFI on devices without U-Boot network support.
We already use IS_ENABLED(CONFIG_NETDEVICES) to avoid invoking
eth_get_dev() if there is no network. CONFIG_NETDEVICES=y selects
CONFIG_DM_ETH.
Why is this not sufficient?
Is there a configuration that does not build?
The point of this series is to disable CMDLINE and fix up what breaks.
In this case we have some sort of breakage...perhaps Tom has already
found it, but otherwise could you take a look?
We should be able to disable NET and LTO in sandbox and still build.
But this fails at present[1]. You can try it on -master
Obviously, it would be necessary to enclose efi_dp_from_eth()
with "if defined(CONFIG_NETDEVICES)" (or DM_ETH).
Then, we could drop "depends on DM_ETH".
Strange that it only happens on the non-LTO board, though?
There's two issues. The first of which is that I think you need to
re-check your error exactly? With my series, and LTO also disabled the
problem is a call to efi_get_image_parameters() as that's defined in
cmd/bootefi.c, but also only used with cmdline invocations. So we can
fix that CMDLINE=n && LTO=n case with a IS_ENABLED(CONFIG_CMDLINE)
around that, and then discard efi_dp_from_name() entirely.
The second issue is that with LTO we more completely find the cases
where if x() calls y() and y() is undefined but nothing calls x() we can
just discard x() and not care that y() is undefined.
I will send a patch for function efi_dp_from_eth().
There's no problem with efi_dp_from_eth as far as I can tell.
@Simon
One thing that I don't understand is why we don't let the linker
eliminate the unused functions on the sandbox.
On other architectures we put each function into a separate text section
and let the linker eliminate the unused text sections:
arch/riscv/config.mk:29:
PLATFORM_RELFLAGS += -fno-common -ffunction-sections -fdata-sections
LDFLAGS_u-boot += --gc-sections -static -pie
Shouldn't we keep the sandbox close to what other architectures do?
Oh my, I didn't realize that sandbox was missing the garbage collection
stuff. Yes, that needs to be fixed first, then we can see what's next
to change, as there are some issues (my series first fixed CMDLINE=n on
qemu_arm64).
My super quick pass at enabling
function-sections/data-sections/gc-sections shows there's nothing
further needed for CMDLINE=n and LTO=n on sandbox, not even the part I
was looking at before.
I am not sure about the original reason, but sandbox probably
pre-dates wholesale enabling of gc-sections (I haven't looked). There
is also the issue of whether the host toolchain supports it. Sandbox
is supposed to run on a variety of OS types.
I am also not sure of the point ot it...since it is normally pretty
easy to use Kconfig to control features.
At least for me 'u-boot -D' doesn't work with Heinrich's patch. Do we
really need to make this change? It will rapidly devolve to the point
that coming back would be extremely painful.
It just shows where the code is not clean.
Linker generated lists referenced by the corresponding macros are
compiled in (as shows up in u-boot.map). But you have decided not to use
those macros to access the sandbox command line options. Now they are
removed as unreferenced garbage.
Can't we use the linker generated list macros instead of duplicating code?
* define SANDBOX_CMDLINE_OPT via ll_entry_declare().
* replace ll_entry_count() instead of __u_boot_sandbox_option_count()
* use ll_start(), ll_end() for iterating
Best regards
Heinrich
