Re: [PATCH v6 08/17] OvmfPkg: Add PlatformHasIoMmuLib

[Laszlo Ersek <lersek@redhat.com>]

comments below:

On 05/26/17 16:43, Brijesh Singh wrote:
> Add the shorter-term library instance outlined in the previous patch to
> OvmfPkg, so that we can imbue PciHostBridgeDxe with a protocol dependency
> on gEdkiiIoMmuProtocolGuid OR gIoMmuAbsentProtocolGuid.
> 
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf | 37 ++++++++++++++++++++
>  OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.c   | 32 +++++++++++++++++
>  2 files changed, 69 insertions(+)
> 
> diff --git a/OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf b/OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
> new file mode 100644
> index 000000000000..6d97293a3334
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
> @@ -0,0 +1,37 @@
> +## @file
> +# A hook-in library for MdeModulePkg/Bus/Pci/PciHostBridgeDxe.
> +#
> +#  Plugging this library instance into PciHostBridgeDxe makes
> +#  EFI_IOMMU_PROTOCOL depend on the platform's dynamic decision whether

(1) s/EFI_IOMMU_PROTOCOL/PciHostBridgeDxe/

> +#  to provide IOMMU implementation (usually through IoMmuDxe driver).
> +#
> +# Copyright (C) 2017, AMD, Inc.

(2) I think you should keep the original Red Hat copyright here as well,
from "EmbeddedPkg/Library/PlatformHasAcpiLib/PlatformHasAcpiLib.inf". (I
think you used that file as basis for this.)

(3) (4) Regarding the C source file below, the same two comments apply.

With those addressed:

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

Don't post v7 just for this; I can fix it up at commit, if nothing else
requries a v7.

Thanks
Laszlo

> +#
> +# 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                    = 1.25
> +  BASE_NAME                      = PlatformHasIoMmuLib
> +  FILE_GUID                      = 49b4b018-0558-448b-1e1a-12226342877d
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformHasIoMmuLib|DXE_DRIVER
> +  CONSTRUCTOR                    = PlatformHasIoMmuInitialize
> +
> +[Sources]
> +  PlatformHasIoMmuLib.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +
> +[Depex]
> +  gEdkiiIoMmuProtocolGuid OR gIoMmuAbsentProtocolGuid
> diff --git a/OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.c b/OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.c
> new file mode 100644
> index 000000000000..9ead2637e2c8
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.c
> @@ -0,0 +1,32 @@
> +/** @file
> +  A hook-in library for MdeModulePkg/Bus/Pci/PciHostBridgeDxe.
> +
> +  Plugging this library instance into PciHostBridgeDxe makes
> +  EFI_IOMMU_PROTOCOL depend on the platform's dynamic decision whether
> +  to provide IOMMU implementation (usually through IoMmuDxe driver).
> +
> +  Copyright (C) 2017, AMD, 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 <Base.h>
> +
> +RETURN_STATUS
> +EFIAPI
> +PlatformHasIoMmuInitialize (
> +  VOID
> +  )
> +{
> +  //
> +  // Do nothing, just imbue PciHostBridgeDxe with a protocol dependency on
> +  // gIoMmuAbsentProtocolGuid OR gEdkiiIoMmuProtocolGuid.
> +  //
> +  return RETURN_SUCCESS;
> +}
>