[PATCH v2 0/4] Add QemuRamfbDxe driver

[PATCH v2 0/4] Add QemuRamfbDxe driver

[Gerd Hoffmann]

  Hi,

This is the efi driver for qemu ramfb, a simple boot framebuffer.  Qemu
patches just have been posted to qemu-devel.

v2: tons of codestyle fixes, some control flow tweaks.

cheers,
  Gerd

Gerd Hoffmann (4):
  OvmfPkg: add QEMU_RAMFB_GUID
  OvmfPkg: add QemuRamfbDxe
  OvmfPkg: add QemuRamfb to platform console
  ArmVirtPkg: add QemuRamfbDxe

 OvmfPkg/Include/Guid/QemuRamfb.h                   |  25 ++
 .../Library/PlatformBootManagerLib/PlatformData.c  |  51 +++
 OvmfPkg/QemuRamfbDxe/QemuRamfb.c                   | 396 +++++++++++++++++++++
 ArmVirtPkg/ArmVirtQemu.dsc                         |   2 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc               |   1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc                   |   2 +
 OvmfPkg/OvmfPkg.dec                                |   1 +
 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/QemuRamfbDxe/QemuRamfbDxe.inf              |  52 +++
 14 files changed, 536 insertions(+)
 create mode 100644 OvmfPkg/Include/Guid/QemuRamfb.h
 create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfb.c
 create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf

-- 
2.9.3

[PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID

[Gerd Hoffmann]

Add GUID header file for the QemuRamfbDxe driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/Include/Guid/QemuRamfb.h | 25 +++++++++++++++++++++++++
 OvmfPkg/OvmfPkg.dec              |  1 +
 2 files changed, 26 insertions(+)
 create mode 100644 OvmfPkg/Include/Guid/QemuRamfb.h

diff --git a/OvmfPkg/Include/Guid/QemuRamfb.h b/OvmfPkg/Include/Guid/QemuRamfb.h
new file mode 100644
index 0000000000..5c9dffb1f0
--- /dev/null
+++ b/OvmfPkg/Include/Guid/QemuRamfb.h
@@ -0,0 +1,25 @@
+/** @file
+  Recommended GUID to be used in the Vendor Hardware device path nodes that
+  identify qemu ramfb devices.
+
+  Copyright (C) 2018, Red Hat, Inc.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License that 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 __QEMU_RAMFB_H__
+#define __QEMU_RAMFB_H__
+
+#define QEMU_RAMFB_GUID \
+{0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
+
+extern EFI_GUID gQemuRamfbGuid;
+
+#endif
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index dc5597db41..7666297cf8 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -75,6 +75,7 @@
   gEfiXenInfoGuid                     = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
   gOvmfPlatformConfigGuid             = {0x7235c51c, 0x0c80, 0x4cab, {0x87, 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}}
   gVirtioMmioTransportGuid            = {0x837dca9e, 0xe874, 0x4d82, {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}}
+  gQemuRamfbGuid                      = {0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
   gXenBusRootDeviceGuid               = {0xa732241f, 0x383d, 0x4d9c, {0x8a, 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
   gRootBridgesConnectedEventGroupGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90, 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
 
-- 
2.9.3

Re: [PATCH v2 1/4] OvmfPkg: add QEMU_RAMFB_GUID

[Laszlo Ersek]

On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add GUID header file for the QemuRamfbDxe driver.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/Include/Guid/QemuRamfb.h | 25 +++++++++++++++++++++++++
>  OvmfPkg/OvmfPkg.dec              |  1 +
>  2 files changed, 26 insertions(+)
>  create mode 100644 OvmfPkg/Include/Guid/QemuRamfb.h
> 
> diff --git a/OvmfPkg/Include/Guid/QemuRamfb.h b/OvmfPkg/Include/Guid/QemuRamfb.h
> new file mode 100644
> index 0000000000..5c9dffb1f0
> --- /dev/null
> +++ b/OvmfPkg/Include/Guid/QemuRamfb.h
> @@ -0,0 +1,25 @@
> +/** @file
> +  Recommended GUID to be used in the Vendor Hardware device path nodes that
> +  identify qemu ramfb devices.
> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License that 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 __QEMU_RAMFB_H__
> +#define __QEMU_RAMFB_H__
> +
> +#define QEMU_RAMFB_GUID \
> +{0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
> +
> +extern EFI_GUID gQemuRamfbGuid;
> +
> +#endif
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index dc5597db41..7666297cf8 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -75,6 +75,7 @@
>    gEfiXenInfoGuid                     = {0xd3b46f3b, 0xd441, 0x1244, {0x9a, 0x12, 0x0, 0x12, 0x27, 0x3f, 0xc1, 0x4d}}
>    gOvmfPlatformConfigGuid             = {0x7235c51c, 0x0c80, 0x4cab, {0x87, 0xac, 0x3b, 0x08, 0x4a, 0x63, 0x04, 0xb1}}
>    gVirtioMmioTransportGuid            = {0x837dca9e, 0xe874, 0x4d82, {0xb2, 0x9a, 0x23, 0xfe, 0x0e, 0x23, 0xd1, 0xe2}}
> +  gQemuRamfbGuid                      = {0x557423a1, 0x63ab, 0x406c, {0xbe, 0x7e, 0x91, 0xcd, 0xbc, 0x08, 0xc4, 0x57}}
>    gXenBusRootDeviceGuid               = {0xa732241f, 0x383d, 0x4d9c, {0x8a, 0xe1, 0x8e, 0x09, 0x83, 0x75, 0x89, 0xd7}}
>    gRootBridgesConnectedEventGroupGuid = {0x24a2d66f, 0xeedd, 0x4086, {0x90, 0x42, 0xf2, 0x6e, 0x47, 0x97, 0xee, 0x69}}
>  
> 

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

[PATCH v2 2/4] OvmfPkg: add QemuRamfbDxe

[Gerd Hoffmann]

Add a driver for the qemu ramfb display device.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/QemuRamfbDxe/QemuRamfb.c      | 396 ++++++++++++++++++++++++++++++++++
 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/QemuRamfbDxe/QemuRamfbDxe.inf |  52 +++++
 8 files changed, 454 insertions(+)
 create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfb.c
 create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf

diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfb.c b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
new file mode 100644
index 0000000000..00a2875e99
--- /dev/null
+++ b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
@@ -0,0 +1,396 @@
+/** @file
+  This driver is a implementation of the Graphics Output Protocol
+  for the QEMU ramfb device.
+
+  Copyright (c) 2018, Red Hat Inc.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution. The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Protocol/GraphicsOutput.h>
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/FrameBufferBltLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/QemuFwCfgLib.h>
+
+#include <Guid/QemuRamfb.h>
+
+#define RAMFB_FORMAT  0x34325258 /* DRM_FORMAT_XRGB8888 */
+#define RAMFB_BPP     4
+
+#pragma pack (1)
+typedef struct RAMFB_CONFIG {
+  UINT64 Address;
+  UINT32 FourCC;
+  UINT32 Flags;
+  UINT32 Width;
+  UINT32 Height;
+  UINT32 Stride;
+} RAMFB_CONFIG;
+#pragma pack ()
+
+static EFI_HANDLE                    mRamfbHandle;
+static EFI_HANDLE                    mGopHandle;
+static FRAME_BUFFER_CONFIGURE        *mQemuRamfbFrameBufferBltConfigure;
+static UINTN                         mQemuRamfbFrameBufferBltConfigureSize;
+static FIRMWARE_CONFIG_ITEM          mRamFbFwCfgItem;
+
+static EFI_GRAPHICS_OUTPUT_MODE_INFORMATION mQemuRamfbModeInfo[] = {
+  {
+    0,    // Version
+    640,  // HorizontalResolution
+    480,  // VerticalResolution
+  },{
+    0,    // Version
+    800,  // HorizontalResolution
+    600,  // VerticalResolution
+  },{
+    0,    // Version
+    1024, // HorizontalResolution
+    768,  // VerticalResolution
+  }
+};
+#define mQemuRamfbModeCount ARRAY_SIZE(mQemuRamfbModeInfo)
+
+static EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE mQemuRamfbMode = {
+  mQemuRamfbModeCount,                            // MaxMode
+  0,                                              // Mode
+  mQemuRamfbModeInfo,                             // Info
+  sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION),  // SizeOfInfo
+};
+
+static
+EFI_STATUS
+EFIAPI
+QemuRamfbGraphicsOutputQueryMode (
+  IN  EFI_GRAPHICS_OUTPUT_PROTOCOL          *This,
+  IN  UINT32                                ModeNumber,
+  OUT UINTN                                 *SizeOfInfo,
+  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  **Info
+  )
+{
+  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *ModeInfo;
+
+  if (Info == NULL || SizeOfInfo == NULL || ModeNumber >= mQemuRamfbMode.MaxMode) {
+    return EFI_INVALID_PARAMETER;
+  }
+  ModeInfo = &mQemuRamfbModeInfo[ModeNumber];
+
+  *Info = AllocateCopyPool (sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION),
+            ModeInfo);
+  if (*Info == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+  *SizeOfInfo = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
+
+  return EFI_SUCCESS;
+}
+
+static
+EFI_STATUS
+EFIAPI
+QemuRamfbGraphicsOutputSetMode (
+  IN  EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
+  IN  UINT32                       ModeNumber
+  )
+{
+  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *ModeInfo;
+  RAMFB_CONFIG                          Config;
+  EFI_GRAPHICS_OUTPUT_BLT_PIXEL         Black;
+  RETURN_STATUS                         Status;
+
+  if (ModeNumber >= mQemuRamfbMode.MaxMode) {
+    return EFI_UNSUPPORTED;
+  }
+  ModeInfo = &mQemuRamfbModeInfo[ModeNumber];
+
+  DEBUG ((DEBUG_INFO, "Ramfb: SetMode %u (%ux%u)\n", ModeNumber,
+    ModeInfo->HorizontalResolution, ModeInfo->VerticalResolution));
+
+  Config.Address = SwapBytes64 (mQemuRamfbMode.FrameBufferBase);
+  Config.FourCC  = SwapBytes32 (RAMFB_FORMAT);
+  Config.Flags   = SwapBytes32 (0);
+  Config.Width   = SwapBytes32 (ModeInfo->HorizontalResolution);
+  Config.Height  = SwapBytes32 (ModeInfo->VerticalResolution);
+  Config.Stride  = SwapBytes32 (ModeInfo->HorizontalResolution * RAMFB_BPP);
+
+  Status = FrameBufferBltConfigure (
+             (VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase,
+             ModeInfo,
+             mQemuRamfbFrameBufferBltConfigure,
+             &mQemuRamfbFrameBufferBltConfigureSize
+             );
+
+  if (Status == RETURN_BUFFER_TOO_SMALL) {
+    if (mQemuRamfbFrameBufferBltConfigure != NULL) {
+      FreePool (mQemuRamfbFrameBufferBltConfigure);
+    }
+    mQemuRamfbFrameBufferBltConfigure =
+      AllocatePool (mQemuRamfbFrameBufferBltConfigureSize);
+    if (mQemuRamfbFrameBufferBltConfigure == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    Status = FrameBufferBltConfigure (
+               (VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase,
+               ModeInfo,
+               mQemuRamfbFrameBufferBltConfigure,
+               &mQemuRamfbFrameBufferBltConfigureSize);
+    if (RETURN_ERROR (Status)) {
+      ASSERT (Status == RETURN_UNSUPPORTED);
+      return Status;
+    }
+  }
+
+  mQemuRamfbMode.Mode = ModeNumber;
+  mQemuRamfbMode.Info = ModeInfo;
+
+  QemuFwCfgSelectItem (mRamFbFwCfgItem);
+  QemuFwCfgWriteBytes (sizeof (Config), &Config);
+
+  //
+  // clear screen
+  //
+  ZeroMem (&Black, sizeof (Black));
+  Status = FrameBufferBlt (
+             mQemuRamfbFrameBufferBltConfigure,
+             &Black,
+             EfiBltVideoFill,
+             0,                               // SourceX -- ignored
+             0,                               // SourceY -- ignored
+             0,                               // DestinationX
+             0,                               // DestinationY
+             ModeInfo->HorizontalResolution,  // Width
+             ModeInfo->VerticalResolution,    // Height
+             0                                // Delta -- ignored
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: clearing the screen failed: %r\n",
+            __FUNCTION__, Status));
+  }
+
+  return EFI_SUCCESS;
+}
+
+static
+EFI_STATUS
+EFIAPI
+QemuRamfbGraphicsOutputBlt (
+  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
+  )
+{
+  return FrameBufferBlt (
+           mQemuRamfbFrameBufferBltConfigure,
+           BltBuffer,
+           BltOperation,
+           SourceX,
+           SourceY,
+           DestinationX,
+           DestinationY,
+           Width,
+           Height,
+           Delta);
+}
+
+EFI_GRAPHICS_OUTPUT_PROTOCOL QemuRamfbGraphicsOutput = {
+  QemuRamfbGraphicsOutputQueryMode,
+  QemuRamfbGraphicsOutputSetMode,
+  QemuRamfbGraphicsOutputBlt,
+  &mQemuRamfbMode,
+};
+
+EFI_STATUS
+EFIAPI
+InitializeQemuRamfb (
+  IN EFI_HANDLE           ImageHandle,
+  IN EFI_SYSTEM_TABLE     *SystemTable
+  )
+{
+  EFI_DEVICE_PATH_PROTOCOL  *RamfbDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL  *GopDevicePath;
+  VOID                      *DevicePath;
+  VENDOR_DEVICE_PATH        VendorDeviceNode;
+  ACPI_ADR_DEVICE_PATH      AcpiDeviceNode;
+  EFI_STATUS                Status;
+  EFI_PHYSICAL_ADDRESS      FbBase;
+  UINTN                     FbSize, MaxFbSize;
+  UINTN                     Size, Pages, Index;
+
+  if (!QemuFwCfgIsAvailable()) {
+    DEBUG ((DEBUG_INFO, "Ramfb: no FwCfg\n"));
+    return EFI_NOT_FOUND;
+  }
+
+  Status = QemuFwCfgFindFile("etc/ramfb", &mRamFbFwCfgItem, &Size);
+  if (EFI_ERROR (Status)) {
+    return EFI_NOT_FOUND;
+  }
+  if (Size != sizeof (RAMFB_CONFIG)) {
+    DEBUG ((DEBUG_ERROR, "Ramfb: FwCfg size mismatch (expected %d, got %d)\n",
+             sizeof (RAMFB_CONFIG), Size));
+    return EFI_NOT_FOUND;
+  }
+
+  MaxFbSize = 0;
+  for (Index = 0; Index < mQemuRamfbModeCount; Index++) {
+    mQemuRamfbModeInfo[Index].PixelsPerScanLine =
+      mQemuRamfbModeInfo[Index].HorizontalResolution;
+    mQemuRamfbModeInfo[Index].PixelFormat =
+      PixelBlueGreenRedReserved8BitPerColor;
+    FbSize = RAMFB_BPP *
+      mQemuRamfbModeInfo[Index].HorizontalResolution *
+      mQemuRamfbModeInfo[Index].VerticalResolution;
+    if (MaxFbSize < FbSize) {
+      MaxFbSize = FbSize;
+    }
+    DEBUG ((DEBUG_INFO, "Ramfb: Mode %u: %ux%u, %lu kB\n", (UINT64)Index,
+      mQemuRamfbModeInfo[Index].HorizontalResolution,
+      mQemuRamfbModeInfo[Index].VerticalResolution,
+      (UINT64)FbSize / 1024));
+  }
+
+  Pages = EFI_SIZE_TO_PAGES (MaxFbSize);
+  MaxFbSize = EFI_PAGES_TO_SIZE (Pages);
+  FbBase = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateReservedPages (Pages);
+  if (FbBase == 0) {
+    DEBUG ((DEBUG_INFO, "Ramfb: memory allocation failed\n"));
+    return EFI_OUT_OF_RESOURCES;
+  }
+  DEBUG ((DEBUG_INFO, "Ramfb: Framebuffer at 0x%lx, %ld kB, %ld pages\n",
+          (UINT64)FbBase, (UINT64)MaxFbSize / 1024, (UINT64)Pages));
+  mQemuRamfbMode.FrameBufferSize = MaxFbSize;
+  mQemuRamfbMode.FrameBufferBase = FbBase;
+
+  //
+  // 800 x 600
+  //
+  QemuRamfbGraphicsOutputSetMode (&QemuRamfbGraphicsOutput, 1);
+
+  //
+  // ramfb vendor devpath
+  //
+  VendorDeviceNode.Header.Type = HARDWARE_DEVICE_PATH;
+  VendorDeviceNode.Header.SubType = HW_VENDOR_DP;
+  CopyGuid(&VendorDeviceNode.Guid, &gQemuRamfbGuid);
+  SetDevicePathNodeLength (&VendorDeviceNode.Header,
+    sizeof (VENDOR_DEVICE_PATH));
+
+  RamfbDevicePath = AppendDevicePathNode (NULL,
+    (EFI_DEVICE_PATH_PROTOCOL *) &VendorDeviceNode);
+  if (RamfbDevicePath == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    goto FreeFramebuffer;
+  }
+
+  Status = gBS->InstallMultipleProtocolInterfaces (
+                  &mRamfbHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  RamfbDevicePath,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Ramfb: install Ramfb Vendor DevicePath failed: %r\n",
+      Status));
+    goto FreeRamfbDevicePath;
+  }
+
+  //
+  // gop devpath + protocol
+  //
+  ZeroMem (&AcpiDeviceNode, sizeof (ACPI_ADR_DEVICE_PATH));
+  AcpiDeviceNode.Header.Type = ACPI_DEVICE_PATH;
+  AcpiDeviceNode.Header.SubType = ACPI_ADR_DP;
+  AcpiDeviceNode.ADR = ACPI_DISPLAY_ADR (
+    1,                                       // DeviceIdScheme
+    0,                                       // HeadId
+    0,                                       // NonVgaOutput
+    1,                                       // BiosCanDetect
+    0,                                       // VendorInfo
+    ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL,  // Type
+    0,                                       // Port
+    0                                        // Index
+    );
+  SetDevicePathNodeLength (&AcpiDeviceNode.Header,
+    sizeof (ACPI_ADR_DEVICE_PATH));
+
+  GopDevicePath = AppendDevicePathNode (RamfbDevicePath,
+    (EFI_DEVICE_PATH_PROTOCOL *) &AcpiDeviceNode);
+  if (GopDevicePath == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    goto FreeRamfbHandle;
+  }
+
+  Status = gBS->InstallMultipleProtocolInterfaces (
+                  &mGopHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  GopDevicePath,
+                  &gEfiGraphicsOutputProtocolGuid,
+                  &QemuRamfbGraphicsOutput,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Ramfb: install GOP DevicePath failed: %r\n",
+      Status));
+    goto FreeGopDevicePath;
+  }
+
+  Status = gBS->OpenProtocol (
+                  mRamfbHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &DevicePath,
+                  gImageHandle,
+                  mGopHandle,
+                  EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Ramfb: OpenProtocol failed: %r\n", Status));
+    goto FreeGopHandle;
+  }
+
+  return EFI_SUCCESS;
+
+FreeGopHandle:
+  gBS->UninstallMultipleProtocolInterfaces (
+         mGopHandle,
+         &gEfiDevicePathProtocolGuid,
+         GopDevicePath,
+         &gEfiGraphicsOutputProtocolGuid,
+         &QemuRamfbGraphicsOutput,
+         NULL
+         );
+FreeGopDevicePath:
+  FreePool (GopDevicePath);
+FreeRamfbHandle:
+  gBS->UninstallMultipleProtocolInterfaces (
+         mRamfbHandle,
+         &gEfiDevicePathProtocolGuid,
+         RamfbDevicePath,
+         NULL
+         );
+FreeRamfbDevicePath:
+  FreePool (RamfbDevicePath);
+FreeFramebuffer:
+  FreePool ((VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase);
+  return Status;
+}
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index a2c995b910..7ddda89999 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -745,6 +745,7 @@
   MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
   OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
   OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 
   #
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index b199713925..52b8b1fea1 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -351,6 +351,7 @@ INF  RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
 !endif
 
 INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
 INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 INF  OvmfPkg/PlatformDxe/Platform.inf
 INF  OvmfPkg/IoMmuDxe/IoMmuDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index bc7db229d2..3481cdc36b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -754,6 +754,7 @@
   MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
   OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
   OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 
   #
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 4ebf64b2b9..70845d6972 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -357,6 +357,7 @@ INF  RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
 !endif
 
 INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
 INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 INF  OvmfPkg/PlatformDxe/Platform.inf
 INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 0767b34d18..8b0895b0ff 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -752,6 +752,7 @@
   MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
   OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
   OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 
   #
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 9ca96f9282..1eb46ac9a2 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -357,6 +357,7 @@ INF  RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
 !endif
 
 INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
 INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 INF  OvmfPkg/PlatformDxe/Platform.inf
 INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
