Add ofw interface support to PCI

[kd]

Summary

Some arm64 SoCs have nodes in their fdts that describe devices connected to the internal PCI bus.
One such SoC is Freescale LS1028A.
In order to access information stored in them we need to add ofw bus support to pci.
Pass devinfo request up to our parent, which is responsible for parsing all the information.
It allows to use ofw interface on PCI devices that support it.
This method is similar to sys/dev/acpica/acpi_pci.c.

[andrew]

This should be a sub class of the existing pci driver, similar to how it's done in sys/dev/acpica/acpi_pci.c

[kd]

In D30181#677797, @andrew wrote:

This should be a sub class of the existing pci driver, similar to how it's done in sys/dev/acpica/acpi_pci.c

Correct, I'll rewrite this patch and update this diff.

[kd]

HI @andrew,

Any thoughts regarding the updated version of this diff?

[andrew]

sys/dev/ofw/ofw_pci.c

80

Would this device get an FDT node? If so we should check for that.

[kd]

sys/dev/ofw/ofw_pci.c

80

Unfortunately we don't get an FDT note for this one.
From kernel perspective it looks this:

(...) -> pcib0(pci_host_generic_fdt) -> pci0 (this driver) -> device.

We only get FDT nodes for pcib0 and devices found on the bus.

[andrew]

sys/dev/ofw/ofw_pci.c

80

Will this driver work correctly if it's a child of PCI-PCI bridge? e.g.

(...) -> pcib0(pci_host_generic_fdt) -> pci0 -> pcib1(PCI-PCI bridge) -> pcib1(this driver)

[kd]

sys/dev/ofw/ofw_pci.c

80

It won't. With a PCI-PCI bridge we'll hit ofw_bus_default_get_devinfo, which will case the ofw bus routines to fail.
At the same time I don't think this is really a problem.
From FDT perspective PCI device nodes are direct children of the PCI bus node.
AFAIK currently there is no SoC that uses FDT to describe devices attached to a PCI-PCI bridge.

[andrew]

sys/dev/ofw/ofw_pci.c

80

In that case we should check if the parent device has an OFW node here. We have platforms that could use FDT and contain a PCI-PCI bridge. We also sometimes need to boot with a DTB for development reasons, e.g. the ACPI bindings aren't ready.

83

This should be OFW PCI bus as it's not FDT specific.

[kd]

Check if our parent supports ofw interface in ofw_pci_probe.

[andrew]

sys/dev/ofw/ofw_pci.c

81

This could also return 0 on error, e.g. from ofw_bus_gen_get_node.

[mmel]

Unfortunately this broke (probably) all existing FDT enabled boards with enumerated (not FDT loaded) PCI(e) interfaces (in my case both mcbin and tegra). This driver probe() matches all PCI buses, and many things (driver, ofw code) depend on the fact that calling ofw_bus_get_node(dev) on a non-FDT installed device returns zero.

[kd]

In D30181#691515, @mmel wrote:

Unfortunately this broke (probably) all existing FDT enabled boards with enumerated (not FDT loaded) PCI(e) interfaces (in my case both mcbin and tegra). This driver probe() matches all PCI buses, and many things (driver, ofw code) depend on the fact that calling ofw_bus_get_node(dev) on a non-FDT installed device returns zero.

Are sure this commit is the root cause?
There are two cases:

1. We have pci_host_generic_fdt as our parent. In that case generic_pcie_ofw_get_devinfo will be called, which will always return NULL on non-FDT devices.
2. IIRC pci_host_generic_fdt is the only pcib driver that implements ofw_bus_get_devinfo. So in all other cases we will hit the default implementation, which returns NULL.

Now ofw_bus_gen_get_node returns 0 when OFW_BUS_GET_DEVINFO returns NULL, so I think we should be fine here.

[mmel]

yes, this commit is root cause .
Please see attached log:

