Re: [PATCH i-g-t] lib/igt_sriov_device: Replace libpciaccess with direct sysfs interaction

["Laguna, Lukasz" <lukasz.laguna@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3477 bytes --]


On 6/27/2024 16:51, Marcin Bernatowicz wrote:
> Previously, libpciaccess was used to fetch PCI device information for
> VFs in SR-IOV, which does not refresh the PCI bus state after the initial
> scan. This can lead to outdated PCI information in dynamic SR-IOV
> environments where VFs are dynamically managed.
>
> The new implementation directly interacts with sysfs to resolve the PCI
> slot address of a VF, ensuring access to the most current state of the
> PCI bus. This change removes the dependency on libpciaccess and uses
> __igt_sriov_get_vf_pci_slot_alloc to fetch the PCI slot addresses
> directly from sysfs based on VF numbers.
>
> Reported-by: Michal Wajdeczko<michal.wajdeczko@intel.com>
> Closes:https://intel-gfx-ci.01.org/tree/intel-xe/xe-pw-135423v1/shard-adlp-9/igt@sriov_basic@bind-unbind-vf@vf-2.html
> Cc: Michal Wajdeczko<michal.wajdeczko@intel.com>
> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
> Cc: Kamil Konieczny<kamil.konieczny@linux.intel.com>
> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
> ---
>   lib/igt_sriov_device.c | 46 +++++++++++++++++++++++++++++++++++-------
>   1 file changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/lib/igt_sriov_device.c b/lib/igt_sriov_device.c
> index a0ef05d5c..416d8f4aa 100644
> --- a/lib/igt_sriov_device.c
> +++ b/lib/igt_sriov_device.c
> @@ -297,26 +297,58 @@ bool igt_sriov_is_vf_drm_driver_probed(int pf, unsigned int vf_num)
>   	return ret;
>   }
>   
> +/*
> + * __igt_sriov_get_vf_pci_slot_alloc:
> + * @pf_sysfs: sysfs directory file descriptor
> + * @vf_num: VF number (1-based)
> + *
> + * Resolve symbolic link from virtfnX to obtain the PCI slot address.
> + * Returns a dynamically allocated string containing the PCI slot address,
> + * or NULL if the link cannot be resolved.
> + * The caller is responsible for freeing the returned memory.
> + */
> +static char *__igt_sriov_get_vf_pci_slot_alloc(int pf_sysfs, unsigned int vf_num)
> +{
> +	char dir_path[PATH_MAX];
> +	char path[PATH_MAX];
> +	char *pci_slot_addr;
> +	int len;
> +
> +	/* Adjust for 0-based index as vf_num is 1-based */
> +	if (vf_num)
> +		snprintf(dir_path, sizeof(dir_path), "device/virtfn%u",
> +			 vf_num - 1);
> +	else
> +		snprintf(dir_path, sizeof(dir_path), "device");
> +
> +	len = readlinkat(pf_sysfs, dir_path, path, sizeof(path));
> +	if (len <= 0)
> +		return NULL;
> +
> +	path[len] = '\0';
> +	pci_slot_addr = strrchr(path, '/') + 1;
> +
> +	return pci_slot_addr ? strdup(pci_slot_addr) : NULL;
> +}
> +
>   static bool __igt_sriov_bind_vf_drm_driver(int pf, unsigned int vf_num, bool bind)
>   {
> -	struct pci_device *pci_dev;
> -	char pci_slot[14];
> +	char *pci_slot;
>   	int sysfs;
>   	bool ret;
>   
>   	igt_assert(vf_num > 0);
>   
> -	pci_dev = __igt_device_get_pci_device(pf, vf_num);
> -	igt_assert_f(pci_dev, "No PCI device for given VF number: %d\n", vf_num);
> -	sprintf(pci_slot, "%04x:%02x:%02x.%x",
> -		pci_dev->domain_16, pci_dev->bus, pci_dev->dev, pci_dev->func);
> -
>   	sysfs = igt_sysfs_open(pf);
>   	igt_assert_fd(sysfs);
>   
> +	pci_slot = __igt_sriov_get_vf_pci_slot_alloc(sysfs, vf_num);
> +	igt_assert(pci_slot);
> +
>   	igt_debug("vf_num: %u, pci_slot: %s\n", vf_num, pci_slot);
>   	ret = igt_sysfs_set(sysfs, bind ? "device/driver/bind" : "device/driver/unbind", pci_slot);
>   
> +	free(pci_slot);
>   	close(sysfs);
>   
>   	return ret;

Reviewed-by: Lukasz Laguna <lukasz.laguna@intel.com 
<mailto:lukasz.laguna@intel.com>>

[-- Attachment #2: Type: text/html, Size: 4630 bytes --]