On 6/8/21 9:54 AM, Kunihiko Hayashi wrote:

Hi,

[...]

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 ?

These are called before relocation, and update gd->env_addr to non-relocated
default_environment by [3].

After that, gd->env_addr is relocated in initr_reloc_global_data()
if CONFIG_SYS_RELOC_GD_ENV_ADDR is defined.

| #ifdef CONFIG_SYS_RELOC_GD_ENV_ADDR
|     /*
|      * Relocate the early env_addr pointer unless we know it is not inside |      * the binary. Some systems need this and for the rest, it doesn't hurt.
|      */
|     gd->env_addr += gd->reloc_off;
| #endif


Shouldn't the post-relocation env update happen in env_relocate() ?

Usually env_relocate() calls env_load() that uses relocated gd->env_addr.
It's no problem.

If CONFIG_SYS_TEXT_BASE is zero, gd->reloc_off becomes illegal.
CONFIG_SYS_RELOC_GD_ENV_ADDR should be disabled in that case.

Sorry this isn't wrong.

But then, if CONFIG_SYS_TEXT_BASE is zero, the env shouldn't be relocated or how should it behave ?

I think the env should be relocated if CONFIG_SYS_RELOC_GD_ENV_ADDR=y
regardless of CONFIG_SYS_TEXT_BASE.

If CONFIG_POSITION_INDEPENDENT=y and CONFIG_SYS_TEXT_BASE is zero,
there is something wrong with the calculation of the relocation address about env.

Ah, got it.

gd->reloc_off is relocated address offset from zero, however,
gd->env_addr has still non-relocated address.

 >>>> |     gd->env_addr += gd->reloc_off;

I think the env should be relocated if CONFIG_SYS_RELOC_GD_ENV_ADDR=y.
But this code sets gd->env_addr incorrectly.

In that case, there is a non-relocated <textbase> address instead of
CONFIG_SYS_TEXT_BASE.

This should be "gd->env_addr = (gd->env_addr - <textbase>) + gd->reloc_off",
However, I'm not sure how we get non-relocated <textbase> address.

Maybe what you need to do is store current $pc register when you enter U-Boot very early on, in _start function, and then use it here ? Although, I am not entirely sure whether this is still possible on arm64.

Reply via email to