[PATCH] board_f: Relocate fdt if SKIP_RELOC and fdt is in bss

[Simon Glass]

Hi Kunihiko,

On Fri, 16 Jun 2023 at 00:59, Kunihiko Hayashi
<hayashi.kunihiko at socionext.com> wrote:
>
> Hi Simon,
>
> Thank you for your comment.
>
> On 2023/06/13 6:17, Simon Glass wrote:
> > Hi Kunihiko,
> >
> > On Thu, 8 Jun 2023 at 07:53, Kunihiko Hayashi
> > <hayashi.kunihiko at socionext.com> wrote:
> >>
> >> There are cases that the devicetree blob is placed after _end, such as
> >> fdt_find_separate() returns _end. This is in bss area cleared before
> >> relocation.
> >>
> >> When GD_FLG_SKIP_RELOC is set, the blob is still in bss, but will be
> >> cleared. As a result, the devicetree become invalid.
> >>
> >> To avoid this issue, should relocate it to the new fdt area using the
> >> latter condition in reloc_fdt().
> >>
> >> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko at socionext.com>
> >> ---
> >>   common/board_f.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/common/board_f.c b/common/board_f.c
> >> index 51d2f3c365e9..9a245872d190 100644
> >> --- a/common/board_f.c
> >> +++ b/common/board_f.c
> >> @@ -622,7 +622,8 @@ static int init_post(void)
> >>   static int reloc_fdt(void)
> >>   {
> >>          if (!IS_ENABLED(CONFIG_OF_EMBED)) {
> >> -               if (gd->flags & GD_FLG_SKIP_RELOC)
> >> +               if (gd->flags & GD_FLG_SKIP_RELOC &&
> >> +                   gd->fdt_blob != &_end)
> >
> > !IS_ENABLED(CONFIG_OF_EMBED)  == IS_ENABLED(CONFIG_OF_SEPARATE)
> >
> > reloc_fdt() is only called by U-Boot (not SP)
> >
> > So the FDT was found by fdt_find_separate() and gd->fdt_blob == &_end
> >
> > So, is there any case where:
> >
> > gd->flags & GD_FLG_SKIP_RELO is true
> > gd->fdt_blob != &_end is true
> >
> > ?
>
> Yes. But in fact,
> if (gd->flags & GD_FLG_SKIP_RELOC is true) and (gd->fdt_blob overlaps .bss section),
> the fdt should be relocated because clear_bss() will clear the fdt after that.

I misstated my point. I am wondering if  gd(->fdt_blob != &_end) is
ever false? If not, then we can always relocate.

>
> > I can't think of one.
> >
> > If that is the case, then you could add a comment to this effect and
> > unconditionally relocate if !CONFIG_OF_EMBED.
>
> I'm not sure if it is possible to unconditionally relocate the fdt,
> I think we need to know if the fdt overlaps .bss section.

What is the down-side of always relocating, and when would the fdt not
overlap the .bss section?

Note that OF_EMBED is not to be used in production boards. That is
violated by a few things at present, but IMO always relocating should
be safe.

>
> > Of course the down size is that you would probably rather relocate it
> > to just after BSS (rather than to the top of memory) but it doesn't
> > much matter, I suspect.
> Thank you,

Regards,
Simon