Re: [PATCH i-g-t 1/2] lib/igt_sriov_device: Add intel_is_vf_device helper

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

On 2/19/2025 12:53, Marcin Bernatowicz wrote:
> Introduce intel_is_vf_device helper to determine whether a given device
> is a Virtual Function (VF). The function reads the VF_CAPABILITY_REGISTER
> and checks if the least significant bit is set to identify VF device.
>
> 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: Lukasz Laguna <lukasz.laguna@intel.com>
> Cc: Michał Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
> Cc: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Tomasz Lis <tomasz.lis@intel.com>
> ---
>   lib/igt_sriov_device.c | 26 ++++++++++++++++++++++++++
>   lib/igt_sriov_device.h |  1 +
>   2 files changed, 27 insertions(+)
>
> diff --git a/lib/igt_sriov_device.c b/lib/igt_sriov_device.c
> index 2b83cd43c..ffa996d6b 100644
> --- a/lib/igt_sriov_device.c
> +++ b/lib/igt_sriov_device.c
> @@ -14,6 +14,7 @@
>   #include "igt_device.h"
>   #include "igt_sriov_device.h"
>   #include "igt_sysfs.h"
> +#include "intel_io.h"
>   #include "xe/xe_query.h"
>   
>   /**
> @@ -464,3 +465,28 @@ bool igt_sriov_device_reset(int pf, unsigned int vf_num)
>   
>   	return ret;
>   }
> +
> +/**
> + * intel_is_vf_device - Check if device is VF
> + * @device: device file descriptor
> + *
> + * Determines if a device is a Virtual Function (VF)
> + * by reading VF_CAPABILITY_REGISTER. If the least
> + * significant bit is set the device is VF.
> + *
> + * Return:
> + * True if device is VF, false otherwise.
> + */
> +bool intel_is_vf_device(int fd)
> +{
> +#define VF_CAP_REG		0x1901f8
> +	struct intel_mmio_data mmio_data;
> +	uint32_t value;
> +
> +	intel_register_access_init(&mmio_data, igt_device_get_pci_device(fd), false);
> +	value = intel_register_read(&mmio_data, VF_CAP_REG);
> +	intel_register_access_fini(&mmio_data);
> +	igt_require((value & ~1) == 0);

Is it really needed?

> +
> +	return (value & 1) != 0;

just return value & 1 ?

> +}
> diff --git a/lib/igt_sriov_device.h b/lib/igt_sriov_device.h
> index 2b6acb6d7..912449086 100644
> --- a/lib/igt_sriov_device.h
> +++ b/lib/igt_sriov_device.h
> @@ -33,6 +33,7 @@ void igt_sriov_unbind_vf_drm_driver(int pf, unsigned int vf_num);
>   int igt_sriov_device_sysfs_open(int pf, unsigned int vf_num);
>   bool igt_sriov_device_reset_exists(int pf, unsigned int vf_num);
>   bool igt_sriov_device_reset(int pf, unsigned int vf_num);
> +bool intel_is_vf_device(int device);
>   
>   /**
>    * __is_valid_range - Helper to check VF range is valid