* [PATCH 01/11] OvmfPkg/QemuVideoDxe: don't incorrectly bind virtio-gpu-pci
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 02/11] OvmfPkg/Virtio10Dxe: don't bind virtio-vga Laszlo Ersek
` (13 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
The PCI (Vendor ID, Device ID) pair (0x1af4, 0x1050) stands for both the
virtio-vga and the virtio-gpu-pci device models of QEMU. They differ in
two things:
- the former has a VGA-compatibility linear framebuffer on top of the
latter,
- the former has PCI_CLASS_DISPLAY_VGA device class, while the latter has
PCI_CLASS_DISPLAY_OTHER.
In commit 94210dc95e9f ("OvmfPkg: QemuVideoDxe: add virtio-vga support"),
we enabled QemuVideoDxe to drive virtio-vga simply by adding its (Vendor
ID, Device ID) pair to gQemuVideoCardList. This change inadvertently
allowed QemuVideoDxe to bind virtio-gpu-pci, which it cannot drive though.
Restrict QemuVideoDxe to PCI_CLASS_DISPLAY_VGA, in order to exclude
virtio-gpu-pci. For the other cards that QemuVideoDxe drives, this makes
no difference. (Note that OvmfPkg's PlatformBootManagerLib instance has
always only added PCI_CLASS_DISPLAY_VGA devices to ConOut; see
DetectAndPreparePlatformPciDevicePath().)
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Fixes: 94210dc95e9f7c6ff4066a9b35a288e6f1c271bf
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/QemuVideoDxe/Driver.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 8d962b443898..fc8025ec46de 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -131,12 +131,15 @@ QemuVideoControllerDriverSupported (
);
if (EFI_ERROR (Status)) {
goto Done;
}
Status = EFI_UNSUPPORTED;
+ if (!IS_PCI_VGA (&Pci)) {
+ goto Done;
+ }
Card = QemuVideoDetect(Pci.Hdr.VendorId, Pci.Hdr.DeviceId);
if (Card != NULL) {
DEBUG ((EFI_D_INFO, "QemuVideo: %s detected\n", Card->Name));
Status = EFI_SUCCESS;
}
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 02/11] OvmfPkg/Virtio10Dxe: don't bind virtio-vga
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
2016-08-19 12:49 ` [PATCH 01/11] OvmfPkg/QemuVideoDxe: don't incorrectly bind virtio-gpu-pci Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 03/11] OvmfPkg/PlatformBootManagerLib: relax device class requirement for ConOut Laszlo Ersek
` (12 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
Commit 9399f68ae359 ("OvmfPkg: Virtio10Dxe: non-transitional driver for
virtio-1.0 PCI devices") created a "competition" between Virtio10Dxe and
QemuVideoDxe for virtio-vga devices. The binding order between these
drivers is unspecified, and the wrong order effectively breaks commit
94210dc95e9f ("OvmfPkg: QemuVideoDxe: add virtio-vga support").
Thus, never bind virtio-vga in Virtio10Dxe; QemuVideoDxe provides better
compatibility for guest OSes that insist on inheriting a linear
framebuffer. Users who prefer the VirtIo GPU interface at boot time should
specify virtio-gpu-pci, which is exactly virtio-vga, minus the VGA
compatibility (such as the framebuffer).
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Fixes: 9399f68ae359234b142c293ad1bef75f470ced30
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/Virtio10Dxe/Virtio10.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.c b/OvmfPkg/Virtio10Dxe/Virtio10.c
index 06f069907753..a8a6a58c3f1d 100644
--- a/OvmfPkg/Virtio10Dxe/Virtio10.c
+++ b/OvmfPkg/Virtio10Dxe/Virtio10.c
@@ -819,25 +819,37 @@ Virtio10BindingSupported (
Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, 0,
sizeof Pci / sizeof (UINT32), &Pci);
if (EFI_ERROR (Status)) {
goto CloseProtocol;
}
+ Status = EFI_UNSUPPORTED;
//
// Recognize non-transitional modern devices. Also, we'll have to parse the
// PCI capability list, so make sure the CapabilityPtr field will be valid.
//
if (Pci.Hdr.VendorId == VIRTIO_VENDOR_ID &&
Pci.Hdr.DeviceId >= 0x1040 &&
Pci.Hdr.DeviceId <= 0x107F &&
Pci.Hdr.RevisionID >= 0x01 &&
Pci.Device.SubsystemID >= 0x40 &&
(Pci.Hdr.Status & EFI_PCI_STATUS_CAPABILITY) != 0) {
- Status = EFI_SUCCESS;
- } else {
- Status = EFI_UNSUPPORTED;
+ //
+ // The virtio-vga device is special. It can be driven both as a VGA device
+ // with a linear framebuffer, and through its underlying, modern,
+ // virtio-gpu-pci device, which has no linear framebuffer itself. For
+ // compatibility with guest OSes that insist on inheriting a linear
+ // framebuffer from the firmware, we should leave virtio-vga to
+ // QemuVideoDxe, and support only virtio-gpu-pci here.
+ //
+ // Both virtio-vga and virtio-gpu-pci have DeviceId 0x1050, but only the
+ // former has device class PCI_CLASS_DISPLAY_VGA.
+ //
+ if (Pci.Hdr.DeviceId != 0x1050 || !IS_PCI_VGA (&Pci)) {
+ Status = EFI_SUCCESS;
+ }
}
CloseProtocol:
gBS->CloseProtocol (DeviceHandle, &gEfiPciIoProtocolGuid,
This->DriverBindingHandle, DeviceHandle);
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 03/11] OvmfPkg/PlatformBootManagerLib: relax device class requirement for ConOut
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
2016-08-19 12:49 ` [PATCH 01/11] OvmfPkg/QemuVideoDxe: don't incorrectly bind virtio-gpu-pci Laszlo Ersek
2016-08-19 12:49 ` [PATCH 02/11] OvmfPkg/Virtio10Dxe: don't bind virtio-vga Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 04/11] OvmfPkg/IndustryStandard: add type definitions for the virtio GPU device Laszlo Ersek
` (11 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
This will add virtio-gpu-pci devices to ConOut automatically.
For further benefit, the change also allows OVMF to use the legacy-free /
secondary VGA adapter (added in QEMU commit 63e3e24d, "vga: add secondary
stdvga variant") as console.
ArmVirtPkg's PlatformBootManagerLib already filters with IS_PCI_DISPLAY();
see IsPciDisplay().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Originally-suggested-by: Gerd Hoffmann <kraxel@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 351fbc1e86c7..87412598f51b 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -615,13 +615,13 @@ GetGopDevicePath (
}
return EFI_SUCCESS;
}
EFI_STATUS
-PreparePciVgaDevicePath (
+PreparePciDisplayDevicePath (
IN EFI_HANDLE DeviceHandle
)
/*++
Routine Description:
@@ -865,20 +865,20 @@ DetectAndPreparePlatformPciDevicePath (
PreparePciSerialDevicePath (Handle);
return EFI_SUCCESS;
}
}
//
- // Here we decide which VGA device to enable in PCI bus
+ // Here we decide which display device to enable in PCI bus
//
- if (IS_PCI_VGA (Pci)) {
+ if (IS_PCI_DISPLAY (Pci)) {
//
// Add them to ConOut.
//
- DEBUG ((EFI_D_INFO, "Found PCI VGA device\n"));
- PreparePciVgaDevicePath (Handle);
+ DEBUG ((EFI_D_INFO, "Found PCI display device\n"));
+ PreparePciDisplayDevicePath (Handle);
return EFI_SUCCESS;
}
return Status;
}
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 04/11] OvmfPkg/IndustryStandard: add type definitions for the virtio GPU device
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (2 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 03/11] OvmfPkg/PlatformBootManagerLib: relax device class requirement for ConOut Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 05/11] OvmfPkg/VirtioGpuDxe: introduce with Component Name 2 and Driver Binding Laszlo Ersek
` (10 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
The GPU additions to VirtIo 1.0 are a work in progress. Mark the relevant
URLs in the source code. Incorporate the absolute minimum from the WIP
spec that is necessary for implementing a GOP driver.
Add the VIRTIO_SUBSYSTEM_GPU_DEVICE macro to
"IndustryStandard/Virtio10.h", since all other such macros (dating back to
VirtIo 0.9.5) are part of "IndustryStandard/Virtio095.h".
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/Include/IndustryStandard/Virtio10.h | 5 +
OvmfPkg/Include/IndustryStandard/VirtioGpu.h | 216 ++++++++++++++++++++
2 files changed, 221 insertions(+)
diff --git a/OvmfPkg/Include/IndustryStandard/Virtio10.h b/OvmfPkg/Include/IndustryStandard/Virtio10.h
index de692c1d1ee9..4c9b62a3cf59 100644
--- a/OvmfPkg/Include/IndustryStandard/Virtio10.h
+++ b/OvmfPkg/Include/IndustryStandard/Virtio10.h
@@ -15,12 +15,17 @@
#ifndef _VIRTIO_1_0_H_
#define _VIRTIO_1_0_H_
#include <IndustryStandard/Virtio095.h>
//
+// Subsystem Device IDs (to be) introduced in VirtIo 1.0
+//
+#define VIRTIO_SUBSYSTEM_GPU_DEVICE 16
+
+//
// Structures for parsing the VirtIo 1.0 specific PCI capabilities from the
// config space
//
#pragma pack (1)
typedef struct {
UINT8 CapId; // Capability identifier (generic)
diff --git a/OvmfPkg/Include/IndustryStandard/VirtioGpu.h b/OvmfPkg/Include/IndustryStandard/VirtioGpu.h
new file mode 100644
index 000000000000..9c3516e71ee3
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/VirtioGpu.h
@@ -0,0 +1,216 @@
+/** @file
+
+ Virtio GPU Device specific type and macro definitions.
+
+ At the time of this writing, the Virtio 1.0 specification has not
+ incorporated the GPU device yet. The following work-in-progress specification
+ is used as basis for the implementation:
+
+ - https://lists.oasis-open.org/archives/virtio-dev/201605/msg00002.html
+ - https://www.kraxel.org/virtio/
+
+ This header file is minimal, and only defines the types and macros that are
+ necessary for the OvmfPkg implementation.
+
+ Copyright (C) 2016, 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.
+
+**/
+
+#ifndef _VIRTIO_GPU_H_
+#define _VIRTIO_GPU_H_
+
+#include <IndustryStandard/Virtio.h>
+
+//
+// Queue number for sending control commands.
+//
+#define VIRTIO_GPU_CONTROL_QUEUE 0
+
+//
+// Command and response types.
+//
+typedef enum {
+ //
+ // Commands related to mode setup:
+ //
+ // - create/release a host-side 2D resource,
+ //
+ VirtioGpuCmdResourceCreate2d = 0x0101,
+ VirtioGpuCmdResourceUnref = 0x0102,
+ //
+ // - attach/detach guest RAM to/from a host-side 2D resource,
+ //
+ VirtioGpuCmdResourceAttachBacking = 0x0106,
+ VirtioGpuCmdResourceDetachBacking = 0x0107,
+ //
+ // - assign/unassign a host-side 2D resource to/from a scanout ("head").
+ //
+ VirtioGpuCmdSetScanout = 0x0103,
+
+ //
+ // Commands related to drawing:
+ //
+ // - transfer a guest RAM update to the host-side 2D resource (does not imply
+ // host display refresh),
+ //
+ VirtioGpuCmdTransferToHost2d = 0x0105,
+ //
+ // - trigger a host display refresh from the 2D resource.
+ //
+ VirtioGpuCmdResourceFlush = 0x0104,
+
+ //
+ // Success code for all of the above commands.
+ //
+ VirtioGpuRespOkNodata = 0x1100,
+} VIRTIO_GPU_CONTROL_TYPE;
+
+//
+// Common request/response header.
+//
+#define VIRTIO_GPU_FLAG_FENCE BIT0
+
+#pragma pack (1)
+typedef struct {
+ //
+ // The guest sets Type to VirtioGpuCmd* in the requests. The host sets Type
+ // to VirtioGpuResp* in the responses.
+ //
+ UINT32 Type;
+
+ //
+ // Fencing forces the host to complete the command before producing a
+ // response.
+ //
+ UINT32 Flags;
+ UINT64 FenceId;
+
+ //
+ // Unused.
+ //
+ UINT32 CtxId;
+ UINT32 Padding;
+} VIRTIO_GPU_CONTROL_HEADER;
+#pragma pack ()
+
+//
+// Rectangle structure used by several operations.
+//
+#pragma pack (1)
+typedef struct {
+ UINT32 X;
+ UINT32 Y;
+ UINT32 Width;
+ UINT32 Height;
+} VIRTIO_GPU_RECTANGLE;
+#pragma pack ()
+
+//
+// Request structure for VirtioGpuCmdResourceCreate2d.
+//
+typedef enum {
+ //
+ // 32-bit depth, BGRX component order, X component ignored.
+ //
+ VirtioGpuFormatB8G8R8X8Unorm = 2,
+} VIRTIO_GPU_FORMATS;
+
+#pragma pack (1)
+typedef struct {
+ VIRTIO_GPU_CONTROL_HEADER Header;
+ UINT32 ResourceId; // note: 0 is invalid
+ UINT32 Format; // from VIRTIO_GPU_FORMATS
+ UINT32 Width;
+ UINT32 Height;
+} VIRTIO_GPU_RESOURCE_CREATE_2D;
+#pragma pack ()
+
+//
+// Request structure for VirtioGpuCmdResourceUnref.
+//
+#pragma pack (1)
+typedef struct {
+ VIRTIO_GPU_CONTROL_HEADER Header;
+ UINT32 ResourceId;
+ UINT32 Padding;
+} VIRTIO_GPU_RESOURCE_UNREF;
+#pragma pack ()
+
+//
+// Request structure for VirtioGpuCmdResourceAttachBacking.
+//
+// The spec allows for a scatter-gather list, but for simplicity we hard-code a
+// single guest buffer.
+//
+#pragma pack (1)
+typedef struct {
+ UINT64 Addr;
+ UINT32 Length;
+ UINT32 Padding;
+} VIRTIO_GPU_MEM_ENTRY;
+
+typedef struct {
+ VIRTIO_GPU_CONTROL_HEADER Header;
+ UINT32 ResourceId;
+ UINT32 NrEntries; // number of entries: constant 1
+ VIRTIO_GPU_MEM_ENTRY Entry;
+} VIRTIO_GPU_RESOURCE_ATTACH_BACKING;
+#pragma pack ()
+
+//
+// Request structure for VirtioGpuCmdResourceDetachBacking.
+//
+#pragma pack (1)
+typedef struct {
+ VIRTIO_GPU_CONTROL_HEADER Header;
+ UINT32 ResourceId;
+ UINT32 Padding;
+} VIRTIO_GPU_RESOURCE_DETACH_BACKING;
+#pragma pack ()
+
+//
+// Request structure for VirtioGpuCmdSetScanout.
+//
+#pragma pack (1)
+typedef struct {
+ VIRTIO_GPU_CONTROL_HEADER Header;
+ VIRTIO_GPU_RECTANGLE Rectangle;
+ UINT32 ScanoutId;
+ UINT32 ResourceId;
+} VIRTIO_GPU_SET_SCANOUT;
+#pragma pack ()
+
+//
+// Request structure for VirtioGpuCmdTransferToHost2d.
+//
+#pragma pack (1)
+typedef struct {
+ VIRTIO_GPU_CONTROL_HEADER Header;
+ VIRTIO_GPU_RECTANGLE Rectangle;
+ UINT64 Offset;
+ UINT32 ResourceId;
+ UINT32 Padding;
+} VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D;
+#pragma pack ()
+
+//
+// Request structure for VirtioGpuCmdResourceFlush.
+//
+#pragma pack (1)
+typedef struct {
+ VIRTIO_GPU_CONTROL_HEADER Header;
+ VIRTIO_GPU_RECTANGLE Rectangle;
+ UINT32 ResourceId;
+ UINT32 Padding;
+} VIRTIO_GPU_RESOURCE_FLUSH;
+#pragma pack ()
+
+#endif // _VIRTIO_GPU_H_
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 05/11] OvmfPkg/VirtioGpuDxe: introduce with Component Name 2 and Driver Binding
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (3 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 04/11] OvmfPkg/IndustryStandard: add type definitions for the virtio GPU device Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 06/11] OvmfPkg: include VirtioGpuDxe in the platform DSC/FDF files Laszlo Ersek
` (9 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
This patch adds the skeleton of the driver: it implements the Component
Name 2 Protocol and the Driver Binding Protocol, in accordance with the
generic and GOP-specific requirements set forth in the UEFI spec and the
Driver Writers' Guide.
The basic idea is that VGPU_DEV abstracts the virtio GPU device, while the
single VGPU_GOP that we intend to support at this point stands for "head"
(aka "scanout") #0.
For now, the Virtio Device Protocol is only used for driver binding; no
actual virtio operations are done yet. Similarly, we use a "dummy" GOP
GUID and protocol structure (a plain UINT8 object) for now, so that
GOP-consuming drivers don't look at what we produce just yet.
The driver is a bit different from the other virtio device drivers written
thus far:
- It implements the GetControllerName() member of the Component Name 2
Protocol. (Formatting helpful names is recommended by UEFI.) As a "best
effort", we format the PCI BDF into the name (a PCI backend is not
guaranteed by VIRTIO_DEVICE_PROTOCOL). It should provide a more friendly
experience in the shell and elsewhere.
- This driver seeks to support all RemainingDevicePath cases:
- NULL: produce all (= one) child handles (= VGPU_GOP heads) at once,
- End of Device Path Node: produce no child handles,
- specific ACPI ADR Node: check if it's supportable, and produce it
(only one specific child controller is supported).
This is one of the reasons for separating VGPU_GOP from VGPU_DEV.
The driver is a hybrid driver: it produces both child handles (one, to be
exact), but also installs a structure (VGPU_DEV) directly on the VirtIo
controller handle, using gEfiCallerIdGuid as protocol GUID. This is a
trick I've seen elsewhere in edk2 (for example, TerminalDxe), and it is
necessary for the following reason:
In EFI_COMPONENT_NAME2_PROTOCOL.GetControllerName(), we must be able to
"cast down" a VirtIo ControllerHandle to our own private data structure
(VGPU_DEV). That's only possible if we install the structure directly on
the VirtIo ControllerHandle (thereby rendering the driver a hybrid
driver), because a child controller with our GOP implementation on it may
not exist / be passed in there.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf | 47 ++
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 106 +++
OvmfPkg/VirtioGpuDxe/DriverBinding.c | 831 ++++++++++++++++++++
3 files changed, 984 insertions(+)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf b/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
new file mode 100644
index 000000000000..948350dbce21
--- /dev/null
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
@@ -0,0 +1,47 @@
+## @file
+#
+# This hybrid driver produces the Graphics Output Protocol for the Virtio GPU
+# device (head #0, only and unconditionally).
+#
+# Copyright (C) 2016, 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.
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = VirtioGpuDxe
+ FILE_GUID = D6099B94-CD97-4CC5-8714-7F6312701A8A
+ MODULE_TYPE = UEFI_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = VirtioGpuEntryPoint
+
+[Sources]
+ DriverBinding.c
+ VirtioGpu.h
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ DevicePathLib
+ MemoryAllocationLib
+ PrintLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[Protocols]
+ gEfiDevicePathProtocolGuid ## TO_START ## BY_START
+ gEfiPciIoProtocolGuid ## TO_START
+ gVirtioDeviceProtocolGuid ## TO_START
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
new file mode 100644
index 000000000000..ca5805df8442
--- /dev/null
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -0,0 +1,106 @@
+/** @file
+
+ Internal type and macro definitions for the Virtio GPU hybrid driver.
+
+ Copyright (C) 2016, 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.
+
+**/
+
+#ifndef _VIRTIO_GPU_DXE_H_
+#define _VIRTIO_GPU_DXE_H_
+
+#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
+#include <Protocol/VirtioDevice.h>
+
+//
+// Forward declaration of VGPU_GOP.
+//
+typedef struct VGPU_GOP_STRUCT VGPU_GOP;
+
+//
+// The abstraction that directly corresponds to a Virtio GPU device.
+//
+// This structure will be installed on the handle that has the VirtIo Device
+// Protocol interface, with GUID gEfiCallerIdGuid. A similar trick is employed
+// in TerminalDxe, and it is necessary so that we can look up VGPU_DEV just
+// from the VirtIo Device Protocol handle in the Component Name 2 Protocol
+// implementation.
+//
+typedef struct {
+ //
+ // VirtIo represents access to the Virtio GPU device. Never NULL.
+ //
+ VIRTIO_DEVICE_PROTOCOL *VirtIo;
+
+ //
+ // BusName carries a customized name for
+ // EFI_COMPONENT_NAME2_PROTOCOL.GetControllerName(). It is expressed in table
+ // form because it can theoretically support several languages. Never NULL.
+ //
+ EFI_UNICODE_STRING_TABLE *BusName;
+
+ //
+ // The Child field references the GOP wrapper structure. If this pointer is
+ // NULL, then the hybrid driver has bound (i.e., started) the
+ // VIRTIO_DEVICE_PROTOCOL controller without producing the child GOP
+ // controller (that is, after Start() was called with RemainingDevicePath
+ // pointing to and End of Device Path node). Child can be created and
+ // destroyed, even repeatedly, independently of VGPU_DEV.
+ //
+ // In practice, this field represents the single head (scanout) that we
+ // support.
+ //
+ VGPU_GOP *Child;
+} VGPU_DEV;
+
+//
+// The Graphics Output Protocol wrapper structure.
+//
+#define VGPU_GOP_SIG SIGNATURE_64 ('V', 'G', 'P', 'U', '_', 'G', 'O', 'P')
+
+struct VGPU_GOP_STRUCT {
+ UINT64 Signature;
+
+ //
+ // ParentBus points to the parent VGPU_DEV object. Never NULL.
+ //
+ VGPU_DEV *ParentBus;
+
+ //
+ // GopName carries a customized name for
+ // EFI_COMPONENT_NAME2_PROTOCOL.GetControllerName(). It is expressed in table
+ // form because it can theoretically support several languages. Never NULL.
+ //
+ EFI_UNICODE_STRING_TABLE *GopName;
+
+ //
+ // GopHandle is the UEFI child handle that carries the device path ending
+ // with the ACPI ADR node, and the Graphics Output Protocol. Never NULL.
+ //
+ EFI_HANDLE GopHandle;
+
+ //
+ // The GopDevicePath field is the device path installed on GopHandle,
+ // ending with an ACPI ADR node. Never NULL.
+ //
+ EFI_DEVICE_PATH_PROTOCOL *GopDevicePath;
+
+ //
+ // The Gop field is installed on the child handle as Graphics Output Protocol
+ // interface.
+ //
+ // For now it is just a placeholder.
+ //
+ UINT8 Gop;
+};
+
+#endif // _VIRTIO_GPU_DXE_H_
diff --git a/OvmfPkg/VirtioGpuDxe/DriverBinding.c b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
new file mode 100644
index 000000000000..b902a07871e0
--- /dev/null
+++ b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
@@ -0,0 +1,831 @@
+/** @file
+
+ Implement the Driver Binding Protocol and the Component Name 2 Protocol for
+ the Virtio GPU hybrid driver.
+
+ Copyright (C) 2016, 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.
+
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PrintLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Protocol/ComponentName2.h>
+#include <Protocol/DevicePath.h>
+#include <Protocol/DriverBinding.h>
+#include <Protocol/PciIo.h>
+
+#include "VirtioGpu.h"
+
+//
+// Dummy Graphics Output Protocol GUID: a temporary placeholder for the EFI
+// counterpart. It will be replaced with the real thing as soon as we implement
+// the EFI GOP. Refer to VGPU_GOP.Gop.
+//
+STATIC EFI_GUID mDummyGraphicsOutputProtocolGuid = {
+ 0x4983f8dc, 0x2782, 0x415b,
+ { 0x91, 0xf5, 0x2c, 0xeb, 0x48, 0x4a, 0x0f, 0xe9 }
+};
+
+//
+// The device path node that describes the Video Output Device Attributes for
+// the single head (UEFI child handle) that we support.
+//
+// The ACPI_DISPLAY_ADR() macro corresponds to Table B-2, section "B.4.2 _DOD"
+// in the ACPI 3.0b spec, or more recently, to Table B-379, section "B.3.2
+// _DOD" in the ACPI 6.0 spec.
+//
+STATIC CONST ACPI_ADR_DEVICE_PATH mAcpiAdr = {
+ { // Header
+ ACPI_DEVICE_PATH, // Type
+ ACPI_ADR_DP, // SubType
+ { sizeof mAcpiAdr, 0 }, // Length
+ },
+ ACPI_DISPLAY_ADR ( // ADR
+ 1, // DeviceIdScheme: use the ACPI
+ // bit-field definitions
+ 0, // HeadId
+ 0, // NonVgaOutput
+ 1, // BiosCanDetect
+ 0, // VendorInfo
+ ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL, // Type
+ 0, // Port
+ 0 // Index
+ )
+};
+
+//
+// Component Name 2 Protocol implementation.
+//
+STATIC CONST EFI_UNICODE_STRING_TABLE mDriverNameTable[] = {
+ { "en", L"Virtio GPU Driver" },
+ { NULL, NULL }
+};
+
+STATIC
+EFI_STATUS
+EFIAPI
+VirtioGpuGetDriverName (
+ IN EFI_COMPONENT_NAME2_PROTOCOL *This,
+ IN CHAR8 *Language,
+ OUT CHAR16 **DriverName
+ )
+{
+ return LookupUnicodeString2 (Language, This->SupportedLanguages,
+ mDriverNameTable, DriverName, FALSE /* Iso639Language */);
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+VirtioGpuGetControllerName (
+ IN EFI_COMPONENT_NAME2_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_HANDLE ChildHandle OPTIONAL,
+ IN CHAR8 *Language,
+ OUT CHAR16 **ControllerName
+ )
+{
+ EFI_STATUS Status;
+ VGPU_DEV *VgpuDev;
+
+ //
+ // Look up the VGPU_DEV "protocol interface" on ControllerHandle.
+ //
+ Status = gBS->OpenProtocol (ControllerHandle, &gEfiCallerIdGuid,
+ (VOID **)&VgpuDev, gImageHandle, ControllerHandle,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ //
+ // Sanity check: if we found gEfiCallerIdGuid on ControllerHandle, then we
+ // keep its Virtio Device Protocol interface open BY_DRIVER.
+ //
+ ASSERT_EFI_ERROR (EfiTestManagedDevice (ControllerHandle, gImageHandle,
+ &gVirtioDeviceProtocolGuid));
+
+ if (ChildHandle == NULL) {
+ //
+ // The caller is querying the name of the VGPU_DEV controller.
+ //
+ return LookupUnicodeString2 (Language, This->SupportedLanguages,
+ VgpuDev->BusName, ControllerName, FALSE /* Iso639Language */);
+ }
+
+ //
+ // Otherwise, the caller is looking for the name of the GOP child controller.
+ // Check if it is asking about the GOP child controller that we manage. (The
+ // condition below covers the case when we haven't produced the GOP child
+ // controller yet, or we've destroyed it since.)
+ //
+ if (VgpuDev->Child == NULL || ChildHandle != VgpuDev->Child->GopHandle) {
+ return EFI_UNSUPPORTED;
+ }
+ //
+ // Sanity check: our GOP child controller keeps the VGPU_DEV controller's
+ // Virtio Device Protocol interface open BY_CHILD_CONTROLLER.
+ //
+ ASSERT_EFI_ERROR (EfiTestChildHandle (ControllerHandle, ChildHandle,
+ &gVirtioDeviceProtocolGuid));
+
+ return LookupUnicodeString2 (Language, This->SupportedLanguages,
+ VgpuDev->Child->GopName, ControllerName,
+ FALSE /* Iso639Language */);
+}
+
+STATIC CONST EFI_COMPONENT_NAME2_PROTOCOL mComponentName2 = {
+ VirtioGpuGetDriverName,
+ VirtioGpuGetControllerName,
+ "en" // SupportedLanguages (RFC 4646)
+};
+
+//
+// Helper functions for the Driver Binding Protocol Implementation.
+//
+/**
+ Format the VGPU_DEV controller name, to be looked up and returned by
+ VirtioGpuGetControllerName().
+
+ @param[in] ControllerHandle The handle that identifies the VGPU_DEV
+ controller.
+
+ @param[in] AgentHandle The handle of the agent that will attempt to
+ temporarily open the PciIo protocol. This is the
+ DriverBindingHandle member of the
+ EFI_DRIVER_BINDING_PROTOCOL whose Start()
+ function is calling this function.
+
+ @param[in] DevicePath The device path that is installed on
+ ControllerHandle.
+
+ @param[out] ControllerName A dynamically allocated unicode string that
+ unconditionally says "Virtio GPU Device", with a
+ PCI Segment:Bus:Device.Function location
+ optionally appended. The latter part is only
+ produced if DevicePath contains at least one
+ PciIo node; in that case, the most specific such
+ node is used for retrieving the location info.
+ The caller is responsible for freeing
+ ControllerName after use.
+
+ @retval EFI_SUCCESS ControllerName has been formatted.
+
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory for ControllerName.
+**/
+STATIC
+EFI_STATUS
+FormatVgpuDevName (
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_HANDLE AgentHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *DevicePath,
+ OUT CHAR16 **ControllerName
+ )
+{
+ EFI_HANDLE PciIoHandle;
+ EFI_PCI_IO_PROTOCOL *PciIo;
+ UINTN Segment, Bus, Device, Function;
+ STATIC CONST CHAR16 ControllerNameStem[] = L"Virtio GPU Device";
+ UINTN ControllerNameSize;
+
+ if (EFI_ERROR (gBS->LocateDevicePath (&gEfiPciIoProtocolGuid, &DevicePath,
+ &PciIoHandle)) ||
+ EFI_ERROR (gBS->OpenProtocol (PciIoHandle, &gEfiPciIoProtocolGuid,
+ (VOID **)&PciIo, AgentHandle, ControllerHandle,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL)) ||
+ EFI_ERROR (PciIo->GetLocation (PciIo, &Segment, &Bus, &Device,
+ &Function))) {
+ //
+ // Failed to retrieve location info, return verbatim copy of static string.
+ //
+ *ControllerName = AllocateCopyPool (sizeof ControllerNameStem,
+ ControllerNameStem);
+ return (*ControllerName == NULL) ? EFI_OUT_OF_RESOURCES : EFI_SUCCESS;
+ }
+ //
+ // Location info available, format ControllerName dynamically.
+ //
+ ControllerNameSize = sizeof ControllerNameStem + // includes L'\0'
+ sizeof (CHAR16) * (1 + 4 + // Segment
+ 1 + 2 + // Bus
+ 1 + 2 + // Device
+ 1 + 1 // Function
+ );
+ *ControllerName = AllocatePool (ControllerNameSize);
+ if (*ControllerName == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ UnicodeSPrintAsciiFormat (*ControllerName, ControllerNameSize,
+ "%s %04x:%02x:%02x.%x", ControllerNameStem, (UINT32)Segment, (UINT32)Bus,
+ (UINT32)Device, (UINT32)Function);
+ return EFI_SUCCESS;
+}
+
+/**
+ Dynamically allocate and initialize the VGPU_GOP child object within an
+ otherwise configured parent VGPU_DEV object.
+
+ This function adds a BY_CHILD_CONTROLLER reference to ParentBusController's
+ VIRTIO_DEVICE_PROTOCOL interface.
+
+ @param[in,out] ParentBus The pre-initialized VGPU_DEV object that the
+ newly created VGPU_GOP object will be the
+ child of.
+
+ @param[in] ParentDevicePath The device path protocol instance that is
+ installed on ParentBusController.
+
+ @param[in] ParentBusController The UEFI controller handle on which the
+ ParentBus VGPU_DEV object and the
+ ParentDevicePath device path protocol are
+ installed.
+
+ @param[in] DriverBindingHandle The DriverBindingHandle member of
+ EFI_DRIVER_BINDING_PROTOCOL whose Start()
+ function is calling this function. It is
+ passed as AgentHandle to gBS->OpenProtocol()
+ when creating the BY_CHILD_CONTROLLER
+ reference.
+
+ @retval EFI_SUCCESS ParentBus->Child has been created and
+ populated, and ParentBus->Child->GopHandle now
+ references ParentBusController->VirtIo
+ BY_CHILD_CONTROLLER.
+
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failed.
+
+ @return Error codes from underlying functions.
+**/
+STATIC
+EFI_STATUS
+InitVgpuGop (
+ IN OUT VGPU_DEV *ParentBus,
+ IN EFI_DEVICE_PATH_PROTOCOL *ParentDevicePath,
+ IN EFI_HANDLE ParentBusController,
+ IN EFI_HANDLE DriverBindingHandle
+ )
+{
+ VGPU_GOP *VgpuGop;
+ EFI_STATUS Status;
+ CHAR16 *ParentBusName;
+ STATIC CONST CHAR16 NameSuffix[] = L" Head #0";
+ UINTN NameSize;
+ CHAR16 *Name;
+ EFI_TPL OldTpl;
+ VOID *ParentVirtIo;
+
+ VgpuGop = AllocateZeroPool (sizeof *VgpuGop);
+ if (VgpuGop == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ VgpuGop->Signature = VGPU_GOP_SIG;
+ VgpuGop->ParentBus = ParentBus;
+
+ //
+ // Format a human-readable controller name for VGPU_GOP, and stash it for
+ // VirtioGpuGetControllerName() to look up. We simply append NameSuffix to
+ // ParentBus->BusName.
+ //
+ Status = LookupUnicodeString2 ("en", mComponentName2.SupportedLanguages,
+ ParentBus->BusName, &ParentBusName, FALSE /* Iso639Language */);
+ ASSERT_EFI_ERROR (Status);
+ NameSize = StrSize (ParentBusName) - sizeof (CHAR16) + sizeof NameSuffix;
+ Name = AllocatePool (NameSize);
+ if (Name == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeVgpuGop;
+ }
+ UnicodeSPrintAsciiFormat (Name, NameSize, "%s%s", ParentBusName, NameSuffix);
+ Status = AddUnicodeString2 ("en", mComponentName2.SupportedLanguages,
+ &VgpuGop->GopName, Name, FALSE /* Iso639Language */);
+ FreePool (Name);
+ if (EFI_ERROR (Status)) {
+ goto FreeVgpuGop;
+ }
+
+ //
+ // Create the child device path.
+ //
+ VgpuGop->GopDevicePath = AppendDevicePathNode (ParentDevicePath,
+ &mAcpiAdr.Header);
+ if (VgpuGop->GopDevicePath == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto FreeVgpuGopName;
+ }
+
+ //
+ // Mask protocol notify callbacks until we're done.
+ //
+ OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+
+ //
+ // Create the child handle with the child device path.
+ //
+ Status = gBS->InstallProtocolInterface (&VgpuGop->GopHandle,
+ &gEfiDevicePathProtocolGuid, EFI_NATIVE_INTERFACE,
+ VgpuGop->GopDevicePath);
+ if (EFI_ERROR (Status)) {
+ goto FreeDevicePath;
+ }
+
+ //
+ // The child handle must present a reference to the parent handle's Virtio
+ // Device Protocol interface.
+ //
+ Status = gBS->OpenProtocol (ParentBusController, &gVirtioDeviceProtocolGuid,
+ &ParentVirtIo, DriverBindingHandle, VgpuGop->GopHandle,
+ EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER);
+ if (EFI_ERROR (Status)) {
+ goto UninstallDevicePath;
+ }
+ ASSERT (ParentVirtIo == ParentBus->VirtIo);
+
+ //
+ // Initialize our Graphics Output Protocol.
+ //
+ // This means "nothing" for now.
+ //
+ Status = EFI_SUCCESS;
+ if (EFI_ERROR (Status)) {
+ goto CloseVirtIoByChild;
+ }
+
+ //
+ // Install the Graphics Output Protocol on the child handle.
+ //
+ Status = gBS->InstallProtocolInterface (&VgpuGop->GopHandle,
+ &mDummyGraphicsOutputProtocolGuid, EFI_NATIVE_INTERFACE,
+ &VgpuGop->Gop);
+ if (EFI_ERROR (Status)) {
+ goto UninitGop;
+ }
+
+ //
+ // We're done.
+ //
+ gBS->RestoreTPL (OldTpl);
+ ParentBus->Child = VgpuGop;
+ return EFI_SUCCESS;
+
+UninitGop:
+ //
+ // Nothing, for now.
+ //
+
+CloseVirtIoByChild:
+ gBS->CloseProtocol (ParentBusController, &gVirtioDeviceProtocolGuid,
+ DriverBindingHandle, VgpuGop->GopHandle);
+
+UninstallDevicePath:
+ gBS->UninstallProtocolInterface (VgpuGop->GopHandle,
+ &gEfiDevicePathProtocolGuid, VgpuGop->GopDevicePath);
+
+FreeDevicePath:
+ gBS->RestoreTPL (OldTpl);
+ FreePool (VgpuGop->GopDevicePath);
+
+FreeVgpuGopName:
+ FreeUnicodeStringTable (VgpuGop->GopName);
+
+FreeVgpuGop:
+ FreePool (VgpuGop);
+
+ return Status;
+}
+
+/**
+ Tear down and release the VGPU_GOP child object within the VGPU_DEV parent
+ object.
+
+ This function removes the BY_CHILD_CONTROLLER reference from
+ ParentBusController's VIRTIO_DEVICE_PROTOCOL interface.
+
+ @param[in,out] ParentBus The VGPU_DEV object that the VGPU_GOP child
+ object will be removed from.
+
+ @param[in] ParentBusController The UEFI controller handle on which the
+ ParentBus VGPU_DEV object is installed.
+
+ @param[in] DriverBindingHandle The DriverBindingHandle member of
+ EFI_DRIVER_BINDING_PROTOCOL whose Stop()
+ function is calling this function. It is
+ passed as AgentHandle to gBS->CloseProtocol()
+ when removing the BY_CHILD_CONTROLLER
+ reference.
+**/
+STATIC
+VOID
+UninitVgpuGop (
+ IN OUT VGPU_DEV *ParentBus,
+ IN EFI_HANDLE ParentBusController,
+ IN EFI_HANDLE DriverBindingHandle
+ )
+{
+ VGPU_GOP *VgpuGop;
+ EFI_STATUS Status;
+
+ VgpuGop = ParentBus->Child;
+ Status = gBS->UninstallProtocolInterface (VgpuGop->GopHandle,
+ &mDummyGraphicsOutputProtocolGuid, &VgpuGop->Gop);
+ ASSERT_EFI_ERROR (Status);
+
+ //
+ // Uninitialize VgpuGop->Gop.
+ //
+ // Nothing, for now.
+ //
+ Status = EFI_SUCCESS;
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gBS->CloseProtocol (ParentBusController, &gVirtioDeviceProtocolGuid,
+ DriverBindingHandle, VgpuGop->GopHandle);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = gBS->UninstallProtocolInterface (VgpuGop->GopHandle,
+ &gEfiDevicePathProtocolGuid, VgpuGop->GopDevicePath);
+ ASSERT_EFI_ERROR (Status);
+
+ FreePool (VgpuGop->GopDevicePath);
+ FreeUnicodeStringTable (VgpuGop->GopName);
+ FreePool (VgpuGop);
+
+ ParentBus->Child = NULL;
+}
+
+//
+// Driver Binding Protocol Implementation.
+//
+STATIC
+EFI_STATUS
+EFIAPI
+VirtioGpuDriverBindingSupported (
+ IN EFI_DRIVER_BINDING_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ VIRTIO_DEVICE_PROTOCOL *VirtIo;
+
+ //
+ // - If RemainingDevicePath is NULL: the caller is interested in creating all
+ // child handles.
+ // - If RemainingDevicePath points to an end node: the caller is not
+ // interested in creating any child handle.
+ // - Otherwise, the caller would like to create the one child handle
+ // specified in RemainingDevicePath. In this case we have to see if the
+ // requested device path is supportable.
+ //
+ if (RemainingDevicePath != NULL &&
+ !IsDevicePathEnd (RemainingDevicePath) &&
+ (DevicePathNodeLength (RemainingDevicePath) != sizeof mAcpiAdr ||
+ CompareMem (RemainingDevicePath, &mAcpiAdr, sizeof mAcpiAdr) != 0)) {
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Open the Virtio Device Protocol interface on the controller, BY_DRIVER.
+ //
+ Status = gBS->OpenProtocol (ControllerHandle, &gVirtioDeviceProtocolGuid,
+ (VOID **)&VirtIo, This->DriverBindingHandle,
+ ControllerHandle, EFI_OPEN_PROTOCOL_BY_DRIVER);
+ if (EFI_ERROR (Status)) {
+ //
+ // If this fails, then by default we cannot support ControllerHandle. There
+ // is one exception: we've already bound the device, have not produced any
+ // GOP child controller, and now the caller wants us to produce the child
+ // controller (either specifically or as part of "all children"). That's
+ // allowed.
+ //
+ if (Status == EFI_ALREADY_STARTED) {
+ EFI_STATUS Status2;
+ VGPU_DEV *VgpuDev;
+
+ Status2 = gBS->OpenProtocol (ControllerHandle, &gEfiCallerIdGuid,
+ (VOID **)&VgpuDev, This->DriverBindingHandle,
+ ControllerHandle, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ ASSERT_EFI_ERROR (Status2);
+
+ if (VgpuDev->Child == NULL &&
+ (RemainingDevicePath == NULL ||
+ !IsDevicePathEnd (RemainingDevicePath))) {
+ Status = EFI_SUCCESS;
+ }
+ }
+
+ return Status;
+ }
+
+ //
+ // First BY_DRIVER open; check the VirtIo revision and subsystem.
+ //
+ if (VirtIo->Revision < VIRTIO_SPEC_REVISION (1, 0, 0) ||
+ VirtIo->SubSystemDeviceId != VIRTIO_SUBSYSTEM_GPU_DEVICE) {
+ Status = EFI_UNSUPPORTED;
+ goto CloseVirtIo;
+ }
+
+ //
+ // We'll need the device path of the VirtIo device both for formatting
+ // VGPU_DEV.BusName and for populating VGPU_GOP.GopDevicePath.
+ //
+ Status = gBS->OpenProtocol (ControllerHandle, &gEfiDevicePathProtocolGuid,
+ NULL, This->DriverBindingHandle, ControllerHandle,
+ EFI_OPEN_PROTOCOL_TEST_PROTOCOL);
+
+CloseVirtIo:
+ gBS->CloseProtocol (ControllerHandle, &gVirtioDeviceProtocolGuid,
+ This->DriverBindingHandle, ControllerHandle);
+
+ return Status;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+VirtioGpuDriverBindingStart (
+ IN EFI_DRIVER_BINDING_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN EFI_DEVICE_PATH_PROTOCOL *RemainingDevicePath OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ VIRTIO_DEVICE_PROTOCOL *VirtIo;
+ BOOLEAN VirtIoBoundJustNow;
+ VGPU_DEV *VgpuDev;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+
+ //
+ // Open the Virtio Device Protocol.
+ //
+ // The result of this operation, combined with the checks in
+ // VirtioGpuDriverBindingSupported(), uniquely tells us whether we are
+ // binding the VirtIo controller on this call (with or without creating child
+ // controllers), or else we're *only* creating child controllers.
+ //
+ Status = gBS->OpenProtocol (ControllerHandle, &gVirtioDeviceProtocolGuid,
+ (VOID **)&VirtIo, This->DriverBindingHandle,
+ ControllerHandle, EFI_OPEN_PROTOCOL_BY_DRIVER);
+ if (EFI_ERROR (Status)) {
+ //
+ // The assertions below are based on the success of
+ // VirtioGpuDriverBindingSupported(): we bound ControllerHandle earlier,
+ // without producing child handles, and now we're producing the GOP child
+ // handle only.
+ //
+ ASSERT (Status == EFI_ALREADY_STARTED);
+
+ Status = gBS->OpenProtocol (ControllerHandle, &gEfiCallerIdGuid,
+ (VOID **)&VgpuDev, This->DriverBindingHandle,
+ ControllerHandle, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ ASSERT_EFI_ERROR (Status);
+
+ ASSERT (VgpuDev->Child == NULL);
+ ASSERT (
+ RemainingDevicePath == NULL || !IsDevicePathEnd (RemainingDevicePath));
+
+ VirtIoBoundJustNow = FALSE;
+ } else {
+ VirtIoBoundJustNow = TRUE;
+
+ //
+ // Allocate the private structure.
+ //
+ VgpuDev = AllocateZeroPool (sizeof *VgpuDev);
+ if (VgpuDev == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto CloseVirtIo;
+ }
+ VgpuDev->VirtIo = VirtIo;
+ }
+
+ //
+ // Grab the VirtIo controller's device path. This is necessary regardless of
+ // VirtIoBoundJustNow.
+ //
+ Status = gBS->OpenProtocol (ControllerHandle, &gEfiDevicePathProtocolGuid,
+ (VOID **)&DevicePath, This->DriverBindingHandle,
+ ControllerHandle, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ if (EFI_ERROR (Status)) {
+ goto FreeVgpuDev;
+ }
+
+ //
+ // Create VGPU_DEV if we've bound the VirtIo controller right now (that is,
+ // if we aren't *only* creating child handles).
+ //
+ if (VirtIoBoundJustNow) {
+ CHAR16 *VgpuDevName;
+
+ //
+ // Format a human-readable controller name for VGPU_DEV, and stash it for
+ // VirtioGpuGetControllerName() to look up.
+ //
+ Status = FormatVgpuDevName (ControllerHandle, This->DriverBindingHandle,
+ DevicePath, &VgpuDevName);
+ if (EFI_ERROR (Status)) {
+ goto FreeVgpuDev;
+ }
+ Status = AddUnicodeString2 ("en", mComponentName2.SupportedLanguages,
+ &VgpuDev->BusName, VgpuDevName, FALSE /* Iso639Language */);
+ FreePool (VgpuDevName);
+ if (EFI_ERROR (Status)) {
+ goto FreeVgpuDev;
+ }
+
+ //
+ // Install the VGPU_DEV "protocol interface" on ControllerHandle.
+ //
+ Status = gBS->InstallProtocolInterface (&ControllerHandle,
+ &gEfiCallerIdGuid, EFI_NATIVE_INTERFACE, VgpuDev);
+ if (EFI_ERROR (Status)) {
+ goto FreeVgpuDevBusName;
+ }
+
+ if (RemainingDevicePath != NULL && IsDevicePathEnd (RemainingDevicePath)) {
+ //
+ // No child handle should be produced; we're done.
+ //
+ DEBUG ((EFI_D_INFO, "%a: bound VirtIo=%p without producing GOP\n",
+ __FUNCTION__, (VOID *)VgpuDev->VirtIo));
+ return EFI_SUCCESS;
+ }
+ }
+
+ //
+ // Below we'll produce our single child handle: the caller requested it
+ // either specifically, or as part of all child handles.
+ //
+ ASSERT (VgpuDev->Child == NULL);
+ ASSERT (
+ RemainingDevicePath == NULL || !IsDevicePathEnd (RemainingDevicePath));
+
+ Status = InitVgpuGop (VgpuDev, DevicePath, ControllerHandle,
+ This->DriverBindingHandle);
+ if (EFI_ERROR (Status)) {
+ goto UninstallVgpuDev;
+ }
+
+ //
+ // We're done.
+ //
+ DEBUG ((EFI_D_INFO, "%a: produced GOP %a VirtIo=%p\n", __FUNCTION__,
+ VirtIoBoundJustNow ? "while binding" : "for pre-bound",
+ (VOID *)VgpuDev->VirtIo));
+ return EFI_SUCCESS;
+
+UninstallVgpuDev:
+ if (VirtIoBoundJustNow) {
+ gBS->UninstallProtocolInterface (ControllerHandle, &gEfiCallerIdGuid,
+ VgpuDev);
+ }
+
+FreeVgpuDevBusName:
+ if (VirtIoBoundJustNow) {
+ FreeUnicodeStringTable (VgpuDev->BusName);
+ }
+
+FreeVgpuDev:
+ if (VirtIoBoundJustNow) {
+ FreePool (VgpuDev);
+ }
+
+CloseVirtIo:
+ if (VirtIoBoundJustNow) {
+ gBS->CloseProtocol (ControllerHandle, &gVirtioDeviceProtocolGuid,
+ This->DriverBindingHandle, ControllerHandle);
+ }
+
+ return Status;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+VirtioGpuDriverBindingStop (
+ IN EFI_DRIVER_BINDING_PROTOCOL *This,
+ IN EFI_HANDLE ControllerHandle,
+ IN UINTN NumberOfChildren,
+ IN EFI_HANDLE *ChildHandleBuffer OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ VGPU_DEV *VgpuDev;
+
+ //
+ // Look up the VGPU_DEV "protocol interface" on ControllerHandle.
+ //
+ Status = gBS->OpenProtocol (ControllerHandle, &gEfiCallerIdGuid,
+ (VOID **)&VgpuDev, This->DriverBindingHandle,
+ ControllerHandle, EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ //
+ // Sanity check: if we found gEfiCallerIdGuid on ControllerHandle, then we
+ // keep its Virtio Device Protocol interface open BY_DRIVER.
+ //
+ ASSERT_EFI_ERROR (EfiTestManagedDevice (ControllerHandle,
+ This->DriverBindingHandle, &gVirtioDeviceProtocolGuid));
+
+ switch (NumberOfChildren) {
+ case 0:
+ //
+ // The caller wants us to unbind the VirtIo controller.
+ //
+ if (VgpuDev->Child != NULL) {
+ //
+ // We still have the GOP child.
+ //
+ Status = EFI_DEVICE_ERROR;
+ break;
+ }
+
+ DEBUG ((EFI_D_INFO, "%a: unbinding GOP-less VirtIo=%p\n", __FUNCTION__,
+ (VOID *)VgpuDev->VirtIo));
+
+ Status = gBS->UninstallProtocolInterface (ControllerHandle,
+ &gEfiCallerIdGuid, VgpuDev);
+ ASSERT_EFI_ERROR (Status);
+
+ FreeUnicodeStringTable (VgpuDev->BusName);
+ FreePool (VgpuDev);
+
+ Status = gBS->CloseProtocol (ControllerHandle, &gVirtioDeviceProtocolGuid,
+ This->DriverBindingHandle, ControllerHandle);
+ ASSERT_EFI_ERROR (Status);
+ break;
+
+ case 1:
+ //
+ // The caller wants us to destroy our child GOP controller.
+ //
+ if (VgpuDev->Child == NULL ||
+ ChildHandleBuffer[0] != VgpuDev->Child->GopHandle) {
+ //
+ // We have no child controller at the moment, or it differs from the one
+ // the caller wants us to destroy. I.e., we don't own the child
+ // controller passed in.
+ //
+ Status = EFI_DEVICE_ERROR;
+ break;
+ }
+ //
+ // Sanity check: our GOP child controller keeps the VGPU_DEV controller's
+ // Virtio Device Protocol interface open BY_CHILD_CONTROLLER.
+ //
+ ASSERT_EFI_ERROR (EfiTestChildHandle (ControllerHandle,
+ VgpuDev->Child->GopHandle,
+ &gVirtioDeviceProtocolGuid));
+
+ DEBUG ((EFI_D_INFO, "%a: destroying GOP under VirtIo=%p\n", __FUNCTION__,
+ (VOID *)VgpuDev->VirtIo));
+ UninitVgpuGop (VgpuDev, ControllerHandle, This->DriverBindingHandle);
+ break;
+
+ default:
+ //
+ // Impossible, we never produced more than one child.
+ //
+ Status = EFI_DEVICE_ERROR;
+ break;
+ }
+ return Status;
+}
+
+STATIC EFI_DRIVER_BINDING_PROTOCOL mDriverBinding = {
+ VirtioGpuDriverBindingSupported,
+ VirtioGpuDriverBindingStart,
+ VirtioGpuDriverBindingStop,
+ 0x10, // Version
+ NULL, // ImageHandle, overwritten in entry point
+ NULL // DriverBindingHandle, ditto
+};
+
+//
+// Entry point of the driver.
+//
+EFI_STATUS
+EFIAPI
+VirtioGpuEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ return EfiLibInstallDriverBindingComponentName2 (ImageHandle, SystemTable,
+ &mDriverBinding, ImageHandle, NULL /* ComponentName */,
+ &mComponentName2);
+}
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 06/11] OvmfPkg: include VirtioGpuDxe in the platform DSC/FDF files
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (4 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 05/11] OvmfPkg/VirtioGpuDxe: introduce with Component Name 2 and Driver Binding Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 07/11] ArmVirtPkg/ArmVirtQemu: " Laszlo Ersek
` (8 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
At this stage, the driver builds, and suffices for testing binding and
unbinding.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfPkgIa32.fdf | 1 +
OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
OvmfPkg/OvmfPkgX64.fdf | 1 +
6 files changed, 6 insertions(+)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 61bf13fea125..591175038655 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -635,12 +635,13 @@ [Components]
MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf {
<LibraryClasses>
BltLib|OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
}
+ OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
#
# ISA Support
#
PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 7a48eab45d28..bc8d3cda214d 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -644,12 +644,13 @@ [Components.X64]
MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf {
<LibraryClasses>
BltLib|OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
}
+ OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
#
# ISA Support
#
PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index eca142a958c2..ac3929e1be8a 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -642,12 +642,13 @@ [Components]
MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf {
<LibraryClasses>
BltLib|OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
}
+ OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
#
# ISA Support
#
PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index b4b0a2201144..a65a0353813b 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -347,12 +347,13 @@ [FV.DXEFV]
INF IntelFrameworkModulePkg/Csm/BiosThunk/VideoDxe/VideoDxe.inf
INF IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
!endif
INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
!if $(SMM_REQUIRE) == TRUE
INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
INF OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 552ab2a703c4..cde07ad70964 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -347,12 +347,13 @@ [FV.DXEFV]
INF IntelFrameworkModulePkg/Csm/BiosThunk/VideoDxe/VideoDxe.inf
INF IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
!endif
INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
!if $(SMM_REQUIRE) == TRUE
INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
INF OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 28b98a9f5cd3..3d692ab5ac9c 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -347,12 +347,13 @@ [FV.DXEFV]
INF IntelFrameworkModulePkg/Csm/BiosThunk/VideoDxe/VideoDxe.inf
INF IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
INF RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
!endif
INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
!if $(SMM_REQUIRE) == TRUE
INF OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
INF OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf
INF UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 07/11] ArmVirtPkg/ArmVirtQemu: include VirtioGpuDxe in the platform DSC/FDF files
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (5 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 06/11] OvmfPkg: include VirtioGpuDxe in the platform DSC/FDF files Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 13:14 ` Ard Biesheuvel
2016-08-19 12:49 ` [PATCH 08/11] OvmfPkg/VirtioGpuDxe: initialize and tear down VirtIo GPU device Laszlo Ersek
` (7 subsequent siblings)
14 siblings, 1 reply; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
At this stage, the driver builds, and suffices for testing binding and
unbinding.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmVirtPkg/ArmVirtQemu.dsc | 1 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
3 files changed, 3 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 9f88786df81c..2eb2780a1fca 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -378,12 +378,13 @@ [Components.common]
# Video support
#
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf {
<LibraryClasses>
BltLib|OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
}
+ OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
OvmfPkg/PlatformDxe/Platform.inf
#
# USB Support
#
MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 6c536d9bbd2d..d9e62c78d28e 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -364,12 +364,13 @@ [Components.common]
# Video support
#
OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf {
<LibraryClasses>
BltLib|OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
}
+ OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
OvmfPkg/PlatformDxe/Platform.inf
#
# USB Support
#
MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index ad7037fe5f63..01e650d9e333 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -157,12 +157,13 @@ [FV.FvMain]
INF OvmfPkg/Virtio10Dxe/Virtio10.inf
#
# Video support
#
INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+ INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
#
# USB Support
#
INF MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 07/11] ArmVirtPkg/ArmVirtQemu: include VirtioGpuDxe in the platform DSC/FDF files
2016-08-19 12:49 ` [PATCH 07/11] ArmVirtPkg/ArmVirtQemu: " Laszlo Ersek
@ 2016-08-19 13:14 ` Ard Biesheuvel
0 siblings, 0 replies; 34+ messages in thread
From: Ard Biesheuvel @ 2016-08-19 13:14 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen
On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
> At this stage, the driver builds, and suffices for testing binding and
> unbinding.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmVirtPkg/ArmVirtQemu.dsc | 1 +
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
> ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
> 3 files changed, 3 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 9f88786df81c..2eb2780a1fca 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -378,12 +378,13 @@ [Components.common]
> # Video support
>
> #
>
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf {
>
> <LibraryClasses>
>
> BltLib|OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
>
> }
>
> + OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>
> OvmfPkg/PlatformDxe/Platform.inf
>
>
>
> #
>
> # USB Support
>
> #
>
> MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
>
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 6c536d9bbd2d..d9e62c78d28e 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -364,12 +364,13 @@ [Components.common]
> # Video support
>
> #
>
> OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf {
>
> <LibraryClasses>
>
> BltLib|OptionRomPkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
>
> }
>
> + OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>
> OvmfPkg/PlatformDxe/Platform.inf
>
>
>
> #
>
> # USB Support
>
> #
>
> MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
>
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index ad7037fe5f63..01e650d9e333 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -157,12 +157,13 @@ [FV.FvMain]
> INF OvmfPkg/Virtio10Dxe/Virtio10.inf
>
>
>
> #
>
> # Video support
>
> #
>
> INF OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
>
> + INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>
> INF OvmfPkg/PlatformDxe/Platform.inf
>
>
>
> #
>
> # USB Support
>
> #
>
> INF MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
>
> --
> 2.9.2
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 08/11] OvmfPkg/VirtioGpuDxe: initialize and tear down VirtIo GPU device
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (6 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 07/11] ArmVirtPkg/ArmVirtQemu: " Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 09/11] OvmfPkg/VirtioGpuDxe: provide functions for sending VirtIo GPU commands Laszlo Ersek
` (6 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
This patch implements the steps listed in section "3.1.1 Driver
Requirements: Device Initialization" of the Virtio V1.0 Committee Spec 04.
The VirtIo GPU is brought up in VirtioGpuDriverBindingStart(), and down in
VirtioGpuDriverBindingStop().
We also add an ExitBootServices() callback that resets the device. This
ensures that the device model abandons any guest memory areas when we
transfer control to the guest OS.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf | 2 +
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 68 +++++++
OvmfPkg/VirtioGpuDxe/Commands.c | 214 ++++++++++++++++++++
OvmfPkg/VirtioGpuDxe/DriverBinding.c | 28 ++-
4 files changed, 311 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf b/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
index 948350dbce21..7a6269eded51 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
@@ -21,12 +21,13 @@ [Defines]
FILE_GUID = D6099B94-CD97-4CC5-8714-7F6312701A8A
MODULE_TYPE = UEFI_DRIVER
VERSION_STRING = 1.0
ENTRY_POINT = VirtioGpuEntryPoint
[Sources]
+ Commands.c
DriverBinding.c
VirtioGpu.h
[Packages]
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
@@ -37,11 +38,12 @@ [LibraryClasses]
DevicePathLib
MemoryAllocationLib
PrintLib
UefiBootServicesTableLib
UefiDriverEntryPoint
UefiLib
+ VirtioLib
[Protocols]
gEfiDevicePathProtocolGuid ## TO_START ## BY_START
gEfiPciIoProtocolGuid ## TO_START
gVirtioDeviceProtocolGuid ## TO_START
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
index ca5805df8442..97767dba709f 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -46,12 +46,22 @@ typedef struct {
// EFI_COMPONENT_NAME2_PROTOCOL.GetControllerName(). It is expressed in table
// form because it can theoretically support several languages. Never NULL.
//
EFI_UNICODE_STRING_TABLE *BusName;
//
+ // VirtIo ring used for VirtIo communication.
+ //
+ VRING Ring;
+
+ //
+ // Event to be signaled at ExitBootServices().
+ //
+ EFI_EVENT ExitBoot;
+
+ //
// The Child field references the GOP wrapper structure. If this pointer is
// NULL, then the hybrid driver has bound (i.e., started) the
// VIRTIO_DEVICE_PROTOCOL controller without producing the child GOP
// controller (that is, after Start() was called with RemainingDevicePath
// pointing to and End of Device Path node). Child can be created and
// destroyed, even repeatedly, independently of VGPU_DEV.
@@ -100,7 +110,65 @@ struct VGPU_GOP_STRUCT {
//
// For now it is just a placeholder.
//
UINT8 Gop;
};
+//
+// VirtIo GPU initialization, and commands (primitives) for the GPU device.
+//
+/**
+ Configure the VirtIo GPU device that underlies VgpuDev.
+
+ @param[in,out] VgpuDev The VGPU_DEV object to set up VirtIo messaging for.
+ On input, the caller is responsible for having
+ initialized VgpuDev->VirtIo. On output, VgpuDev->Ring
+ has been initialized, and synchronous VirtIo GPU
+ commands (primitives) can be submitted to the device.
+
+ @retval EFI_SUCCESS VirtIo GPU configuration successful.
+
+ @retval EFI_UNSUPPORTED The host-side configuration of the VirtIo GPU is not
+ supported by this driver.
+
+ @retval Error codes from underlying functions.
+**/
+EFI_STATUS
+VirtioGpuInit (
+ IN OUT VGPU_DEV *VgpuDev
+ );
+
+/**
+ De-configure the VirtIo GPU device that underlies VgpuDev.
+
+ @param[in,out] VgpuDev The VGPU_DEV object to tear down VirtIo messaging
+ for. On input, the caller is responsible for having
+ called VirtioGpuInit(). On output, VgpuDev->Ring has
+ been uninitialized; VirtIo GPU commands (primitives)
+ can no longer be submitted to the device.
+**/
+VOID
+VirtioGpuUninit (
+ IN OUT VGPU_DEV *VgpuDev
+ );
+
+/**
+ EFI_EVENT_NOTIFY function for the VGPU_DEV.ExitBoot event. It resets the
+ VirtIo device, causing it to release its resources and to forget its
+ configuration.
+
+ This function may only be called (that is, VGPU_DEV.ExitBoot may only be
+ signaled) after VirtioGpuInit() returns and before VirtioGpuUninit() is
+ called.
+
+ @param[in] Event Event whose notification function is being invoked.
+
+ @param[in] Context Pointer to the associated VGPU_DEV object.
+**/
+VOID
+EFIAPI
+VirtioGpuExitBoot (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ );
+
#endif // _VIRTIO_GPU_DXE_H_
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
new file mode 100644
index 000000000000..804de950ff24
--- /dev/null
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -0,0 +1,214 @@
+/** @file
+
+ VirtIo GPU initialization, and commands (primitives) for the GPU device.
+
+ Copyright (C) 2016, 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.
+
+**/
+
+#include <IndustryStandard/VirtioGpu.h>
+#include <Library/VirtioLib.h>
+
+#include "VirtioGpu.h"
+
+/**
+ Configure the VirtIo GPU device that underlies VgpuDev.
+
+ @param[in,out] VgpuDev The VGPU_DEV object to set up VirtIo messaging for.
+ On input, the caller is responsible for having
+ initialized VgpuDev->VirtIo. On output, VgpuDev->Ring
+ has been initialized, and synchronous VirtIo GPU
+ commands (primitives) can be submitted to the device.
+
+ @retval EFI_SUCCESS VirtIo GPU configuration successful.
+
+ @retval EFI_UNSUPPORTED The host-side configuration of the VirtIo GPU is not
+ supported by this driver.
+
+ @retval Error codes from underlying functions.
+**/
+EFI_STATUS
+VirtioGpuInit (
+ IN OUT VGPU_DEV *VgpuDev
+ )
+{
+ UINT8 NextDevStat;
+ EFI_STATUS Status;
+ UINT64 Features;
+ UINT16 QueueSize;
+
+ //
+ // Execute virtio-v1.0-cs04, 3.1.1 Driver Requirements: Device
+ // Initialization.
+ //
+ // 1. Reset the device.
+ //
+ NextDevStat = 0;
+ Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+
+ //
+ // 2. Set the ACKNOWLEDGE status bit [...]
+ //
+ NextDevStat |= VSTAT_ACK;
+ Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+
+ //
+ // 3. Set the DRIVER status bit [...]
+ //
+ NextDevStat |= VSTAT_DRIVER;
+ Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+
+ //
+ // 4. Read device feature bits...
+ //
+ Status = VgpuDev->VirtIo->GetDeviceFeatures (VgpuDev->VirtIo, &Features);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+ if ((Features & VIRTIO_F_VERSION_1) == 0) {
+ Status = EFI_UNSUPPORTED;
+ goto Failed;
+ }
+ //
+ // We only want the most basic 2D features.
+ //
+ Features &= VIRTIO_F_VERSION_1;
+
+ //
+ // ... and write the subset of feature bits understood by the [...] driver to
+ // the device. [...]
+ // 5. Set the FEATURES_OK status bit.
+ // 6. Re-read device status to ensure the FEATURES_OK bit is still set [...]
+ //
+ Status = Virtio10WriteFeatures (VgpuDev->VirtIo, Features, &NextDevStat);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+
+ //
+ // 7. Perform device-specific setup, including discovery of virtqueues for
+ // the device [...]
+ //
+ Status = VgpuDev->VirtIo->SetQueueSel (VgpuDev->VirtIo,
+ VIRTIO_GPU_CONTROL_QUEUE);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+ Status = VgpuDev->VirtIo->GetQueueNumMax (VgpuDev->VirtIo, &QueueSize);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+
+ //
+ // We implement each VirtIo GPU command that we use with two descriptors:
+ // request, response.
+ //
+ if (QueueSize < 2) {
+ Status = EFI_UNSUPPORTED;
+ goto Failed;
+ }
+
+ //
+ // [...] population of virtqueues [...]
+ //
+ Status = VirtioRingInit (QueueSize, &VgpuDev->Ring);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+ Status = VgpuDev->VirtIo->SetQueueAddress (VgpuDev->VirtIo, &VgpuDev->Ring);
+ if (EFI_ERROR (Status)) {
+ goto ReleaseQueue;
+ }
+
+ //
+ // 8. Set the DRIVER_OK status bit.
+ //
+ NextDevStat |= VSTAT_DRIVER_OK;
+ Status = VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
+ if (EFI_ERROR (Status)) {
+ goto ReleaseQueue;
+ }
+
+ return EFI_SUCCESS;
+
+ReleaseQueue:
+ VirtioRingUninit (&VgpuDev->Ring);
+
+Failed:
+ //
+ // If any of these steps go irrecoverably wrong, the driver SHOULD set the
+ // FAILED status bit to indicate that it has given up on the device (it can
+ // reset the device later to restart if desired). [...]
+ //
+ // VirtIo access failure here should not mask the original error.
+ //
+ NextDevStat |= VSTAT_FAILED;
+ VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, NextDevStat);
+
+ return Status;
+}
+
+/**
+ De-configure the VirtIo GPU device that underlies VgpuDev.
+
+ @param[in,out] VgpuDev The VGPU_DEV object to tear down VirtIo messaging
+ for. On input, the caller is responsible for having
+ called VirtioGpuInit(). On output, VgpuDev->Ring has
+ been uninitialized; VirtIo GPU commands (primitives)
+ can no longer be submitted to the device.
+**/
+VOID
+VirtioGpuUninit (
+ IN OUT VGPU_DEV *VgpuDev
+ )
+{
+ //
+ // Resetting the VirtIo device makes it release its resources and forget its
+ // configuration.
+ //
+ VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
+ VirtioRingUninit (&VgpuDev->Ring);
+}
+
+/**
+ EFI_EVENT_NOTIFY function for the VGPU_DEV.ExitBoot event. It resets the
+ VirtIo device, causing it to release its resources and to forget its
+ configuration.
+
+ This function may only be called (that is, VGPU_DEV.ExitBoot may only be
+ signaled) after VirtioGpuInit() returns and before VirtioGpuUninit() is
+ called.
+
+ @param[in] Event Event whose notification function is being invoked.
+
+ @param[in] Context Pointer to the associated VGPU_DEV object.
+**/
+VOID
+EFIAPI
+VirtioGpuExitBoot (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ VGPU_DEV *VgpuDev;
+
+ VgpuDev = Context;
+ VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
+}
diff --git a/OvmfPkg/VirtioGpuDxe/DriverBinding.c b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
index b902a07871e0..bdea55ef7dbf 100644
--- a/OvmfPkg/VirtioGpuDxe/DriverBinding.c
+++ b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
@@ -643,19 +643,31 @@ VirtioGpuDriverBindingStart (
&VgpuDev->BusName, VgpuDevName, FALSE /* Iso639Language */);
FreePool (VgpuDevName);
if (EFI_ERROR (Status)) {
goto FreeVgpuDev;
}
+ Status = VirtioGpuInit (VgpuDev);
+ if (EFI_ERROR (Status)) {
+ goto FreeVgpuDevBusName;
+ }
+
+ Status = gBS->CreateEvent (EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,
+ VirtioGpuExitBoot, VgpuDev /* NotifyContext */,
+ &VgpuDev->ExitBoot);
+ if (EFI_ERROR (Status)) {
+ goto UninitGpu;
+ }
+
//
// Install the VGPU_DEV "protocol interface" on ControllerHandle.
//
Status = gBS->InstallProtocolInterface (&ControllerHandle,
&gEfiCallerIdGuid, EFI_NATIVE_INTERFACE, VgpuDev);
if (EFI_ERROR (Status)) {
- goto FreeVgpuDevBusName;
+ goto CloseExitBoot;
}
if (RemainingDevicePath != NULL && IsDevicePathEnd (RemainingDevicePath)) {
//
// No child handle should be produced; we're done.
//
@@ -690,12 +702,22 @@ VirtioGpuDriverBindingStart (
UninstallVgpuDev:
if (VirtIoBoundJustNow) {
gBS->UninstallProtocolInterface (ControllerHandle, &gEfiCallerIdGuid,
VgpuDev);
}
+CloseExitBoot:
+ if (VirtIoBoundJustNow) {
+ gBS->CloseEvent (VgpuDev->ExitBoot);
+ }
+
+UninitGpu:
+ if (VirtIoBoundJustNow) {
+ VirtioGpuUninit (VgpuDev);
+ }
+
FreeVgpuDevBusName:
if (VirtIoBoundJustNow) {
FreeUnicodeStringTable (VgpuDev->BusName);
}
FreeVgpuDev:
@@ -758,12 +780,16 @@ VirtioGpuDriverBindingStop (
(VOID *)VgpuDev->VirtIo));
Status = gBS->UninstallProtocolInterface (ControllerHandle,
&gEfiCallerIdGuid, VgpuDev);
ASSERT_EFI_ERROR (Status);
+ Status = gBS->CloseEvent (VgpuDev->ExitBoot);
+ ASSERT_EFI_ERROR (Status);
+
+ VirtioGpuUninit (VgpuDev);
FreeUnicodeStringTable (VgpuDev->BusName);
FreePool (VgpuDev);
Status = gBS->CloseProtocol (ControllerHandle, &gVirtioDeviceProtocolGuid,
This->DriverBindingHandle, ControllerHandle);
ASSERT_EFI_ERROR (Status);
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 09/11] OvmfPkg/VirtioGpuDxe: provide functions for sending VirtIo GPU commands
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (7 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 08/11] OvmfPkg/VirtioGpuDxe: initialize and tear down VirtIo GPU device Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 10/11] OvmfPkg/VirtioGpuDxe: implement EFI_GRAPHICS_OUTPUT_PROTOCOL Laszlo Ersek
` (5 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
In this patch we add a "workhorse" function called VirtioGpuSendCommand(),
and implement seven simple RPCs atop, for the command types listed in
"OvmfPkg/Include/IndustryStandard/VirtioGpu.h".
These functions will be called by our EFI_GRAPHICS_OUTPUT_PROTOCOL
implementation.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 93 ++++++
OvmfPkg/VirtioGpuDxe/Commands.c | 347 +++++++++++++++++++-
2 files changed, 439 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
index 97767dba709f..f8839922487c 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -14,12 +14,13 @@
**/
#ifndef _VIRTIO_GPU_DXE_H_
#define _VIRTIO_GPU_DXE_H_
+#include <IndustryStandard/VirtioGpu.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
#include <Protocol/VirtioDevice.h>
//
// Forward declaration of VGPU_GOP.
@@ -56,12 +57,17 @@ typedef struct {
//
// Event to be signaled at ExitBootServices().
//
EFI_EVENT ExitBoot;
//
+ // Common running counter for all VirtIo GPU requests that ask for fencing.
+ //
+ UINT64 FenceId;
+
+ //
// The Child field references the GOP wrapper structure. If this pointer is
// NULL, then the hybrid driver has bound (i.e., started) the
// VIRTIO_DEVICE_PROTOCOL controller without producing the child GOP
// controller (that is, after Start() was called with RemainingDevicePath
// pointing to and End of Device Path node). Child can be created and
// destroyed, even repeatedly, independently of VGPU_DEV.
@@ -168,7 +174,94 @@ VOID
EFIAPI
VirtioGpuExitBoot (
IN EFI_EVENT Event,
IN VOID *Context
);
+/**
+ The following functions send requests to the VirtIo GPU device model, await
+ the answer from the host, and return a status. They share the following
+ interface details:
+
+ @param[in,out] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
+ device. The caller is responsible to have
+ successfully invoked VirtioGpuInit() on VgpuDev
+ previously, while VirtioGpuUninit() must not have
+ been called on VgpuDev.
+
+ @retval EFI_INVALID_PARAMETER Invalid command-specific parameters were
+ detected by this driver.
+
+ @retval EFI_SUCCESS Operation successful.
+
+ @retval EFI_DEVICE_ERROR The host rejected the request. The host error
+ code has been logged on the EFI_D_ERROR level.
+
+ @return Codes for unexpected errors in VirtIo
+ messaging.
+
+ For the command-specific parameters, please consult the GPU Device section of
+ the VirtIo 1.0 specification (see references in
+ "OvmfPkg/Include/IndustryStandard/VirtioGpu.h").
+**/
+EFI_STATUS
+VirtioGpuResourceCreate2d (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId,
+ IN VIRTIO_GPU_FORMATS Format,
+ IN UINT32 Width,
+ IN UINT32 Height
+ );
+
+EFI_STATUS
+VirtioGpuResourceUnref (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId
+ );
+
+EFI_STATUS
+VirtioGpuResourceAttachBacking (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId,
+ IN VOID *FirstBackingPage,
+ IN UINTN NumberOfPages
+ );
+
+EFI_STATUS
+VirtioGpuResourceDetachBacking (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId
+ );
+
+EFI_STATUS
+VirtioGpuSetScanout (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 X,
+ IN UINT32 Y,
+ IN UINT32 Width,
+ IN UINT32 Height,
+ IN UINT32 ScanoutId,
+ IN UINT32 ResourceId
+ );
+
+EFI_STATUS
+VirtioGpuTransferToHost2d (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 X,
+ IN UINT32 Y,
+ IN UINT32 Width,
+ IN UINT32 Height,
+ IN UINT64 Offset,
+ IN UINT32 ResourceId
+ );
+
+EFI_STATUS
+VirtioGpuResourceFlush (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 X,
+ IN UINT32 Y,
+ IN UINT32 Width,
+ IN UINT32 Height,
+ IN UINT32 ResourceId
+ );
+
#endif // _VIRTIO_GPU_DXE_H_
diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index 804de950ff24..b369dc3a7abc 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -11,13 +11,12 @@
THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
-#include <IndustryStandard/VirtioGpu.h>
#include <Library/VirtioLib.h>
#include "VirtioGpu.h"
/**
Configure the VirtIo GPU device that underlies VgpuDev.
@@ -209,6 +208,352 @@ VirtioGpuExitBoot (
{
VGPU_DEV *VgpuDev;
VgpuDev = Context;
VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
}
+
+/**
+ Internal utility function that sends a request to the VirtIo GPU device
+ model, awaits the answer from the host, and returns a status.
+
+ @param[in,out] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
+ device. The caller is responsible to have
+ successfully invoked VirtioGpuInit() on VgpuDev
+ previously, while VirtioGpuUninit() must not have
+ been called on VgpuDev.
+
+ @param[in] RequestType The type of the request. The caller is responsible
+ for providing a VirtioGpuCmd* RequestType which, on
+ success, elicits a VirtioGpuRespOkNodata response
+ from the host.
+
+ @param[in] Fence Whether to enable fencing for this request. Fencing
+ forces the host to complete the command before
+ producing a response. If Fence is TRUE, then
+ VgpuDev->FenceId is consumed, and incremented.
+
+ @param[in,out] Header Pointer to the caller-allocated request object. The
+ request must start with VIRTIO_GPU_CONTROL_HEADER.
+ This function overwrites all fields of Header before
+ submitting the request to the host:
+
+ - it sets Type from RequestType,
+
+ - it sets Flags and FenceId based on Fence,
+
+ - it zeroes CtxId and Padding.
+
+ @param[in] RequestSize Size of the entire caller-allocated request object,
+ including the leading VIRTIO_GPU_CONTROL_HEADER.
+
+ @retval EFI_SUCCESS Operation successful.
+
+ @retval EFI_DEVICE_ERROR The host rejected the request. The host error
+ code has been logged on the EFI_D_ERROR level.
+
+ @return Codes for unexpected errors in VirtIo
+ messaging.
+**/
+STATIC
+EFI_STATUS
+VirtioGpuSendCommand (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN VIRTIO_GPU_CONTROL_TYPE RequestType,
+ IN BOOLEAN Fence,
+ IN OUT volatile VIRTIO_GPU_CONTROL_HEADER *Header,
+ IN UINTN RequestSize
+ )
+{
+ DESC_INDICES Indices;
+ volatile VIRTIO_GPU_CONTROL_HEADER Response;
+ EFI_STATUS Status;
+ UINT32 ResponseSize;
+
+ //
+ // Initialize Header.
+ //
+ Header->Type = RequestType;
+ if (Fence) {
+ Header->Flags = VIRTIO_GPU_FLAG_FENCE;
+ Header->FenceId = VgpuDev->FenceId++;
+ } else {
+ Header->Flags = 0;
+ Header->FenceId = 0;
+ }
+ Header->CtxId = 0;
+ Header->Padding = 0;
+
+ ASSERT (RequestSize >= sizeof *Header);
+
+ //
+ // Compose the descriptor chain.
+ //
+ VirtioPrepare (&VgpuDev->Ring, &Indices);
+ VirtioAppendDesc (&VgpuDev->Ring, (UINTN)Header, RequestSize,
+ VRING_DESC_F_NEXT, &Indices);
+ VirtioAppendDesc (&VgpuDev->Ring, (UINTN)&Response, sizeof Response,
+ VRING_DESC_F_WRITE, &Indices);
+
+ //
+ // Send the command.
+ //
+ Status = VirtioFlush (VgpuDev->VirtIo, VIRTIO_GPU_CONTROL_QUEUE,
+ &VgpuDev->Ring, &Indices, &ResponseSize);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Parse the response.
+ //
+ if (ResponseSize != sizeof Response) {
+ DEBUG ((EFI_D_ERROR, "%a: malformed response to Request=0x%x\n",
+ __FUNCTION__, (UINT32)RequestType));
+ return EFI_PROTOCOL_ERROR;
+ }
+
+ if (Response.Type == VirtioGpuRespOkNodata) {
+ return EFI_SUCCESS;
+ }
+
+ DEBUG ((EFI_D_ERROR, "%a: Request=0x%x Response=0x%x\n", __FUNCTION__,
+ (UINT32)RequestType, Response.Type));
+ return EFI_DEVICE_ERROR;
+}
+
+/**
+ The following functions send requests to the VirtIo GPU device model, await
+ the answer from the host, and return a status. They share the following
+ interface details:
+
+ @param[in,out] VgpuDev The VGPU_DEV object that represents the VirtIo GPU
+ device. The caller is responsible to have
+ successfully invoked VirtioGpuInit() on VgpuDev
+ previously, while VirtioGpuUninit() must not have
+ been called on VgpuDev.
+
+ @retval EFI_INVALID_PARAMETER Invalid command-specific parameters were
+ detected by this driver.
+
+ @retval EFI_SUCCESS Operation successful.
+
+ @retval EFI_DEVICE_ERROR The host rejected the request. The host error
+ code has been logged on the EFI_D_ERROR level.
+
+ @return Codes for unexpected errors in VirtIo
+ messaging.
+
+ For the command-specific parameters, please consult the GPU Device section of
+ the VirtIo 1.0 specification (see references in
+ "OvmfPkg/Include/IndustryStandard/VirtioGpu.h").
+**/
+EFI_STATUS
+VirtioGpuResourceCreate2d (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId,
+ IN VIRTIO_GPU_FORMATS Format,
+ IN UINT32 Width,
+ IN UINT32 Height
+ )
+{
+ volatile VIRTIO_GPU_RESOURCE_CREATE_2D Request;
+
+ if (ResourceId == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Request.ResourceId = ResourceId;
+ Request.Format = (UINT32)Format;
+ Request.Width = Width;
+ Request.Height = Height;
+
+ return VirtioGpuSendCommand (
+ VgpuDev,
+ VirtioGpuCmdResourceCreate2d,
+ FALSE, // Fence
+ &Request.Header,
+ sizeof Request
+ );
+}
+
+EFI_STATUS
+VirtioGpuResourceUnref (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId
+ )
+{
+ volatile VIRTIO_GPU_RESOURCE_UNREF Request;
+
+ if (ResourceId == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Request.ResourceId = ResourceId;
+ Request.Padding = 0;
+
+ return VirtioGpuSendCommand (
+ VgpuDev,
+ VirtioGpuCmdResourceUnref,
+ FALSE, // Fence
+ &Request.Header,
+ sizeof Request
+ );
+}
+
+EFI_STATUS
+VirtioGpuResourceAttachBacking (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId,
+ IN VOID *FirstBackingPage,
+ IN UINTN NumberOfPages
+ )
+{
+ volatile VIRTIO_GPU_RESOURCE_ATTACH_BACKING Request;
+
+ if (ResourceId == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Request.ResourceId = ResourceId;
+ Request.NrEntries = 1;
+ Request.Entry.Addr = (UINTN)FirstBackingPage;
+ Request.Entry.Length = (UINT32)EFI_PAGES_TO_SIZE (NumberOfPages);
+ Request.Entry.Padding = 0;
+
+ return VirtioGpuSendCommand (
+ VgpuDev,
+ VirtioGpuCmdResourceAttachBacking,
+ FALSE, // Fence
+ &Request.Header,
+ sizeof Request
+ );
+}
+
+EFI_STATUS
+VirtioGpuResourceDetachBacking (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 ResourceId
+ )
+{
+ volatile VIRTIO_GPU_RESOURCE_DETACH_BACKING Request;
+
+ if (ResourceId == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Request.ResourceId = ResourceId;
+ Request.Padding = 0;
+
+ //
+ // In this case, we set Fence to TRUE, because after this function returns,
+ // the caller might reasonably want to repurpose the backing pages
+ // immediately. Thus we should ensure that the host releases all references
+ // to the backing pages before we return.
+ //
+ return VirtioGpuSendCommand (
+ VgpuDev,
+ VirtioGpuCmdResourceDetachBacking,
+ TRUE, // Fence
+ &Request.Header,
+ sizeof Request
+ );
+}
+
+EFI_STATUS
+VirtioGpuSetScanout (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 X,
+ IN UINT32 Y,
+ IN UINT32 Width,
+ IN UINT32 Height,
+ IN UINT32 ScanoutId,
+ IN UINT32 ResourceId
+ )
+{
+ volatile VIRTIO_GPU_SET_SCANOUT Request;
+
+ //
+ // Unlike for most other commands, ResourceId=0 is valid; it
+ // is used to disable a scanout.
+ //
+ Request.Rectangle.X = X;
+ Request.Rectangle.Y = Y;
+ Request.Rectangle.Width = Width;
+ Request.Rectangle.Height = Height;
+ Request.ScanoutId = ScanoutId;
+ Request.ResourceId = ResourceId;
+
+ return VirtioGpuSendCommand (
+ VgpuDev,
+ VirtioGpuCmdSetScanout,
+ FALSE, // Fence
+ &Request.Header,
+ sizeof Request
+ );
+}
+
+EFI_STATUS
+VirtioGpuTransferToHost2d (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 X,
+ IN UINT32 Y,
+ IN UINT32 Width,
+ IN UINT32 Height,
+ IN UINT64 Offset,
+ IN UINT32 ResourceId
+ )
+{
+ volatile VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D Request;
+
+ if (ResourceId == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Request.Rectangle.X = X;
+ Request.Rectangle.Y = Y;
+ Request.Rectangle.Width = Width;
+ Request.Rectangle.Height = Height;
+ Request.Offset = Offset;
+ Request.ResourceId = ResourceId;
+ Request.Padding = 0;
+
+ return VirtioGpuSendCommand (
+ VgpuDev,
+ VirtioGpuCmdTransferToHost2d,
+ FALSE, // Fence
+ &Request.Header,
+ sizeof Request
+ );
+}
+
+EFI_STATUS
+VirtioGpuResourceFlush (
+ IN OUT VGPU_DEV *VgpuDev,
+ IN UINT32 X,
+ IN UINT32 Y,
+ IN UINT32 Width,
+ IN UINT32 Height,
+ IN UINT32 ResourceId
+ )
+{
+ volatile VIRTIO_GPU_RESOURCE_FLUSH Request;
+
+ if (ResourceId == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Request.Rectangle.X = X;
+ Request.Rectangle.Y = Y;
+ Request.Rectangle.Width = Width;
+ Request.Rectangle.Height = Height;
+ Request.ResourceId = ResourceId;
+ Request.Padding = 0;
+
+ return VirtioGpuSendCommand (
+ VgpuDev,
+ VirtioGpuCmdResourceFlush,
+ FALSE, // Fence
+ &Request.Header,
+ sizeof Request
+ );
+}
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 10/11] OvmfPkg/VirtioGpuDxe: implement EFI_GRAPHICS_OUTPUT_PROTOCOL
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (8 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 09/11] OvmfPkg/VirtioGpuDxe: provide functions for sending VirtIo GPU commands Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 12:49 ` [PATCH 11/11] ArmVirtPkg: remove PcdKludgeMapPciMmioAsCached Laszlo Ersek
` (4 subsequent siblings)
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
In this patch we replace our "dummy" Graphics Output Protocol interface
with the real one. We exploit that EFI_GRAPHICS_OUTPUT_BLT_PIXEL and
VirtioGpuFormatB8G8R8X8Unorm have identical representations; this lets us
forego any pixel format conversions in the guest. For messaging the VirtIo
GPU device, we use the primitives introduced in the previous patch.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/VirtioGpuDxe/VirtioGpu.inf | 2 +
OvmfPkg/VirtioGpuDxe/VirtioGpu.h | 64 +-
OvmfPkg/VirtioGpuDxe/DriverBinding.c | 29 +-
OvmfPkg/VirtioGpuDxe/Gop.c | 647 ++++++++++++++++++++
4 files changed, 719 insertions(+), 23 deletions(-)
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf b/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
index 7a6269eded51..04bc2964c223 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
@@ -23,12 +23,13 @@ [Defines]
VERSION_STRING = 1.0
ENTRY_POINT = VirtioGpuEntryPoint
[Sources]
Commands.c
DriverBinding.c
+ Gop.c
VirtioGpu.h
[Packages]
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
@@ -42,8 +43,9 @@ [LibraryClasses]
UefiDriverEntryPoint
UefiLib
VirtioLib
[Protocols]
gEfiDevicePathProtocolGuid ## TO_START ## BY_START
+ gEfiGraphicsOutputProtocolGuid ## BY_START
gEfiPciIoProtocolGuid ## TO_START
gVirtioDeviceProtocolGuid ## TO_START
diff --git a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
index f8839922487c..078b7d44d83e 100644
--- a/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
+++ b/OvmfPkg/VirtioGpuDxe/VirtioGpu.h
@@ -17,12 +17,13 @@
#ifndef _VIRTIO_GPU_DXE_H_
#define _VIRTIO_GPU_DXE_H_
#include <IndustryStandard/VirtioGpu.h>
#include <Library/DebugLib.h>
#include <Library/UefiLib.h>
+#include <Protocol/GraphicsOutput.h>
#include <Protocol/VirtioDevice.h>
//
// Forward declaration of VGPU_GOP.
//
typedef struct VGPU_GOP_STRUCT VGPU_GOP;
@@ -111,15 +112,40 @@ struct VGPU_GOP_STRUCT {
EFI_DEVICE_PATH_PROTOCOL *GopDevicePath;
//
// The Gop field is installed on the child handle as Graphics Output Protocol
// interface.
//
- // For now it is just a placeholder.
+ EFI_GRAPHICS_OUTPUT_PROTOCOL Gop;
+
//
- UINT8 Gop;
+ // Referenced by Gop.Mode, GopMode provides a summary about the supported
+ // graphics modes, and the current mode.
+ //
+ EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE GopMode;
+
+ //
+ // Referenced by GopMode.Info, GopModeInfo provides detailed information
+ // about the current mode.
+ //
+ EFI_GRAPHICS_OUTPUT_MODE_INFORMATION GopModeInfo;
+
+ //
+ // Identifier of the 2D host resource that is in use by this head (scanout)
+ // of the VirtIo GPU device. Zero until the first successful -- internal --
+ // Gop.SetMode() call, never zero afterwards.
+ //
+ UINT32 ResourceId;
+
+ //
+ // A number of whole pages providing the backing store for the 2D host
+ // resource identified by ResourceId above. NULL until the first successful
+ // -- internal -- Gop.SetMode() call, never NULL afterwards.
+ //
+ UINT32 *BackingStore;
+ UINTN NumberOfPages;
};
//
// VirtIo GPU initialization, and commands (primitives) for the GPU device.
//
/**
@@ -261,7 +287,41 @@ VirtioGpuResourceFlush (
IN UINT32 Y,
IN UINT32 Width,
IN UINT32 Height,
IN UINT32 ResourceId
);
+/**
+ Release guest-side and host-side resources that are related to an initialized
+ VGPU_GOP.Gop.
+
+ param[in,out] VgpuGop The VGPU_GOP object to release resources for.
+
+ On input, the caller is responsible for having called
+ VgpuGop->Gop.SetMode() at least once successfully.
+ (This is equivalent to the requirement that
+ VgpuGop->BackingStore be non-NULL. It is also
+ equivalent to the requirement that VgpuGop->ResourceId
+ be nonzero.)
+
+ On output, resources will be released, and
+ VgpuGop->BackingStore and VgpuGop->ResourceId will be
+ nulled.
+
+ param[in] DisableHead Whether this head (scanout) currently references the
+ resource identified by VgpuGop->ResourceId. Only pass
+ FALSE when VgpuGop->Gop.SetMode() calls this function
+ while switching between modes, and set it to TRUE
+ every other time.
+**/
+VOID
+ReleaseGopResources (
+ IN OUT VGPU_GOP *VgpuGop,
+ IN BOOLEAN DisableHead
+ );
+
+//
+// Template for initializing VGPU_GOP.Gop.
+//
+extern CONST EFI_GRAPHICS_OUTPUT_PROTOCOL mGopTemplate;
+
#endif // _VIRTIO_GPU_DXE_H_
diff --git a/OvmfPkg/VirtioGpuDxe/DriverBinding.c b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
index bdea55ef7dbf..33c1ad3b3110 100644
--- a/OvmfPkg/VirtioGpuDxe/DriverBinding.c
+++ b/OvmfPkg/VirtioGpuDxe/DriverBinding.c
@@ -26,22 +26,12 @@
#include <Protocol/DriverBinding.h>
#include <Protocol/PciIo.h>
#include "VirtioGpu.h"
//
-// Dummy Graphics Output Protocol GUID: a temporary placeholder for the EFI
-// counterpart. It will be replaced with the real thing as soon as we implement
-// the EFI GOP. Refer to VGPU_GOP.Gop.
-//
-STATIC EFI_GUID mDummyGraphicsOutputProtocolGuid = {
- 0x4983f8dc, 0x2782, 0x415b,
- { 0x91, 0xf5, 0x2c, 0xeb, 0x48, 0x4a, 0x0f, 0xe9 }
-};
-
-//
// The device path node that describes the Video Output Device Attributes for
// the single head (UEFI child handle) that we support.
//
// The ACPI_DISPLAY_ADR() macro corresponds to Table B-2, section "B.4.2 _DOD"
// in the ACPI 3.0b spec, or more recently, to Table B-379, section "B.3.2
// _DOD" in the ACPI 6.0 spec.
@@ -353,24 +343,26 @@ InitVgpuGop (
}
ASSERT (ParentVirtIo == ParentBus->VirtIo);
//
// Initialize our Graphics Output Protocol.
//
- // This means "nothing" for now.
+ // Fill in the function members of VgpuGop->Gop from the template, then set
+ // up the rest of the GOP infrastructure by calling SetMode() right now.
//
- Status = EFI_SUCCESS;
+ CopyMem (&VgpuGop->Gop, &mGopTemplate, sizeof mGopTemplate);
+ Status = VgpuGop->Gop.SetMode (&VgpuGop->Gop, 0);
if (EFI_ERROR (Status)) {
goto CloseVirtIoByChild;
}
//
// Install the Graphics Output Protocol on the child handle.
//
Status = gBS->InstallProtocolInterface (&VgpuGop->GopHandle,
- &mDummyGraphicsOutputProtocolGuid, EFI_NATIVE_INTERFACE,
+ &gEfiGraphicsOutputProtocolGuid, EFI_NATIVE_INTERFACE,
&VgpuGop->Gop);
if (EFI_ERROR (Status)) {
goto UninitGop;
}
//
@@ -378,15 +370,13 @@ InitVgpuGop (
//
gBS->RestoreTPL (OldTpl);
ParentBus->Child = VgpuGop;
return EFI_SUCCESS;
UninitGop:
- //
- // Nothing, for now.
- //
+ ReleaseGopResources (VgpuGop, TRUE /* DisableHead */);
CloseVirtIoByChild:
gBS->CloseProtocol (ParentBusController, &gVirtioDeviceProtocolGuid,
DriverBindingHandle, VgpuGop->GopHandle);
UninstallDevicePath:
@@ -436,22 +426,19 @@ UninitVgpuGop (
{
VGPU_GOP *VgpuGop;
EFI_STATUS Status;
VgpuGop = ParentBus->Child;
Status = gBS->UninstallProtocolInterface (VgpuGop->GopHandle,
- &mDummyGraphicsOutputProtocolGuid, &VgpuGop->Gop);
+ &gEfiGraphicsOutputProtocolGuid, &VgpuGop->Gop);
ASSERT_EFI_ERROR (Status);
//
// Uninitialize VgpuGop->Gop.
//
- // Nothing, for now.
- //
- Status = EFI_SUCCESS;
- ASSERT_EFI_ERROR (Status);
+ ReleaseGopResources (VgpuGop, TRUE /* DisableHead */);
Status = gBS->CloseProtocol (ParentBusController, &gVirtioDeviceProtocolGuid,
DriverBindingHandle, VgpuGop->GopHandle);
ASSERT_EFI_ERROR (Status);
Status = gBS->UninstallProtocolInterface (VgpuGop->GopHandle,
diff --git a/OvmfPkg/VirtioGpuDxe/Gop.c b/OvmfPkg/VirtioGpuDxe/Gop.c
new file mode 100644
index 000000000000..c6ff9ed57461
--- /dev/null
+++ b/OvmfPkg/VirtioGpuDxe/Gop.c
@@ -0,0 +1,647 @@
+/** @file
+
+ EFI_GRAPHICS_OUTPUT_PROTOCOL member functions for the VirtIo GPU driver.
+
+ Copyright (C) 2016, 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.
+
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+
+#include "VirtioGpu.h"
+
+/**
+ Release guest-side and host-side resources that are related to an initialized
+ VGPU_GOP.Gop.
+
+ param[in,out] VgpuGop The VGPU_GOP object to release resources for.
+
+ On input, the caller is responsible for having called
+ VgpuGop->Gop.SetMode() at least once successfully.
+ (This is equivalent to the requirement that
+ VgpuGop->BackingStore be non-NULL. It is also
+ equivalent to the requirement that VgpuGop->ResourceId
+ be nonzero.)
+
+ On output, resources will be released, and
+ VgpuGop->BackingStore and VgpuGop->ResourceId will be
+ nulled.
+
+ param[in] DisableHead Whether this head (scanout) currently references the
+ resource identified by VgpuGop->ResourceId. Only pass
+ FALSE when VgpuGop->Gop.SetMode() calls this function
+ while switching between modes, and set it to TRUE
+ every other time.
+**/
+VOID
+ReleaseGopResources (
+ IN OUT VGPU_GOP *VgpuGop,
+ IN BOOLEAN DisableHead
+ )
+{
+ EFI_STATUS Status;
+
+ ASSERT (VgpuGop->ResourceId != 0);
+ ASSERT (VgpuGop->BackingStore != NULL);
+
+ //
+ // If any of the following host-side destruction steps fail, we can't get out
+ // of an inconsistent state, so we'll hang. In general errors in object
+ // destruction can hardly be recovered from.
+ //
+ if (DisableHead) {
+ //
+ // Dissociate head (scanout) #0 from the currently used 2D host resource,
+ // by setting ResourceId=0 for it.
+ //
+ Status = VirtioGpuSetScanout (
+ VgpuGop->ParentBus, // VgpuDev
+ 0, 0, 0, 0, // X, Y, Width, Height
+ 0, // ScanoutId
+ 0 // ResourceId
+ );
+ //
+ // HACK BEGINS HERE
+ //
+ // According to the GPU Device section of the VirtIo specification, the
+ // above operation is valid:
+ //
+ // "The driver can use resource_id = 0 to disable a scanout."
+ //
+ // However, in practice QEMU does not allow us to disable head (scanout) #0
+ // -- it rejects the command with response code 0x1202
+ // (VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID). Looking at the QEMU source
+ // code, function virtio_gpu_set_scanout() in "hw/display/virtio-gpu.c",
+ // this appears fully intentional, despite not being documented in the
+ // spec.
+ //
+ // Surprisingly, ignoring the error here, and proceeding to release
+ // host-side resources that presumably underlie head (scanout) #0, work
+ // without any problems -- the driver survives repeated "disconnect" /
+ // "connect -r" commands in the UEFI shell.
+ //
+ // So, for now, let's just suppress the error.
+ //
+ Status = EFI_SUCCESS;
+ //
+ // HACK ENDS HERE
+ //
+
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ CpuDeadLoop ();
+ }
+ }
+
+ //
+ // Detach backing pages from the currently used 2D host resource.
+ //
+ Status = VirtioGpuResourceDetachBacking (
+ VgpuGop->ParentBus, // VgpuDev
+ VgpuGop->ResourceId // ResourceId
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ CpuDeadLoop ();
+ }
+
+ //
+ // Release backing pages.
+ //
+ FreePages (VgpuGop->BackingStore, VgpuGop->NumberOfPages);
+ VgpuGop->BackingStore = NULL;
+ VgpuGop->NumberOfPages = 0;
+
+ //
+ // Destroy the currently used 2D host resource.
+ //
+ Status = VirtioGpuResourceUnref (
+ VgpuGop->ParentBus, // VgpuDev
+ VgpuGop->ResourceId // ResourceId
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ CpuDeadLoop ();
+ }
+ VgpuGop->ResourceId = 0;
+}
+
+//
+// The resolutions supported by this driver.
+//
+typedef struct {
+ UINT32 Width;
+ UINT32 Height;
+} GOP_RESOLUTION;
+
+STATIC CONST GOP_RESOLUTION mGopResolutions[] = {
+ { 640, 480 },
+ { 800, 480 },
+ { 800, 600 },
+ { 832, 624 },
+ { 960, 640 },
+ { 1024, 600 },
+ { 1024, 768 },
+ { 1152, 864 },
+ { 1152, 870 },
+ { 1280, 720 },
+ { 1280, 760 },
+ { 1280, 768 },
+ { 1280, 800 },
+ { 1280, 960 },
+ { 1280, 1024 },
+ { 1360, 768 },
+ { 1366, 768 },
+ { 1400, 1050 },
+ { 1440, 900 },
+ { 1600, 900 },
+ { 1600, 1200 },
+ { 1680, 1050 },
+ { 1920, 1080 },
+ { 1920, 1200 },
+ { 1920, 1440 },
+ { 2000, 2000 },
+ { 2048, 1536 },
+ { 2048, 2048 },
+ { 2560, 1440 },
+ { 2560, 1600 },
+ { 2560, 2048 },
+ { 2800, 2100 },
+ { 3200, 2400 },
+ { 3840, 2160 },
+ { 4096, 2160 },
+ { 7680, 4320 },
+ { 8192, 4320 },
+};
+
+//
+// Macro for casting VGPU_GOP.Gop to VGPU_GOP.
+//
+#define VGPU_GOP_FROM_GOP(GopPointer) \
+ CR (GopPointer, VGPU_GOP, Gop, VGPU_GOP_SIG)
+
+//
+// EFI_GRAPHICS_OUTPUT_PROTOCOL member functions.
+//
+STATIC
+EFI_STATUS
+EFIAPI
+GopQueryMode (
+ IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
+ IN UINT32 ModeNumber,
+ OUT UINTN *SizeOfInfo,
+ OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION **Info
+ )
+{
+ EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *GopModeInfo;
+
+ if (ModeNumber >= sizeof mGopResolutions / sizeof mGopResolutions[0]) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ GopModeInfo = AllocateZeroPool (sizeof *GopModeInfo);
+ if (GopModeInfo == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ GopModeInfo->HorizontalResolution = mGopResolutions[ModeNumber].Width;
+ GopModeInfo->VerticalResolution = mGopResolutions[ModeNumber].Height;
+ GopModeInfo->PixelFormat = PixelBltOnly;
+ GopModeInfo->PixelsPerScanLine = mGopResolutions[ModeNumber].Width;
+
+ *SizeOfInfo = sizeof *GopModeInfo;
+ *Info = GopModeInfo;
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+GopSetMode (
+ IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
+ IN UINT32 ModeNumber
+ )
+{
+ VGPU_GOP *VgpuGop;
+ UINT32 NewResourceId;
+ UINTN NewNumberOfBytes;
+ UINTN NewNumberOfPages;
+ VOID *NewBackingStore;
+ EFI_STATUS Status;
+ EFI_STATUS Status2;
+
+ if (ModeNumber >= sizeof mGopResolutions / sizeof mGopResolutions[0]) {
+ return EFI_UNSUPPORTED;
+ }
+
+ VgpuGop = VGPU_GOP_FROM_GOP (This);
+
+ //
+ // Distinguish the first (internal) call from the other (protocol consumer)
+ // calls.
+ //
+ if (VgpuGop->ResourceId == 0) {
+ //
+ // Set up the Gop -> GopMode -> GopModeInfo pointer chain, and the other
+ // (nonzero) constant fields.
+ //
+ // No direct framebuffer access is supported, only Blt() is.
+ //
+ VgpuGop->Gop.Mode = &VgpuGop->GopMode;
+
+ VgpuGop->GopMode.MaxMode = (UINT32)(sizeof mGopResolutions /
+ sizeof mGopResolutions[0]);
+ VgpuGop->GopMode.Info = &VgpuGop->GopModeInfo;
+ VgpuGop->GopMode.SizeOfInfo = sizeof VgpuGop->GopModeInfo;
+
+ VgpuGop->GopModeInfo.PixelFormat = PixelBltOnly;
+
+ //
+ // This is the first time we create a host side resource.
+ //
+ NewResourceId = 1;
+ } else {
+ //
+ // We already have an active host side resource. Create the new one without
+ // interfering with the current one, so that we can cleanly bail out on
+ // error, without disturbing the current graphics mode.
+ //
+ // The formula below will alternate between IDs 1 and 2.
+ //
+ NewResourceId = 3 - VgpuGop->ResourceId;
+ }
+
+ //
+ // Create the 2D host resource.
+ //
+ Status = VirtioGpuResourceCreate2d (
+ VgpuGop->ParentBus, // VgpuDev
+ NewResourceId, // ResourceId
+ VirtioGpuFormatB8G8R8X8Unorm, // Format
+ mGopResolutions[ModeNumber].Width, // Width
+ mGopResolutions[ModeNumber].Height // Height
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Allocate guest backing store.
+ //
+ NewNumberOfBytes = mGopResolutions[ModeNumber].Width *
+ mGopResolutions[ModeNumber].Height * sizeof (UINT32);
+ NewNumberOfPages = EFI_SIZE_TO_PAGES (NewNumberOfBytes);
+ NewBackingStore = AllocatePages (NewNumberOfPages);
+ if (NewBackingStore == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto DestroyHostResource;
+ }
+ //
+ // Fill visible part of backing store with black.
+ //
+ ZeroMem (NewBackingStore, NewNumberOfBytes);
+
+ //
+ // Attach backing store to the host resource.
+ //
+ Status = VirtioGpuResourceAttachBacking (
+ VgpuGop->ParentBus, // VgpuDev
+ NewResourceId, // ResourceId
+ NewBackingStore, // FirstBackingPage
+ NewNumberOfPages // NumberOfPages
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeBackingStore;
+ }
+
+ //
+ // Point head (scanout) #0 to the host resource.
+ //
+ Status = VirtioGpuSetScanout (
+ VgpuGop->ParentBus, // VgpuDev
+ 0, // X
+ 0, // Y
+ mGopResolutions[ModeNumber].Width, // Width
+ mGopResolutions[ModeNumber].Height, // Height
+ 0, // ScanoutId
+ NewResourceId // ResourceId
+ );
+ if (EFI_ERROR (Status)) {
+ goto DetachBackingStore;
+ }
+
+ //
+ // If this is not the first (i.e., internal) call, then we have to (a) flush
+ // the new resource to head (scanout) #0, after having flipped the latter to
+ // the former above, plus (b) release the old resources.
+ //
+ if (VgpuGop->ResourceId != 0) {
+ Status = VirtioGpuResourceFlush (
+ VgpuGop->ParentBus, // VgpuDev
+ 0, // X
+ 0, // Y
+ mGopResolutions[ModeNumber].Width, // Width
+ mGopResolutions[ModeNumber].Height, // Height
+ NewResourceId // ResourceId
+ );
+ if (EFI_ERROR (Status)) {
+ //
+ // Flip head (scanout) #0 back to the current resource. If this fails, we
+ // cannot continue, as this error occurs on the error path and is
+ // therefore non-recoverable.
+ //
+ Status2 = VirtioGpuSetScanout (
+ VgpuGop->ParentBus, // VgpuDev
+ 0, // X
+ 0, // Y
+ mGopResolutions[This->Mode->Mode].Width, // Width
+ mGopResolutions[This->Mode->Mode].Height, // Height
+ 0, // ScanoutId
+ VgpuGop->ResourceId // ResourceId
+ );
+ ASSERT_EFI_ERROR (Status2);
+ if (EFI_ERROR (Status2)) {
+ CpuDeadLoop ();
+ }
+ goto DetachBackingStore;
+ }
+
+ //
+ // Flush successful; release the old resources (without disabling head
+ // (scanout) #0).
+ //
+ ReleaseGopResources (VgpuGop, FALSE /* DisableHead */);
+ }
+
+ //
+ // This is either the first (internal) call when we have no old resources
+ // yet, or we've changed the mode successfully and released the old
+ // resources.
+ //
+ ASSERT (VgpuGop->ResourceId == 0);
+ ASSERT (VgpuGop->BackingStore == NULL);
+
+ VgpuGop->ResourceId = NewResourceId;
+ VgpuGop->BackingStore = NewBackingStore;
+ VgpuGop->NumberOfPages = NewNumberOfPages;
+
+ //
+ // Populate Mode and ModeInfo (mutable fields only).
+ //
+ VgpuGop->GopMode.Mode = ModeNumber;
+ VgpuGop->GopModeInfo.HorizontalResolution =
+ mGopResolutions[ModeNumber].Width;
+ VgpuGop->GopModeInfo.VerticalResolution = mGopResolutions[ModeNumber].Height;
+ VgpuGop->GopModeInfo.PixelsPerScanLine = mGopResolutions[ModeNumber].Width;
+ return EFI_SUCCESS;
+
+DetachBackingStore:
+ Status2 = VirtioGpuResourceDetachBacking (VgpuGop->ParentBus, NewResourceId);
+ ASSERT_EFI_ERROR (Status2);
+ if (EFI_ERROR (Status2)) {
+ CpuDeadLoop ();
+ }
+
+FreeBackingStore:
+ FreePages (NewBackingStore, NewNumberOfPages);
+
+DestroyHostResource:
+ Status2 = VirtioGpuResourceUnref (VgpuGop->ParentBus, NewResourceId);
+ ASSERT_EFI_ERROR (Status2);
+ if (EFI_ERROR (Status2)) {
+ CpuDeadLoop ();
+ }
+
+ return Status;
+}
+
+STATIC
+EFI_STATUS
+EFIAPI
+GopBlt (
+ IN EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
+ IN EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer, OPTIONAL
+ IN EFI_GRAPHICS_OUTPUT_BLT_OPERATION BltOperation,
+ IN UINTN SourceX,
+ IN UINTN SourceY,
+ IN UINTN DestinationX,
+ IN UINTN DestinationY,
+ IN UINTN Width,
+ IN UINTN Height,
+ IN UINTN Delta OPTIONAL
+ )
+{
+ VGPU_GOP *VgpuGop;
+ UINT32 CurrentHorizontal;
+ UINT32 CurrentVertical;
+ UINTN SegmentSize;
+ UINTN Y;
+ UINTN ResourceOffset;
+ EFI_STATUS Status;
+
+ VgpuGop = VGPU_GOP_FROM_GOP (This);
+ CurrentHorizontal = VgpuGop->GopModeInfo.HorizontalResolution;
+ CurrentVertical = VgpuGop->GopModeInfo.VerticalResolution;
+
+ //
+ // We can avoid pixel format conversion in the guest because the internal
+ // representation of EFI_GRAPHICS_OUTPUT_BLT_PIXEL and that of
+ // VirtioGpuFormatB8G8R8X8Unorm are identical.
+ //
+ SegmentSize = Width * sizeof (UINT32);
+
+ //
+ // Delta is relevant for operations that read a rectangle from, or write a
+ // rectangle to, BltBuffer.
+ //
+ // In these cases, Delta is the stride of BltBuffer, in bytes. If Delta is
+ // zero, then Width is the entire width of BltBuffer, and the stride is
+ // supposed to be calculated from Width.
+ //
+ if (BltOperation == EfiBltVideoToBltBuffer ||
+ BltOperation == EfiBltBufferToVideo) {
+ if (Delta == 0) {
+ Delta = SegmentSize;
+ }
+ }
+
+ //
+ // For operations that write to the display, check if the destination fits
+ // onto the display.
+ //
+ if (BltOperation == EfiBltVideoFill ||
+ BltOperation == EfiBltBufferToVideo ||
+ BltOperation == EfiBltVideoToVideo) {
+ if (DestinationX > CurrentHorizontal ||
+ Width > CurrentHorizontal - DestinationX ||
+ DestinationY > CurrentVertical ||
+ Height > CurrentVertical - DestinationY) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ //
+ // For operations that read from the display, check if the source fits onto
+ // the display.
+ //
+ if (BltOperation == EfiBltVideoToBltBuffer ||
+ BltOperation == EfiBltVideoToVideo) {
+ if (SourceX > CurrentHorizontal ||
+ Width > CurrentHorizontal - SourceX ||
+ SourceY > CurrentVertical ||
+ Height > CurrentVertical - SourceY) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ //
+ // Render the request. For requests that do not modify the display, there
+ // won't be further steps.
+ //
+ switch (BltOperation) {
+ case EfiBltVideoFill:
+ //
+ // Write data from the BltBuffer pixel (0, 0) directly to every pixel of
+ // the video display rectangle (DestinationX, DestinationY) (DestinationX +
+ // Width, DestinationY + Height). Only one pixel will be used from the
+ // BltBuffer. Delta is NOT used.
+ //
+ for (Y = 0; Y < Height; ++Y) {
+ SetMem32 (
+ VgpuGop->BackingStore +
+ (DestinationY + Y) * CurrentHorizontal + DestinationX,
+ SegmentSize,
+ *(UINT32 *)BltBuffer
+ );
+ }
+ break;
+
+ case EfiBltVideoToBltBuffer:
+ //
+ // Read data from the video display rectangle (SourceX, SourceY) (SourceX +
+ // Width, SourceY + Height) and place it in the BltBuffer rectangle
+ // (DestinationX, DestinationY ) (DestinationX + Width, DestinationY +
+ // Height). If DestinationX or DestinationY is not zero then Delta must be
+ // set to the length in bytes of a row in the BltBuffer.
+ //
+ for (Y = 0; Y < Height; ++Y) {
+ CopyMem (
+ (UINT8 *)BltBuffer +
+ (DestinationY + Y) * Delta + DestinationX * sizeof *BltBuffer,
+ VgpuGop->BackingStore +
+ (SourceY + Y) * CurrentHorizontal + SourceX,
+ SegmentSize
+ );
+ }
+ return EFI_SUCCESS;
+
+ case EfiBltBufferToVideo:
+ //
+ // Write data from the BltBuffer rectangle (SourceX, SourceY) (SourceX +
+ // Width, SourceY + Height) directly to the video display rectangle
+ // (DestinationX, DestinationY) (DestinationX + Width, DestinationY +
+ // Height). If SourceX or SourceY is not zero then Delta must be set to the
+ // length in bytes of a row in the BltBuffer.
+ //
+ for (Y = 0; Y < Height; ++Y) {
+ CopyMem (
+ VgpuGop->BackingStore +
+ (DestinationY + Y) * CurrentHorizontal + DestinationX,
+ (UINT8 *)BltBuffer +
+ (SourceY + Y) * Delta + SourceX * sizeof *BltBuffer,
+ SegmentSize
+ );
+ }
+ break;
+
+ case EfiBltVideoToVideo:
+ //
+ // Copy from the video display rectangle (SourceX, SourceY) (SourceX +
+ // Width, SourceY + Height) to the video display rectangle (DestinationX,
+ // DestinationY) (DestinationX + Width, DestinationY + Height). The
+ // BltBuffer and Delta are not used in this mode.
+ //
+ // A single invocation of CopyMem() handles overlap between source and
+ // destination (that is, within a single line), but for multiple
+ // invocations, we must handle overlaps.
+ //
+ if (SourceY < DestinationY) {
+ Y = Height;
+ while (Y > 0) {
+ --Y;
+ CopyMem (
+ VgpuGop->BackingStore +
+ (DestinationY + Y) * CurrentHorizontal + DestinationX,
+ VgpuGop->BackingStore +
+ (SourceY + Y) * CurrentHorizontal + SourceX,
+ SegmentSize
+ );
+ }
+ } else {
+ for (Y = 0; Y < Height; ++Y) {
+ CopyMem (
+ VgpuGop->BackingStore +
+ (DestinationY + Y) * CurrentHorizontal + DestinationX,
+ VgpuGop->BackingStore +
+ (SourceY + Y) * CurrentHorizontal + SourceX,
+ SegmentSize
+ );
+ }
+ }
+ break;
+
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // For operations that wrote to the display, submit the updated area to the
+ // host -- update the host resource from guest memory.
+ //
+ ResourceOffset = sizeof (UINT32) * (DestinationY * CurrentHorizontal +
+ DestinationX);
+ Status = VirtioGpuTransferToHost2d (
+ VgpuGop->ParentBus, // VgpuDev
+ (UINT32)DestinationX, // X
+ (UINT32)DestinationY, // Y
+ (UINT32)Width, // Width
+ (UINT32)Height, // Height
+ ResourceOffset, // Offset
+ VgpuGop->ResourceId // ResourceId
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Flush the updated resource to the display.
+ //
+ Status = VirtioGpuResourceFlush (
+ VgpuGop->ParentBus, // VgpuDev
+ (UINT32)DestinationX, // X
+ (UINT32)DestinationY, // Y
+ (UINT32)Width, // Width
+ (UINT32)Height, // Height
+ VgpuGop->ResourceId // ResourceId
+ );
+ return Status;
+}
+
+//
+// Template for initializing VGPU_GOP.Gop.
+//
+CONST EFI_GRAPHICS_OUTPUT_PROTOCOL mGopTemplate = {
+ GopQueryMode,
+ GopSetMode,
+ GopBlt,
+ NULL // Mode, to be overwritten in the actual protocol instance
+};
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 11/11] ArmVirtPkg: remove PcdKludgeMapPciMmioAsCached
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (9 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 10/11] OvmfPkg/VirtioGpuDxe: implement EFI_GRAPHICS_OUTPUT_PROTOCOL Laszlo Ersek
@ 2016-08-19 12:49 ` Laszlo Ersek
2016-08-19 13:16 ` Ard Biesheuvel
2016-08-19 13:06 ` [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Ard Biesheuvel
` (3 subsequent siblings)
14 siblings, 1 reply; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 12:49 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Ard Biesheuvel, Jordan Justen
In ARM/AARCH64 guests that run on KVM, we can now use virtio-gpu-pci, so
PcdKludgeMapPciMmioAsCached is no longer necessary. Standard VGA continues
to work on TCG without the kludge.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ArmVirtPkg/ArmVirtPkg.dec | 24 --------------------
ArmVirtPkg/ArmVirtQemu.dsc | 3 ---
ArmVirtPkg/ArmVirtQemuKernel.dsc | 3 ---
ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 -
ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c | 3 +--
5 files changed, 1 insertion(+), 33 deletions(-)
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 9f98f28f01e1..a5ec42166445 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -58,36 +58,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# EFI_VT_100_GUID.
#
gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
[PcdsFeatureFlag]
#
- # "Map PCI MMIO as Cached"
- #
- # Due to the way Stage1 and Stage2 mappings are combined on Aarch64, and
- # because KVM -- for the time being -- does not try to interfere with the
- # Stage1 mappings, we must not set EFI_MEMORY_UC for emulated PCI MMIO
- # regions.
- #
- # EFI_MEMORY_UC is mapped to Device-nGnRnE, and that Stage1 attribute would
- # direct guest writes to host DRAM immediately, bypassing the cache
- # regardless of Stage2 attributes. However, QEMU's reads of the same range
- # can easily be served from the (stale) CPU cache.
- #
- # Setting this PCD to TRUE will use EFI_MEMORY_WB for mapping PCI MMIO
- # regions, which ensures that guest writes to such regions go through the CPU
- # cache. Strictly speaking this is wrong, but it is needed as a temporary
- # workaround for emulated PCI devices. Setting the PCD to FALSE results in
- # the theoretically correct EFI_MEMORY_UC mapping, and should be the long
- # term choice, especially with assigned devices.
- #
- # The default is to turn off the kludge; DSC's can selectively enable it.
- #
- gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|FALSE|BOOLEAN|0x00000006
-
- #
# Pure ACPI boot
#
# Inhibit installation of the FDT as a configuration table if this feature
# PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
# description of the platform, and it is up to the OS to choose.
#
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 2eb2780a1fca..1d459756f61b 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -97,15 +97,12 @@ [PcdsFeatureFlag.common]
## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
# It could be set FALSE to save size.
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
- # Activate KVM workaround for now.
- gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE
-
!if $(PURE_ACPI_BOOT_ENABLE) == TRUE
gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
!endif
[PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PcdCoreCount|1
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index d9e62c78d28e..99d2feba202b 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -98,15 +98,12 @@ [PcdsFeatureFlag.common]
## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
# It could be set FALSE to save size.
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
- # Activate KVM workaround for now.
- gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE
-
[PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PcdCoreCount|1
!if $(ARCH) == AARCH64
gArmTokenSpaceGuid.PcdVFPEnabled|1
!endif
diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
index 41e134b24b0e..8c75eda3deb5 100644
--- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -53,13 +53,12 @@ [Protocols]
gFdtClientProtocolGuid ## CONSUMES
[Pcd]
gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
[FeaturePcd]
- gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached
gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
[depex]
gEfiMetronomeArchProtocolGuid AND
gEfiCpuArchProtocolGuid AND
gFdtClientProtocolGuid
diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
index 4b2b6a562a3f..5063782bb392 100644
--- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
+++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
@@ -362,14 +362,13 @@ InitializePciHostBridge (
EfiGcdIoTypeIo,
IoBase,
IoSize
);
ASSERT_EFI_ERROR (Status);
- MmioAttributes = FeaturePcdGet (PcdKludgeMapPciMmioAsCached) ?
- EFI_MEMORY_WB : EFI_MEMORY_UC;
+ MmioAttributes = EFI_MEMORY_UC;
Status = gDS->AddMemorySpace (
EfiGcdMemoryTypeMemoryMappedIo,
MmioBase,
MmioSize,
MmioAttributes
--
2.9.2
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 11/11] ArmVirtPkg: remove PcdKludgeMapPciMmioAsCached
2016-08-19 12:49 ` [PATCH 11/11] ArmVirtPkg: remove PcdKludgeMapPciMmioAsCached Laszlo Ersek
@ 2016-08-19 13:16 ` Ard Biesheuvel
0 siblings, 0 replies; 34+ messages in thread
From: Ard Biesheuvel @ 2016-08-19 13:16 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen
On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
> In ARM/AARCH64 guests that run on KVM, we can now use virtio-gpu-pci, so
> PcdKludgeMapPciMmioAsCached is no longer necessary. Standard VGA continues
> to work on TCG without the kludge.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=66
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmVirtPkg/ArmVirtPkg.dec | 24 --------------------
> ArmVirtPkg/ArmVirtQemu.dsc | 3 ---
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 3 ---
> ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 -
> ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c | 3 +--
> 5 files changed, 1 insertion(+), 33 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index 9f98f28f01e1..a5ec42166445 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -58,36 +58,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> # EFI_VT_100_GUID.
>
> #
>
> gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007
>
>
>
> [PcdsFeatureFlag]
>
> #
>
> - # "Map PCI MMIO as Cached"
>
> - #
>
> - # Due to the way Stage1 and Stage2 mappings are combined on Aarch64, and
>
> - # because KVM -- for the time being -- does not try to interfere with the
>
> - # Stage1 mappings, we must not set EFI_MEMORY_UC for emulated PCI MMIO
>
> - # regions.
>
> - #
>
> - # EFI_MEMORY_UC is mapped to Device-nGnRnE, and that Stage1 attribute would
>
> - # direct guest writes to host DRAM immediately, bypassing the cache
>
> - # regardless of Stage2 attributes. However, QEMU's reads of the same range
>
> - # can easily be served from the (stale) CPU cache.
>
> - #
>
> - # Setting this PCD to TRUE will use EFI_MEMORY_WB for mapping PCI MMIO
>
> - # regions, which ensures that guest writes to such regions go through the CPU
>
> - # cache. Strictly speaking this is wrong, but it is needed as a temporary
>
> - # workaround for emulated PCI devices. Setting the PCD to FALSE results in
>
> - # the theoretically correct EFI_MEMORY_UC mapping, and should be the long
>
> - # term choice, especially with assigned devices.
>
> - #
>
> - # The default is to turn off the kludge; DSC's can selectively enable it.
>
> - #
>
> - gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|FALSE|BOOLEAN|0x00000006
>
> -
>
> - #
>
> # Pure ACPI boot
>
> #
>
> # Inhibit installation of the FDT as a configuration table if this feature
>
> # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI
>
> # description of the platform, and it is up to the OS to choose.
>
> #
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 2eb2780a1fca..1d459756f61b 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -97,15 +97,12 @@ [PcdsFeatureFlag.common]
>
>
> ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
>
> # It could be set FALSE to save size.
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>
>
>
> - # Activate KVM workaround for now.
>
> - gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE
>
> -
>
> !if $(PURE_ACPI_BOOT_ENABLE) == TRUE
>
> gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE
>
> !endif
>
>
>
> [PcdsFixedAtBuild.common]
>
> gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index d9e62c78d28e..99d2feba202b 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -98,15 +98,12 @@ [PcdsFeatureFlag.common]
>
>
> ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
>
> # It could be set FALSE to save size.
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
>
>
>
> - # Activate KVM workaround for now.
>
> - gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached|TRUE
>
> -
>
> [PcdsFixedAtBuild.common]
>
> gArmPlatformTokenSpaceGuid.PcdCoreCount|1
>
> !if $(ARCH) == AARCH64
>
> gArmTokenSpaceGuid.PcdVFPEnabled|1
>
> !endif
>
>
>
> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> index 41e134b24b0e..8c75eda3deb5 100644
> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> @@ -53,13 +53,12 @@ [Protocols]
> gFdtClientProtocolGuid ## CONSUMES
>
>
>
> [Pcd]
>
> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
>
>
> [FeaturePcd]
>
> - gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached
>
> gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
>
>
>
> [depex]
>
> gEfiMetronomeArchProtocolGuid AND
>
> gEfiCpuArchProtocolGuid AND
>
> gFdtClientProtocolGuid
>
> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
> index 4b2b6a562a3f..5063782bb392 100644
> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
> @@ -362,14 +362,13 @@ InitializePciHostBridge (
> EfiGcdIoTypeIo,
>
> IoBase,
>
> IoSize
>
> );
>
> ASSERT_EFI_ERROR (Status);
>
>
>
> - MmioAttributes = FeaturePcdGet (PcdKludgeMapPciMmioAsCached) ?
>
> - EFI_MEMORY_WB : EFI_MEMORY_UC;
>
> + MmioAttributes = EFI_MEMORY_UC;
>
>
>
> Status = gDS->AddMemorySpace (
>
> EfiGcdMemoryTypeMemoryMappedIo,
>
> MmioBase,
>
> MmioSize,
>
> MmioAttributes
>
> --
> 2.9.2
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (10 preceding siblings ...)
2016-08-19 12:49 ` [PATCH 11/11] ArmVirtPkg: remove PcdKludgeMapPciMmioAsCached Laszlo Ersek
@ 2016-08-19 13:06 ` Ard Biesheuvel
2016-08-19 14:25 ` Laszlo Ersek
2016-08-30 15:07 ` Laszlo Ersek
` (2 subsequent siblings)
14 siblings, 1 reply; 34+ messages in thread
From: Ard Biesheuvel @ 2016-08-19 13:06 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel-01, Jordan Justen
On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> 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.
>
Impressive! I suppose this means no direct frame buffer access for the
OS using the GOP? That is fine with me, btw, after finding out that
VGA is really the only problematic QEMU/KVM device (unlike the
reported USB issues, which were solved by making the PCI RC
dma-coherent in the DT), I think this approach is the best solution,
since OS accessing the GOP is a hack anyway (and breaks with the PCI
reconfiguration that occurs under ARM/Linux, even in the ACPI case,
which I expected would leave the firmware PCI setup alone. /me makes
mental note to revert the 'pci-probe-only' patch)
> 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).
>
OK, so grub drives the framebuffer using Blt(), also on ARM? That is lovely!
> - 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.
>
Indeed. So on ARM, we should drive virtio-gpu in non-VGA mode, using
Blt() equivalent calls.
We should probably propose some patches for Linux then, to make this
the default (or simply the only supported mode)
> - 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! ;)
>
Next time we meet face to face, beers are on me! In fact, there were
some rumors about having our Linaro conference in Budapest next
spring, so I will make sure to put you on the guest list if that
happens.
BTW, your patches look a bit white space impaired to me, i.e., I get
blank lines between each line of patch content.
--
Ard.
> 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
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-19 13:06 ` [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Ard Biesheuvel
@ 2016-08-19 14:25 ` Laszlo Ersek
2016-08-31 20:43 ` Jordan Justen
0 siblings, 1 reply; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-19 14:25 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel-01, Jordan Justen
On 08/19/16 15:06, Ard Biesheuvel wrote:
> On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> 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.
>>
>
> Impressive! I suppose this means no direct frame buffer access for the
> OS using the GOP?
That's correct.
> That is fine with me, btw, after finding out that
> VGA is really the only problematic QEMU/KVM device (unlike the
> reported USB issues, which were solved by making the PCI RC
> dma-coherent in the DT),
Good to know, thanks!
> I think this approach is the best solution,
> since OS accessing the GOP is a hack anyway (and breaks with the PCI
> reconfiguration that occurs under ARM/Linux, even in the ACPI case,
> which I expected would leave the firmware PCI setup alone. /me makes
> mental note to revert the 'pci-probe-only' patch)
The expectation is that the AARCH64 installer media of all guest OSes
should come with a native virtio-gpu-pci driver included. So this driver
should allow the user to boot, and then the native driver should pick up
the GPU even during the initial installation of the guest OS.
[snip]
>> - Tested GRUB, using Fedora 23 / Fedora 24 installer ISOs (both
>> platforms).
>>
>
> OK, so grub drives the framebuffer using Blt(), also on ARM? That is lovely!
At this point I believe that GRUB uses Simple Text Out only. (I've been
confused about this myself. Not a proud moment! :)) That Simple Text Out
protocol is provided by
"MdeModulePkg/Universal/Console/GraphicsConsoleDxe", on top of the GOP.
So GRUB itself shouldn't render graphics, just write text, and the
rendering is done by GraphicsConsoleDxe. GraphicsConsoleDxe apparently
knows how to handle Blt()-only GOPs, which is indeed awesome. :)
(I've probably forgotten ConSplitterDxe out of the middle somewhere
above, but it shouldn't affect the concept that GRUB writes text only,
and GraphicsConsoleDxe translates that to GOP.Blt().)
This should work for any other Simple Text Out-using boot loader as well.
>> - 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.
>>
>
> Indeed. So on ARM, we should drive virtio-gpu in non-VGA mode, using
> Blt() equivalent calls.
> We should probably propose some patches for Linux then, to make this
> the default (or simply the only supported mode)
Graphics Output Protocol is provided by a UEFI_DRIVER, not a
DXE_RUNTIME_DRIVER, so the GOP abstraction is unusable after
ExitBootServices() -- Blt() lives in EfiBootServicesCode type memory.
The framebuffer access works (on x86 :)) only because that doesn't need
*code* from the firmware. The OS's UEFI bootloader reads the framebuffer
base, size, and pixel format from the GOP, and saves the information in
a private data struct. Then it calls ExitBootServices(), and the OS can
poke at the framebuffer directly only because (a) there *is* a
framebuffer to begin with, (b) the private struct from the boot loader
informs the OS where to poke.
So, this driver is absolutely no help at all after ExitBootServices().
Put differently, efifb support for it in Linux is impossible. "efifb"
has "fb" (framebuffer) in its name, but virtio-gpu-pci has no framebuffer.
Just use Linux's native virtio-gpu-pci driver (CONFIG_DRM_VIRTIO_GPU);
it lives at
drivers/gpu/drm/virtio/
in the kernel tree.
Also, virtio-gpu-pci has no VGA mode. Only virtio-vga has. Some of the
commit messages in the series elaborate on this.
Briefly, the virtio-vga device will be bound by QemuVideoDxe, and
utilized through the VGA framebuffer. This allows you to install (x86)
Windows 8 and Windows 10 with virtio-vga, because those OSes *insist* on
inheriting the framebuffer from the GOP. Once you installed the guest
(using slow, direct access to the framebuffer), you can install an
accelerated native virtio driver for the display (if that driver exists,
I don't know).
Corollary: you can't use virtio-vga for AARCH64 guests, because the
framebuffer exposed by QemuVideoDxe won't work well.
Whereas virtio-gpu-pci (which lacks the VGA framebuffer) will be bound
by VirtioGpuDxe. It will not let you install Windows 8 and Windows 10 on
x86, but it will function well on AARCH64, boot time only.
Here's a summary:
- UEFI Linux guest on x86 KVM:
- QXL: QemuVideoDxe works at boot, GOOD
OS has native accelerated driver
- virtio-vga: QemuVideoDxe works at boot, GOOD
OS has native accelerated driver
- virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
OS has native accelerated driver
- UEFI Linux guest on AARCH64 KVM:
- QXL: QemuVideDxe doesn't work well at boot,
OS has native accelerated driver
- virtio-vga: QemuVideDxe doesn't work well at boot,
OS has native accelerated driver
- virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
OS has native accelerated driver
- UEFI Windows 7 guest on x86 KVM:
- QXL: QemuVideDxe works at boot, GOOD
OS installer uses QemuVideDxe's VBE shim,
installed OS can install accelerated driver
- virtio-vga: QemuVideDxe works at boot,
no OS support
- virtio-gpu-pci: VirtioGpuDxe works at boot,
no OS support
- UEFI Windows 8 / 10 guest on x86 KVM:
- QXL: QemuVideDxe works at boot, GOOD
OS installer inherits the framebuffer OK,
installed OS can install accelerated driver
- virtio-vga: QemuVideDxe works at boot, COMING UP?
OS installer inherits the framebuffer OK,
installed OS: no accelerated driver yet (AIUI)
- virtio-gpu-pci: VirtioGpuDxe works at boot,
no OS support
- UEFI Windows ??? guest on AARCH64:
- QXL: QemuVideDxe doesn't work well at boot,
no clue about native OS driver
- virtio-vga: QemuVideDxe doesn't work well at boot,
native driver *should* be on installer media
- virtio-gpu-pci: VirtioGpuDxe works at boot, HOPE
native driver *should* be on installer media
[snip]
> Next time we meet face to face, beers are on me! In fact, there were
> some rumors about having our Linaro conference in Budapest next
> spring, so I will make sure to put you on the guest list if that
> happens.
That would be huge. Please do this! :)
> BTW, your patches look a bit white space impaired to me, i.e., I get
> blank lines between each line of patch content.
Hrmpf, interesting. I did move to a new git version (2.9.2), and I also
set the dependent
[sendemail]
transferEncoding = quoted-printable
option. This prevents mailing list software from messing up the CRLF
line endings on the patches, so "git am" no longer needs --keep-cr, and
even new files, and file deletions, will work correctly.
... I checked my patches: for me they look fine:
- in the mailing list archive:
https://lists.01.org/pipermail/edk2-devel/2016-August/000930.html
- in Thunderbird
- when saved to a local file from ThunderBird
This might need more analysis later, but I'm done for the week, I think
:) I hope you can test the patches by fetching them from my github repo.
Cheers!
Laszlo
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-19 14:25 ` Laszlo Ersek
@ 2016-08-31 20:43 ` Jordan Justen
2016-09-01 7:44 ` Ard Biesheuvel
2016-09-01 16:29 ` Laszlo Ersek
0 siblings, 2 replies; 34+ messages in thread
From: Jordan Justen @ 2016-08-31 20:43 UTC (permalink / raw)
To: Laszlo Ersek, Ard Biesheuvel; +Cc: edk2-devel-01
On 2016-08-19 07:25:54, Laszlo Ersek wrote:
> On 08/19/16 15:06, Ard Biesheuvel wrote:
> > On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> 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.
> >>
> >
> > Impressive! I suppose this means no direct frame buffer access for the
> > OS using the GOP?
>
> That's correct.
>
> > That is fine with me, btw, after finding out that
> > VGA is really the only problematic QEMU/KVM device (unlike the
> > reported USB issues, which were solved by making the PCI RC
> > dma-coherent in the DT),
>
> Good to know, thanks!
>
> > I think this approach is the best solution,
> > since OS accessing the GOP is a hack anyway
How it is a hack? It seems to be pretty standard for graphics devices
to provide a simple framebuffer mode. True, it is not required by the
GOP protocol, but many devices and GOP drivers enable it. Thus, it
seems reasonable for a UEFI OS to take advantage of it while loading
the native driver.
If an OS can't load or find the native driver, the framebuffer also
provides a way to communicate with the user.
> > (and breaks with the PCI
> > reconfiguration that occurs under ARM/Linux, even in the ACPI case,
> > which I expected would leave the firmware PCI setup alone. /me makes
> > mental note to revert the 'pci-probe-only' patch)
>
> The expectation is that the AARCH64 installer media of all guest OSes
> should come with a native virtio-gpu-pci driver included.
Like mentioned above, there are potential cases where the OS may want
to update the screen before loading the the native drivers, or if
loading the native driver failed.
Regarding VirtIo GPU: Shouldn't we wait until it makes it into the
actual specs?
Is there any chance to update the spec provide a simple (directly
scanned out) framebuffer mode?
-Jordan
> So this driver
> should allow the user to boot, and then the native driver should pick up
> the GPU even during the initial installation of the guest OS.
>
> [snip]
>
> >> - Tested GRUB, using Fedora 23 / Fedora 24 installer ISOs (both
> >> platforms).
> >>
> >
> > OK, so grub drives the framebuffer using Blt(), also on ARM? That is lovely!
>
> At this point I believe that GRUB uses Simple Text Out only. (I've been
> confused about this myself. Not a proud moment! :)) That Simple Text Out
> protocol is provided by
> "MdeModulePkg/Universal/Console/GraphicsConsoleDxe", on top of the GOP.
>
> So GRUB itself shouldn't render graphics, just write text, and the
> rendering is done by GraphicsConsoleDxe. GraphicsConsoleDxe apparently
> knows how to handle Blt()-only GOPs, which is indeed awesome. :)
>
> (I've probably forgotten ConSplitterDxe out of the middle somewhere
> above, but it shouldn't affect the concept that GRUB writes text only,
> and GraphicsConsoleDxe translates that to GOP.Blt().)
>
> This should work for any other Simple Text Out-using boot loader as well.
>
> >> - 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.
> >>
> >
> > Indeed. So on ARM, we should drive virtio-gpu in non-VGA mode, using
> > Blt() equivalent calls.
> > We should probably propose some patches for Linux then, to make this
> > the default (or simply the only supported mode)
>
> Graphics Output Protocol is provided by a UEFI_DRIVER, not a
> DXE_RUNTIME_DRIVER, so the GOP abstraction is unusable after
> ExitBootServices() -- Blt() lives in EfiBootServicesCode type memory.
>
> The framebuffer access works (on x86 :)) only because that doesn't need
> *code* from the firmware. The OS's UEFI bootloader reads the framebuffer
> base, size, and pixel format from the GOP, and saves the information in
> a private data struct. Then it calls ExitBootServices(), and the OS can
> poke at the framebuffer directly only because (a) there *is* a
> framebuffer to begin with, (b) the private struct from the boot loader
> informs the OS where to poke.
>
> So, this driver is absolutely no help at all after ExitBootServices().
> Put differently, efifb support for it in Linux is impossible. "efifb"
> has "fb" (framebuffer) in its name, but virtio-gpu-pci has no framebuffer.
>
> Just use Linux's native virtio-gpu-pci driver (CONFIG_DRM_VIRTIO_GPU);
> it lives at
>
> drivers/gpu/drm/virtio/
>
> in the kernel tree.
>
>
> Also, virtio-gpu-pci has no VGA mode. Only virtio-vga has. Some of the
> commit messages in the series elaborate on this.
>
> Briefly, the virtio-vga device will be bound by QemuVideoDxe, and
> utilized through the VGA framebuffer. This allows you to install (x86)
> Windows 8 and Windows 10 with virtio-vga, because those OSes *insist* on
> inheriting the framebuffer from the GOP. Once you installed the guest
> (using slow, direct access to the framebuffer), you can install an
> accelerated native virtio driver for the display (if that driver exists,
> I don't know).
>
> Corollary: you can't use virtio-vga for AARCH64 guests, because the
> framebuffer exposed by QemuVideoDxe won't work well.
>
> Whereas virtio-gpu-pci (which lacks the VGA framebuffer) will be bound
> by VirtioGpuDxe. It will not let you install Windows 8 and Windows 10 on
> x86, but it will function well on AARCH64, boot time only.
>
> Here's a summary:
> - UEFI Linux guest on x86 KVM:
> - QXL: QemuVideoDxe works at boot, GOOD
> OS has native accelerated driver
> - virtio-vga: QemuVideoDxe works at boot, GOOD
> OS has native accelerated driver
> - virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
> OS has native accelerated driver
>
> - UEFI Linux guest on AARCH64 KVM:
> - QXL: QemuVideDxe doesn't work well at boot,
> OS has native accelerated driver
> - virtio-vga: QemuVideDxe doesn't work well at boot,
> OS has native accelerated driver
> - virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
> OS has native accelerated driver
>
> - UEFI Windows 7 guest on x86 KVM:
> - QXL: QemuVideDxe works at boot, GOOD
> OS installer uses QemuVideDxe's VBE shim,
> installed OS can install accelerated driver
> - virtio-vga: QemuVideDxe works at boot,
> no OS support
> - virtio-gpu-pci: VirtioGpuDxe works at boot,
> no OS support
>
> - UEFI Windows 8 / 10 guest on x86 KVM:
> - QXL: QemuVideDxe works at boot, GOOD
> OS installer inherits the framebuffer OK,
> installed OS can install accelerated driver
> - virtio-vga: QemuVideDxe works at boot, COMING UP?
> OS installer inherits the framebuffer OK,
> installed OS: no accelerated driver yet (AIUI)
> - virtio-gpu-pci: VirtioGpuDxe works at boot,
> no OS support
>
> - UEFI Windows ??? guest on AARCH64:
> - QXL: QemuVideDxe doesn't work well at boot,
> no clue about native OS driver
> - virtio-vga: QemuVideDxe doesn't work well at boot,
> native driver *should* be on installer media
> - virtio-gpu-pci: VirtioGpuDxe works at boot, HOPE
> native driver *should* be on installer media
>
> [snip]
>
> > Next time we meet face to face, beers are on me! In fact, there were
> > some rumors about having our Linaro conference in Budapest next
> > spring, so I will make sure to put you on the guest list if that
> > happens.
>
> That would be huge. Please do this! :)
>
> > BTW, your patches look a bit white space impaired to me, i.e., I get
> > blank lines between each line of patch content.
>
> Hrmpf, interesting. I did move to a new git version (2.9.2), and I also
> set the dependent
>
> [sendemail]
> transferEncoding = quoted-printable
>
> option. This prevents mailing list software from messing up the CRLF
> line endings on the patches, so "git am" no longer needs --keep-cr, and
> even new files, and file deletions, will work correctly.
>
> ... I checked my patches: for me they look fine:
> - in the mailing list archive:
> https://lists.01.org/pipermail/edk2-devel/2016-August/000930.html
> - in Thunderbird
> - when saved to a local file from ThunderBird
>
> This might need more analysis later, but I'm done for the week, I think
> :) I hope you can test the patches by fetching them from my github repo.
>
> Cheers!
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-31 20:43 ` Jordan Justen
@ 2016-09-01 7:44 ` Ard Biesheuvel
2016-09-01 16:48 ` Laszlo Ersek
2016-09-01 16:29 ` Laszlo Ersek
1 sibling, 1 reply; 34+ messages in thread
From: Ard Biesheuvel @ 2016-09-01 7:44 UTC (permalink / raw)
To: Jordan Justen; +Cc: Laszlo Ersek, edk2-devel-01
On 31 August 2016 at 21:43, Jordan Justen <jordan.l.justen@intel.com> wrote:
> On 2016-08-19 07:25:54, Laszlo Ersek wrote:
>> On 08/19/16 15:06, Ard Biesheuvel wrote:
>> > On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> 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.
>> >>
>> >
>> > Impressive! I suppose this means no direct frame buffer access for the
>> > OS using the GOP?
>>
>> That's correct.
>>
>> > That is fine with me, btw, after finding out that
>> > VGA is really the only problematic QEMU/KVM device (unlike the
>> > reported USB issues, which were solved by making the PCI RC
>> > dma-coherent in the DT),
>>
>> Good to know, thanks!
>>
>> > I think this approach is the best solution,
>> > since OS accessing the GOP is a hack anyway
>
> How it is a hack? It seems to be pretty standard for graphics devices
> to provide a simple framebuffer mode. True, it is not required by the
> GOP protocol, but many devices and GOP drivers enable it. Thus, it
> seems reasonable for a UEFI OS to take advantage of it while loading
> the native driver.
>
Because ExitBootServices() tears down the whole driver stack, protocol
database, etc but leaves a single struct in place which describes a
framebuffer whose methods are now inoperable but which can be driven
in the mode that the firmware happened to leave it in. Furthermore,
there is no context anymore that describes which device owns the
framebuffer, and so it is not generally possible to decide if it is
safe to reconfigure any part of the PCI layer without interfering with
the framebuffer mapping.
So yes, it is a hack. A useful one, but still a hack.
> If an OS can't load or find the native driver, the framebuffer also
> provides a way to communicate with the user.
>
Of course.
>> > (and breaks with the PCI
>> > reconfiguration that occurs under ARM/Linux, even in the ACPI case,
>> > which I expected would leave the firmware PCI setup alone. /me makes
>> > mental note to revert the 'pci-probe-only' patch)
>>
>> The expectation is that the AARCH64 installer media of all guest OSes
>> should come with a native virtio-gpu-pci driver included.
>
> Like mentioned above, there are potential cases where the OS may want
> to update the screen before loading the the native drivers, or if
> loading the native driver failed.
>
Yes, but this is fundamentally problematic on ARM under
virtualization. Emulated framebuffers are backed by host memory, which
is mapped cacheable. Typical framebuffer mapping guest code uses
uncached or write-combining mappings, which are incoherent with the
host mapping, which means the host does not get to see what the guest
puts into the framebuffer without major surgery. This means the
standard VGA QEMU device is unusable on ARM with KVM acceleration.
> Regarding VirtIo GPU: Shouldn't we wait until it makes it into the
> actual specs?
>
As I explained above, virtio-gpu support without the framebuffer is
indispensable for supporting graphics under QEMU/KVM. So I would
rather have this in sooner than later, either under OvmfPkg or
ArmVirtPkg. Also, I do believe that it is generally useful to make
implementations such as this one widely available especially when the
spec is not finalized yet, so the additional exposure may help in
validation before it is set in stone.
> Is there any chance to update the spec provide a simple (directly
> scanned out) framebuffer mode?
>
Lack of a framebuffer is a deliberate choice. If you need a
framebuffer, you can use the virtio-vga flavor, which exposes a VGA
compatible framebuffer + registers in addition to the standard virtio
GPU.
>> So this driver
>> should allow the user to boot, and then the native driver should pick up
>> the GPU even during the initial installation of the guest OS.
>>
>> [snip]
>>
>> >> - Tested GRUB, using Fedora 23 / Fedora 24 installer ISOs (both
>> >> platforms).
>> >>
>> >
>> > OK, so grub drives the framebuffer using Blt(), also on ARM? That is lovely!
>>
>> At this point I believe that GRUB uses Simple Text Out only. (I've been
>> confused about this myself. Not a proud moment! :)) That Simple Text Out
>> protocol is provided by
>> "MdeModulePkg/Universal/Console/GraphicsConsoleDxe", on top of the GOP.
>>
>> So GRUB itself shouldn't render graphics, just write text, and the
>> rendering is done by GraphicsConsoleDxe. GraphicsConsoleDxe apparently
>> knows how to handle Blt()-only GOPs, which is indeed awesome. :)
>>
>> (I've probably forgotten ConSplitterDxe out of the middle somewhere
>> above, but it shouldn't affect the concept that GRUB writes text only,
>> and GraphicsConsoleDxe translates that to GOP.Blt().)
>>
>> This should work for any other Simple Text Out-using boot loader as well.
>>
>> >> - 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.
>> >>
>> >
>> > Indeed. So on ARM, we should drive virtio-gpu in non-VGA mode, using
>> > Blt() equivalent calls.
>> > We should probably propose some patches for Linux then, to make this
>> > the default (or simply the only supported mode)
>>
>> Graphics Output Protocol is provided by a UEFI_DRIVER, not a
>> DXE_RUNTIME_DRIVER, so the GOP abstraction is unusable after
>> ExitBootServices() -- Blt() lives in EfiBootServicesCode type memory.
>>
>> The framebuffer access works (on x86 :)) only because that doesn't need
>> *code* from the firmware. The OS's UEFI bootloader reads the framebuffer
>> base, size, and pixel format from the GOP, and saves the information in
>> a private data struct. Then it calls ExitBootServices(), and the OS can
>> poke at the framebuffer directly only because (a) there *is* a
>> framebuffer to begin with, (b) the private struct from the boot loader
>> informs the OS where to poke.
>>
>> So, this driver is absolutely no help at all after ExitBootServices().
>> Put differently, efifb support for it in Linux is impossible. "efifb"
>> has "fb" (framebuffer) in its name, but virtio-gpu-pci has no framebuffer.
>>
>> Just use Linux's native virtio-gpu-pci driver (CONFIG_DRM_VIRTIO_GPU);
>> it lives at
>>
>> drivers/gpu/drm/virtio/
>>
>> in the kernel tree.
>>
>>
>> Also, virtio-gpu-pci has no VGA mode. Only virtio-vga has. Some of the
>> commit messages in the series elaborate on this.
>>
>> Briefly, the virtio-vga device will be bound by QemuVideoDxe, and
>> utilized through the VGA framebuffer. This allows you to install (x86)
>> Windows 8 and Windows 10 with virtio-vga, because those OSes *insist* on
>> inheriting the framebuffer from the GOP. Once you installed the guest
>> (using slow, direct access to the framebuffer), you can install an
>> accelerated native virtio driver for the display (if that driver exists,
>> I don't know).
>>
>> Corollary: you can't use virtio-vga for AARCH64 guests, because the
>> framebuffer exposed by QemuVideoDxe won't work well.
>>
>> Whereas virtio-gpu-pci (which lacks the VGA framebuffer) will be bound
>> by VirtioGpuDxe. It will not let you install Windows 8 and Windows 10 on
>> x86, but it will function well on AARCH64, boot time only.
>>
>> Here's a summary:
>> - UEFI Linux guest on x86 KVM:
>> - QXL: QemuVideoDxe works at boot, GOOD
>> OS has native accelerated driver
>> - virtio-vga: QemuVideoDxe works at boot, GOOD
>> OS has native accelerated driver
>> - virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
>> OS has native accelerated driver
>>
>> - UEFI Linux guest on AARCH64 KVM:
>> - QXL: QemuVideDxe doesn't work well at boot,
>> OS has native accelerated driver
>> - virtio-vga: QemuVideDxe doesn't work well at boot,
>> OS has native accelerated driver
>> - virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
>> OS has native accelerated driver
>>
>> - UEFI Windows 7 guest on x86 KVM:
>> - QXL: QemuVideDxe works at boot, GOOD
>> OS installer uses QemuVideDxe's VBE shim,
>> installed OS can install accelerated driver
>> - virtio-vga: QemuVideDxe works at boot,
>> no OS support
>> - virtio-gpu-pci: VirtioGpuDxe works at boot,
>> no OS support
>>
>> - UEFI Windows 8 / 10 guest on x86 KVM:
>> - QXL: QemuVideDxe works at boot, GOOD
>> OS installer inherits the framebuffer OK,
>> installed OS can install accelerated driver
>> - virtio-vga: QemuVideDxe works at boot, COMING UP?
>> OS installer inherits the framebuffer OK,
>> installed OS: no accelerated driver yet (AIUI)
>> - virtio-gpu-pci: VirtioGpuDxe works at boot,
>> no OS support
>>
>> - UEFI Windows ??? guest on AARCH64:
>> - QXL: QemuVideDxe doesn't work well at boot,
>> no clue about native OS driver
>> - virtio-vga: QemuVideDxe doesn't work well at boot,
>> native driver *should* be on installer media
>> - virtio-gpu-pci: VirtioGpuDxe works at boot, HOPE
>> native driver *should* be on installer media
>>
>> [snip]
>>
>> > Next time we meet face to face, beers are on me! In fact, there were
>> > some rumors about having our Linaro conference in Budapest next
>> > spring, so I will make sure to put you on the guest list if that
>> > happens.
>>
>> That would be huge. Please do this! :)
>>
>> > BTW, your patches look a bit white space impaired to me, i.e., I get
>> > blank lines between each line of patch content.
>>
>> Hrmpf, interesting. I did move to a new git version (2.9.2), and I also
>> set the dependent
>>
>> [sendemail]
>> transferEncoding = quoted-printable
>>
>> option. This prevents mailing list software from messing up the CRLF
>> line endings on the patches, so "git am" no longer needs --keep-cr, and
>> even new files, and file deletions, will work correctly.
>>
>> ... I checked my patches: for me they look fine:
>> - in the mailing list archive:
>> https://lists.01.org/pipermail/edk2-devel/2016-August/000930.html
>> - in Thunderbird
>> - when saved to a local file from ThunderBird
>>
>> This might need more analysis later, but I'm done for the week, I think
>> :) I hope you can test the patches by fetching them from my github repo.
>>
>> Cheers!
>> Laszlo
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 7:44 ` Ard Biesheuvel
@ 2016-09-01 16:48 ` Laszlo Ersek
0 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-09-01 16:48 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Jordan Justen, edk2-devel-01
On 09/01/16 09:44, Ard Biesheuvel wrote:
> On 31 August 2016 at 21:43, Jordan Justen <jordan.l.justen@intel.com> wrote:
>> On 2016-08-19 07:25:54, Laszlo Ersek wrote:
>>> On 08/19/16 15:06, Ard Biesheuvel wrote:
>>>> On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> 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.
>>>>>
>>>>
>>>> Impressive! I suppose this means no direct frame buffer access for the
>>>> OS using the GOP?
>>>
>>> That's correct.
>>>
>>>> That is fine with me, btw, after finding out that
>>>> VGA is really the only problematic QEMU/KVM device (unlike the
>>>> reported USB issues, which were solved by making the PCI RC
>>>> dma-coherent in the DT),
>>>
>>> Good to know, thanks!
>>>
>>>> I think this approach is the best solution,
>>>> since OS accessing the GOP is a hack anyway
>>
>> How it is a hack? It seems to be pretty standard for graphics devices
>> to provide a simple framebuffer mode. True, it is not required by the
>> GOP protocol, but many devices and GOP drivers enable it. Thus, it
>> seems reasonable for a UEFI OS to take advantage of it while loading
>> the native driver.
>>
>
> Because ExitBootServices() tears down the whole driver stack, protocol
> database, etc but leaves a single struct in place which describes a
> framebuffer whose methods are now inoperable but which can be driven
> in the mode that the firmware happened to leave it in. Furthermore,
> there is no context anymore that describes which device owns the
> framebuffer, and so it is not generally possible to decide if it is
> safe to reconfigure any part of the PCI layer without interfering with
> the framebuffer mapping.
>
> So yes, it is a hack. A useful one, but still a hack.
I can't disagree with this argument either! ;)
The real deal-breaker for me is naturally the fact that the framebuffer
address inherited from the GOP almost universally points into some PCI
device's MMIO BAR. If the guest runtime OS decides to re-enumerate PCI
resources, the LFB address from the GOP will point into outer space.
>
>> If an OS can't load or find the native driver, the framebuffer also
>> provides a way to communicate with the user.
>>
>
> Of course.
>
>>>> (and breaks with the PCI
>>>> reconfiguration that occurs under ARM/Linux, even in the ACPI case,
>>>> which I expected would leave the firmware PCI setup alone. /me makes
>>>> mental note to revert the 'pci-probe-only' patch)
>>>
>>> The expectation is that the AARCH64 installer media of all guest OSes
>>> should come with a native virtio-gpu-pci driver included.
>>
>> Like mentioned above, there are potential cases where the OS may want
>> to update the screen before loading the the native drivers, or if
>> loading the native driver failed.
>>
>
> Yes, but this is fundamentally problematic on ARM under
> virtualization. Emulated framebuffers are backed by host memory, which
> is mapped cacheable. Typical framebuffer mapping guest code uses
> uncached or write-combining mappings, which are incoherent with the
> host mapping, which means the host does not get to see what the guest
> puts into the framebuffer without major surgery. This means the
> standard VGA QEMU device is unusable on ARM with KVM acceleration.
Thanks for spelling out the details, and apologies that I described the
same as you already had described -- I did it superfluously *and* less
precisely :)
>> Regarding VirtIo GPU: Shouldn't we wait until it makes it into the
>> actual specs?
>>
>
> As I explained above, virtio-gpu support without the framebuffer is
> indispensable for supporting graphics under QEMU/KVM. So I would
> rather have this in sooner than later, either under OvmfPkg or
> ArmVirtPkg.
I'd prefer to keep it under OvmfPkg, because it does work well for
x86_64 KVM guests too (if you specify "-device virtio-gpu-pci").
However, I do agree that keeping the driver under ArmVirtPkg would make
*perfect* sense:
- for x86_64 KVM guests, we recommend QXL or virtio-vga anyway, for
better compatibility with Windows 8 / Windows 10 (for Windows 7, QXL
or stdvga), which are all bound by QemuVideoDxe,
- while for aarch64 KVM guests, virtio-gpu-pci (bound by VirtioGpuDxe)
is the only choice.
In practice, the separation between QemuVideoDxe and VirtioGpuDxe is a
very clear one: do you need (and can have) a linear framebuffer, based
on your guest architecture?
- If so, use QemuVideoDxe, with Cirrus / stdvga / QXL / virtio-vga (from
these, pick dependent on other factors, like guest OS driver support, S3
support, etc).
- Otherwise, use VirtioGpuDxe, with virtio-gpu-pci.
So, an argument can certainly be made that VirtioGpuDxe be included in
the ArmVirtQemu DSC / FDF files *only*, and that VirtioGpuDxe actually
*replace* QemuVideoDxe in the ArmVirtQemu DSC / FDF files. (The current
series doesn't remove QemuVideoDxe from ArmVirtQemu, because with TCG
*emulation*, the QXL / stdvga framebuffer happens to function. But, for
"production use", QemuVideoDxe is certainly useless in ArmVirtQemu.)
> Also, I do believe that it is generally useful to make
> implementations such as this one widely available especially when the
> spec is not finalized yet, so the additional exposure may help in
> validation before it is set in stone.
I agree absolutely. I can honestly claim that I made the same argument
in my previous email independently, not having read yours yet.
>> Is there any chance to update the spec provide a simple (directly
>> scanned out) framebuffer mode?
>>
>
> Lack of a framebuffer is a deliberate choice. If you need a
> framebuffer, you can use the virtio-vga flavor, which exposes a VGA
> compatible framebuffer + registers in addition to the standard virtio
> GPU.
Glad we agree on this one too! :)
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-31 20:43 ` Jordan Justen
2016-09-01 7:44 ` Ard Biesheuvel
@ 2016-09-01 16:29 ` Laszlo Ersek
2016-09-01 18:03 ` Jordan Justen
1 sibling, 1 reply; 34+ messages in thread
From: Laszlo Ersek @ 2016-09-01 16:29 UTC (permalink / raw)
To: Jordan Justen; +Cc: Ard Biesheuvel, edk2-devel-01
On 08/31/16 22:43, Jordan Justen wrote:
> On 2016-08-19 07:25:54, Laszlo Ersek wrote:
>> On 08/19/16 15:06, Ard Biesheuvel wrote:
>>> On 19 August 2016 at 14:49, Laszlo Ersek <lersek@redhat.com> 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.
>>>>
>>>
>>> Impressive! I suppose this means no direct frame buffer access for the
>>> OS using the GOP?
>>
>> That's correct.
>>
>>> That is fine with me, btw, after finding out that
>>> VGA is really the only problematic QEMU/KVM device (unlike the
>>> reported USB issues, which were solved by making the PCI RC
>>> dma-coherent in the DT),
>>
>> Good to know, thanks!
>>
>>> I think this approach is the best solution,
>>> since OS accessing the GOP is a hack anyway
>
> How it is a hack? It seems to be pretty standard for graphics devices
> to provide a simple framebuffer mode. True, it is not required by the
> GOP protocol, but many devices and GOP drivers enable it. Thus, it
> seems reasonable for a UEFI OS to take advantage of it while loading
> the native driver.
>
> If an OS can't load or find the native driver, the framebuffer also
> provides a way to communicate with the user.
I agree that exposing the framebuffer address through the GOP (at boot
time) so that the runtime OS can directly access the framebuffer (at
runtime) is useful. For example, in Windows 8 and Windows 10 guests,
this is how QXL can be initially used, interactively, until the native
QXL drivers are installed.
I also agree that making a directly accessible framebuffer an *optional*
feature in the UEFI spec was the right thing to do! Graphics output
*should* be supportable at least at boot time even if the device lacks a
linear framebuffer (and even if the above OS fallback will therefore not
work).
>>> (and breaks with the PCI
>>> reconfiguration that occurs under ARM/Linux, even in the ACPI case,
>>> which I expected would leave the firmware PCI setup alone. /me makes
>>> mental note to revert the 'pci-probe-only' patch)
>>
>> The expectation is that the AARCH64 installer media of all guest OSes
>> should come with a native virtio-gpu-pci driver included.
>
> Like mentioned above, there are potential cases where the OS may want
> to update the screen before loading the the native drivers, or if
> loading the native driver failed.
>
> Regarding VirtIo GPU: Shouldn't we wait until it makes it into the
> actual specs?
No, we should not, at this point.
Believe it or not :), this was also my first request before embarking on
the implementation:
(1) https://www.mail-archive.com/edk2-devel@lists.01.org/msg12348.html
On 05/19/16 12:04, Laszlo Ersek wrote:
> The longer-term idea is a driver for QEMU's virtio-gpu device. The
> specification of that QEMU device is under review for the official
> virtio spec. Once it is accepted, I plan to implement a virtio-gpu
> driver for OvmfPkg and ArmVirtPkg. With that device and driver, we can
> (hopefully!) drop support for the legacy VGA device in ArmVirtPkg.
(2) https://www.mail-archive.com/kvmarm@lists.cs.columbia.edu/msg07061.html
On 06/28/16 13:02, Laszlo Ersek wrote:
> I've offered to (attempt to) implement a GOP driver for virtio-gpu, to
> be used by OvmfPkg and ArmVirtPkg, once the virtio-gpu bits become
> part of the official virtio specification.
(These are just the two public references I could find now, but I had
named the virtio GPU spec becoming official as a "requirement" elsewhere
too, before implementing the driver.)
However, things have changed recently. I've received pings from three
organizations that the driver has become more urgent for them. Combined
with the facts that the 2D feature set of virtio-gpu has been part of
QEMU since 2.4 [1], and a 2D Linux guest driver has existed for
virtio-gpu since Linux 4.2 [2] [3], it seemed justified to shift the
focus to working code, rather than standardization.
[1] http://wiki.qemu.org/ChangeLog/2.4#virtio
[2]
https://kvmforum2016.sched.org/event/7L4X/qemu-graphics-update-2016-gerd-hoffmann-red-hat
[3] https://www.kraxel.org/slides/qemu-gfx-2016/#virtio-gpu-guest
(It is generally helpful to have working guest drivers in at least one
system before standardizing a virtio device -- in that regard the
OvmfPkg driver could even be considered "in support of" the
standardization effort!)
> Is there any chance to update the spec provide a simple (directly
> scanned out) framebuffer mode?
As described in both the spec and several places in this series, such a
device already exists (both specified and implemented): it is called
"virtio-vga". "Virtio-vga" is the combination of "virtio-gpu-pci" plus a
legacy VGA framebuffer (the one you also get with QXL and stdvga).
It is exactly this framebuffer that breaks in aarch64 KVM guests, due to
architectural / KVM cache coherence issues (= as in, stuff that has been
shipped in physical hardware already). For that reason, the
"qemu-system-aarch64" binary, unlike "qemu-system-x86_64", does not even
include the "virtio-vga" device model -- the framebuffer would be broken
*anyway* on KVM. Instead, there we have to use "virtio-gpu-pci".
In other words, lacking a framebuffer is a *bonus* (which side-steps the
aarch64 architectural / KVM problem).
I already provided a "map" of what devices and drivers we should use in
what kinds of guest, in the message you replied to. I agree it was
somewhat large, so just to emphasize the main points, for when the patch
series is applied:
- "virtio-vga" is available in x86_64 KVM guests. In the firmware,
QemuVideoDxe drives it, using and exposing the linear framebuffer. The
linear framebuffer works well, so Windows 8 and Windows 10 guests can
use the display, even without having native drivers. Linux guests have
native drivers.
- "virtio-vga" is unavailable in aarch64 KVM guests (and even if it was
available, the framebuffer would be broken). Therefore "virtio-gpu-pci"
should be used. In the firmware, VirtioGpuDxe drives it, providing a
Blt()-only GOP. Linux guests have a native driver. The status of Windows
guests is unknown, but we expect / hope that the installer media will
come with native virtio-gpu-pci drivers.
Thanks
Laszlo
> -Jordan
>
>> So this driver
>> should allow the user to boot, and then the native driver should pick up
>> the GPU even during the initial installation of the guest OS.
>>
>> [snip]
>>
>>>> - Tested GRUB, using Fedora 23 / Fedora 24 installer ISOs (both
>>>> platforms).
>>>>
>>>
>>> OK, so grub drives the framebuffer using Blt(), also on ARM? That is lovely!
>>
>> At this point I believe that GRUB uses Simple Text Out only. (I've been
>> confused about this myself. Not a proud moment! :)) That Simple Text Out
>> protocol is provided by
>> "MdeModulePkg/Universal/Console/GraphicsConsoleDxe", on top of the GOP.
>>
>> So GRUB itself shouldn't render graphics, just write text, and the
>> rendering is done by GraphicsConsoleDxe. GraphicsConsoleDxe apparently
>> knows how to handle Blt()-only GOPs, which is indeed awesome. :)
>>
>> (I've probably forgotten ConSplitterDxe out of the middle somewhere
>> above, but it shouldn't affect the concept that GRUB writes text only,
>> and GraphicsConsoleDxe translates that to GOP.Blt().)
>>
>> This should work for any other Simple Text Out-using boot loader as well.
>>
>>>> - 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.
>>>>
>>>
>>> Indeed. So on ARM, we should drive virtio-gpu in non-VGA mode, using
>>> Blt() equivalent calls.
>>> We should probably propose some patches for Linux then, to make this
>>> the default (or simply the only supported mode)
>>
>> Graphics Output Protocol is provided by a UEFI_DRIVER, not a
>> DXE_RUNTIME_DRIVER, so the GOP abstraction is unusable after
>> ExitBootServices() -- Blt() lives in EfiBootServicesCode type memory.
>>
>> The framebuffer access works (on x86 :)) only because that doesn't need
>> *code* from the firmware. The OS's UEFI bootloader reads the framebuffer
>> base, size, and pixel format from the GOP, and saves the information in
>> a private data struct. Then it calls ExitBootServices(), and the OS can
>> poke at the framebuffer directly only because (a) there *is* a
>> framebuffer to begin with, (b) the private struct from the boot loader
>> informs the OS where to poke.
>>
>> So, this driver is absolutely no help at all after ExitBootServices().
>> Put differently, efifb support for it in Linux is impossible. "efifb"
>> has "fb" (framebuffer) in its name, but virtio-gpu-pci has no framebuffer.
>>
>> Just use Linux's native virtio-gpu-pci driver (CONFIG_DRM_VIRTIO_GPU);
>> it lives at
>>
>> drivers/gpu/drm/virtio/
>>
>> in the kernel tree.
>>
>>
>> Also, virtio-gpu-pci has no VGA mode. Only virtio-vga has. Some of the
>> commit messages in the series elaborate on this.
>>
>> Briefly, the virtio-vga device will be bound by QemuVideoDxe, and
>> utilized through the VGA framebuffer. This allows you to install (x86)
>> Windows 8 and Windows 10 with virtio-vga, because those OSes *insist* on
>> inheriting the framebuffer from the GOP. Once you installed the guest
>> (using slow, direct access to the framebuffer), you can install an
>> accelerated native virtio driver for the display (if that driver exists,
>> I don't know).
>>
>> Corollary: you can't use virtio-vga for AARCH64 guests, because the
>> framebuffer exposed by QemuVideoDxe won't work well.
>>
>> Whereas virtio-gpu-pci (which lacks the VGA framebuffer) will be bound
>> by VirtioGpuDxe. It will not let you install Windows 8 and Windows 10 on
>> x86, but it will function well on AARCH64, boot time only.
>>
>> Here's a summary:
>> - UEFI Linux guest on x86 KVM:
>> - QXL: QemuVideoDxe works at boot, GOOD
>> OS has native accelerated driver
>> - virtio-vga: QemuVideoDxe works at boot, GOOD
>> OS has native accelerated driver
>> - virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
>> OS has native accelerated driver
>>
>> - UEFI Linux guest on AARCH64 KVM:
>> - QXL: QemuVideDxe doesn't work well at boot,
>> OS has native accelerated driver
>> - virtio-vga: QemuVideDxe doesn't work well at boot,
>> OS has native accelerated driver
>> - virtio-gpu-pci: VirtioGpuDxe works at boot, GOOD
>> OS has native accelerated driver
>>
>> - UEFI Windows 7 guest on x86 KVM:
>> - QXL: QemuVideDxe works at boot, GOOD
>> OS installer uses QemuVideDxe's VBE shim,
>> installed OS can install accelerated driver
>> - virtio-vga: QemuVideDxe works at boot,
>> no OS support
>> - virtio-gpu-pci: VirtioGpuDxe works at boot,
>> no OS support
>>
>> - UEFI Windows 8 / 10 guest on x86 KVM:
>> - QXL: QemuVideDxe works at boot, GOOD
>> OS installer inherits the framebuffer OK,
>> installed OS can install accelerated driver
>> - virtio-vga: QemuVideDxe works at boot, COMING UP?
>> OS installer inherits the framebuffer OK,
>> installed OS: no accelerated driver yet (AIUI)
>> - virtio-gpu-pci: VirtioGpuDxe works at boot,
>> no OS support
>>
>> - UEFI Windows ??? guest on AARCH64:
>> - QXL: QemuVideDxe doesn't work well at boot,
>> no clue about native OS driver
>> - virtio-vga: QemuVideDxe doesn't work well at boot,
>> native driver *should* be on installer media
>> - virtio-gpu-pci: VirtioGpuDxe works at boot, HOPE
>> native driver *should* be on installer media
>>
>> [snip]
>>
>>> Next time we meet face to face, beers are on me! In fact, there were
>>> some rumors about having our Linaro conference in Budapest next
>>> spring, so I will make sure to put you on the guest list if that
>>> happens.
>>
>> That would be huge. Please do this! :)
>>
>>> BTW, your patches look a bit white space impaired to me, i.e., I get
>>> blank lines between each line of patch content.
>>
>> Hrmpf, interesting. I did move to a new git version (2.9.2), and I also
>> set the dependent
>>
>> [sendemail]
>> transferEncoding = quoted-printable
>>
>> option. This prevents mailing list software from messing up the CRLF
>> line endings on the patches, so "git am" no longer needs --keep-cr, and
>> even new files, and file deletions, will work correctly.
>>
>> ... I checked my patches: for me they look fine:
>> - in the mailing list archive:
>> https://lists.01.org/pipermail/edk2-devel/2016-August/000930.html
>> - in Thunderbird
>> - when saved to a local file from ThunderBird
>>
>> This might need more analysis later, but I'm done for the week, I think
>> :) I hope you can test the patches by fetching them from my github repo.
>>
>> Cheers!
>> Laszlo
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 16:29 ` Laszlo Ersek
@ 2016-09-01 18:03 ` Jordan Justen
2016-09-01 18:46 ` Laszlo Ersek
0 siblings, 1 reply; 34+ messages in thread
From: Jordan Justen @ 2016-09-01 18:03 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Ard Biesheuvel, edk2-devel-01
On 2016-09-01 09:29:28, Laszlo Ersek wrote:
> On 08/31/16 22:43, Jordan Justen wrote:
> >
> > If an OS can't load or find the native driver, the framebuffer also
> > provides a way to communicate with the user.
>
> I agree that exposing the framebuffer address through the GOP (at boot
> time) so that the runtime OS can directly access the framebuffer (at
> runtime) is useful. For example, in Windows 8 and Windows 10 guests,
> this is how QXL can be initially used, interactively, until the native
> QXL drivers are installed.
>
> I also agree that making a directly accessible framebuffer an *optional*
I can't dispute that it is optional by UEFI spec, but I think it is
commonly available (and for good reason).
> feature in the UEFI spec was the right thing to do! Graphics output
> *should* be supportable at least at boot time even if the device lacks a
> linear framebuffer (and even if the above OS fallback will therefore not
> work).
>
Isn't there generally a window after ExitBootServices is called, and
before the OS has it's own native driver loaded? For example, often
the Linux boot loader calls EBS before starting the kernel. What if
the kernel doesn't have the virtio gpu driver? Now there's no possible
way to signal the user.
This is just the Linux example, but for various OS's there could be
any number of reasons for something similar to occur.
> >
> > Regarding VirtIo GPU: Shouldn't we wait until it makes it into the
> > actual specs?
>
> No, we should not, at this point.
>
<snip>
> However, things have changed recently. I've received pings from three
> organizations that the driver has become more urgent for them. Combined
> with the facts that the 2D feature set of virtio-gpu has been part of
> QEMU since 2.4 [1], and a 2D Linux guest driver has existed for
> virtio-gpu since Linux 4.2 [2] [3], it seemed justified to shift the
> focus to working code, rather than standardization.
>
Ok. I guess it is the de-facto standard then. That is a good argument
to move forward, but I hope standardization doesn't lead to any
annoying compat issues.
> > Is there any chance to update the spec provide a simple (directly
> > scanned out) framebuffer mode?
>
> As described in both the spec and several places in this series, such a
> device already exists (both specified and implemented): it is called
> "virtio-vga". "Virtio-vga" is the combination of "virtio-gpu-pci" plus a
> legacy VGA framebuffer (the one you also get with QXL and stdvga).
>
> It is exactly this framebuffer that breaks in aarch64 KVM guests, due to
> architectural / KVM cache coherence issues (= as in, stuff that has been
> shipped in physical hardware already). For that reason, the
> "qemu-system-aarch64" binary, unlike "qemu-system-x86_64", does not even
> include the "virtio-vga" device model -- the framebuffer would be broken
> *anyway* on KVM. Instead, there we have to use "virtio-gpu-pci".
>
I think there would be value to have a non-VGA device that could still
configure a simple framebuffer. VGA does bring a fair amount of other
baggage. But, your point is good, that this does essentially 'fill the
gap' allowing for a temporary framebuffer while also allowing virtio
gpu to be used by the native OS driver.
> In other words, lacking a framebuffer is a *bonus* (which side-steps the
> aarch64 architectural / KVM problem).
>
It doesn't sound like a bonus. :)
Couldn't framebuffer support be an optional capability of the virtio
gpu device?
-Jordan
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 18:03 ` Jordan Justen
@ 2016-09-01 18:46 ` Laszlo Ersek
2016-09-01 19:52 ` Jordan Justen
0 siblings, 1 reply; 34+ messages in thread
From: Laszlo Ersek @ 2016-09-01 18:46 UTC (permalink / raw)
To: Jordan Justen; +Cc: Ard Biesheuvel, edk2-devel-01
On 09/01/16 20:03, Jordan Justen wrote:
> On 2016-09-01 09:29:28, Laszlo Ersek wrote:
>> On 08/31/16 22:43, Jordan Justen wrote:
>>>
>>> If an OS can't load or find the native driver, the framebuffer also
>>> provides a way to communicate with the user.
>>
>> I agree that exposing the framebuffer address through the GOP (at boot
>> time) so that the runtime OS can directly access the framebuffer (at
>> runtime) is useful. For example, in Windows 8 and Windows 10 guests,
>> this is how QXL can be initially used, interactively, until the native
>> QXL drivers are installed.
>>
>> I also agree that making a directly accessible framebuffer an *optional*
>
> I can't dispute that it is optional by UEFI spec, but I think it is
> commonly available (and for good reason).
Agree.
>
>> feature in the UEFI spec was the right thing to do! Graphics output
>> *should* be supportable at least at boot time even if the device lacks a
>> linear framebuffer (and even if the above OS fallback will therefore not
>> work).
>>
>
> Isn't there generally a window after ExitBootServices is called, and
> before the OS has it's own native driver loaded?
Yes, there is.
> For example, often
> the Linux boot loader calls EBS before starting the kernel. What if
> the kernel doesn't have the virtio gpu driver? Now there's no possible
> way to signal the user.
Correct, there is no way to signal the user *on the graphics console*.
(And, if the guest kernel does have a virtio-gpu-pci driver, then no
messages can be printed to the graphics console in the window that you
mention.) Put differently, the "efifb" driver in Linux has no chance of
working.
But this is no worse than the firmware having no virtio-gpu-pci driver
at all -- in that case, you cannot even boot your guest kernel with
GRUB, because you can't read anything.
So, the virtio-gpu-pci device (and the VirtioGpuDxe driver) does not
give you everything you could justifiedly wish for (which is the
framebuffer), but it does give you what you can reasonably expect on a
platform where the framebuffer is plain broken. Given that the
framebuffer is plan broken (architecturally), GRUB wouldn't give you a
usable menu anyway, and a guest kernel without the virtio-gpu-pci driver
couldn't print readable messages to the framebuffer anyway. (You get
garbage on the screen.)
IOW, the argument is not that "framebuffer is wrong, boo". The argument
is that "framebuffer is broken, fact of life"; virtio-gpu-pci deals with
it (sidesteps it). It fills the gap, not completely, but sufficiently.
It doesn't give you the exact same experience as a linear framebuffer
does *on x86_64*, but for aarch64, that option does not exist, so *on
aarch64*, virtio-gpu-pci is a strict improvement.
> This is just the Linux example, but for various OS's there could be
> any number of reasons for something similar to occur.
Yes.
[snip]
> I think there would be value to have a non-VGA device that could still
> configure a simple framebuffer. VGA does bring a fair amount of other
> baggage.
Ah, I see your point. You distinguish "VGA" from "non-VGA device with
framebuffer".
For this discussion however, this distinction makes no difference. The
suggested "non-VGA device with framebuffer" would be broken exactly the
same way. In other words, it's not the "other baggage" that is broken,
it is the framebuffer.
> But, your point is good, that this does essentially 'fill the
> gap' allowing for a temporary framebuffer while also allowing virtio
> gpu to be used by the native OS driver.
Correction: it fills the gap allowing for a temporary, Blt()-only *GOP*
(not framebuffer!), while also allowing virtio-gpu-pci to be used by the
native OS driver.
And, it turns out that "Blt()-only" is fully sufficient for GRUB, the
UEFI shell (i.e., whatever uses Simple Text Out), huge kudos to
GraphicsConsoleDxe. :) The setup utility works fine too.
>> In other words, lacking a framebuffer is a *bonus* (which side-steps the
>> aarch64 architectural / KVM problem).
>>
>
> It doesn't sound like a bonus. :)
It is certainly not a bonus that the framebuffer is broken on
aarch64/KVM. It has been a huge headache.
Given that fact however, it *is* a bonus that we have virtio-gpu-pci,
which allows us to display stuff on the screen, without using the
(broken) framebuffer :)
> Couldn't framebuffer support be an optional capability of the virtio
> gpu device?
It already is, you just have to call it "virtio-vga" on the QEMU command
line, rather than "virtio-gpu-pci".
As seen from both the spec and the patches in this series, the guest
tells apart these two device models (i.e., it checks whether we have the
compat framebuffer) from the PCI device class.
Please read
<https://www.kraxel.org/virtio/virtio-v1.0-cs03-virtio-gpu.html#x1-2980007>
("5.7.7 VGA Compatibility").
Please see also the first three patches.
The "qemu-system-aarch64" binary does not include the virtio-vga device
(or, in your words, the "optional framebuffer capability"), because the
framebuffer is architecturally broken on aarch64/KVM. aarch64 combines
guest- and host-side caching attributes differently from x86_64: on the
latter, the laxest attribute prevails, on aarch64, the strictest does.
The PCI host bridge / root bridge driver in edk2 maps MMIO BARs as
Uncached (which is where the framebuffer lives), while KVM maps the same
BAR as cached (in host RAM). Therefore, on x86_64, the end-to-end access
is cached (= laxests prevails), hence guest updates go through the PCPU
cache, where QEMU sees them and can display them correctly. On aarch64,
guest updates *avoid* the PCPU cache (they go directly do host RAM:
strictest attribute prevails), but QEMU still reads through the PCPU
cache, and whatever it displays from there can be stale.
The stage1/stage2 caching attribute combination cannot be changed on
aarch64 (it is part of the architecture spec and has been shipped in
hardware), and all attempts at changing QEMU's host side read accesses /
KVM's mappings to Uncached have failed. (The people who have looked into
this include the kvm-arm maintainers.)
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 18:46 ` Laszlo Ersek
@ 2016-09-01 19:52 ` Jordan Justen
2016-09-01 20:23 ` Ard Biesheuvel
` (2 more replies)
0 siblings, 3 replies; 34+ messages in thread
From: Jordan Justen @ 2016-09-01 19:52 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: Ard Biesheuvel, edk2-devel-01
On 2016-09-01 11:46:04, Laszlo Ersek wrote:
> On 09/01/16 20:03, Jordan Justen wrote:
>
> > I think there would be value to have a non-VGA device that could still
> > configure a simple framebuffer. VGA does bring a fair amount of other
> > baggage.
>
> Ah, I see your point. You distinguish "VGA" from "non-VGA device with
> framebuffer".
>
> For this discussion however, this distinction makes no difference. The
> suggested "non-VGA device with framebuffer" would be broken exactly the
> same way. In other words, it's not the "other baggage" that is broken,
> it is the framebuffer.
>
This is only focusing on the current ARM issue. I'm just pointing out
that I think virtio gpu without VGA, but with a framebuffer could be
useful on IA32/X64 as well.
1. You don't have to deal with the PCI bus
2. The OS re-enumerating the PCI bus would not break the framebuffer
Is there no chance that ARM KVM might someday also be able to support
a framebuffer?
Obviously this is not too important for IA32/X64 OVMF, because we have
reasonable alternatives. But, it seems like virtio gpu could be a
significantly better option for IA32/X64 OVMF if an optional
framebuffer was possible.
-Jordan
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 19:52 ` Jordan Justen
@ 2016-09-01 20:23 ` Ard Biesheuvel
2016-09-01 20:26 ` Ard Biesheuvel
2016-09-01 20:52 ` Jordan Justen
2016-09-01 20:44 ` Laszlo Ersek
2016-09-05 14:17 ` Gerd Hoffmann
2 siblings, 2 replies; 34+ messages in thread
From: Ard Biesheuvel @ 2016-09-01 20:23 UTC (permalink / raw)
To: Jordan Justen; +Cc: Laszlo Ersek, edk2-devel-01
On 1 September 2016 at 20:52, Jordan Justen <jordan.l.justen@intel.com> wrote:
> On 2016-09-01 11:46:04, Laszlo Ersek wrote:
>> On 09/01/16 20:03, Jordan Justen wrote:
>>
>> > I think there would be value to have a non-VGA device that could still
>> > configure a simple framebuffer. VGA does bring a fair amount of other
>> > baggage.
>>
>> Ah, I see your point. You distinguish "VGA" from "non-VGA device with
>> framebuffer".
>>
>> For this discussion however, this distinction makes no difference. The
>> suggested "non-VGA device with framebuffer" would be broken exactly the
>> same way. In other words, it's not the "other baggage" that is broken,
>> it is the framebuffer.
>>
>
> This is only focusing on the current ARM issue. I'm just pointing out
> that I think virtio gpu without VGA, but with a framebuffer could be
> useful on IA32/X64 as well.
>
> 1. You don't have to deal with the PCI bus
>
> 2. The OS re-enumerating the PCI bus would not break the framebuffer
>
How does adding a framebuffer to virtio-gpu solve any of that?
virtio-gpu is a PCI device as well
> Is there no chance that ARM KVM might someday also be able to support
> a framebuffer?
>
There are workarounds imaginable, but none of them are feasible in
terms. A new revision of the architecture may address this, but that
will take years to turn up in real hardware.
> Obviously this is not too important for IA32/X64 OVMF, because we have
> reasonable alternatives. But, it seems like virtio gpu could be a
> significantly better option for IA32/X64 OVMF if an optional
> framebuffer was possible.
>
IIUC, not having a linear framebuffer was one of the design goals,
since it is essentially a layering violation in the virtio stack.
virtio-gpu is strictly ring based, like other virtio devices.
If you want a framebuffer, you should use virtio-vga. Adding a
framebuffer to virtio-gpu is a step back rather than a step forward.
Also, the loader->OS handover already suffers from the issues you
mention, since the PCI reconfiguration breaks GOP on almost every
platform. The reason that it is less of a concern on ARM is that most
systems use UART as the primary console.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 20:23 ` Ard Biesheuvel
@ 2016-09-01 20:26 ` Ard Biesheuvel
2016-09-01 20:52 ` Jordan Justen
1 sibling, 0 replies; 34+ messages in thread
From: Ard Biesheuvel @ 2016-09-01 20:26 UTC (permalink / raw)
To: Jordan Justen; +Cc: Laszlo Ersek, edk2-devel-01
On 1 September 2016 at 21:23, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 1 September 2016 at 20:52, Jordan Justen <jordan.l.justen@intel.com> wrote:
>> On 2016-09-01 11:46:04, Laszlo Ersek wrote:
>>> On 09/01/16 20:03, Jordan Justen wrote:
>>>
>>> > I think there would be value to have a non-VGA device that could still
>>> > configure a simple framebuffer. VGA does bring a fair amount of other
>>> > baggage.
>>>
>>> Ah, I see your point. You distinguish "VGA" from "non-VGA device with
>>> framebuffer".
>>>
>>> For this discussion however, this distinction makes no difference. The
>>> suggested "non-VGA device with framebuffer" would be broken exactly the
>>> same way. In other words, it's not the "other baggage" that is broken,
>>> it is the framebuffer.
>>>
>>
>> This is only focusing on the current ARM issue. I'm just pointing out
>> that I think virtio gpu without VGA, but with a framebuffer could be
>> useful on IA32/X64 as well.
>>
>> 1. You don't have to deal with the PCI bus
>>
>> 2. The OS re-enumerating the PCI bus would not break the framebuffer
>>
>
> How does adding a framebuffer to virtio-gpu solve any of that?
> virtio-gpu is a PCI device as well
>
>> Is there no chance that ARM KVM might someday also be able to support
>> a framebuffer?
>>
>
> There are workarounds imaginable, but none of them are feasible in
> terms
... of performance
> A new revision of the architecture may address this, but that
> will take years to turn up in real hardware.
>
>> Obviously this is not too important for IA32/X64 OVMF, because we have
>> reasonable alternatives. But, it seems like virtio gpu could be a
>> significantly better option for IA32/X64 OVMF if an optional
>> framebuffer was possible.
>>
>
> IIUC, not having a linear framebuffer was one of the design goals,
> since it is essentially a layering violation in the virtio stack.
> virtio-gpu is strictly ring based, like other virtio devices.
>
> If you want a framebuffer, you should use virtio-vga. Adding a
> framebuffer to virtio-gpu is a step back rather than a step forward.
>
> Also, the loader->OS handover already suffers from the issues you
> mention, since the PCI reconfiguration breaks GOP on almost every
> platform. The reason that it is less of a concern on ARM is that most
> systems use UART as the primary console.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 20:23 ` Ard Biesheuvel
2016-09-01 20:26 ` Ard Biesheuvel
@ 2016-09-01 20:52 ` Jordan Justen
1 sibling, 0 replies; 34+ messages in thread
From: Jordan Justen @ 2016-09-01 20:52 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Laszlo Ersek, edk2-devel-01
Points taken. Oh well. :\
On 2016-09-01 13:23:27, Ard Biesheuvel wrote:
> On 1 September 2016 at 20:52, Jordan Justen <jordan.l.justen@intel.com> wrote:
> > On 2016-09-01 11:46:04, Laszlo Ersek wrote:
> >> On 09/01/16 20:03, Jordan Justen wrote:
> >>
> >> > I think there would be value to have a non-VGA device that could still
> >> > configure a simple framebuffer. VGA does bring a fair amount of other
> >> > baggage.
> >>
> >> Ah, I see your point. You distinguish "VGA" from "non-VGA device with
> >> framebuffer".
> >>
> >> For this discussion however, this distinction makes no difference. The
> >> suggested "non-VGA device with framebuffer" would be broken exactly the
> >> same way. In other words, it's not the "other baggage" that is broken,
> >> it is the framebuffer.
> >>
> >
> > This is only focusing on the current ARM issue. I'm just pointing out
> > that I think virtio gpu without VGA, but with a framebuffer could be
> > useful on IA32/X64 as well.
> >
> > 1. You don't have to deal with the PCI bus
> >
> > 2. The OS re-enumerating the PCI bus would not break the framebuffer
> >
>
> How does adding a framebuffer to virtio-gpu solve any of that?
> virtio-gpu is a PCI device as well
>
> > Is there no chance that ARM KVM might someday also be able to support
> > a framebuffer?
> >
>
> There are workarounds imaginable, but none of them are feasible in
> terms. A new revision of the architecture may address this, but that
> will take years to turn up in real hardware.
>
> > Obviously this is not too important for IA32/X64 OVMF, because we have
> > reasonable alternatives. But, it seems like virtio gpu could be a
> > significantly better option for IA32/X64 OVMF if an optional
> > framebuffer was possible.
> >
>
> IIUC, not having a linear framebuffer was one of the design goals,
> since it is essentially a layering violation in the virtio stack.
> virtio-gpu is strictly ring based, like other virtio devices.
>
> If you want a framebuffer, you should use virtio-vga. Adding a
> framebuffer to virtio-gpu is a step back rather than a step forward.
>
> Also, the loader->OS handover already suffers from the issues you
> mention, since the PCI reconfiguration breaks GOP on almost every
> platform. The reason that it is less of a concern on ARM is that most
> systems use UART as the primary console.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 19:52 ` Jordan Justen
2016-09-01 20:23 ` Ard Biesheuvel
@ 2016-09-01 20:44 ` Laszlo Ersek
2016-09-05 14:17 ` Gerd Hoffmann
2 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-09-01 20:44 UTC (permalink / raw)
To: Jordan Justen; +Cc: Ard Biesheuvel, edk2-devel-01
On 09/01/16 21:52, Jordan Justen wrote:
> On 2016-09-01 11:46:04, Laszlo Ersek wrote:
>> On 09/01/16 20:03, Jordan Justen wrote:
>>
>>> I think there would be value to have a non-VGA device that could still
>>> configure a simple framebuffer. VGA does bring a fair amount of other
>>> baggage.
>>
>> Ah, I see your point. You distinguish "VGA" from "non-VGA device with
>> framebuffer".
>>
>> For this discussion however, this distinction makes no difference. The
>> suggested "non-VGA device with framebuffer" would be broken exactly the
>> same way. In other words, it's not the "other baggage" that is broken,
>> it is the framebuffer.
>>
>
> This is only focusing on the current ARM issue. I'm just pointing out
> that I think virtio gpu without VGA, but with a framebuffer could be
> useful on IA32/X64 as well.
I believe your suggestion concerns the spec itself (that is, what it
should specify, and how the QEMU-side VirtIo GPU Device should be
designed). I didn't partake in the design of, nor QEMU's implementation
of, the VirtIo GPU Device, so I can't comment (Gerd Hoffmann and David
Airlie could, I assume).
The current design is certainly sufficient for the ARM issue.
> 1. You don't have to deal with the PCI bus
>
> 2. The OS re-enumerating the PCI bus would not break the framebuffer
>
> Is there no chance that ARM KVM might someday also be able to support
> a framebuffer?
>
> Obviously this is not too important for IA32/X64 OVMF, because we have
> reasonable alternatives. But, it seems like virtio gpu could be a
> significantly better option for IA32/X64 OVMF if an optional
> framebuffer was possible.
Well, I can't (and shouldn't) comment, due to the above, but I might try
speculating.
If I understand correctly, you are proposing a device that is somewhere
"between" virtio-gpu-pci and virtio-vga, in the sense that it'd have a
framebuffer, but (a) the framebuffer wouldn't live in a PCI MMIO BAR,
and (b) it wouldn't have any of the other "VGA baggage".
In my experience (which may or may not be authoritative), devices that
"own" a subset of the guest-phyisical address space but are *not* PCI
devices, are platform devices. Platform devices are hard to enumerate
and to manage. They usually have ties to fw_cfg and/or ACPI and/or DT
(Device Tree). Virtio-mmio is a good example I believe.
People seem to frown upon platform devices, and prefer buses that are
enumerable via hw access (for example, virtio-pci is considered vastly
superior to virtio-mmio). In my mind (as I perceive others' opinions),
PCI is a clear winner. Arguing that a PCI graphics device present its
framebuffer differently from an MMIO BAR would be an uphill battle, I think.
But, again, as you are commenting on the design of the VirtIo GPU
Device, I'm not the right person to talk to.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 19:52 ` Jordan Justen
2016-09-01 20:23 ` Ard Biesheuvel
2016-09-01 20:44 ` Laszlo Ersek
@ 2016-09-05 14:17 ` Gerd Hoffmann
2 siblings, 0 replies; 34+ messages in thread
From: Gerd Hoffmann @ 2016-09-05 14:17 UTC (permalink / raw)
To: Jordan Justen; +Cc: Laszlo Ersek, edk2-devel-01, Ard Biesheuvel
Hi,
> Is there no chance that ARM KVM might someday also be able to support
> a framebuffer?
Problem on arm is that you can't expose something as pci memory to the
guest which in reality isn't pci memory but normal ram (like the
emulated vga memory pci bar).
Storing a framebuffer in normal ram is (as far I know) no problem. But
updates are a problem then, you have to:
(a) notify the host about updates (which is what virtio-gpu does), or
(b) turn on dirty tracking for *all* guest ram so you can detect
display updates automatically (lot of overhead, vga needs dirty
tracking only for the 16M pci bar), or
(c) update whole framebuffer unconditionally in regular intervals
(doesn't look great too).
cheers,
Gerd
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (11 preceding siblings ...)
2016-08-19 13:06 ` [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Ard Biesheuvel
@ 2016-08-30 15:07 ` Laszlo Ersek
2016-09-01 20:32 ` Jordan Justen
2016-09-01 22:02 ` Laszlo Ersek
14 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-08-30 15:07 UTC (permalink / raw)
To: Jordan Justen; +Cc: edk2-devel-01, Ard Biesheuvel
On 08/19/16 14:49, 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
>
Ping.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (12 preceding siblings ...)
2016-08-30 15:07 ` Laszlo Ersek
@ 2016-09-01 20:32 ` Jordan Justen
2016-09-01 21:07 ` Laszlo Ersek
2016-09-01 22:02 ` Laszlo Ersek
14 siblings, 1 reply; 34+ messages in thread
From: Jordan Justen @ 2016-09-01 20:32 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Ard Biesheuvel
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
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-09-01 20:32 ` Jordan Justen
@ 2016-09-01 21:07 ` Laszlo Ersek
0 siblings, 0 replies; 34+ messages in thread
From: Laszlo Ersek @ 2016-09-01 21:07 UTC (permalink / raw)
To: Jordan Justen, edk2-devel-01; +Cc: Ard Biesheuvel
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
>>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci)
2016-08-19 12:49 [PATCH 00/11] OvmfPkg, ArmVirtPkg: GOP driver for the VirtIo GPU (virtio-gpu-pci) Laszlo Ersek
` (13 preceding siblings ...)
2016-09-01 20:32 ` Jordan Justen
@ 2016-09-01 22:02 ` Laszlo Ersek
[not found] ` <57CD6463.90903@suse.de>
14 siblings, 1 reply; 34+ messages in thread
From: Laszlo Ersek @ 2016-09-01 22:02 UTC (permalink / raw)
To: edk2-devel-01
Cc: Jordan Justen, Ard Biesheuvel, Shannon Zhao, Christoffer Dall,
Alexander Graf
On 08/19/16 14:49, Laszlo Ersek wrote:
> - 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.
Update:
Once booted from GRUB -- on top of OVMF / ArmVirtQemu respectively --,
both x86_64 and aarch64 Fedora *24* guests work well with
virtio-gpu-pci, as far as the character console (tty1, "virtiodrmfb") is
concerned.
However, for getting an X.org graphical environment (such as Xfce) to
work with virtio-gpu-pci, more gymnastics are needed in the guest, on
both guest arches.
This is because X.org considers "virtio-gpu-pci" a secondary graphics
card, and it won't use it even in a single-head setup *unless* the BusID
option is specified in the X.org config file, in the Device section.
Refer to <http://linux.die.net/man/5/xorg.conf>:
BusID bus-id [...] when using a secondary graphics card in a
single-head configuration, this entry is mandatory
After I placed
Section "Screen"
Identifier "Default Screen"
Device "Default Device"
Monitor "Default Monitor"
EndSection
Section "Device"
Identifier "Default Device"
Driver "modesetting"
BusID "PCI:0:1:0"
EndSection
Section "Monitor"
Identifier "Default Monitor"
Option "PreferredMode" "1024x768"
EndSection
in
/etc/X11/xorg.conf.d/01-resolution.conf
Xfce too is working fine for me. (Note that BusID must match the PCI
$BUS/$DEV/$FUN that QEMU assigns to virtio-gpu-pci, either
automatically, or in response to the user's "-device
virtio-gpu-pci,addr=$DEV" property.)
I guess if someone is into xserver development, this could be made a bit
more convenient, because for aarch64/KVM guests at least, virtio-gpu-pci
should be considered a primary (not secondary) graphics card.
Anyway, here's a screenshot:
http://people.redhat.com/lersek/virtio-gpu-pci-xorg-xfce.png
Thanks
Laszlo
^ permalink raw reply [flat|nested] 34+ messages in thread
