Re: pci-mvebu driver on km_kirkwood

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Gerlando, Bjorn,

Bjorn, I added you as the To: because there is a PCI related question
for you below :)

On Wed, 19 Feb 2014 10:39:07 +0100, Gerlando Falauto wrote:

> spoiler first: SUCCESS!!!!

Awesome :)

> > I'm obviously interested in seeing the message that gets shown, as well
> > as the new mvebu-mbus debugfs output.
> 
> ----------
> pci 0000:00:01.0:   bridge window [mem 0xe0000000-0xebffffff]
> PCIE 0.0: creating window at 0xe0000000, size 0xbffffff rounded up to 
> 0x10000000

Right, rounding from 192 MB to 265 MB.

>   cat /sys/kernel/debug/mvebu-mbus/devices
> [00] disabled
> [01] disabled
> [02] disabled
> [03] disabled
> [04] 00000000ff000000 - 00000000ff010000 : nand
> [05] 00000000f4000000 - 00000000f8000000 : vpcie
> [06] 00000000fe000000 - 00000000fe010000 : dragonite
> [07] 00000000e0000000 - 00000000f0000000 : pcie0.0
> 
> > For good measure, if you could also dump the registers of the PCIe
> > window. In your case, it was window 7, so dumping 0xf1020070 and
> > 0xf1020074 would be useful.
> 
> Isn't that where the output of debugfs comes from?

It is, but the mvebu-mbus is interpreting the sequence of 1s and 0s to
give the real size, and this involves a little bit of magic of bit
manipulation, which I wanted to check by having a look at the raw
values of the registers.

> > I am not sure, but since we are configuring an invalid memory size,
> > maybe the MBus behavior is undefined, and we get some completely funky
> > behavior, where parts of the 192 MB window are actually work, but parts
> > of it are not.
> 
> And... Ladies and gentlemen... it turns out YOU'RE RIGHT!!!
> With your patch now everything works fine!!!
> 
> No words (or quads, for that matter) can express how grateful I am! ;-)

Cool. However, I am not sure my fix is really correct, because is you
had another PCIe device that needed 64 MB of memory space, the PCIe
core would have allocated addresses 0xec000000 -> 0xf0000000 to it,
which would have conflicted with the forced "power of 2 up-rounding"
we've applied on the memory space of the first device.

Therefore, I believe this constraint should be taken into account by
the PCIe core when allocating the different memory regions for each
device.

Bjorn, the mvebu PCIe host driver has the constraint that the I/O and
memory regions associated to each PCIe device of the emulated bridge
have a size that is a power of 2.

I am currently using the ->align_resource() hook to ensure that the
start address of the resource matches certain other constraints, but I
don't see a way of telling the PCI core that I need the resource to
have its size rounded up to the next power of 2 size. Is there a way of
doing this?

In the case described by Gerlando, the PCI core has assigned a 192 MB
region, but the Marvell hardware can only create windows that have a
power of two size, i.e 256 MB. Therefore, the PCI core should be told
this constraint, so that it doesn't allocate the next resource right
after the 192 MB one.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Bjorn Helgaas]

On Wed, Feb 19, 2014 at 6:37 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Gerlando, Bjorn,
>
> Bjorn, I added you as the To: because there is a PCI related question
> for you below :)
>
> On Wed, 19 Feb 2014 10:39:07 +0100, Gerlando Falauto wrote:
>
>> spoiler first: SUCCESS!!!!
>
> Awesome :)
>
>> > I'm obviously interested in seeing the message that gets shown, as well
>> > as the new mvebu-mbus debugfs output.
>>
>> ----------
>> pci 0000:00:01.0:   bridge window [mem 0xe0000000-0xebffffff]
>> PCIE 0.0: creating window at 0xe0000000, size 0xbffffff rounded up to
>> 0x10000000
>
> Right, rounding from 192 MB to 265 MB.
>
>>   cat /sys/kernel/debug/mvebu-mbus/devices
>> [00] disabled
>> [01] disabled
>> [02] disabled
>> [03] disabled
>> [04] 00000000ff000000 - 00000000ff010000 : nand
>> [05] 00000000f4000000 - 00000000f8000000 : vpcie
>> [06] 00000000fe000000 - 00000000fe010000 : dragonite
>> [07] 00000000e0000000 - 00000000f0000000 : pcie0.0
>>
>> > For good measure, if you could also dump the registers of the PCIe
>> > window. In your case, it was window 7, so dumping 0xf1020070 and
>> > 0xf1020074 would be useful.
>>
>> Isn't that where the output of debugfs comes from?
>
> It is, but the mvebu-mbus is interpreting the sequence of 1s and 0s to
> give the real size, and this involves a little bit of magic of bit
> manipulation, which I wanted to check by having a look at the raw
> values of the registers.
>
>> > I am not sure, but since we are configuring an invalid memory size,
>> > maybe the MBus behavior is undefined, and we get some completely funky
>> > behavior, where parts of the 192 MB window are actually work, but parts
>> > of it are not.
>>
>> And... Ladies and gentlemen... it turns out YOU'RE RIGHT!!!
>> With your patch now everything works fine!!!
>>
>> No words (or quads, for that matter) can express how grateful I am! ;-)
>
> Cool. However, I am not sure my fix is really correct, because is you
> had another PCIe device that needed 64 MB of memory space, the PCIe
> core would have allocated addresses 0xec000000 -> 0xf0000000 to it,
> which would have conflicted with the forced "power of 2 up-rounding"
> we've applied on the memory space of the first device.
>
> Therefore, I believe this constraint should be taken into account by
> the PCIe core when allocating the different memory regions for each
> device.
>
> Bjorn, the mvebu PCIe host driver has the constraint that the I/O and
> memory regions associated to each PCIe device of the emulated bridge
> have a size that is a power of 2.
>
> I am currently using the ->align_resource() hook to ensure that the
> start address of the resource matches certain other constraints, but I
> don't see a way of telling the PCI core that I need the resource to
> have its size rounded up to the next power of 2 size. Is there a way of
> doing this?
>
> In the case described by Gerlando, the PCI core has assigned a 192 MB
> region, but the Marvell hardware can only create windows that have a
> power of two size, i.e 256 MB. Therefore, the PCI core should be told
> this constraint, so that it doesn't allocate the next resource right
> after the 192 MB one.

I'm not sure I understand this correctly, but I *think* this 192 MB
region that gets rounded up to 256 MB because of the Marvell
constraint is a host bridge aperture.  If that's the case, it's
entirely up to you (the host bridge driver author) to round it as
needed before passing it to pci_add_resource_offset().

The PCI core will never allocate any space that is outside the host
bridge apertures.

But maybe I don't understand your situation well enough.

Bjorn

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Bjorn Helgaas,

+ Jason Gunthorpe.

On Wed, 19 Feb 2014 14:45:48 -0700, Bjorn Helgaas wrote:

> > Cool. However, I am not sure my fix is really correct, because is you
> > had another PCIe device that needed 64 MB of memory space, the PCIe
> > core would have allocated addresses 0xec000000 -> 0xf0000000 to it,
> > which would have conflicted with the forced "power of 2 up-rounding"
> > we've applied on the memory space of the first device.
> >
> > Therefore, I believe this constraint should be taken into account by
> > the PCIe core when allocating the different memory regions for each
> > device.
> >
> > Bjorn, the mvebu PCIe host driver has the constraint that the I/O and
> > memory regions associated to each PCIe device of the emulated bridge
> > have a size that is a power of 2.
> >
> > I am currently using the ->align_resource() hook to ensure that the
> > start address of the resource matches certain other constraints, but I
> > don't see a way of telling the PCI core that I need the resource to
> > have its size rounded up to the next power of 2 size. Is there a way of
> > doing this?
> >
> > In the case described by Gerlando, the PCI core has assigned a 192 MB
> > region, but the Marvell hardware can only create windows that have a
> > power of two size, i.e 256 MB. Therefore, the PCI core should be told
> > this constraint, so that it doesn't allocate the next resource right
> > after the 192 MB one.
> 
> I'm not sure I understand this correctly, but I *think* this 192 MB
> region that gets rounded up to 256 MB because of the Marvell
> constraint is a host bridge aperture.  If that's the case, it's
> entirely up to you (the host bridge driver author) to round it as
> needed before passing it to pci_add_resource_offset().
> 
> The PCI core will never allocate any space that is outside the host
> bridge apertures.

