PCI IO resource question.

Quoted 1 line

keystone-pcie 21021000.pcie: error -22: failed to map resource [io 0x0000-0x400000003fff]

Bjorn,

Keystone PCI h/w doesn't support IO access in the inbound direction as stated in the hardware spec cut-n-pasted below.

• No support for IO access in inbound direction in RC or EP mode

Way back when the driver was ported, I vaguely recollect I had to add a dummy IO range to satisfy PCI Driver core. I am assuming the first entry in the DTS is for this IO port access. Is it true?

arch/arm/boot/dts/k2e.dtsi

                       ranges = <0x81000000 0 0 0x23260000 0x4000 0x4000
                                 0x82000000 0 0x60000000 0x60000000 0 0x10000000>;

I see following boot log when I boot with 4.4.x kernel

Quoted 1 line

Quoted 1 line

keystone-pcie 21021000.pcie: error -22: failed to map resource [io 0x0000-0x400000003fff]

I need to fix the above entry to

(quoted diff) +1 -1

-                       ranges = <0x81000000 0 0 0x23260000 0x4000 0x4000
+                       ranges = <0x81000000 0 0 0x23260000 0x0 0x4000
                                0x82000000 0 0x60000000 0x60000000 0 0x10000000>;

to remove this error message.

However I am wondering if it is mandatory to define this range at all. I tried to remove the entry and I get following error then

[    0.448772] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    0.448783] pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
[    0.448792] pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
[    0.448800] pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]

[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]
[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]

[    0.431879] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[    0.431906] pci 0000:00:00.0: [104c:b009] type 01 class 0x060400
[    0.432173] PCI: bus0: Fast back to back transfers disabled
[    0.432334] pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601
[    0.432382] pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007]
[    0.432402] pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043]
[    0.432420] pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107]
[    0.432439] pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143]
[    0.432457] pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]
[    0.432476] pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff]
[    0.432495] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]
[    0.432561] pci 0000:01:00.0: PME# supported from D3hot
[    0.448635] PCI: bus1: Fast back to back transfers disabled
[    0.448717] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    0.448729] pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
[    0.448738] pci 0000:00:00.0: BAR 7: assigned [io  0x1000-0x1fff]
[    0.448751] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448760] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448772] pci 0000:01:00.0: BAR 4: assigned [io  0x1000-0x100f]
[    0.448784] pci 0000:01:00.0: BAR 0: assigned [io  0x1010-0x1017]
[    0.448796] pci 0000:01:00.0: BAR 2: assigned [io  0x1018-0x101f]
[    0.448808] pci 0000:01:00.0: BAR 1: assigned [io  0x1020-0x1023]
[    0.448820] pci 0000:01:00.0: BAR 3: assigned [io  0x1024-0x1027]
[    0.448833] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.448842] pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
[    0.448851] pci 0000:00:00.0:   bridge window [mem 0x60000000-0x600fffff]
[    0.448860] pci 0000:00:00.0:   bridge window [mem 0x60100000-0x601fffff pref]
[    0.449077] pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
[    0.449086] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[    0.449095] pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
[    0.449240] aer 0000:00:00.0:pcie02: service driver aer loaded

Thanks and regards,

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

However I am wondering if it is mandatory to define this range at all. I tried to remove the entry and I get following error then

[    0.448772] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    0.448783] pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
[    0.448792] pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
[    0.448800] pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]
[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]

[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]
[    0.431879] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[    0.431906] pci 0000:00:00.0: [104c:b009] type 01 class 0x060400
[    0.432173] PCI: bus0: Fast back to back transfers disabled
[    0.432334] pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601
[    0.432382] pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007]
[    0.432402] pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043]
[    0.432420] pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107]
[    0.432439] pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143]
[    0.432457] pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]
[    0.432476] pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff]
[    0.432495] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]
[    0.432561] pci 0000:01:00.0: PME# supported from D3hot
[    0.448635] PCI: bus1: Fast back to back transfers disabled
[    0.448717] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    0.448729] pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
[    0.448738] pci 0000:00:00.0: BAR 7: assigned [io  0x1000-0x1fff]
[    0.448751] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448760] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448772] pci 0000:01:00.0: BAR 4: assigned [io  0x1000-0x100f]
[    0.448784] pci 0000:01:00.0: BAR 0: assigned [io  0x1010-0x1017]
[    0.448796] pci 0000:01:00.0: BAR 2: assigned [io  0x1018-0x101f]
[    0.448808] pci 0000:01:00.0: BAR 1: assigned [io  0x1020-0x1023]
[    0.448820] pci 0000:01:00.0: BAR 3: assigned [io  0x1024-0x1027]
[    0.448833] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.448842] pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
[    0.448851] pci 0000:00:00.0:   bridge window [mem 0x60000000-0x600fffff]
[    0.448860] pci 0000:00:00.0:   bridge window [mem 0x60100000-0x601fffff pref]
[    0.449077] pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
[    0.449086] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[    0.449095] pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
[    0.449240] aer 0000:00:00.0:pcie02: service driver aer loaded

Thanks and regards,

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Obviously if the host bridge doesn't support I/O port space, we will be unable to assign space for I/O BARs, so you will see errors like this.

We may be able to improve the message and/or make this less noisy. Guenter Roeck looked at a similar issue a while ago, but it's not completely trivial:

http://lkml.kernel.org/r/[email protected]

The PCI core should check in pci_enable_device() whether all the device BARs have been assigned. If not, it should fail. But if a driver doesn't need I/O space, it can use pci_enable_device_mem() to indicate that it only needs the MEM BARs. That should succeed even if the I/O BARs aren't assigned.

Bottom line, if you omit I/O space on your host bridge:

- You will see annoying "no space for" and "failed to assign" messages
- Drivers that don't need I/O ports should still work
- It's far better to have the messages than it was to pretend that the host bridge supported I/O space when it really didn't.

Quoted 19 lines

[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

Quoted 9 lines

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]
[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]

This range is obviously bogus, since it's way too big and not a nice round size. I guess this is what you're fixing.

Quoted 43 lines

[    0.431879] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[    0.431906] pci 0000:00:00.0: [104c:b009] type 01 class 0x060400
[    0.432173] PCI: bus0: Fast back to back transfers disabled
[    0.432334] pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601
[    0.432382] pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007]
[    0.432402] pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043]
[    0.432420] pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107]
[    0.432439] pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143]
[    0.432457] pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]
[    0.432476] pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff]
[    0.432495] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]
[    0.432561] pci 0000:01:00.0: PME# supported from D3hot
[    0.448635] PCI: bus1: Fast back to back transfers disabled
[    0.448717] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    0.448729] pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
[    0.448738] pci 0000:00:00.0: BAR 7: assigned [io  0x1000-0x1fff]
[    0.448751] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448760] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448772] pci 0000:01:00.0: BAR 4: assigned [io  0x1000-0x100f]
[    0.448784] pci 0000:01:00.0: BAR 0: assigned [io  0x1010-0x1017]
[    0.448796] pci 0000:01:00.0: BAR 2: assigned [io  0x1018-0x101f]
[    0.448808] pci 0000:01:00.0: BAR 1: assigned [io  0x1020-0x1023]
[    0.448820] pci 0000:01:00.0: BAR 3: assigned [io  0x1024-0x1027]
[    0.448833] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.448842] pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
[    0.448851] pci 0000:00:00.0:   bridge window [mem 0x60000000-0x600fffff]
[    0.448860] pci 0000:00:00.0:   bridge window [mem 0x60100000-0x601fffff pref]
[    0.449077] pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
[    0.449086] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[    0.449095] pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
[    0.449240] aer 0000:00:00.0:pcie02: service driver aer loaded

Thanks and regards,

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bjorn,

Thanks for your quick response! Please see below some clarification and follow up question.

On 03/16/2016 12:45 PM, Bjorn Helgaas wrote:
0x1000]

Quoted 53 lines

Obviously if the host bridge doesn't support I/O port space, we will be unable to assign space for I/O BARs, so you will see errors like this.

We may be able to improve the message and/or make this less noisy. Guenter Roeck looked at a similar issue a while ago, but it's not completely trivial:

http://lkml.kernel.org/r/[email protected]

The PCI core should check in pci_enable_device() whether all the device BARs have been assigned. If not, it should fail. But if a driver doesn't need I/O space, it can use pci_enable_device_mem() to indicate that it only needs the MEM BARs. That should succeed even if the I/O BARs aren't assigned.

Bottom line, if you omit I/O space on your host bridge:

