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

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

On 3/3/2025 16:44, Bernatowicz, Marcin wrote:
>
>
> On 3/3/2025 9:20 AM, Laguna, Lukasz wrote:
>>
>> 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>

Reviewed-by: 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?
>
> This should protect against 0xFFFFFFFF reads.
>
>>
>>> +
>>> +    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
>