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

[Chris Lalancette <clalance@xxxxxxxxxx>]

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).

Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
---
 src/util/pci.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/util/pci.c b/src/util/pci.c
index 26d55b8..df30e04 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -513,7 +513,7 @@ static int
 pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
 {
     uint16_t device_class;
-    uint8_t header_type, secondary, subordinate;
+    uint8_t header_type, secondary;
 
     if (dev->domain != check->domain)
         return 0;
@@ -529,12 +529,11 @@ pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
         return 0;
 
     secondary   = pciRead8(check, PCI_SECONDARY_BUS);
-    subordinate = pciRead8(check, PCI_SUBORDINATE_BUS);
 
     VIR_DEBUG("%s %s: found parent device %s", dev->id, dev->name, check->name);
 
     /* No, it's superman! */
-    return (dev->bus >= secondary && dev->bus <= subordinate);
+    return (dev->bus == secondary);
 }
 
 static pciDevice *
-- 
1.7.1.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list