Re: [PATCH] QemuVideoDxe: round up FrameBufferSize to full page

[Laszlo Ersek <lersek@redhat.com>]

Hi Gerd,

On 04/25/18 14:10, Gerd Hoffmann wrote:
> Guests do the same, because the framebuffer is mapped somewhere, which
> obviously works with page granularity only.
> 
> When not rounding up to full page size we get messages like this one
> (linux kernel):
> 
>     efifb: framebuffer at 0x80000000, using 1876k, total 1875k
>                                             ^^^^^        ^^^^^
> Also sysfb is confused and throws an error:
> 
>     sysfb: VRAM smaller than advertised
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/QemuVideoDxe/Gop.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
> index d51efc2a83..2bd905efea 100644
> --- a/OvmfPkg/QemuVideoDxe/Gop.c
> +++ b/OvmfPkg/QemuVideoDxe/Gop.c
> @@ -69,6 +69,8 @@ QemuVideoCompleteModeData (
>    Mode->FrameBufferBase = FrameBufDesc->AddrRangeMin;
>    Mode->FrameBufferSize = Info->HorizontalResolution * Info->VerticalResolution;
>    Mode->FrameBufferSize = Mode->FrameBufferSize * ((ModeData->ColorDepth + 7) / 8);
> +  Mode->FrameBufferSize =
> +    (Mode->FrameBufferSize + EFI_PAGE_SIZE - 1) & ~EFI_PAGE_MASK;
>    DEBUG ((EFI_D_INFO, "FrameBufferBase: 0x%Lx, FrameBufferSize: 0x%Lx\n",
>      Mode->FrameBufferBase, (UINT64)Mode->FrameBufferSize));
>  
> 

thanks for the patch; can you please implement three updates:

(1) Please use the EFI_SIZE_TO_PAGES() macro for rounding up the size.

(EFI_SIZE_TO_PAGES() takes one UINTN argument, but there's no need for
an explicit cast, because
"EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE.FrameBufferSize" is already UINTN.)


(2) Please do the same rounding-up in the
QemuVideoVmwareSvgaCompleteModeData() function.

(I'm copying Phil, should he want to comment.)


(3) Please replace "QemuVideoDxe" with "OvmfPkg/QemuVideoDxe" in the
subject line.


Also, can you please verify that the "basic MS video adapter" works in
Windows 8 / Windows 10, after this change? (Apologies if you've already
done that.)


Thank you!
Laszlo