new file mode 100644
index 0000000000..5a9fd42c32
--- /dev/null
+++ b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
@@ -0,0 +1,52 @@
+## @file
+#  This driver is a implementation of the Graphics Output Protocol
+#  for the QEMU ramfb device.
+#
+#  Copyright (c) 2018, Red Hat Inc.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution. The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = QemuRamfbDxe
+  FILE_GUID                      = dce1b094-7dc6-45d0-9fdd-d7fc3cc3e4ef
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+
+  ENTRY_POINT                    = InitializeQemuRamfb
+
+[Sources]
+  QemuRamfb.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  DevicePathLib
+  FrameBufferBltLib
+  MemoryAllocationLib
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  QemuFwCfgLib
+
+[Protocols]
+  gEfiGraphicsOutputProtocolGuid                # BY_START
+
+[Guids]
+  gQemuRamfbGuid
+
+[Depex]
+  TRUE
-- 
2.9.3

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

[Laszlo Ersek]

Hi Gerd,

On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add a driver for the qemu ramfb display device.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/QemuRamfbDxe/QemuRamfb.c      | 396 ++++++++++++++++++++++++++++++++++
>  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/QemuRamfbDxe/QemuRamfbDxe.inf |  52 +++++
>  8 files changed, 454 insertions(+)
>  create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfb.c
>  create mode 100644 OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> 
> diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfb.c b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> new file mode 100644
> index 0000000000..00a2875e99
> --- /dev/null
> +++ b/OvmfPkg/QemuRamfbDxe/QemuRamfb.c
> @@ -0,0 +1,396 @@
> +/** @file
> +  This driver is a implementation of the Graphics Output Protocol
> +  for the QEMU ramfb device.
> +
> +  Copyright (c) 2018, Red Hat Inc.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution. The full text of the license may be found at

(1) These lines are overlong, can you please wrap them to 80 chars?

> +  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 <Protocol/GraphicsOutput.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/FrameBufferBltLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/QemuFwCfgLib.h>
> +
> +#include <Guid/QemuRamfb.h>
> +
> +#define RAMFB_FORMAT  0x34325258 /* DRM_FORMAT_XRGB8888 */
> +#define RAMFB_BPP     4
> +
> +#pragma pack (1)
> +typedef struct RAMFB_CONFIG {
> +  UINT64 Address;
> +  UINT32 FourCC;
> +  UINT32 Flags;
> +  UINT32 Width;
> +  UINT32 Height;
> +  UINT32 Stride;
> +} RAMFB_CONFIG;
> +#pragma pack ()
> +
> +static EFI_HANDLE                    mRamfbHandle;

