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>

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

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

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