Page MenuHomeFreeBSD

pci_pci: Support growing windows in bus_adjust_resource for NEW_PCIB
ClosedPublic

Authored by jrtc27 on Jul 5 2021, 12:15 AM.
Tags
None
Referenced Files
F102803347: D31035.diff
Sun, Nov 17, 9:33 AM
Unknown Object (File)
Fri, Nov 8, 3:46 PM
Unknown Object (File)
Fri, Nov 8, 12:54 AM
Unknown Object (File)
Wed, Nov 6, 9:32 PM
Unknown Object (File)
Wed, Nov 6, 8:00 PM
Unknown Object (File)
Wed, Nov 6, 12:59 PM
Unknown Object (File)
Wed, Nov 6, 9:35 AM
Unknown Object (File)
Tue, Nov 5, 8:49 AM
Subscribers
None

Details

Summary

If we allocate a new window for a bridge rather than reusing an existing
one set up by firmware to cover all the devices then the new window only
includes the range needed for the first device to allocate the resource.
If a request comes in to adjust this resource in order to extend a
downstream window for another device then this will fail as the rman
doesn't have any space, so we must first grow the bridge's own window.

This is needed to support successfully attaching more than one PCI
device on SiFive's HiFive Unmatched, which has the following topology:

Root Port <---> Bridge <---> Bridge <-+-> Bridge <---> (Unused)
 (pcib0)        (pcib1)      (pcib2)  |   (pcib3)
                                      +-> Bridge <---> xHCI
                                      |   (pcib4)
                                      +-> Bridge <---> M.2 E-key
                                      |   (pcib5)
                                      +-> Bridge <---> M.2 M-key
                                      |   (pcib6)
                                      +-> Bridge <---> x16 slot
                                          (pcib7)

Without this, the xHCI endpoint successfully attaches but NVMe M.2 M-key
endpoint fails to attach as, when its adjacent bridge (pcib6) attempts
to allocate a window from its parent (pcib2) on the other side of the
switch, its parent to grow its window by calling bus_adjust_resource on
its own parent (pcib1) which fails to call the root port device (pcib0)
to request more memory to grow its own window. Had the root port been
directly connected to the switch without the bridge in the middle then
the existing code would have worked, but the extra hop broke it.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 40282
Build 37171: arc lint + arc unit

Event Timeline

jrtc27 created this revision.

It looks OK, but I don't remember the resource allocation code enough to review without deeper investigating it myself.

First pass, this looks good to me, but I'd be quite interested in hearing what jhb@ has to say as well.

sys/dev/pci/pci_pci.c
183

Hmmm, I wonder if this can be dropped now. It looks like all our platforms define this.

sys/dev/pci/pci_pci.c
183

Even in stable/12 all our NEW_PCIB-using platforms defined it it seems, so yeah we should probably have a sweep to drop these #ifdef's so long as it's under NEW_PCIB (though I struggle to see how you would end up with code that checked for PCI_RES_BUS without NEW_PCIB...).

Humm, I guess it's only the nested grandchildren that hit this then? I've booted x86 machines with the right knobs to clear all the windows which should grow them for multiple devices like this (hw.pci.pcib_clear=1 IIRC), but I don't know if they had the extra bridges in the middle (though PCI-e gives you lots of PCI-PCI bridges). I think my assumption had been that you would only have to grow windows due to a child doing a bus_alloc_resource(), but if a child PCI-PCI bridge tries to grow a window, that would end up taking this path instead when it tried to adjust its window.

sys/dev/pci/pci_pci.c
183

Even in stable/12 all our NEW_PCIB-using platforms defined it it seems, so yeah we should probably have a sweep to drop these #ifdef's so long as it's under NEW_PCIB (though I struggle to see how you would end up with code that checked for PCI_RES_BUS without NEW_PCIB...).

NEW_PCIB is effectively a prerequisite for PCI_RES_BUS. At this point it is probably fine to mandate that platforms that support NEW_PCIB also support PCI bus numbers.

This revision is now accepted and ready to land.Jul 29 2021, 10:28 PM