[edk2-devel] [PATCH v3 1/5] OvmfPkg: Introduce PciHostBridgeUtilityLib class

Jiahui Cen via groups.io cenjiahui=huawei.com at groups.io
Thu Jan 7 05:44:16 UTC 2021


Hi Laszlo,

On 2021/1/6 16:51, Laszlo Ersek wrote:
> On 12/22/20 10:59, Jiahui Cen via groups.io wrote:
>> Introduce a new PciHostBridgeUtilityLib class to share duplicate code
>> between OvmfPkg and ArmVirtPkg.
>>
>> Extract function PciHostBridgeUtilityResourceConflict from
>> PciHostBridgeResourceConflict in OvmfPkg/PciHostBridgeLib.
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
>>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>> Cc: Laszlo Ersek <lersek at redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
>> Cc: Rebecca Cran <rebecca at bsdio.com>
>> Cc: Peter Grehan <grehan at freebsd.org>
>> Cc: Anthony Perard <anthony.perard at citrix.com>
>> Cc: Julien Grall <julien at xen.org>
>> Signed-off-by: Jiahui Cen <cenjiahui at huawei.com>
>> Signed-off-by: Yubo Miao <miaoyubo at huawei.com>
>> ---
>>  OvmfPkg/OvmfPkg.dec                                                 |  4 +
>>  OvmfPkg/Bhyve/BhyveX64.dsc                                          |  1 +
>>  OvmfPkg/OvmfPkgIa32.dsc                                             |  1 +
>>  OvmfPkg/OvmfPkgIa32X64.dsc                                          |  1 +
>>  OvmfPkg/OvmfPkgX64.dsc                                              |  1 +
>>  OvmfPkg/OvmfXen.dsc                                                 |  1 +
>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf               |  1 +
>>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf | 37 ++++++++++
>>  OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h                   | 37 ++++++++++
>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c                 | 41 +----------
>>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c   | 77 ++++++++++++++++++++
>>  11 files changed, 163 insertions(+), 39 deletions(-)
> 
> (1) Since you posted v2, OvmfPkg gained a new platform DSC file. The
> full set of DSC files in OvmfPkg is, at this point:
> 
> OvmfPkg/AmdSev/AmdSevX64.dsc
> OvmfPkg/Bhyve/BhyveX64.dsc
> OvmfPkg/OvmfPkgIa32.dsc
> OvmfPkg/OvmfPkgIa32X64.dsc
> OvmfPkg/OvmfPkgX64.dsc
> OvmfPkg/OvmfXen.dsc
> 
> "OvmfPkg/AmdSev/AmdSevX64.dsc" consumes
> "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf" as well; please
> add the new PciHostBridgeUtilityLib resolution to that DSC file too, in v4.
> 
> (You should check for any and all DSC files with such a dependency,
> every time you rebase.)

Thanks for your review. I'll fix it in v4, and will be careful about
the dependencies in the future.

Thanks,
Jiahui

