Re: [edk2-platform][PATCH v3] MinPlatformPkg: Add multiple segment support for PciHostBridgeLib

["Kubacki, Michael A" <michael.a.kubacki@intel.com>]

Reviewed-by: Michael Kubacki <michael.a.kubacki@intel.com>

> -----Original Message-----
> From: Chen, Marc W
> Sent: Friday, May 17, 2019 12:53 AM
> To: devel@edk2.groups.io
> Cc: Chen, Marc W <marc.w.chen@intel.com>; Kubacki, Michael A
> <michael.a.kubacki@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>
> Subject: [edk2-platform][PATCH v3] MinPlatformPkg: Add multiple segment
> support for PciHostBridgeLib
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1799
> 
> 1. Add PcdPciSegmentCount PCD in MinPlatformPkg.dec and set default to
> 1 2. Base on PciHostBridge related PCDs to Initialize RootBridges.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc Chen <marc.w.chen@intel.com>
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> ---
>  Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec   |  3 +-
>  .../PciHostBridgeLibSimple.c                       | 83 ++++++++++++++--------
>  .../PciHostBridgeLibSimple.inf                     |  4 +-
>  3 files changed, 59 insertions(+), 31 deletions(-)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index 3185776ac3..e1ae8004cb 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -6,7 +6,7 @@
>  # INF files to generate AutoGen.c and AutoGen.h files  # for the build
> infrastructure.
>  #
> -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  # This program and the accompanying materials are licensed and made
> available under  # the terms and conditions of the BSD License which
> accompanies this distribution.
> @@ -223,6 +223,7 @@
> gMinPlatformPkgTokenSpaceGuid.PcdPcIoApicEnable|0x0|UINT32|0x9000
> 0019
>    gMinPlatformPkgTokenSpaceGuid.PcdPciDmaAbove4G
> |FALSE|BOOLEAN|0x4001004B
>    gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
> |FALSE|BOOLEAN|0x4001004C
>    gMinPlatformPkgTokenSpaceGuid.PcdPciResourceAssigned
> |FALSE|BOOLEAN|0x4001004D
> +  gMinPlatformPkgTokenSpaceGuid.PcdPciSegmentCount             |0x1
> |UINT8|0x4001004E
> 
> 
> gMinPlatformPkgTokenSpaceGuid.PcdAcpiPm1AEventBlockAddress|0x180
> 0|UINT16|0x00010035
> 
> gMinPlatformPkgTokenSpaceGuid.PcdAcpiPm1BEventBlockAddress|0x0000
> |UINT16|0x00010036
> diff --git
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciH
> ostBridgeLibSimple.c
> b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciH
> ostBridgeLibSimple.c
> index 557ac2a5b3..25259e2f2d 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciH
> ostBridgeLibSimple.c
> +++
> b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/P
> +++ ciHostBridgeLibSimple.c
> @@ -1,7 +1,7 @@
>  /** @file
> -  SA PciHostBridge Library
> +  PciHostBridge Library
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
>  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 @@ -15,6 +15,7 @@ WITHOUT
> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
>  #include <IndustryStandard/Pci.h>
>  #include <Protocol/PciHostBridgeResourceAllocation.h>
>  #include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/DevicePathLib.h>
>  #include <Library/MemoryAllocationLib.h>  #include <Library/PcdLib.h>
> @@ -28,7 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>  GLOBAL_REMOVE_IF_UNREFERENCED CHAR16
> *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
>    L"Mem", L"I/O", L"Bus"
>  };
> -ACPI_HID_DEVICE_PATH mRootBridgeDeviceNode = {
> +ACPI_HID_DEVICE_PATH mRootBridgeDeviceNodeTemplate = {
>    {
>      ACPI_DEVICE_PATH,
>      ACPI_DP,
> @@ -41,7 +42,7 @@ ACPI_HID_DEVICE_PATH mRootBridgeDeviceNode = {
>    0
>  };
> 
> -PCI_ROOT_BRIDGE mRootBridge = {
> +PCI_ROOT_BRIDGE mRootBridgeTemplate = {
>    0,
>    EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO |
>    EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO |
> @@ -66,41 +67,67 @@ PCI_ROOT_BRIDGE mRootBridge = {
>    NULL // DevicePath;
>  };
> 
> +/**
> +  Return all the root bridge instances.
> +
> +  @param Count  Return the count of root bridge instances.
> +
> +  @return All the root bridge instances, it will be NULL if system has
> insufficient memory
> +          resources available and count will be zero.
> +**/
> +
>  PCI_ROOT_BRIDGE *
>  EFIAPI
>  PciHostBridgeGetRootBridges (
>    UINTN                                 *Count
>    )
>  {
> -  mRootBridge.Mem.Base = PcdGet32 (PcdPciReservedMemBase);
> +  UINT8 Index;
> +  PCI_ROOT_BRIDGE *RootBridge;
> +
> +  RootBridge = AllocateZeroPool (sizeof (PCI_ROOT_BRIDGE) * PcdGet8
> + (PcdPciSegmentCount));  if (RootBridge == NULL) {
> +    DEBUG ((DEBUG_ERROR, "PciHostBridge: Out of resource\n"));
> +    *Count = 0;
> +    return RootBridge;
> +  }
> +
> +  mRootBridgeTemplate.Mem.Base = PcdGet32
> (PcdPciReservedMemBase);
>    if (PcdGet32(PcdPciReservedMemLimit) != 0) {
> -    mRootBridge.Mem.Limit = PcdGet32 (PcdPciReservedMemLimit);
> +    mRootBridgeTemplate.Mem.Limit = PcdGet32
> (PcdPciReservedMemLimit);
>    } else {
> -    mRootBridge.Mem.Limit = (UINT32)PcdGet64
> (PcdPciExpressBaseAddress);
> +    mRootBridgeTemplate.Mem.Limit = (UINT32) PcdGet64
> + (PcdPciExpressBaseAddress);
>    }
> 
> -  mRootBridge.MemAbove4G.Base = PcdGet64
> (PcdPciReservedMemAbove4GBBase);
> -  mRootBridge.MemAbove4G.Limit = PcdGet64
> (PcdPciReservedMemAbove4GBLimit);
> +  mRootBridgeTemplate.MemAbove4G.Base = PcdGet64
> + (PcdPciReservedMemAbove4GBBase);
> mRootBridgeTemplate.MemAbove4G.Limit
> + = PcdGet64 (PcdPciReservedMemAbove4GBLimit);
> 
> -  mRootBridge.PMem.Base = PcdGet32 (PcdPciReservedPMemBase);
> -  mRootBridge.PMem.Limit = PcdGet32 (PcdPciReservedPMemLimit);
> -  mRootBridge.PMemAbove4G.Base = PcdGet64
> (PcdPciReservedPMemAbove4GBBase);
> -  mRootBridge.PMemAbove4G.Limit = PcdGet64
> (PcdPciReservedPMemAbove4GBLimit);
> +  mRootBridgeTemplate.PMem.Base = PcdGet32
> (PcdPciReservedPMemBase);
> + mRootBridgeTemplate.PMem.Limit = PcdGet32
> (PcdPciReservedPMemLimit);
> + mRootBridgeTemplate.PMemAbove4G.Base = PcdGet64
> + (PcdPciReservedPMemAbove4GBBase);
> + mRootBridgeTemplate.PMemAbove4G.Limit = PcdGet64
> + (PcdPciReservedPMemAbove4GBLimit);
> 
> -  if (mRootBridge.MemAbove4G.Base < mRootBridge.MemAbove4G.Limit) {
> -    mRootBridge.AllocationAttributes |=
> EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
> +  if (mRootBridgeTemplate.MemAbove4G.Base <
> mRootBridgeTemplate.MemAbove4G.Limit) {
> +    mRootBridgeTemplate.AllocationAttributes |=
> + EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
>    }
> 
> -  mRootBridge.Io.Base = PcdGet16 (PcdPciReservedIobase);
> -  mRootBridge.Io.Limit = PcdGet16 (PcdPciReservedIoLimit);
> +  mRootBridgeTemplate.Io.Base = PcdGet16 (PcdPciReservedIobase);
> + mRootBridgeTemplate.Io.Limit = PcdGet16 (PcdPciReservedIoLimit);
> 
> -  mRootBridge.DmaAbove4G = PcdGetBool (PcdPciDmaAbove4G);
> -  mRootBridge.NoExtendedConfigSpace = PcdGetBool
> (PcdPciNoExtendedConfigSpace);
> -  mRootBridge.ResourceAssigned = PcdGetBool (PcdPciResourceAssigned);
> +  mRootBridgeTemplate.DmaAbove4G = PcdGetBool
> (PcdPciDmaAbove4G);
> + mRootBridgeTemplate.NoExtendedConfigSpace = PcdGetBool
> + (PcdPciNoExtendedConfigSpace);
> mRootBridgeTemplate.ResourceAssigned =
> + PcdGetBool (PcdPciResourceAssigned);
> +
> +  for (Index = 0; Index < PcdGet8 (PcdPciSegmentCount); Index ++) {
> +    mRootBridgeDeviceNodeTemplate.UID = Index;
> +    mRootBridgeTemplate.Segment = Index;
> +    mRootBridgeTemplate.DevicePath = NULL;
> +    mRootBridgeTemplate.DevicePath = AppendDevicePathNode (NULL,
> &mRootBridgeDeviceNodeTemplate.Header);
> +    CopyMem (RootBridge + Index, &mRootBridgeTemplate, sizeof
> + (PCI_ROOT_BRIDGE));  }
> 
> -  mRootBridge.DevicePath = AppendDevicePathNode (NULL,
> &mRootBridgeDeviceNode.Header);
> -  *Count = 1;
> -  return &mRootBridge;
> +  *Count = PcdGet8 (PcdPciSegmentCount);  return RootBridge;
>  }
> 
>  VOID
> @@ -110,7 +137,7 @@ PciHostBridgeFreeRootBridges (
>    UINTN           Count
>    )
>  {
> -  ASSERT (Count == 1);
> +  ASSERT (Count <= PcdGet8 (PcdPciSegmentCount));
>    FreePool (Bridges->DevicePath);
>  }
> 
> @@ -136,20 +163,20 @@ PciHostBridgeResourceConflict (
>  {
>    EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
>    UINTN                             RootBridgeIndex;
> -  DEBUG ((EFI_D_ERROR, "PciHostBridge: Resource conflict happens!\n"));
> +  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 ((EFI_D_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
> +    DEBUG ((DEBUG_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
>      for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR;
> Descriptor++) {
>        ASSERT (Descriptor->ResType <
>                sizeof (mPciHostBridgeLibAcpiAddressSpaceTypeStr) / sizeof
> (mPciHostBridgeLibAcpiAddressSpaceTypeStr[0])
>                );
> -      DEBUG ((EFI_D_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
> +      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 ((EFI_D_ERROR, "     Granularity/SpecificFlag = %ld /
> %02x%s\n",
> +        DEBUG ((DEBUG_ERROR, "     Granularity/SpecificFlag = %ld /
> %02x%s\n",
>                  Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
>                  ((Descriptor->SpecificFlag &
> EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABL
> E) != 0) ? L" (Prefetchable)" : L""
>                  ));
> diff --git
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciH
> ostBridgeLibSimple.inf
> b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciH
> ostBridgeLibSimple.inf
> index f9a769155b..b37488e512 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciH
> ostBridgeLibSimple.inf
> +++
> b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciH
> ostBridgeLibSimple.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Component description file for the SA PciHostBridge library
>  #
> -# Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
>  #
>  # This program and the accompanying materials are licensed and made
> available under
>  # the terms and conditions of the BSD License which accompanies this
> distribution.
> @@ -56,4 +56,4 @@
>    gMinPlatformPkgTokenSpaceGuid.PcdPciDmaAbove4G
>    gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
>    gMinPlatformPkgTokenSpaceGuid.PcdPciResourceAssigned
> -
> +  gMinPlatformPkgTokenSpaceGuid.PcdPciSegmentCount
> --
> 2.16.2.windows.1