diff mbox series

[RFC,RFT] env: spl: filter the variables in default environment of SPL or TPL

Message ID 20200318143602.23253-1-patrick.delaunay@st.com
State New
Headers show
Series [RFC,RFT] env: spl: filter the variables in default environment of SPL or TPL | expand

Commit Message

Patrick Delaunay March 18, 2020, 2:36 p.m. UTC
Use a new option CONFIG_SPL_ENV_VARS to filter the variables included
in the default environment used in SPL (and TPL).

That allows to reduce the size of default_environment[].

If the new configuration flags CONFIG_SPL_ENV_VARS is
- "" the default environment is empty (default)
- list of variables (comma separated) to keept in SPL/TPL
  (it searches ^<var>= in u-boot-initial-env)
- "*" the variables are not filtered

Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
---
Hi,

I propose this patch to reduce the SPL/TPL size when they support the
U-Boot environment.

But I need feedback, review and test (mainly for first boot when
environment in storage device is empty) to:
- confirm this patch can be acceptable
- confirm my assumptions on the list of variable used in SPL:
  empty by default in this first version.

This patch adds a filter on the environment variables present
in default environment of U-Boot, so SPL/TPL only include the required
variables.

I assumed the SPL/TPL environment can be empty by default (I filter all
the variables) because I think the environment is only needed when it is
loaded from external storage device; on cold boot and before to save env,
the content of default environment it is not necessary (To be confirmed).

For example, with falcon mode SPL need the environment to have the bootcmd
(load address , the file name to load and the the bootargs) only after
a first boot and after a correct 'env save' in U-Boot.

But I can update the default value for CONFIG_SPL_ENV_VARS to add
some generic variable to solve any raised issues:
        .flags
        .callback
        cpu
        arch
        vendor
        soc

PS: the current behavior (no filter) is kept when
    CONFIG_SPL_ENV_VARS = "*"

I propose this patch after remarks on previous patch v4
env: Add CONFIG_ENV_FULL_SUPPORT

http://patchwork.ozlabs.org/patch/1171180/

It is a preliminary step for v5 of this serie.

Patrick


 Makefile              | 32 ++++++++++++++++++++++++++++++--
 env/Kconfig           | 11 +++++++++++
 include/env_default.h |  4 ++++
 3 files changed, 45 insertions(+), 2 deletions(-)

Comments

Wolfgang Denk March 18, 2020, 2:51 p.m. UTC | #1
Dear Patrick,

In message <20200318143602.23253-1-patrick.delaunay at st.com> you wrote:
> Use a new option CONFIG_SPL_ENV_VARS to filter the variables included
> in the default environment used in SPL (and TPL).
>
> That allows to reduce the size of default_environment[].

Sorry, but NAK.  we had this discussion a couple of times before.
It is mandatory that both SPL and U-Boot proper will use the _same_
environment, including the same default environment, or all kind of
havoc may result.  Just think of situations where Falcon Mode is
being used and SPL and U-Boot proper would be using different
settings.

If your default environment is too big for the SPL, then make it
smaller.  If you need additional settings in U-Boot, there are many
ways to load thise there.

Best regards,

Wolfgang Denk
Patrick Delaunay March 18, 2020, 3:57 p.m. UTC | #2
Hi Wolfgang,

> From: Wolfgang Denk <wd at denx.de>
> Sent: mercredi 18 mars 2020 15:51
> To: Patrick DELAUNAY <patrick.delaunay at st.com>
> Cc: u-boot at lists.denx.de; Bin Meng <bmeng.cn at gmail.com>; Fabio Estevam
> <festevam at gmail.com>; Heinrich Schuchardt <xypron.glpk at gmx.de>; Jagan
> Teki <jagan at amarulasolutions.com>; Joe Hershberger
> <joe.hershberger at ni.com>; Kever Yang <kever.yang at rock-chips.com>; Marek
> Vasut <marex at denx.de>; Simon Glass <sjg at chromium.org>; U-Boot STM32
> <uboot-stm32 at st-md-mailman.stormreply.com>
> Subject: Re: [RFC RFT PATCH] env: spl: filter the variables in default
> environment of SPL or TPL
> 
> Dear Patrick,
> 
> In message <20200318143602.23253-1-patrick.delaunay at st.com> you wrote:
> > Use a new option CONFIG_SPL_ENV_VARS to filter the variables included
> > in the default environment used in SPL (and TPL).
> >
> > That allows to reduce the size of default_environment[].
> 
> Sorry, but NAK.  we had this discussion a couple of times before.
> It is mandatory that both SPL and U-Boot proper will use the _same_ environment,
> including the same default environment, or all kind of havoc may result.  Just think
> of situations where Falcon Mode is being used and SPL and U-Boot proper would
> be using different settings.
> 
> If your default environment is too big for the SPL, then make it smaller.  If you
> need additional settings in U-Boot, there are many ways to load thise there.

Thanks for the answer, 
so it was clearly not an option and I abandon this patch.

To complete my answer, today I don't have issue with SPL environment size on the stm32 targets,

I propose this patch only to solve issue with my previous proposed patch on other target