(2) So, when I mentioned "STATIC" in my last email, I meant "STATIC",
not "static" :) I see you couldn't believe me! :) I apologize; yes,
indeed in edk2 we spell "static" "STATIC". Can you please update all
occurrences?

> +static EFI_HANDLE                    mGopHandle;
> +static FRAME_BUFFER_CONFIGURE        *mQemuRamfbFrameBufferBltConfigure;
> +static UINTN                         mQemuRamfbFrameBufferBltConfigureSize;
> +static FIRMWARE_CONFIG_ITEM          mRamFbFwCfgItem;

(3) My bad, I suggested inconsistent spelling for "mRamFbFwCfgItem". You
spelled it everywhere consistently "Ramfb", in identifiers.

Can you please update this to "mRamfbFwCfgItem" too?

> +
> +static EFI_GRAPHICS_OUTPUT_MODE_INFORMATION mQemuRamfbModeInfo[] = {
> +  {
> +    0,    // Version
> +    640,  // HorizontalResolution
> +    480,  // VerticalResolution
> +  },{
> +    0,    // Version
> +    800,  // HorizontalResolution
> +    600,  // VerticalResolution
> +  },{
> +    0,    // Version
> +    1024, // HorizontalResolution
> +    768,  // VerticalResolution
> +  }
> +};
> +#define mQemuRamfbModeCount ARRAY_SIZE(mQemuRamfbModeInfo)

(4) Sorry I was unclear about this in my last email. What I meant is to
eliminate the "mQemuRamfbModeCount" macro entirely, and use the
ARRAY_SIZE() in its place. I count two instances in the code.

Alternatively, if you really like a dedicated macro for this, please
spell it QEMU_RAMFB_MODE_COUNT.

Either way: please insert a space character after ARRAY_SIZE and before "(".

> +
> +static EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE mQemuRamfbMode = {
> +  mQemuRamfbModeCount,                            // MaxMode
> +  0,                                              // Mode
> +  mQemuRamfbModeInfo,                             // Info
> +  sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION),  // SizeOfInfo
> +};
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +QemuRamfbGraphicsOutputQueryMode (
> +  IN  EFI_GRAPHICS_OUTPUT_PROTOCOL          *This,
> +  IN  UINT32                                ModeNumber,
> +  OUT UINTN                                 *SizeOfInfo,
> +  OUT EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  **Info
> +  )
> +{
> +  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *ModeInfo;
> +
> +  if (Info == NULL || SizeOfInfo == NULL || ModeNumber >= mQemuRamfbMode.MaxMode) {

(5) This line is too long; please use a 80 chars width.

> +    return EFI_INVALID_PARAMETER;
> +  }
> +  ModeInfo = &mQemuRamfbModeInfo[ModeNumber];
> +
> +  *Info = AllocateCopyPool (sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION),
> +            ModeInfo);
> +  if (*Info == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +  *SizeOfInfo = sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +QemuRamfbGraphicsOutputSetMode (
> +  IN  EFI_GRAPHICS_OUTPUT_PROTOCOL *This,
> +  IN  UINT32                       ModeNumber
> +  )
> +{
> +  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *ModeInfo;
> +  RAMFB_CONFIG                          Config;
> +  EFI_GRAPHICS_OUTPUT_BLT_PIXEL         Black;
> +  RETURN_STATUS                         Status;
> +
> +  if (ModeNumber >= mQemuRamfbMode.MaxMode) {
> +    return EFI_UNSUPPORTED;
> +  }
> +  ModeInfo = &mQemuRamfbModeInfo[ModeNumber];
> +
> +  DEBUG ((DEBUG_INFO, "Ramfb: SetMode %u (%ux%u)\n", ModeNumber,
> +    ModeInfo->HorizontalResolution, ModeInfo->VerticalResolution));
> +
> +  Config.Address = SwapBytes64 (mQemuRamfbMode.FrameBufferBase);
> +  Config.FourCC  = SwapBytes32 (RAMFB_FORMAT);
> +  Config.Flags   = SwapBytes32 (0);
> +  Config.Width   = SwapBytes32 (ModeInfo->HorizontalResolution);
> +  Config.Height  = SwapBytes32 (ModeInfo->VerticalResolution);
> +  Config.Stride  = SwapBytes32 (ModeInfo->HorizontalResolution * RAMFB_BPP);
> +
> +  Status = FrameBufferBltConfigure (
> +             (VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase,
> +             ModeInfo,
> +             mQemuRamfbFrameBufferBltConfigure,
> +             &mQemuRamfbFrameBufferBltConfigureSize
> +             );
> +
> +  if (Status == RETURN_BUFFER_TOO_SMALL) {
> +    if (mQemuRamfbFrameBufferBltConfigure != NULL) {
> +      FreePool (mQemuRamfbFrameBufferBltConfigure);
> +    }
> +    mQemuRamfbFrameBufferBltConfigure =
> +      AllocatePool (mQemuRamfbFrameBufferBltConfigureSize);
> +    if (mQemuRamfbFrameBufferBltConfigure == NULL) {
> +      return EFI_OUT_OF_RESOURCES;

(6) Small logic issue here: at this point
"mQemuRamfbFrameBufferBltConfigure" is NULL (which is good), but
"mQemuRamfbFrameBufferBltConfigureSize" is nonzero (in fact it is "large
enough"; it's just been returned to us with the correct size).

As a result, at the next attempt to set the same mode,
FrameBufferBltConfigure() will dereference NULL.

So please, before returning, assign
"mQemuRamfbFrameBufferBltConfigureSize" zero.

> +    }
> +
> +    Status = FrameBufferBltConfigure (
> +               (VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase,
> +               ModeInfo,
> +               mQemuRamfbFrameBufferBltConfigure,
> +               &mQemuRamfbFrameBufferBltConfigureSize);

(7) Please break off the ");" to a separate line, as discussed before.

> +    if (RETURN_ERROR (Status)) {
> +      ASSERT (Status == RETURN_UNSUPPORTED);
> +      return Status;
> +    }

(8) I think I was unclear about this in my last email, sorry about that
again. I meant for this RETURN_ERROR() check to be placed *after* (not
inside) the RETURN_BUFFER_TOO_SMALL branch.

With the idea being, after the end of the "too small" branch, regardless
of us actually having taken the "too small" branch, Status must either
be "success", or "unsupported".

Currently the code, as-is, will not catch the first
FrameBufferBltConfigure() call returning "unsupported".

> +  }
> +
> +  mQemuRamfbMode.Mode = ModeNumber;
> +  mQemuRamfbMode.Info = ModeInfo;
> +
> +  QemuFwCfgSelectItem (mRamFbFwCfgItem);
> +  QemuFwCfgWriteBytes (sizeof (Config), &Config);
> +
> +  //
> +  // clear screen
> +  //
> +  ZeroMem (&Black, sizeof (Black));
> +  Status = FrameBufferBlt (
> +             mQemuRamfbFrameBufferBltConfigure,
> +             &Black,
> +             EfiBltVideoFill,
> +             0,                               // SourceX -- ignored
> +             0,                               // SourceY -- ignored
> +             0,                               // DestinationX
> +             0,                               // DestinationY
> +             ModeInfo->HorizontalResolution,  // Width
> +             ModeInfo->VerticalResolution,    // Height
> +             0                                // Delta -- ignored
> +             );
> +  if (EFI_ERROR (Status)) {

(9) For stylistic reasons, please use "RETURN_ERROR (Status)" here. (See
the previous discussion on RETURN_STATUS vs. EFI_STATUS.)

> +    DEBUG ((DEBUG_WARN, "%a: clearing the screen failed: %r\n",
> +            __FUNCTION__, Status));

(10) Please fix the indentation.

> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +static
> +EFI_STATUS
> +EFIAPI
> +QemuRamfbGraphicsOutputBlt (
> +  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
> +  )
> +{
> +  return FrameBufferBlt (
> +           mQemuRamfbFrameBufferBltConfigure,
> +           BltBuffer,
> +           BltOperation,
> +           SourceX,
> +           SourceY,
> +           DestinationX,
> +           DestinationY,
> +           Width,
> +           Height,
> +           Delta);

(11) Please break off the ");" to a separate line.

> +}
> +
> +EFI_GRAPHICS_OUTPUT_PROTOCOL QemuRamfbGraphicsOutput = {

(12) Please make this STATIC.

(13) Please rename it to "mQemuRamfbGraphicsOutput".

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

(14) Can you rename "Size" to "FwCfgSize" as I requested earlier please?

(15) I see that you are reluctant accepting that edk2 really prefers to
break almost all variable definitions to separate lines :) I suggest the
following compromise:

  UINTN                     FbSize, MaxFbSize, Pages;
  UINTN                     FwCfgSize;
  UINTN                     Index;

Because, an argument can be made that FbSize, MaxFbSize, and Pages
belong together. However, FwCfgSize and Index should each stand alone.

> +
> +  if (!QemuFwCfgIsAvailable()) {

(16) Oops, I missed this last time; please add a space between function
identifier and ().

> +    DEBUG ((DEBUG_INFO, "Ramfb: no FwCfg\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  Status = QemuFwCfgFindFile("etc/ramfb", &mRamFbFwCfgItem, &Size);

(17) Ditto.

> +  if (EFI_ERROR (Status)) {
> +    return EFI_NOT_FOUND;
> +  }
> +  if (Size != sizeof (RAMFB_CONFIG)) {
> +    DEBUG ((DEBUG_ERROR, "Ramfb: FwCfg size mismatch (expected %d, got %d)\n",
> +             sizeof (RAMFB_CONFIG), Size));

(18) Indentation.

(19) Additionally, "sizeof" has type UINTN, and "Size" (FwCfgSize) has
type UINTN too. Please cast them both to UINT64 and print them with %lu.

> +    return EFI_NOT_FOUND;

(20) I generally return EFI_PROTOCOL_ERROR whenever fw_cfg produces
garbage, but EFI_NOT_FOUND is perfectly acceptable as well. Up to you.

> +  }
> +
> +  MaxFbSize = 0;
> +  for (Index = 0; Index < mQemuRamfbModeCount; Index++) {
> +    mQemuRamfbModeInfo[Index].PixelsPerScanLine =
> +      mQemuRamfbModeInfo[Index].HorizontalResolution;
> +    mQemuRamfbModeInfo[Index].PixelFormat =
> +      PixelBlueGreenRedReserved8BitPerColor;
> +    FbSize = RAMFB_BPP *
> +      mQemuRamfbModeInfo[Index].HorizontalResolution *
> +      mQemuRamfbModeInfo[Index].VerticalResolution;
> +    if (MaxFbSize < FbSize) {
> +      MaxFbSize = FbSize;
> +    }
> +    DEBUG ((DEBUG_INFO, "Ramfb: Mode %u: %ux%u, %lu kB\n", (UINT64)Index,

(21) The cast is OK, but the conversion specifier should be %lu.

> +      mQemuRamfbModeInfo[Index].HorizontalResolution,
> +      mQemuRamfbModeInfo[Index].VerticalResolution,
> +      (UINT64)FbSize / 1024));

(22) Sorry, this division is no longer portable (between 64-bit and
32-bit edk2 builds) in this form. The reason is that UINT64 values can't
*generally* be divided in 32-bit builds without compiler intrinsics; in
order to avoid those, we use named helper functions for such divisions.

However, using those helpers wasn't what I meant; I meant

  (UINT64)(FbSize / 1024)

Because, FbSize is UINTN (i.e. UINT32 in 32-bit builds, UINT64 in 64-bit
builds), and so it can be divided by 1024 (of type INT32) without
helpers. It's the UINTN *quotient* that we should cast, for portable
printing.

I apologize if my previous comment on this was misleading.

> +  }
> +
> +  Pages = EFI_SIZE_TO_PAGES (MaxFbSize);
> +  MaxFbSize = EFI_PAGES_TO_SIZE (Pages);
> +  FbBase = (EFI_PHYSICAL_ADDRESS)(UINTN)AllocateReservedPages (Pages);
> +  if (FbBase == 0) {
> +    DEBUG ((DEBUG_INFO, "Ramfb: memory allocation failed\n"));

(23) Sorry, missed this last time, should be DEBUG_ERROR.

> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +  DEBUG ((DEBUG_INFO, "Ramfb: Framebuffer at 0x%lx, %ld kB, %ld pages\n",
> +          (UINT64)FbBase, (UINT64)MaxFbSize / 1024, (UINT64)Pages));

(24) Indentation.

(25) Regarding the MaxFbSize division, please see (22).

(26) Please use %lu for printing both the MaxFbSize quotient (see above)
and Pages. %ld is for INT64.

> +  mQemuRamfbMode.FrameBufferSize = MaxFbSize;
> +  mQemuRamfbMode.FrameBufferBase = FbBase;
> +
> +  //
> +  // 800 x 600
> +  //
> +  QemuRamfbGraphicsOutputSetMode (&QemuRamfbGraphicsOutput, 1);
> +
> +  //
> +  // ramfb vendor devpath
> +  //
> +  VendorDeviceNode.Header.Type = HARDWARE_DEVICE_PATH;
> +  VendorDeviceNode.Header.SubType = HW_VENDOR_DP;
> +  CopyGuid(&VendorDeviceNode.Guid, &gQemuRamfbGuid);

(27) Please add a space after "CopyGuid".

> +  SetDevicePathNodeLength (&VendorDeviceNode.Header,
> +    sizeof (VENDOR_DEVICE_PATH));
> +
> +  RamfbDevicePath = AppendDevicePathNode (NULL,
> +    (EFI_DEVICE_PATH_PROTOCOL *) &VendorDeviceNode);
> +  if (RamfbDevicePath == NULL) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto FreeFramebuffer;
> +  }
> +
> +  Status = gBS->InstallMultipleProtocolInterfaces (
> +                  &mRamfbHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  RamfbDevicePath,
> +                  NULL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Ramfb: install Ramfb Vendor DevicePath failed: %r\n",
> +      Status));
> +    goto FreeRamfbDevicePath;
> +  }
> +
> +  //
> +  // gop devpath + protocol
> +  //
> +  ZeroMem (&AcpiDeviceNode, sizeof (ACPI_ADR_DEVICE_PATH));

(28) I think you can drop the ZeroMem(); all fields are set explicitly.

> +  AcpiDeviceNode.Header.Type = ACPI_DEVICE_PATH;
> +  AcpiDeviceNode.Header.SubType = ACPI_ADR_DP;
> +  AcpiDeviceNode.ADR = ACPI_DISPLAY_ADR (
> +    1,                                       // DeviceIdScheme
> +    0,                                       // HeadId
> +    0,                                       // NonVgaOutput
> +    1,                                       // BiosCanDetect
> +    0,                                       // VendorInfo
> +    ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL,  // Type
> +    0,                                       // Port
> +    0                                        // Index
> +    );

Sigh, this is one of those situations when different coding style
guidelines conflict. One style requirement would be to indent the
arguments two columns to the right of the beginning of
"ACPI_DISPLAY_ADR". However, in that case, we wouldn't fit in 80
characters; and in my book, that's a lot worse. So, just leave this as
it is. :/

> +  SetDevicePathNodeLength (&AcpiDeviceNode.Header,
> +    sizeof (ACPI_ADR_DEVICE_PATH));
> +
> +  GopDevicePath = AppendDevicePathNode (RamfbDevicePath,
> +    (EFI_DEVICE_PATH_PROTOCOL *) &AcpiDeviceNode);
> +  if (GopDevicePath == NULL) {
> +    Status = EFI_OUT_OF_RESOURCES;
> +    goto FreeRamfbHandle;
> +  }
> +
> +  Status = gBS->InstallMultipleProtocolInterfaces (
> +                  &mGopHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  GopDevicePath,
> +                  &gEfiGraphicsOutputProtocolGuid,
> +                  &QemuRamfbGraphicsOutput,
> +                  NULL
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Ramfb: install GOP DevicePath failed: %r\n",
> +      Status));
> +    goto FreeGopDevicePath;
> +  }
> +
> +  Status = gBS->OpenProtocol (
> +                  mRamfbHandle,
> +                  &gEfiDevicePathProtocolGuid,
> +                  &DevicePath,
> +                  gImageHandle,
> +                  mGopHandle,
> +                  EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
> +                  );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "Ramfb: OpenProtocol failed: %r\n", Status));
> +    goto FreeGopHandle;
> +  }
> +
> +  return EFI_SUCCESS;
> +
> +FreeGopHandle:
> +  gBS->UninstallMultipleProtocolInterfaces (
> +         mGopHandle,
> +         &gEfiDevicePathProtocolGuid,
> +         GopDevicePath,
> +         &gEfiGraphicsOutputProtocolGuid,
> +         &QemuRamfbGraphicsOutput,
> +         NULL
> +         );
> +FreeGopDevicePath:
> +  FreePool (GopDevicePath);
> +FreeRamfbHandle:
> +  gBS->UninstallMultipleProtocolInterfaces (
> +         mRamfbHandle,
> +         &gEfiDevicePathProtocolGuid,
> +         RamfbDevicePath,
> +         NULL
> +         );
> +FreeRamfbDevicePath:
> +  FreePool (RamfbDevicePath);
> +FreeFramebuffer:
> +  FreePool ((VOID*)(UINTN)mQemuRamfbMode.FrameBufferBase);

(29) Oops, completely missed this in the previous review, multiple times
-- please use FreePages() for releasing FrameBufferBase, not FreePool()!

> +  return Status;
> +}
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index a2c995b910..7ddda89999 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -745,6 +745,7 @@
>    MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>  
>    OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>    OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  
>    #
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index b199713925..52b8b1fea1 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -351,6 +351,7 @@ INF  RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
>  !endif
>  
>  INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>  INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  INF  OvmfPkg/PlatformDxe/Platform.inf
>  INF  OvmfPkg/IoMmuDxe/IoMmuDxe.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index bc7db229d2..3481cdc36b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -754,6 +754,7 @@
>    MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>  
>    OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>    OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  
>    #
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 4ebf64b2b9..70845d6972 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -357,6 +357,7 @@ INF  RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
>  !endif
>  
>  INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>  INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  INF  OvmfPkg/PlatformDxe/Platform.inf
>  INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 0767b34d18..8b0895b0ff 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -752,6 +752,7 @@
>    MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>  
>    OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>    OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  
>    #
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 9ca96f9282..1eb46ac9a2 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -357,6 +357,7 @@ INF  RuleOverride=CSM OvmfPkg/Csm/Csm16/Csm16.inf
>  !endif
>  
>  INF  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>  INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>  INF  OvmfPkg/PlatformDxe/Platform.inf
>  INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> diff --git a/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> new file mode 100644
> index 0000000000..5a9fd42c32
> --- /dev/null
> +++ b/OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
> @@ -0,0 +1,52 @@
> +## @file
> +#  This driver is a implementation of the Graphics Output Protocol
> +#  for the QEMU ramfb device.
> +#
> +#  Copyright (c) 2018, Red Hat Inc.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution. The full text of the license may be found at

(30) These two lines are too long, can you please rewrap them to 80 chars?

> +#  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                      = QemuRamfbDxe
> +  FILE_GUID                      = dce1b094-7dc6-45d0-9fdd-d7fc3cc3e4ef
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +
> +  ENTRY_POINT                    = InitializeQemuRamfb
> +
> +[Sources]
> +  QemuRamfb.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  DevicePathLib
> +  FrameBufferBltLib
> +  MemoryAllocationLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  QemuFwCfgLib
> +
> +[Protocols]
> +  gEfiGraphicsOutputProtocolGuid                # BY_START

(31) As requested previously, drop "# BY_START" and say ""## PRODUCES"
please.

> +
> +[Guids]
> +  gQemuRamfbGuid
> +
> +[Depex]
> +  TRUE
> 

Thanks!
Laszlo

[PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console

[Gerd Hoffmann]

Add QemuRamfbDxe device path to the list of platform console devices,
so ConSplitter will pick up the device even though it isn't a PCI GPU.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 .../Library/PlatformBootManagerLib/PlatformData.c  | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
index a50cd7bcaf..1250a6d351 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
@@ -14,6 +14,7 @@
 **/
 
 #include "BdsPlatform.h"
+#include <Guid/QemuRamfb.h>
 
 //
 // Debug Agent UART Device Path structure
@@ -37,6 +38,17 @@ typedef struct {
 } USB_KEYBOARD_DEVICE_PATH;
 #pragma pack ()
 
+//
+// QemuRamfb Device Path structure
+//
+#pragma pack (1)
+typedef struct {
+  VENDOR_DEVICE_PATH        Vendor;
+  ACPI_ADR_DEVICE_PATH      AcpiAdr;
+  EFI_DEVICE_PATH_PROTOCOL  End;
+} VENDOR_RAMFB_DEVICE_PATH;
+#pragma pack ()
+
 ACPI_HID_DEVICE_PATH       gPnpPs2KeyboardDeviceNode  = gPnpPs2Keyboard;
 ACPI_HID_DEVICE_PATH       gPnp16550ComPortDeviceNode = gPnp16550ComPort;
 UART_DEVICE_PATH           gUartDeviceNode            = gUart;
@@ -100,6 +112,41 @@ STATIC USB_KEYBOARD_DEVICE_PATH gUsbKeyboardDevicePath = {
   gEndEntire
 };
 
+STATIC VENDOR_RAMFB_DEVICE_PATH gQemuRamfbDevicePath = {
+  {
+    {
+      HARDWARE_DEVICE_PATH,
+      HW_VENDOR_DP,
+      {
+        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
+        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
+      }
+    },
+    QEMU_RAMFB_GUID,
+  },
+  {
+    {
+      ACPI_DEVICE_PATH,
+      ACPI_ADR_DP,
+      {
+        (UINT8) (sizeof (ACPI_ADR_DEVICE_PATH)),
+        (UINT8) ((sizeof (ACPI_ADR_DEVICE_PATH)) >> 8)
+      }
+    },
+    ACPI_DISPLAY_ADR (
+      1,                                       // DeviceIdScheme
+      0,                                       // HeadId
+      0,                                       // NonVgaOutput
+      1,                                       // BiosCanDetect
+      0,                                       // VendorInfo
+      ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL,  // Type
+      0,                                       // Port
+      0                                        // Index
+      ),
+  },
+  gEndEntire
+};
+
 //
 // Predefined platform default console device path
 //
@@ -113,6 +160,10 @@ PLATFORM_CONSOLE_CONNECT_ENTRY   gPlatformConsole[] = {
     CONSOLE_IN
   },
   {
+    (EFI_DEVICE_PATH_PROTOCOL *)&gQemuRamfbDevicePath,
+    CONSOLE_OUT
+  },
+  {
     NULL,
     0
   }
-- 
2.9.3

Re: [PATCH v2 3/4] OvmfPkg: add QemuRamfb to platform console

[Laszlo Ersek]

On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add QemuRamfbDxe device path to the list of platform console devices,
> so ConSplitter will pick up the device even though it isn't a PCI GPU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  .../Library/PlatformBootManagerLib/PlatformData.c  | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
> index a50cd7bcaf..1250a6d351 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
> @@ -14,6 +14,7 @@
>  **/
>  
>  #include "BdsPlatform.h"
> +#include <Guid/QemuRamfb.h>
>  
>  //
>  // Debug Agent UART Device Path structure
> @@ -37,6 +38,17 @@ typedef struct {
>  } USB_KEYBOARD_DEVICE_PATH;
>  #pragma pack ()
>  
> +//
> +// QemuRamfb Device Path structure
> +//
> +#pragma pack (1)
> +typedef struct {
> +  VENDOR_DEVICE_PATH        Vendor;
> +  ACPI_ADR_DEVICE_PATH      AcpiAdr;
> +  EFI_DEVICE_PATH_PROTOCOL  End;
> +} VENDOR_RAMFB_DEVICE_PATH;
> +#pragma pack ()
> +
>  ACPI_HID_DEVICE_PATH       gPnpPs2KeyboardDeviceNode  = gPnpPs2Keyboard;
>  ACPI_HID_DEVICE_PATH       gPnp16550ComPortDeviceNode = gPnp16550ComPort;
>  UART_DEVICE_PATH           gUartDeviceNode            = gUart;
> @@ -100,6 +112,41 @@ STATIC USB_KEYBOARD_DEVICE_PATH gUsbKeyboardDevicePath = {
>    gEndEntire
>  };
>  
> +STATIC VENDOR_RAMFB_DEVICE_PATH gQemuRamfbDevicePath = {
> +  {
> +    {
> +      HARDWARE_DEVICE_PATH,
> +      HW_VENDOR_DP,
> +      {
> +        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
> +        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    QEMU_RAMFB_GUID,
> +  },
> +  {
> +    {
> +      ACPI_DEVICE_PATH,
> +      ACPI_ADR_DP,
> +      {
> +        (UINT8) (sizeof (ACPI_ADR_DEVICE_PATH)),
> +        (UINT8) ((sizeof (ACPI_ADR_DEVICE_PATH)) >> 8)
> +      }
> +    },
> +    ACPI_DISPLAY_ADR (
> +      1,                                       // DeviceIdScheme
> +      0,                                       // HeadId
> +      0,                                       // NonVgaOutput
> +      1,                                       // BiosCanDetect
> +      0,                                       // VendorInfo
> +      ACPI_ADR_DISPLAY_TYPE_EXTERNAL_DIGITAL,  // Type
> +      0,                                       // Port
> +      0                                        // Index
> +      ),
> +  },
> +  gEndEntire
> +};
> +
>  //
>  // Predefined platform default console device path
>  //
> @@ -113,6 +160,10 @@ PLATFORM_CONSOLE_CONNECT_ENTRY   gPlatformConsole[] = {
>      CONSOLE_IN
>    },
>    {
> +    (EFI_DEVICE_PATH_PROTOCOL *)&gQemuRamfbDevicePath,
> +    CONSOLE_OUT
> +  },
> +  {
>      NULL,
>      0
>    }
> 

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

[PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe

[Gerd Hoffmann]

Add QemuRamfbDxe to dsc and fdf files for ArmVirt package.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ArmVirtPkg/ArmVirtQemu.dsc           | 2 ++
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc     | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index d74feb709c..744d127a10 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -57,6 +57,7 @@
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+  FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
   QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
   PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
@@ -391,6 +392,7 @@
   #
   # Video support
   #
+  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
   OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
   OvmfPkg/PlatformDxe/Platform.inf
 
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 89f95b2d99..63a202c788 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -175,6 +175,7 @@ READ_LOCK_STATUS   = TRUE
   #
   # Video support
   #
+  INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
   INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
   INF OvmfPkg/PlatformDxe/Platform.inf
 
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 1e823aeab7..e59f53b728 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -57,6 +57,7 @@
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+  FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
   QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
   PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
@@ -380,6 +381,7 @@
   #
   # Video support
   #
+  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
   OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
   OvmfPkg/PlatformDxe/Platform.inf
 
-- 
2.9.3

Re: [PATCH v2 4/4] ArmVirtPkg: add QemuRamfbDxe

[Laszlo Ersek]

On 06/12/18 11:31, Gerd Hoffmann wrote:
> Add QemuRamfbDxe to dsc and fdf files for ArmVirt package.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ArmVirtPkg/ArmVirtQemu.dsc           | 2 ++
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 1 +
>  ArmVirtPkg/ArmVirtQemuKernel.dsc     | 2 ++
>  3 files changed, 5 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index d74feb709c..744d127a10 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -57,6 +57,7 @@
>    BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
>    PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>    CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
> +  FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
>    QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
>    FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
>    PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> @@ -391,6 +392,7 @@
>    #
>    # Video support
>    #
> +  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>    OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>    OvmfPkg/PlatformDxe/Platform.inf
>  
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index 89f95b2d99..63a202c788 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -175,6 +175,7 @@ READ_LOCK_STATUS   = TRUE
>    #
>    # Video support
>    #
> +  INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>    INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>    INF OvmfPkg/PlatformDxe/Platform.inf
>  
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 1e823aeab7..e59f53b728 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -57,6 +57,7 @@
>    BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
>    PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>    CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
> +  FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
>    QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
>    FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
>    PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> @@ -380,6 +381,7 @@
>    #
>    # Video support
>    #
> +  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
>    OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>    OvmfPkg/PlatformDxe/Platform.inf
>  
> 

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