Revert "MdeModulePkg: Fix a PciBusDxe hot plug bug"

Leif suggested to split the big patch to smaller ones.

This reverts commit 73b7f115c653c807b9d0be97bf516871d8aff7ba.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Feng Tian <feng.tian@intel.com>

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18717 6f19259b-4bc3-4df7-8a09-765794883524

4 files changed, 102 insertions, 208 deletions

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index a6ade26..f7aea4f 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -325,81 +325,6 @@ PciSearchDevice (
 }

 

 /**

-  Dump the PPB padding resource information.

-

-  @param PciIoDevice     PCI IO instance.

-  @param ResourceType    The desired resource type to dump.

-                         PciBarTypeUnknown means to dump all types of resources.

-**/

-VOID

-DumpPpbPaddingResource (

-  IN PCI_IO_DEVICE                    *PciIoDevice,

-  IN PCI_BAR_TYPE                     ResourceType

-  )

-{

-  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;

-  PCI_BAR_TYPE                      Type;

-

-  if (PciIoDevice->ResourcePaddingDescriptors == NULL) {

-    return;

-  }

-

-  if (ResourceType == PciBarTypeIo16 || ResourceType == PciBarTypeIo32) {

-    ResourceType = PciBarTypeIo;

-  }

-

-  for (Descriptor = PciIoDevice->ResourcePaddingDescriptors; Descriptor->Desc != ACPI_END_TAG_DESCRIPTOR; Descriptor++) {

-

-    Type = PciBarTypeUnknown;

-    if (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR && Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_IO) {

-      Type = PciBarTypeIo;

-    } else if (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR && Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {

-

-      if (Descriptor->AddrSpaceGranularity == 32) {

-        //

-        // prefechable

-        //

-        if (Descriptor->SpecificFlag == EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE) {

-          Type = PciBarTypePMem32;

-        }

-

-        //

-        // Non-prefechable

-        //

-        if (Descriptor->SpecificFlag == 0) {

-          Type = PciBarTypeMem32;

-        }

-      }

-

-      if (Descriptor->AddrSpaceGranularity == 64) {

-        //

-        // prefechable

-        //

-        if (Descriptor->SpecificFlag == EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE) {

-          Type = PciBarTypePMem64;

-        }

-

-        //

-        // Non-prefechable

-        //

-        if (Descriptor->SpecificFlag == 0) {

-          Type = PciBarTypeMem64;

-        }

-      }

-    }

-

-    if ((Type != PciBarTypeUnknown) && ((ResourceType == PciBarTypeUnknown) || (ResourceType == Type))) {

-      DEBUG ((

-        EFI_D_INFO,

-        "   Padding: Type = %s; Alignment = 0x%lx;\tLength = 0x%lx\n",

-        mBarTypeStr[Type], Descriptor->AddrRangeMax, Descriptor->AddrLen

-        ));

-    }

-  }

-

-}

-

-/**

   Dump the PCI BAR information.

 

   @param PciIoDevice     PCI IO instance.

@@ -661,10 +586,7 @@ GatherPpbInfo (
 

   GetResourcePaddingPpb (PciIoDevice);

 

-  DEBUG_CODE (

-    DumpPpbPaddingResource (PciIoDevice, PciBarTypeUnknown);

-    DumpPciBars (PciIoDevice);

-  );

+  DEBUG_CODE (DumpPciBars (PciIoDevice););

 

   return PciIoDevice;

 }

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
index 4d7b3b7..a4489b8 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h
@@ -460,17 +460,4 @@ ResetAllPpbBusNumber (
   IN UINT8                              StartBusNumber

   );

 

-/**

-  Dump the PPB padding resource information.

-

-  @param PciIoDevice     PCI IO instance.

-  @param ResourceType    The desired resource type to dump.

-                         PciBarTypeUnknown means to dump all types of resources.

-**/

-VOID

-DumpPpbPaddingResource (

-  IN PCI_IO_DEVICE                    *PciIoDevice,

-  IN PCI_BAR_TYPE                     ResourceType

-  );

-

 #endif

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index f4b6ebf..3e275e3 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -188,21 +188,19 @@ DumpBridgeResource (
       BridgeResource->PciDev->PciBar[BridgeResource->Bar].BaseAddress,

       BridgeResource->Length, BridgeResource->Alignment

       ));

-    for ( Link = GetFirstNode (&BridgeResource->ChildList)

-        ; !IsNull (&BridgeResource->ChildList, Link)

-        ; Link = GetNextNode (&BridgeResource->ChildList, Link)

+    for ( Link = BridgeResource->ChildList.ForwardLink

+        ; Link != &BridgeResource->ChildList

+        ; Link = Link->ForwardLink

         ) {

       Resource = RESOURCE_NODE_FROM_LINK (Link);

       if (Resource->ResourceUsage == PciResUsageTypical) {

         Bar = Resource->Virtual ? Resource->PciDev->VfPciBar : Resource->PciDev->PciBar;

         DEBUG ((

-          EFI_D_INFO, "   Base = 0x%lx;\tLength = 0x%lx;\tAlignment = 0x%lx;\tOwner = %s [%02x|%02x|%02x:",

+          EFI_D_INFO, " Base = 0x%lx;\tLength = 0x%lx;\tAlignment = 0x%lx;\tOwner = %s ",

           Bar[Resource->Bar].BaseAddress, Resource->Length, Resource->Alignment,

           IS_PCI_BRIDGE (&Resource->PciDev->Pci)     ? L"PPB" :

           IS_CARDBUS_BRIDGE (&Resource->PciDev->Pci) ? L"P2C" :

-                                                       L"PCI",

-          Resource->PciDev->BusNumber, Resource->PciDev->DeviceNumber,

-          Resource->PciDev->FunctionNumber

+                                                       L"PCI"

           ));

 

         if ((!IS_PCI_BRIDGE (&Resource->PciDev->Pci) && !IS_CARDBUS_BRIDGE (&Resource->PciDev->Pci)) ||

@@ -212,20 +210,24 @@ DumpBridgeResource (
           //

           // The resource requirement comes from the device itself.

           //

-          DEBUG ((EFI_D_INFO, "%02x]", Bar[Resource->Bar].Offset));

+          DEBUG ((

+            EFI_D_INFO, " [%02x|%02x|%02x:%02x]\n",

+            Resource->PciDev->BusNumber, Resource->PciDev->DeviceNumber,

+            Resource->PciDev->FunctionNumber, Bar[Resource->Bar].Offset

+            ));

         } else {

           //

           // The resource requirement comes from the subordinate devices.

           //

-          DEBUG ((EFI_D_INFO, "**]"));

+          DEBUG ((

+            EFI_D_INFO, " [%02x|%02x|%02x:**]\n",

+            Resource->PciDev->BusNumber, Resource->PciDev->DeviceNumber,

+            Resource->PciDev->FunctionNumber

+            ));

         }

       } else {

-        DEBUG ((EFI_D_INFO, "   Base = Padding;\tLength = 0x%lx;\tAlignment = 0x%lx", Resource->Length, Resource->Alignment));

+        DEBUG ((EFI_D_INFO, " Padding:Length = 0x%lx;\tAlignment = 0x%lx\n", Resource->Length, Resource->Alignment));

       }

-      if (BridgeResource->ResType != Resource->ResType) {

-        DEBUG ((EFI_D_INFO, "; Type = %s", mBarTypeStr[MIN (Resource->ResType, PciBarTypeMaxType)]));

-      }

-      DEBUG ((EFI_D_INFO, "\n"));

     }

   }

 }

@@ -233,61 +235,63 @@ DumpBridgeResource (
 /**

   Find the corresponding resource node for the Device in child list of BridgeResource.

   

-  @param[in]  Device          Pointer to PCI_IO_DEVICE.

-  @param[in]  BridgeResource  Pointer to PCI_RESOURCE_NODE.

-  @param[out] DeviceResources Pointer to a buffer to receive resources for the Device.

+  @param[in] Device         Pointer to PCI_IO_DEVICE.

+  @param[in] BridgeResource Pointer to PCI_RESOURCE_NODE.

   

-  @return Count of the resource descriptors returned.

+  @return !NULL  The corresponding resource node for the Device.

+  @return NULL   No corresponding resource node for the Device.

 **/

-UINTN

+PCI_RESOURCE_NODE *

 FindResourceNode (

-  IN  PCI_IO_DEVICE     *Device,

-  IN  PCI_RESOURCE_NODE *BridgeResource,

-  OUT PCI_RESOURCE_NODE **DeviceResources OPTIONAL

+  IN PCI_IO_DEVICE     *Device,

+  IN PCI_RESOURCE_NODE *BridgeResource

   )

 {

   LIST_ENTRY               *Link;

   PCI_RESOURCE_NODE        *Resource;

-  UINTN                    Count;

 

-  Count = 0;

   for ( Link = BridgeResource->ChildList.ForwardLink

       ; Link != &BridgeResource->ChildList

       ; Link = Link->ForwardLink

       ) {

     Resource = RESOURCE_NODE_FROM_LINK (Link);

     if (Resource->PciDev == Device) {

-      if (DeviceResources != NULL) {

-        DeviceResources[Count] = Resource;

-      }

-      Count++;

+      return Resource;

     }

   }

 

-  return Count;

+  return NULL;

 }

 

 /**

   Dump the resource map of all the devices under Bridge.

   

-  @param[in] Bridge        Bridge device instance.

-  @param[in] Resources     Resource descriptors for the bridge device.

-  @param[in] ResourceCount Count of resource descriptors.

+  @param[in] Bridge     Bridge device instance.

+  @param[in] IoNode     IO resource descriptor for the bridge device.

+  @param[in] Mem32Node  Mem32 resource descriptor for the bridge device.

+  @param[in] PMem32Node PMem32 resource descriptor for the bridge device.

+  @param[in] Mem64Node  Mem64 resource descriptor for the bridge device.

+  @param[in] PMem64Node PMem64 resource descriptor for the bridge device.

 **/

 VOID

 DumpResourceMap (

   IN PCI_IO_DEVICE     *Bridge,

-  IN PCI_RESOURCE_NODE **Resources,

-  IN UINTN             ResourceCount

+  IN PCI_RESOURCE_NODE *IoNode,

+  IN PCI_RESOURCE_NODE *Mem32Node,

+  IN PCI_RESOURCE_NODE *PMem32Node,

+  IN PCI_RESOURCE_NODE *Mem64Node,

+  IN PCI_RESOURCE_NODE *PMem64Node

   )

 {

-  EFI_STATUS           Status;

-  LIST_ENTRY           *Link;

-  PCI_IO_DEVICE        *Device;

-  UINTN                Index;

-  CHAR16               *Str;

-  PCI_RESOURCE_NODE    **ChildResources;

-  UINTN                ChildResourceCount;

+  EFI_STATUS                       Status;

+  LIST_ENTRY                       *Link;

+  PCI_IO_DEVICE                    *Device;

+  PCI_RESOURCE_NODE                *ChildIoNode;

+  PCI_RESOURCE_NODE                *ChildMem32Node;

+  PCI_RESOURCE_NODE                *ChildPMem32Node;

+  PCI_RESOURCE_NODE                *ChildMem64Node;

+  PCI_RESOURCE_NODE                *ChildPMem64Node;

+  CHAR16                           *Str;

 

   DEBUG ((EFI_D_INFO, "PciBus: Resource Map for "));

 

@@ -316,9 +320,11 @@ DumpResourceMap (
     }

   }

 

-  for (Index = 0; Index < ResourceCount; Index++) {

-    DumpBridgeResource (Resources[Index]);

-  }

+  DumpBridgeResource (IoNode);

+  DumpBridgeResource (Mem32Node);

+  DumpBridgeResource (PMem32Node);

+  DumpBridgeResource (Mem64Node);

+  DumpBridgeResource (PMem64Node);

   DEBUG ((EFI_D_INFO, "\n"));

 

   for ( Link = Bridge->ChildList.ForwardLink

@@ -328,19 +334,20 @@ DumpResourceMap (
     Device = PCI_IO_DEVICE_FROM_LINK (Link);

     if (IS_PCI_BRIDGE (&Device->Pci)) {

 

-      ChildResourceCount = 0;

-      for (Index = 0; Index < ResourceCount; Index++) {

-        ChildResourceCount += FindResourceNode (Device, Resources[Index], NULL);

-      }

-      ChildResources = AllocatePool (sizeof (PCI_RESOURCE_NODE *) * ChildResourceCount);

-      ASSERT (ChildResources != NULL);

-      ChildResourceCount = 0;

-      for (Index = 0; Index < ResourceCount; Index++) {

-        ChildResourceCount += FindResourceNode (Device, Resources[Index], &ChildResources[ChildResourceCount]);

-      }

-

-      DumpResourceMap (Device, ChildResources, ChildResourceCount);

-      FreePool (ChildResources);

+      ChildIoNode     = (IoNode     == NULL ? NULL : FindResourceNode (Device, IoNode));

+      ChildMem32Node  = (Mem32Node  == NULL ? NULL : FindResourceNode (Device, Mem32Node));

+      ChildPMem32Node = (PMem32Node == NULL ? NULL : FindResourceNode (Device, PMem32Node));

+      ChildMem64Node  = (Mem64Node  == NULL ? NULL : FindResourceNode (Device, Mem64Node));

+      ChildPMem64Node = (PMem64Node == NULL ? NULL : FindResourceNode (Device, PMem64Node));

+

+      DumpResourceMap (

+        Device,

+        ChildIoNode,

+        ChildMem32Node,

+        ChildPMem32Node,

+        ChildMem64Node,

+        ChildPMem64Node

+        );

     }

   }

 }

@@ -800,11 +807,11 @@ PciHostBridgeResourceAllocator (
     // Create the entire system resource map from the information collected by

     // enumerator. Several resource tree was created

     //

-    FindResourceNode (RootBridgeDev, &IoPool, &IoBridge);

-    FindResourceNode (RootBridgeDev, &Mem32Pool, &Mem32Bridge);

-    FindResourceNode (RootBridgeDev, &PMem32Pool, &PMem32Bridge);

-    FindResourceNode (RootBridgeDev, &Mem64Pool, &Mem64Bridge);

-    FindResourceNode (RootBridgeDev, &PMem64Pool, &PMem64Bridge);

+    IoBridge     = FindResourceNode (RootBridgeDev, &IoPool);

+    Mem32Bridge  = FindResourceNode (RootBridgeDev, &Mem32Pool);

+    PMem32Bridge = FindResourceNode (RootBridgeDev, &PMem32Pool);

+    Mem64Bridge  = FindResourceNode (RootBridgeDev, &Mem64Pool);

+    PMem64Bridge = FindResourceNode (RootBridgeDev, &PMem64Pool);

 

     ASSERT (IoBridge     != NULL);

     ASSERT (Mem32Bridge  != NULL);

@@ -862,13 +869,14 @@ PciHostBridgeResourceAllocator (
     // Dump the resource map for current root bridge

     //

     DEBUG_CODE (

-      PCI_RESOURCE_NODE *Resources[5];

-      Resources[0] = IoBridge;

-      Resources[1] = Mem32Bridge;

-      Resources[2] = PMem32Bridge;

-      Resources[3] = Mem64Bridge;

-      Resources[4] = PMem64Bridge;

-      DumpResourceMap (RootBridgeDev, Resources, sizeof (Resources) / sizeof (Resources[0]));

+      DumpResourceMap (

+        RootBridgeDev,

+        IoBridge,

+        Mem32Bridge,

+        PMem32Bridge,

+        Mem64Bridge,

+        PMem64Bridge

+        );

     );

 

     FreePool (AcpiConfig);

@@ -976,8 +984,7 @@ PciScanBus (
   UINT8                             Device;

   UINT8                             Func;

   UINT64                            Address;

-  UINT8                             SecondBus;

-  UINT8                             PaddedSubBus;

+  UINTN                             SecondBus;

   UINT16                            Register;

   UINTN                             HpIndex;

   PCI_IO_DEVICE                     *PciDevice;

@@ -1211,7 +1218,7 @@ PciScanBus (
 

           Status = PciScanBus (

                     PciDevice,

-                    SecondBus,

+                    (UINT8) (SecondBus),

                     SubBusNumber,

                     PaddedBusRange

                     );

@@ -1227,16 +1234,12 @@ PciScanBus (
           if ((Attributes == EfiPaddingPciRootBridge) &&

               (State & EFI_HPC_STATE_ENABLED) != 0    &&

               (State & EFI_HPC_STATE_INITIALIZED) != 0) {

-            *PaddedBusRange = (UINT8) ((UINT8) (BusRange) + *PaddedBusRange);

+            *PaddedBusRange = (UINT8) ((UINT8) (BusRange) +*PaddedBusRange);

           } else {

-            //

-            // Reserve the larger one between the actual occupied bus number and padded bus number

-            //

-            Status = PciAllocateBusNumber (PciDevice, StartBusNumber, (UINT8) (BusRange), &PaddedSubBus);

+            Status = PciAllocateBusNumber (PciDevice, *SubBusNumber, (UINT8) (BusRange), SubBusNumber);

             if (EFI_ERROR (Status)) {

               return Status;

             }

-            *SubBusNumber = MAX (PaddedSubBus, *SubBusNumber);

           }

         }

 

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index b106abe..d8d988c 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -196,7 +196,6 @@ CalculateApertureIo16 (
   PCI_RESOURCE_NODE       *Node;

   UINT64                  Offset;

   EFI_PCI_PLATFORM_POLICY PciPolicy;

-  UINT64                  PaddingAperture;

 

   if (!mPolicyDetermined) {

     //

@@ -229,27 +228,21 @@ CalculateApertureIo16 (
     mPolicyDetermined = TRUE;

   }

 

-  Aperture        = 0;

-  PaddingAperture = 0;

+  Aperture = 0;

 

   if (Bridge == NULL) {

     return ;

   }

 

+  CurrentLink = Bridge->ChildList.ForwardLink;

+

   //

   // Assume the bridge is aligned

   //

-  for ( CurrentLink = GetFirstNode (&Bridge->ChildList)

-      ; !IsNull (&Bridge->ChildList, CurrentLink)

-      ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink)

-      ) {

+  while (CurrentLink != &Bridge->ChildList) {

 

     Node = RESOURCE_NODE_FROM_LINK (CurrentLink);

-    if (Node->ResourceUsage == PciResUsagePadding) {

-      ASSERT (PaddingAperture == 0);

-      PaddingAperture = Node->Length;

-      continue;

-    }

+

     //

     // Consider the aperture alignment

     //

@@ -300,10 +293,13 @@ CalculateApertureIo16 (
     // Increment aperture by the length of node

     //

     Aperture += Node->Length;

+

+    CurrentLink = CurrentLink->ForwardLink;

   }

 

   //

-  // Adjust the aperture with the bridge's alignment

+  // At last, adjust the aperture with the bridge's

+  // alignment

   //

   Offset = Aperture & (Bridge->Alignment);

 

@@ -323,12 +319,6 @@ CalculateApertureIo16 (
       Bridge->Alignment = Node->Alignment;

     }

   }

-

-  //

-  // Hotplug controller needs padding resources.

-  // Use the larger one between the padding resource and actual occupied resource.

-  //

-  Bridge->Length = MAX (Bridge->Length, PaddingAperture);

 }

 

 /**

@@ -346,11 +336,10 @@ CalculateResourceAperture (
   UINT64            Aperture;

   LIST_ENTRY        *CurrentLink;

   PCI_RESOURCE_NODE *Node;

-  UINT64            PaddingAperture;

+

   UINT64            Offset;

 

-  Aperture        = 0;

-  PaddingAperture = 0;

+  Aperture = 0;

 

   if (Bridge == NULL) {

     return ;

@@ -362,20 +351,14 @@ CalculateResourceAperture (
     return ;

   }

 

+  CurrentLink = Bridge->ChildList.ForwardLink;

+

   //

   // Assume the bridge is aligned

   //

-  for ( CurrentLink = GetFirstNode (&Bridge->ChildList)

-      ; !IsNull (&Bridge->ChildList, CurrentLink)

-      ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink)

-      ) {

+  while (CurrentLink != &Bridge->ChildList) {

 

     Node = RESOURCE_NODE_FROM_LINK (CurrentLink);

-    if (Node->ResourceUsage == PciResUsagePadding) {

-      ASSERT (PaddingAperture == 0);

-      PaddingAperture = Node->Length;

-      continue;

-    }

 

     //

     // Apply padding resource if available

@@ -398,6 +381,11 @@ CalculateResourceAperture (
     // Increment aperture by the length of node

     //

     Aperture += Node->Length;

+

+    //

+    // Consider the aperture alignment

+    //

+    CurrentLink = CurrentLink->ForwardLink;

   }

 

   //

@@ -419,7 +407,7 @@ CalculateResourceAperture (
   }

 

   //

-  // Adjust the bridge's alignment to the first child's alignment

+  // At last, adjust the bridge's alignment to the first child's alignment

   // if the bridge has at least one child

   //

   CurrentLink = Bridge->ChildList.ForwardLink;

@@ -429,12 +417,6 @@ CalculateResourceAperture (
       Bridge->Alignment = Node->Alignment;

     }

   }

-

-  //

-  // Hotplug controller needs padding resources.

-  // Use the larger one between the padding resource and actual occupied resource.

-  //

-  Bridge->Length = MAX (Bridge->Length, PaddingAperture);

 }

 

 /**