Hum, I believe there is a misunderstanding here. We are already using
pci_add_resource_offset() to define the global aperture for the entire
PCI bridge. This is not causing any problem.

Let me give a little bit of background first.

On Marvell hardware, the physical address space layout is configurable,
through the use of "MBus windows". A "MBus window" is defined by a base
address, a size, and a target device. So if the CPU needs to access a
given device (such as PCIe 0.0 for example), then we need to create a
"MBus window" whose size and target device match PCIe 0.0.

Since Armada XP has 10 PCIe interfaces, we cannot just statically
create as many MBus windows as there are PCIe interfaces: it would both
exhaust the number of MBus windows available, and also exhaust the
physical address space, because we would have to create very large
windows, just in case the PCIe device plugged behind this interface
needs large BARs.

So, what the pci-mvebu.c driver does is that it creates an emulated PCI
bridge. This emulated bridge is used to let the Linux PCI core
enumerate the real physical PCI devices behind the bridge, allocate a
range of physical addresses that is available for each of these
devices, and write them to the bridge registers. Since the bridge is
not a real one, but emulated, but trap those writes, and use them to
create the MBus windows that will allow the CPU to actually access the
device, at the base address chosen by the Linux PCI core during the
enumeration process.

However, MBus windows have a certain constraint that they must have a
power of two size, so the Linux PCI core should not write to one of the
bridge PCI_MEMORY_BASE / PCI_MEMORY_LIMIT registers any range of
address whose size is not a power of 2.

Let me take the example of Gerlando:

pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
pci 0000:00:01.0: PCI bridge to [bus 01]
pci 0000:00:01.0:   bridge window [mem 0xe0000000-0xebffffff]

So, pci 0000:01:00 is the real device, which has a number of BARs of a
certain size. Taking into account all those BARs, the Linux PCI core
decides to assign [mem 0xe0000000-0xebffffff] to the bridge (last line
of the log above). The problem is that [mem 0xe0000000-0xebffffff] is
192 MB, but we would like the Linux PCI core to extend that to 256 MB.

As you can see it is not about the global aperture associated to the
bridge, but about the size of the window associated to each "port" of
the bridge.

Does that make sense? Keep in mind that I'm still not completely
familiar with the PCI terminology, so maybe the above explanation does
not use the right terms.

Thanks for your feedback,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Jason Gunthorpe]

On Thu, Feb 20, 2014 at 09:55:18AM +0100, Thomas Petazzoni wrote:

> Does that make sense? Keep in mind that I'm still not completely
> familiar with the PCI terminology, so maybe the above explanation does
> not use the right terms.

Stated another way, the Marvel PCI-E to PCI-E bridge config space has
a quirk that requires the window BARs to be aligned on their size and
sized to a power of 2.

The first requirement is already being handled by hooking through
ARM's 'align_resource' callback.

One avenue would be to have mvebu_pcie_align_resource return a struct
resource and manipulate the size as well. Assuming the PCI core will
accommodate that.

Jason

Re: pci-mvebu driver on km_kirkwood

[Bjorn Helgaas]

On Thu, Feb 20, 2014 at 1:55 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Bjorn Helgaas,
>
> + Jason Gunthorpe.
>
> On Wed, 19 Feb 2014 14:45:48 -0700, Bjorn Helgaas wrote:
>
>> > Cool. However, I am not sure my fix is really correct, because is you
>> > had another PCIe device that needed 64 MB of memory space, the PCIe
>> > core would have allocated addresses 0xec000000 -> 0xf0000000 to it,
>> > which would have conflicted with the forced "power of 2 up-rounding"
>> > we've applied on the memory space of the first device.
>> >
>> > Therefore, I believe this constraint should be taken into account by
>> > the PCIe core when allocating the different memory regions for each
>> > device.
>> >
>> > Bjorn, the mvebu PCIe host driver has the constraint that the I/O and
>> > memory regions associated to each PCIe device of the emulated bridge
>> > have a size that is a power of 2.
>> >
>> > I am currently using the ->align_resource() hook to ensure that the
>> > start address of the resource matches certain other constraints, but I
>> > don't see a way of telling the PCI core that I need the resource to
>> > have its size rounded up to the next power of 2 size. Is there a way of
>> > doing this?
>> >
>> > In the case described by Gerlando, the PCI core has assigned a 192 MB
>> > region, but the Marvell hardware can only create windows that have a
>> > power of two size, i.e 256 MB. Therefore, the PCI core should be told
>> > this constraint, so that it doesn't allocate the next resource right
>> > after the 192 MB one.
>>
>> I'm not sure I understand this correctly, but I *think* this 192 MB
>> region that gets rounded up to 256 MB because of the Marvell
>> constraint is a host bridge aperture.  If that's the case, it's
>> entirely up to you (the host bridge driver author) to round it as
>> needed before passing it to pci_add_resource_offset().
>>
>> The PCI core will never allocate any space that is outside the host
>> bridge apertures.
>
> Hum, I believe there is a misunderstanding here. We are already using
> pci_add_resource_offset() to define the global aperture for the entire
> PCI bridge. This is not causing any problem.
>
> Let me give a little bit of background first.
>
> On Marvell hardware, the physical address space layout is configurable,
> through the use of "MBus windows". A "MBus window" is defined by a base
> address, a size, and a target device. So if the CPU needs to access a
> given device (such as PCIe 0.0 for example), then we need to create a
> "MBus window" whose size and target device match PCIe 0.0.

I was assuming "PCIe 0.0" was a host bridge, but it sounds like maybe
that's not true.  Is it really a PCIe root port?  That would mean the
MBus windows are some non-PCIe-compliant thing between the root
complex and the root ports, I guess.

> Since Armada XP has 10 PCIe interfaces, we cannot just statically
> create as many MBus windows as there are PCIe interfaces: it would both
> exhaust the number of MBus windows available, and also exhaust the
> physical address space, because we would have to create very large
> windows, just in case the PCIe device plugged behind this interface
> needs large BARs.

Everybody else in the world *does* statically configure host bridge
apertures before enumerating the devices below the bridge.  I see why
you want to know what devices are there before deciding whether and
how large to make an MBus window.  But that is new functionality that
we don't have today, and the general idea is not Marvell-specific, so
other systems might want something like this, too.  So I'm not sure if
using quirks to try to wedge it into the current PCI core is the right
approach.  I don't have another proposal, but we should at least think
about what direction we want to take.

> So, what the pci-mvebu.c driver does is that it creates an emulated PCI
> bridge. This emulated bridge is used to let the Linux PCI core
> enumerate the real physical PCI devices behind the bridge, allocate a
> range of physical addresses that is available for each of these
> devices, and write them to the bridge registers. Since the bridge is
> not a real one, but emulated, but trap those writes, and use them to
> create the MBus windows that will allow the CPU to actually access the
> device, at the base address chosen by the Linux PCI core during the
> enumeration process.
>
> However, MBus windows have a certain constraint that they must have a
> power of two size, so the Linux PCI core should not write to one of the
> bridge PCI_MEMORY_BASE / PCI_MEMORY_LIMIT registers any range of
> address whose size is not a power of 2.

I'm still not sure I understand what's going on here.  It sounds like
your emulated bridge basically wraps the host bridge and makes it look
like a PCI-PCI bridge.  But I assume the host bridge itself is also
visible, and has apertures (I guess these are the MBus windows?)  So
when you first discover the host bridge, before enumerating anything
below it, what apertures does it have?  Do you leave them disabled
until after we enumerate the devices, figure out how much space they
need, and configure the emulated PCI-PCI bridge to enable the MBus
windows?

It'd be nice if dmesg mentioned the host bridge explicitly as we do on
other architectures; maybe that would help understand what's going on
under the covers.  Maybe a longer excerpt would already have this; you
already use pci_add_resource_offset(), which is used when creating the
root bus, so you must have some sort of aperture before enumerating.

> Let me take the example of Gerlando:
>
> pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
> pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
> pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
> pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
> pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
> pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
> pci 0000:00:01.0: PCI bridge to [bus 01]
> pci 0000:00:01.0:   bridge window [mem 0xe0000000-0xebffffff]
>
> So, pci 0000:01:00 is the real device, which has a number of BARs of a
> certain size. Taking into account all those BARs, the Linux PCI core
> decides to assign [mem 0xe0000000-0xebffffff] to the bridge (last line
> of the log above). The problem is that [mem 0xe0000000-0xebffffff] is
> 192 MB, but we would like the Linux PCI core to extend that to 256 MB.

