Introduce a sysfs liibrary to handle the common reads/writes to the
PCI sysfs files, for example, getting the total number of VFs supported
by the device via /sys/bus/pci/devices/$BDF/sriov_totalvfs. The library
will be used in the upcoming test patch to configure the VFs for a given
PF device.
Opportunistically, move vfio_pci_get_group_from_dev() to this library as
it falls under the same bucket. Rename it to sysfs_get_device_group() to
align with other function names.
Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
---
.../selftests/vfio/lib/include/libvfio.h | 1 +
.../vfio/lib/include/libvfio/sysfs.h | 16 ++
tools/testing/selftests/vfio/lib/libvfio.mk | 1 +
tools/testing/selftests/vfio/lib/sysfs.c | 151 ++++++++++++++++++
.../selftests/vfio/lib/vfio_pci_device.c | 22 +--
5 files changed, 170 insertions(+), 21 deletions(-)
create mode 100644 tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
create mode 100644 tools/testing/selftests/vfio/lib/sysfs.c
diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
index 279ddcd701944..bbe1d7616a648 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio.h
@@ -5,6 +5,7 @@
#include <libvfio/assert.h>
#include <libvfio/iommu.h>
#include <libvfio/iova_allocator.h>
+#include <libvfio/sysfs.h>
#include <libvfio/vfio_pci_device.h>
#include <libvfio/vfio_pci_driver.h>
diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
new file mode 100644
index 0000000000000..1eca6b5cbcfcc
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
+#define SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
+
+int sysfs_get_sriov_totalvfs(const char *bdf);
+int sysfs_get_sriov_numvfs(const char *bdf);
+void sysfs_set_sriov_numvfs(const char *bdfs, int numvfs);
+void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf);
+bool sysfs_get_sriov_drivers_autoprobe(const char *bdf);
+void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val);
+void sysfs_bind_driver(const char *bdf, const char *driver);
+void sysfs_unbind_driver(const char *bdf, const char *driver);
+int sysfs_get_driver(const char *bdf, char *out_driver);
+unsigned int sysfs_get_device_group(const char *bdf);
+
+#endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H */
diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
index 9f47bceed16f4..b7857319c3f1f 100644
--- a/tools/testing/selftests/vfio/lib/libvfio.mk
+++ b/tools/testing/selftests/vfio/lib/libvfio.mk
@@ -6,6 +6,7 @@ LIBVFIO_SRCDIR := $(selfdir)/vfio/lib
LIBVFIO_C := iommu.c
LIBVFIO_C += iova_allocator.c
LIBVFIO_C += libvfio.c
+LIBVFIO_C += sysfs.c
LIBVFIO_C += vfio_pci_device.c
LIBVFIO_C += vfio_pci_driver.c
diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
new file mode 100644
index 0000000000000..5551e8b981075
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/sysfs.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <linux/limits.h>
+
+#include <libvfio.h>
+
+static int sysfs_get_val(const char *component, const char *name,
+ const char *file)
+{
+ char path[PATH_MAX] = {0};
+ char buf[32] = {0};
+ int fd;
+
+ snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ return fd;
+
+ VFIO_ASSERT_GT(read(fd, buf, ARRAY_SIZE(buf)), 0);
+ VFIO_ASSERT_EQ(close(fd), 0);
+
+ return strtol(buf, NULL, 0);
+}
+
+static void sysfs_set_val(const char *component, const char *name,
+ const char *file, const char *val)
+{
+ char path[PATH_MAX] = {0};
+ int fd;
+
+ snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
+ VFIO_ASSERT_GT(fd = open(path, O_WRONLY), 0);
+
+ VFIO_ASSERT_EQ(write(fd, val, strlen(val)), strlen(val));
+ VFIO_ASSERT_EQ(close(fd), 0);
+}
+
+static int sysfs_get_device_val(const char *bdf, const char *file)
+{
+ sysfs_get_val("devices", bdf, file);
+}
+
+static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)
+{
+ sysfs_set_val("devices", bdf, file, val);
+}
+
+static void sysfs_set_driver_val(const char *driver, const char *file, const char *val)
+{
+ sysfs_set_val("drivers", driver, file, val);
+}
+
+static void sysfs_set_device_val_int(const char *bdf, const char *file, int val)
+{
+ char val_str[32] = {0};
+
+ snprintf(val_str, sizeof(val_str), "%d", val);
+ sysfs_set_device_val(bdf, file, val_str);
+}
+
+int sysfs_get_sriov_totalvfs(const char *bdf)
+{
+ return sysfs_get_device_val(bdf, "sriov_totalvfs");
+}
+
+int sysfs_get_sriov_numvfs(const char *bdf)
+{
+ return sysfs_get_device_val(bdf, "sriov_numvfs");
+}
+
+void sysfs_set_sriov_numvfs(const char *bdf, int numvfs)
+{
+ sysfs_set_device_val_int(bdf, "sriov_numvfs", numvfs);
+}
+
+bool sysfs_get_sriov_drivers_autoprobe(const char *bdf)
+{
+ return (bool)sysfs_get_device_val(bdf, "sriov_drivers_autoprobe");
+}
+
+void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val)
+{
+ sysfs_set_device_val_int(bdf, "sriov_drivers_autoprobe", val);
+}
+
+void sysfs_bind_driver(const char *bdf, const char *driver)
+{
+ sysfs_set_driver_val(driver, "bind", bdf);
+}
+
+void sysfs_unbind_driver(const char *bdf, const char *driver)
+{
+ sysfs_set_driver_val(driver, "unbind", bdf);
+}
+
+void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf)
+{
+ char vf_path[PATH_MAX] = {0};
+ char path[PATH_MAX] = {0};
+ int ret;
+
+ snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/virtfn%d", pf_bdf, i);
+
+ ret = readlink(path, vf_path, PATH_MAX);
+ VFIO_ASSERT_NE(ret, -1);
+
+ ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
+ VFIO_ASSERT_EQ(ret, 1);
+}
+
+unsigned int sysfs_get_device_group(const char *bdf)
+{
+ char dev_iommu_group_path[PATH_MAX] = {0};
+ char path[PATH_MAX] = {0};
+ unsigned int group;
+ int ret;
+
+ snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/iommu_group", bdf);
+
+ ret = readlink(path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
+ VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
+
+ ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
+ VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
+
+ return group;
+}
+
+int sysfs_get_driver(const char *bdf, char *out_driver)
+{
+ char driver_path[PATH_MAX] = {0};
+ char path[PATH_MAX] = {0};
+ int ret;
+
+ snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
+ ret = readlink(path, driver_path, PATH_MAX);
+ if (ret == -1) {
+ if (errno == ENOENT)
+ return -1;
+
+ VFIO_FAIL("Failed to read %s\n", path);
+ }
+
+ ret = sscanf(basename(driver_path), "%s", out_driver);
+ VFIO_ASSERT_EQ(ret, 1);
+
+ return 0;
+}
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 64a19481b734f..9b2a123cee5fc 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -22,8 +22,6 @@
#include "../../../kselftest.h"
#include <libvfio.h>
-#define PCI_SYSFS_PATH "/sys/bus/pci/devices"
-
static void vfio_pci_irq_set(struct vfio_pci_device *device,
u32 index, u32 vector, u32 count, int *fds)
{
@@ -181,24 +179,6 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
ioctl_assert(device->fd, VFIO_DEVICE_RESET, NULL);
}
-static unsigned int vfio_pci_get_group_from_dev(const char *bdf)
-{
- char dev_iommu_group_path[PATH_MAX] = {0};
- char sysfs_path[PATH_MAX] = {0};
- unsigned int group;
- int ret;
-
- snprintf_assert(sysfs_path, PATH_MAX, "%s/%s/iommu_group", PCI_SYSFS_PATH, bdf);
-
- ret = readlink(sysfs_path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
- VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
-
- ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
- VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
-
- return group;
-}
-
static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
{
struct vfio_group_status group_status = {
@@ -207,7 +187,7 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
char group_path[32];
int group;
- group = vfio_pci_get_group_from_dev(bdf);
+ group = sysfs_get_device_group(bdf);
snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
device->group_fd = open(group_path, O_RDWR);
--
2.52.0.239.gd5f0c6e74e-googOn 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> Introduce a sysfs liibrary to handle the common reads/writes to the
library
> PCI sysfs files, for example, getting the total number of VFs supported
> by the device via /sys/bus/pci/devices/$BDF/sriov_totalvfs. The library
> will be used in the upcoming test patch to configure the VFs for a given
> PF device.
>
> Opportunistically, move vfio_pci_get_group_from_dev() to this library as
> it falls under the same bucket. Rename it to sysfs_get_device_group() to
> align with other function names.
>
> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> ---
> .../selftests/vfio/lib/include/libvfio.h | 1 +
> .../vfio/lib/include/libvfio/sysfs.h | 16 ++
> tools/testing/selftests/vfio/lib/libvfio.mk | 1 +
> tools/testing/selftests/vfio/lib/sysfs.c | 151 ++++++++++++++++++
> .../selftests/vfio/lib/vfio_pci_device.c | 22 +--
> 5 files changed, 170 insertions(+), 21 deletions(-)
> create mode 100644 tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> create mode 100644 tools/testing/selftests/vfio/lib/sysfs.c
>
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
> index 279ddcd701944..bbe1d7616a648 100644
> --- a/tools/testing/selftests/vfio/lib/include/libvfio.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio.h
> @@ -5,6 +5,7 @@
> #include <libvfio/assert.h>
> #include <libvfio/iommu.h>
> #include <libvfio/iova_allocator.h>
> +#include <libvfio/sysfs.h>
> #include <libvfio/vfio_pci_device.h>
> #include <libvfio/vfio_pci_driver.h>
>
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> new file mode 100644
> index 0000000000000..1eca6b5cbcfcc
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> +#define SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> +
> +int sysfs_get_sriov_totalvfs(const char *bdf);
> +int sysfs_get_sriov_numvfs(const char *bdf);
> +void sysfs_set_sriov_numvfs(const char *bdfs, int numvfs);
> +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf);
> +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf);
> +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val);
> +void sysfs_bind_driver(const char *bdf, const char *driver);
> +void sysfs_unbind_driver(const char *bdf, const char *driver);
> +int sysfs_get_driver(const char *bdf, char *out_driver);
> +unsigned int sysfs_get_device_group(const char *bdf);
> +
> +#endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H */
> diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
> index 9f47bceed16f4..b7857319c3f1f 100644
> --- a/tools/testing/selftests/vfio/lib/libvfio.mk
> +++ b/tools/testing/selftests/vfio/lib/libvfio.mk
> @@ -6,6 +6,7 @@ LIBVFIO_SRCDIR := $(selfdir)/vfio/lib
> LIBVFIO_C := iommu.c
> LIBVFIO_C += iova_allocator.c
> LIBVFIO_C += libvfio.c
> +LIBVFIO_C += sysfs.c
> LIBVFIO_C += vfio_pci_device.c
> LIBVFIO_C += vfio_pci_driver.c
>
> diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
> new file mode 100644
> index 0000000000000..5551e8b981075
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/sysfs.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <linux/limits.h>
> +
> +#include <libvfio.h>
> +
> +static int sysfs_get_val(const char *component, const char *name,
nit: I'm partial to putting the verbs at the end of function names for
library calls.
e.g.
vfio_pci_config_read()
vfio_pci_config_write()
vfio_pci_msi_enable()
vfio_pci_msi_disable()
So these would be:
sysfs_val_set()
sysfs_val_get()
sysfs_device_val_set()
sysfs_device_val_get()
sysfs_sriov_numvfs_set()
sysfs_sriov_numvfs_get()
...
> + const char *file)
> +{
> + char path[PATH_MAX] = {0};
> + char buf[32] = {0};
nit: You don't need to zero-initialize these since you only use them
after they are intitialized below.
> + int fd;
> +
> + snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
Use the new snprintf_assert() :)
> + fd = open(path, O_RDONLY);
> + if (fd < 0)
> + return fd;
> +
> + VFIO_ASSERT_GT(read(fd, buf, ARRAY_SIZE(buf)), 0);
> + VFIO_ASSERT_EQ(close(fd), 0);
> +
> + return strtol(buf, NULL, 0);
> +}
> +
> +static void sysfs_set_val(const char *component, const char *name,
> + const char *file, const char *val)
> +{
> + char path[PATH_MAX] = {0};
Ditto here about zero-intialization being unnecessary.
> + int fd;
> +
> + snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
Ditto here about snprintf_assert()
You get the idea... I won't comment on the ones below.
> + VFIO_ASSERT_GT(fd = open(path, O_WRONLY), 0);
> +
> + VFIO_ASSERT_EQ(write(fd, val, strlen(val)), strlen(val));
> + VFIO_ASSERT_EQ(close(fd), 0);
> +}
> +
> +static int sysfs_get_device_val(const char *bdf, const char *file)
> +{
> + sysfs_get_val("devices", bdf, file);
> +}
> +
> +static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)
> +{
> + sysfs_set_val("devices", bdf, file, val);
> +}
> +
> +static void sysfs_set_driver_val(const char *driver, const char *file, const char *val)
> +{
> + sysfs_set_val("drivers", driver, file, val);
> +}
> +
> +static void sysfs_set_device_val_int(const char *bdf, const char *file, int val)
> +{
> + char val_str[32] = {0};
> +
> + snprintf(val_str, sizeof(val_str), "%d", val);
> + sysfs_set_device_val(bdf, file, val_str);
> +}
> +
> +int sysfs_get_sriov_totalvfs(const char *bdf)
> +{
> + return sysfs_get_device_val(bdf, "sriov_totalvfs");
> +}
> +
> +int sysfs_get_sriov_numvfs(const char *bdf)
> +{
> + return sysfs_get_device_val(bdf, "sriov_numvfs");
> +}
> +
> +void sysfs_set_sriov_numvfs(const char *bdf, int numvfs)
> +{
> + sysfs_set_device_val_int(bdf, "sriov_numvfs", numvfs);
> +}
> +
> +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf)
> +{
> + return (bool)sysfs_get_device_val(bdf, "sriov_drivers_autoprobe");
> +}
> +
> +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val)
> +{
> + sysfs_set_device_val_int(bdf, "sriov_drivers_autoprobe", val);
> +}
> +
> +void sysfs_bind_driver(const char *bdf, const char *driver)
> +{
> + sysfs_set_driver_val(driver, "bind", bdf);
> +}
> +
> +void sysfs_unbind_driver(const char *bdf, const char *driver)
> +{
> + sysfs_set_driver_val(driver, "unbind", bdf);
> +}
> +
> +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf)
> +{
> + char vf_path[PATH_MAX] = {0};
> + char path[PATH_MAX] = {0};
> + int ret;
> +
> + snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/virtfn%d", pf_bdf, i);
> +
> + ret = readlink(path, vf_path, PATH_MAX);
> + VFIO_ASSERT_NE(ret, -1);
> +
> + ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
> + VFIO_ASSERT_EQ(ret, 1);
> +}
> +
> +unsigned int sysfs_get_device_group(const char *bdf)
nit: s/device_group/iommu_group/
> +{
> + char dev_iommu_group_path[PATH_MAX] = {0};
> + char path[PATH_MAX] = {0};
> + unsigned int group;
> + int ret;
> +
> + snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/iommu_group", bdf);
> +
> + ret = readlink(path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
> + VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
> +
> + ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
> + VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> +
> + return group;
> +}
> +
> +int sysfs_get_driver(const char *bdf, char *out_driver)
> +{
> + char driver_path[PATH_MAX] = {0};
> + char path[PATH_MAX] = {0};
> + int ret;
> +
> + snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
> + ret = readlink(path, driver_path, PATH_MAX);
> + if (ret == -1) {
> + if (errno == ENOENT)
> + return -1;
> +
> + VFIO_FAIL("Failed to read %s\n", path);
> + }
> +
> + ret = sscanf(basename(driver_path), "%s", out_driver);
I think this is equivalent to:
out_driver = basename(driver_path);
... which means out_driver to point within driver_path, which is stack
allocated? I think you want to do strcpy() after basename() to copy the
driver name to out_driver.
Also how do you prevent overflowing out_driver? Maybe it would be
cleaner for sysfs_get_driver() to allocate out_driver and return it to
the caller?
We can return an empty string for the ENOENT case.
> + VFIO_ASSERT_EQ(ret, 1);
> +
> + return 0;
> +}
> diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> index 64a19481b734f..9b2a123cee5fc 100644
> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> @@ -22,8 +22,6 @@
> #include "../../../kselftest.h"
> #include <libvfio.h>
>
> -#define PCI_SYSFS_PATH "/sys/bus/pci/devices"
> -
> static void vfio_pci_irq_set(struct vfio_pci_device *device,
> u32 index, u32 vector, u32 count, int *fds)
> {
> @@ -181,24 +179,6 @@ void vfio_pci_device_reset(struct vfio_pci_device *device)
> ioctl_assert(device->fd, VFIO_DEVICE_RESET, NULL);
> }
>
> -static unsigned int vfio_pci_get_group_from_dev(const char *bdf)
> -{
> - char dev_iommu_group_path[PATH_MAX] = {0};
> - char sysfs_path[PATH_MAX] = {0};
> - unsigned int group;
> - int ret;
> -
> - snprintf_assert(sysfs_path, PATH_MAX, "%s/%s/iommu_group", PCI_SYSFS_PATH, bdf);
> -
> - ret = readlink(sysfs_path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
> - VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
> -
> - ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
> - VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> -
> - return group;
> -}
> -
> static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf)
> {
> struct vfio_group_status group_status = {
> @@ -207,7 +187,7 @@ static void vfio_pci_group_setup(struct vfio_pci_device *device, const char *bdf
> char group_path[32];
> int group;
>
> - group = vfio_pci_get_group_from_dev(bdf);
> + group = sysfs_get_device_group(bdf);
> snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
>
> device->group_fd = open(group_path, O_RDWR);
> --
> 2.52.0.239.gd5f0c6e74e-goog
>
On Wed, Jan 7, 2026 at 2:41 PM David Matlack <dmatlack@google.com> wrote:
>
> On 2025-12-10 06:14 PM, Raghavendra Rao Ananta wrote:
> > Introduce a sysfs liibrary to handle the common reads/writes to the
> library
>
> > PCI sysfs files, for example, getting the total number of VFs supported
> > by the device via /sys/bus/pci/devices/$BDF/sriov_totalvfs. The library
> > will be used in the upcoming test patch to configure the VFs for a given
> > PF device.
> >
> > Opportunistically, move vfio_pci_get_group_from_dev() to this library as
> > it falls under the same bucket. Rename it to sysfs_get_device_group() to
> > align with other function names.
> >
> > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
> > ---
> > .../selftests/vfio/lib/include/libvfio.h | 1 +
> > .../vfio/lib/include/libvfio/sysfs.h | 16 ++
> > tools/testing/selftests/vfio/lib/libvfio.mk | 1 +
> > tools/testing/selftests/vfio/lib/sysfs.c | 151 ++++++++++++++++++
> > .../selftests/vfio/lib/vfio_pci_device.c | 22 +--
> > 5 files changed, 170 insertions(+), 21 deletions(-)
> > create mode 100644 tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> > create mode 100644 tools/testing/selftests/vfio/lib/sysfs.c
> >
> > diff --git a/tools/testing/selftests/vfio/lib/include/libvfio.h b/tools/testing/selftests/vfio/lib/include/libvfio.h
> > index 279ddcd701944..bbe1d7616a648 100644
> > --- a/tools/testing/selftests/vfio/lib/include/libvfio.h
> > +++ b/tools/testing/selftests/vfio/lib/include/libvfio.h
> > @@ -5,6 +5,7 @@
> > #include <libvfio/assert.h>
> > #include <libvfio/iommu.h>
> > #include <libvfio/iova_allocator.h>
> > +#include <libvfio/sysfs.h>
> > #include <libvfio/vfio_pci_device.h>
> > #include <libvfio/vfio_pci_driver.h>
> >
> > diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> > new file mode 100644
> > index 0000000000000..1eca6b5cbcfcc
> > --- /dev/null
> > +++ b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> > +#define SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H
> > +
> > +int sysfs_get_sriov_totalvfs(const char *bdf);
> > +int sysfs_get_sriov_numvfs(const char *bdf);
> > +void sysfs_set_sriov_numvfs(const char *bdfs, int numvfs);
> > +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf);
> > +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf);
> > +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val);
> > +void sysfs_bind_driver(const char *bdf, const char *driver);
> > +void sysfs_unbind_driver(const char *bdf, const char *driver);
> > +int sysfs_get_driver(const char *bdf, char *out_driver);
> > +unsigned int sysfs_get_device_group(const char *bdf);
> > +
> > +#endif /* SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_SYSFS_H */
> > diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
> > index 9f47bceed16f4..b7857319c3f1f 100644
> > --- a/tools/testing/selftests/vfio/lib/libvfio.mk
> > +++ b/tools/testing/selftests/vfio/lib/libvfio.mk
> > @@ -6,6 +6,7 @@ LIBVFIO_SRCDIR := $(selfdir)/vfio/lib
> > LIBVFIO_C := iommu.c
> > LIBVFIO_C += iova_allocator.c
> > LIBVFIO_C += libvfio.c
> > +LIBVFIO_C += sysfs.c
> > LIBVFIO_C += vfio_pci_device.c
> > LIBVFIO_C += vfio_pci_driver.c
> >
> > diff --git a/tools/testing/selftests/vfio/lib/sysfs.c b/tools/testing/selftests/vfio/lib/sysfs.c
> > new file mode 100644
> > index 0000000000000..5551e8b981075
> > --- /dev/null
> > +++ b/tools/testing/selftests/vfio/lib/sysfs.c
> > @@ -0,0 +1,151 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <linux/limits.h>
> > +
> > +#include <libvfio.h>
> > +
> > +static int sysfs_get_val(const char *component, const char *name,
>
> nit: I'm partial to putting the verbs at the end of function names for
> library calls.
>
> e.g.
>
> vfio_pci_config_read()
> vfio_pci_config_write()
> vfio_pci_msi_enable()
> vfio_pci_msi_disable()
>
> So these would be:
>
> sysfs_val_set()
> sysfs_val_get()
> sysfs_device_val_set()
> sysfs_device_val_get()
> sysfs_sriov_numvfs_set()
> sysfs_sriov_numvfs_get()
> ...
>
> > + const char *file)
> > +{
> > + char path[PATH_MAX] = {0};
> > + char buf[32] = {0};
>
> nit: You don't need to zero-initialize these since you only use them
> after they are intitialized below.
>
> > + int fd;
> > +
> > + snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
>
> Use the new snprintf_assert() :)
>
> > + fd = open(path, O_RDONLY);
> > + if (fd < 0)
> > + return fd;
> > +
> > + VFIO_ASSERT_GT(read(fd, buf, ARRAY_SIZE(buf)), 0);
> > + VFIO_ASSERT_EQ(close(fd), 0);
> > +
> > + return strtol(buf, NULL, 0);
> > +}
> > +
> > +static void sysfs_set_val(const char *component, const char *name,
> > + const char *file, const char *val)
> > +{
> > + char path[PATH_MAX] = {0};
>
> Ditto here about zero-intialization being unnecessary.
>
> > + int fd;
> > +
> > + snprintf(path, PATH_MAX, "/sys/bus/pci/%s/%s/%s", component, name, file);
>
> Ditto here about snprintf_assert()
>
> You get the idea... I won't comment on the ones below.
>
> > + VFIO_ASSERT_GT(fd = open(path, O_WRONLY), 0);
> > +
> > + VFIO_ASSERT_EQ(write(fd, val, strlen(val)), strlen(val));
> > + VFIO_ASSERT_EQ(close(fd), 0);
> > +}
> > +
> > +static int sysfs_get_device_val(const char *bdf, const char *file)
> > +{
> > + sysfs_get_val("devices", bdf, file);
> > +}
> > +
> > +static void sysfs_set_device_val(const char *bdf, const char *file, const char *val)
> > +{
> > + sysfs_set_val("devices", bdf, file, val);
> > +}
> > +
> > +static void sysfs_set_driver_val(const char *driver, const char *file, const char *val)
> > +{
> > + sysfs_set_val("drivers", driver, file, val);
> > +}
> > +
> > +static void sysfs_set_device_val_int(const char *bdf, const char *file, int val)
> > +{
> > + char val_str[32] = {0};
> > +
> > + snprintf(val_str, sizeof(val_str), "%d", val);
> > + sysfs_set_device_val(bdf, file, val_str);
> > +}
> > +
> > +int sysfs_get_sriov_totalvfs(const char *bdf)
> > +{
> > + return sysfs_get_device_val(bdf, "sriov_totalvfs");
> > +}
> > +
> > +int sysfs_get_sriov_numvfs(const char *bdf)
> > +{
> > + return sysfs_get_device_val(bdf, "sriov_numvfs");
> > +}
> > +
> > +void sysfs_set_sriov_numvfs(const char *bdf, int numvfs)
> > +{
> > + sysfs_set_device_val_int(bdf, "sriov_numvfs", numvfs);
> > +}
> > +
> > +bool sysfs_get_sriov_drivers_autoprobe(const char *bdf)
> > +{
> > + return (bool)sysfs_get_device_val(bdf, "sriov_drivers_autoprobe");
> > +}
> > +
> > +void sysfs_set_sriov_drivers_autoprobe(const char *bdf, bool val)
> > +{
> > + sysfs_set_device_val_int(bdf, "sriov_drivers_autoprobe", val);
> > +}
> > +
> > +void sysfs_bind_driver(const char *bdf, const char *driver)
> > +{
> > + sysfs_set_driver_val(driver, "bind", bdf);
> > +}
> > +
> > +void sysfs_unbind_driver(const char *bdf, const char *driver)
> > +{
> > + sysfs_set_driver_val(driver, "unbind", bdf);
> > +}
> > +
> > +void sysfs_get_sriov_vf_bdf(const char *pf_bdf, int i, char *out_vf_bdf)
> > +{
> > + char vf_path[PATH_MAX] = {0};
> > + char path[PATH_MAX] = {0};
> > + int ret;
> > +
> > + snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/virtfn%d", pf_bdf, i);
> > +
> > + ret = readlink(path, vf_path, PATH_MAX);
> > + VFIO_ASSERT_NE(ret, -1);
> > +
> > + ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
> > + VFIO_ASSERT_EQ(ret, 1);
> > +}
> > +
> > +unsigned int sysfs_get_device_group(const char *bdf)
>
> nit: s/device_group/iommu_group/
>
> > +{
> > + char dev_iommu_group_path[PATH_MAX] = {0};
> > + char path[PATH_MAX] = {0};
> > + unsigned int group;
> > + int ret;
> > +
> > + snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/iommu_group", bdf);
> > +
> > + ret = readlink(path, dev_iommu_group_path, sizeof(dev_iommu_group_path));
> > + VFIO_ASSERT_NE(ret, -1, "Failed to get the IOMMU group for device: %s\n", bdf);
> > +
> > + ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
> > + VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> > +
> > + return group;
> > +}
> > +
> > +int sysfs_get_driver(const char *bdf, char *out_driver)
> > +{
> > + char driver_path[PATH_MAX] = {0};
> > + char path[PATH_MAX] = {0};
> > + int ret;
> > +
> > + snprintf(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
> > + ret = readlink(path, driver_path, PATH_MAX);
> > + if (ret == -1) {
> > + if (errno == ENOENT)
> > + return -1;
> > +
> > + VFIO_FAIL("Failed to read %s\n", path);
> > + }
> > +
> > + ret = sscanf(basename(driver_path), "%s", out_driver);
>
> I think this is equivalent to:
>
> out_driver = basename(driver_path);
>
> ... which means out_driver to point within driver_path, which is stack
> allocated? I think you want to do strcpy() after basename() to copy the
> driver name to out_driver.
>
> Also how do you prevent overflowing out_driver? Maybe it would be
> cleaner for sysfs_get_driver() to allocate out_driver and return it to
> the caller?
>
> We can return an empty string for the ENOENT case.
>
Good point. Better to alloc 'out_driver'.
I'll take care of this and other nits that you pointed out in v3.
Thank you.
Raghavendra
On Wed, Dec 10, 2025 at 10:14 AM Raghavendra Rao Ananta
<rananta@google.com> wrote:
> +
> +static int sysfs_get_device_val(const char *bdf, const char *file)
> +{
> + sysfs_get_val("devices", bdf, file);
This should be 'return sysfs_get_val("devices", bdf, file);' . I'll
fix it in v3.
> +}
On 2025-12-12 10:27 AM, Raghavendra Rao Ananta wrote:
> On Wed, Dec 10, 2025 at 10:14 AM Raghavendra Rao Ananta
> <rananta@google.com> wrote:
> > +
> > +static int sysfs_get_device_val(const char *bdf, const char *file)
> > +{
> > + sysfs_get_val("devices", bdf, file);
> This should be 'return sysfs_get_val("devices", bdf, file);' . I'll
> fix it in v3.
Can you add a patch to add -Wall and -Werror to
tools/testing/selftests/vfio/Makefile in the next version? That would
have caught both this bug and the unused ret variable in the other
patch.
© 2016 - 2026 Red Hat, Inc.