[libvirt] [PATCH] Fix the ACS checking in the PCI code.

[Chris Wright]

* Chris Lalancette (clalance at redhat.com) wrote:
> The code in libvirt that does ACS checking has a pretty
> serious bug that was essentially rendering the check useless.
> When trying to assign a device, we have to check that all
> bridges upstream of this device support ACS.  That means
> that we have to find the parent of the current device,
> check for ACS, then find the parent of that device, check
> for ACS, etc.
> 
> However, the code to find the parent of the device had a
> much too relaxed check.  It would iterate through all PCI
> devices on the system, looking for a device that had a range
> of busses that included the current device's bus.
> 
> That's not correct, though.  Depending on how we iterated
> through the list of PCI devices, we could first find the
> *topmost* bridge in the system; since it necessarily had
> a range of busses including the current device's bus, we
> would only ever check the topmost bridge, and not check
> any of the intermediate bridges.
> 
> This patch is simple in that it looks for the PCI device
> whose secondary device *exactly* equals the bus of the
> device we are looking for.  That means that one, and only one
> bridge will be found, and it will be the correct device.
> 
> Note that this also caused a fairly serious bug in the
> secondary bus reset code, where we could erroneously
> reset the topmost bus instead of the inner bus.
> 
> This patch was tested by me on a 4-port NIC with a
> bridge without ACS (where assignment failed), a 4-port
> NIC with a bridge with ACS (where assignment succeeded),
> and a 2-port NIC with no bridges (where assignment
> succeeded).

Hmm, I'm not sure  this will work with all SR-IOV devices.  VFs can
exist on a bus number that is >= than the PF bus number.  When it's > PF,
the upstream switch effectively addresses it via the subordinate value
(not secondary).

Can this be loosened to look for the "leaf" parent?  I.e. one where the
secondary bus matches, or the secondary < bus <= subordinate, and no
other bridge has the subordinate value as the secondary value.

thanks,
-chris