archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <>
To: Gavin Shan <>
Subject: Re: [PATCH 5/8] pci: resource assignment based on p2p alignment
Date: Tue, 21 Aug 2012 11:46:23 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, Aug 21, 2012 at 12:36 AM, Gavin Shan <> wrote:
> Last night, I talked to Bjorn through IRC. It seems that Bjorn still have
> concerns on what we're doing in function pcibios_window_alignment() and
> pcibios_align_resource(). Another question that Bjorn has is that's possible
> to put the PPC alignment (alignment on p2p memory windows) to pcibios_align_resource().
> Actually, I've replied to that before and it seems the reply was lost again.

It's not that I missed the previous discussion, it's just that I
didn't think we had come up with a clear explanation for why we needed
two kinds of alignment hooks and a way to decide when to use one vs.
the other.  It might be clear in *your* head, but it's not clear to me
yet :)

> Anyway, here's the full message about the discussion.
>>> Hmm..'struct resource *window' may not yet know which aperature it is
>>> allocated from. Will it? We are still in the sizing process, the allocation will
>>> be done much later.
>> Of course, you're absolutely right; I had this backwards.  In
>> pbus_size_io/mem(), we do "b_res = find_free_bus_resource()", so b_res
>> is a bus resource that matches the desired type (IO/MEM).  This
>> resource represents an aperture of the upstream bridge leading to the
>> bus.  I was thinking that b_res->start would contain address
>> information that the arch could use to decide alignment.
>> But at this point, in pbus_size_io/mem(), we set "b_res->start =
>> min_align", so obviously b_res contains no information about the
>> window base that will eventually be assigned.  I think b_res is
>> basically the *container* into which we'll eventually put the P2P
>> aperture start/end, but here, we're using that container to hold the
>> information about the size and alignment we need for that aperture.
>> The fact that we deal with alignment in pbus_size_mem() and again in
>> __pci_assign_resource() (via pcibios_align_resource) is confusing to
>> me -- I don't have a clear idea of what sorts of alignment are done in
>> each place.  Could this powerpc alignment be done in
>> pcibios_align_resource()?  We do have the actual proposed address
>> there, as well as the pci_dev.
> Currently, we have 2 phases for resourece reallocation. During the first
> phase that is being covered by function pbus_size_io() and pbus_size_mem(),
> the PCI hierarchy tree is scanned from bottom to top so that we can reserve
> enough resources in p2p I/O or memory windows for its child devices (including
> child p2p bridge possiblly). During the time, pcibios_window_alignment() will
> be called to determine the minimal alignment of the p2p windows (I/O or memory)
> so that the p2p windows could be aligned to what the specific platform requires.
> In the 2nd phase, the resources of p2p windows as well as PCI devices will
> be allocated. That will be done according to the PCI hierarchy tree from top
> to bottom. During the period, function pcibios_align_resource() will be called
> to do _little_ adjustment on those resources that have special requirments. If
> I understood correctly, it's unsafe to change the resource size (or start address)
> greatly.
> Lets have one example here for more explanation on why it's unsafe to change
> the resource size (or start address) greatly by pcibios_align_resource(). As
> the following figure shows, we have 2 p2p bridges (A, B) and another one PCI
> device (C). The memory resources they're owning are shown below them after the
> phase one. Now, we're running in phase 2 (resource allocation) for (B) when the
> allocation on resources of (A) should have done because phase 2 is expected to
> be done from top to bottom of the PCI tree. It is impossible to extend the resource
> of (B) for another one 1MB to [0x100000, 0x2fffff] through pcibios_align_resource()
> since (C) needs some resources as well. The cause is that we never did enough resource
> reservation for (A) during phase 1. So pcibios_align_resource() isn't safe enough to
> cover the PowerPC alignment we requires, but pcibios_window_alignment() can accomodate
> that very well :-)
>                 [ p2p bridge A ]
>                 [0x100000, 0x2fffff]
>                       |
>         ---------------------------------
>         |                               |
>       [ p2p bridge B ]              [ PCI dev C]
>       [0x100000, 0x1fffff]
> In summary, pcibios_window_alignment() takes affect in phase 1 (resource reservation),
> but pcibios_align_resource() will function in phase 2 (resource allocation). They're
> different.

This is a good start.  You're saying that "large" alignments must be
done in phase 1 with pcibios_window_alignment() and
pcibios_align_resource() can do "small" adjustments in phase 2.  How
do we quantify that?  *Can* it be quantified?

I think pcibios_align_resource() is primarily used to deal with
devices that only do partial decoding, e.g., ISA 10-bit I/O port
resources.  By the time we call it, I think the upstream aperture has
already been determined, so the allocation may fail completely.

It seems like we can't accurately do even the resource reservation
part without knowing what pcibios_align_resource() is going to do.


  parent reply	other threads:[~2012-08-21 17:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-20 13:46 [PATCH V8 0/8] Minimal alignment for p2p bars Gavin Shan
2012-08-20 13:46 ` [PATCH 1/8] pci: change variable name for find_pci_host_bridge Gavin Shan
2012-09-06 23:20   ` Bjorn Helgaas
2012-08-20 13:46 ` [PATCH 2/8] pci: argument pci_bus " Gavin Shan
2012-08-20 13:46 ` [PATCH 3/8] pci: fiddle with conversion of pci and CPU address Gavin Shan
2012-08-20 13:46 ` [PATCH 4/8] pci: weak function returns alignment Gavin Shan
2012-08-20 13:46 ` [PATCH 5/8] pci: resource assignment based on p2p alignment Gavin Shan
     [not found]   ` <>
2012-08-21 17:46     ` Bjorn Helgaas [this message]
2012-09-06 23:21   ` Bjorn Helgaas
2012-08-20 13:46 ` [PATCH 6/8] pci: refactor function pbus_size_mem Gavin Shan
2012-08-20 13:46 ` [PATCH 7/8] ppc/pci: override pcibios_window_alignment Gavin Shan
2012-08-20 13:46 ` [PATCH 8/8] ppc/pnv: I/O and memory alignment for p2p bridges Gavin Shan
  -- strict thread matches above, loose matches on Subject: below --
2012-07-25  1:49 [PATCH Resend v7 0/8] minimal alignment for p2p bars Gavin Shan
2012-07-25  1:49 ` [PATCH 5/8] pci: resource assignment based on p2p alignment Gavin Shan

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-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \ \ \

* 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;
as well as URLs for NNTP newsgroup(s).