Re: [PATCH] MinPlatformPkg: Add multiple segment support for PciHostBridgeLib

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

Please update the copyright years. The other changes look fine.

Thanks,
Michael

> -----Original Message-----
> From: Chen, Marc W
> Sent: Monday, May 13, 2019 2:47 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: [PATCH] MinPlatformPkg: Add multiple segment support for
> PciHostBridgeLib
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1799
> 
> Add PcdPciSegmentCount PCD in MinPlatformPkg.dec and set default to 1,
> then 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   |  1 +
>  .../PciHostBridgeLibSimple.c                       | 71 +++++++++++++++-------
>  .../PciHostBridgeLibSimple.inf                     |  2 +-
>  3 files changed, 51 insertions(+), 23 deletions(-)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index 3185776ac3..09701bd004 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -223,6 +223,7 @@
> gMinPlatformPkgTokenSpaceGuid.PcdPcIoApicEnable|0x0|UINT32|0x90000
> 019
>    gMinPlatformPkgTokenSpaceGuid.PcdPciDmaAbove4G
> |FALSE|BOOLEAN|0x4001004B
>    gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
> |FALSE|BOOLEAN|0x4001004C
>    gMinPlatformPkgTokenSpaceGuid.PcdPciResourceAssigned
> |FALSE|BOOLEAN|0x4001004D
> +  gMinPlatformPkgTokenSpaceGuid.PcdPciSegmentCount             |0x1
> |UINT8|0x4001004E
> 
> 
> gMinPlatformPkgTokenSpaceGuid.PcdAcpiPm1AEventBlockAddress|0x1800|
> UINT16|0x00010035
> 
> gMinPlatformPkgTokenSpaceGuid.PcdAcpiPm1BEventBlockAddress|0x0000|
> UINT16|0x00010036
> diff --git
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHo
> stBridgeLibSimple.c
> b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHo
> stBridgeLibSimple.c
> index 557ac2a5b3..aafadc598b 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHo
> stBridgeLibSimple.c
> +++ b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/P
> +++ ciHostBridgeLibSimple.c
> @@ -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 ((EFI_D_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);
>  }
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHo
> stBridgeLibSimple.inf
> b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHo
> stBridgeLibSimple.inf
> index f9a769155b..e5c0ca2774 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHo
> stBridgeLibSimple.inf
> +++
> b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHo
> stBridgeLibSimple.inf
> @@ -56,4 +56,4 @@
>    gMinPlatformPkgTokenSpaceGuid.PcdPciDmaAbove4G
>    gMinPlatformPkgTokenSpaceGuid.PcdPciNoExtendedConfigSpace
>    gMinPlatformPkgTokenSpaceGuid.PcdPciResourceAssigned
> -
> +  gMinPlatformPkgTokenSpaceGuid.PcdPciSegmentCount
> --
> 2.16.2.windows.1