- You will see annoying "no space for" and "failed to assign" messages
- Drivers that don't need I/O ports should still work
- It's far better to have the messages than it was to pretend that the host bridge supported I/O space when it really didn't.

Quoted 28 lines

[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]
[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]

Quoted 2 lines

This range is obviously bogus, since it's way too big and not a nice round size. I guess this is what you're fixing.

Yes. But from your response, I gather it is better to remove the bogus range. I removed the range, and did a read/write test to the hard drive connected to the Marvel SATA that is hooked to the PCIe interface and it still work without issues.

Another thing to worry about is the customers who are using custom fpga pci devices connected to the pcie bus and presently using pci_enable_device() in their driver. So I guess if they fix their driver to use pci_enable_device_mem() instead, it should continue to work without issues, right?

Quoted 6 lines · Murali Karicheri

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]

This unearthed an issue that we've got to fix. If the IO space remap fails (pci_remap_iospace()) we must remove the IO resource from the host bridge resources list, code below shows that we end up assigning IO space even if the remapping of the host bridge CPU physical address corresponding to IO space fails, which is wrong.

Do you want me to fix the host bridges concerned in one go ?

Quoted 63 lines · Murali Karicheri

Bjorn,

Thanks for your quick response! Please see below some clarification and follow up question.

On 03/16/2016 12:45 PM, Bjorn Helgaas wrote:
0x1000]

Quoted 55 lines

Obviously if the host bridge doesn't support I/O port space, we will be unable to assign space for I/O BARs, so you will see errors like this.

We may be able to improve the message and/or make this less noisy. Guenter Roeck looked at a similar issue a while ago, but it's not completely trivial:

http://lkml.kernel.org/r/[email protected]

The PCI core should check in pci_enable_device() whether all the device BARs have been assigned. If not, it should fail. But if a driver doesn't need I/O space, it can use pci_enable_device_mem() to indicate that it only needs the MEM BARs. That should succeed even if the I/O BARs aren't assigned.

Bottom line, if you omit I/O space on your host bridge:

- You will see annoying "no space for" and "failed to assign" messages
- Drivers that don't need I/O ports should still work
- It's far better to have the messages than it was to pretend that the host bridge supported I/O space when it really didn't.

Quoted 19 lines

[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

Quoted 9 lines

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]
[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]

This range is obviously bogus, since it's way too big and not a nice round size. I guess this is what you're fixing.

Quoted 4 lines

Yes. But from your response, I gather it is better to remove the bogus range. I removed the range, and did a read/write test to the hard drive connected to the Marvel SATA that is hooked to the PCIe interface and it still work without issues.

If your bridge doesn't support I/O space, you should definitely remove the range.

I'm curious about this Marvell SATA device, though. It is this device?

pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601

  pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007] 
  pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043] 
  pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107] 
  pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143] 
  pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]

pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]

If so, it looks like it uses the drivers/ata/ahci.c driver, which uses pcim_enable_device(), which should require all BARs to be assigned. (It doesn't look like there is a pcim_enable_device_mem() variant.)

But if you're on an arm or arm64 platform and you have PCI_PROBE_ONLY set, pcibios_enable_device() doesn't check whether resources are assigned, so the problem would be masked. We're trying to remove PCI_PROBE_ONLY, or at least remove it from paths like this, so this might become a problem soon.

This might be a reason to add a pcim_enable_device_mem() interface that ahci_init_one() could use. It looks like ahci_init_one() doesn't actually depend on the I/O BAR.

Quoted 5 lines

Another thing to worry about is the customers who are using custom fpga pci devices connected to the pcie bus and presently using pci_enable_device() in their driver. So I guess if they fix their driver to use pci_enable_device_mem() instead, it should continue to work without issues, right?

Yes.

If the FPGA PCI devices don't have I/O BARs, it doesn't matter whether the driver uses pci_enable_device() or pci_enable_device_mem(). If the device has an I/O BAR, but the driver doesn't need it, the driver should use pci_enable_device_mem(). That will make it more portable.

Bjorn

On Wed, Mar 16, 2016 at 12:20:08PM -0400, Murali Karicheri wrote:

[...]

Quoted 2 lines

However I am wondering if it is mandatory to define this range at all. I tried to remove the entry and I get following error then

If your PCI host controller does not support IO space the DT must not contain a range for it, can you elaborate on "dummy IO range" please ?

If you have it in DT, the host controller driver uses it to map that range into virtual address space, depending on which CPU physical address you pass into the IO range this may be go totally wild with drivers poking at random (ie your dummy range) CPU physical addresses in the worst case.

Remove the IO range if the PCI host controller does not support it, you must not add one to make the kernel happy, that's potentially a very dangerous thing to do.

Quoted 23 lines

[    0.448772] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    0.448783] pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
[    0.448792] pci 0000:00:00.0: BAR 7: no space for [io  size 0x1000]
[    0.448800] pci 0000:00:00.0: BAR 7: failed to assign [io  size 0x1000]
[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

Quoted 6 lines

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]

This unearthed an issue that we've got to fix. If the IO space remap fails (pci_remap_iospace()) we must remove the IO resource from the host bridge resources list, code below shows that we end up assigning IO space even if the remapping of the host bridge CPU physical address corresponding to IO space fails, which is wrong.

Do you want me to fix the host bridges concerned in one go ?

Thanks,
Lorenzo

Quoted 46 lines

[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]
[    0.431879] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[    0.431906] pci 0000:00:00.0: [104c:b009] type 01 class 0x060400
[    0.432173] PCI: bus0: Fast back to back transfers disabled
[    0.432334] pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601
[    0.432382] pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007]
[    0.432402] pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043]
[    0.432420] pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107]
[    0.432439] pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143]
[    0.432457] pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]
[    0.432476] pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff]
[    0.432495] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]
[    0.432561] pci 0000:01:00.0: PME# supported from D3hot
[    0.448635] PCI: bus1: Fast back to back transfers disabled
[    0.448717] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    0.448729] pci 0000:00:00.0: BAR 9: assigned [mem 0x60100000-0x601fffff pref]
[    0.448738] pci 0000:00:00.0: BAR 7: assigned [io  0x1000-0x1fff]
[    0.448751] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448760] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448772] pci 0000:01:00.0: BAR 4: assigned [io  0x1000-0x100f]
[    0.448784] pci 0000:01:00.0: BAR 0: assigned [io  0x1010-0x1017]
[    0.448796] pci 0000:01:00.0: BAR 2: assigned [io  0x1018-0x101f]
[    0.448808] pci 0000:01:00.0: BAR 1: assigned [io  0x1020-0x1023]
[    0.448820] pci 0000:01:00.0: BAR 3: assigned [io  0x1024-0x1027]
[    0.448833] pci 0000:00:00.0: PCI bridge to [bus 01]
[    0.448842] pci 0000:00:00.0:   bridge window [io  0x1000-0x1fff]
[    0.448851] pci 0000:00:00.0:   bridge window [mem 0x60000000-0x600fffff]
[    0.448860] pci 0000:00:00.0:   bridge window [mem 0x60100000-0x601fffff pref]
[    0.449077] pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
[    0.449086] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
[    0.449095] pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
[    0.449240] aer 0000:00:00.0:pcie02: service driver aer loaded

Thanks and regards,

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 64 lines · Bjorn Helgaas

Quoted 63 lines · Murali Karicheri

Bjorn,

Thanks for your quick response! Please see below some clarification and follow up question.

On 03/16/2016 12:45 PM, Bjorn Helgaas wrote:
0x1000]

Quoted 55 lines

Obviously if the host bridge doesn't support I/O port space, we will be unable to assign space for I/O BARs, so you will see errors like this.

We may be able to improve the message and/or make this less noisy. Guenter Roeck looked at a similar issue a while ago, but it's not completely trivial:

http://lkml.kernel.org/r/[email protected]

The PCI core should check in pci_enable_device() whether all the device BARs have been assigned. If not, it should fail. But if a driver doesn't need I/O space, it can use pci_enable_device_mem() to indicate that it only needs the MEM BARs. That should succeed even if the I/O BARs aren't assigned.

Bottom line, if you omit I/O space on your host bridge:

- You will see annoying "no space for" and "failed to assign" messages
- Drivers that don't need I/O ports should still work
- It's far better to have the messages than it was to pretend that the host bridge supported I/O space when it really didn't.

Quoted 19 lines

[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

Quoted 9 lines

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]
[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]

This range is obviously bogus, since it's way too big and not a nice round size. I guess this is what you're fixing.

Quoted 8 lines

Quoted 4 lines

