Re: [PATCH v3 2/4] OvmfPkg: add QemuRamfbDxe

[Laszlo Ersek <lersek@redhat.com>]

On 06/13/18 09:29, Gerd Hoffmann wrote:

> +EFI_STATUS
> +EFIAPI
> +InitializeQemuRamfb (
> +  IN EFI_HANDLE           ImageHandle,
> +  IN EFI_SYSTEM_TABLE     *SystemTable
> +  )
> +{
> +  EFI_DEVICE_PATH_PROTOCOL  *RamfbDevicePath;
> +  EFI_DEVICE_PATH_PROTOCOL  *GopDevicePath;
> +  VOID                      *DevicePath;
> +  VENDOR_DEVICE_PATH        VendorDeviceNode;
> +  ACPI_ADR_DEVICE_PATH      AcpiDeviceNode;
> +  EFI_STATUS                Status;
> +  EFI_PHYSICAL_ADDRESS      FbBase;
> +  UINTN                     FbSize, MaxFbSize;
> +  UINTN                     FwCfgSize, Pages, Index;

(1) Lol, you really disliked point (15) from my previous review :) OK,
I'll fix this up before pushing.

> diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> new file mode 100644
> index 0000000000..a91fabd4d8
> --- /dev/null
> +++ b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> @@ -0,0 +1,52 @@
> +## @file
> +#  This driver is a implementation of the Graphics Output Protocol
> +#  for the QEMU ramfb device.
> +#
> +#  Copyright (c) 2018, Red Hat Inc.
> +#
> +#  This program and the accompanying materials are licensed and made
> +#  available under the terms and conditions of the BSD License which
> +#  accompanies this distribution. The full text of the license may be
> +#  found at http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> +#  BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> +#  EXPRESS OR IMPLIED.
> +#

(2) Whatever editor you used to rewrap this removed the trailing "##".
But, I'll fix that up for you.

So here's the patch I'm going to squash into yours:

> diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> index a91fabd4d8f7..013edef72b59 100644
> --- a/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> +++ b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> @@ -13,6 +13,7 @@
>  #  BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>  #  EXPRESS OR IMPLIED.
>  #
> +##
>
>  [Defines]
>    INF_VERSION                    = 0x00010005
> diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfb.c b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> index 044e5c325e80..b49f2ca6e891 100644
> --- a/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> +++ b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> @@ -238,8 +238,9 @@ InitializeQemuRamfb (
>    ACPI_ADR_DEVICE_PATH      AcpiDeviceNode;
>    EFI_STATUS                Status;
>    EFI_PHYSICAL_ADDRESS      FbBase;
> -  UINTN                     FbSize, MaxFbSize;
> -  UINTN                     FwCfgSize, Pages, Index;
> +  UINTN                     FbSize, MaxFbSize, Pages;
> +  UINTN                     FwCfgSize;
> +  UINTN                     Index;
>
>    if (!QemuFwCfgIsAvailable ()) {
>      DEBUG ((DEBUG_INFO, "Ramfb: no FwCfg\n"));

[lersek@redhat.com: fix INF banner typo]
[lersek@redhat.com: make some local variable definitions more idiomatic]

With that:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

--*--

I've also tested the series, against your "sirius/ramfb" QEMU branch
(currently at commit 778450b87275) that you noted in:

  [Qemu-devel] [PATCH v5 0/4] ramfb: simple boot framebuffer
  http://mid.mail-archive.com/20180613122948.18149-1-kraxel@redhat.com

I used the plain "ramfb" device model.

* testing on x86_64/KVM:

I booted a Fedora 27 ISO, and verified that efifb worked fine (including
X11/GDM).

Furthermore, in

  Re: [edk2] [PATCH 0/4] Add QemuRamfbDxe driver
  http://mid.mail-archive.com/20180613074006.pawpkylp5ok76qdz@sirius.home.kraxel.org

we discussed disconnecting the child handle and the parent handle. For
my QEMU cmdline, the "dh -d -v -p GraphicsOutput" UEFI shell command
lists two handles; from those, this driver produces the following
(child) handle:

> 44: ConsoleOut(0) SimpleTextOut(7ED59830)   Address: 7ED59830 Attrib 07
>      mode 0: Col 80 Row 25
>      mode 1: Col -1 Row -1
>      mode 2: Col 100 Row 31
>   *  mode 3: Col 128 Row 40
> GraphicsOutput(7E6822E0)
>   Max Mode..............: 0x00000003
>   Current Mode..........: 0x00000002
>   Frame Buffer Base.....: 0x000000007E37A000
>   Frame Buffer Size.....: 0x0000000000300000
>   Mode Info Size........: 0x0000000000000024
>   Information
>     Version.............: 0x00000000
>     HorizontalResolution: 1024
>     VerticalResolution..: 768
>     Pixel Format........: PixelBlueGreenRedReserved8BitPerColor
>     Pixels / Scan Line..: 1024
>     Pixel Info
>       RedMask...........: 0x00000000
>       GreenMask.........: 0x00000000
>       BlueMask..........: 0x00000000
>   Supported Resolution List
>     Resolution[0]:
>       Horizontal........: 640
>       Vertical..........: 480
>     Resolution[1]:
>       Horizontal........: 800
>       Vertical..........: 600
>     Resolution[2]:
>       Horizontal........: 1024
>       Vertical..........: 768
> DevicePath(7ED7D598)
>   VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)/AcpiAdr(0x80010300)
>    Controller Name    : VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)/AcpiAdr(0x80010300)
>    Device Path        : VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)/AcpiAdr(0x80010300)
>    Controller Type    : BUS
>    Configuration      : NO
>    Diagnostics        : NO
>    Managed by         :
>      Drv[6C]          : Platform Console Management Driver
>      Drv[70]          : Console Splitter Driver
>      Drv[75]          : Graphics Console Driver
>    Parent Controllers :
>      Parent[43]       : VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)
>    Child Controllers  :
>      Child[73]        : Primary Console Output Device

The parent handle is, in turn:

> 43: 7ED7D118
> DevicePath(7ED7D998)
>   VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)

When we try to disconnect all drivers from the parent handle, we get:

> Shell> disconnect 43
> Disconnect - (43,7FE6F0C0,7EBBB898) Result Success.

and nothing changes in behavior. This is a good result -- the parent
handle is not bound by any UEFI driver that follows the UEFI driver
model, so there's nothing to disconnect from it.

When we try to disconnect all drivers from the child handle, we get:

> Shell> disconnect 44
> Disconnect - (44,7FE6F0C0,7EB9D598) Result Success.

and graphical updates stop (serial continues to work). In addition, the
dump for handle 44 now says:

> 44: 7ED7D698
> GraphicsOutput(7E6822E0)
>   Max Mode..............: 0x00000003
>   Current Mode..........: 0x00000002
>   Frame Buffer Base.....: 0x000000007E37A000
>   Frame Buffer Size.....: 0x0000000000300000
>   Mode Info Size........: 0x0000000000000024
>   Information
>     Version.............: 0x00000000
>     HorizontalResolution: 1024
>     VerticalResolution..: 768
>     Pixel Format........: PixelBlueGreenRedReserved8BitPerColor
>     Pixels / Scan Line..: 1024
>     Pixel Info
>       RedMask...........: 0x00000000
>       GreenMask.........: 0x00000000
>       BlueMask..........: 0x00000000
>   Supported Resolution List
>     Resolution[0]:
>       Horizontal........: 640
>       Vertical..........: 480
>     Resolution[1]:
>       Horizontal........: 800
>       Vertical..........: 600
>     Resolution[2]:
>       Horizontal........: 1024
>       Vertical..........: 768
> DevicePath(7ED7D598)
>   VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)/AcpiAdr(0x80010300)
>    Controller Name    : VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)/AcpiAdr(0x80010300)
>    Device Path        : VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)/AcpiAdr(0x80010300)
>    Controller Type    : DEVICE
>    Configuration      : NO
>    Diagnostics        : NO
>    Managed by         : <None>
>    Parent Controllers :
>      Parent[43]       : VenHw(557423A1-63AB-406C-BE7E-91CDBC08C457)
>    Child Controllers  : <None>

We see that ConPlatformDxe unbound the GOP --> the ConsoleOut protocol
interface disappeared; GraphicsConsoleDxe unbound the GOP --> the
SimpleTextOut protocol interface disappeared; and ConSplitterDxe unbound
the GOP and both afore-mentioned protocol interfaces. The GOP and the
devpath themselves remain installed though; they come from this driver
(a platform DXE driver). So, this is a good result.

After running "connect 44", the graphics window comes to life again.

I checked the absence of ramfb too, using one of my long-term
libvirtd-managed guests. The OVMF debug log says:

> Loading driver at 0x0007DAC2000 EntryPoint=0x0007DAC61DC QemuRamfbDxe.efi
> ...
> Error: Image at 0007DAC2000 start failed: Not Found

Which is good.

* testing on aarch64/KVM:

The graphics output looks great as long as I'm in the UEFI shell / the
setup TUI / the grub menu. However, once I boot a Fedora 28 Server ISO,
and the graphical Anacona Welcome screen appears, I get the exact same
display corruption as before, with QemuVideoDxe + the VGA device model.
I don't have the slightest idea why that is the case, but it's very
visible, if I quickly move the thick blue line cursor around the
language and keyboard layout selection lists. It's visible to the naked
eye how dirty memory is gradually flushed to the display.

Here's my setup:

- host hardware: APM Mustang A3
- host kernel: 4.14.0-49.2.2.el7a.aarch64
- edk2: built at cbba5ca104fb, plus your v3 (present) patches applied,
  plus the above small changes squashed
- QEMU: built at commit 778450b from your branch (see above)
- QEMU command line:

  qemu-system-aarch64 \
    -nodefaults \
    -m 2048 \
    -machine virt,accel=kvm \
    -cpu host \
    -device ramfb \
    -drive if=pflash,readonly,format=raw,file=/root/QEMU_EFI.fd.padded \
    -drive if=pflash,format=raw,file=vars21.fd \
    -chardev stdio,signal=off,mux=on,id=char0 \
    -mon chardev=char0,mode=readline \
    -serial chardev:char0 \
    -drive if=none,readonly=on,format=raw,id=cd0,file=/home/virt-images/isos/Fedora-Server-dvd-aarch64-28-1.1.iso \
    -device virtio-scsi-pci,id=scsi0 \
    -device scsi-cd,drive=cd0,bus=scsi0.0 \
    -device qemu-xhci,id=xhci1 \
    -device usb-kbd,bus=xhci1.0

If it matters, I used the SDL display type (I usually don't build the
GTK one), and I forwarded the graphics from my Mustang to my laptop via
"ssh -X -Y".

The guest kernel dmesg includes:

> [    7.330898] efifb: probing for efifb
> [    7.331670] efifb: framebuffer at 0xb8450000, using 1876k, total 1875k
> [    7.332999] efifb: mode is 800x600x32, linelength=3200, pages=1
> [    7.334177] efifb: scrolling: redraw
> [    7.334893] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0
> [    7.338207] Console: switching to colour frame buffer device 100x37
> [    7.341113] fb0: EFI VGA frame buffer device

The address b8450000 is consistent with the ArmVirtQemu log (and the
related QEMU debug message).


There's another small wart (I guess this relates more to the QEMU
series): on aarch64, I get the following warning at startup:

> Could not open option rom 'vgabios-ramfb.bin': No such file or directory


Argh, this is so frustrating! I was getting ready to push this version!
:(

Laszlo