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