There's a third case: any other pcib (not pci_host_generic_fdt) is our parent. (pci_dw_mv in my case ). An attempt to call OFW_BUS_GET_DEVINFO on enumerated child bus returns devinfo of pci controller itself -> because ofw_pci is probed as bus drivers. And unlike original bus driver it passes devinfo request to parent.

In logs please note presence of pci_add_resources() -> pcib_route_interrupt -> ofw_pcib_route_interrupt() -> ofw_bus_lookup_imap() sequence.

system without D30181:

pcib0: <Marvell Armada8K PCI-E Controller> mem 0xf2600000-0xf260ffff,0xf6f00000-0xf6f7ffff irq 23 on simplebus2
pcib0: Bus is cache-coherent
pci0: <PCI bus> on pcib0
pci0: domain=0, physical bus=0
found-> vendor=0x11ab, dev=0x0110, revid=0x00
        domain=0, bus=0, slot=0, func=0
        class=06-04-00, hdrtype=0x01, mfdev=0
        cmdreg=0x0107, statreg=0x0010, cachelnsz=8 (dwords)
        lattimer=0x00 (0 ns), mingnt=0x00 (0 ns), maxlat=0x00 (0 ns)
        intpin=a, irq=255
        powerspec 3  supports D0 D1 D2 D3  current D0
        MSI supports 32 messages, 64 bit, vector masks
        MSI-X supports 1 message in maps 0x10 and 0x18
        map[10]: type Memory, range 64, base 0, size 20, enabled
        secbus=1, subbus=255
pcib1: <PCI-PCI bridge> at device 0.0 on pci0
pcib0: failed to reserve resource for pcib1
pcib1: failed to allocate initial I/O port window: 0-0xffff
pcib0: failed to reserve resource for pcib1
pcib1: failed to allocate initial memory window: 0xf6000000-0xf63fffff
pcib1:   domain            0
pcib1:   secondary bus     1
pcib1:   subordinate bus   255
pci1: <PCI bus> on pcib1
pcib1: allocated bus range (1-1) for rid 0 of pci1
pci1: domain=0, physical bus=1
found-> vendor=0x8086, dev=0x1521, revid=0x01
        domain=0, bus=1, slot=0, func=0
        class=02-00-00, hdrtype=0x00, mfdev=1
        cmdreg=0x0007, statreg=0x0010, cachelnsz=8 (dwords)
        lattimer=0x00 (0 ns), mingnt=0x00 (0 ns), maxlat=0x00 (0 ns)
        intpin=a, irq=0
        powerspec 3  supports D0 D3  current D0
        MSI supports 1 message, 64 bit, vector masks
        MSI-X supports 10 messages in map 0x1c
        map[10]: type Memory, range 32, base 0xf6000000, size 20, enabled
pcib0: failed to reserve resource for pcib1
pcib1: failed to allocate initial memory window (0xf6000000-0xf60fffff,0x100000)
pcib1: allocated initial memory window of 0xc0000000-0xc00fffff
pcib1: allocated memory range (0xc0000000-0xc00fffff) for rid 10 of pci0:1:0:0
        map[18]: type I/O Port, range 32, base 0xffffffe0, size  5, enabled
        map[1c]: type Memory, range 32, base 0xf6100000, size 14, enabled
pcib1: attempting to grow memory window for (0xf6100000-0xf6103fff,0x4000)
        back candidate range: 0xf6100000-0xf6103fff
pcib1: attempting to grow memory window for (0-0xffffffff,0x4000)
        front candidate range: 0xbfffc000-0xbfffffff
        back candidate range: 0xc0100000-0xc0103fff
