Read PCI class from sysfs class file instead of config space.

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>

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 8ec642f37fdde822d7b7c6de9241a31f3e358881..bad6e0f8e1637722c286c7ac1730d4cf539dc755 100644 (file)
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -343,6 +343,37 @@ virPCIDeviceRead32(virPCIDevicePtr dev, int cfgfd, unsigned int pos)
     return (buf[0] << 0) | (buf[1] << 8) | (buf[2] << 16) | (buf[3] << 24);
 }
 
+static int
+virPCIDeviceReadClass(virPCIDevicePtr dev, uint16_t *device_class)
+{
+    char *path = NULL;
+    char *id_str = NULL;
+    int ret = -1;
+    unsigned int value;
+
+    if (virPCIFile(&path, dev->name, "class") < 0)
+        return ret;
+
+    /* class string is '0xNNNNNN\n' ... i.e. 9 bytes */
+    if (virFileReadAll(path, 9, &id_str) < 0)
+        goto cleanup;
+
+    id_str[8] = '\0';
+    if (virStrToLong_ui(id_str, NULL, 16, &value) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unusual value in %s/devices/%s/class: %s"),
+                       PCI_SYSFS, dev->name, id_str);
+        goto cleanup;
+    }
+
+    *device_class = (value >> 8) & 0xFFFF;
+    ret = 0;
+cleanup:
+    VIR_FREE(id_str);
+    VIR_FREE(path);
+    return ret;
+}
+
 static int
 virPCIDeviceWrite(virPCIDevicePtr dev,
                   int cfgfd,
@@ -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 < 0 || device_class != PCI_CLASS_BRIDGE_PCI)
         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) < 0)
+        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);

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index a5cef463d5b088d2badc9ff2f0926fd0729463cc..49759b0d890ac78563c238cd65bad3b4e4f51a7c 100644 (file)
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -29,6 +29,7 @@
 # include <fcntl.h>
 # include <sys/stat.h>
 # include <stdarg.h>
+# include <dirent.h>
 # include "viralloc.h"
 # include "virstring.h"
 # include "virfile.h"
@@ -42,6 +43,7 @@ static int (*real__xstat)(int ver, const char *path, struct stat *sb);
 static char *(*realcanonicalize_file_name)(const char *path);
 static int (*realopen)(const char *path, int flags, ...);
 static int (*realclose)(int fd);
+static DIR * (*realopendir)(const char *name);
 
 /* Don't make static, since it causes problems with clang
  * when passed as an arg to virAsprintf()
@@ -112,6 +114,7 @@ struct pciDevice {
     char *id;
     int vendor;
     int device;
+    int class;
     struct pciDriver *driver;   /* Driver attached. NULL if attached to no driver */
 };
 
@@ -351,6 +354,10 @@ pci_device_new_from_stub(const struct pciDevice *data)
         ABORT("@tmp overflow");
     make_file(devpath, "device", tmp, -1);
 
+    if (snprintf(tmp, sizeof(tmp),  "0x%.4x", dev->class) < 0)
+        ABORT("@tmp overflow");
+    make_file(devpath, "class", tmp, -1);
+
     if (pci_device_autobind(dev) < 0)
         ABORT("Unable to bind: %s", data->id);
 
@@ -747,6 +754,7 @@ init_syms(void)
     LOAD_SYM(canonicalize_file_name);
     LOAD_SYM(open);
     LOAD_SYM(close);
+    LOAD_SYM(opendir);
 }
 
 static void
@@ -776,6 +784,13 @@ init_env(void)
     MAKE_PCI_DEVICE("0000:00:01.0", 0x8086, 0x0044);
     MAKE_PCI_DEVICE("0000:00:02.0", 0x8086, 0x0046);
     MAKE_PCI_DEVICE("0000:00:03.0", 0x8086, 0x0048);
+    MAKE_PCI_DEVICE("0001:00:00.0", 0x1014, 0x03b9, .class = 0x060400);
+    MAKE_PCI_DEVICE("0001:01:00.0", 0x8086, 0x105e);
+    MAKE_PCI_DEVICE("0001:01:00.1", 0x8086, 0x105e);
+    MAKE_PCI_DEVICE("0005:80:00.0", 0x10b5, 0x8112, .class = 0x060400);
+    MAKE_PCI_DEVICE("0005:90:01.0", 0x1033, 0x0035);
+    MAKE_PCI_DEVICE("0005:90:01.1", 0x1033, 0x0035);
+    MAKE_PCI_DEVICE("0005:90:01.2", 0x1033, 0x00e0);
 }
 
 
