[libvirt] [PATCH] Read PCI class from sysfs class file instead of config space.

Thadeu Lima de Souza Cascardo cascardo at linux.vnet.ibm.com
Tue Dec 24 18:07:27 UTC 2013


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 at 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;
+}
+
+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)
         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;
+
     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 a5cef46..49759b0 100644
--- 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 5fe6d49..82a173a 100644
--- 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
index 0000000000000000000000000000000000000000..808d48993cfc0f41223fcb5f49deffc594f136b7
GIT binary patch
literal 4096
zcmeH at I}XAy5Jca`5~RZg60MJb!~ys;a0<jx5^hCDNy!mt=t)n(8Yfir2x&(4YG(bB
z{ig90wibyn0^=hytn<`78f&|D=zEvd5U8+Sxa1hw*nI*%I6fEDtkd58IUH=(- at Ei&
z`ONZ@#r(MD|GagrnWu5_32tAW*RPg6sv;l)A|L`HAOa#F0wN#+A|L`H at J9q*PZkdc

literal 0
HcmV?d00001

diff --git a/tests/virpcitestdata/0001:01:00.0.config b/tests/virpcitestdata/0001:01:00.0.config
new file mode 100644
index 0000000000000000000000000000000000000000..f63aacbea08fc83b345e1ee404756145e7d349a5
GIT binary patch
literal 4096
zcmZo`h!bFD5MW?qU|>>UXkY*WAi>nY$iN6<qW}>B2WF7K379CR5||9#X~qjmCm0kM
z6j>iMK<#Fl0AdIL{c(^7NNY$jIHrJ{?<fFO0H%Qc6alFM0YL>exUYeN42%s7Ec~1x
z877=QWd`bav)TXue_{D2AeEzFGz3ONU^E0qLtr!nMnhmU1V%$(Gz3ONU^E0qLtr!n
I24e^S0CUn1m;e9(

literal 0
HcmV?d00001

diff --git a/tests/virpcitestdata/0001:01:00.1.config b/tests/virpcitestdata/0001:01:00.1.config
new file mode 100644
index 0000000000000000000000000000000000000000..db9e3876a528266bb5a6e9651d7cd18b45b7d71f
GIT binary patch
literal 4096
zcmZo`h!bFD5MW?qU|>>UXkcJqU;z?r4T=nmKsG865pZAziJX85Gr<@GPcvRnI>Dg8
zpvd~50qPj02_PQ`0R3^02S{s3F*v4xobM<ARRE?K85l)Csz5+cfer3!pdbTd0|N^`
zCrE|~GcZ8HK)?*t at n*CC|Np}BPaq<r<Y)+thQMeDjE2By2#kinXb6mkz-S1JhQMeD
NjE2By2n at au000k05W)Ze

literal 0
HcmV?d00001

diff --git a/tests/virpcitestdata/0005:80:00.0.config b/tests/virpcitestdata/0005:80:00.0.config
new file mode 100644
index 0000000000000000000000000000000000000000..cc4f94a849ed3d0a47f5b47331aaecdea88765c3
GIT binary patch
literal 4096
zcmdlgAk at gtAi%JSfrX8Mfsp|Q8YawV_`sl`#L)Di>BE1RD1>%^ae>Mi1DK*fTmcO}
zuqbOn1DJ*p0t|%^rUDm(pbry}Ey&Qo0}^ro5pYlpXVI6zg5+oV+Kg)3K=~il6$8f`
z_5Z+y{a~<pH2w!eEDW3*M&p0r!hSH=JR1LlAr=PC4WsctaA7|fY#xpO!4L}r=Z4Yv
IADFNQ0PH*z9smFU

literal 0
HcmV?d00001

diff --git a/tests/virpcitestdata/0005:90:01.0.config b/tests/virpcitestdata/0005:90:01.0.config
new file mode 100644
index 0000000000000000000000000000000000000000..a60599bd342d3ebcdc7b8367ca36ad337f602fde
GIT binary patch
literal 256
zcmXpOFlAt45MXi^VCG at qXkY+>CJ=!Q7z5RUfCHEW5{!&mj0{Y5Fz#TaS&cX3;ByxM
DCDjDB

literal 0
HcmV?d00001

diff --git a/tests/virpcitestdata/0005:90:01.1.config b/tests/virpcitestdata/0005:90:01.1.config
new file mode 100644
index 0000000000000000000000000000000000000000..beee76534041a7020c08ae9ac03d9a349c6ea12e
GIT binary patch
literal 256
ycmXpOFlAt45MXi^VCG at qU|?VnU}yr8Sb;H6EeJS(Ng%<*sKv;@R0rb at MH&E<&;s)S

literal 0
HcmV?d00001

diff --git a/tests/virpcitestdata/0005:90:01.2.config b/tests/virpcitestdata/0005:90:01.2.config
new file mode 100644
index 0000000000000000000000000000000000000000..cfd72e4d7165bff2751ecbdc570ce7f1e0646226
GIT binary patch
literal 256
zcmXpOc)-BMAi%_;z|6zo!oa|wz|aIFu>xbDS`csmlR$!5K#7rosSd`)Mk^@TV-u#E
T7_0Gy9FS#<5E~CbC<F-rZiWW}

literal 0
HcmV?d00001

-- 
1.7.1




More information about the libvir-list mailing list