pcib1: grew memory window to 0xc0000000-0xc01fffff
pcib1: allocated memory range (0xc0100000-0xc0103fff) for rid 1c of pci0:1:0:0
pcib1: ofw_pcib_route_interrupt pin:1
pcib1: slot 0 INTA is routed to irq 96
found-> vendor=0x8086, dev=0x1521, revid=0x01
        domain=0, bus=1, slot=0, func=1
        class=02-00-00, hdrtype=0x00, mfdev=1
        cmdreg=0x0007, statreg=0x0010, cachelnsz=8 (dwords)
        lattimer=0x00 (0 ns), mingnt=0x00 (0 ns), maxlat=0x00 (0 ns)
        intpin=b, irq=0
        powerspec 3  supports D0 D3  current D0
        MSI supports 1 message, 64 bit, vector masks
        MSI-X supports 10 messages in map 0x1c
        map[10]: type Memory, range 32, base 0xf6200000, size 20, enabled
pcib1: attempting to grow memory window for (0xf6200000-0xf62fffff,0x100000)
        back candidate range: 0xf6200000-0xf62fffff
pcib1: attempting to grow memory window for (0-0xffffffff,0x100000)
        front candidate range: 0xbff00000-0xbfffffff
        back candidate range: 0xc0200000-0xc02fffff
pcib1: grew memory window to 0xc0000000-0xc02fffff
pcib1: allocated memory range (0xc0200000-0xc02fffff) for rid 10 of pci0:1:0:1
        map[18]: type I/O Port, range 32, base 0xffffffe0, size  5, enabled
        map[1c]: type Memory, range 32, base 0xf6300000, size 14, enabled
pcib1: attempting to grow memory window for (0xf6300000-0xf6303fff,0x4000)
        back candidate range: 0xf6300000-0xf6303fff
pcib1: allocated memory range (0xc0104000-0xc0107fff) for rid 1c of pci0:1:0:1
pcib1: ofw_pcib_route_interrupt pin:2
pcib1: slot 0 INTB is routed to irq 97
igb0: <Intel(R) I350 (Copper)> mem 0xc0000000-0xc00fffff,0xc0100000-0xc0103fff irq 96 at device 0.0 on pci1
igb0: attach_pre capping queues at 8
ofw_pci mapdev: start c0000000, len 1048576
igb0: Using 1024 TX descriptors and 1024 RX descriptors
igb0: msix_init qsets capped at 8
ofw_pci mapdev: start c0100000, len 16384

system with D30181:

pcib0: <Marvell Armada8K PCI-E Controller> mem 0xf2600000-0xf260ffff,0xf6f00000-0xf6f7ffff irq 23 on simplebus2
pcib0: Bus is cache-coherent
pci0: <OFW PCI bus> on pcib0
pci0: domain=0, physical bus=0
found-> vendor=0x11ab, dev=0x0110, revid=0x00
        domain=0, bus=0, slot=0, func=0
        class=06-04-00, hdrtype=0x01, mfdev=0
        cmdreg=0x0107, statreg=0x0010, cachelnsz=8 (dwords)
        lattimer=0x00 (0 ns), mingnt=0x00 (0 ns), maxlat=0x00 (0 ns)
        intpin=a, irq=255
        powerspec 3  supports D0 D1 D2 D3  current D0
        MSI supports 32 messages, 64 bit, vector masks
        MSI-X supports 1 message in maps 0x10 and 0x18
        map[10]: type Memory, range 64, base 0, size 20, enabled
        secbus=1, subbus=255
pcib1: <PCI-PCI bridge> at device 0.0 on pci0
pcib0: failed to reserve resource for pcib1
pcib1: failed to allocate initial I/O port window: 0-0xffff
pcib0: failed to reserve resource for pcib1
pcib1: failed to allocate initial memory window: 0xf6000000-0xf63fffff
pcib1:   domain            0
pcib1:   secondary bus     1
pcib1:   subordinate bus   255
pci1: <PCI bus> on pcib1
pcib1: allocated bus range (1-1) for rid 0 of pci1
pci1: domain=0, physical bus=1
found-> vendor=0x8086, dev=0x1521, revid=0x01
        domain=0, bus=1, slot=0, func=0
        class=02-00-00, hdrtype=0x00, mfdev=1
        cmdreg=0x0007, statreg=0x0010, cachelnsz=8 (dwords)
        lattimer=0x00 (0 ns), mingnt=0x00 (0 ns), maxlat=0x00 (0 ns)
        intpin=a, irq=0
        powerspec 3  supports D0 D3  current D0
        MSI supports 1 message, 64 bit, vector masks
        MSI-X supports 10 messages in map 0x1c
        map[10]: type Memory, range 32, base 0xf6000000, size 20, enabled
