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 08:23:15 -0700 [thread overview]
Message-ID: <45404ff573f49b7807e06e0462ab4018@mail.gmail.com> (raw)
In-Reply-To: f63b4f3ce6c563cd42538eda762dcd84@mail.gmail.com
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 15:23 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 [this message]
2018-05-03 19:02 ` Laszlo Ersek
2018-05-03 16:45 ` Roman Bacik
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=45404ff573f49b7807e06e0462ab4018@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