Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)

[Jordan Justen <jordan.l.justen@intel.com>]

Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

On 2016-08-19 05:49:21, Laszlo Ersek wrote:
> This series solves
> <https://tianocore.acgmultimedia.com/show_bug.cgi?id=66>. In particular,
> it gives AARCH64 guests running on KVM a clean, uncorrupted graphical
> console.
> 
> Anatomy of the series:
> 
> - Patches 01 and 02 fix small bugs in QemuVideoDxe and Virtio10Dxe, so
>   that virtio-vga is bound only by QemuVideoDxe, and virtio-gpu-pci is
>   bound only by Virtio10Dxe (and then, on top, by the driver being
>   posted).
> 
> - Patch 03 makes OVMF' BDS pick up virtio-gpu-pci automatically as
>   ConOut. (ArmVirtPkg already has the necessary code.) A small tweak.
> 
> - Patch 04 adds the VirtIo GPU stuff to IndustryStandard, using Gerd's
>   WIP additions to the VirtIo 1.0 spec, at
>   <https://www.kraxel.org/virtio>. (See the "GPU Device" section.) The
>   URLs are also captured in the code.
> 
> - Patches 05 through 07 add the basic (= UEFI driver model) skeleton of
>   the driver, and include it in OvmfPkg and ArmVirtPkg.
> 
>   In this series I managed to erect the driver in such a way that the
>   series doesn't only build at every stage, the driver even runs at
>   every stage (and the then-present functionality is testable). I used
>   this to test each layer of functionality in separation, during
>   development.
> 
> - Patch 08 implements the basic VirtIo GPU initialization.
> 
> - Patch 09 implements VirtIo GPU command primitives ("remote procedure
>   calls"), for performing graphics operations.
> 
> - Patch 10 implements the Graphics Output Protocol.
> 
> - Patch 11 drops PcdKludgeMapPciMmioAsCached from ArmVirtPkg.
> 
> Tests done:
> 
> - Virtio-vga regression testing on x86_64 KVM (OvmfPkgIa32X64 build).
> 
> - Repeated "disconnect" and "connect -r" checks in the UEFI shell,
>   issued (also) from the serial console (ArmVirtQemu build on AARCH64
>   KVM, OvmfPkgIa32X64 build on x86_64 KVM).
> 
> - Repeated mode switches with the "mode" command in the UEFI shell (same
>   platforms).
> 
> - Changing the preferred resolution in the Device Manager, then
>   verifying with "mode" in the UEFI shell after reboot (same platforms).
> 
> - Tested GRUB, using Fedora 23 / Fedora 24 installer ISOs (both
>   platforms).
> 
> - Also tested loading and launching Linux from GRUB. Here the results
>   differ of course: x86_64 Fedora 24 drives virtio-gpu-pci with its
>   native driver without problems, whereas AARCH64 Fedora 23 doesn't even
>   look for virtio-gpu-pci, apparently.
> 
> - Standard VGA continues to work in AARCH64 guests with TCG (tested on
>   x86_64 host). In addition, I've verified that the display corruption
>   readily reproduces when using standard VGA in AARCH64/KVM guests, at
>   the end of the series.
> 
> - Verified Driver Name and Controller Name in the UEFI shell, issuing
>   "devices", "drivers", and "dh" commands.
> 
>   Interestingly, they work perfectly with OVMF, but in ArmVirtQemu, the
>   formatted names are not displayed for *any* driver or device. Likely a
>   general problem with the ArmVirtQemu (or more generally, AARCH64)
>   build of the shell.
> 
>   I also ran into a couple of UEFI shell crashes with "devtree" and "dh
>   -d -v" (when listing all devices in the system). However, those
>   crashes reproduce identically when the series is not applied.
> 
> Testing instructions:
> 
> * OVMF guests: just add
> 
>     -device virtio-gpu-pci
> 
>   to the QEMU command line.
> 
>   For the host display, I prefer SDL when I use the raw QEMU command
>   line:
> 
>     -display sdl
> 
>   but that's entirely up to the user.
> 
> * ArmVirtQemu guests: again, add
> 
>     -device virtio-gpu-pci
> 
>   I ran my AARCH64/KVM tests on my Mustang, and for host display, there
>   I prefer VNC (when using the raw QEMU command line):
> 
>     -display vnc=:0
> 
>   Connect with "vncviewer" or another VNC client, optionally forwarded
>   over SSH.
> 
>   If you'd even like to type on the graphical console, I recommend
>   adding a USB 3.0 keyboard:
> 
>     -device nec-usb-xhci -device usb-kbd
> 
> * On both guest arches, check out the nice graphical progress bar (five
>   seconds, and you can enter the Setup utility with F2 or ESC):
> 
>     -boot menu=on,splash-time=5000
> 
> 
> ... This got to be one of my longest blurbs. Sorry about that :)
> 
> Ard: I haven't forgotten about the beer you promised! ;)
> 
> Public branch: <https://github.com/lersek/edk2/commits/virtio_gpu>.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (11):
>   OvmfPkg/QemuVideoDxe: don't incorrectly bind virtio-gpu-pci
>   OvmfPkg/Virtio10Dxe: don't bind virtio-vga
>   OvmfPkg/PlatformBootManagerLib: relax device class requirement for
>     ConOut
>   OvmfPkg/IndustryStandard: add type definitions for the virtio GPU
>     device
>   OvmfPkg/VirtioGpuDxe: introduce with Component Name 2 and Driver
>     Binding
>   OvmfPkg: include VirtioGpuDxe in the platform DSC/FDF files
>   ArmVirtPkg/ArmVirtQemu: include VirtioGpuDxe in the platform DSC/FDF
>     files
>   OvmfPkg/VirtioGpuDxe: initialize and tear down VirtIo GPU device
>   OvmfPkg/VirtioGpuDxe: provide functions for sending VirtIo GPU
>     commands
>   OvmfPkg/VirtioGpuDxe: implement EFI_GRAPHICS_OUTPUT_PROTOCOL
>   ArmVirtPkg: remove PcdKludgeMapPciMmioAsCached
> 
>  ArmVirtPkg/ArmVirtPkg.dec                            |  24 -
>  ArmVirtPkg/ArmVirtQemu.dsc                           |   4 +-
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc                 |   1 +
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                     |   4 +-
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c          |   3 +-
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf     |   1 -
>  OvmfPkg/Include/IndustryStandard/Virtio10.h          |   5 +
>  OvmfPkg/Include/IndustryStandard/VirtioGpu.h         | 216 +++++
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c |  10 +-
>  OvmfPkg/OvmfPkgIa32.dsc                              |   1 +
>  OvmfPkg/OvmfPkgIa32.fdf                              |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                           |   1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf                           |   1 +
>  OvmfPkg/OvmfPkgX64.dsc                               |   1 +
>  OvmfPkg/OvmfPkgX64.fdf                               |   1 +
>  OvmfPkg/QemuVideoDxe/Driver.c                        |   3 +
>  OvmfPkg/Virtio10Dxe/Virtio10.c                       |  18 +-
>  OvmfPkg/VirtioGpuDxe/Commands.c                      | 559 +++++++++++++
>  OvmfPkg/VirtioGpuDxe/DriverBinding.c                 | 844 ++++++++++++++++++++
>  OvmfPkg/VirtioGpuDxe/Gop.c                           | 647 +++++++++++++++
>  OvmfPkg/VirtioGpuDxe/VirtioGpu.h                     | 327 ++++++++
>  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf                   |  51 ++
>  22 files changed, 2682 insertions(+), 41 deletions(-)
>  create mode 100644 OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  create mode 100644 OvmfPkg/Include/IndustryStandard/VirtioGpu.h
>  create mode 100644 OvmfPkg/VirtioGpuDxe/VirtioGpu.h
>  create mode 100644 OvmfPkg/VirtioGpuDxe/Commands.c
>  create mode 100644 OvmfPkg/VirtioGpuDxe/DriverBinding.c
>  create mode 100644 OvmfPkg/VirtioGpuDxe/Gop.c
> 
> -- 
> 2.9.2
>