Yes. But from your response, I gather it is better to remove the bogus range. I removed the range, and did a read/write test to the hard drive connected to the Marvel SATA that is hooked to the PCIe interface and it still work without issues.

If your bridge doesn't support I/O space, you should definitely remove the range.

Ok. Will do.

Quoted 3 lines

I'm curious about this Marvell SATA device, though. It is this device?

Yes.

Quoted 6 lines

pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601

  pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007] 
  pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043] 
  pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107] 
  pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143] 
  pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]

Quoted 7 lines

pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]

If so, it looks like it uses the drivers/ata/ahci.c driver, which uses pcim_enable_device(), which should require all BARs to be assigned. (It doesn't look like there is a pcim_enable_device_mem() variant.)

What does it mean in the context of removing the IO resource DT binding? My AHCI SATA driver works fine with the IO DT bindings removed except that I see the the error log for assigning IO BAR

If it expects all resources to be assigned, then it should have failed, right? But not.

I see following

[    1.547690] ahci 0000:01:00.0: version 3.0
[    1.551833] ahci 0000:01:00.0: limiting MRRS to 256
[    1.556822] ahci 0000:01:00.0: AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
[    1.564943] ahci 0000:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 

And Then

[    1.940284] ata1: SATA link down (SStatus 0 SControl 300)
[    2.140278] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[    2.147029] ata2.00: ATA-9: WDC WD10EZEX-08M2NA0, 01.01A01, max UDMA/100
[    2.153752] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 31/32), AA
[    2.161524] ata2.00: configured for UDMA/100
[    2.165957] bounce: isa pool size: 16 pages
[    2.170289] scsi 1:0:0:0: Direct-Access     ATA      WDC WD10EZEX-08M 1A01 PQ: 0 ANSI: 5
[    2.178967] sd 1:0:0:0: [sda] 1953525168 512-byte logical blocks: (1.00 TB/932 GiB)
[    2.186632] sd 1:0:0:0: [sda] 4096-byte physical blocks
[    2.192047] sd 1:0:0:0: [sda] Write Protect is off
[    2.196835] sd 1:0:0:0: [sda] Mode Sense: 00 3a 00 00
[    2.201968] sd 1:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[    2.226610]  sda: sda1 sda2
[    2.230300] sd 1:0:0:0: [sda] Attached SCSI removable disk

Quoted 6 lines

But if you're on an arm or arm64 platform and you have PCI_PROBE_ONLY set, pcibios_enable_device() doesn't check whether resources are assigned, so the problem would be masked. We're trying to remove PCI_PROBE_ONLY, or at least remove it from paths like this, so this might become a problem soon.

Keystone is ARM v7 A15 based. The driver doesn't set PCI_PROBE_ONLY. So am expect face problem when you remove PCI_PROBE_ONLY? I guess not.

I looked at pci_enable_resources()

	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;
		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

		if (r->flags & IORESOURCE_UNSET) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not assigned\n",
				i, r);
			return -EINVAL;
		}

		if (!r->parent) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not claimed\n",
				i, r);
			return -EINVAL;
		}

I don't see the error "can't enable device: BAR %d %pR not assigned" , so it doesn't depend on IO bar as you mention below or is it in a different function?

Quoted 94 lines · Murali Karicheri

Quoted 64 lines · Bjorn Helgaas

Quoted 63 lines · Murali Karicheri

Bjorn,

Thanks for your quick response! Please see below some clarification and follow up question.

On 03/16/2016 12:45 PM, Bjorn Helgaas wrote:
0x1000]

Quoted 55 lines

Obviously if the host bridge doesn't support I/O port space, we will be unable to assign space for I/O BARs, so you will see errors like this.

We may be able to improve the message and/or make this less noisy. Guenter Roeck looked at a similar issue a while ago, but it's not completely trivial:

http://lkml.kernel.org/r/[email protected]

The PCI core should check in pci_enable_device() whether all the device BARs have been assigned. If not, it should fail. But if a driver doesn't need I/O space, it can use pci_enable_device_mem() to indicate that it only needs the MEM BARs. That should succeed even if the I/O BARs aren't assigned.

Bottom line, if you omit I/O space on your host bridge:

- You will see annoying "no space for" and "failed to assign" messages
- Drivers that don't need I/O ports should still work
- It's far better to have the messages than it was to pretend that the host bridge supported I/O space when it really didn't.

Quoted 19 lines

[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

Quoted 9 lines

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]
[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]

This range is obviously bogus, since it's way too big and not a nice round size. I guess this is what you're fixing.

Quoted 8 lines

Quoted 4 lines

Yes. But from your response, I gather it is better to remove the bogus range. I removed the range, and did a read/write test to the hard drive connected to the Marvel SATA that is hooked to the PCIe interface and it still work without issues.

If your bridge doesn't support I/O space, you should definitely remove the range.

Ok. Will do.

Quoted 3 lines

I'm curious about this Marvell SATA device, though. It is this device?

Yes.

Quoted 6 lines

pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601

  pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007] 
  pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043] 
  pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107] 
  pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143] 
  pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]

Quoted 7 lines

pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]

If so, it looks like it uses the drivers/ata/ahci.c driver, which uses pcim_enable_device(), which should require all BARs to be assigned. (It doesn't look like there is a pcim_enable_device_mem() variant.)

Quoted 6 lines

What does it mean in the context of removing the IO resource DT binding? My AHCI SATA driver works fine with the IO DT bindings removed except that I see the the error log for assigning IO BAR

If it expects all resources to be assigned, then it should have failed, right? But not.

Right, I'm expecting the SATA driver to fail when you remove the DT I/O resource binding. I want to figure out why it doesn't fail.

Quoted 33 lines

I see following

[    1.547690] ahci 0000:01:00.0: version 3.0
[    1.551833] ahci 0000:01:00.0: limiting MRRS to 256
[    1.556822] ahci 0000:01:00.0: AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
[    1.564943] ahci 0000:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 

And Then

[    1.940284] ata1: SATA link down (SStatus 0 SControl 300)
[    2.140278] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[    2.147029] ata2.00: ATA-9: WDC WD10EZEX-08M2NA0, 01.01A01, max UDMA/100
[    2.153752] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 31/32), AA
[    2.161524] ata2.00: configured for UDMA/100
[    2.165957] bounce: isa pool size: 16 pages
[    2.170289] scsi 1:0:0:0: Direct-Access     ATA      WDC WD10EZEX-08M 1A01 PQ: 0 ANSI: 5
[    2.178967] sd 1:0:0:0: [sda] 1953525168 512-byte logical blocks: (1.00 TB/932 GiB)
[    2.186632] sd 1:0:0:0: [sda] 4096-byte physical blocks
[    2.192047] sd 1:0:0:0: [sda] Write Protect is off
[    2.196835] sd 1:0:0:0: [sda] Mode Sense: 00 3a 00 00
[    2.201968] sd 1:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[    2.226610]  sda: sda1 sda2
[    2.230300] sd 1:0:0:0: [sda] Attached SCSI removable disk

Quoted 6 lines

But if you're on an arm or arm64 platform and you have PCI_PROBE_ONLY set, pcibios_enable_device() doesn't check whether resources are assigned, so the problem would be masked. We're trying to remove PCI_PROBE_ONLY, or at least remove it from paths like this, so this might become a problem soon.

Quoted 2 lines

Keystone is ARM v7 A15 based. The driver doesn't set PCI_PROBE_ONLY. So am expect face problem when you remove PCI_PROBE_ONLY? I guess not.

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Can you add a little debug code like this to verify that we're in this path?

Quoted 7 lines

I looked at pci_enable_resources()

	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

                dev_info(&dev->dev, "BAR %d %pR mask %#04x parent %p\n", i, r, mask, r->parent);

Quoted 21 lines

		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;
		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

		if (r->flags & IORESOURCE_UNSET) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not assigned\n",
				i, r);
			return -EINVAL;
		}

		if (!r->parent) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not claimed\n",
				i, r);
			return -EINVAL;
		}

I don't see the error "can't enable device: BAR %d %pR not assigned" , so it doesn't depend on IO bar as you mention below or is it in a different function?

Quoted 94 lines · Murali Karicheri

Quoted 64 lines · Bjorn Helgaas

Quoted 63 lines · Murali Karicheri

Bjorn,

Thanks for your quick response! Please see below some clarification and follow up question.

On 03/16/2016 12:45 PM, Bjorn Helgaas wrote:
0x1000]

Quoted 55 lines

Obviously if the host bridge doesn't support I/O port space, we will be unable to assign space for I/O BARs, so you will see errors like this.

We may be able to improve the message and/or make this less noisy. Guenter Roeck looked at a similar issue a while ago, but it's not completely trivial:

http://lkml.kernel.org/r/[email protected]

The PCI core should check in pci_enable_device() whether all the device BARs have been assigned. If not, it should fail. But if a driver doesn't need I/O space, it can use pci_enable_device_mem() to indicate that it only needs the MEM BARs. That should succeed even if the I/O BARs aren't assigned.

Bottom line, if you omit I/O space on your host bridge:

- You will see annoying "no space for" and "failed to assign" messages
- Drivers that don't need I/O ports should still work
- It's far better to have the messages than it was to pretend that the host bridge supported I/O space when it really didn't.

Quoted 19 lines

[    0.448813] pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref]
[    0.448822] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff]
[    0.448834] pci 0000:01:00.0: BAR 4: no space for [io  size 0x0010]
[    0.448841] pci 0000:01:00.0: BAR 4: failed to assign [io  size 0x0010]
[    0.448848] pci 0000:01:00.0: BAR 0: no space for [io  size 0x0008]
[    0.448855] pci 0000:01:00.0: BAR 0: failed to assign [io  size 0x0008]
[    0.448863] pci 0000:01:00.0: BAR 2: no space for [io  size 0x0008]
[    0.448870] pci 0000:01:00.0: BAR 2: failed to assign [io  size 0x0008]
[    0.448877] pci 0000:01:00.0: BAR 1: no space for [io  size 0x0004]
[    0.448884] pci 0000:01:00.0: BAR 1: failed to assign [io  size 0x0004]
[    0.448891] pci 0000:01:00.0: BAR 3: no space for [io  size 0x0004]
[    0.448898] pci 0000:01:00.0: BAR 3: failed to assign [io  size 0x0004]
[    0.448907] pci 0000:00:00.0: PCI bridge to [bus 01]

The original log is below and even with the error, I am able to have SATA drive function as expected over this PCIe interface.

Quoted 9 lines

[    0.420648] PCI host bridge /soc/pcie@21020000 ranges:
[    0.420659]   No bus range found for /soc/pcie@21020000, using [bus 00-ff]
[    0.420679]    IO 0x23260000..0x400023263fff -> 0x00000000
[    0.420685] Requested IO range too big, new size set to 64K
[    0.420702]   MEM 0x60000000..0x6fffffff -> 0x60000000
[    0.420713] keystone-pcie 21021000.pcie: error -22: failed to map resource [io  0x0000-0x400000003fff]
[    0.431849] keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00
[    0.431861] pci_bus 0000:00: root bus resource [bus 00-ff]
[    0.431870] pci_bus 0000:00: root bus resource [io  0x0000-0x400000003fff]

This range is obviously bogus, since it's way too big and not a nice round size. I guess this is what you're fixing.

Quoted 8 lines

Quoted 4 lines

Yes. But from your response, I gather it is better to remove the bogus range. I removed the range, and did a read/write test to the hard drive connected to the Marvel SATA that is hooked to the PCIe interface and it still work without issues.

If your bridge doesn't support I/O space, you should definitely remove the range.

Ok. Will do.

Quoted 3 lines

I'm curious about this Marvell SATA device, though. It is this device?

Yes.

Quoted 6 lines

pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601

  pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007] 
  pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043] 
  pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107] 
  pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143] 
  pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]

Quoted 7 lines

pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]

If so, it looks like it uses the drivers/ata/ahci.c driver, which uses pcim_enable_device(), which should require all BARs to be assigned. (It doesn't look like there is a pcim_enable_device_mem() variant.)

Quoted 6 lines

What does it mean in the context of removing the IO resource DT binding? My AHCI SATA driver works fine with the IO DT bindings removed except that I see the the error log for assigning IO BAR

If it expects all resources to be assigned, then it should have failed, right? But not.

Right, I'm expecting the SATA driver to fail when you remove the DT I/O resource binding. I want to figure out why it doesn't fail.

Quoted 33 lines

I see following

[    1.547690] ahci 0000:01:00.0: version 3.0
[    1.551833] ahci 0000:01:00.0: limiting MRRS to 256
[    1.556822] ahci 0000:01:00.0: AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
[    1.564943] ahci 0000:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 

And Then

[    1.940284] ata1: SATA link down (SStatus 0 SControl 300)
[    2.140278] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
[    2.147029] ata2.00: ATA-9: WDC WD10EZEX-08M2NA0, 01.01A01, max UDMA/100
[    2.153752] ata2.00: 1953525168 sectors, multi 0: LBA48 NCQ (depth 31/32), AA
[    2.161524] ata2.00: configured for UDMA/100
[    2.165957] bounce: isa pool size: 16 pages
[    2.170289] scsi 1:0:0:0: Direct-Access     ATA      WDC WD10EZEX-08M 1A01 PQ: 0 ANSI: 5
[    2.178967] sd 1:0:0:0: [sda] 1953525168 512-byte logical blocks: (1.00 TB/932 GiB)
[    2.186632] sd 1:0:0:0: [sda] 4096-byte physical blocks
[    2.192047] sd 1:0:0:0: [sda] Write Protect is off
[    2.196835] sd 1:0:0:0: [sda] Mode Sense: 00 3a 00 00
[    2.201968] sd 1:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[    2.226610]  sda: sda1 sda2
[    2.230300] sd 1:0:0:0: [sda] Attached SCSI removable disk

Quoted 6 lines

But if you're on an arm or arm64 platform and you have PCI_PROBE_ONLY set, pcibios_enable_device() doesn't check whether resources are assigned, so the problem would be masked. We're trying to remove PCI_PROBE_ONLY, or at least remove it from paths like this, so this might become a problem soon.

Quoted 2 lines

Keystone is ARM v7 A15 based. The driver doesn't set PCI_PROBE_ONLY. So am expect face problem when you remove PCI_PROBE_ONLY? I guess not.

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Can you add a little debug code like this to verify that we're in this path?

Quoted 8 lines

I looked at pci_enable_resources()

	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

                dev_info(&dev->dev, "BAR %d %pR mask %#04x parent %p\n", i, r, mask, r->parent);

Quoted 21 lines

		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;
		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

		if (r->flags & IORESOURCE_UNSET) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not assigned\n",
				i, r);
			return -EINVAL;
		}

		if (!r->parent) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not claimed\n",
				i, r);
			return -EINVAL;
		}

I don't see the error "can't enable device: BAR %d %pR not assigned" , so it doesn't depend on IO bar as you mention below or is it in a different function?

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

[    1.583608] ahci 0000:01:00.0: limiting MRRS to 256
[    1.588595] ahci 0000:01:00.0: AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
[    1.596716] ahci 0000:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 
[    1.606183] scsi host0: ahci
[    1.609448] scsi host1: ahci
[    1.612636] ata1: SATA max UDMA/133 abar m512@0x60000000 port 0x60000100 irq 82
[    1.619974] ata2: SATA max UDMA/133 abar m512@0x60000000 port 0x60000180 irq 82

Quoted 33 lines

Quoted 8 lines

I looked at pci_enable_resources()

	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

                dev_info(&dev->dev, "BAR %d %pR mask %#04x parent %p\n", i, r, mask, r->parent);

Quoted 21 lines

		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;
		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

		if (r->flags & IORESOURCE_UNSET) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not assigned\n",
				i, r);
			return -EINVAL;
		}

		if (!r->parent) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not claimed\n",
				i, r);
			return -EINVAL;
		}

I don't see the error "can't enable device: BAR %d %pR not assigned" , so it doesn't depend on IO bar as you mention below or is it in a different function?

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Not saying that's what should happen, I think that's what's happening, that's the only reason I see why you do not have pci_enable_resources() failures when you remove the IO range from the host bridge.

Quoted 38 lines

[    1.583608] ahci 0000:01:00.0: limiting MRRS to 256
[    1.588595] ahci 0000:01:00.0: AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
[    1.596716] ahci 0000:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 
[    1.606183] scsi host0: ahci
[    1.609448] scsi host1: ahci
[    1.612636] ata1: SATA max UDMA/133 abar m512@0x60000000 port 0x60000100 irq 82
[    1.619974] ata2: SATA max UDMA/133 abar m512@0x60000000 port 0x60000180 irq 82

Quoted 30 lines

Quoted 8 lines

I looked at pci_enable_resources()

	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

                dev_info(&dev->dev, "BAR %d %pR mask %#04x parent %p\n", i, r, mask, r->parent);

Quoted 19 lines

		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;
		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

		if (r->flags & IORESOURCE_UNSET) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not assigned\n",
				i, r);
			return -EINVAL;
		}

		if (!r->parent) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not claimed\n",
				i, r);
			return -EINVAL;
		}

