On 24.12.2013 19:07, 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'.
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@linux.vnet.ibm.com> ---
The new test cases succeed with this patch. The second test fails without the changes to src/util/virpci.c.
--- src/util/virpci.c | 41 +++++++++++++++++++++++++++-- tests/virpcimock.c | 33 ++++++++++++++++++++++++ tests/virpcitest.c | 34 ++++++++++++++++++++++++ tests/virpcitestdata/0001:00:00.0.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0001:01:00.0.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0001:01:00.1.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0005:80:00.0.config | Bin 0 -> 4096 bytes tests/virpcitestdata/0005:90:01.0.config | Bin 0 -> 256 bytes tests/virpcitestdata/0005:90:01.1.config | Bin 0 -> 256 bytes tests/virpcitestdata/0005:90:01.2.config | Bin 0 -> 256 bytes 10 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 tests/virpcitestdata/0001:00:00.0.config create mode 100644 tests/virpcitestdata/0001:01:00.0.config create mode 100644 tests/virpcitestdata/0001:01:00.1.config create mode 100644 tests/virpcitestdata/0005:80:00.0.config create mode 100644 tests/virpcitestdata/0005:90:01.0.config create mode 100644 tests/virpcitestdata/0005:90:01.1.config create mode 100644 tests/virpcitestdata/0005:90:01.2.config
diff --git a/src/util/virpci.c b/src/util/virpci.c index 8ec642f..8c10a93 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -344,6 +344,37 @@ 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; + else + VIR_WARN("Unusual value in " PCI_SYSFS "devices/%s/class: %s", + dev->name, id_str); + + VIR_FREE(id_str); + return ret; +} +
I've changed this function slightly to match the pattern used in the rest of our code.
+static int virPCIDeviceWrite(virPCIDevicePtr dev, int cfgfd, unsigned int pos, @@ -645,8 +676,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)
s/== -1/< 0/
goto cleanup;
/* Is it a plane? */ @@ -2110,6 +2141,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 +2151,11 @@ virPCIDeviceDownstreamLacksACS(virPCIDevicePtr dev) goto cleanup; }
+ if (virPCIDeviceReadClass(dev, &device_class)) + goto cleanup; +
'if (virPCIDevicReadClass(dev,&device_class) < 0) ' looks nicer.
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);
diff --git a/tests/virpcitestdata/0001:00:00.0.config b/tests/virpcitestdata/0001:00:00.0.config new file mode 100644 index 0000000000000000000000000000000000000000..808d48993cfc0f41223fcb5f49deffc594f136b7 GIT binary patch literal 4096 zcmeH@I}XAy5Jca`5~RZg60MJb!~ys;a0<jx5^hCDNy!mt=t)n(8Yfir2x&(4YG(bB z{ig90wibyn0^=hytn<`78f&|D=zEvd5U8+Sxa1hw*nI*%I6fEDtkd58IUH=(-@Ei& z`ONZ@#r(MD|GagrnWu5_32tAW*RPg6sv;l)A|L`HAOa#F0wN#+A|L`H@J9q*PZkdc
literal 0 HcmV?d00001
Some binary garbage ... Fixed, ACKed and pushed. Congratulations on your first libvirt patch! Michal