[PATCH] MdeModulePkg/PciBus: Count multiple hotplug resource paddings

* [PATCH] MdeModulePkg/PciBus: Count multiple hotplug resource paddings
@ 2017-09-30  5:10 Ruiyu Ni
  2017-09-30  5:12 ` Ni, Ruiyu
  2017-10-09  2:24 ` Zeng, Star
  0 siblings, 2 replies; 4+ messages in thread
From: Ruiyu Ni @ 2017-09-30  5:10 UTC (permalink / raw)
  To: edk2-devel; +Cc: Laszlo Ersek

The current implementation assumes there is only one hotplug resource
padding for each resource type. It's not true considering
DegradeResource(): MEM64 resource could be degraded to MEM32
resource.

The patch treat the resource paddings using the same logic as
treating typical/actual resources and the total resource of a bridge
is set to the MAX of typical/actual resources and resource paddings.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c         | 67 +++++++---------------
 1 file changed, 21 insertions(+), 46 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index e93134613b..f086b1732d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -343,14 +343,9 @@ CalculateResourceAperture (
   IN PCI_RESOURCE_NODE    *Bridge
   )
 {
-  UINT64            Aperture;
+  UINT64            Aperture[2];
   LIST_ENTRY        *CurrentLink;
   PCI_RESOURCE_NODE *Node;
-  UINT64            PaddingAperture;
-  UINT64            Offset;
-
-  Aperture        = 0;
-  PaddingAperture = 0;
 
   if (Bridge == NULL) {
     return ;
@@ -362,6 +357,8 @@ CalculateResourceAperture (
     return ;
   }
 
+  Aperture[PciResUsageTypical] = 0;
+  Aperture[PciResUsagePadding] = 0;
   //
   // Assume the bridge is aligned
   //
@@ -369,58 +366,30 @@ CalculateResourceAperture (
       ; !IsNull (&Bridge->ChildList, CurrentLink)
       ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink)
       ) {
-
     Node = RESOURCE_NODE_FROM_LINK (CurrentLink);
-    if (Node->ResourceUsage == PciResUsagePadding) {
-      ASSERT (PaddingAperture == 0);
-      PaddingAperture = Node->Length;
-      continue;
-    }
 
     //
-    // Apply padding resource if available
+    // It's possible for a bridge to contain multiple padding resource
+    // nodes due to DegradeResource().
     //
-    Offset = Aperture & (Node->Alignment);
-
-    if (Offset != 0) {
-
-      Aperture = Aperture + (Node->Alignment + 1) - Offset;
-
-    }
-
+    ASSERT ((Node->ResourceUsage == PciResUsageTypical) ||
+            (Node->ResourceUsage == PciResUsagePadding));
+    ASSERT (Node->ResourceUsage < ARRAY_SIZE (Aperture));
     //
     // Recode current aperture as a offset
-    // this offset will be used in future real allocation
+    // Apply padding resource to meet alignment requirement
+    // Node offset will be used in future real allocation
     //
-    Node->Offset = Aperture;
+    Node->Offset = ALIGN_VALUE (Aperture[Node->ResourceUsage], Node->Alignment + 1);
 
     //
-    // Increment aperture by the length of node
+    // Record the total aperture.
     //
-    Aperture += Node->Length;
-  }
-
-  //
-  // At last, adjust the aperture with the bridge's
-  // alignment
-  //
-  Offset = Aperture & (Bridge->Alignment);
-  if (Offset != 0) {
-    Aperture = Aperture + (Bridge->Alignment + 1) - Offset;
+    Aperture[Node->ResourceUsage] = Node->Offset + Node->Length;
   }
 
   //
-  // If the bridge has already padded the resource and the
-  // amount of padded resource is larger, then keep the
-  // padded resource
-  //
-  if (Bridge->Length < Aperture) {
-    Bridge->Length = Aperture;
-  }
-
-  //
-  // Adjust the bridge's alignment to the first child's alignment
-  // if the bridge has at least one child
+  // Adjust the bridge's alignment to the MAX (first) alignment of all children.
   //
   CurrentLink = Bridge->ChildList.ForwardLink;
   if (CurrentLink != &Bridge->ChildList) {
@@ -431,10 +400,16 @@ CalculateResourceAperture (
   }
 
   //
+  // At last, adjust the aperture with the bridge's alignment
+  //
+  Aperture[PciResUsageTypical] = ALIGN_VALUE (Aperture[PciResUsageTypical], Bridge->Alignment + 1);
+  Aperture[PciResUsagePadding] = ALIGN_VALUE (Aperture[PciResUsagePadding], Bridge->Alignment + 1);
+
+  //
   // Hotplug controller needs padding resources.
   // Use the larger one between the padding resource and actual occupied resource.
   //
-  Bridge->Length = MAX (Bridge->Length, PaddingAperture);
+  Bridge->Length = MAX (Aperture[PciResUsageTypical], Aperture[PciResUsagePadding]);
 }
 
 /**
-- 
2.12.2.windows.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread