On 5/12/21 4:09 PM, Kunihiko Hayashi wrote:
When CONFIG_ENV_IS_NOWHERE is enabled, env_nowhere_init() sets ENV_INVALID
to gd->env_valid, and sets default_environment before relocation to
gd->env_addr. After that, env_init() switches gd->env_valid to ENV_VALID
by the previous fix.

If gd->env_valid is ENV_INVALID, env_get_char() returns relocated
default_environment, however, env_get_char() returns gd->env_addr before
relocation since gd->env_valid is ENV_VALID, and access to gd->env_addr
will cause a fault.

This leaves gd->env_valid as ENV_INVALID for "nowhere" location.

So do I understand this correctly that _after_ relocation, env_init() is called and env_init() does not update gd->env_addr to the relocated one?

I would expect that after relocation, if all you have is env_nowhere driver, the env_nowhere_init() is called again from the first for() loop of env_init() [1], which would set gd->env_valid to ENV_INVALID [1], and that for() loop would exit with ret = -ENOENT [2], so then the last part of env_init() would check for ret == -ENOENT and update gd->env_addr to relocated default_environment [3].

324  int env_init(void)
325  {
326    struct env_driver *drv;
327    int ret = -ENOENT;
328    int prio;
329
330    for (prio = 0; (drv = env_driver_lookup(ENVOP_INIT, prio)); prio++) {
         /* Part [1] */
331      if (!drv->init || !(ret = drv->init()))
332        env_set_inited(drv->location);
333      if (ret == -ENOENT)
334        env_set_inited(drv->location);
335
336      debug("%s: Environment %s init done (ret=%d)\n", __func__,
337            drv->name, ret);
338
         /* Part [2] */
339      if (gd->env_valid == ENV_INVALID)
340        ret = -ENOENT;
341    }
342
343    if (!prio)
344      return -ENODEV;
345
       /* Part [3] */
346    if (ret == -ENOENT) {
         /* This should be relocated default_environment address */
347      gd->env_addr = (ulong)&default_environment[0];
348      gd->env_valid = ENV_VALID;
349
350      return 0;
351    }
352
353    return ret;
354  }

Or am I missing something obvious ?

diff --git a/env/env.c b/env/env.c
index e534008..3233172 100644
--- a/env/env.c
+++ b/env/env.c
@@ -336,7 +336,8 @@ int env_init(void)
                debug("%s: Environment %s init done (ret=%d)\n", __func__,
                      drv->name, ret);
- if (gd->env_valid == ENV_INVALID)
+               if (gd->env_valid == ENV_INVALID
+                   && drv->location != ENVL_NOWHERE)
                        ret = -ENOENT;
        }

Reply via email to