public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch] OvmfPkg/QemuVideoDxe: Remove dependency on OptionRomPkg
@ 2019-05-09 17:39 Michael D Kinney
  2019-05-09 23:25 ` Laszlo Ersek
  0 siblings, 1 reply; 2+ messages in thread
From: Michael D Kinney @ 2019-05-09 17:39 UTC (permalink / raw)
  To: devel; +Cc: Jordan Justen, Laszlo Ersek, Ard Biesheuvel

Update the QemuVideoDxe driver to not depend on the
OptionRomPkg to support moving OptionRomPkg to the
edk2-platforms repository.

The only dependency on the OptionRomPkg is the use of
PcdDriverSupportedEfiVersion to set the version value in the
EFI Driver Supported EFI Version Protocol.  This protocol is
intended for use in drivers for add-in devices, which does not
apply to the QEMU integrated video controller.  Since this
protocol does not apply to QEMU environment, remove both the
PCD and the installation of the EFI Driver Supported EFI
Version Protocol.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 OvmfPkg/QemuVideoDxe/Driver.c                    | 15 +--------------
 OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c | 15 ---------------
 OvmfPkg/QemuVideoDxe/Qemu.h                      |  3 +--
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf            |  6 +-----
 4 files changed, 3 insertions(+), 36 deletions(-)
 delete mode 100644 OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c

diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 45bcfb8fd1..e8a613ef33 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -2,7 +2,7 @@
   This driver is a sample implementation of the Graphics Output Protocol for
   the QEMU (Cirrus Logic 5446) video controller.
 
-  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -982,18 +982,5 @@ InitializeQemuVideo (
              );
   ASSERT_EFI_ERROR (Status);
 
-  //
-  // Install EFI Driver Supported EFI Version Protocol required for
-  // EFI drivers that are on PCI and other plug in cards.
-  //
-  gQemuVideoDriverSupportedEfiVersion.FirmwareVersion = PcdGet32 (PcdDriverSupportedEfiVersion);
-  Status = gBS->InstallMultipleProtocolInterfaces (
-                  &ImageHandle,
-                  &gEfiDriverSupportedEfiVersionProtocolGuid,
-                  &gQemuVideoDriverSupportedEfiVersion,
-                  NULL
-                  );
-  ASSERT_EFI_ERROR (Status);
-
   return Status;
 }
diff --git a/OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c b/OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c
deleted file mode 100644
index c06f1d73cf..0000000000
--- a/OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c
+++ /dev/null
@@ -1,15 +0,0 @@
-/** @file
-  Driver supported version protocol for the QEMU video driver.
-
-  Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-#include "Qemu.h"
-
-EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL gQemuVideoDriverSupportedEfiVersion = {
-  sizeof (EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL), // Size of Protocol structure.
-  0                                                   // Version number to be filled at start up.
-};
-
diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index 2b64f1e2b0..87c933935f 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -1,7 +1,7 @@
 /** @file
   QEMU Video Controller Driver
 
-  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -156,7 +156,6 @@ extern QEMU_VIDEO_BOCHS_MODES                     QemuVideoBochsModes[];
 extern EFI_DRIVER_BINDING_PROTOCOL                gQemuVideoDriverBinding;
 extern EFI_COMPONENT_NAME_PROTOCOL                gQemuVideoComponentName;
 extern EFI_COMPONENT_NAME2_PROTOCOL               gQemuVideoComponentName2;
-extern EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL  gQemuVideoDriverSupportedEfiVersion;
 
 //
 // Io Registers defined by VGA
diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index 4e29b0c20f..fe8befd51d 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -2,7 +2,7 @@
 #  This driver is a sample implementation of the Graphics Output Protocol for
 #  the QEMU (Cirrus Logic 5446) video controller.
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -29,7 +29,6 @@ [Defines]
 [Sources.common]
   ComponentName.c
   Driver.c
-  DriverSupportedEfiVersion.c
   Gop.c
   Initialize.c
   Qemu.h
@@ -41,7 +40,6 @@ [Sources.Ia32, Sources.X64]
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
-  OptionRomPkg/OptionRomPkg.dec
   OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
@@ -59,12 +57,10 @@ [LibraryClasses]
   UefiLib
 
 [Protocols]
-  gEfiDriverSupportedEfiVersionProtocolGuid     # PROTOCOL ALWAYS_PRODUCED
   gEfiGraphicsOutputProtocolGuid                # PROTOCOL BY_START
   gEfiDevicePathProtocolGuid                    # PROTOCOL BY_START
   gEfiPciIoProtocolGuid                         # PROTOCOL TO_START
 
 [Pcd]
-  gOptionRomPkgTokenSpaceGuid.PcdDriverSupportedEfiVersion
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
   gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Patch] OvmfPkg/QemuVideoDxe: Remove dependency on OptionRomPkg
  2019-05-09 17:39 [Patch] OvmfPkg/QemuVideoDxe: Remove dependency on OptionRomPkg Michael D Kinney
@ 2019-05-09 23:25 ` Laszlo Ersek
  0 siblings, 0 replies; 2+ messages in thread
From: Laszlo Ersek @ 2019-05-09 23:25 UTC (permalink / raw)
  To: Michael D Kinney, devel; +Cc: Jordan Justen, Ard Biesheuvel

Hello Mike,

