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

[Laszlo Ersek <lersek@redhat.com>]

On 09/01/16 22:32, Jordan Justen wrote:
> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Thank you!

Series committed as b10d5ddc0385..3ef3209d3028.

Cheers,
Laszlo

> 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
>>