Hi Devarsh, On Tue, 25 Jul 2023 at 03:21, Devarsh Thakkar <devar...@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 <s...@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