On Fri, Aug 17, 2018 at 12:20 PM Marek Vasut <ma...@denx.de> wrote: > > On 08/17/2018 08:56 AM, Simon Goldschmidt wrote: > > On Fri, Aug 17, 2018 at 1:57 AM Marek Vasut <ma...@denx.de> wrote: > >> > >> On 08/16/2018 09:44 PM, Simon Goldschmidt wrote: > >>> On Thu, Aug 16, 2018 at 4:04 PM Marek Vasut <ma...@denx.de> wrote: > >>>> > >>>> On 08/16/2018 03:50 PM, Simon Goldschmidt wrote: > >>>>> On Thu, Aug 16, 2018 at 3:15 PM Marek Vasut <ma...@denx.de> wrote: > >>>>>> > >>>>>> On 08/16/2018 03:12 PM, Simon Goldschmidt wrote: > >>>>>>> > >>>>>>> > >>>>>>> Marek Vasut <ma...@denx.de <mailto:ma...@denx.de>> 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 <ma...@denx.de > >>>>>>> <mailto:ma...@denx.de>> 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 > >>>>>>> <simon.k.r.goldschm...@gmail.com > >>>>>>> <mailto:simon.k.r.goldschm...@gmail.com>> > >>>>>>> >> > >>>>>>> >> 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? 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. 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
