hi bjorn, Lorenzo,
should i do some testing with patch from yesterday?
please let me know if can test anything.
best regards,
Hannes
On Wed, Sep 2, 2015 at 10:32 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> Hi Lorenzo, thanks for jumping on this!
>
> On Wed, Sep 02, 2015 at 06:47:16PM +0100, Lorenzo Pieralisi wrote:
>> Hi Hannes,
>>
>> On Wed, Sep 02, 2015 at 10:51:18AM +0100, oe5hpm wrote:
>> > Hi Lorenzo,
>> >
>> > today i tried to boot up the most recent vanilla kernel on my
>> > Freescale i.mx6 board.
>> > I ran into trouble regarding PCI enumeration.
>> >
>> > [ 0.431949] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
>> > [ 0.431976] pci_bus 0000:00: root bus resource [io 0x1000-0xffff]
>> > [ 0.431996] pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
>> > [ 0.432022] pci_bus 0000:00: root bus resource [bus 00-ff]
>> > [ 0.433271] PCI: bus0: Fast back to back transfers disabled
>> > [ 0.433629] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
>> > [ 0.435181] PCI: bus1: Fast back to back transfers disabled
>> > [ 0.435564] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
>> > [ 0.435593] pci 0000:00:00.0: BAR 8: no space for [mem size 0x01000000]
>> > [ 0.435613] pci 0000:00:00.0: BAR 8: failed to assign [mem size 0x01000000]
>> > [ 0.435635] pci 0000:00:00.0: BAR 9: assigned [mem
>> > 0x01100000-0x011fffff pref]
>> > [ 0.435655] pci 0000:00:00.0: BAR 6: assigned [mem
>> > 0x01200000-0x0120ffff pref]
>> > [ 0.435676] pci 0000:00:00.0: BAR 7: assigned [io 0x1000-0x1fff]
>> > [ 0.435705] pci 0000:01:00.0: BAR 2: no space for [mem size 0x00200000]
>> > [ 0.435722] pci 0000:01:00.0: BAR 2: failed to assign [mem size 0x00200000]
>> > [ 0.435739] pci 0000:01:00.0: BAR 1: no space for [mem size 0x00004000]
>> > [ 0.435754] pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x00004000]
>> > [ 0.435770] pci 0000:01:00.0: BAR 0: no space for [mem size 0x00000100]
>> > [ 0.435786] pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00000100]
>> > [ 0.435804] pci 0000:00:00.0: PCI bridge to [bus 01]
>> > [ 0.435826] pci 0000:00:00.0: bridge window [io 0x1000-0x1fff]
>> > [ 0.435855] pci 0000:00:00.0: bridge window [mem
>> > 0x01100000-0x011fffff pref]
>> >
>> > there are several fails assigning memory ressources to pci-devices.
>> >
>> > i bisect down this trouble to commit id:
>> > dff22d2054b5dbb1889f20c03959dd0c494fab8c : PCI: Call
>> > pci_read_bridge_bases() from core instead of arch code
>> >
>> > For testing purpose i've reverted this commit on a local branch and
>> > everythings works fine, as before.
>> >
>> > [ 0.431976] imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
>> > [ 0.432004] pci_bus 0000:00: root bus resource [io 0x1000-0xffff]
>> > [ 0.432023] pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
>> > [ 0.432047] pci_bus 0000:00: root bus resource [bus 00-ff]
>> > [ 0.433302] PCI: bus0: Fast back to back transfers disabled
>> > [ 0.435122] PCI: bus1: Fast back to back transfers disabled
>> > [ 0.435504] pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
>> > [ 0.435535] pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x013fffff]
>> > [ 0.435557] pci 0000:00:00.0: BAR 6: assigned [mem
>> > 0x01400000-0x0140ffff pref]
>> > [ 0.435585] pci 0000:01:00.0: BAR 2: assigned [mem 0x01200000-0x013fffff]
>> > [ 0.435626] pci 0000:01:00.0: BAR 1: assigned [mem 0x01100000-0x01103fff]
>> > [ 0.435665] pci 0000:01:00.0: BAR 0: assigned [mem 0x01104000-0x011040ff]
>> > [ 0.435703] pci 0000:00:00.0: PCI bridge to [bus 01]
>> > [ 0.435728] pci 0000:00:00.0: bridge window [mem 0x01100000-0x013fffff]
>> >
>> > Further i can break down the failure to "drivers/pci/probe.c" line #924.
>> > If i comment out the "pci_read_bridge_bases(child);" also everything works well.
>> >
>> > I have to confess, that my knowledge about the whole PCI thing in the
>> > kernel is not very deep, so it is not possible for me to figure out
>> > what is going wrong.
>>
>> It looks like a bogus bridge aperture size is causing this to happen,
>> and this prevents reassignment on arm (bridge aperture is too big),
>> which proves that reading the bridge bases without vetting the corresponding
>> resources may break (on platforms that were not reading them before).
>>
>> arm was the only platform not reading the bridge bases, here is an
>> answer why. So, to prevent reverting the commit I put together this
>> patch (to be reworked if we deem it reasonable), subject to discussion
>> (I fear it may end up breaking other arm platforms, I do not have all
>> ARM boards and required host controllers to test, I managed to test it on
>> an iMX6 Sabrelite though).
>>
>> Here, please let me know if it works for you, I will keep on thinking
>> to find the best solution.
>>
>> I will have to do this for arm64 too, comments very welcome.
>>
>> Thanks,
>> Lorenzo
>>
>> -- >8 --
>> Subject: [PATCH] arm: kernel: pci: fixup erroneous PCI bridge apertures
>>
>> Bridge apertures read by core PCI code through pci_read_bridge_bases()
>> might be erroneous (bogus platform setup). If the arch code does not vet
>> the bridge resources (ie by trying to claim them), we can end up in a
>> situation where wrong bridge apertures can prevent resources assignment
>> for downstream devices causing enumeration failures (eg a bridge
>> aperture does not fit in the respective host controller resource window,
>> so it can't be assigned).
>>
>> This patch adds arm arch code that vets bridge resources by trying
>> to claim them, and reset them on claiming failure so that they can
>> be properly reassigned.
>
> We definitely should not depend on the platform to set up the bridge
> windows. Do we know what the platform left in the 00:00.0 window
> registers?
>
> I see that bus 01 requires 0x204100 of mem space, which must be
> rounded up to a megabyte boundary, so the window must be at least 3M
> (0x00300000):
>
> pci 0000:01:00.0: BAR 2: failed to assign [mem size 0x00200000]
> pci 0000:01:00.0: BAR 1: failed to assign [mem size 0x00004000]
> pci 0000:01:00.0: BAR 0: failed to assign [mem size 0x00000100]
>
> I don't understand the connection with dff22d2054b5 yet. If we don't
> call pci_read_bridge_bases(), apparently some assign-resources path
> figures out the required size and assigns a 3M window.
>
> If we *do* call pci_read_bridge_bases(), do we read a bogus 16M window
> size, fail to assign that because the host controller window isn't big
> enough, and then the assign-resources path just gives up? I assume
> clearing r->flags in your patch is the critical thing? Is there
> something in assign-resources that checks for r->flags == 0?
>
> I think it would be ideal if we could someday claim the resource
> immediately, as soon as we read it from a BAR or bridge window, and
> mark it as IORESOURCE_UNSET if claiming it fails. Then if the
> platform set up reasonable windows, we could use them; if it didn't,
> we could just assign our own.
>
> I'd like to avoid adding things to pcibios_fixup_bus() if possible
> because most of what is done there is arch-independent, and I'd like
> to get the arch-independent stuff into the PCI core.
>
> Bjorn
>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
>> ---
>> arch/arm/kernel/bios32.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
>> index 874e182..ebbe052 100644
>> --- a/arch/arm/kernel/bios32.c
>> +++ b/arch/arm/kernel/bios32.c
>> @@ -282,6 +282,27 @@ static inline int pdev_bad_for_parity(struct pci_dev *dev)
>>
>> }
>>
>> +static void pcibios_fixup_bridge_resources(struct pci_dev *dev)
>> +{
>> + int idx;
>> +
>> + if (!dev->bus)
>> + return;
>> +
>> + for (idx = PCI_BRIDGE_RESOURCES; idx < PCI_NUM_RESOURCES; idx++) {
>> + struct resource *r = &dev->resource[idx];
>> +
>> + if (!r->flags || r->parent)
>> + continue;
>> +
>> + if (pci_claim_resource(dev, idx)) {
>> + r->flags = 0;
>> + r->start = 0;
>> + r->end = -1;
>> + }
>> + }
>> +}
>> +
>> /*
>> * pcibios_fixup_bus - Called after each bus is probed,
>> * but before its children are examined.
>> @@ -352,6 +373,9 @@ void pcibios_fixup_bus(struct pci_bus *bus)
>> bus->bridge_ctl |= PCI_BRIDGE_CTL_PARITY;
>> }
>>
>> + if (bus->self)
>> + pcibios_fixup_bridge_resources(bus->self);
>> +
>> /*
>> * Report what we did for this bus
>> */
>> --
>> 1.9.1
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
