On 12/10/2013 11:57 AM, Thadeu Lima de Souza Cascardo wrote:
When determining if a device is behind a PCI bridge, the PCI device
class is checked by reading the config space. However, there are some
devices which have the wrong class on the config space, but the class is
initialized by Linux correctly as a PCI BRIDGE. This class can be read
by the sysfs file '/sys/bus/pci/devices/xxxx:xx:xx.x/class'.
Since I'm not an expert on the PCI spec, I'll just have to take your
word for this :-)
One example of such bridge is IBM PCI Bridge 1014:03b9, which is
identified as a Host Bridge when reading the config space.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo(a)linux.vnet.ibm.com>
---
src/util/virpci.c | 38 +++++++++++++++++++++++++++++++++++---
1 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 8ec642f..8353411 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -344,6 +344,34 @@ virPCIDeviceRead32(virPCIDevicePtr dev, int cfgfd, unsigned int
pos)
}
static int
+virPCIDeviceReadClass(virPCIDevicePtr dev, uint16_t *device_class)
+{
+ char *path = NULL;
+ char *id_str;
+ int ret = 0;
+ unsigned int value;
+
+ if (virPCIFile(&path, dev->name, "class") < 0)
+ return -1;
+
+ /* class string is '0xNNNNNN\n' ... i.e. 9 bytes */
+ if (virFileReadAll(path, 9, &id_str) < 0) {
+ VIR_FREE(path);
+ return -1;
+ }
+
+ VIR_FREE(path);
+
+ id_str[8] = '\0';
+ ret = virStrToLong_ui(id_str, NULL, 16, &value);
+ if (ret == 0)
+ *device_class = (value >> 8) & 0xFFFF;
If the class file for some reason doesn't contain a hexadecimal number,
this will return an error (-1) without having logged an error. This
results in one of those "An error occurred, but the cause is unknown"
messages, which is always very frustrating to debug. Even though it's
*highly* unlikely that such an error would occur, we should still have
the code to log it just in case.
+
+ VIR_FREE(id_str);
+ return ret;
+}
+
+static int
virPCIDeviceWrite(virPCIDevicePtr dev,
int cfgfd,
unsigned int pos,
@@ -645,8 +673,8 @@ virPCIDeviceIsParent(virPCIDevicePtr dev, virPCIDevicePtr check, void
*data)
return 0;
/* Is it a bridge? */
- device_class = virPCIDeviceRead16(check, fd, PCI_CLASS_DEVICE);
- if (device_class != PCI_CLASS_BRIDGE_PCI)
+ ret = virPCIDeviceReadClass(check, &device_class);
+ if (ret == -1 || device_class != PCI_CLASS_BRIDGE_PCI)
goto cleanup;
/* Is it a plane? */
@@ -2110,6 +2138,7 @@ virPCIDeviceDownstreamLacksACS(virPCIDevicePtr dev)
unsigned int pos;
int fd;
int ret = 0;
+ uint16_t device_class;
if ((fd = virPCIDeviceConfigOpen(dev, true)) < 0)
return -1;
@@ -2119,8 +2148,11 @@ virPCIDeviceDownstreamLacksACS(virPCIDevicePtr dev)
goto cleanup;
}
+ if (virPCIDeviceReadClass(dev, &device_class))
+ goto cleanup;
+
pos = dev->pcie_cap_pos;
- if (!pos || virPCIDeviceRead16(dev, fd, PCI_CLASS_DEVICE) != PCI_CLASS_BRIDGE_PCI)
+ if (!pos || device_class != PCI_CLASS_BRIDGE_PCI)
goto cleanup;
flags = virPCIDeviceRead16(dev, fd, pos + PCI_EXP_FLAGS);
Aside from the above lack of error logging, as Michal suggested a test
case would be very useful.