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

Hi Simon,

Thanks for the patch.

On 30/07/23 22:46, 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") Suggested-by: Nikhil M Jain n-jain1@ti.com

---

Changes in v2:

• Add a Kconfig as the suggested conditional did not work

Overall this approach looks fine too but just curious to know, what is the ho->fb and gd->relocaddr in your case, so that we could understand your scenario better and maybe devise a more generic condition as a later-on patch since you mentioned that shared condition did not work for you.

common/board_f.c | 3 ++- drivers/video/Kconfig | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/common/board_f.c b/common/board_f.c index 7d2c380e91e2..5173d0a0c2d5 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -419,7 +419,8 @@ static int reserve_video(void) if (!ho) return log_msg_ret("blf", -ENOENT); video_reserve_from_bloblist(ho);

• gd->relocaddr = ho->fb;
  

• if (IS_ENABLED(CONFIG_VIDEO_RESERVE_SPL))
  

• 	gd->relocaddr = ho->fb;
  

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

Ok, so as I understand in your case you have enabled CONFIG_SPL_VIDEO but not calling reserve_video in SPL stage ?

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index b41dc60cec59..e0e07ed0cda5 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -1106,6 +1106,14 @@ config SPL_VIDEO_REMOVE if this option is enabled video driver will be removed at the end of SPL stage, beforeloading the next stage.

+config VIDEO_RESERVE_SPL

• bool
• help

•  This adjusts reserve_video() to redirect memory reservation when it
  

•  sees a video handoff blob (BLOBLISTT_U_BOOT_VIDEO). This avoids the
  

•  memory used for video being allocated to U-Boot, thus having some
  

{some minor suggestion for correction on above text} avoids the memory used for framebuffer from being allocated to u-boot, thus preventing any further memory reservations done by u-boot from overwriting the framebuffer ?

I think as mentioned earlier, overall this approach looks ok to me too, maybe later on we can add some conditions to check validity of passed framebuffer address and move the relocaddr pointer more intelligently.

Regards Devarsh

•  data structures overwrite the framebuffer.
  

if SPL_SPLASH_SCREEN

config SPL_SPLASH_SCREEN_ALIGN