@@ -934,6 +949,24 @@ open(const char *path, int flags, ...)
     return ret;
 }
 
+DIR *
+opendir(const char *path)
+{
+    DIR *ret;
+    char *newpath = NULL;
+
+    init_syms();
+
+    if (STRPREFIX(path, PCI_SYSFS_PREFIX) &&
+        getrealpath(&newpath, path) < 0)
+        return NULL;
+
+    ret = realopendir(newpath ? newpath : path);
+
+    VIR_FREE(newpath);
+    return ret;
+}
+
 int
 close(int fd)
 {

diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 5fe6d49b1cd549127cb048a9f89ee1d8fbe2f7e6..82a173af5605135c460d2ebc8a6a3bad17b9e084 100644 (file)
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -183,6 +183,31 @@ cleanup:
     return ret;
 }
 
+struct testPCIDevData {
+    unsigned int domain;
+    unsigned int bus;
+    unsigned int slot;
+    unsigned int function;
+};
+
+static int
+testVirPCIDeviceIsAssignable(const void *opaque)
+{
+    const struct testPCIDevData *data = opaque;
+    int ret = -1;
+    virPCIDevicePtr dev;
+
+    if (!(dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function)))
+        goto cleanup;
+
+    if (virPCIDeviceIsAssignable(dev, true))
+        ret = 0;
+
+    virPCIDeviceFree(dev);
+cleanup:
+    return ret;
+}
+
 # define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX"
 
 static int
@@ -209,10 +234,19 @@ mymain(void)
             ret = -1;                                   \
     } while (0)
 
+# define DO_TEST_PCI(fnc, domain, bus, slot, function)                  \
+    do {                                                                \
+        struct testPCIDevData data = { domain, bus, slot, function };   \
+        if (virtTestRun(#fnc, fnc, &data) < 0)                          \
+            ret = -1;                                                   \
+    } while (0)
+
     DO_TEST(testVirPCIDeviceNew);
     DO_TEST(testVirPCIDeviceDetach);
     DO_TEST(testVirPCIDeviceReset);
     DO_TEST(testVirPCIDeviceReattach);
+    DO_TEST_PCI(testVirPCIDeviceIsAssignable, 5, 0x90, 1, 0);
+    DO_TEST_PCI(testVirPCIDeviceIsAssignable, 1, 1, 0, 0);
 
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakesysfsdir);

diff --git a/tests/virpcitestdata/0001:00:00.0.config b/tests/virpcitestdata/0001:00:00.0.config
new file mode 100644 (file)
index 0000000..808d489
Binary files /dev/null and b/tests/virpcitestdata/0001:00:00.0.config differ

diff --git a/tests/virpcitestdata/0001:01:00.0.config b/tests/virpcitestdata/0001:01:00.0.config
new file mode 100644 (file)
index 0000000..f63aacb
Binary files /dev/null and b/tests/virpcitestdata/0001:01:00.0.config differ

diff --git a/tests/virpcitestdata/0001:01:00.1.config b/tests/virpcitestdata/0001:01:00.1.config
new file mode 100644 (file)
index 0000000..db9e387
Binary files /dev/null and b/tests/virpcitestdata/0001:01:00.1.config differ

diff --git a/tests/virpcitestdata/0005:80:00.0.config b/tests/virpcitestdata/0005:80:00.0.config
new file mode 100644 (file)
index 0000000..cc4f94a
Binary files /dev/null and b/tests/virpcitestdata/0005:80:00.0.config differ

diff --git a/tests/virpcitestdata/0005:90:01.0.config b/tests/virpcitestdata/0005:90:01.0.config
new file mode 100644 (file)
index 0000000..a60599b
Binary files /dev/null and b/tests/virpcitestdata/0005:90:01.0.config differ

diff --git a/tests/virpcitestdata/0005:90:01.1.config b/tests/virpcitestdata/0005:90:01.1.config
new file mode 100644 (file)
index 0000000..beee765
Binary files /dev/null and b/tests/virpcitestdata/0005:90:01.1.config differ

diff --git a/tests/virpcitestdata/0005:90:01.2.config b/tests/virpcitestdata/0005:90:01.2.config
new file mode 100644 (file)
index 0000000..cfd72e4
Binary files /dev/null and b/tests/virpcitestdata/0005:90:01.2.config differ