[PATCH 1/1] efi_loader: set correct frame buffer address

[Heinrich Schuchardt]

On 5/1/26 17:05, Heinrich Schuchardt wrote:
> If we use video copy, bit image transfers need to write to the in memory
> copy of the physical frame buffer. Damage control will sync the changes
> to the physical frame buffer.
> 
> Cyclic video copy will catch all changes done by EFI applications directly
> accessing the frame buffer copy.
> 
> gopobj->mode.fb_base must be a valid pointer to memory and not a virtual
> sandbox address.
> 
> With this change the block image transfer test works again on the sandbox.
> 
>      setenv efi_selftest block image transfer
>      bootefi selftest
> 
> Fixes: a75cf70d23ac ("efi: Correct handling of frame buffer")
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> ---
>   lib/efi_loader/efi_gop.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
> index 9403e09691e..ae44d140289 100644
> --- a/lib/efi_loader/efi_gop.c
> +++ b/lib/efi_loader/efi_gop.c
> @@ -471,7 +471,7 @@ efi_status_t efi_gop_register(void)
>   {
>   	struct efi_gop_obj *gopobj;
>   	u32 bpix, format, col, row;
> -	u64 fb_base, fb_size;
> +	u64 fb_size;
>   	efi_status_t ret;
>   	struct udevice *vdev;
>   	struct video_priv *priv;
> @@ -490,7 +490,6 @@ efi_status_t efi_gop_register(void)
>   	row = video_get_ysize(vdev);
>   
>   	plat = dev_get_uclass_plat(vdev);
> -	fb_base = IS_ENABLED(CONFIG_VIDEO_COPY) ? plat->copy_base : plat->base;
>   	fb_size = plat->size;
>   
>   	switch (bpix) {
> @@ -528,7 +527,7 @@ efi_status_t efi_gop_register(void)
>   	gopobj->mode.info = &gopobj->info;
>   	gopobj->mode.info_size = sizeof(gopobj->info);
>   
> -	gopobj->mode.fb_base = fb_base;
> +	gopobj->mode.fb_base = (uintptr_t)priv->fb;

I discussed the value of FrameBufferBase with Simon.

The code before the patch is incorrect, as on the sandbox it does not 
provide a pointer value but a sandbox virtual address. EFI applications 
may use the pointer for directly writing to the frame buffer. This would 
lead to a crash on the sandbox.

For the case of CONFIG_VIDEO_COPY=y it is problematic to pass the memory 
buffer address here, as Linux has an EFI framebuffer driver and would 
write to the buffer instead of the physical framebuffer. But our memory 
buffer is neither reserved memory not is it copied to the physical 
framebuffer after ExitBootServices.

If we pass the physical framebuffer address here, then an EFI 
application might write to it and the cyclic video sync would overwrite 
the changed pixels.

Setting PixelBltOnly in field Mode.PixelFormat forbids EFI applications 
to directly write to the framebuffer.

So if plat->copy_base is set we should pass PixelBltOnly as PixelFormat 
and copy_base as FrameBufferBase.

We the aforementioned change we will need to change function 
gop_get_bpp() to not rely on PixelFormat. Instead we need to pass 
priv->format in a private field of the graphics protocol.


>   	gopobj->mode.fb_size = fb_size;
>   
>   	gopobj->info.version = 0;
> @@ -553,7 +552,7 @@ efi_status_t efi_gop_register(void)
>   	}
>   	gopobj->info.pixels_per_scanline = col;
>   	gopobj->bpix = bpix;
> -	gopobj->fb = map_sysmem(fb_base, fb_size);
> +	gopobj->fb = priv->fb;

priv->fb is also the value used by the TrueType drivers.

Best regards

Heinrich

>   	gopobj->vdev = vdev;
>   
>   	return EFI_SUCCESS;