> 
> With that update, I'm ready to grant an R-b for this patch.
> 
> Thanks!
> Laszlo
> 
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 50d7b27d941c..e39097a253a1 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -49,6 +49,10 @@ [LibraryClasses]
>>    #                  access.
>>    PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h
>>  
>> +  ##  @libraryclass  Provide common utility functions to PciHostBridgeLib
>> +  #                  instances in ArmVirtPkg and OvmfPkg.
>> +  PciHostBridgeUtilityLib|Include/Library/PciHostBridgeUtilityLib.h
>> +
>>    ##  @libraryclass  Register a status code handler for printing the Boot
>>    #                  Manager's LoadImage() and StartImage() preparations, and
>>    #                  return codes, to the UEFI console.
>> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
>> index b93fe30ae4e0..f1fdd85d1911 100644
>> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
>> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
>> @@ -660,6 +660,7 @@ [Components]
>>    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>>      <LibraryClasses>
>>        PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +      PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>        NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
>>    }
>>    MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 26a013ec353e..6eef5e0cfa9c 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -745,6 +745,7 @@ [Components]
>>    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>>      <LibraryClasses>
>>        PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +      PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>        NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
>>    }
>>    MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 10579fe46c5b..4b2f48406543 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -759,6 +759,7 @@ [Components.X64]
>>    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>>      <LibraryClasses>
>>        PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +      PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>        NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
>>    }
>>    MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index c9235e48ad62..8577ccaa35af 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -755,6 +755,7 @@ [Components]
>>    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>>      <LibraryClasses>
>>        PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +      PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>        NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
>>    }
>>    MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
>> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>> index 12b7a87ee877..fa35d122cf3e 100644
>> --- a/OvmfPkg/OvmfXen.dsc
>> +++ b/OvmfPkg/OvmfXen.dsc
>> @@ -550,6 +550,7 @@ [Components]
>>    MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
>>      <LibraryClasses>
>>        PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +      PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>        NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
>>    }
>>    MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
>> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> index 6ec9ec751478..4c56f3c90b3b 100644
>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> @@ -39,6 +39,7 @@ [LibraryClasses]
>>    DebugLib
>>    DevicePathLib
>>    MemoryAllocationLib
>> +  PciHostBridgeUtilityLib
>>    PciLib
>>    QemuFwCfgLib
>>  
>> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>> new file mode 100644
>> index 000000000000..1ba8ec3e03c7
>> --- /dev/null
>> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>> @@ -0,0 +1,37 @@
>> +## @file
>> +#  Provide common utility functions to PciHostBridgeLib instances in
>> +#  ArmVirtPkg and OvmfPkg.
>> +#
>> +#  Copyright (C) 2016, Red Hat, Inc.
>> +#  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>> +#  Copyright (c) 2020, Huawei Corporation. All rights reserved.<BR>
>> +#
>> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +#
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 1.29
>> +  BASE_NAME                      = PciHostBridgeUtilityLib
>> +  FILE_GUID                      = e3aa5932-527a-42e7-86f5-81b144c7e5f1
>> +  MODULE_TYPE                    = DXE_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = PciHostBridgeUtilityLib
>> +
>> +#
>> +# The following information is for reference only and not required by the build
>> +# tools.
>> +#
>> +#  VALID_ARCHITECTURES           = IA32 X64 AARCH64 ARM
>> +#
>> +
>> +[Sources]
>> +  PciHostBridgeUtilityLib.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +  OvmfPkg/OvmfPkg.dec
>> +
>> +[LibraryClasses]
>> +  DebugLib
>> diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
>> new file mode 100644
>> index 000000000000..f932d412aa10
>> --- /dev/null
>> +++ b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
>> @@ -0,0 +1,37 @@
>> +/** @file
>> +  Provide common utility functions to PciHostBridgeLib instances in
>> +  ArmVirtPkg and OvmfPkg.
>> +
>> +  Copyright (C) 2016, Red Hat, Inc.
>> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>> +  Copyright (c) 2020, Huawei Corporation. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#ifndef __PCI_HOST_BRIDGE_UTILITY_LIB_H__
>> +#define __PCI_HOST_BRIDGE_UTILITY_LIB_H__
>> +
>> +
>> +/**
>> +  Utility function to inform the platform that the resource conflict happens.
>> +
>> +  @param Configuration    Pointer to PCI I/O and PCI memory resource
>> +                          descriptors. The Configuration contains the resources
>> +                          for all the root bridges. The resource for each root
>> +                          bridge is terminated with END descriptor and an
>> +                          additional END is appended indicating the end of the
>> +                          entire resources. The resource descriptor field
>> +                          values follow the description in
>> +                          EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
>> +                          .SubmitResources().
>> +**/
>> +VOID
>> +EFIAPI
>> +PciHostBridgeUtilityResourceConflict (
>> +  VOID                              *Configuration
>> +  );
>> +
>> +
>> +#endif // __PCI_HOST_BRIDGE_UTILITY_LIB_H__
>> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> index e850f7d183ee..4a176347fd49 100644
>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> @@ -20,6 +20,7 @@
>>  #include <Library/DevicePathLib.h>
>>  #include <Library/MemoryAllocationLib.h>
>>  #include <Library/PciHostBridgeLib.h>
>> +#include <Library/PciHostBridgeUtilityLib.h>
>>  #include <Library/PciLib.h>
>>  #include <Library/QemuFwCfgLib.h>
>>  #include "PciHostBridge.h"
>> @@ -33,12 +34,6 @@ typedef struct {
>>  #pragma pack ()
>>  
>>  
>> -GLOBAL_REMOVE_IF_UNREFERENCED
>> -CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
>> -  L"Mem", L"I/O", L"Bus"
>> -};
>> -
>> -
>>  STATIC
>>  CONST
>>  OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH mRootBridgeDevicePathTemplate = {
>> @@ -407,37 +402,5 @@ PciHostBridgeResourceConflict (
>>    VOID                              *Configuration
>>    )
>>  {
>> -  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
>> -  UINTN                             RootBridgeIndex;
>> -  DEBUG ((DEBUG_ERROR, "PciHostBridge: Resource conflict happens!\n"));
>> -
>> -  RootBridgeIndex = 0;
>> -  Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration;
>> -  while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
>> -    DEBUG ((DEBUG_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
>> -    for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) {
>> -      ASSERT (Descriptor->ResType <
>> -              ARRAY_SIZE (mPciHostBridgeLibAcpiAddressSpaceTypeStr)
>> -              );
>> -      DEBUG ((DEBUG_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
>> -              mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType],
>> -              Descriptor->AddrLen, Descriptor->AddrRangeMax
>> -              ));
>> -      if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
>> -        DEBUG ((DEBUG_ERROR, "     Granularity/SpecificFlag = %ld / %02x%s\n",
>> -                Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
>> -                ((Descriptor->SpecificFlag &
>> -                  EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE
>> -                  ) != 0) ? L" (Prefetchable)" : L""
>> -                ));
>> -      }
>> -    }
>> -    //
>> -    // Skip the END descriptor for root bridge
>> -    //
>> -    ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR);
>> -    Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)(
>> -                   (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1
>> -                   );
>> -  }
>> +  PciHostBridgeUtilityResourceConflict (Configuration);
>>  }
>> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
>> new file mode 100644
>> index 000000000000..bbaa5f830c98
>> --- /dev/null
>> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
>> @@ -0,0 +1,77 @@
>> +/** @file
>> +  Provide common utility functions to PciHostBridgeLib instances in
>> +  ArmVirtPkg and OvmfPkg.
>> +
>> +  Copyright (C) 2016, Red Hat, Inc.
>> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>> +  Copyright (c) 2020, Huawei Corporation. All rights reserved.<BR>
>> +
>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include <IndustryStandard/Acpi10.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/PciHostBridgeUtilityLib.h>
>> +
>> +
>> +GLOBAL_REMOVE_IF_UNREFERENCED
>> +CHAR16 *mPciHostBridgeUtilityLibAcpiAddressSpaceTypeStr[] = {
>> +  L"Mem", L"I/O", L"Bus"
>> +};
>> +
>> +
>> +/**
>> +  Utility function to inform the platform that the resource conflict happens.
>> +
>> +  @param Configuration    Pointer to PCI I/O and PCI memory resource
>> +                          descriptors. The Configuration contains the resources
>> +                          for all the root bridges. The resource for each root
>> +                          bridge is terminated with END descriptor and an
>> +                          additional END is appended indicating the end of the
>> +                          entire resources. The resource descriptor field
>> +                          values follow the description in
>> +                          EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
>> +                          .SubmitResources().
>> +**/
>> +VOID
>> +EFIAPI
>> +PciHostBridgeUtilityResourceConflict (
>> +  VOID                              *Configuration
>> +  )
>> +{
>> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
>> +  UINTN                             RootBridgeIndex;
>> +  DEBUG ((DEBUG_ERROR, "PciHostBridge: Resource conflict happens!\n"));
>> +
>> +  RootBridgeIndex = 0;
>> +  Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration;
>> +  while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
>> +    DEBUG ((DEBUG_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
>> +    for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) {
>> +      ASSERT (Descriptor->ResType <
>> +              ARRAY_SIZE (mPciHostBridgeUtilityLibAcpiAddressSpaceTypeStr)
>> +              );
>> +      DEBUG ((DEBUG_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
>> +              mPciHostBridgeUtilityLibAcpiAddressSpaceTypeStr[Descriptor->ResType],
>> +              Descriptor->AddrLen, Descriptor->AddrRangeMax
>> +              ));
>> +      if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
>> +        DEBUG ((DEBUG_ERROR, "     Granularity/SpecificFlag = %ld / %02x%s\n",
>> +                Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
>> +                ((Descriptor->SpecificFlag &
>> +                  EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE
>> +                  ) != 0) ? L" (Prefetchable)" : L""
>> +                ));
>> +      }
>> +    }
>> +    //
>> +    // Skip the END descriptor for root bridge
>> +    //
>> +    ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR);
>> +    Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)(
>> +                   (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1
>> +                   );
>> +  }
>> +}
>> +
>>
> 
> .
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69894): https://edk2.groups.io/g/devel/message/69894
Mute This Topic: https://groups.io/mt/79146010/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list