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::22e; helo=mail-wr0-x22e.google.com; envelope-from=roman.bacik@broadcom.com; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x22e.google.com (mail-wr0-x22e.google.com [IPv6:2a00:1450:400c:c0c::22e]) (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 A00C320957B00 for ; Thu, 3 May 2018 09:45:59 -0700 (PDT) Received: by mail-wr0-x22e.google.com with SMTP id v5-v6so18316117wrf.9 for ; Thu, 03 May 2018 09:45:59 -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=YyO2w/9Qafe6+WQxjaPQ+AgwBHj29vSQUdK+6KmDmHw=; b=WrO8kWakshWLaZTEHhPwSD5NEboNVZOtoGmgFpWtZutsklmCbnnm+eL5Hq+xeyczmf h5BIJ1DQO4T64piQ8Nez2ik+4Dko5G+kgy9A9DpJpr6K8G+O5JwQjb+8+s/ksH2+dgmM Y3v6/4fv95qgCtyfYej7CQ9Q4gBXnzhIovT8E= 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=YyO2w/9Qafe6+WQxjaPQ+AgwBHj29vSQUdK+6KmDmHw=; b=B7Y8JIHpl0/ZN+fP2AOZeVpSCxW5nVyH55+W6YNvteiJ8JKV+PE4ft24okg0AKsCfL dsH2gh8s42xS9n0a3LHl1DLy1tdKV3mUCwd12JzIcuIeqNKOo/MB8NAGd9y0+cjKDGyU JhXQu55Hr91J3AsSRIWZHtUd7kHAxZlE9tLczNqRlkuf+iJEWyofmIgsGERXDmEtjbr/ 9RpnwMK5wlX+D0VVqjz6Y7AC8nm82vxzP615hG0Fuew4jhtXsqEqnXYEOYNzBcR9HEEp qFIy3lxxMgFtuYtx+2L6oYNozIxiZa+bjicwwFvDHFx8TDJua2hpTAM91UenS0b/ODCr d/1Q== X-Gm-Message-State: ALQs6tC8L4ziDaFjBW2Uqx4xzKrwYDESEgohfIH7+ft3Arlcyenq3eJM Xw799s5kt+Xy35x1jREmOOUjTGM/wxSaeN10SAq8Aw== X-Google-Smtp-Source: AB8JxZoyRMejxib9kphwsHOk4N95Ztj8jBki+Ht/aD0KHjc2JeTbukkZ+AZc390NyDb0l12NWlTRudTFzR1ufgfIINs= X-Received: by 2002:adf:a617:: with SMTP id k23-v6mr19860845wrc.200.1525365957656; Thu, 03 May 2018 09:45:57 -0700 (PDT) From: Roman Bacik References: f63b4f3ce6c563cd42538eda762dcd84@mail.gmail.com 45404ff573f49b7807e06e0462ab4018@mail.gmail.com In-Reply-To: 45404ff573f49b7807e06e0462ab4018@mail.gmail.com MIME-Version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQGKesxFKDyBnoQwMKioqNjIzx29aAFK0DnSpKaPgYCAAAlmIIAAFBkA Date: Thu, 3 May 2018 09:45:56 -0700 Message-ID: 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 16:46:00 -0000 Content-Type: text/plain; charset="UTF-8" Laszlo, After changing gcd allocate type to EfiGcdAllocateAnySearchBottomUp we are getting a correct placement. However when using the original descending order, we are getting a crash later unless we change the resource order to ascending. That is why we would like to enable ascending order. This is the crash we are getting without the patch: 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|03:18]; Type = PMem64 Base = 0x60020000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [01|00|02:18]; Type = PMem64 Base = 0x60040000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [01|00|01:18]; Type = PMem64 Base = 0x60060000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [01|00|00:18]; Type = PMem64 Base = 0x60080000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|03:10]; Type = PMem64 Base = 0x60090000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|02:10]; Type = PMem64 Base = 0x600A0000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|01:10]; Type = PMem64 Base = 0x600B0000; Length = 0x10000; Alignment = 0xFFFF; Owner = PCI [01|00|00:10]; Type = PMem64 Base = 0x600C0000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|03:20]; Type = PMem64 Base = 0x600C1000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|02:20]; Type = PMem64 Base = 0x600C2000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|01:20]; Type = PMem64 Base = 0x600C3000; Length = 0x1000; Alignment = 0xFFF; Owner = PCI [01|00|00:20]; Type = PMem64 ... SError Exception at 0x00000000FDDF6338 PC 0x0000FDDF6338 (0x0000FDDF4000+0x00002338) [ 0] CpuIo2Dxe.dll PC 0x0000FDDF62F4 (0x0000FDDF4000+0x000022F4) [ 0] CpuIo2Dxe.dll PC 0x0000FDDF5634 (0x0000FDDF4000+0x00001634) [ 0] CpuIo2Dxe.dll PC 0x0000F87EAFAC (0x0000F87E7000+0x00003FAC) [ 1] PciHostBridge.dll PC 0x0000F81E926C (0x0000F81DA000+0x0000F26C) [ 2] PciBusDxe.dll PC 0x0000F8765F8C (0x0000F8757000+0x0000EF8C) [ 3] cxundi.dll PC 0x0000F875C96C (0x0000F8757000+0x0000596C) [ 3] cxundi.dll PC 0x0000F875B68C (0x0000F8757000+0x0000468C) [ 3] cxundi.dll PC 0x0000F8762480 (0x0000F8757000+0x0000B480) [ 3] cxundi.dll PC 0x0000FE66191C (0x0000FE653000+0x0000E91C) [ 4] DxeCore.dll PC 0x0000FE660CE8 (0x0000FE653000+0x0000DCE8) [ 4] DxeCore.dll PC 0x0000FE661038 (0x0000FE653000+0x0000E038) [ 4] DxeCore.dll PC 0x0000F8645114 (0x0000F8636000+0x0000F114) [ 5] BdsDxe.dll PC 0x0000F8645184 (0x0000F8636000+0x0000F184) [ 5] BdsDxe.dll PC 0x0000F8653B38 (0x0000F8636000+0x0001DB38) [ 5] BdsDxe.dll PC 0x0000F8638E34 (0x0000F8636000+0x00002E34) [ 5] BdsDxe.dll PC 0x0000FE655524 (0x0000FE653000+0x00002524) [ 6] DxeCore.dll PC 0x0000FE6544A0 (0x0000FE653000+0x000014A0) [ 6] DxeCore.dll PC 0x0000FE654024 (0x0000FE653000+0x00001024) [ 6] DxeCore.dll PC 0x0000850099F4 PC 0x000085009BD8 PC 0x000085000DCC PC 0x000085000FC4 PC 0x000085000F18 PC 0x0000850008BC [ 0] /local/ldk/edk2/Build/NS2Pkg/NOOPT_GCC5/AARCH64/UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe/DEBUG/CpuIo2Dxe.dll [ 1] /local/ldk/edk2/Build/NS2Pkg/NOOPT_GCC5/AARCH64/BroadcomPlatformPkg/Drivers/BrcmPciHostBridgeDxe/PciHostBridgeDxe/DEBUG/PciHostBridge.dll [ 2] /local/ldk/edk2/Build/NS2Pkg/NOOPT_GCC5/AARCH64/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe/DEBUG/PciBusDxe.dll [ 3] /local/ldk/edk2/Build/NS2Pkg/NOOPT_GCC5/AARCH64/BroadcomPlatformPkg/Drivers/CxUndiDxe/cxundi_auto/DEBUG/cxundi.dll [ 4] /local/ldk/edk2/Build/NS2Pkg/NOOPT_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll [ 5] /local/ldk/edk2/Build/NS2Pkg/NOOPT_GCC5/AARCH64/MdeModulePkg/Universal/BdsDxe/BdsDxe/DEBUG/BdsDxe.dll [ 6] /local/ldk/edk2/Build/NS2Pkg/NOOPT_GCC5/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll Thanks, Roman > -----Original Message----- > From: Roman Bacik [mailto:roman.bacik@broadcom.com] > Sent: Thursday, May 3, 2018 8:23 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 > > 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