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

["Leif Lindholm" <leif.lindholm@linaro.org>]

Hi Liming,

Actually, we have tended to use
--subject-prefix="PATCH edk2-platforms".

This is settable per-repository using
git config format.subjectPrefix "PATCH edk2-platforms".

Which of course can be overridden with --subject-prefix="RFC
edk2-platforms" when needed.

I should add something about that to Laszlo's guide.

Best Regards,

Leif

On Tue, May 14, 2019 at 04:59:44AM +0000, Liming Gao wrote:
> This patch is for edk2-platform. Please add edk2-platform in the patch title, like [edk2-platforms][Patch]. 
> 
> When you generate the patch, you can use the command  git format-patch -1 --subject-prefix="edk2-platform"
> 
> Thanks
> Liming
> >-----Original Message-----
> >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >Marc W Chen
> >Sent: Tuesday, May 14, 2019 11:17 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-devel] [PATCH] MinPlatformPkg: Add multiple segment
> >support for PciHostBridgeLib
> >
> >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.
> >
> >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|0x900000
> >19
> >   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/PciHost
> >BridgeLibSimple.c
> >b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHost
> >BridgeLibSimple.c
> >index 557ac2a5b3..25259e2f2d 100644
> >---
> >a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHost
> >BridgeLibSimple.c
> >+++
> >b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHost
> >BridgeLibSimple.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_PREFETCHABLE
> >) != 0) ? L" (Prefetchable)" : L""
> >                 ));
> >diff --git
> >a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHost
> >BridgeLibSimple.inf
> >b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHost
> >BridgeLibSimple.inf
> >index f9a769155b..b37488e512 100644
> >---
> >a/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHost
> >BridgeLibSimple.inf
> >+++
> >b/Platform/Intel/MinPlatformPkg/Pci/Library/PciHostBridgeLibSimple/PciHost
> >BridgeLibSimple.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
> >
> >
> >
> 
> 
> 
>