pcib0: failed to reserve resource for pcib1
pcib1: failed to allocate initial memory window (0xf6000000-0xf60fffff,0x100000)
pcib1: allocated initial memory window of 0xc0000000-0xc00fffff
pcib1: allocated memory range (0xc0000000-0xc00fffff) for rid 10 of pci0:1:0:0
        map[18]: type I/O Port, range 32, base 0xffffffe0, size  5, enabled
        map[1c]: type Memory, range 32, base 0xf6100000, size 14, enabled
pcib1: attempting to grow memory window for (0xf6100000-0xf6103fff,0x4000)
        back candidate range: 0xf6100000-0xf6103fff
pcib1: attempting to grow memory window for (0-0xffffffff,0x4000)
        front candidate range: 0xbfffc000-0xbfffffff
        back candidate range: 0xc0100000-0xc0103fff
pcib1: grew memory window to 0xc0000000-0xc01fffff
pcib1: allocated memory range (0xc0100000-0xc0103fff) for rid 1c of pci0:1:0:0
pcib1: ofw_pcib_route_interrupt pin:1
panic: ofw_bus_lookup_imap: cannot get reg property
cpuid = 0
time = 1
KDB: stack backtrace:
db_trace_self() at db_trace_self
db_fetch_ksymtab() at db_fetch_ksymtab+0x160
vpanic() at vpanic+0x184
panic() at panic+0x44
ofw_bus_lookup_imap() at ofw_bus_lookup_imap+0x124
ofw_pcib_route_interrupt() at ofw_pcib_route_interrupt+0x1f4
pcib_route_interrupt() at pcib_route_interrupt+0xf8
pci_add_resources() at pci_add_resources+0x23b4
pci_add_resources() at pci_add_resources+0x664
pci_add_child() at pci_add_child+0x7c
pci_add_children() at pci_add_children+0x58
pci_attach() at pci_attach+0xe0
device_attach() at device_attach+0x400
device_probe_and_attach() at device_probe_and_attach+0x7c
bus_generic_attach() at bus_generic_attach+0x18
device_attach() at device_attach+0x400
device_probe_and_attach() at device_probe_and_attach+0x7c
bus_generic_attach() at bus_generic_attach+0x18
pci_attach() at pci_attach+0xe8
device_attach() at device_attach+0x400
device_probe_and_attach() at device_probe_and_attach+0x7c
bus_generic_attach() at bus_generic_attach+0x18
generic_pcie_get_id() at generic_pcie_get_id+0x1554
device_attach() at device_attach+0x400
device_probe_and_attach() at device_probe_and_attach+0x7c
bus_generic_new_pass() at bus_generic_new_pass+0xf8
bus_generic_new_pass() at bus_generic_new_pass+0xa8
bus_generic_new_pass() at bus_generic_new_pass+0xa8
bus_generic_new_pass() at bus_generic_new_pass+0xa8
bus_set_pass() at bus_set_pass+0x4c
mi_startup() at mi_startup+0x12c
_start() at _start+0xa8
KDB: enter: panic
[ thread pid 0 tid 100000 ]
Stopped at      kdb_enter+0x44: undefined       f904011f
db>

[kd]

You're right, I've also managed to replicate it on mcbin with uboot+dts.
I'll try to fix it ASAP.

[kd]

@mmel Please take a look at D30761, it fixed the issue for me.