On 08/18/2018 10:55 AM, Simon Goldschmidt wrote: > On Fri, Aug 17, 2018 at 12:20 PM Marek Vasut <[email protected]> wrote: >> >> On 08/17/2018 08:56 AM, Simon Goldschmidt wrote: >>> On Fri, Aug 17, 2018 at 1:57 AM Marek Vasut <[email protected]> wrote: >>>> >>>> On 08/16/2018 09:44 PM, Simon Goldschmidt wrote: >>>>> On Thu, Aug 16, 2018 at 4:04 PM Marek Vasut <[email protected]> wrote: >>>>>> >>>>>> On 08/16/2018 03:50 PM, Simon Goldschmidt wrote: >>>>>>> On Thu, Aug 16, 2018 at 3:15 PM Marek Vasut <[email protected]> wrote: >>>>>>>> >>>>>>>> On 08/16/2018 03:12 PM, Simon Goldschmidt wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> Marek Vasut <[email protected] <mailto:[email protected]>> schrieb am Do., 16. >>>>>>>>> Aug. 2018, 15:06: >>>>>>>>> >>>>>>>>> On 08/16/2018 03:00 PM, Simon Goldschmidt wrote: >>>>>>>>> > On Thu, Aug 16, 2018 at 1:18 PM Marek Vasut <[email protected] >>>>>>>>> <mailto:[email protected]>> wrote: >>>>>>>>> >> >>>>>>>>> >> On 08/16/2018 09:38 AM, Simon Goldschmidt wrote: >>>>>>>>> >>> gd->env_addr points to pre-relocation address even after >>>>>>>>> >>> relocation. This leads to an abort in env_callback_init >>>>>>>>> >>> when loading the environment. >>>>>>>>> >>> >>>>>>>>> >>> Fix this by enabling CONFIG_SYS_EXTRA_ENV_RELOC. >>>>>>>>> >>> >>>>>>>>> >>> Signed-off-by: Simon Goldschmidt >>>>>>>>> <[email protected] >>>>>>>>> <mailto:[email protected]>> >>>>>>>>> >> >>>>>>>>> >> I have one last question -- does this somehow influence SPL ? >>>>>>>>> > >>>>>>>>> > No, it doesn't. The code that gets enabled by this define is in >>>>>>>>> > common/board_r.c, which is not linked for SPL. >>>>>>>>> >>>>>>>>> Ah, thanks for checking. >>>>>>>>> >>>>>>>>> btw do you think it'd make sense to just enable this by default >>>>>>>>> on all >>>>>>>>> systems and zap the EXTRA_ENV_RELOC macro altogether ? >>>>>>>>> >>>>>>>>> >>>>>>>>> Yes, that's what I have thought about already. Just like the for the >>>>>>>>> embedded device tree relocation, we could then probably use >>>>>>>>> gd->reloc_off instead of CONFIG_SYS_MONITOR_BASE. I'm just not sure >>>>>>>>> this >>>>>>>>> really works for all boards, but it would be worth a try to push after >>>>>>>>> this release is out. >>>>>>>> >>>>>>>> I think so too. I cannot think of a reason why this shouldn't be >>>>>>>> enabled >>>>>>>> in fact. >>>>>>> >>>>>>> Exactly. Too me it seems like a leftover, especially given the use of >>>>>>> CONFIG_SYS_MONITOR_BASE, which seems a little outdated, too. >>>>>>> I've set up a reminder for a patch to remove it after the release. >>>>>> >>>>>> Feel free to send it now. >>>>> >>>>> OK, I have tried, but it seems it's not that easy: some boards >>>>> override the initial gd->env_addr by setting CONFIG_ENV_ADDR. So if >>>>> this is outside of U-Boot's pre-relocation range, it clearly should >>>>> not be relocated. One might find an improved way to relocate >>>>> gd->env_addr if it is internal (e.g. checking the range to be in >>>>> pre-relocation?). But simply removing the EXTRA_ENV_RELOC does not >>>>> seem to work. >>>> >>>> Shouldn't most of those boards be easily fixable ? >>> >>> Well, if we unconditionally alter gd->env_addr by gd->reloc_off, >>> boards that have their initial gd->env_addr outside of the initial >>> binary can be fixed only by changing their behaviour. I don't know how >>> widely used this feature is, but since it's a config option >>> (CONFIG_ENV_ADDR), how would we know? >> >> git grep ? But aren't you mixing CONFIG_ENV_ADDR and CONFIG_ENV_RELOC ? > > Have a look at env_sf_init() in env/sf.c (called from env_init(), > which in turn is called from board_init_f()). There gd->env_addr is > initialized by a config setting. If this user-supplied address is > outside of the binary, relocating it is wrong, isn't it?
I think you want to relocate the env close to where U-Boot is relocated in all cases, no ? > Anyway, I'm off for 2 weeks now (holiday time here) with some email > access at most, so I'll continue on this when I get back. Have a nice vacation. > Simon > >> >>> So to me that means we still have to make this overridable and could >>> change the "default" state of such an option only. Meaning that the >>> default is "relocate gd->env_addr" with an option to leave it. But is >>> this really worth breaking existing boards? >> >> I think you do want to relocate the env alongside U-Boot, always, no ? >> >> -- >> Best regards, >> Marek Vasut -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
