Hi
On Fri, Oct 28, 2022 at 6:40 PM Pali Rohár pali@kernel.org wrote:
Hello! This is really much better solution! Few comments are below.
On Friday 28 October 2022 18:18:49 Max Krummenacher wrote:
From: Max Krummenacher max.krummenacher@toradex.com
With LTO enabled the U-Boot initial environment is no longer stored in an easy accessible section in env/common.o. I.e. the section name changes from build to build, its content maybe compressed and it is annotated with additional data.
Drop trying to read the initial env with elf tools from the compiler specific object file in favour of adding and using a host tool with the only functionality of printing the initial env to stdout.
See also: https://lore.kernel.org/all/927b122e-1f62-e790-f5ca-30bae4332c77@foss.st.com...
Signed-off-by: Max Krummenacher max.krummenacher@toradex.com
Changes in v2:
- reworked to build a host tool which prints the configured environment as proposed by Pali Rohár https://lore.kernel.org/u-boot/20221018174827.1393211-1-max.oss.09@gmail.com...
- renamed patch, v1 used "Makefile: fix u-boot-initial-env target if lto is enabled"
Makefile | 7 ++++--- scripts/.gitignore | 1 + scripts/Makefile | 5 +++++ scripts/printinitialenv.c | 44 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 scripts/printinitialenv.c
diff --git a/Makefile b/Makefile index 0f1174718f7..2f97d63c398 100644 --- a/Makefile +++ b/Makefile @@ -2442,9 +2442,10 @@ endif $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
quiet_cmd_genenv = GENENV $@ -cmd_genenv = $(OBJCOPY) --dump-section .rodata.default_environment=$@ env/common.o; \
sed --in-place -e 's/\x00/\x0A/g' $@; sed --in-place -e '/^\s*$$/d' $@; \sort --field-separator== -k1,1 --stable $@ -o $@+cmd_genenv = \
scripts/printinitialenv | \sed -e 's/\x00/\x0A/g' -e '/^\s*$$/d' | \I think that you do not need this sed anymore as you print newline in host tool.
Missed that one, will change in a V3.
sort --field-separator== -k1,1 --stable -o $@u-boot-initial-env: u-boot.bin
It is needed to update dependencies for u-boot-initial-env target. Now it does not depend on u-boot.bin but rather on printinitialenv tool.
I'm unsure if that is the best way forward. The initial solution would also not need to depend on u-boot.bin but rather on env/common.o.
I guess that the intention was that the U-Boot binary and the u-boot-initial-env file should not be out of sync.
Opinions?
$(call if_changed,genenv)diff --git a/scripts/.gitignore b/scripts/.gitignore index 08cc824bac3..6068724a0d4 100644 --- a/scripts/.gitignore +++ b/scripts/.gitignore @@ -2,3 +2,4 @@ # Generated files # bin2c +printinitialenv diff --git a/scripts/Makefile b/scripts/Makefile index 8731e6cecd7..ba993820571 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -5,8 +5,13 @@ # ---------------------------------------------------------------------------
hostprogs-$(CONFIG_BUILD_BIN2C) += bin2c +hostprogs-y += printinitialenv
I'm not sure if the scripts/ is the correct directory for the this tool. Maybe it should be in tools? Lets wait what maintainers or Tom say.
According to tools/Makefile tools should be used for tools which are not dependent on any boards. This here is a helper during the build of a particular U-Boot configuration. That is why I put it into scripts/.
Max
always := $(hostprogs-y)
+HOSTCFLAGS_printinitialenv.o += \
$(patsubst -I%,-idirafter%, $(filter -I%, $(UBOOTINCLUDE))) \-DUSE_HOSTCC# Let clean descend into subdirs subdir- += basic kconfig dtc diff --git a/scripts/printinitialenv.c b/scripts/printinitialenv.c new file mode 100644 index 00000000000..c58b234d679 --- /dev/null +++ b/scripts/printinitialenv.c @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- (C) Copyright 2022
- Max Krummenacher, Toradex
- Snippets taken from tools/env/fw_env.c
- This prints the list of default environment variables as currently
- configured.
- */
+#include <stdio.h>
+/* Pull in the current config to define the default environment */ +#include <linux/kconfig.h>
+#ifndef __ASSEMBLY__ +#define __ASSEMBLY__ /* get only #defines from config.h */ +#include <config.h> +#undef __ASSEMBLY__ +#else +#include <config.h> +#endif
+#define DEFAULT_ENV_INSTANCE_STATIC +#include <generated/environment.h> +#include <env_default.h>
+int main(void) +{
char *env, *nxt;for (env = default_environment; *env; env = nxt + 1) {for (nxt = env; *nxt; ++nxt) {if (nxt >= &default_environment[sizeof(default_environment)]) {fprintf(stderr, "## Error: environment not terminated\n");return -1;}}printf("%s\n", env);}return 0;+}
2.35.3