Hi Marek,
On 2021/06/10 10:07, Marek Vasut wrote:
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.
Exactly. I guess it's reasonable to fix gd->env_addr when POSITION_INDEPENDENT=y before relocation. I'll try it.
Thank you,
--- Best Regards Kunihiko Hayashi