* [PATCH v5 0/1] Makefile: rework u-boot-initial-env target @ 2022-11-28 8:41 Max Krummenacher 2022-11-28 8:41 ` [PATCH v5 1/1] u-boot-initial-env: rework make target Max Krummenacher 0 siblings, 1 reply; 6+ messages in thread From: Max Krummenacher @ 2022-11-28 8:41 UTC (permalink / raw) To: u-boot Cc: Adam Ford, Pali Rohár, U-Boot STM32, Patrick DELAUNAY, Tom Rini, Patrice CHOTARD, Max Krummenacher, AKASHI Takahiro, Du Huanpeng, Heiko Thiery, Heinrich Schuchardt, Marek Behún, Quentin Schulz, Samuel Holland, Simon Glass, Stefan Roese From: Max Krummenacher <max.krummenacher@toradex.com> With CONFIG_LTO enabled the current way of extracting the configured environment no longer works, i.e. the object file content changes due to LTO. Build a host tool which prints the configured environment instead of using objcopy and friends to achive the same. The code and Makefile changes were mostly stolen from tools/env/ i.e. the target userspace tools to access the environment. Changes in v5: - don't build the printinitialenv tool unconditionally but build it only as part of the u-boot-initial-env target. This no longer fails the 'make tools-only_defconfig tools-only' use-case which is reported by Tom Rini. Adding the $(env_h) dependencies to the tools target might give circular dependencies issues with some future tool. - add Acked-by: Pali Rohár <pali@kernel.org> Changes in v4: - add '(objtree)/' when calling the tool. Suggested by Pali Rohár. - renamed patch, as more than just the Makefile has changes Changes in v3: - moved the tool from scripts/ to tools/. Suggested by Tom Rini - changed the dependencies to '$(env_h)' and 'tools'. Suggested by Tom Rini and Pali Rohár. - removed the sed rule which replaces \x00 with \x0A as this is already done by the tool. Suggested by Pali Rohár. 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" Max Krummenacher (1): u-boot-initial-env: rework make target Makefile | 10 ++++++---- tools/.gitignore | 1 + tools/Makefile | 4 ++++ tools/printinitialenv.c | 44 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 tools/printinitialenv.c -- 2.35.3 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/1] u-boot-initial-env: rework make target 2022-11-28 8:41 [PATCH v5 0/1] Makefile: rework u-boot-initial-env target Max Krummenacher @ 2022-11-28 8:41 ` Max Krummenacher 2022-12-04 21:16 ` Simon Glass 2022-12-08 20:24 ` Tom Rini 0 siblings, 2 replies; 6+ messages in thread From: Max Krummenacher @ 2022-11-28 8:41 UTC (permalink / raw) To: u-boot Cc: Adam Ford, Pali Rohár, U-Boot STM32, Patrick DELAUNAY, Tom Rini, Patrice CHOTARD, Max Krummenacher, AKASHI Takahiro, Du Huanpeng, Heiko Thiery, Heinrich Schuchardt, Marek Behún, Quentin Schulz, Samuel Holland, Simon Glass, Stefan Roese 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> Acked-by: Pali Rohár <pali@kernel.org> --- Changes in v5: - don't build the printinitialenv tool unconditionally but build it only as part of the u-boot-initial-env target. This no longer fails the 'make tools-only_defconfig tools-only' use-case which is reported by Tom Rini. Adding the $(env_h) dependencies to the tools target might give circular dependencies issues with some future tool. - add Acked-by: Pali Rohár <pali@kernel.org> Changes in v4: - add '(objtree)/' when calling the tool. Suggested by Pali Rohár. - renamed patch, as more than just the Makefile has changes Changes in v3: - moved the tool from scripts/ to tools/. Suggested by Tom Rini - changed the dependencies to '$(env_h)' and 'tools'. Suggested by Tom Rini and Pali Rohár. - removed the sed rule which replaces \x00 with \x0A as this is already done by the tool. Suggested by Pali Rohár. 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 | 10 ++++++---- tools/.gitignore | 1 + tools/Makefile | 4 ++++ tools/printinitialenv.c | 44 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 tools/printinitialenv.c diff --git a/Makefile b/Makefile index 2d24ac3959f..32e4bef10f5 100644 --- a/Makefile +++ b/Makefile @@ -2439,11 +2439,13 @@ 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 = \ + $(objtree)/tools/printinitialenv | \ + sed -e '/^\s*$$/d' | \ + sort --field-separator== -k1,1 --stable -o $@ -u-boot-initial-env: u-boot.bin +u-boot-initial-env: $(env_h) FORCE + $(Q)$(MAKE) $(build)=tools $(objtree)/tools/printinitialenv $(call if_changed,genenv) # Consistency checks diff --git a/tools/.gitignore b/tools/.gitignore index d3a93ff294a..28e8ce2a07a 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -28,6 +28,7 @@ /mxsboot /ncb /prelink-riscv +/printinitialenv /proftool /relocate-rela /spl_size_limit diff --git a/tools/Makefile b/tools/Makefile index 26be0a7ba2e..80bc62befcb 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -275,6 +275,10 @@ clean-dirs := lib common always := $(hostprogs-y) +# Host tool to dump the currently configured default environment, +# build it on demand, i.e. not add it to 'always'. +hostprogs-y += printinitialenv + # Generated LCD/video logo LOGO_H = $(objtree)/include/bmp_logo.h LOGO_DATA_H = $(objtree)/include/bmp_logo_data.h diff --git a/tools/printinitialenv.c b/tools/printinitialenv.c new file mode 100644 index 00000000000..c58b234d679 --- /dev/null +++ b/tools/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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] u-boot-initial-env: rework make target 2022-11-28 8:41 ` [PATCH v5 1/1] u-boot-initial-env: rework make target Max Krummenacher @ 2022-12-04 21:16 ` Simon Glass 2022-12-08 20:24 ` Tom Rini 1 sibling, 0 replies; 6+ messages in thread From: Simon Glass @ 2022-12-04 21:16 UTC (permalink / raw) To: Max Krummenacher Cc: u-boot, Adam Ford, Pali Rohár, U-Boot STM32, Patrick DELAUNAY, Tom Rini, Patrice CHOTARD, Max Krummenacher, AKASHI Takahiro, Du Huanpeng, Heiko Thiery, Heinrich Schuchardt, Marek Behún, Quentin Schulz, Samuel Holland, Stefan Roese Hi Max, On Mon, 28 Nov 2022 at 21:41, Max Krummenacher <max.oss.09@gmail.com> 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> > Acked-by: Pali Rohár <pali@kernel.org> > > --- > > Changes in v5: > - don't build the printinitialenv tool unconditionally but build it > only as part of the u-boot-initial-env target. > This no longer fails the 'make tools-only_defconfig tools-only' > use-case which is reported by Tom Rini. > Adding the $(env_h) dependencies to the tools target might give > circular dependencies issues with some future tool. > - add Acked-by: Pali Rohár <pali@kernel.org> > > Changes in v4: > - add '(objtree)/' when calling the tool. Suggested by Pali Rohár. > - renamed patch, as more than just the Makefile has changes > > Changes in v3: > - moved the tool from scripts/ to tools/. Suggested by Tom Rini > - changed the dependencies to '$(env_h)' and 'tools'. > Suggested by Tom Rini and Pali Rohár. > - removed the sed rule which replaces \x00 with \x0A as this is already > done by the tool. Suggested by Pali Rohár. > > 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 | 10 ++++++---- > tools/.gitignore | 1 + > tools/Makefile | 4 ++++ > tools/printinitialenv.c | 44 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 55 insertions(+), 4 deletions(-) > create mode 100644 tools/printinitialenv.c Reviewed-by: Simon Glass <sjg@chromium.org> It would be nice to have a test for this. It could go in test_env.py and you can use test_event.py as an example. - Simon > > diff --git a/Makefile b/Makefile > index 2d24ac3959f..32e4bef10f5 100644 > --- a/Makefile > +++ b/Makefile > @@ -2439,11 +2439,13 @@ 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 = \ > + $(objtree)/tools/printinitialenv | \ > + sed -e '/^\s*$$/d' | \ > + sort --field-separator== -k1,1 --stable -o $@ > > -u-boot-initial-env: u-boot.bin > +u-boot-initial-env: $(env_h) FORCE > + $(Q)$(MAKE) $(build)=tools $(objtree)/tools/printinitialenv > $(call if_changed,genenv) > > # Consistency checks > diff --git a/tools/.gitignore b/tools/.gitignore > index d3a93ff294a..28e8ce2a07a 100644 > --- a/tools/.gitignore > +++ b/tools/.gitignore > @@ -28,6 +28,7 @@ > /mxsboot > /ncb > /prelink-riscv > +/printinitialenv > /proftool > /relocate-rela > /spl_size_limit > diff --git a/tools/Makefile b/tools/Makefile > index 26be0a7ba2e..80bc62befcb 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -275,6 +275,10 @@ clean-dirs := lib common > > always := $(hostprogs-y) > > +# Host tool to dump the currently configured default environment, > +# build it on demand, i.e. not add it to 'always'. > +hostprogs-y += printinitialenv > + > # Generated LCD/video logo > LOGO_H = $(objtree)/include/bmp_logo.h > LOGO_DATA_H = $(objtree)/include/bmp_logo_data.h > diff --git a/tools/printinitialenv.c b/tools/printinitialenv.c > new file mode 100644 > index 00000000000..c58b234d679 > --- /dev/null > +++ b/tools/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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] u-boot-initial-env: rework make target 2022-11-28 8:41 ` [PATCH v5 1/1] u-boot-initial-env: rework make target Max Krummenacher 2022-12-04 21:16 ` Simon Glass @ 2022-12-08 20:24 ` Tom Rini 2022-12-12 13:39 ` Max Krummenacher 1 sibling, 1 reply; 6+ messages in thread From: Tom Rini @ 2022-12-08 20:24 UTC (permalink / raw) To: Max Krummenacher Cc: u-boot, Adam Ford, Pali Rohár, U-Boot STM32, Patrick DELAUNAY, Patrice CHOTARD, Max Krummenacher, AKASHI Takahiro, Du Huanpeng, Heiko Thiery, Heinrich Schuchardt, Marek Behún, Quentin Schulz, Samuel Holland, Simon Glass, Stefan Roese [-- Attachment #1: Type: text/plain, Size: 895 bytes --] On Mon, Nov 28, 2022 at 09:41:22AM +0100, 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> > Acked-by: Pali Rohár <pali@kernel.org> > Reviewed-by: Simon Glass <sjg@chromium.org> Applied to u-boot/next, thanks! -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] u-boot-initial-env: rework make target 2022-12-08 20:24 ` Tom Rini @ 2022-12-12 13:39 ` Max Krummenacher 2022-12-12 14:00 ` Tom Rini 0 siblings, 1 reply; 6+ messages in thread From: Max Krummenacher @ 2022-12-12 13:39 UTC (permalink / raw) To: Tom Rini Cc: u-boot, Adam Ford, Pali Rohár, U-Boot STM32, Patrick DELAUNAY, Patrice CHOTARD, Max Krummenacher, AKASHI Takahiro, Du Huanpeng, Heiko Thiery, Heinrich Schuchardt, Marek Behún, Quentin Schulz, Samuel Holland, Simon Glass, Stefan Roese Hi Tom On Thu, Dec 8, 2022 at 9:24 PM Tom Rini <trini@konsulko.com> wrote: > > On Mon, Nov 28, 2022 at 09:41:22AM +0100, 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> > > Acked-by: Pali Rohár <pali@kernel.org> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Applied to u-boot/next, thanks! The commit not only fixes the use case on arm64 with LTO enabled, it also fixes sandbox for x86-64. For me on Fedora with a `gcc (GCC) 11.3.1 20220421` both `make sandbox_defconfig; make u-boot-initial-env` and `make sandbox_defconfig; <unselect LTO>; make u-boot-initial-env` fail with ``` GENENV u-boot-initial-env objcopy: env/common.o: can't dump section '.rodata.default_environment' - it does not exist: file format not recognized sed: can't read u-boot-initial-env: No such file or directory ``` Wouldn't that merit applying the commit to master, i.e. include it in v2023.01? Regards Max P.S.: The test in [1] catches this failure, so [1] should go in after this commit to have CI tests not failing. [1] https://lore.kernel.org/all/20221209120956.2286619-1-max.oss.09@gmail.com/ > > -- > Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/1] u-boot-initial-env: rework make target 2022-12-12 13:39 ` Max Krummenacher @ 2022-12-12 14:00 ` Tom Rini 0 siblings, 0 replies; 6+ messages in thread From: Tom Rini @ 2022-12-12 14:00 UTC (permalink / raw) To: Max Krummenacher Cc: u-boot, Adam Ford, Pali Rohár, U-Boot STM32, Patrick DELAUNAY, Patrice CHOTARD, Max Krummenacher, AKASHI Takahiro, Du Huanpeng, Heiko Thiery, Heinrich Schuchardt, Marek Behún, Quentin Schulz, Samuel Holland, Simon Glass, Stefan Roese [-- Attachment #1: Type: text/plain, Size: 1954 bytes --] On Mon, Dec 12, 2022 at 02:39:09PM +0100, Max Krummenacher wrote: > Hi Tom > > On Thu, Dec 8, 2022 at 9:24 PM Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Nov 28, 2022 at 09:41:22AM +0100, 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> > > > Acked-by: Pali Rohár <pali@kernel.org> > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Applied to u-boot/next, thanks! > > The commit not only fixes the use case on arm64 with LTO enabled, it also > fixes sandbox for x86-64. For me on Fedora with a `gcc (GCC) 11.3.1 20220421` > both `make sandbox_defconfig; make u-boot-initial-env` and > `make sandbox_defconfig; <unselect LTO>; make u-boot-initial-env` fail with > ``` > GENENV u-boot-initial-env > objcopy: env/common.o: can't dump section > '.rodata.default_environment' - it does not exist: file format not > recognized > sed: can't read u-boot-initial-env: No such file or directory > ``` > > Wouldn't that merit applying the commit to master, i.e. include it in v2023.01? You can just disable LTO, and it's been an issue for a while now. It's also easy enough to cherry-pick if there's cases out there that can't wait. Thanks again for addressing the problem! -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-12 14:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-28 8:41 [PATCH v5 0/1] Makefile: rework u-boot-initial-env target Max Krummenacher 2022-11-28 8:41 ` [PATCH v5 1/1] u-boot-initial-env: rework make target Max Krummenacher 2022-12-04 21:16 ` Simon Glass 2022-12-08 20:24 ` Tom Rini 2022-12-12 13:39 ` Max Krummenacher 2022-12-12 14:00 ` Tom Rini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox