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@xxxxxxxxxxxxxxxxxx>
> ---
>
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list