Introduce a sysfs library 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_iommu_group_get() to
align with other function names.
Signed-off-by: Raghavendra Rao Ananta <rananta@google.com>
Reviewed-by: David Matlack <dmatlack@google.com>
---
.../selftests/vfio/lib/include/libvfio.h | 1 +
.../vfio/lib/include/libvfio/sysfs.h | 12 ++
tools/testing/selftests/vfio/lib/libvfio.mk | 1 +
tools/testing/selftests/vfio/lib/sysfs.c | 144 ++++++++++++++++++
.../selftests/vfio/lib/vfio_pci_device.c | 22 +--
5 files changed, 159 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 1b6da54cc2cb7..07862b470777b 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..c48d5ef00ba6f
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/sysfs.h
@@ -0,0 +1,12 @@
+/* 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_sriov_totalvfs_get(const char *bdf);
+int sysfs_sriov_numvfs_get(const char *bdf);
+void sysfs_sriov_numvfs_set(const char *bdfs, int numvfs);
+char *sysfs_sriov_vf_bdf_get(const char *pf_bdf, int i);
+unsigned int sysfs_iommu_group_get(const char *bdf);
+char *sysfs_driver_get(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..1fec6c7a7fce7
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/sysfs.c
@@ -0,0 +1,144 @@
+// 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_val_get_int(const char *component, const char *name,
+ const char *file)
+{
+ char path[PATH_MAX];
+ char buf[32];
+ int ret;
+ int fd;
+
+ snprintf_assert(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);
+
+ errno = 0;
+ ret = strtol(buf, NULL, 0);
+ VFIO_ASSERT_EQ(errno, 0, "sysfs path \"%s\" is not an integer: \"%s\"\n", path, buf);
+
+ return ret;
+}
+
+static void sysfs_val_set(const char *component, const char *name,
+ const char *file, const char *val)
+{
+ char path[PATH_MAX];
+ int fd;
+
+ snprintf_assert(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_device_val_get(const char *bdf, const char *file)
+{
+ return sysfs_val_get_int("devices", bdf, file);
+}
+
+static void sysfs_device_val_set(const char *bdf, const char *file, const char *val)
+{
+ sysfs_val_set("devices", bdf, file, val);
+}
+
+static void sysfs_device_val_set_int(const char *bdf, const char *file, int val)
+{
+ char val_str[32];
+
+ snprintf_assert(val_str, sizeof(val_str), "%d", val);
+ sysfs_device_val_set(bdf, file, val_str);
+}
+
+int sysfs_sriov_totalvfs_get(const char *bdf)
+{
+ return sysfs_device_val_get(bdf, "sriov_totalvfs");
+}
+
+int sysfs_sriov_numvfs_get(const char *bdf)
+{
+ return sysfs_device_val_get(bdf, "sriov_numvfs");
+}
+
+void sysfs_sriov_numvfs_set(const char *bdf, int numvfs)
+{
+ sysfs_device_val_set_int(bdf, "sriov_numvfs", numvfs);
+}
+
+char *sysfs_sriov_vf_bdf_get(const char *pf_bdf, int i)
+{
+ char vf_path[PATH_MAX];
+ char path[PATH_MAX];
+ char *out_vf_bdf;
+ int ret;
+
+ out_vf_bdf = calloc(16, sizeof(char));
+ VFIO_ASSERT_NOT_NULL(out_vf_bdf);
+
+ snprintf_assert(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);
+ vf_path[ret] = '\0';
+
+ ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
+ VFIO_ASSERT_EQ(ret, 1);
+
+ return out_vf_bdf;
+}
+
+unsigned int sysfs_iommu_group_get(const char *bdf)
+{
+ char dev_iommu_group_path[PATH_MAX];
+ char path[PATH_MAX];
+ unsigned int group;
+ int ret;
+
+ snprintf_assert(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);
+ dev_iommu_group_path[ret] = '\0';
+
+ 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;
+}
+
+char *sysfs_driver_get(const char *bdf)
+{
+ char driver_path[PATH_MAX];
+ char path[PATH_MAX];
+ char *out_driver;
+ int ret;
+
+ out_driver = calloc(64, sizeof(char));
+ VFIO_ASSERT_NOT_NULL(out_driver);
+
+ snprintf_assert(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
+ ret = readlink(path, driver_path, PATH_MAX);
+ if (ret == -1) {
+ free(out_driver);
+
+ if (errno == ENOENT)
+ return NULL;
+
+ VFIO_FAIL("Failed to read %s\n", path);
+ }
+ driver_path[ret] = '\0';
+
+ strcpy(out_driver, basename(driver_path));
+ return out_driver;
+}
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index 1538d2b30ae59..82f255f0486dc 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -25,8 +25,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)
{
@@ -202,24 +200,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 = {
@@ -228,7 +208,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_iommu_group_get(bdf);
snprintf_assert(group_path, sizeof(group_path), "/dev/vfio/%d", group);
device->group_fd = open(group_path, O_RDWR);
--
2.53.0.473.g4a7958ca14-googOn 2026-03-03 19:38:17, Raghavendra Rao Ananta wrote:
> +char *sysfs_sriov_vf_bdf_get(const char *pf_bdf, int i)
> +{
> + char vf_path[PATH_MAX];
> + char path[PATH_MAX];
> + char *out_vf_bdf;
> + int ret;
> +
> + out_vf_bdf = calloc(16, sizeof(char));
A comment of /* ../0000:00:00.0 */ would be nice to tell why 16 is
chosen.
> + VFIO_ASSERT_NOT_NULL(out_vf_bdf);
> +
> + snprintf_assert(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);
> + vf_path[ret] = '\0';
Can we just initialize vf_path to {0} at the beginning and not worry
here?
> +
> + ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
> + VFIO_ASSERT_EQ(ret, 1);
> +
> + return out_vf_bdf;
> +}
> +
> +unsigned int sysfs_iommu_group_get(const char *bdf)
> +{
> + char dev_iommu_group_path[PATH_MAX];
> + char path[PATH_MAX];
> + unsigned int group;
Why unsigned int? In kernel iommu_group id is int itself. Caller of this
function also casting it to int.
> + int ret;
> +
> + snprintf_assert(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);
> + dev_iommu_group_path[ret] = '\0';
Same, initialize at beginning.
> +
> + ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
Can we combine these operations into a single function? Seems like vfio,
iommu_group and driver all use the same pattern.
> + VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> +
> + return group;
> +}
> +
> +char *sysfs_driver_get(const char *bdf)
> +{
> + char driver_path[PATH_MAX];
> + char path[PATH_MAX];
> + char *out_driver;
> + int ret;
> +
> + out_driver = calloc(64, sizeof(char));
Comment on why 64?
> + VFIO_ASSERT_NOT_NULL(out_driver);
> +
> + snprintf_assert(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
> + ret = readlink(path, driver_path, PATH_MAX);
> + if (ret == -1) {
> + free(out_driver);
> +
> + if (errno == ENOENT)
> + return NULL;
> +
> + VFIO_FAIL("Failed to read %s\n", path);
> + }
> + driver_path[ret] = '\0';
Same, initialize at beginning.
> +
> + strcpy(out_driver, basename(driver_path));
They both have different lengths. Should we use strncpy()?
> + return out_driver;
On Mon, Mar 9, 2026 at 3:06 PM Vipin Sharma <vipinsh@google.com> wrote:
>
> On 2026-03-03 19:38:17, Raghavendra Rao Ananta wrote:
> > +char *sysfs_sriov_vf_bdf_get(const char *pf_bdf, int i)
> > +{
> > + char vf_path[PATH_MAX];
> > + char path[PATH_MAX];
> > + char *out_vf_bdf;
> > + int ret;
> > +
> > + out_vf_bdf = calloc(16, sizeof(char));
>
> A comment of /* ../0000:00:00.0 */ would be nice to tell why 16 is
> chosen.
>
Sure, will add it in v7.
> > + VFIO_ASSERT_NOT_NULL(out_vf_bdf);
> > +
> > + snprintf_assert(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);
> > + vf_path[ret] = '\0';
>
> Can we just initialize vf_path to {0} at the beginning and not worry
> here?
>
Hmm, we can but we don't want to fill 4K (PATH_MAX) worth of 0s for
every function call. Hence, I preferred to set it after the call to
readlink(). This also seems to be the common practise, at least by
looking in the "tools/" dir. With David's suggestion of introducing
readlink_safe(), the caller need not even worry about this. Same
response to the similar comments below.
> > +
> > + ret = sscanf(basename(vf_path), "%s", out_vf_bdf);
> > + VFIO_ASSERT_EQ(ret, 1);
> > +
> > + return out_vf_bdf;
> > +}
> > +
> > +unsigned int sysfs_iommu_group_get(const char *bdf)
> > +{
> > + char dev_iommu_group_path[PATH_MAX];
> > + char path[PATH_MAX];
> > + unsigned int group;
>
> Why unsigned int? In kernel iommu_group id is int itself. Caller of this
> function also casting it to int.
>
Ah, it should be 'int'. I guess I copied the original code as is. I'll
update it in v7.
> > + int ret;
> > +
> > + snprintf_assert(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);
> > + dev_iommu_group_path[ret] = '\0';
>
> Same, initialize at beginning.
>
> > +
> > + ret = sscanf(basename(dev_iommu_group_path), "%u", &group);
>
> Can we combine these operations into a single function? Seems like vfio,
> iommu_group and driver all use the same pattern.
>
I think we can, at least for the vf_driver and the iommu_group. The
'sysfs_driver_get' handles things slightly differently. I'll update in
v7.
> > + VFIO_ASSERT_EQ(ret, 1, "Failed to get the IOMMU group for device: %s\n", bdf);
> > +
> > + return group;
> > +}
> > +
> > +char *sysfs_driver_get(const char *bdf)
> > +{
> > + char driver_path[PATH_MAX];
> > + char path[PATH_MAX];
> > + char *out_driver;
> > + int ret;
> > +
> > + out_driver = calloc(64, sizeof(char));
>
> Comment on why 64?
>
Will do in v7.
> > + VFIO_ASSERT_NOT_NULL(out_driver);
> > +
> > + snprintf_assert(path, PATH_MAX, "/sys/bus/pci/devices/%s/driver", bdf);
> > + ret = readlink(path, driver_path, PATH_MAX);
> > + if (ret == -1) {
> > + free(out_driver);
> > +
> > + if (errno == ENOENT)
> > + return NULL;
> > +
> > + VFIO_FAIL("Failed to read %s\n", path);
> > + }
> > + driver_path[ret] = '\0';
>
> Same, initialize at beginning.
>
> > +
> > + strcpy(out_driver, basename(driver_path));
>
> They both have different lengths. Should we use strncpy()?
>
The assumption was that 'out_driver' is allocated to accommodate a max
driver length and should fit any driver name. But I can switch to
strncpy() to make this more clear in v7.
Thank you.
Raghavendra
On 2026-03-03 07:38 PM, Raghavendra Rao Ananta wrote:
> + ret = readlink(path, vf_path, PATH_MAX);
> + VFIO_ASSERT_NE(ret, -1);
> + vf_path[ret] = '\0';
...
> + 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);
> + dev_iommu_group_path[ret] = '\0';
...
> + ret = readlink(path, driver_path, PATH_MAX);
> + if (ret == -1) {
> + free(out_driver);
> +
> + if (errno == ENOENT)
> + return NULL;
> +
> + VFIO_FAIL("Failed to read %s\n", path);
> + }
> + driver_path[ret] = '\0';
These can all write off the end of the buffer if ret == PATH_MAX.
Also there is an inconsistency in these calls. 2 use hard-coded PATH_MAX
while the other uses sizeof().
Perhaps add a wrapper to handle all the details?
#define readlink_safe(path, buf) ({ \
int __ret = readlink(_path, _buf, sizeof(_buf) - 1); \
if (__ret != -1) \
_buf[__ret] = 0; \
__ret;
})
On Wed, Mar 4, 2026 at 2:53 PM David Matlack <dmatlack@google.com> wrote:
>
> On 2026-03-03 07:38 PM, Raghavendra Rao Ananta wrote:
>
> > + ret = readlink(path, vf_path, PATH_MAX);
> > + VFIO_ASSERT_NE(ret, -1);
> > + vf_path[ret] = '\0';
>
> ...
>
> > + 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);
> > + dev_iommu_group_path[ret] = '\0';
>
> ...
>
> > + ret = readlink(path, driver_path, PATH_MAX);
> > + if (ret == -1) {
> > + free(out_driver);
> > +
> > + if (errno == ENOENT)
> > + return NULL;
> > +
> > + VFIO_FAIL("Failed to read %s\n", path);
> > + }
> > + driver_path[ret] = '\0';
>
> These can all write off the end of the buffer if ret == PATH_MAX.
>
> Also there is an inconsistency in these calls. 2 use hard-coded PATH_MAX
> while the other uses sizeof().
>
> Perhaps add a wrapper to handle all the details?
>
> #define readlink_safe(path, buf) ({ \
> int __ret = readlink(_path, _buf, sizeof(_buf) - 1); \
> if (__ret != -1) \
> _buf[__ret] = 0; \
> __ret;
> })
Yeah, good idea. I'll consider this in v7.
Thank you.
Raghavendra
© 2016 - 2026 Red Hat, Inc.