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(a)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