Disable attaching of generic PCI drv on FDT nodes

[wma]

Summary

When the FDT node for PCI controller has also other FDT
devices under it, the generic PCI driver should not be
allowed to attach to these children.

[jhb]

Ugh, this seems like a really gross way to fix this. Why can't you create a 'pcib' device on the OFW bus that creates a "pci" child? This is what ACPI does for example (acpi_pcib_acpi.c is the example of doing this in ACPI).

[wma]

It makes problems with loadable modules. On a ThunderX we've got DTS which has ethernet PHYs present under PCI-RC node (which are not PCI devices though). If I use a PHY driver compiled in the kernel then everything is fine, because during enumeration it finds the most appropriate driver and ignores default PCI because of BUS_PROBE_GENERIC. But when the PHY driver is expected to be loaded later, kernel attaches this driver to that particular node, tries to access device config space, which obviously fails and floods the console with errors (device is not a PCI one).
What;s more, when I load the appropriate driver, it no longer gets attached, because the PHY device already has a generic-pci driver.

ACPI is way different in this case. This commit is valid only for devices using messed-up DTS, where we;ve got non-pci devices under pcib node.

[wma]

If you have any idea if that can be resolved in another way, then I'm open to suggestions. Unfortunately fixing DTS is not an option :(

[jhb]

In D6336#135880, @wma wrote:

It makes problems with loadable modules. On a ThunderX we've got DTS which has ethernet PHYs present under PCI-RC node (which are not PCI devices though). If I use a PHY driver compiled in the kernel then everything is fine, because during enumeration it finds the most appropriate driver and ignores default PCI because of BUS_PROBE_GENERIC. But when the PHY driver is expected to be loaded later, kernel attaches this driver to that particular node, tries to access device config space, which obviously fails and floods the console with errors (device is not a PCI one).
What;s more, when I load the appropriate driver, it no longer gets attached, because the PHY device already has a generic-pci driver.

ACPI is way different in this case. This commit is valid only for devices using messed-up DTS, where we;ve got non-pci devices under pcib node.

How is ACPI "way different"? In the ACPI namespace, there isn't a separate node for just the bus from the bridge. You get:

• _SB_
  • PCI0
    • PCI1
      • IDE0

We create "pcib" devices for the 'PCIx' ACPI devices, we don't create "pci" devices. The ACPI PCI driver walks the children of the handle of its parent bridge and looks for devices in the ACPI namespace that match PCI-enumerated devices, but non-PCI devices that are children of PCI0 (like system resource devices that do sometimes show up) work fine. The OFW PCI bus drivers work the same.

[jhb]

In D6336#136112, @jhb wrote:

In D6336#135880, @wma wrote:

It makes problems with loadable modules. On a ThunderX we've got DTS which has ethernet PHYs present under PCI-RC node (which are not PCI devices though). If I use a PHY driver compiled in the kernel then everything is fine, because during enumeration it finds the most appropriate driver and ignores default PCI because of BUS_PROBE_GENERIC. But when the PHY driver is expected to be loaded later, kernel attaches this driver to that particular node, tries to access device config space, which obviously fails and floods the console with errors (device is not a PCI one).
What;s more, when I load the appropriate driver, it no longer gets attached, because the PHY device already has a generic-pci driver.

ACPI is way different in this case. This commit is valid only for devices using messed-up DTS, where we;ve got non-pci devices under pcib node.

How is ACPI "way different"? In the ACPI namespace, there isn't a separate node for just the bus from the bridge. You get:

• _SB_
  • PCI0
    • PCI1
      • IDE0

We create "pcib" devices for the 'PCIx' ACPI devices, we don't create "pci" devices. The ACPI PCI driver walks the children of the handle of its parent bridge and looks for devices in the ACPI namespace that match PCI-enumerated devices, but non-PCI devices that are children of PCI0 (like system resource devices that do sometimes show up) work fine. The OFW PCI bus drivers work the same.

To be clear, the "pcib" devices create the "pci" child because they are a "pcib" device, however, they aren't created in response to a specific node in the tree. Only "pcib" devices are created from logical nodes as PCI busses are a logical construct that exist downstream of a bridge (where a bridge is the actual device).

[wma]

Thank you for the response.

I think it'd be best to describe what we have on ThunderX. When it comes to PCI node, here is how the tree looks like now:

<pcib>    (PCIe-RC controller node in DTS)
  <pci>   (PCI bus device, added in pci_host_generic_attach)
   <... some devices>   (all PCI devices enumerated in traditional way)
  <mrmlbus>   (parent node for ethernet PHYs)
   <thunder_mdio0>   (MDIO interface)
   <thunder_mdio1>   (MDIO interface)

In normal case, when I have all drivers compiled in kernel, the system looks for the most suitable one. This ends up with following assignment:

<pcib>      driver: pci_host_generic
  <pci>    driver: pci
    <... some devices>
  <mrmlbus>            driver: mrmlbus
   <thunder_mdio0>    driver: thunder_mdio
   <thunder_mdio1>    driver: thunder_mdio

The problem starts when I want to use drivers compiled as a modules. Then, the mrmlbus driver does not exist when pcib is being attached, so as you said, the system attaches the most appropriate driver for mrmlbus it can find, which is "pci". Then, it tries to enumerate whole bus which is supposed to be under this driver, but it's obvious it fails. The tree looks as following:

<pcib>      driver: pci_host_generic
  <pci>    driver: pci
    <... some devices>
  <mrmlbus>            driver: pci
    <... enumeration failures>

This is why I think the situation with ACPI and IDE is different, because there we have IDE node under PCI, which is not the case on ThunderX.

[jhb]

Well, the situation is the same actually, but IDE on ACPI tends to already be a PCI device (and if it isn't and has a _HID it ends up being attached as a direct child of acpi0, not of pcib0). That is, for ACPI we "flatten" any devices with ACPI _HIDs and make them children of acpi0.

Another solution is to change pci_probe() to use 'BUS_PROBE_NO_WILDCARD'. This means it will only match devices that were created with the explicit name "pci". However, that is a bit of a one-trick pony in that it doesn't work as well with having sub-class drivers with higher probe priorities (e.g. ofw_pci and acpi_pci) as the sub-class drivers cannot use BUS_PROBE_NO_WILDCARD.

As a general rule, buses in FreeBSD either have all "no_wildcard" named devices (so devices added with an explicit name) or they only have unnamed devices that use ivars (like PCI device IDs) for probing. We avoid crossing the streams because it doesn't work well. This means that "pcib" devices generally only have "pci" children and nothing else. Otherwise we'd have to extend your hack here to start avoiding other bus ivars like ACPI handles, ISA plug and play IDs and whatever else comes up in the future. That is why your current approach isn't really scalable. For example, right now it is only by "luck" that the acpi_pci driver doesn't attach to your mrmlbus device as it doesn't happen to have an ACPI handle.

In ACPI the equivalent would be that 'mrmlbus' would be a sibling of pcib instead of a child of pcib. I'm not sure you want to do that. Another option is to have the PCI-e R-C device use a different bus driver that creates a 'pcib' child to represent the 'PCI bridge' part of the PCI-e R-C device's personality (since that device clearly has both PCI and non-PCI children). That pcib would then only have a single "pci" device. That is a tree like this:

<pcirc>  (PCI-e RC controller node in DTS)
 <pcib>  (Driver for just the "Host-PCI bridge portion of 'pcirc')
   <pci>
 <mrmlbus>

You would have to take your existing <pcib> driver and split into a pcirc driver that attaches to the DTS node and a child pcib driver that handles the PCI bridge portion of its behavior. We have some other psuedo-buses like this in the tree for devices that have multiple sub-functions. vgapci in particular creates separate child devices to handle DRM vs AGP vs acpi_video, etc. for a single VGA device.

[wma]

It's already outdated