- [U-Boot,v4,3/3] env: Add CONFIG_ENV_FULL_SUPPORT
- [U-Boot,v4,2/3] env: introduce macro ENV_IS_IN_SOMEWHERE

This proposal raise many size issue for other boards when I activated
CONFIG_SPL_ENV_SUPPORT and CONFIG_SPL_ENV_IS_NOWHERE by default....

Today I don?t see solution except accept the current situation:
for some target, the environment isn't supported/activated in SPL, even default one
(when CONFIG_SPL_ENV_SUPPORT not activated, the env source files aren't compiled)

So I will also abandon the previous patchset
- http://patchwork.ozlabs.org/patch/1171180/ 
- http://patchwork.ozlabs.org/patch/1171098/

Patrick

> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "If
> that makes any sense to you, you have a big problem."
>                                   -- C. Durance, Computer Science 234
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index fa687f13a5..425709ed02 100644
--- a/Makefile
+++ b/Makefile
@@ -1843,6 +1843,34 @@  $(timestamp_h): $(srctree)/Makefile FORCE
 $(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE
 	$(call filechk,defaultenv.h)
 
+# ---------------------------------------------------------------------------
+# the SPL default environment is filtered by CONFIG_SPL_ENV_VARS
+spl_defaultenv_h := include/generated/spl_defaultenv_autogenerated.h
+
+quiet_cmd_gensplenv = GENENV $@
+
+ifeq ($(CONFIG_SPL_ENV_VARS),"*")
+cmd_gensplenv = cp $< $@
+else
+ifeq ($(CONFIG_SPL_ENV_VARS),"")
+cmd_gensplenv = echo '\0'  > $@
+else
+# use grep to filter SPL env with patern "^\(var1\|var2\)=",
+# with $CONFIG_SPL_ENV_VARS = "var1,var2"
+cmd_gensplenv = grep "^\($(subst $(comma),\|,$(CONFIG_SPL_ENV_VARS:"%"=%))\)=" $< > $@
+endif
+endif
+
+u-boot-spl-initial-env: u-boot-initial-env FORCE
+	$(call cmd,gensplenv)
+
+$(spl_defaultenv_h): u-boot-spl-initial-env
+	$(call filechk,defaultenv.h)
+
+spl_prepare: prepare $(spl_defaultenv_h)
+
+PHONY += spl_prepare
+
 # ---------------------------------------------------------------------------
 quiet_cmd_cpp_lds = LDS     $@
 cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \
@@ -1855,7 +1883,7 @@  spl/u-boot-spl.bin: spl/u-boot-spl
 	@:
 	$(SPL_SIZE_CHECK)
 
-spl/u-boot-spl: tools prepare \
+spl/u-boot-spl: tools prepare spl_prepare \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SPL_OF_PLATDATA),dts/dt.dtb) \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_TPL_OF_PLATDATA),dts/dt.dtb)
 	$(Q)$(MAKE) obj=spl -f $(srctree)/scripts/Makefile.spl all
@@ -1872,7 +1900,7 @@  spl/u-boot-spl.sfp: spl/u-boot-spl
 spl/boot.bin: spl/u-boot-spl
 	@:
 
-tpl/u-boot-tpl.bin: tools prepare \
+tpl/u-boot-tpl.bin: tools prepare spl_prepare \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SPL_OF_PLATDATA),dts/dt.dtb)
 	$(Q)$(MAKE) obj=tpl -f $(srctree)/scripts/Makefile.spl all
 	$(TPL_SIZE_CHECK)
diff --git a/env/Kconfig b/env/Kconfig
index 0d6f559b39..ff7a8c1d6c 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -588,7 +588,18 @@  config ENV_VARS_UBOOT_RUNTIME_CONFIG
 	  run-time determined information about the hardware to the
 	  environment.  These will be named board_name, board_rev.
 
+config SPL_ENV_VARS
+	string "list of variables keept in SPL/TPL default environment"
+	default ""
+	help
+		List of variables, coma separated, included in the default
+		environment in SPL and TPL.
+		It is used to reduce the size impact of environment in SPL/TPL
+		None variable are included when the option is empty "".
+		The filtre is deactivated when option is "*".
+
 if SPL_ENV_SUPPORT
+
 config SPL_ENV_IS_NOWHERE
 	bool "SPL Environment is not stored"
 	default y if ENV_IS_NOWHERE
diff --git a/include/env_default.h b/include/env_default.h
index 56a8bae39a..9ea6136803 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -21,6 +21,9 @@  static char default_environment[] = {
 #else
 const uchar default_environment[] = {
 #endif
+#ifdef CONFIG_SPL_BUILD
+#include "generated/spl_defaultenv_autogenerated.h"
+#else
 #ifndef CONFIG_USE_DEFAULT_ENV_FILE
 #ifdef	CONFIG_ENV_CALLBACK_LIST_DEFAULT
 	ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0"
@@ -114,6 +117,7 @@  const uchar default_environment[] = {
 #else /* CONFIG_USE_DEFAULT_ENV_FILE */
 #include "generated/defaultenv_autogenerated.h"
 #endif
+#endif
 #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED
 	}
 #endif