[PATCH 5/9] board_f: Fix corruption of relocaddr

[Simon Glass]

Hi Devarsh,

On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar <devarsht at ti.com> wrote:
>
> Hi Simon,
>
> On 24/07/23 20:22, Simon Glass wrote:
> > When the video framebuffer comes from the bloblist, we should not change
> > relocaddr to this address, since it interfers with the normal memory
> > allocation.
> >
> > This fixes a boot loop in qemu-x86_64
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL to u-boot")
> > ---
> >
> >  common/board_f.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/common/board_f.c b/common/board_f.c
> > index 7d2c380e91e2..5c8646b22283 100644
> > --- a/common/board_f.c
> > +++ b/common/board_f.c
> > @@ -419,7 +419,6 @@ static int reserve_video(void)
> >               if (!ho)
> >                       return log_msg_ret("blf", -ENOENT);
> >               video_reserve_from_bloblist(ho);
> > -             gd->relocaddr = ho->fb;
>
> I think this change was done as relocaddr pointer was required to be updated
> to move after frame-buffer reserved area to ensure that any further memory
> reservations done using gd->relocaddr (for e.g. in reserve_trace/uboot/malloc)
> don't overlap with frame-buffer reserved area passed from blob, so I think
> removing this line may cause further memory reservations to overlap with
> reserved framebuffer.
>
> Could you please confirm?

SPL and U-Boot have different memory layouts. The current code is
interrupting U-Boot's careful building up of chunks of memory used for
different purposes.

The video memory has already been allocated by SPL, so we don't need
to insert a new one here, as your code demonstrates.

But also we have no way of knowing if it is legal to relocate U-Boot
(and various other things) just below the frame buffer chosen by SPL.

The SPL frame buffer needs to be considered pre-allocated. It makes no
sense to set relocaddr to this value. It will break all sorts of
things. E.g. qemu-x86_64 crashes with this.

>
>
> Regards
> Devarsh
>
> >       } else if (CONFIG_IS_ENABLED(VIDEO)) {
> >               ulong addr;
> >               int ret;

Regards,
Simon