Quoted 13 lines

Quoted 3 lines

Quoted 2 lines

I don't see the error "can't enable device: BAR %d %pR not assigned" , so it doesn't depend on IO bar as you mention below or is it in a different function?

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Not saying that's what should happen, I think that's what's happening, that's the only reason I see why you do not have pci_enable_resources() failures when you remove the IO range from the host bridge.

Lorenzo

Quoted 51 lines

[    1.583608] ahci 0000:01:00.0: limiting MRRS to 256
[    1.588595] ahci 0000:01:00.0: AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
[    1.596716] ahci 0000:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 
[    1.606183] scsi host0: ahci
[    1.609448] scsi host1: ahci
[    1.612636] ata1: SATA max UDMA/133 abar m512@0x60000000 port 0x60000100 irq 82
[    1.619974] ata2: SATA max UDMA/133 abar m512@0x60000000 port 0x60000180 irq 82

Quoted 33 lines

Quoted 8 lines

I looked at pci_enable_resources()

	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

                dev_info(&dev->dev, "BAR %d %pR mask %#04x parent %p\n", i, r, mask, r->parent);

Quoted 21 lines

		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;
		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

		if (r->flags & IORESOURCE_UNSET) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not assigned\n",
				i, r);
			return -EINVAL;
		}

		if (!r->parent) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not claimed\n",
				i, r);
			return -EINVAL;
		}

I don't see the error "can't enable device: BAR %d %pR not assigned" , so it doesn't depend on IO bar as you mention below or is it in a different function?

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 39 lines · Lorenzo Pieralisi

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

Quoted 5 lines

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Here it is

[    1.556507] ahci_init_one
[    1.559124] ahci 0000:01:00.0: version 3.0
[    1.563246] pcim_enable_device
[    1.566294] pci_enable_device
[    1.569252] pci_enable_device_flags
[    1.572766] do_pci_enable_device
[    1.575985] pcibios_enable_device 60
[    1.579551] pci_enable_resources

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

Murali

Quoted 58 lines

Not saying that's what should happen, I think that's what's happening, that's the only reason I see why you do not have pci_enable_resources() failures when you remove the IO range from the host bridge.

Lorenzo

Quoted 51 lines

[    1.583608] ahci 0000:01:00.0: limiting MRRS to 256
[    1.588595] ahci 0000:01:00.0: AHCI 0001.0000 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
[    1.596716] ahci 0000:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio slum part sxs 
[    1.606183] scsi host0: ahci
[    1.609448] scsi host1: ahci
[    1.612636] ata1: SATA max UDMA/133 abar m512@0x60000000 port 0x60000100 irq 82
[    1.619974] ata2: SATA max UDMA/133 abar m512@0x60000000 port 0x60000180 irq 82

Quoted 33 lines

Quoted 8 lines

I looked at pci_enable_resources()

	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

                dev_info(&dev->dev, "BAR %d %pR mask %#04x parent %p\n", i, r, mask, r->parent);

Quoted 21 lines

		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;
		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

		if (r->flags & IORESOURCE_UNSET) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not assigned\n",
				i, r);
			return -EINVAL;
		}

		if (!r->parent) {
			dev_err(&dev->dev, "can't enable device: BAR %d %pR not claimed\n",
				i, r);
			return -EINVAL;
		}

I don't see the error "can't enable device: BAR %d %pR not assigned" , so it doesn't depend on IO bar as you mention below or is it in a different function?

Signature · 6 lines

Murali Karicheri
Linux Kernel, Keystone
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 39 lines · Lorenzo Pieralisi

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

Quoted 5 lines

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Here it is

[    1.556507] ahci_init_one
[    1.559124] ahci 0000:01:00.0: version 3.0
[    1.563246] pcim_enable_device
[    1.566294] pci_enable_device
[    1.569252] pci_enable_device_flags
[    1.572766] do_pci_enable_device
[    1.575985] pcibios_enable_device 60
[    1.579551] pci_enable_resources

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

Quoted 57 lines · Murali Karicheri

Quoted 39 lines · Lorenzo Pieralisi

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

Quoted 5 lines

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Here it is

[    1.556507] ahci_init_one
[    1.559124] ahci 0000:01:00.0: version 3.0
[    1.563246] pcim_enable_device
[    1.566294] pci_enable_device
[    1.569252] pci_enable_device_flags
[    1.572766] do_pci_enable_device
[    1.575985] pcibios_enable_device 60
[    1.579551] pci_enable_resources

Quoted 3 lines

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

We're only getting little pieces of the story here. Can you apply the following patch and collect the entire dmesg log? I want to see:

- the root bus resources (which presumably include no I/O space)
- all the SATA resources during enumeration (which should include an I/O BAR)
- the reset_resource() call that clears the I/O BAR flags
- all the SATA resources in pci_enable_resources() (the I/O BAR should be cleared out)
- the PCI_COMMAND register values before and after pci_enable_resources()

Bjorn

drivers/pci/setup-bus.c +1 -0

index 55641a3..83e8d42 100644
@@ -211,6 +211,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
 
 static inline void reset_resource(struct resource *res)
 {
+	printk("%s: %pR\n", __func__, res);
 	res->start = 0;
 	res->end = 0;
 	res->flags = 0;

drivers/pci/setup-res.c +3 -0

index 66c4d8f..c2c45f9 100644
@@ -364,11 +364,14 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
 	old_cmd = cmd;
 
+	dev_info(&dev->dev, "%s: mask %#x old_cmd %#x\n", __func__, mask, old_cmd);
+
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		if (!(mask & (1 << i)))
 			continue;
 
 		r = &dev->resource[i];
+		dev_info(&dev->dev, "  BAR %d %pR parent %p\n", i, r, r->parent);
 
 		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
 			continue;

Quoted 111 lines · Bjorn Helgaas

Quoted 57 lines · Murali Karicheri

Quoted 39 lines · Lorenzo Pieralisi

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

Quoted 5 lines

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Here it is

[    1.556507] ahci_init_one
[    1.559124] ahci 0000:01:00.0: version 3.0
[    1.563246] pcim_enable_device
[    1.566294] pci_enable_device
[    1.569252] pci_enable_device_flags
[    1.572766] do_pci_enable_device
[    1.575985] pcibios_enable_device 60
[    1.579551] pci_enable_resources

Quoted 3 lines

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

We're only getting little pieces of the story here. Can you apply the following patch and collect the entire dmesg log? I want to see:

- the root bus resources (which presumably include no I/O space)
- all the SATA resources during enumeration (which should include an I/O BAR)
- the reset_resource() call that clears the I/O BAR flags
- all the SATA resources in pci_enable_resources() (the I/O BAR should be cleared out)
- the PCI_COMMAND register values before and after pci_enable_resources()

Bjorn

drivers/pci/setup-bus.c +1 -0

index 55641a3..83e8d42 100644
@@ -211,6 +211,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
 
 static inline void reset_resource(struct resource *res)
 {
+	printk("%s: %pR\n", __func__, res);
 	res->start = 0;
 	res->end = 0;
 	res->flags = 0;

drivers/pci/setup-res.c +3 -0

index 66c4d8f..c2c45f9 100644
@@ -364,11 +364,14 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
 	old_cmd = cmd;
 
+	dev_info(&dev->dev, "%s: mask %#x old_cmd %#x\n", __func__, mask, old_cmd);
+
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		if (!(mask & (1 << i)))
 			continue;
 
 		r = &dev->resource[i];
+		dev_info(&dev->dev, "  BAR %d %pR parent %p\n", i, r, r->parent);
 
 		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
 			continue;

@@ -394,6 +397,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
 			cmd |= PCI_COMMAND_MEMORY;
 	}
 

drivers/pci/setup-res.c int pci_enable_resources(struct pci_dev *dev, int mask) · line 403

Quoted 5 lines

drivers/pci/setup-res.c +1 -0

+	dev_info(&dev->dev, "%s: cmd %#x\n", __func__, cmd);
 	if (cmd != old_cmd) {
 		dev_info(&dev->dev, "enabling device (%04x -> %04x)\n",
 			 old_cmd, cmd);

You can see complete bootlog with above at
http://pastebin.ubuntu.com/15416575/

Quoted 112 lines · Murali Karicheri

Quoted 111 lines · Bjorn Helgaas

Quoted 57 lines · Murali Karicheri

Quoted 39 lines · Lorenzo Pieralisi

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

Quoted 5 lines

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Here it is

[    1.556507] ahci_init_one
[    1.559124] ahci 0000:01:00.0: version 3.0
[    1.563246] pcim_enable_device
[    1.566294] pci_enable_device
[    1.569252] pci_enable_device_flags
[    1.572766] do_pci_enable_device
[    1.575985] pcibios_enable_device 60
[    1.579551] pci_enable_resources

Quoted 3 lines

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

We're only getting little pieces of the story here. Can you apply the following patch and collect the entire dmesg log? I want to see:

- the root bus resources (which presumably include no I/O space)
- all the SATA resources during enumeration (which should include an I/O BAR)
- the reset_resource() call that clears the I/O BAR flags
- all the SATA resources in pci_enable_resources() (the I/O BAR should be cleared out)
- the PCI_COMMAND register values before and after pci_enable_resources()

Bjorn

drivers/pci/setup-bus.c +1 -0

index 55641a3..83e8d42 100644
@@ -211,6 +211,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
 
 static inline void reset_resource(struct resource *res)
 {
+	printk("%s: %pR\n", __func__, res);
 	res->start = 0;
 	res->end = 0;
 	res->flags = 0;

drivers/pci/setup-res.c +3 -0

index 66c4d8f..c2c45f9 100644
@@ -364,11 +364,14 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
 	old_cmd = cmd;
 
+	dev_info(&dev->dev, "%s: mask %#x old_cmd %#x\n", __func__, mask, old_cmd);
+
 	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
 		if (!(mask & (1 << i)))
 			continue;
 
 		r = &dev->resource[i];
+		dev_info(&dev->dev, "  BAR %d %pR parent %p\n", i, r, r->parent);
 
 		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
 			continue;

@@ -394,6 +397,7 @@ int pci_enable_resources(struct pci_dev *dev, int mask)
 			cmd |= PCI_COMMAND_MEMORY;
 	}
 

Quoted 7 lines

drivers/pci/setup-res.c int pci_enable_resources(struct pci_dev *dev, int mask) · line 403

Quoted 5 lines

drivers/pci/setup-res.c +1 -0

+	dev_info(&dev->dev, "%s: cmd %#x\n", __func__, cmd);
 	if (cmd != old_cmd) {
 		dev_info(&dev->dev, "enabling device (%04x -> %04x)\n",
 			 old_cmd, cmd);

You can see complete bootlog with above at
http://pastebin.ubuntu.com/15416575/

Here are the interesting parts:

keystone-pcie 21021000.pcie: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-ff] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]

No I/O space, as we expected.

pci 0000:01:00.0: [1b4b:9182] type 00 class 0x010601

  pci 0000:01:00.0: reg 0x10: [io  0x8000-0x8007]
  pci 0000:01:00.0: reg 0x14: [io  0x8040-0x8043]
  pci 0000:01:00.0: reg 0x18: [io  0x8100-0x8107]
  pci 0000:01:00.0: reg 0x1c: [io  0x8140-0x8143]
  pci 0000:01:00.0: reg 0x20: [io  0x800000-0x80000f]

pci 0000:01:00.0: reg 0x24: [mem 0x00900000-0x009001ff] pci 0000:01:00.0: reg 0x30: [mem 0xd0000000-0xd000ffff pref]

Several I/O BARs shown above.

pci 0000:01:00.0: BAR 6: assigned [mem 0x60100000-0x6010ffff pref] pci 0000:01:00.0: BAR 5: assigned [mem 0x60000000-0x600001ff] pci 0000:01:00.0: BAR 4: failed to assign [io size 0x0010]
reset_resource: [io size 0x0010]
pci 0000:01:00.0: BAR 0: failed to assign [io size 0x0008]
reset_resource: [io size 0x0008]
pci 0000:01:00.0: BAR 2: failed to assign [io size 0x0008]
reset_resource: [io size 0x0008]
pci 0000:01:00.0: BAR 1: failed to assign [io size 0x0004]
reset_resource: [io size 0x0004]
pci 0000:01:00.0: BAR 3: failed to assign [io size 0x0004]
reset_resource: [io size 0x0004]

reset_resource() shows "size 0x...." instead of the address because we set the IORESOURCE_UNSET bit when we failed to assign space. That part is fine, but then reset_resource() goes on to clear res->flags, which is not fine.

ahci 0000:01:00.0: ahci_init_one:
ahci 0000:01:00.0: version 3.0
ahci 0000:01:00.0: pcim_enable_device:
ahci 0000:01:00.0: pci_enable_device:
ahci 0000:01:00.0: pci_enable_device_flags:
ahci 0000:01:00.0: do_pci_enable_device:
ahci 0000:01:00.0: pcibios_enable_device: 60
ahci 0000:01:00.0: pci_enable_resources: mask 0x60 old_cmd 0x143
ahci 0000:01:00.0: BAR 5 [mem 0x60000000-0x600001ff] parent eb149b10
ahci 0000:01:00.0: BAR 6 [mem 0x60100000-0x6010ffff pref] parent eb149b38
ahci 0000:01:00.0: pci_enable_resources: cmd 0x143

pci_enable_device() requests all resources of type IORESOURCE_MEM | IORESOURCE_IO. pci_enable_device_flags() builds "mask" (0x60 here) based on which resources match that type. For the I/O resources, res->flags has been cleared out by reset_resource(), so only the MMIO resources (BARs 5 & 6) match, hence we have bits 5 and 6 set in "mask".

So pci_enable_resources() only looks at the MMIO resources, which are both fine. It thinks no IORESOURCE_IO resources are needed, so it doesn't turn on PCI_COMMAND_IO. Somebody (maybe firmware) had previously enabled PCI_COMMAND_IO, and we leave it enabled. This is a potential problem because those I/O BARs are still enabled and the device will respond if it receives an I/O access to those regions. This isn't a problem on your particular system because there's no way to generate I/O accesses, but it *is* a problem in general.

There are lots of things I think we should fix here. They're all in the PCI core and in drivers, not in anything Keystone-related:

- reset_resource() shouldn't clear the IORESOURCE_TYPE_BITS. This probably has implications in the rest of resource assignment.
- pci_enable_resources() probably should clear PCI_COMMAND_IO if any I/O resources are unset.
- There should be a pcim_enable_device_mem().
- ahci_init_one() and similar drivers that don't need I/O space should use pcim_enable_device_mem().

Bjorn

Quoted 39 lines · Lorenzo Pieralisi

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

Quoted 5 lines

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Here it is

[    1.556507] ahci_init_one
[    1.559124] ahci 0000:01:00.0: version 3.0
[    1.563246] pcim_enable_device
[    1.566294] pci_enable_device
[    1.569252] pci_enable_device_flags
[    1.572766] do_pci_enable_device
[    1.575985] pcibios_enable_device 60
[    1.579551] pci_enable_resources

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

Quoted 57 lines · Murali Karicheri

Quoted 39 lines · Lorenzo Pieralisi

On Thu, Mar 17, 2016 at 05:28:31PM -0400, Murali Karicheri wrote:

[...]

Quoted 22 lines

Quoted 16 lines

The only ways I see that PCI_PROBE_ONLY can be set on ARM are if you have "linux,pci-probe-only" in your DT or you boot with "pci=firmware".

I expect you're in this path:

ahci_init_one
pcim_enable_device
pci_enable_device
pci_enable_device_flags(dev, IORESOURCE_MEM | IORESOURCE_IO)
# build "bars" mask
do_pci_enable_device(dev, bars)
pcibios_enable_device
if (pci_has_flag(PCI_PROBE_ONLY))
return 0;
pci_enable_resources

Quoted 2 lines

Can you add a little debug code like this to verify that we're in this path?

Yes we are in the path.

Quoted 8 lines

[    1.557561] ahci_init_one
[    1.560214] ahci 0000:01:00.0: version 3.0
[    1.564302] pcim_enable_device
[    1.567349] pci_enable_device
[    1.570340] pci_enable_device_flags
[    1.573824] do_pci_enable_device
[    1.577042] pcibios_enable_device
[    1.580380] pci_enable_resources

So resources are actually enabled (ie PCI_PROBE_ONLY is not set) and that makes sense otherwise you would not be able to use the MEM resources anyway (ie they would not be enabled).

Quoted 5 lines

I suspect the PCI dev IO resources were reset in reset_resource() in assign_requested_resource_sorted(), hence the bar mask that is built in pci_enable_device_flags() does not contain the IO resources, it would be helpful if you can print the bar mask passed to pcibios_enable_device() (ie the mask parameter).

Here it is

[    1.556507] ahci_init_one
[    1.559124] ahci 0000:01:00.0: version 3.0
[    1.563246] pcim_enable_device
[    1.566294] pci_enable_device
[    1.569252] pci_enable_device_flags
[    1.572766] do_pci_enable_device
[    1.575985] pcibios_enable_device 60
[    1.579551] pci_enable_resources

Quoted 4 lines

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

Please, tell us more about it, what does "would like to honour" mean ?

Do they (ab)use pci=firmware bootarg just to keep FW PCIe SATA settings ?

There are two ways of setting PCI_PROBE_ONLY on ARM/ARM64 (unfortunately):

- DT chosen node
- pci=firmware (on arm 32-bit platforms only)

PCIe designware does not check the DT chosen node property, so you are telling me that some customers are abusing pci=firmware command line, that was never meant to be used on platforms other than ARM IXP2000 systems (see Documentation/kernel-parameters.txt).

Usage of PCI_PROBE_ONLY on ARM has been controversial and is completely ill-defined, so we are trying to get rid of its usage.

Now, commit ed00c83cd490 ("PCI: designware: Remove PCI_PROBE_ONLY handling") removed, after mailing list initial investigation and patch review

http://www.spinics.net/lists/linux-pci/msg48248.html

PCI_PROBE_ONLY handling from PCIe designware, which means that even if you pass pci=firmware parameter on the command line (wrongly, see above) the kernel reassigns resources and set-up PCIe settings.

Does this trigger issues on Keystone ? Or the systems that set that bootarg work even if the flag is ignored ? I want to understand and I really would like to avoid asking Bjorn to revert that commit for what looks like an abuse, I prefer finding a workaround if this is really an issue.

For the records, I am about to send a patch to remove pci=firmware to Russell patch system, so I really have to know what to expect on Keystone:

http://www.spinics.net/lists/linux-pci/msg49328.html

Please let me know, I ultimately want to implement code that on ARM carries out proper PCI resources allocation (claiming firmware and assign resources that fails to be claimed - ie they are not configured by FW properly), removing PCI_PROBE_ONLY handling is a stepping stone to get there.

Thank you for your patience,
Lorenzo

Quoted 7 lines

Quoted 4 lines

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

Please, tell us more about it, what does "would like to honour" mean ?

Quoted 7 lines

Do they (ab)use pci=firmware bootarg just to keep FW PCIe SATA settings ?

There are two ways of setting PCI_PROBE_ONLY on ARM/ARM64 (unfortunately):

- DT chosen node
- pci=firmware (on arm 32-bit platforms only)

second one. pci=firmware.

Quoted 17 lines

PCIe designware does not check the DT chosen node property, so you are telling me that some customers are abusing pci=firmware command line, that was never meant to be used on platforms other than ARM IXP2000 systems (see Documentation/kernel-parameters.txt).

Usage of PCI_PROBE_ONLY on ARM has been controversial and is completely ill-defined, so we are trying to get rid of its usage.

Now, commit ed00c83cd490 ("PCI: designware: Remove PCI_PROBE_ONLY handling") removed, after mailing list initial investigation and patch review

http://www.spinics.net/lists/linux-pci/msg48248.html

PCI_PROBE_ONLY handling from PCIe designware, which means that even if you pass pci=firmware parameter on the command line (wrongly, see above) the kernel reassigns resources and set-up PCIe settings.

Quoted 5 lines

Does this trigger issues on Keystone ? Or the systems that set that bootarg work even if the flag is ignored ? I want to understand and I really would like to avoid asking Bjorn to revert that commit for what looks like an abuse, I prefer finding a workaround if this is really an issue.

Unfortunately the customer uses an older version of kernel v3.13. The Ubuntu Linux distro runs on Keystone based board using this version of kernel. We have internal version of the PCI-keystone driver that runs on this kernel. The requirement was like this at a high level. The U-Boot requires the SATA hard drive to be initialized to load and run images from hard disk. When Linux boots up, the hard drive file system is used as rootfs.

So few changes added in Kernel PCI driver to support this.
1. Linux PCI driver assumes, the PCI Link is initialized in u-boot. So it just don't do re-initialization of the SerDes link. It checks the link status and continue initialize the PCI-Keystone RC driver.
2. Uses pci=firmware to make sure the memory BARs are not re-assigned.

Without that, the Linux kernel PCI driver got some issues and I don't have the details now as this happened almost 2 years ago.

I believe at least on K2E, this is a use case that we need to support. K2E board has a Marvel SATA controller with a SATA port that we can hook up a hard drive. I have a TODO item to bring up PCI and SATA on u-boot and boot up Linux the same way as described above. So how this use case is expected to work on ARM? I assume arch/arm/kernel/bios32.c is a pseudo bios driver to emulate BIOS specific handling PCI initialization.

I am sure this is a use case that needs to be supported. Aren't there other ARM based boards that uses SATA hard disks in u-boot to boot to Linux?

How the above use case is handled if you remove the PCI_PROBE_ONLY support?

On Mon, Mar 21, 2016 at 11:24:26AM -0400, Murali Karicheri wrote:

[...]

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

Please, tell us more about it, what does "would like to honour" mean ?

Do they (ab)use pci=firmware bootarg just to keep FW PCIe SATA settings ?

There are two ways of setting PCI_PROBE_ONLY on ARM/ARM64 (unfortunately):

Quoted 4 lines

Quoted 3 lines

- DT chosen node
- pci=firmware (on arm 32-bit platforms only)

second one. pci=firmware.

And that's an abuse that has to be stopped, I am removing that flag straight away for this precise reason.

Quoted 4 lines

Quoted 4 lines

PCIe designware does not check the DT chosen node property, so you are telling me that some customers are abusing pci=firmware command line, that was never meant to be used on platforms other than ARM IXP2000 systems (see Documentation/kernel-parameters.txt).

Quoted 3 lines

Quoted 3 lines

Usage of PCI_PROBE_ONLY on ARM has been controversial and is completely ill-defined, so we are trying to get rid of its usage.

Quoted 10 lines

Quoted 10 lines

Now, commit ed00c83cd490 ("PCI: designware: Remove PCI_PROBE_ONLY handling") removed, after mailing list initial investigation and patch review

http://www.spinics.net/lists/linux-pci/msg48248.html

PCI_PROBE_ONLY handling from PCIe designware, which means that even if you pass pci=firmware parameter on the command line (wrongly, see above) the kernel reassigns resources and set-up PCIe settings.

Quoted 5 lines

Quoted 5 lines

Does this trigger issues on Keystone ? Or the systems that set that bootarg work even if the flag is ignored ? I want to understand and I really would like to avoid asking Bjorn to revert that commit for what looks like an abuse, I prefer finding a workaround if this is really an issue.

Quoted 9 lines

Unfortunately the customer uses an older version of kernel v3.13. The Ubuntu Linux distro runs on Keystone based board using this version of kernel. We have internal version of the PCI-keystone driver that runs on this kernel. The requirement was like this at a high level. The U-Boot requires the SATA hard drive to be initialized to load and run images from hard disk. When Linux boots up, the hard drive file system is used as rootfs.

Quoted 8 lines

So few changes added in Kernel PCI driver to support this.
1. Linux PCI driver assumes, the PCI Link is initialized in u-boot. So it just don't do re-initialization of the SerDes link. It checks the link status and continue initialize the PCI-Keystone RC driver.
2. Uses pci=firmware to make sure the memory BARs are not re-assigned.

Without that, the Linux kernel PCI driver got some issues and I don't have the details now as this happened almost 2 years ago.

Ok, it is not a mainline problem then and it is not something we have to fix, other than removing that bootarg flag as soon as possible since as you describe here it can be (ab/mis)used.

Quoted 6 lines

I believe at least on K2E, this is a use case that we need to support. K2E board has a Marvel SATA controller with a SATA port that we can hook up a hard drive. I have a TODO item to bring up PCI and SATA on u-boot and boot up Linux the same way as described above. So how this use case is expected to work on ARM? I assume arch/arm/kernel/bios32.c is a pseudo bios driver to emulate BIOS specific handling PCI initialization.

PCI bios is a set of helper functions that are there as PCI arm arch back-end and to be honest I am not entirely sure I understand what you expect it to do, it certainly does not emulate any BIOS to the best of my knowledge.

Quoted 3 lines

I am sure this is a use case that needs to be supported. Aren't there other ARM based boards that uses SATA hard disks in u-boot to boot to Linux?

First off, PCI_PROBE_ONLY prevents reassigning resources and reprogramming PCIe settings for the whole PCI(e) hierarchy managed by all host controllers, it is not about one single device (and I do not even understand what's the problem you are facing), this for the records so that we are on the same page (maybe that's the only device in that system, but I thought I should mention that).

Quoted 1 line

How the above use case is handled if you remove the PCI_PROBE_ONLY support?

U-boot loads and boots the kernel in memory, quiesces the PCIe SATA device before kernel handover and the kernel carries out PCI enumeration, loads and probes the required device drivers.

Can you define precisely what the problem is please ? In particular I want to understand why, by setting the PCI_PROBE_ONLY flag you want to make sure your PCIe SATA device BARs are not reassigned.

Now, the question related to FW/OS handover of PCI resources is a complicated one and on ARM it was never solved/defined properly.

The question here, and I am not sure it can be answered with a simple answer, is how we make sure the OS does not touch PCI resources that have been initialized in FW and are not meant to be changed (whatever that means).

To at least have some hope to sort this out, the kernel must try to claim PCI resources as set-up by FW, if they are in a "reasonable" state they are claimed and not touched. For resources for which claiming fails, a reassignment policy must be implemented, that might require re-sizing bridges and release resources so that they can be effectively reallocated.

At present, on ALL ARM platforms out there (except for some oddballs and possibly some arm platforms that abused the pci=firmware parameter that I am hunting down and I am not aware of) the whole PCI resource space is always reassigned by the kernel (which does not mean that's always the right thing to do BTW, as it is it just works).

We must not try to "fix" this by misusing boot arguments and PCI_PROBE_ONLY, we must come up with a PCI resources allocation policy that implements what I mention above, my point is, PCI_PROBE_ONLY must mean the same thing for all ARM/ARM64 platforms otherwise it becomes complicated to handle in a sane way.

Does this help ? Please let us know.

Thanks,
Lorenzo

Quoted 15 lines · Lorenzo Pieralisi

On Mon, Mar 21, 2016 at 11:24:26AM -0400, Murali Karicheri wrote:

[...]

Quoted 11 lines

Quoted 11 lines

Quoted 4 lines

I know that some of our customers use PCIe SATA from u-boot and would like to honor the assignment in Linux space.. I believe they use PCI_PROBE_ONLY by setting the bootarg. So Keystone PCI should work in both cases.

Please, tell us more about it, what does "would like to honour" mean ?

Do they (ab)use pci=firmware bootarg just to keep FW PCIe SATA settings ?

There are two ways of setting PCI_PROBE_ONLY on ARM/ARM64 (unfortunately):

Quoted 8 lines

Quoted 4 lines

Quoted 3 lines

- DT chosen node
- pci=firmware (on arm 32-bit platforms only)

second one. pci=firmware.

And that's an abuse that has to be stopped, I am removing that flag straight away for this precise reason.

Let me speak up again. Keystone usage of pci=firmware was used by our customer to address a specific problem. I am assuming they used it work around the BAR re-assignment problem in Linux space in v3.13 kernel they were working with.

The kernel-parameters.txt states

firmware [ARM] Do not re-enumerate the bus but instead
just use the configuration from the bootloader. This is currently used on IXP2000 systems where the bus has to be

Probably they used it to avoid re-enumerate similar to IXP2000. But as I said before, I can't confirm the reason as this happened almost 2 years ago.

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

PCIe designware does not check the DT chosen node property, so you are telling me that some customers are abusing pci=firmware command line, that was never meant to be used on platforms other than ARM IXP2000 systems (see Documentation/kernel-parameters.txt).

If the problem is same why you blame using this approach on non IXP2000 system?

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Usage of PCI_PROBE_ONLY on ARM has been controversial and is completely ill-defined, so we are trying to get rid of its usage.

No issues. At this point, I don't have the use case I described working in our internal TI kernel based on v4.1.x or v4.4.x or upstream. However it will be interesting to find the ARM platforms out there that uses firmware option, irrespective of whether they abused its usage or not. Someone hopefully respond to your patches if it breaks these platforms.

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Now, commit ed00c83cd490 ("PCI: designware: Remove PCI_PROBE_ONLY handling") removed, after mailing list initial investigation and patch review

http://www.spinics.net/lists/linux-pci/msg48248.html

PCI_PROBE_ONLY handling from PCIe designware, which means that even if you pass pci=firmware parameter on the command line (wrongly, see above) the kernel reassigns resources and set-up PCIe settings.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Does this trigger issues on Keystone ? Or the systems that set that bootarg work even if the flag is ignored ? I want to understand and I really would like to avoid asking Bjorn to revert that commit for what looks like an abuse, I prefer finding a workaround if this is really an issue.

As I have said, I don't have the use case with pci=firmware working to help you in this.

Quoted 47 lines

Quoted 8 lines

Unfortunately the customer uses an older version of kernel v3.13. The Ubuntu Linux distro runs on Keystone based board using this version of kernel. We have internal version of the PCI-keystone driver that runs on this kernel. The requirement was like this at a high level. The U-Boot requires the SATA hard drive to be initialized to load and run images from hard disk. When Linux boots up, the hard drive file system is used as rootfs.

Quoted 8 lines

So few changes added in Kernel PCI driver to support this.
1. Linux PCI driver assumes, the PCI Link is initialized in u-boot. So it just don't do re-initialization of the SerDes link. It checks the link status and continue initialize the PCI-Keystone RC driver.
2. Uses pci=firmware to make sure the memory BARs are not re-assigned.

Without that, the Linux kernel PCI driver got some issues and I don't have the details now as this happened almost 2 years ago.

Ok, it is not a mainline problem then and it is not something we have to fix, other than removing that bootarg flag as soon as possible since as you describe here it can be (ab/mis)used.

Quoted 6 lines

I believe at least on K2E, this is a use case that we need to support. K2E board has a Marvel SATA controller with a SATA port that we can hook up a hard drive. I have a TODO item to bring up PCI and SATA on u-boot and boot up Linux the same way as described above. So how this use case is expected to work on ARM? I assume arch/arm/kernel/bios32.c is a pseudo bios driver to emulate BIOS specific handling PCI initialization.

PCI bios is a set of helper functions that are there as PCI arm arch back-end and to be honest I am not entirely sure I understand what you expect it to do, it certainly does not emulate any BIOS to the best of my knowledge.

Quoted 3 lines

I am sure this is a use case that needs to be supported. Aren't there other ARM based boards that uses SATA hard disks in u-boot to boot to Linux?

First off, PCI_PROBE_ONLY prevents reassigning resources and reprogramming PCIe settings for the whole PCI(e) hierarchy managed by all host controllers, it is not about one single device (and I do not even understand what's the problem you are facing), this for the records so that we are on the same page (maybe that's the only device in that system, but I thought I should mention that).

Quoted 1 line

How the above use case is handled if you remove the PCI_PROBE_ONLY support?

Quoted 4 lines

U-boot loads and boots the kernel in memory, quiesces the PCIe SATA device before kernel handover and the kernel carries out PCI enumeration, loads and probes the required device drivers.

I am with you on this. Will re-visit if I ever has to support PCI in u-boot to work with SATA.

Quoted 4 lines

Can you define precisely what the problem is please ? In particular I want to understand why, by setting the PCI_PROBE_ONLY flag you want to make sure your PCIe SATA device BARs are not reassigned.

No idea. As I have said, the customer did this in their custom board.

Quoted 21 lines

Now, the question related to FW/OS handover of PCI resources is a complicated one and on ARM it was never solved/defined properly.

The question here, and I am not sure it can be answered with a simple answer, is how we make sure the OS does not touch PCI resources that have been initialized in FW and are not meant to be changed (whatever that means).

To at least have some hope to sort this out, the kernel must try to claim PCI resources as set-up by FW, if they are in a "reasonable" state they are claimed and not touched. For resources for which claiming fails, a reassignment policy must be implemented, that might require re-sizing bridges and release resources so that they can be effectively reallocated.

At present, on ALL ARM platforms out there (except for some oddballs and possibly some arm platforms that abused the pci=firmware parameter that I am hunting down and I am not aware of) the whole PCI resource space is always reassigned by the kernel (which does not mean that's always the right thing to do BTW, as it is it just works).

Quoted 6 lines

We must not try to "fix" this by misusing boot arguments and PCI_PROBE_ONLY, we must come up with a PCI resources allocation policy that implements what I mention above, my point is, PCI_PROBE_ONLY must mean the same thing for all ARM/ARM64 platforms otherwise it becomes complicated to handle in a sane way.

Sounds good. Personally, I have never understood the usage of pci=firmware or PCI_PROBE_ONLY. As you have said, ideally SATA over PCI should work fine in u-boot and then again in the linux space like any other device drivers do. Will re-visit when I have to support this use case again on our board with latest kernel.