If 01:00.0 is a PCIe endpoint, it must have a root port above it, so
that means 00:01.0 must be the root port.  But I think you're saying
that 00:01.0 is actually *emulated* and isn't PCIe-compliant, e.g., it
has extra window alignment restrictions.  I'm scared about what other
non-PCIe-compliant things there might be.  What happens when the PCI
core configures MPS, ASPM, etc.,

> As you can see it is not about the global aperture associated to the
> bridge, but about the size of the window associated to each "port" of
> the bridge.

Bjorn

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Jason Gunthorpe,

On Thu, 20 Feb 2014 10:35:18 -0700, Jason Gunthorpe wrote:
> On Thu, Feb 20, 2014 at 09:55:18AM +0100, Thomas Petazzoni wrote:
> 
> > Does that make sense? Keep in mind that I'm still not completely
> > familiar with the PCI terminology, so maybe the above explanation does
> > not use the right terms.
> 
> Stated another way, the Marvel PCI-E to PCI-E bridge config space has
> a quirk that requires the window BARs to be aligned on their size and
> sized to a power of 2.

Correct.

> The first requirement is already being handled by hooking through
> ARM's 'align_resource' callback.

Absolutely.

> One avenue would be to have mvebu_pcie_align_resource return a struct
> resource and manipulate the size as well. Assuming the PCI core will
> accommodate that.

That would effectively be the easiest solution from the point of view
of the PCIe driver.

In practice, the story is a little bit more subtle than that: the PCIe
driver may want to decide to either tell the PCI core to enlarge the
window BAR up to the next power of two size, or to dedicate two windows
to it.

For example:

 * If the PCI core allocates a 96 KB BAR, we clearly want it to be
   enlarged to 128 KB, so that we have to create a single window for it.

 * However, if the PCI core allocates a 192 MB BAR, we may want to
   instead create two windows: a first one of 128 MB and a second one
   of 64 MB. This consumes two windows, but saves 64 MB of physical
   address space.

(Note that I haven't tested myself the creation of two windows for the
same target device, but I was told by Lior that it should work).

As you can see from the two examples above, we may not necessarily want
to enforce this power-of-two constraint in all cases. We may want to
accept a non-power-of-2 size in the case of the 192 MB BAR, and let the
mvebu-mbus driver figure out that it should allocate several
consecutive windows to cover these 192 MB.

But to begin with, rounding up all window BARs up to the next power of
two size would be perfectly OK.

Jason, would you mind maybe replying to Bjorn Helgaas email (Thu, 20
Feb 2014 12:18:42 -0700) ? I believe that a lot of the misunderstanding
between Bjorn and me is due to the fact that I don't use the correct
PCI terminology to describe how the Marvell hardware works, and how the
Marvell PCIe driver copes with it. I'm sure you would explain it in a
way that would be more easily understood by someone very familiar with
the PCI terminology such as Bjorn. Thanks a lot!

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Jason Gunthorpe]

On Thu, Feb 20, 2014 at 12:18:42PM -0700, Bjorn Helgaas wrote:

> > On Marvell hardware, the physical address space layout is configurable,
> > through the use of "MBus windows". A "MBus window" is defined by a base
> > address, a size, and a target device. So if the CPU needs to access a
> > given device (such as PCIe 0.0 for example), then we need to create a
> > "MBus window" whose size and target device match PCIe 0.0.
> 
> I was assuming "PCIe 0.0" was a host bridge, but it sounds like maybe
> that's not true.  Is it really a PCIe root port?  That would mean the
> MBus windows are some non-PCIe-compliant thing between the root
> complex and the root ports, I guess.

It really is a root port. The hardware acts like a root port at the
TLP level. It has all the root port specific stuff in some format but
critically, completely lacks a compliant config space for a root
port bridge.

So the driver creates a 'compliant' config space for the root
port. Building the config space requires harmonizing registers related
to the PCI-E and registers related to internal routing and dealing
with the mismatch between what the hardware can actualy provide and
what the PCI spec requires it provide.

The only mismatch that gets exposed to the PCI core we know about is
the bridge window address alignment restrictions.

This is what Thomas has been asking about.

> > Since Armada XP has 10 PCIe interfaces, we cannot just statically
> > create as many MBus windows as there are PCIe interfaces: it would both
> > exhaust the number of MBus windows available, and also exhaust the
> > physical address space, because we would have to create very large
> > windows, just in case the PCIe device plugged behind this interface
> > needs large BARs.
> 
> Everybody else in the world *does* statically configure host bridge
> apertures before enumerating the devices below the bridge.  

The original PCI-E driver for this hardware did use a 1 root port per
host bridge model, with static host bridge aperture allocation and so
forth.

It works fine, just like everyone else in the world, as long as you
have only 1 or 2 ports. The XP hardware had *10* ports on a single
32 bit machine. You run out of address space, you run out of
HW routing resources, it just doesn't work acceptably.

> I see why you want to know what devices are there before deciding
> whether and how large to make an MBus window.  But that is new
> functionality that we don't have today, and the general idea is not

Well, in general, it isn't new core functionality, it is functionality
that already exists to support PCI bridges.

Choosing to use a one host bridge to N root port bridge model lets the
driver use all that functionality and the only wrinkle that becomes
visible to the PCI core as a whole is the non-compliant alignment
restriction on the bridge window BAR.

This also puts the driver in alignment with the PCI-E specs for root
complexes, which means user space can actually see things like the
PCI-E root port link capability block and makes it hot plug work
properly (I am actively using hot plug with this driver)

I personaly think this is a reasonable way to support this highly
flexible HW.

> I'm still not sure I understand what's going on here.  It sounds like
> your emulated bridge basically wraps the host bridge and makes it look
> like a PCI-PCI bridge.  But I assume the host bridge itself is also
> visible, and has apertures (I guess these are the MBus windows?)  

No, there is only one bridge, it is a per-physical-port MBUS / PCI-E
bridge. It performs an identical function to the root port bridge
described in PCI-E. MBUS serves as the root-complex internal bus 0.

There isn't 2 levels of bridging, so the MBUS / PCI-E bridge can
claim any system address and there is no such thing as a 'host
bridge'.

What Linux calls 'the host bridge aperture' is simply a wack of
otherwise unused physical address space, it has no special properties.

> It'd be nice if dmesg mentioned the host bridge explicitly as we do on
> other architectures; maybe that would help understand what's going on
> under the covers.  Maybe a longer excerpt would already have this; you
> already use pci_add_resource_offset(), which is used when creating the
> root bus, so you must have some sort of aperture before enumerating.

Well, /proc/iomem looks like this:

e0000000-efffffff : PCI MEM 0000
  e0000000-e00fffff : PCI Bus 0000:01
    e0000000-e001ffff : 0000:01:00.0

'PCI MEM 0000' is the 'host bridge aperture' it is an arbitary
range of address space that doesn't overlap anything.

'PCI Bus 0000:01' is the MBUS / PCI-E root port bridge for physical
port 0

'0000:01:00.0' is BAR 0 of an an off-chip device.

> If 01:00.0 is a PCIe endpoint, it must have a root port above it, so
> that means 00:01.0 must be the root port.  But I think you're saying
> that 00:01.0 is actually *emulated* and isn't PCIe-compliant, e.g., it
> has extra window alignment restrictions.  

It is important to understand that the emulation is only of the root
port bridge configuration space. The underlying TLP processing is done
in HW and is compliant.

> I'm scared about what other non-PCIe-compliant things there might
> be.  What happens when the PCI core configures MPS, ASPM, etc.,

As the TLP processing and the underlying PHY are all compliant these
things are all supported in HW.

MPS is supported directly by the HW

ASPM is supported by the HW, as is the entire link capability and
status block.

AER is supported directly by the HW

But here is the thing, without the software emulated config space
there would be no sane way for the Linux PCI core to access these
features. The HW simply does not present them in a way that the core
code can understand without a SW intervention of some kind.

Jason

Re: pci-mvebu driver on km_kirkwood

[Jason Gunthorpe]

On Thu, Feb 20, 2014 at 09:29:14PM +0100, Thomas Petazzoni wrote:

> In practice, the story is a little bit more subtle than that: the PCIe
> driver may want to decide to either tell the PCI core to enlarge the
> window BAR up to the next power of two size, or to dedicate two windows
> to it.

That is a smart, easy solution! Maybe that is the least invasive way
to proceed for now?

I have no idea how you decide when to round up and when to allocate
more windows, that feels like a fairly complex optimization problem!

Alternatively, I suspect you can use the PCI quirk mechanism to alter
the resource sizing on a bridge?

> Jason, would you mind maybe replying to Bjorn Helgaas email (Thu, 20
> Feb 2014 12:18:42 -0700) ? I believe that a lot of the misunderstanding
> between Bjorn and me is due to the fact that I don't use the correct
> PCI terminology to describe how the Marvell hardware works, and how the
> Marvell PCIe driver copes with it. I'm sure you would explain it in a
> way that would be more easily understood by someone very familiar with
> the PCI terminology such as Bjorn. Thanks a lot!

Done!

Hope it helps,
Jason

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Jason Gunthorpe,

On Thu, 20 Feb 2014 17:32:27 -0700, Jason Gunthorpe wrote:

> > In practice, the story is a little bit more subtle than that: the PCIe
> > driver may want to decide to either tell the PCI core to enlarge the
> > window BAR up to the next power of two size, or to dedicate two windows
> > to it.
> 
> That is a smart, easy solution! Maybe that is the least invasive way
> to proceed for now?

So you suggest that the mvebu-mbus driver should accept a
non power-of-two window size, and do internally the job of cutting that
into several power-of-two sized areas and creating the corresponding
windows?

> I have no idea how you decide when to round up and when to allocate
> more windows, that feels like a fairly complex optimization problem!

Yes, it is a fairly complex problem. I was thinking of a threshold of
"lost space". Below this threshold, it's better to enlarge the window,
above the threshold it's better to create two windows. But not easy.

> Alternatively, I suspect you can use the PCI quirk mechanism to alter
> the resource sizing on a bridge?

Can you give more details about this mechanism, and how it could be
used to alter the size of resources on a bridge?

> > Jason, would you mind maybe replying to Bjorn Helgaas email (Thu, 20
> > Feb 2014 12:18:42 -0700) ? I believe that a lot of the misunderstanding
> > between Bjorn and me is due to the fact that I don't use the correct
> > PCI terminology to describe how the Marvell hardware works, and how the
> > Marvell PCIe driver copes with it. I'm sure you would explain it in a
> > way that would be more easily understood by someone very familiar with
> > the PCI terminology such as Bjorn. Thanks a lot!
> 
> Done!

Thanks a lot! Really appreciated.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Gerlando Falauto]

Hi guys,

first of all thank you for your support and the explanations.
I'm slowly starting to understand something more about this kind of stuff.

On 02/21/2014 09:34 AM, Thomas Petazzoni wrote:
> Dear Jason Gunthorpe,
>
> On Thu, 20 Feb 2014 17:32:27 -0700, Jason Gunthorpe wrote:
>
>>> In practice, the story is a little bit more subtle than that: the PCIe
>>> driver may want to decide to either tell the PCI core to enlarge the
>>> window BAR up to the next power of two size, or to dedicate two windows
>>> to it.
>>
>> That is a smart, easy solution! Maybe that is the least invasive way
>> to proceed for now?
>
> So you suggest that the mvebu-mbus driver should accept a
> non power-of-two window size, and do internally the job of cutting that
> into several power-of-two sized areas and creating the corresponding
> windows?
>
>> I have no idea how you decide when to round up and when to allocate
>> more windows, that feels like a fairly complex optimization problem!
>
> Yes, it is a fairly complex problem. I was thinking of a threshold of
> "lost space". Below this threshold, it's better to enlarge the window,
> above the threshold it's better to create two windows. But not easy.
>
>> Alternatively, I suspect you can use the PCI quirk mechanism to alter
>> the resource sizing on a bridge?
>
> Can you give more details about this mechanism, and how it could be
> used to alter the size of resources on a bridge?

I'm not sure I understand all the details... but I guess some sort of 
rounding mechanism is indeed already in place somewhere:

pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
pci 0000:00:01.0: PCI bridge to [bus 01]

If you look at the numbers, the total size required by BAR0-5 is 
0x8805000, so around 136MB, that is 128MB+8MB+2K+1K+1K.
This gets rounded up (on this 'virtual' BAR 8) to 192MB (I don't know 
where or why), which is 1.5x a power of two (i.e. two consecutive bits 
followed by all zeroes).

If that's not just a coincidence, finding a coverage subset becomes a 
trivial matter (128MB+64MB).

In any case, even if we have an odd number like the above (0x8805000), I 
believe we could easily find a suboptimal coverage by just taking the 
most significant one and the second most significant one (possibly left 
shifted by 1 if there's a third one somewhere else).
In the above case, that would be 0x8000000 + 0x1000000. That's 
128MB+16MB, which is even smaller than the rounding above (192MB).

What do you think?

Thanks again!
Gerlando

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Gerlando Falauto,

On Fri, 21 Feb 2014 09:58:21 +0100, Gerlando Falauto wrote:

> > Can you give more details about this mechanism, and how it could be
> > used to alter the size of resources on a bridge?
> 
> I'm not sure I understand all the details... but I guess some sort of 
> rounding mechanism is indeed already in place somewhere:
> 
> pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
> pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
> pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
> pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
> pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
> pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
> pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
> pci 0000:00:01.0: PCI bridge to [bus 01]
> 
> If you look at the numbers, the total size required by BAR0-5 is 
> 0x8805000, so around 136MB, that is 128MB+8MB+2K+1K+1K.
> This gets rounded up (on this 'virtual' BAR 8) to 192MB (I don't know 
> where or why), which is 1.5x a power of two (i.e. two consecutive bits 
> followed by all zeroes).

Would indeed be interesting to know who does this rounding, and why,
and according to what rules.

> If that's not just a coincidence, finding a coverage subset becomes a 
> trivial matter (128MB+64MB).
> 
> In any case, even if we have an odd number like the above (0x8805000), I 
> believe we could easily find a suboptimal coverage by just taking the 
> most significant one and the second most significant one (possibly left 
> shifted by 1 if there's a third one somewhere else).
> In the above case, that would be 0x8000000 + 0x1000000. That's 
> 128MB+16MB, which is even smaller than the rounding above (192MB).
> 
> What do you think?

Sure, but whichever choice we make, the Linux PCI core must know by how
much we've enlarge the bridge window BAR, otherwise the Linux PCI core
may allocate for the next bridge window BAR a range of addresses that
doesn't overlap with what it has allocate for the previous bridge
window BAR, but that ends up overlapping due to us "extending" the
previous bridge window BAR to match the MBus requirements.

Gerlando, would you be able to test a quick hack that creates 2 windows
to cover exactly 128 MB + 64 MB ? This would at least allow us to
confirm that the strategy of splitting in multiple windows is usable.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Gerlando Falauto]

Hi Thomas,

On 02/21/2014 10:12 AM, Thomas Petazzoni wrote:
> Dear Gerlando Falauto,
>
> On Fri, 21 Feb 2014 09:58:21 +0100, Gerlando Falauto wrote:
>
>>> Can you give more details about this mechanism, and how it could be
>>> used to alter the size of resources on a bridge?
>>
>> I'm not sure I understand all the details... but I guess some sort of
>> rounding mechanism is indeed already in place somewhere:
>>
>> pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
>> pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
>> pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
>> pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
>> pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
>> pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
>> pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
>> pci 0000:00:01.0: PCI bridge to [bus 01]
>>
>> If you look at the numbers, the total size required by BAR0-5 is
>> 0x8805000, so around 136MB, that is 128MB+8MB+2K+1K+1K.
>> This gets rounded up (on this 'virtual' BAR 8) to 192MB (I don't know
>> where or why), which is 1.5x a power of two (i.e. two consecutive bits
>> followed by all zeroes).
>
> Would indeed be interesting to know who does this rounding, and why,
> and according to what rules.
>
>> If that's not just a coincidence, finding a coverage subset becomes a
>> trivial matter (128MB+64MB).
>>
>> In any case, even if we have an odd number like the above (0x8805000), I
>> believe we could easily find a suboptimal coverage by just taking the
>> most significant one and the second most significant one (possibly left
>> shifted by 1 if there's a third one somewhere else).
>> In the above case, that would be 0x8000000 + 0x1000000. That's
>> 128MB+16MB, which is even smaller than the rounding above (192MB).
>>
>> What do you think?
>
> Sure, but whichever choice we make, the Linux PCI core must know by how
> much we've enlarge the bridge window BAR, otherwise the Linux PCI core
> may allocate for the next bridge window BAR a range of addresses that
> doesn't overlap with what it has allocate for the previous bridge
> window BAR, but that ends up overlapping due to us "extending" the
> previous bridge window BAR to match the MBus requirements.
>
> Gerlando, would you be able to test a quick hack that creates 2 windows
> to cover exactly 128 MB + 64 MB ? This would at least allow us to
> confirm that the strategy of splitting in multiple windows is usable.

Sure, though probably not until next week.
I guess it would then also be useful to restore my previous setup, where 
the total PCIe aperture is 192MB, right?

Thank you guys!
Gerlando

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Gerlando Falauto,

On Fri, 21 Feb 2014 10:16:32 +0100, Gerlando Falauto wrote:

> > Sure, but whichever choice we make, the Linux PCI core must know by how
> > much we've enlarge the bridge window BAR, otherwise the Linux PCI core
> > may allocate for the next bridge window BAR a range of addresses that
> > doesn't overlap with what it has allocate for the previous bridge
> > window BAR, but that ends up overlapping due to us "extending" the
> > previous bridge window BAR to match the MBus requirements.
> >
> > Gerlando, would you be able to test a quick hack that creates 2 windows
> > to cover exactly 128 MB + 64 MB ? This would at least allow us to
> > confirm that the strategy of splitting in multiple windows is usable.
> 
> Sure, though probably not until next week.

No problem at all.

> I guess it would then also be useful to restore my previous setup, where 
> the total PCIe aperture is 192MB, right?

Yes, that's the case I'm interested in at the moment. If you could try
the above (ugly) patch, and see if you can access all your device BARs,
it would be interesting. It would tell us if two separate windows
having the same target/attribute and consecutive placement in the
physical address space can actually work to address a given PCIe
device. As you will see, the patch makes a very ugly special case for
192 MB :-)

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index 2394e97..f763ecc 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -223,11 +223,13 @@ static int mvebu_mbus_window_conflicts(struct mvebu_mbus_state *mbus,
                if ((u64)base < wend && end > wbase)
                        return 0;
 
+#if 0
                /*
                 * Check if target/attribute conflicts
                 */
                if (target == wtarget && attr == wattr)
                        return 0;
+#endif
        }
 
        return 1;
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 2aa7b77c..67fe6df 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -361,8 +361,15 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
                (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
                port->memwin_base;
 
-       mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
-                                   port->memwin_base, port->memwin_size);
+       if (port->memwin_size == (SZ_128M + SZ_64M)) {
+               mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
+                                           port->memwin_base, SZ_128M);
+               mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
+                                           port->memwin_base + SZ_128M, SZ_64M);
+       } else {
+               mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
+                                           port->memwin_base, port->memwin_size);
+       }
 }
 
 /*



-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Gerlando Falauto]

Hi Thomas,

On 02/21/2014 10:39 AM, Thomas Petazzoni wrote:
> Dear Gerlando Falauto,

[...]

>> I guess it would then also be useful to restore my previous setup, where
>> the total PCIe aperture is 192MB, right?
>
> Yes, that's the case I'm interested in at the moment. If you could try
> the above (ugly) patch, and see if you can access all your device BARs,
> it would be interesting. It would tell us if two separate windows
> having the same target/attribute and consecutive placement in the
> physical address space can actually work to address a given PCIe
> device. As you will see, the patch makes a very ugly special case for
> 192 MB :-)
>

So I restored the total aperture size to 192MB.
I had to rework your patch a bit because:

a) I'm running an older kernel and driver
b) sizes are actually 1-byte offset

So here it is:

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index dd4445f..27fe162 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -251,11 +251,13 @@ static int mvebu_mbus_window_conflicts(struct 
mvebu_mbus_state *mbus,
  		if ((u64)base < wend && end > wbase)
  			return 0;

+#if 0
  		/*
  		 * Check if target/attribute conflicts
  		 */
  		if (target == wtarget && attr == wattr)
  			return 0;
