From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4359D2007D1EC for ; Sat, 30 Sep 2017 15:31:07 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 546A6356C0; Sat, 30 Sep 2017 22:34:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 546A6356C0 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-198.rdu2.redhat.com [10.10.120.198]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0B97C424C; Sat, 30 Sep 2017 22:34:22 +0000 (UTC) To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" References: <20170930051030.200300-1-ruiyu.ni@intel.com> <734D49CCEBEEF84792F5B80ED585239D5BA83A43@SHSMSX104.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: Date: Sun, 1 Oct 2017 00:34:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5BA83A43@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Sat, 30 Sep 2017 22:34:24 +0000 (UTC) Subject: Re: [PATCH] MdeModulePkg/PciBus: Count multiple hotplug resource paddings X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Sep 2017 22:31:08 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 Can you add a reference to to the commit message? Thanks! Laszlo >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@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 >> 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 >> Cc: Laszlo Ersek >> --- >> .../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 >