On 09/30/17 07:12, Ni, Ruiyu wrote: > Laszlo, > Please check whether this patch can resolve the hang issue you reported. > I don't use your suggestion to use the MAX resource paddings. > Instead, I count all the resource paddings. > > If it can work, I will submit another patch to clean up the code in > CalculateApertureIo16(). > That function could be similar to the CalculateAperture().
Thank you very much for the quick patch, it works! Tested-by: Laszlo Ersek <ler...@redhat.com> Can you add a reference to <https://bugzilla.tianocore.org/show_bug.cgi?id=720> to the commit message? Thanks! Laszlo >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu >> Ni >> Sent: Saturday, September 30, 2017 1:11 PM >> To: edk2-devel@lists.01.org >> Cc: Laszlo Ersek <ler...@redhat.com> >> Subject: [edk2] [PATCH] MdeModulePkg/PciBus: Count multiple hotplug >> resource paddings >> >> 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...@intel.com> >> Cc: Laszlo Ersek <ler...@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 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