+#endif
  	}

  	return 1;
diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index c8397c4..120a822 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -332,10 +332,21 @@ static void 
mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
  		(((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
  		port->memwin_base;

-	mvebu_mbus_add_window_remap_flags(port->name, port->memwin_base,
-					  port->memwin_size,
-					  MVEBU_MBUS_NO_REMAP,
-					  MVEBU_MBUS_PCI_MEM);
+	if (port->memwin_size + 1 == (SZ_128M + SZ_64M)) {
+		mvebu_mbus_add_window_remap_flags(port->name, port->memwin_base,
+						  SZ_128M - 1,
+						  MVEBU_MBUS_NO_REMAP,
+						  MVEBU_MBUS_PCI_MEM);
+		mvebu_mbus_add_window_remap_flags(port->name, port->memwin_base + 
SZ_128M,
+						  SZ_64M - 1,
+						  MVEBU_MBUS_NO_REMAP,
+						  MVEBU_MBUS_PCI_MEM);
+	} else {
+		mvebu_mbus_add_window_remap_flags(port->name, port->memwin_base,
+						  port->memwin_size,
+						  MVEBU_MBUS_NO_REMAP,
+						  MVEBU_MBUS_PCI_MEM);
+	}
  }

  /*


Here's the assignment (same as before):

pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]

And here's the output I get from:

# cat /sys/kernel/debug/mvebu-mbus/devices
[00] 00000000e8000000 - 00000000ec000000 : pcie0.0 (remap 00000000e8000000)
[01] disabled
[02] disabled
[03] disabled
[04] 00000000ff000000 - 00000000ff010000 : nand
[05] 00000000f4000000 - 00000000f8000000 : vpcie
[06] 00000000fe000000 - 00000000fe010000 : dragonite
[07] 00000000e0000000 - 00000000e8000000 : pcie0.0

I did not get to test the whole address space thoroughly, but all the 
BARs are still accessible (mainly BAR0 which contains the control space 
and is mapped on the "new" MBUS window, and BAR1 which is the "big" 
one). So at least, the issues we had before are now gone.
So I'd say this looks like a very promising approach. :-)

Thank you,
Gerlando

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Gerlando Falauto,

On Fri, 21 Feb 2014 13:24:36 +0100, Gerlando Falauto wrote:

> So I restored the total aperture size to 192MB.
> I had to rework your patch a bit because:
> 
> a) I'm running an older kernel and driver
> b) sizes are actually 1-byte offset

Hum, right. This is a bit weird, maybe I should change that, I don't
think the mvebu-mbus driver should accept 1-byte offset sizes.

> Here's the assignment (same as before):
> 
> pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
> pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
> pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
> pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
> pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
> pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
> pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
> 
> And here's the output I get from:
> 
> # cat /sys/kernel/debug/mvebu-mbus/devices
> [00] 00000000e8000000 - 00000000ec000000 : pcie0.0 (remap 00000000e8000000)
> [01] disabled
> [02] disabled
> [03] disabled
> [04] 00000000ff000000 - 00000000ff010000 : nand
> [05] 00000000f4000000 - 00000000f8000000 : vpcie
> [06] 00000000fe000000 - 00000000fe010000 : dragonite
> [07] 00000000e0000000 - 00000000e8000000 : pcie0.0

This seems correct: we have two windows pointing to the same device,
and they have consecutive addresses.

> I did not get to test the whole address space thoroughly, but all the 
> BARs are still accessible (mainly BAR0 which contains the control space 
> and is mapped on the "new" MBUS window, and BAR1 which is the "big" 
> one). So at least, the issues we had before are now gone.

Did you check that what you read from BAR0 (which is mapped on the new
MBUS window) is really what you expect, and not just the same thing as
BAR1 accessible for the big window? I just want to make sure that the
hardware indeed properly handles two windows for the same device.

> So I'd say this looks like a very promising approach. :-)

Indeed. However, I don't think this approach solves the entire problem,
for two reasons:

 *) For small BARs that are not power-of-two sized, we may not want to
    consume two windows, but instead consume a little bit more address
    space. Using two windows to map a 96 KB BAR would be a waste of
    windows: using a single 128 KB window is much more efficient.

 *) I don't know if the algorithm to split the BAR into multiple
    windows is going to be trivial.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Arnd Bergmann]

On Friday 21 February 2014 14:47:08 Thomas Petazzoni wrote:
> 
> > So I'd say this looks like a very promising approach. 
> 
> Indeed. However, I don't think this approach solves the entire problem,
> for two reasons:
> 
>  *) For small BARs that are not power-of-two sized, we may not want to
>     consume two windows, but instead consume a little bit more address
>     space. Using two windows to map a 96 KB BAR would be a waste of
>     windows: using a single 128 KB window is much more efficient.

definitely.
 
>  *) I don't know if the algorithm to split the BAR into multiple
>     windows is going to be trivial.

The easiest solution would be to special case 'size is between
128MB+1 and 192MB' if that turns out to be the most interesting
case. It's easy enough to make the second window smaller than 64MB
if we want.

If we want things to be a little fancier, we could use:

	switch (size) {
		case (SZ_32M+1) ... (SZ_32M+SZ_16M):
			size2 = size - SZ_32M;
			size -= SZ_32M;
			break;
		case (SZ_64M+1) ... (SZ_64M+SZ_32M):
			size2 = size - SZ_64M;
			size -= SZ_64M;
			break;
		case (SZ_128M+1) ... (SZ_128M+SZ_64M):
			size2 = size - SZ_128M;
			size -= SZ_128M;
			break;
	};


	Arnd

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Fri, 21 Feb 2014 16:05:16 +0100, Arnd Bergmann wrote:

> >  *) I don't know if the algorithm to split the BAR into multiple
> >     windows is going to be trivial.
> 
> The easiest solution would be to special case 'size is between
> 128MB+1 and 192MB' if that turns out to be the most interesting
> case. It's easy enough to make the second window smaller than 64MB
> if we want.
> 
> If we want things to be a little fancier, we could use:
> 
> 	switch (size) {
> 		case (SZ_32M+1) ... (SZ_32M+SZ_16M):
> 			size2 = size - SZ_32M;
> 			size -= SZ_32M;
> 			break;
> 		case (SZ_64M+1) ... (SZ_64M+SZ_32M):
> 			size2 = size - SZ_64M;
> 			size -= SZ_64M;
> 			break;
> 		case (SZ_128M+1) ... (SZ_128M+SZ_64M):
> 			size2 = size - SZ_128M;
> 			size -= SZ_128M;
> 			break;
> 	};

What if the size of your BAR is 128 MB + 64 MB + 32 MB ? Then you need
three windows, and your algorithm doesn't work :-)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Arnd Bergmann]

On Friday 21 February 2014 16:11:08 Thomas Petazzoni wrote:
> On Fri, 21 Feb 2014 16:05:16 +0100, Arnd Bergmann wrote:
> 
> > >  *) I don't know if the algorithm to split the BAR into multiple
> > >     windows is going to be trivial.
> > 
> > The easiest solution would be to special case 'size is between
> > 128MB+1 and 192MB' if that turns out to be the most interesting
> > case. It's easy enough to make the second window smaller than 64MB
> > if we want.
> > 
> > If we want things to be a little fancier, we could use:
> > 
> >       switch (size) {
> >               case (SZ_32M+1) ... (SZ_32M+SZ_16M):
> >                       size2 = size - SZ_32M;
> >                       size -= SZ_32M;
> >                       break;
> >               case (SZ_64M+1) ... (SZ_64M+SZ_32M):
> >                       size2 = size - SZ_64M;
> >                       size -= SZ_64M;
> >                       break;
> >               case (SZ_128M+1) ... (SZ_128M+SZ_64M):
> >                       size2 = size - SZ_128M;
> >                       size -= SZ_128M;
> >                       break;
> >       };
> 
> What if the size of your BAR is 128 MB + 64 MB + 32 MB ? Then you need
> three windows, and your algorithm doesn't work 

I was hoping we could avoid using more than two windows.
With the algorithm above we would round up to 256MB and
fail if that doesn't fit, which is the same thing that
happens when you run out of space.

	Arnd

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Arnd Bergmann,

On Fri, 21 Feb 2014 16:20:45 +0100, Arnd Bergmann wrote:

> > What if the size of your BAR is 128 MB + 64 MB + 32 MB ? Then you need
> > three windows, and your algorithm doesn't work 
> 
> I was hoping we could avoid using more than two windows.
> With the algorithm above we would round up to 256MB and
> fail if that doesn't fit, which is the same thing that
> happens when you run out of space.

The problem is precisely that we currently don't have any well to tell
the Linux PCI core that we need to round up the size of a BAR. That's
the whole starting point of the discussion :-)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Jason Gunthorpe]

On Fri, Feb 21, 2014 at 02:47:08PM +0100, Thomas Petazzoni wrote:
>  *) I don't know if the algorithm to split the BAR into multiple
>     windows is going to be trivial.

physaddr_t base,size;

while (size != 0) {
   physaddr_t window_size = 1 << log2_round_down(size);
   create_window(base,window_size);
   base += window_size;
   size -= window_size;
}

At the very worst log2_round_down is approxmiately

unsigned int log2_round_down(unsigned int val)
{
	unsigned int res = 0;
	while ((1<<res) <= val)
		res++;
	return res - 1;
}

Minimum PCI required alignment for windows is 1MB so it will always
work out into some number of mbus windows..

Jason

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Jason Gunthorpe,

On Fri, 21 Feb 2014 09:39:02 -0700, Jason Gunthorpe wrote:
> On Fri, Feb 21, 2014 at 02:47:08PM +0100, Thomas Petazzoni wrote:
> >  *) I don't know if the algorithm to split the BAR into multiple
> >     windows is going to be trivial.
> 
> physaddr_t base,size;
> 
> while (size != 0) {
>    physaddr_t window_size = 1 << log2_round_down(size);
>    create_window(base,window_size);
>    base += window_size;
>    size -= window_size;
> }
> 
> At the very worst log2_round_down is approxmiately
> 
> unsigned int log2_round_down(unsigned int val)
> {
> 	unsigned int res = 0;
> 	while ((1<<res) <= val)
> 		res++;
> 	return res - 1;
> }
> 
> Minimum PCI required alignment for windows is 1MB so it will always
> work out into some number of mbus windows..

Interesting! Thanks!

Now I have another question: our mvebu_pcie_align_resource() function
makes sure that the base address of the BAR is aligned on its size,
because it is a requirement of MBus windows. However, if you later
split the BAR into multiple windows, will this continue to work out?

Let's take an example: a 96 MB BAR. If it gets put at 0xe0000000, then
no problem: we create one 64 MB window at 0xe0000000 and a 32 MB window
at 0xe4000000. Both base addresses are aligned on the size of the
window.

However, if the 96 MB BAR gets put at 0xea000000 (which is aligned on a
96 MB boundary, as required by our mvebu_pcie_align_resource). We
create one 64 MB window at 0xea000000, and one 32 MB window at
0xee000000. Unfortunately, while 0xea000000 is aligned on a 96 MB
boundary, it is not aligned on a 64 MB boundary, so the 64 MB window we
have created is wrong.

Which also makes me think that our mvebu_pcie_align_resource()
function uses round_up(start, size), which most likely doesn't work with
non power-of-two sizes.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Jason Gunthorpe]

On Fri, Feb 21, 2014 at 06:05:08PM +0100, Thomas Petazzoni wrote:
 
> Now I have another question: our mvebu_pcie_align_resource() function
> makes sure that the base address of the BAR is aligned on its size,
> because it is a requirement of MBus windows. However, if you later
> split the BAR into multiple windows, will this continue to work out?

No, you must align to (1 << log2_round_down(size)) - that will always
be the largest mbus window created and thus the highest starting
alignment requirement.

I looked for a bit to see if I could find why the core is rounding up
to 196MB and it wasn't clear to me either.

Gerlando, if you instrument the code in setup-bus.c, particularly
pbus_size_mem, you will probably find out.

Jason

Re: pci-mvebu driver on km_kirkwood

[Arnd Bergmann]

On Friday 21 February 2014 10:31:05 Jason Gunthorpe wrote:
> On Fri, Feb 21, 2014 at 06:05:08PM +0100, Thomas Petazzoni wrote:
>  
> > Now I have another question: our mvebu_pcie_align_resource() function
> > makes sure that the base address of the BAR is aligned on its size,
> > because it is a requirement of MBus windows. However, if you later
> > split the BAR into multiple windows, will this continue to work out?
> 
> No, you must align to (1 << log2_round_down(size)) - that will always
> be the largest mbus window created and thus the highest starting
> alignment requirement.

Unless you allow reordering the two windows. If you want a 96MB
window, you only need 32MB alignment because you can either put
the actual 64MB window first if you have 64MB alignment, or you
put the 32MB window first if you don't and then the following
64MB will be aligned.

It gets more complicated if you want to allow a 72MB window
(16MB+64MB), as that could either be 64MB aligned or start 16MB
before the next multiple of 64MB.

I don't think there is any reason why code anywhere should align
the window to a multiple of the size though if the size is not
power-of-two, such as aligning to multiples of 96MB. That wouldn't
help anyone.

	Arnd

Re: pci-mvebu driver on km_kirkwood

[Gerlando Falauto]

Dear Thomas,

On 02/21/2014 02:47 PM, Thomas Petazzoni wrote:
> Dear Gerlando Falauto,
>
> On Fri, 21 Feb 2014 13:24:36 +0100, Gerlando Falauto wrote:
>
>> So I restored the total aperture size to 192MB.
>> I had to rework your patch a bit because:
>>
>> a) I'm running an older kernel and driver
>> b) sizes are actually 1-byte offset
>
> Hum, right. This is a bit weird, maybe I should change that, I don't
> think the mvebu-mbus driver should accept 1-byte offset sizes.

I don't know anything about this, I only know the size dumped is of the 
form 0x...ffff, that's all.

>> Here's the assignment (same as before):
>>
>> pci 0000:00:01.0: BAR 8: assigned [mem 0xe0000000-0xebffffff]
>> pci 0000:01:00.0: BAR 1: assigned [mem 0xe0000000-0xe7ffffff]
>> pci 0000:01:00.0: BAR 3: assigned [mem 0xe8000000-0xe87fffff]
>> pci 0000:01:00.0: BAR 4: assigned [mem 0xe8800000-0xe8801fff]
>> pci 0000:01:00.0: BAR 0: assigned [mem 0xe8802000-0xe8802fff]
>> pci 0000:01:00.0: BAR 2: assigned [mem 0xe8803000-0xe8803fff]
>> pci 0000:01:00.0: BAR 5: assigned [mem 0xe8804000-0xe8804fff]
>>
>> And here's the output I get from:
>>
>> # cat /sys/kernel/debug/mvebu-mbus/devices
>> [00] 00000000e8000000 - 00000000ec000000 : pcie0.0 (remap 00000000e8000000)
>> [01] disabled
>> [02] disabled
>> [03] disabled
>> [04] 00000000ff000000 - 00000000ff010000 : nand
>> [05] 00000000f4000000 - 00000000f8000000 : vpcie
>> [06] 00000000fe000000 - 00000000fe010000 : dragonite
>> [07] 00000000e0000000 - 00000000e8000000 : pcie0.0
>
> This seems correct: we have two windows pointing to the same device,
> and they have consecutive addresses.

I don't know how to interpret the (remap ... ) bit, but yes, this looks 
right to me as well. I just don't know why mbus window 7 gets picked 
before 0, but apart from that, it looks nice.

>> I did not get to test the whole address space thoroughly, but all the
>> BARs are still accessible (mainly BAR0 which contains the control space
>> and is mapped on the "new" MBUS window, and BAR1 which is the "big"
>> one). So at least, the issues we had before are now gone.
>
> Did you check that what you read from BAR0 (which is mapped on the new
> MBUS window) is really what you expect, and not just the same thing as
> BAR1 accessible for the big window? I just want to make sure that the
> hardware indeed properly handles two windows for the same device.

Yes, there's no way the two BARs could be aliased. It's a fairly complex 
FPGA design, where BAR1 is the huge address space for a PCI-to-localbus 
bridge (whose connected devices are recognized correctly) and BAR0 is 
the control BAR (and its registers are read and written without a problem).


>> So I'd say this looks like a very promising approach. :-)
>
> Indeed. However, I don't think this approach solves the entire problem,
> for two reasons:
>
>   *) For small BARs that are not power-of-two sized, we may not want to
>      consume two windows, but instead consume a little bit more address
>      space. Using two windows to map a 96 KB BAR would be a waste of
>      windows: using a single 128 KB window is much more efficient.
>
>   *) I don't know if the algorithm to split the BAR into multiple
>      windows is going to be trivial.

I see others have already replied and I pretty much agree with them.

Thanks,
Gerlando

Re: pci-mvebu driver on km_kirkwood

[Gerlando Falauto]

On 02/21/2014 07:05 PM, Arnd Bergmann wrote:
> On Friday 21 February 2014 10:31:05 Jason Gunthorpe wrote:
>> On Fri, Feb 21, 2014 at 06:05:08PM +0100, Thomas Petazzoni wrote:
>>
>>> Now I have another question: our mvebu_pcie_align_resource() function
>>> makes sure that the base address of the BAR is aligned on its size,
>>> because it is a requirement of MBus windows. However, if you later
>>> split the BAR into multiple windows, will this continue to work out?
>>
>> No, you must align to (1 << log2_round_down(size)) - that will always
>> be the largest mbus window created and thus the highest starting
>> alignment requirement.
>
> Unless you allow reordering the two windows. If you want a 96MB
> window, you only need 32MB alignment because you can either put
> the actual 64MB window first if you have 64MB alignment, or you
> put the 32MB window first if you don't and then the following
> 64MB will be aligned.
>
> It gets more complicated if you want to allow a 72MB window
> (16MB+64MB), as that could either be 64MB aligned or start 16MB
> before the next multiple of 64MB.
>
> I don't think there is any reason why code anywhere should align
> the window to a multiple of the size though if the size is not
> power-of-two, such as aligning to multiples of 96MB. That wouldn't
> help anyone.

I also don't see why in the world there would be a requirement of having 
a given "oddly-sized" range (e.g. 96MB) aligned to a multiple of its 
size. In the end, AFAIK aligment requirements' only purpose is to make 
hardware simpler. Cant'see how aligning to an "odd" number would help 
achieving this purpose. But that's just me, of course.

Thanks guys!
Gerlando

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Gerlando Falauto,

On Fri, 21 Feb 2014 19:18:25 +0100, Gerlando Falauto wrote:

> > Hum, right. This is a bit weird, maybe I should change that, I don't
> > think the mvebu-mbus driver should accept 1-byte offset sizes.
> 
> I don't know anything about this, I only know the size dumped is of the 
> form 0x...ffff, that's all.

I'll have to look into this.

> >> # cat /sys/kernel/debug/mvebu-mbus/devices
> >> [00] 00000000e8000000 - 00000000ec000000 : pcie0.0 (remap 00000000e8000000)
> >> [01] disabled
> >> [02] disabled
> >> [03] disabled
> >> [04] 00000000ff000000 - 00000000ff010000 : nand
> >> [05] 00000000f4000000 - 00000000f8000000 : vpcie
> >> [06] 00000000fe000000 - 00000000fe010000 : dragonite
> >> [07] 00000000e0000000 - 00000000e8000000 : pcie0.0
> >
> > This seems correct: we have two windows pointing to the same device,
> > and they have consecutive addresses.
> 
> I don't know how to interpret the (remap ... ) bit, but yes, this looks 
> right to me as well. I just don't know why mbus window 7 gets picked 
> before 0, but apart from that, it looks nice.

Basically, some windows have an additional capability: they are
"remappable". On Kirkwood, the first 4 windows are remappable, and the
last 4 are not. Therefore, unless you request a remappable window, we
allocate a non-remappable one, which is why window 4 to 7 get used
first. And then, even though we don't need the remappable feature for
the last window, there are no more non-remappable windows available, so
window 0 gets allocated for our second PCIe window.

It matches fine with the expected behavior of the mvebu-mbus driver.

> > Did you check that what you read from BAR0 (which is mapped on the new
> > MBUS window) is really what you expect, and not just the same thing as
> > BAR1 accessible for the big window? I just want to make sure that the
> > hardware indeed properly handles two windows for the same device.
> 
> Yes, there's no way the two BARs could be aliased. It's a fairly complex 
> FPGA design, where BAR1 is the huge address space for a PCI-to-localbus 
> bridge (whose connected devices are recognized correctly) and BAR0 is 
> the control BAR (and its registers are read and written without a problem).

Great, so it means that it really works!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Bjorn Helgaas]

[+cc Gavin, Ben for EEH alignment question below]

On Thu, Feb 20, 2014 at 5:24 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Thu, Feb 20, 2014 at 12:18:42PM -0700, Bjorn Helgaas wrote:
>
>> > On Marvell hardware, the physical address space layout is configurable,
>> > through the use of "MBus windows". A "MBus window" is defined by a base
>> > address, a size, and a target device. So if the CPU needs to access a
>> > given device (such as PCIe 0.0 for example), then we need to create a
>> > "MBus window" whose size and target device match PCIe 0.0.
> ...

> So the driver creates a 'compliant' config space for the root
> port. Building the config space requires harmonizing registers related
> to the PCI-E and registers related to internal routing and dealing
> with the mismatch between what the hardware can actualy provide and
> what the PCI spec requires it provide.
> ...

>> > Since Armada XP has 10 PCIe interfaces, we cannot just statically
>> > create as many MBus windows as there are PCIe interfaces: it would both
>> > exhaust the number of MBus windows available, and also exhaust the
>> > physical address space, because we would have to create very large
>> > windows, just in case the PCIe device plugged behind this interface
>> > needs large BARs.
> ...

>> I'm still not sure I understand what's going on here.  It sounds like
>> your emulated bridge basically wraps the host bridge and makes it look
>> like a PCI-PCI bridge.  But I assume the host bridge itself is also
>> visible, and has apertures (I guess these are the MBus windows?)
>
> No, there is only one bridge, it is a per-physical-port MBUS / PCI-E
> bridge. It performs an identical function to the root port bridge
> described in PCI-E. MBUS serves as the root-complex internal bus 0.
>
> There isn't 2 levels of bridging, so the MBUS / PCI-E bridge can
> claim any system address and there is no such thing as a 'host
> bridge'.
>
> What Linux calls 'the host bridge aperture' is simply a wack of
> otherwise unused physical address space, it has no special properties.
>
>> It'd be nice if dmesg mentioned the host bridge explicitly as we do on
>> other architectures; maybe that would help understand what's going on
>> under the covers.  Maybe a longer excerpt would already have this; you
>> already use pci_add_resource_offset(), which is used when creating the
>> root bus, so you must have some sort of aperture before enumerating.
>
> Well, /proc/iomem looks like this:
>
> e0000000-efffffff : PCI MEM 0000
>   e0000000-e00fffff : PCI Bus 0000:01
>     e0000000-e001ffff : 0000:01:00.0
>
> 'PCI MEM 0000' is the 'host bridge aperture' it is an arbitary
> range of address space that doesn't overlap anything.
>
> 'PCI Bus 0000:01' is the MBUS / PCI-E root port bridge for physical
> port 0

Thanks for making this more concrete.  Let me see if I understand any better:

- e0000000-efffffff is the "host bridge aperture" but it doesn't
correspond to an actual aperture in hardware (there are no registers
where you set this range).  The only real use for this range is to be
the arena within which the PCI core can assign space to the Root
Ports.  This is static and you don't need to change it based on what
devices we discover.

- There may be several MBus/PCIe Root Ports, and you want to configure
their apertures at enumeration-time based on what devices are below
them.  As you say, the PCI core supports this except that MBus
apertures must be a power-of-two in size and aligned on their size,
while ordinary PCI bridge windows only need to start and end on 1MB
boundaries.

- e0000000-e00fffff is an example of one MBus/PCIe aperture, and this
space is available on PCI bus 01.  This one happens to be 1MB in size,
but it could be 2MB, 4MB, etc., but not 3MB like a normal bridge
window could be.

- You're currently using the ARM ->align_resource() hook (part of
pcibios_align_resource()), which is used in the bowels of the
allocator (__find_resource()) and affects the starting address of the
region we allocate, but not the size.  So you can force the start of
an MBus aperture to be power-of-two aligned, but not the end.

The allocate_resource() alignf argument is only used by PCI and
PCMCIA, so it doesn't seem like it would be too terrible to extend the
alignf interface so it could control the size, too.  Would something
like that solve this problem?

I first wondered if you could use pcibios_window_alignment(), but it
doesn't know the amount of space we need below the bridge, and it also
can't affect the size of the window or the ending address, so I don't
think it will help.

But I wonder if powerpc has a similar issue here: I think EEH might
need, for example 16MB bridge window alignment.  Since
pcibios_window_alignment() only affects the *starting* address, could
the core assign a 9MB window whose starting address is 16MB-aligned?
Could EEH deal with that?  What if the PCI core assigned the space
right after the 9MB window to another device?

Bjorn

Re: pci-mvebu driver on km_kirkwood

[Thomas Petazzoni]

Dear Bjorn Helgaas,

On Fri, 21 Feb 2014 12:05:49 -0700, Bjorn Helgaas wrote:

> Thanks for making this more concrete.  Let me see if I understand any better:

Good to see that Jason Gunthorpe could explain this in better words.
I'll try to answer to your questions below, I'm sure Jason will correct
me if I say incorrect things, or things that are imprecise.

> - e0000000-efffffff is the "host bridge aperture" but it doesn't
> correspond to an actual aperture in hardware (there are no registers
> where you set this range).  The only real use for this range is to be
> the arena within which the PCI core can assign space to the Root
> Ports.  This is static and you don't need to change it based on what
> devices we discover.

Correct. We don't configure this in any hardware register. We just give
this aperture to the Linux PCI core to tell it "please allocate all
BAR physical ranges from this global aperture".

> - There may be several MBus/PCIe Root Ports, and you want to configure
> their apertures at enumeration-time based on what devices are below
> them.  As you say, the PCI core supports this except that MBus
> apertures must be a power-of-two in size and aligned on their size,
> while ordinary PCI bridge windows only need to start and end on 1MB
> boundaries.

Exactly.

> - e0000000-e00fffff is an example of one MBus/PCIe aperture, and this
> space is available on PCI bus 01.  This one happens to be 1MB in size,
> but it could be 2MB, 4MB, etc., but not 3MB like a normal bridge
> window could be.

Absolutely.

Note that we have the possibility of mapping a 3 MB BAR, by using a 2
MB window followed by a 1 MB window. However, since the number of
windows is limited (8 on Kirkwood, 20 on Armada 370/XP), we will prefer
to enlarge the BAR size if it's size is fairly small, and only resort
to using multiple windows if the amount of lost physical space is big.

So, for a 3 MB BAR, we will definitely prefer to extend it to a single 4
MB window, because losing 1 MB of physical address space is preferable
over losing one window.

For a 192 MB BAR, we may prefer to use one 128 MB window followed by
one 64 MB window.

But as long as the pci-mvebu driver can control the size of the BAR, it
can decide on its own whether its prefers enlarging the BAR, or using
multiple windows.

> - You're currently using the ARM ->align_resource() hook (part of
> pcibios_align_resource()), which is used in the bowels of the
> allocator (__find_resource()) and affects the starting address of the
> region we allocate, but not the size.  So you can force the start of
> an MBus aperture to be power-of-two aligned, but not the end.

Correct.

Happy to see that we've managed to get an understanding on what the
problem.

> The allocate_resource() alignf argument is only used by PCI and
> PCMCIA, so it doesn't seem like it would be too terrible to extend the
> alignf interface so it could control the size, too.  Would something
> like that solve this problem?

I don't know, I would have to look more precisely into this alignf
argument, and see how it could be extended to solve our constraints.

> I first wondered if you could use pcibios_window_alignment(), but it
> doesn't know the amount of space we need below the bridge, and it also
> can't affect the size of the window or the ending address, so I don't
> think it will help.
> 
> But I wonder if powerpc has a similar issue here: I think EEH might
> need, for example 16MB bridge window alignment.  Since
> pcibios_window_alignment() only affects the *starting* address, could
> the core assign a 9MB window whose starting address is 16MB-aligned?
> Could EEH deal with that?  What if the PCI core assigned the space
> right after the 9MB window to another device?

I'll let the other PCI people answer this :-)

Thanks a lot for your feedback!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: pci-mvebu driver on km_kirkwood

[Benjamin Herrenschmidt]

On Fri, 2014-02-21 at 12:05 -0700, Bjorn Helgaas wrote:
> But I wonder if powerpc has a similar issue here: I think EEH might
> need, for example 16MB bridge window alignment.  Since
> pcibios_window_alignment() only affects the *starting* address, could
> the core assign a 9MB window whose starting address is 16MB-aligned?
> Could EEH deal with that?  What if the PCI core assigned the space
> right after the 9MB window to another device?

Gavin, did you guys deal with that at all ? Are we aligning the size as
well somewhat ?

Cheers,
Ben.