From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::22b; helo=mail-wr0-x22b.google.com; envelope-from=roman.bacik@broadcom.com; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x22b.google.com (mail-wr0-x22b.google.com [IPv6:2a00:1450:400c:c0c::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3F0D12063E318 for ; Thu, 3 May 2018 08:23:18 -0700 (PDT) Received: by mail-wr0-x22b.google.com with SMTP id g21-v6so18033531wrb.8 for ; Thu, 03 May 2018 08:23:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:references:in-reply-to:mime-version:thread-index:date :message-id:subject:to:cc; bh=S969DYuoZQojiYiIi1fpf1bwlko324Chm59u6YtLIQQ=; b=Zmrc0rSNCSzy5CdhQr3rsUe94mlZGQ7m7Wn2wq+cQx7n3N2ZLpjN7W3Oqwu3iUZECp w/TcE+vGXqgXUIQUr/sR/S+NXIQWivTLSzP837tCCedmfOmug+oexaWVjYhJ0/PNZcyg 1s0rNLdLA/AXNbAzYhJ6DW2sfRoEiMpNeItsU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:references:in-reply-to:mime-version :thread-index:date:message-id:subject:to:cc; bh=S969DYuoZQojiYiIi1fpf1bwlko324Chm59u6YtLIQQ=; b=gVH4VpEik42RiCTynPW2l5WVi3Gtt+Tt3LbNRKRWZ2dvXi4kNbk6WYsmM08SOqdq2R WPJOMpyoeSMzk3XJtOoRGBrG6b9WRxBlHR6WKpcJQxnq0m37HgWKele/xnFsMkOriAqF 9zy/osqmLY1RsrDDOlEWlFcnfpSE3mL/aGQ9lv40LWiF8Kh/Ss4ezv0DnLmLqRs3DJQz BQpYUpcMbTGOdk55P2ZAddI5G4CszzF43d4kPabmiTdY8vOdxUpozCahklK0B3cJOeoa 028fqJhdqQgDz7YDUTLAxxotCV8KfAFfaVc5BANrkjnpdiLu+waDRJDCTwswCb+zxPLi FqGw== X-Gm-Message-State: ALQs6tCg9dGLePp9pHM27NxP+47fq+ikdwYJDWTAC3vRcRNE0Uk8tM2w 0EO1DJxjvHbpF4c9GwrKM4dV6ftKUP//7JCzGZ2KbOvI X-Google-Smtp-Source: AB8JxZr3jGIZ2y93+3H6h+Gf2l7uv0Vn7dPKFOG2PzD5YMTWZ69eEYX277PvrB4zbbQAPMsKJj8NTOTH+ocameYZR0U= X-Received: by 2002:adf:c3cd:: with SMTP id d13-v6mr14418813wrg.282.1525360997174; Thu, 03 May 2018 08:23:17 -0700 (PDT) From: Roman Bacik References: f63b4f3ce6c563cd42538eda762dcd84@mail.gmail.com In-Reply-To: f63b4f3ce6c563cd42538eda762dcd84@mail.gmail.com MIME-Version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQGKesxFKDyBnoQwMKioqNjIzx29aAFK0DnSpKaPgYCAAAlmIA== Date: Thu, 3 May 2018 08:23:15 -0700 Message-ID: <45404ff573f49b7807e06e0462ab4018@mail.gmail.com> To: Laszlo Ersek Cc: edk2-devel@lists.01.org, Ruiyu Ni , Vladimir Olovyannikov 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 15:23:19 -0000 Content-Type: text/plain; charset="UTF-8" Without any patches we would get the following undesirable placement: PCI Bus First Scanning PciBus: Discovered PPB @ [00|00|00] PciBus: Discovered PCI @ [01|00|00] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: Discovered PCI @ [01|00|01] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: Discovered PCI @ [01|00|02] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: Discovered PCI @ [01|00|03] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: Discovered PPB @ [00|00|00] PciBus: Discovered PCI @ [01|00|00] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: Discovered PCI @ [01|00|01] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: Discovered PCI @ [01|00|02] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: Discovered PCI @ [01|00|03] ARI: CapOffset = 0x1B8 BAR[0]: Type = PMem64; Alignment = 0xFFFF; Length = 0x10000; Offset = 0x10 BAR[1]: Type = PMem64; Alignment = 0x1FFFF; Length = 0x20000; Offset = 0x18 BAR[2]: Type = PMem64; Alignment = 0xFFF; Length = 0x1000; Offset = 0x20 PciBus: HostBridge->SubmitResources() - Success PciBus: HostBridge->NotifyPhase(AllocateResources) - Success PciBus: Resource Map for Root Bridge Acpi(PNP0A05,0x0) Type = PMem32; Base = 0x60B00000; Length = 0x100000; Alignment = 0xFFFFF Base = 0x60B00000; Length = 0x100000; Alignment = 0xFFFFF; Owner = PPB [00|00|00:**] PciBus: Resource Map for Bridge [00|00|00] Type = PMem32; Base = 0x60B00000; Length = 0x100000; Alignment = 0xFFFFF Base = 0x60B00000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [01|00|03:18]; Type = PMem64 Base = 0x60B20000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [01|00|02:18]; Type = PMem64 Base = 0x60B40000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [01|00|01:18]; Type = PMem64 Base = 0x60B60000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [01|00|00:18]; Type = PMem64 Base = 0x60B80000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|03:10]; Type = PMem64 Base = 0x60B90000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|02:10]; Type = PMem64 Base = 0x60BA0000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|01:10]; Type = PMem64 Base = 0x60BB0000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|00:10]; Type = PMem64 Base = 0x60BC0000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|03:20]; Type = PMem64 Base = 0x60BC1000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|02:20]; Type = PMem64 Base = 0x60BC2000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|01:20]; Type = PMem64 Base = 0x60BC3000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|00:20]; Type = PMem64 Thanks, Roman > -----Original Message----- > From: Roman Bacik [mailto:roman.bacik@broadcom.com] > Sent: Thursday, May 3, 2018 7:58 AM > To: 'Laszlo Ersek' > Cc: 'edk2-devel@lists.01.org'; 'Ruiyu Ni'; Vladimir Olovyannikov > Subject: RE: [edk2] [PATCH v2] MdeModulePkg/Bus: Enable ascending > resource list > > Laszlo, > > Thank you very much for your explanation. > > > -----Original Message----- > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > Sent: Thursday, May 3, 2018 6:52 AM > > To: Roman Bacik > > Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov > > Subject: Re: [edk2] [PATCH v2] MdeModulePkg/Bus: Enable ascending > > resource list > > > > 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? > > We are also patching allocation order bottom up search starting at > BaseAddress. > > This placement is desirable and working for us after applying the > submitted > two patches: > > PciBus: Resource Map for Bridge [00|00|00] > Type = PMem32; Base = 0x60000000; Length = 0x100000; Alignment > = > 0xFFFFF > Base = 0x60000000; Length = 0x20000; Alignment = 0x1FFFF; > Owner = > PCI [01|00|00:18]; Type = PMem64 > Base = 0x60020000; Length = 0x20000; Alignment = 0x1FFFF; > Owner = > PCI [01|00|01:18]; Type = PMem64 > Base = 0x60040000; Length = 0x20000; Alignment = 0x1FFFF; > Owner = > PCI [01|00|02:18]; Type = PMem64 > Base = 0x60060000; Length = 0x20000; Alignment = 0x1FFFF; > Owner = > PCI [01|00|03:18]; Type = PMem64 > Base = 0x60080000; Length = 0x10000; Alignment = 0xFFFF; > Owner = > PCI [01|00|00:10]; Type = PMem64 > Base = 0x60090000; Length = 0x10000; Alignment = 0xFFFF; > Owner = > PCI [01|00|01:10]; Type = PMem64 > Base = 0x600A0000; Length = 0x10000; Alignment = 0xFFFF; > Owner = > PCI [01|00|02:10]; Type = PMem64 > Base = 0x600B0000; Length = 0x10000; Alignment = 0xFFFF; > Owner = > PCI [01|00|03:10]; Type = PMem64 > Base = 0x600C0000; Length = 0x1000; Alignment = 0xFFF; > Owner = > PCI [01|00|00:20]; Type = PMem64 > Base = 0x600C1000; Length = 0x1000; Alignment = 0xFFF; > Owner = > PCI [01|00|01:20]; Type = PMem64 > Base = 0x600C2000; Length = 0x1000; Alignment = 0xFFF; > Owner = > PCI [01|00|02:20]; Type = PMem64 > Base = 0x600C3000; Length = 0x1000; Alignment = 0xFFF; > Owner = > PCI [01|00|03:20]; Type = PMem64 > > > > > Thanks, > > Laszlo > > Thanks, > Roman