Hi Quentin, On 3 January 2018 at 02:18, Quentin Schulz <quentin.sch...@free-electrons.com> wrote: > Hi Simon, > > On 29/12/2017 04:13, Simon Glass wrote: >> Hi Quentin, >> >> On 22 December 2017 at 14:13, Quentin Schulz >> <quentin.sch...@free-electrons.com> wrote: >>> This patch series is based on this[1] patch series from Maxime. >>> >>> This is an RFC. It's been only tested in a specific use case on a custom >>> i.MX6 board. It's known to break compilation on a few boards. >>> >>> I have a use case where we want some variables from a first environment to >>> be overriden by variables from a second environment. For example, we want >>> to load variables from the default env (ENV_IS_NOWHERE) and then load only >>> a handful of other variables from, e.g., NAND. >>> >>> In our use case, we basically can be sure that the default env in the >>> U-Boot binary is secure but we want only a few variables to be modified, >>> thus keeping control over the overall behaviour of U-Boot in secure mode. >>> >>> It works in that way: >>> - from highest to lowest priority, the first environment that can be >>> loaded (that has successfully init and whose load function has returned >>> no errors) will be the main environment, >>> - then, all the following environment that could be successfully loaded >>> (same conditions as the main environment) are secondary environment. The >>> env variables that are defined both in CONFIG_ENV_VAR_WHITELIST_LIST and >>> in the secondary environments override the ones in the main environment, >>> - for saving, we save the whole environment to all environments >>> available, be they main or secondary (it does not matter to save the >>> whole environment on secondary environments as only the whitelisted >>> variables will be overriden in the loading process, >>> >>> I have also a few questions that could help me to get the whole thing to >>> work. >>> >>> 1) I can't really get my head around the use of gd->env_addr, what is it >>> used >>> for? It is set in a bunch of different places but only once is it >>> explicitly used (basically to alternate the env_addr between the one >>> associated to main and redundant environment (in NAND for example)). >>> >>> 2) Why do we consider ENV_IS_NOWHERE an invalid environment? The only place >>> I >>> found a use for it was to just say that if the environment is invalid, we >>> should set to default environment (in env_relocate in env/common.c). With >>> my patch series I guess that we could remove this fallback and force >>> ENV_IS_NOWHERE to be always there. >>> >>> 3) There are a few (20) boards that set gd->env_addr and gd->env_valid in >>> their board file. What is the reason to do such a thing? Isn't those >>> overriden anyway by the environment driver? >>> >>> I'm looking forward to getting your feedback on this patch series. >>> >> >> I certainly understand the goal and it seems generally useful. >> >> But I wonder whether this is the best way to implement it. >> >> We could have a UCLASS_ENV uclass, with driver-model drivers which >> load the environment (i.e. have load() and save() methods). Config for >> the drivers would come from the device tree. Useful drivers would be: >> > > I'm not sure the device tree is the place to set/get such info. That has > nothing to do with hardware, it's only logic for the bootloader. > >> - one that loads the env from a single location >> - one that loads multiple envs from different locations in priority order >> - one that does what you want >> >> Then people could set their own policy by adding a driver. >> > > I'll have to document myself on driver-model and how U-Boot implement it > then :) > >> I worry that what we have here is quite heavyweight, and will be >> imposed on all users, e.g. the size increase of gd, the new logic. >> Where does it end? I think splitting things up into different use >> cases makes sense. >> > > Agree on that. > >> When I did the env refactor I envisaged using driver-model for the >> post-reloc env load/save at some point in the future. It solves the >> problem of getting the config (can use device tree) and different >> boards wanting to do different things (boards can provide an env >> driver). >> > > Overall, I prefer Lukasz's suggestion as it's quicker and easier to > implement and require (I think) less changes in the code.
Do you mean selecting from different locations? Yes that is a good thing, but is orthogonal to this series. Here you are trying to add functionality which will apply to any env location, or to them as a whole. So we should think of your change as implementing a new policy rather than a new env location. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
