[PATCH 05/15] pci: resource assignment based on p2p alignment

[Bjorn Helgaas]

On Tue, Jul 17, 2012 at 10:25 PM, Ram Pai <linuxram at us.ibm.com> wrote:
> On Tue, Jul 17, 2012 at 11:14:51AM -0600, Bjorn Helgaas wrote:
>> On Tue, Jul 17, 2012 at 4:38 AM, Benjamin Herrenschmidt
>> <benh at kernel.crashing.org> wrote:
>> > On Tue, 2012-07-17 at 18:03 +0800, Ram Pai wrote:
>> >>         Lets say we passed that 'type' flag to size the minimum
>> >>         alignment constraints for that b_res.  And window_alignment(bus,
>> >>         type) of your platform  used that 'type' information to
>> >>         determine whether to use the alignment constraints of 32-bit
>> >>         window or 64-bit window.
>> >>
>> >>         However, later when the b_res is actually allocated a resource,
>> >>         the pci_assign_resource() has no idea whether to allocate 32-bit
>> >>         window resource or 64-bit window resource, because the 'type'
>> >>         information is not captured anywhere in b_res.
>> >>
>> >>         You would basically have a disconnect between what is sized and
>> >>         what is allocated. Unless offcourse you pass that 'type' to
>> >>         the b_res->flags, which is currently not the case.
>> >
>> > Right, we ideally would need the core to query the alignment once per
>> > "apertures" it tries as a candidate to allocate a given resource... but
>> > that's for later.
>> >
>> > For now we can probably live with giving out the max of the minimum
>> > alignment we support for M64 and our M32 segment size.
>>
>> We already know the aperture we're proposing to allocate from (the
>> result of find_free_bus_resource()), don't we?  What if we passed it
>> to pcibios_window_alignment() along with the struct pci_bus *, e.g.:
>>
>>   resource_size_t pcibios_window_alignment(struct pci_bus *bus, struct
>> resource *window)
>
> 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.

Bjorn