On 05/09/19 19:39, Michael D Kinney wrote:
> Update the QemuVideoDxe driver to not depend on the
> OptionRomPkg to support moving OptionRomPkg to the
> edk2-platforms repository.
> 
> The only dependency on the OptionRomPkg is the use of
> PcdDriverSupportedEfiVersion to set the version value in the
> EFI Driver Supported EFI Version Protocol.  This protocol is
> intended for use in drivers for add-in devices, which does not
> apply to the QEMU integrated video controller.  Since this
> protocol does not apply to QEMU environment, remove both the
> PCD and the installation of the EFI Driver Supported EFI
> Version Protocol.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  OvmfPkg/QemuVideoDxe/Driver.c                    | 15 +--------------
>  OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c | 15 ---------------
>  OvmfPkg/QemuVideoDxe/Qemu.h                      |  3 +--
>  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf            |  6 +-----
>  4 files changed, 3 insertions(+), 36 deletions(-)
>  delete mode 100644 OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c

Thanks for the patch!

The code is ready to go in. I'd like to request one update with the
commit message. Please replace the following sentence in the middle:

    This protocol is intended for use in drivers for add-in devices,
    which does not apply to the QEMU integrated video controller.

with:

    Quoting the UEFI-2.8 spec, "This protocol is required for EFI
    drivers that are *on* PCI and other plug in cards" (emphasis ours).
    However, QemuVideoDxe is always part of the OVMF platform firmware,
    and is never read by PciBusDxe from the PCI ROM BAR of QEMU's
    emulated graphics cards.

(I'm asking for this update because I don't consider the QEMU GPUs in
question integrated devices (such as the IGD is, on physical boards).
The QEMU GPUs may not be hotpluggable, yes, but I think they still
qualify as discrete (cold-pluggable) PCI devices. For example, OVMF can
be run on "headless" (serial port only) QEMU VMs too.)

With that update:

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

Thank you very much!
Laszlo

> 
> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
> index 45bcfb8fd1..e8a613ef33 100644
> --- a/OvmfPkg/QemuVideoDxe/Driver.c
> +++ b/OvmfPkg/QemuVideoDxe/Driver.c
> @@ -2,7 +2,7 @@
>    This driver is a sample implementation of the Graphics Output Protocol for
>    the QEMU (Cirrus Logic 5446) video controller.
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
> @@ -982,18 +982,5 @@ InitializeQemuVideo (
>               );
>    ASSERT_EFI_ERROR (Status);
>  
> -  //
> -  // Install EFI Driver Supported EFI Version Protocol required for
> -  // EFI drivers that are on PCI and other plug in cards.
> -  //
> -  gQemuVideoDriverSupportedEfiVersion.FirmwareVersion = PcdGet32 (PcdDriverSupportedEfiVersion);
> -  Status = gBS->InstallMultipleProtocolInterfaces (
> -                  &ImageHandle,
> -                  &gEfiDriverSupportedEfiVersionProtocolGuid,
> -                  &gQemuVideoDriverSupportedEfiVersion,
> -                  NULL
> -                  );
> -  ASSERT_EFI_ERROR (Status);
> -
>    return Status;
>  }
> diff --git a/OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c b/OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c
> deleted file mode 100644
> index c06f1d73cf..0000000000
> --- a/OvmfPkg/QemuVideoDxe/DriverSupportedEfiVersion.c
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -/** @file
> -  Driver supported version protocol for the QEMU video driver.
> -
> -  Copyright (c) 2007 - 2010, Intel Corporation. All rights reserved.<BR>
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -#include "Qemu.h"
> -
> -EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL gQemuVideoDriverSupportedEfiVersion = {
> -  sizeof (EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL), // Size of Protocol structure.
> -  0                                                   // Version number to be filled at start up.
> -};
> -
> diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
> index 2b64f1e2b0..87c933935f 100644
> --- a/OvmfPkg/QemuVideoDxe/Qemu.h
> +++ b/OvmfPkg/QemuVideoDxe/Qemu.h
> @@ -1,7 +1,7 @@
>  /** @file
>    QEMU Video Controller Driver
>  
> -  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -156,7 +156,6 @@ extern QEMU_VIDEO_BOCHS_MODES                     QemuVideoBochsModes[];
>  extern EFI_DRIVER_BINDING_PROTOCOL                gQemuVideoDriverBinding;
>  extern EFI_COMPONENT_NAME_PROTOCOL                gQemuVideoComponentName;
>  extern EFI_COMPONENT_NAME2_PROTOCOL               gQemuVideoComponentName2;
> -extern EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL  gQemuVideoDriverSupportedEfiVersion;
>  
>  //
>  // Io Registers defined by VGA
> diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> index 4e29b0c20f..fe8befd51d 100644
> --- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> +++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
> @@ -2,7 +2,7 @@
>  #  This driver is a sample implementation of the Graphics Output Protocol for
>  #  the QEMU (Cirrus Logic 5446) video controller.
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -29,7 +29,6 @@ [Defines]
>  [Sources.common]
>    ComponentName.c
>    Driver.c
> -  DriverSupportedEfiVersion.c
>    Gop.c
>    Initialize.c
>    Qemu.h
> @@ -41,7 +40,6 @@ [Sources.Ia32, Sources.X64]
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> -  OptionRomPkg/OptionRomPkg.dec
>    OvmfPkg/OvmfPkg.dec
>  
>  [LibraryClasses]
> @@ -59,12 +57,10 @@ [LibraryClasses]
>    UefiLib
>  
>  [Protocols]
> -  gEfiDriverSupportedEfiVersionProtocolGuid     # PROTOCOL ALWAYS_PRODUCED
>    gEfiGraphicsOutputProtocolGuid                # PROTOCOL BY_START
>    gEfiDevicePathProtocolGuid                    # PROTOCOL BY_START
>    gEfiPciIoProtocolGuid                         # PROTOCOL TO_START
>  
>  [Pcd]
> -  gOptionRomPkgTokenSpaceGuid.PcdDriverSupportedEfiVersion
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
>    gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-05-09 23:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-09 17:39 [Patch] OvmfPkg/QemuVideoDxe: Remove dependency on OptionRomPkg Michael D Kinney
2019-05-09 23:25 ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox