From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (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 458A52063E318 for ; Thu, 3 May 2018 06:51:42 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3E101406C748; Thu, 3 May 2018 13:51:41 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-12.rdu2.redhat.com [10.10.120.12]) by smtp.corp.redhat.com (Postfix) with ESMTP id 604762024CA1; Thu, 3 May 2018 13:51:40 +0000 (UTC) To: Roman Bacik Cc: edk2-devel@lists.01.org, Ruiyu Ni , Vladimir Olovyannikov References: From: Laszlo Ersek Message-ID: Date: Thu, 3 May 2018 15:51:39 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Thu, 03 May 2018 13:51:41 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Thu, 03 May 2018 13:51:41 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH v2] MdeModulePkg/Bus: Enable ascending resource list X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 May 2018 13:51:42 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 05/02/18 20:07, Roman Bacik wrote: > Some processors require resource list sorted in ascending order. > > Cc: Ruiyu Ni > Cc: Vladimir Olovyannikov > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Roman Bacik > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 + > .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 43 ++++++++++++++++++---- > MdeModulePkg/MdeModulePkg.dec | 3 ++ > MdeModulePkg/MdeModulePkg.dsc | 1 + > 4 files changed, 41 insertions(+), 7 deletions(-) I don't understand the goal of this patch. (Please don't take my comments as "review" or arguments against this patch, I'd just like to understand the issue.) To my understanding, InsertResourceNode() keeps PCI resources sorted in descending *alignment* order -- because a larger power-of-two alignment is also suitable for a smaller power-of-two alignment. Say, we have a bridge with two devices behind it. The first device has one MMIO BAR of size 32MB (requiring the same 32MB natural alignment), while the other device has one MMIO BAR, of size 4MB (requiring the same 4MB natural alignment). Ordering the resources in descending *alignment* order means that we'll 1st allocate the 32MB BAR, at a suitably aligned address. The important thing is that the end of that allocation will *also* be aligned at 32MB, hence we can allocate, as 2nd step, the 4MB BAR right there. No gaps needed. If the 4MB BAR was allocated 1st (naturally aligned), then its end address might not be naturally aligned for becoming the base address of the remaining 32MB BAR. Thus we might have to waste some MMIO aperture to align the large BAR correctly. Here's an example output from PciBusDxe running as part of OVMF: > PciBus: Resource Map for Root Bridge PciRoot(0x0) > [...] > Type = Mem32; Base = 0x80000000; Length = 0x8100000; Alignment = 0x3FFFFFF > Base = 0x80000000; Length = 0x4000000; Alignment = 0x3FFFFFF; Owner = PCI [00|02|00:14] > Base = 0x84000000; Length = 0x4000000; Alignment = 0x3FFFFFF; Owner = PCI [00|02|00:10] > Base = 0x88000000; Length = 0x2000; Alignment = 0x1FFF; Owner = PCI [00|02|00:18] > Base = 0x88002000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [00|07|00:14] > Base = 0x88003000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [00|06|07:10] > Base = 0x88004000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [00|05|00:14] > Base = 0x88005000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [00|03|00:14] > [...] The base addresses are already sorted in ascending order; what's descending is the alignment -- and that seems correct, because it conserves MMIO aperture (no gaps needed). Can you please explain what's wrong with this approach for your platform, and what the desired behavior is? Can you post a log snippet that shows a placement that's right for your platform? Thanks, Laszlo