From: Roman Bacik <roman.bacik@broadcom.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel@lists.01.org, Ruiyu Ni <ruiyu.ni@intel.com>,
Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Subject: Re: [PATCH v2] MdeModulePkg/Bus: Enable ascending resource list
Date: Thu, 3 May 2018 09:45:56 -0700 [thread overview]
Message-ID: <c192969c7676be54c7426fa56f0b721b@mail.gmail.com> (raw)
In-Reply-To: 45404ff573f49b7807e06e0462ab4018@mail.gmail.com
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 <ruiyu.ni@intel.com>
> > > > Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Roman Bacik <roman.bacik@broadcom.com>
> > > > ---
> > > > 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
next prev parent reply other threads:[~2018-05-03 16:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-02 18:07 [PATCH v2] MdeModulePkg/Bus: Enable ascending resource list Roman Bacik
2018-05-03 13:51 ` Laszlo Ersek
2018-05-03 14:58 ` Roman Bacik
2018-05-03 15:23 ` Roman Bacik
2018-05-03 19:02 ` Laszlo Ersek
2018-05-03 16:45 ` Roman Bacik [this message]
2018-05-03 19:01 ` Roman Bacik
2018-05-03 19:35 ` Ard Biesheuvel
2018-05-03 19:39 ` Roman Bacik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c192969c7676be54c7426fa56f0b721b@mail.gmail.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox