pci: Reserve bus numbers required by SR-IOV VFs

[kbowling]

Summary

Some firmware assigns only one bus number to each PCI-PCI bridge.  This
prevents later SR-IOV VF enumeration when a VF routing ID falls on a bus
number already allocated to a sibling bridge.

Reserve only the additional bus numbers required by SR-IOV PFs.
Enumerate all directly attached functions before child drivers and
bridges attach, inspect their device_t objects for SR-IOV, and grow the
PCI bus resource through the highest possible VF routing ID.

First VF Offset and VF Stride may change when NumVFs changes.  Probe
every valid NumVFs value and preserve the original setting.  When the
upstream hierarchy uses ARI, temporarily enable the SR-IOV ARI Hierarchy
control in the lowest-numbered PF while sizing, then restore it.  Scope
active-VF detection to each conventional PCI slot; an ARI bus remains
one slot-0 hierarchy.  If firmware left VFs enabled on a device, do not
modify it and reserve only its active layout.

During runtime configuration, consult the PCI bus's owned resource
range rather than PCI-PCI bridge registers.  This recognizes an existing
boot-time reservation beneath both PCI-PCI and host bridges and avoids a
second, overlapping bus-number allocation.

This avoids consuming bus numbers behind unrelated bridges.  The runtime
allocation in pci_iov.c remains as a fallback when the boot-time range
cannot be enlarged.  hw.pci.clear_buses remains useful when firmware
assigned a required number to another bridge before enumeration.

Validated the targeted implementation on an Intel E810-XXV behind a
non-ARI root port.  With hw.pci.clear_buses=1 and no global reserve
tunable, the PF bridge received buses 1-2 while three unrelated bridges
each received one bus.  A VF with First VF Offset 0x100 attached as
iavf0 at pci0:2:0:0, and detached cleanly.


MFC after:      1 month

Test Plan

Use case, some ewaste Pro Desk SFF machine to test NICs, has VT-d but no SR-IOV option firmware:

• Firmware assigned the 82576 root port only bus 1.
• The non-ARI 82576’s First VF RID Offset = 0x180 places its VFs on bus 2.
• Bus 2 was already assigned to the NVMe root port, so SR-IOV failed with ENOSPC.
• hw.pci.clear_buses=1
• The NVMe remained operational and the extensive 82576 PF/VF matrix passed.

[kib]

If this is a solution, then IMO it should be activated by default. How would user know to tweak this knob if SR-IOV fails?

What is the concern for not reserving some number of bus numbers behind each bridge? The possibility of exhausting the bus number space?

Also, suppose that we have bridges hierarchy where there is a switch behind switch. How would it work?

[mav]

I haven't worked with SR-IOV myself, so I've never seen this problem to seriously comment on it. I've tried to do re-allocation for other resources, but it always created some problems with devices already running at the time, but with resources not always even visible to the device tree, like console. I guess bus renumbering might be easier than memory, but for example, what happen with dmidecode output after that? Will data there no longer reflect the actual bus allocation? I saw some Supermicro motherboard BIOS'es, allowing to set resource reservation for all hot-plug-capable PCIe ports. Unfortunately those are quite rare, and again, cover only hot-pluggable ports. I am not too familiar with SR-IOV, but at least from the point of limited bus number space I wonder if this could be limited to buses with SR-IOV-capable devices, or something like that? Though it would still be a renumbering, unless you allocate the new bus ranges from a range above what BIOS already allocated, to minimize effects on unrelated devices. But if as Konstantin mentioned you have nested bridges, I suppose that would renumber the parents and siblings, which would be invasive and much harder.

[kbowling]

@jhb halp us

[jhb]

This does seem a bit of a large hammer that is probably hard to use safely, and not something you'd want to enable by default as I'd expect it would break various systems. It's probably ok as a hack for specific use cases, though it might be nicer to make the knob a bit more fine-grained. For example, to mav@'s point, a more useful knob for HotPlug might be a HotPlug specific knob that we use instead of '1' if the bridge is HotPlug-capable. For the SRIOV case, it would get a bit messy as you'd have to manually go scan the bus to see if it has any children that support SRIOV, but in order to do that, you first need at least one valid bus number. If you wanted an SRIOV-specific knob, I would perhaps implement this differently by still passing 1 to pcib_setup_secbus() here, then later in attach doing an explicit scan of the child bus to check for any direct children that support SRIOV and growing the bus range (if needed) to the value of a new SRIOV-specific knob.

[kbowling]

Attempt to address everyone's feedback. This is far more complicated and follows algorithmically a Linux precedence of counting NumVFs and making reservations based on that so I think it is safe. My DUT still needs hw.pci_clear_busses in this setup but it safely automates only the expected reservations.

[jhb]

I'm somewhat surprised you are doing this when enumerating the PCI bus vs doing it in pcib_attach() FWIW. If the bus numbers are available for growing, that should be just as doable in pcib_attach() time before you attach drivers to child devices, and then you don't need the manual PCI config read/write code as you will have the normal device_t objects around to use with pci_read_config/pci_write_config.

sys/dev/pci/pci.c

5143

Maybe ari = PCIB_ARI_ENABLED(pcib);?

5171

I'm somewhat surprised you don't have an assertion or some such that the slot is only 0 if ari is true? (Or am I misunderstanding this loop)?

5179

Why do you need to set this bit now rather than when VFs are enabled? I guess that is for your loop counting VFs below?

However, why do you need to clear it? That is, I wonder if you can get by with something like:

/* existing comment */
if (ari) {
    group = &groups[0];
    MPASS(group->have_iov);
    group->saved_ctl = PCIB_READ_CONFIG(...);
    ctl = group->saved_ctl | PCM_SRIOV_ARI_EN;
    group->ctl_changed = !groups->vfs_enabled &&
        ctl != group->saved_ctl;
    if (group->ctl_changed)
        PCIB_WRITE_CONFIG(...);
}

/* loop over PFs */

if (ari && group[0].ctl_changed)
   PCIB_WRITE_CONFIG(...);

That is, I don't think you need to worry about clearing a spurious ARI_EN underneath a non-ARI bridge? We don't do that today in pci_iov_set_ari() in pci_iov.c.

[kbowling]

I reworked the implementation around this feedback but I'm not sure it can be done in pcib (please let me know if you see otherwise).

The reservation now runs after pci_add_children() has created the normal device_t objects, but before bus_attach_children() attaches child drivers and bridges. The sizing code therefore uses pci_find_extcap() and pci_read_config()/pci_write_config() rather than manually enumerating config space.

Reservation is automatic when PCI_IOV is present and is limited to the bus numbers actually required by directly attached SR-IOV PFs. It remains best-effort so existing conflicting assignment is not displaced unless hw.pci.clear_buses.

I also corrected the runtime path to use the PCI bus’s owned sc_bus range. This recognizes a boot-time reservation beneath either a PCI-PCI bridge or a host bridge and avoids attempting a second overlapping allocation.