public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>
To: "Laguna, Lukasz" <lukasz.laguna@intel.com>,
	igt-dev@lists.freedesktop.org
Cc: Adam Miszczak <adam.miszczak@linux.intel.com>,
	Jakub Kolakowski <jakub1.kolakowski@intel.com>,
	Kamil Konieczny <kamil.konieczny@linux.intel.com>
Subject: Re: [PATCH v3 i-g-t 2/7] lib/igt_pci: Add generic PCI driver override and bind/unbind helpers
Date: Wed, 11 Feb 2026 12:21:07 +0100	[thread overview]
Message-ID: <a5dce3b2-4627-4719-bf52-1f676e38ff0b@linux.intel.com> (raw)
In-Reply-To: <3f0acb60-218d-4935-ba2f-0314ba481cf3@intel.com>


On 2/11/2026 7:39 AM, Laguna, Lukasz wrote:
>
> On 2/10/2026 11:19, Bernatowicz, Marcin wrote:
>>
>> On 2/9/2026 11:16 AM, Laguna, Lukasz wrote:
>>>
>>> On 2/4/2026 17:32, Marcin Bernatowicz wrote:
>>>> Add generic helpers for controlling PCI driver binding via sysfs.
>>>>
>>>> The new APIs provide driver- and device-centric primitives for:
>>>>    - setting and clearing driver_override
>>>>    - triggering PCI driver reprobe
>>>>    - binding and unbinding devices to a specific PCI driver
>>>>    - query the currently bound PCI driver
>>>>
>>>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
>>>> Cc: Adam Miszczak <adam.miszczak@linux.intel.com>
>>>> Cc: Jakub Kolakowski <jakub1.kolakowski@intel.com>
>>>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>>>> Cc: Lukasz Laguna <lukasz.laguna@intel.com>
>>>>
>>>> ---
>>>> v2:
>>>> - Add igt_pci_get_bound_driver_name() to query the currently bound PCI
>>>>    driver via the /sys/bus/pci/devices/<BDF>/driver symlink.
>>>> - Extend igt_pci_bind_driver_override() and 
>>>> igt_pci_unbind_driver_override()
>>>>    with a timeout_ms parameter so callers can wait for bind/unbind to
>>>>    actually complete, instead of relying on drivers_probe write 
>>>> success.
>>>>
>>>> ---
>>>>   lib/igt_pci.c | 351 
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   lib/igt_pci.h |  13 +-
>>>>   2 files changed, 363 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/igt_pci.c b/lib/igt_pci.c
>>>> index 61aaf939d..80aaf07c5 100644
>>>> --- a/lib/igt_pci.c
>>>> +++ b/lib/igt_pci.c
>>>> @@ -3,9 +3,18 @@
>>>>    * Copyright © 2022 Intel Corporation
>>>>    */
>>>>   +#include <ctype.h>
>>>> +#include <dirent.h>
>>>> +#include <errno.h>
>>>> +#include <fcntl.h>
>>>> +#include <limits.h>
>>>> +#include <string.h>
>>>> +#include <unistd.h>
>>>>   #include <pciaccess.h>
>>>> +#include "igt_aux.h"
>>>>   #include "igt_core.h"
>>>>   #include "igt_pci.h"
>>>> +#include "igt_sysfs.h"
>>>>     static int find_pci_cap_offset_at(struct pci_device *dev, enum 
>>>> pci_cap_id cap_id,
>>>>                     int start_offset)
>>>> @@ -51,3 +60,345 @@ int find_pci_cap_offset(struct pci_device *dev, 
>>>> enum pci_cap_id cap_id)
>>>>   {
>>>>       return find_pci_cap_offset_at(dev, cap_id, PCI_CAPS_START);
>>>>   }
>>>> +
>>>> +static int open_pci_driver_dir(const char *driver)
>>>> +{
>>>> +    char path[PATH_MAX];
>>>> +
>>>> +    snprintf(path, sizeof(path), "/sys/bus/pci/drivers/%s", driver);
>>>> +    return open(path, O_RDONLY | O_CLOEXEC);
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_device_unbind:
>>>> + * @pci_slot: BDF like "0000:01:00.0"
>>>> + *
>>>> + * Unbind @pci_slot from its currently bound driver, if any.
>>>> + * Returns 0 on success, or a negative errno-like value.
>>>> + */
>>>> +int igt_pci_device_unbind(const char *pci_slot)
>>>> +{
>>>> +    char path[PATH_MAX];
>>>> +    int dirfd;
>>>> +    int ret;
>>>> +
>>>> +    snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/driver", 
>>>> pci_slot);
>>>> +    dirfd = open(path, O_RDONLY | O_CLOEXEC);
>>>> +    if (dirfd < 0)
>>>> +        return 0; /* already unbound */
>>>> +
>>>> +    ret = igt_sysfs_set(dirfd, "unbind", pci_slot) ? 0 : -errno;
>>>> +    close(dirfd);
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_driver_bind:
>>>> + * @driver: PCI driver name under /sys/bus/pci/drivers/<driver>
>>>> + * @pci_slot: device to bind
>>>> + *
>>>> + * Bind @pci_slot to @driver. Driver must be present/loaded.
>>>> + * Returns 0 on success, or a negative errno-like value.
>>>> + */
>>>> +int igt_pci_driver_bind(const char *driver, const char *pci_slot)
>>>> +{
>>>> +    int dirfd, ret;
>>>> +
>>>> +    dirfd = open_pci_driver_dir(driver);
>>>> +    if (dirfd < 0)
>>>> +        return -errno;
>>>> +
>>>> +    ret = igt_sysfs_set(dirfd, "bind", pci_slot) ? 0 : -errno;
>>>> +    close(dirfd);
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_driver_unbind:
>>>> + * @driver: PCI driver name
>>>> + * @pci_slot: device to unbind
>>>> + *
>>>> + * Unbind @pci_slot from @driver.
>>>> + * Returns 0 on success, or a negative errno-like value.
>>>> + */
>>>> +int igt_pci_driver_unbind(const char *driver, const char *pci_slot)
>>>> +{
>>>> +    int dirfd, ret;
>>>> +
>>>> +    dirfd = open_pci_driver_dir(driver);
>>>> +    if (dirfd < 0)
>>>> +        return -errno;
>>>> +
>>>> +    ret = igt_sysfs_set(dirfd, "unbind", pci_slot) ? 0 : -errno;
>>>> +    close(dirfd);
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_driver_unbind_all:
>>>> + * @driver: PCI driver name
>>>> + *
>>>> + * Unbind all devices currently bound to @driver.
>>>> + * Returns 0 on success, or a negative errno-like value.
>>>> + */
>>>> +int igt_pci_driver_unbind_all(const char *driver)
>>>> +{
>>>> +    char path[PATH_MAX];
>>>> +    DIR *dir;
>>>> +    struct dirent *de;
>>>> +    int driver_fd;
>>>> +
>>>> +    snprintf(path, sizeof(path), "/sys/bus/pci/drivers/%s", driver);
>>>> +    dir = opendir(path);
>>>> +    if (!dir)
>>>> +        return -errno;
>>>> +
>>>> +    driver_fd = dirfd(dir);
>>>> +
>>>> +    while ((de = readdir(dir))) {
>>>> +        bool ok;
>>>> +
>>>> +        /* BDF symlinks are like "0000:01:00.0" and start with 
>>>> digit */
>>>> +        if (de->d_type != DT_LNK || !isdigit(de->d_name[0]))
>>>> +            continue;
>>>> +
>>>> +        ok = igt_sysfs_set(driver_fd, "unbind", de->d_name);
>>>> +        if (!ok) {
>>>> +            int err = -errno;
>>>> +
>>>> +            closedir(dir);
>>>> +            return err;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    closedir(dir);
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_set_driver_override:
>>>> + * @pci_slot: PCI device BDF (e.g. "0000:01:00.0")
>>>> + * @driver: PCI driver name to force-bind (e.g. "xe-vfio-pci"), or
>>>> + *          NULL / empty string to clear an existing override
>>>> + *
>>>> + * Set or clear the PCI driver_override for @pci_slot via sysfs.
>>>> + *
>>>> + * This does not trigger driver reprobe by itself. Call
>>>> + * igt_pci_probe_drivers() afterwards to apply the override.
>>>> + *
>>>> + * Returns: 0 on success, negative errno on failure.
>>>> + */
>>>> +int igt_pci_set_driver_override(const char *pci_slot, const char 
>>>> *driver)
>>>> +{
>>>> +    char devpath[PATH_MAX];
>>>> +    int dev;
>>>> +    bool ok;
>>>> +
>>>> +    snprintf(devpath, sizeof(devpath), "/sys/bus/pci/devices/%s", 
>>>> pci_slot);
>>>> +    dev = open(devpath, O_DIRECTORY | O_RDONLY);
>>>> +    if (dev < 0)
>>>> +        return -errno;
>>>> +
>>>> +    ok = igt_sysfs_set(dev, "driver_override", driver ? driver : "");
>>>> +    close(dev);
>>>> +
>>>> +    return ok ? 0 : -errno;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_probe_drivers:
>>>> + * @pci_slot: PCI device BDF (e.g. "0000:01:00.0")
>>>> + *
>>>> + * Trigger PCI driver reprobe for @pci_slot by writing to
>>>> + * /sys/bus/pci/drivers_probe.
>>>> + *
>>>> + * This causes the kernel to attempt binding the device, honoring any
>>>> + * driver_override previously set.
>>>> + *
>>>> + * Note: a successful write only means the reprobe request was 
>>>> accepted.
>>>> + * It does not guarantee that a driver actually bound to the device.
>>>> + *
>>>> + * Returns: 0 on success, negative errno on failure.
>>>> + */
>>>> +int igt_pci_probe_drivers(const char *pci_slot)
>>>> +{
>>>> +    int pci;
>>>> +    bool ok;
>>>> +
>>>> +    pci = open("/sys/bus/pci", O_DIRECTORY | O_RDONLY);
>>>> +    if (pci < 0)
>>>> +        return -errno;
>>>> +
>>>> +    ok = igt_sysfs_set(pci, "drivers_probe", pci_slot);
>>>> +    close(pci);
>>>> +
>>>> +    return ok ? 0 : -errno;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_get_bound_driver_name:
>>>> + * @pci_slot: PCI device BDF (e.g. "0000:01:00.0")
>>>> + * @driver: destination buffer for the bound driver name
>>>> + * @driver_len: size of @driver in bytes
>>>> + *
>>>> + * Read the currently bound PCI driver name for @pci_slot by 
>>>> inspecting the
>>>> + * /sys/bus/pci/devices/<BDF>/driver symlink.
>>>> + *
>>>> + * Return values:
>>>> + *  1: device is bound and @driver contains the driver name
>>>> + *  0: device is unbound (no driver symlink)
>>>> + * <0: negative errno-like value on error
>>>> + */
>>>> +int igt_pci_get_bound_driver_name(const char *pci_slot, char 
>>>> *driver, size_t driver_len)
>>>> +{
>>>> +    char path[PATH_MAX];
>>>> +    char link[PATH_MAX];
>>>> +    const char *base;
>>>> +    ssize_t len;
>>>> +
>>>> +    if (driver && driver_len)
>>>> +        driver[0] = '\0';
>>>> +
>>>> +    snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/driver", 
>>>> pci_slot);
>>>> +    len = readlink(path, link, sizeof(link) - 1);
>>>> +    if (len < 0) {
>>>> +        if (errno == ENOENT)
>>>> +            return 0; /* unbound */
>>>> +
>>>> +        return -errno;
>>>> +    }
>>>> +
>>>> +    link[len] = '\0';
>>>> +    base = strrchr(link, '/');
>>>> +    base = base ? base + 1 : link;
>>>> +
>>>> +    if (driver && driver_len)
>>>
>>> You can check the input params at the beginning of the function and 
>>> return error if they are invalid.
>>
>> driver/driver_len are optional, callers may pass NULL and/or 0 when 
>> they only need the
>>
>> return value (bound/unbound) and don’t care about the driver name...
>>
>
> Right... now I see it, but it wasn't clear at first glance. Maybe 
> function doc should be updated?
> Or probably it would be even better to introduce separate helper 
> igt_pci_is_driver_bound()? It would call 
> igt_pci_get_bound_driver_name() without passing buffer.
>
Send v4 with updated doc.
>>>
>>>> +        snprintf(driver, driver_len, "%s", base);
>>>> +
>>>> +    return 1;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_bind_driver_override:
>>>> + * @pci_slot: PCI device BDF (e.g. "0000:01:00.0")
>>>> + * @driver: PCI driver name to bind (must not be NULL or empty)
>>>> + * @timeout_ms: how long to wait for the device to become bound.
>>>> + *              If 0, don't wait (best-effort immediate check only).
>>>> + *
>>>> + * Bind @pci_slot to @driver using the driver_override mechanism.
>>>> + *
>>>> + * This helper sets driver_override and immediately triggers driver
>>>> + * reprobe so that the device is bound to the requested driver.
>>>> + *
>>>> + * Returns: 0 on success, negative errno-like value on failure.
>>>> + * A reprobe request can be accepted by sysfs while the driver probe
>>>> + * fails later; this helper verifies the device ended up bound.
>>>> + *
>>>> + * On bind failure, returns a negative error and the failure 
>>>> reason may
>>>> + * also be logged to dmesg by the kernel driver.
>>>> + */
>>>> +int igt_pci_bind_driver_override(const char *pci_slot, const char 
>>>> *driver,
>>>> +                 unsigned int timeout_ms)
>>>> +{
>>>> +    int ret;
>>>> +    char bound[64];
>>>> +    int bound_ret;
>>>> +    bool bound_ok;
>>>> +
>>>> +    if (!driver || !driver[0])
>>>> +        return -EINVAL;
>>>> +
>>>> +    ret = igt_pci_set_driver_override(pci_slot, driver);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    ret = igt_pci_probe_drivers(pci_slot);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    /*
>>>> +     * Writing to drivers_probe only tells us the kernel accepted 
>>>> the request.
>>>> +     * The actual driver probe may still fail (and only be 
>>>> reported via dmesg).
>>>> +     * Verify that the device ended up bound to the requested driver.
>>>> +     */
>>>> +    bound_ret = igt_pci_get_bound_driver_name(pci_slot, bound, 
>>>> sizeof(bound));
>>>> +    if (bound_ret < 0)
>>>> +        return bound_ret;
>>>> +
>>>> +    if (timeout_ms == 0) {
>>>> +        /*
>>>> +         * No waiting requested. If the device is already bound, 
>>>> validate
>>>> +         * it is bound to the expected driver; otherwise treat as
>>>> +         * best-effort request-only success.
>>>> +         */
>>>> +        if (bound_ret > 0 && strcmp(bound, driver))
>>>> +            return -EBUSY;
>>>> +
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    bound_ok = igt_wait((bound_ret =
>>>> +                 igt_pci_get_bound_driver_name(pci_slot, bound, 
>>>> sizeof(bound))) != 0,
>>>> +                timeout_ms, 1);
>>>> +    if (!bound_ok)
>>>> +        return -EIO;
>>>> +
>>>> +    if (bound_ret < 0)
>>>> +        return bound_ret;
>>>> +
>>>> +    if (strcmp(bound, driver))
>>>> +        return -EBUSY;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +/**
>>>> + * igt_pci_unbind_driver_override:
>>>> + * @pci_slot: PCI device BDF (e.g. "0000:01:00.0")
>>>> + * @timeout_ms: how long to wait for the device to become unbound.
>>>> + *              If 0, don't wait (best-effort immediate check only).
>>>> + *
>>>> + * Unbind @pci_slot from its currently bound driver (if any) and 
>>>> clear
>>>> + * any driver_override setting.
>>>> + *
>>>> + * This is the inverse operation of igt_pci_bind_driver_override().
>>>> + *
>>>> + * Returns: 0 on success, negative errno on failure.
>>>> + */
>>>> +int igt_pci_unbind_driver_override(const char *pci_slot, unsigned 
>>>> int timeout_ms)
>>>> +{
>>>> +    int ret;
>>>> +    int bound_ret;
>>>> +    char bound[64];
>>>> +    bool unbound_ok;
>>>> +
>>>> +    ret = igt_pci_device_unbind(pci_slot);
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    ret = igt_pci_set_driver_override(pci_slot, "");
>>>> +    if (ret)
>>>> +        return ret;
>>>> +
>>>> +    bound_ret = igt_pci_get_bound_driver_name(pci_slot, bound, 
>>>> sizeof(bound));
>>>> +    if (bound_ret < 0)
>>>> +        return bound_ret;
>>>> +
>>>> +    if (timeout_ms == 0)
>>>> +        return 0;
>>>> +
>>>> +    /* Verify the device actually ends up unbound (driver symlink 
>>>> removed). */
>>>> +    unbound_ok = igt_wait((bound_ret =
>>>> +                   igt_pci_get_bound_driver_name(pci_slot, bound, 
>>>> sizeof(bound))) == 0,
>>>> +                  timeout_ms, 1);
>>>> +    if (!unbound_ok)
>>>> +        return -EBUSY;
>>>> +
>>>> +    if (bound_ret < 0)
>>>> +        return bound_ret;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> diff --git a/lib/igt_pci.h b/lib/igt_pci.h
>>>> index 92b9cc392..a66eeebf2 100644
>>>> --- a/lib/igt_pci.h
>>>> +++ b/lib/igt_pci.h
>>>> @@ -6,8 +6,9 @@
>>>>   #ifndef __IGT_PCI_H__
>>>>   #define __IGT_PCI_H__
>>>>   -#include <stdint.h>
>>>>   #include <endian.h>
>>>> +#include <stddef.h>
>>>> +#include <stdint.h>
>>>>     /* forward declaration */
>>>>   struct pci_device;
>>>> @@ -24,5 +25,15 @@ enum pci_cap_id {
>>>>   #define  PCI_SLOT_PWR_CTRL_PRESENT (1 << 1)
>>>>     int find_pci_cap_offset(struct pci_device *dev, enum pci_cap_id 
>>>> cap_id);
>>>> +int igt_pci_device_unbind(const char *pci_slot);
>>>> +int igt_pci_driver_bind(const char *driver, const char *pci_slot);
>>>> +int igt_pci_driver_unbind(const char *driver, const char *pci_slot);
>>>> +int igt_pci_driver_unbind_all(const char *driver);
>>>> +int igt_pci_set_driver_override(const char *pci_slot, const char 
>>>> *driver);
>>>> +int igt_pci_probe_drivers(const char *pci_slot);
>>>> +int igt_pci_get_bound_driver_name(const char *pci_slot, char 
>>>> *driver, size_t driver_len);
>>>> +int igt_pci_bind_driver_override(const char *pci_slot, const char 
>>>> *driver,
>>>> +                 unsigned int timeout_ms);
>>>> +int igt_pci_unbind_driver_override(const char *pci_slot, unsigned 
>>>> int timeout_ms);
>>>>     #endif

  reply	other threads:[~2026-02-11 11:21 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-04 16:32 [PATCH v3 i-g-t 0/7] PCI driver helpers and xe-vfio-pci FLR improvement Marcin Bernatowicz
2026-02-04 16:32 ` [PATCH v3 i-g-t 1/7] lib/igt_sriov_device: Add helper to get VF PCI slot address Marcin Bernatowicz
2026-02-04 16:32 ` [PATCH v3 i-g-t 2/7] lib/igt_pci: Add generic PCI driver override and bind/unbind helpers Marcin Bernatowicz
2026-02-09 10:16   ` Laguna, Lukasz
2026-02-10 10:19     ` Bernatowicz, Marcin
2026-02-11  6:39       ` Laguna, Lukasz
2026-02-11 11:21         ` Bernatowicz, Marcin [this message]
2026-02-04 16:32 ` [PATCH v3 i-g-t 3/7] tests/intel/xe_sriov_flr: Attach VFs to xe-vfio-pci before initiating FLR Marcin Bernatowicz
2026-02-09 10:17   ` Laguna, Lukasz
2026-02-04 16:32 ` [PATCH v3 i-g-t 4/7] lib/igt_kmod: Fix PCI bind/unbind for module/driver name mismatch Marcin Bernatowicz
2026-02-04 16:32 ` [PATCH v3 i-g-t 5/7] tests/intel/xe_sriov_flr: Add --wait-flr-ms option Marcin Bernatowicz
2026-02-09 10:18   ` Laguna, Lukasz
2026-02-04 16:32 ` [PATCH v3 i-g-t 6/7] tests/intel/xe_sriov_flr: Add --no-xe-vfio-pci option Marcin Bernatowicz
2026-02-09 10:18   ` Laguna, Lukasz
2026-02-04 16:32 ` [PATCH v3 i-g-t 7/7] tests/intel/xe_sriov_flr: Skip xe-vfio-pci load/bind when IOMMU is off Marcin Bernatowicz
2026-02-09 10:42   ` Laguna, Lukasz
2026-02-10 11:23     ` Bernatowicz, Marcin
2026-02-11  6:43       ` Laguna, Lukasz
2026-02-04 18:27 ` ✓ Xe.CI.BAT: success for PCI driver helpers and xe-vfio-pci FLR improvement (rev3) Patchwork
2026-02-04 18:41 ` ✓ i915.CI.BAT: " Patchwork
2026-02-05  5:02 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-02-05 10:13   ` Bernatowicz, Marcin
2026-02-05  8:13 ` ✓ i915.CI.Full: success " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5dce3b2-4627-4719-bf52-1f676e38ff0b@linux.intel.com \
    --to=marcin.bernatowicz@linux.intel.com \
    --cc=adam.miszczak@linux.intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jakub1.kolakowski@intel.com \
    --cc=kamil.konieczny@linux.intel.com \
    --cc=lukasz.laguna@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox