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. Thanks for the review, Quentin -- Quentin Schulz, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
