[PATCH] QemuVideoDxe: round up FrameBufferSize to full page

[PATCH] QemuVideoDxe: round up FrameBufferSize to full page

[Gerd Hoffmann]

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));
 
-- 
2.9.3

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

[Laszlo Ersek]

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

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

[Laszlo Ersek]

On 04/25/18 16:08, Laszlo Ersek wrote:

> (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.)

Argh, this request was incomplete. I meant to say:

* Please use EFI_SIZE_TO_PAGES() *with* EFI_PAGES_TO_SIZE() to round up
  the value -- this is a pattern we use in edk2 quite a bit, see:

  git grep -w -e EFI_PAGES_TO_SIZE --and -e EFI_SIZE_TO_PAGES

(ALIGN_VALUE(), EFI_PAGE_SIZE, EFI_PAGE_MASK etc are also OK to use, but
they generally require casting even the constants to UINTN first --
because those are genuinely "int", i.e. INT32 --, and the casting is
arguably uglier than the above-proposed pattern, where the size that we
start out with is already UINTN.)

Thanks!
Laszlo