Re: [PATCH i-g-t 1/5] lib/xe/xe_sriov_debugfs: Add debugfs get/set functions for u32, u64, bool

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

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

On 12/18/2024 13:00, Marcin Bernatowicz wrote:
> Add helper functions to get and set SR-IOV debugfs attributes for u32,
> u64, and boolean types.
>
> Functions added:
> - __xe_sriov_pf_debugfs_get_u32
> - __xe_sriov_pf_debugfs_set_u32
> - __xe_sriov_pf_debugfs_get_u64
> - __xe_sriov_pf_debugfs_set_u64
> - __xe_sriov_pf_debugfs_get_boolean
> - __xe_sriov_pf_debugfs_set_boolean
>
> 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: Narasimha C V<narasimha.c.v@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/xe/xe_sriov_debugfs.c | 151 ++++++++++++++++++++++++++++++++++++++
>   lib/xe/xe_sriov_debugfs.h |  18 +++++
>   2 files changed, 169 insertions(+)
>
> diff --git a/lib/xe/xe_sriov_debugfs.c b/lib/xe/xe_sriov_debugfs.c
> index c87f91492..121dabe73 100644
> --- a/lib/xe/xe_sriov_debugfs.c
> +++ b/lib/xe/xe_sriov_debugfs.c
> @@ -9,6 +9,7 @@
>   #include "drmtest.h"
>   #include "igt_debugfs.h"
>   #include "igt_sriov_device.h"
> +#include "igt_sysfs.h"
>   #include "xe/xe_query.h"
>   #include "xe/xe_sriov_debugfs.h"
>   #include "xe/xe_sriov_provisioning.h"
> @@ -204,3 +205,153 @@ cleanup:
>   
>   	return ret;
>   }
> +
> +static int xe_sriov_pf_debugfs_path_open(int pf, unsigned int vf_num,
> +					 unsigned int gt_num)
> +{
> +	char path[PATH_MAX];
> +
> +	if (igt_debug_on_f(!xe_sriov_pf_debugfs_path(pf, vf_num, gt_num, path,
> +						     sizeof(path)),
> +			   "path: %s\n", path))
> +		return -1;
> +
> +	return open(path, O_RDONLY);
> +}
> +
> +/**
> + * DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC - Define a function for accessing debugfs attributes
> + * @type: Data type of the value to read or write (e.g., `uint32_t`, `bool`, etc.)
> + * @suffix: Function suffix, appended to `__xe_sriov_pf_debugfs_` to name the function

nit: maybe "Function name suffix appended to `__xe_sriov_pf_debugfs_`" ?

> + * @sysfs_func: The sysfs helper function to perform the actual read or write operation
> + *
> + * Generates a function for accessing a debugfs attribute of a PF device.
> + * It handles opening the debugfs path, performing the sysfs operation, and closing the
> + * debugfs directory.
> + *
> + * The generated function has the following signature:
> + *
> + *	int __xe_sriov_pf_debugfs_<suffix>(int pf, unsigned int vf_num,
> + *					   unsigned int gt_num,
> + *					   const char *attr, type value)
> + *
> + * where:
> + * - `pf` is the PF device file descriptor.
> + * - `vf_num` is the VF number.
> + * - `gt_num` is the GT number.
> + * - `attr` is the name of the debugfs attribute.
> + * - `value` is the data to read or write, depending on the sysfs function.
> + *
> + * Example:
> + *
> + *	DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(uint32_t, set_u32, __igt_sysfs_set_u32);
> + *
> + * This expands to a function:
> + *
> + *	int __xe_sriov_pf_debugfs_set_u32(int pf, unsigned int vf_num,
> + *					  unsigned int gt_num,
> + *					  const char *attr, uint32_t value);
> + *
> + * The function returns:
> + * - `0` on success
> + * - Negative error code on failure
> + */
> +#define DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(type, suffix, sysfs_func)		\
> +	int __xe_sriov_pf_debugfs_##suffix(int pf, unsigned int vf_num,		\
> +					   unsigned int gt_num,			\
> +					   const char *attr, type value)	\
> +	{									\
> +		bool ret;							\
> +		int dir = xe_sriov_pf_debugfs_path_open(pf, vf_num, gt_num);	\
> +										\
> +		if (igt_debug_on(dir < 0))					\
> +			return dir;						\
> +										\
> +		ret = sysfs_func(dir, attr, value);				\
> +		close(dir);							\
> +		return ret ? 0 : -1;						\
> +	}
> +
> +/**
> + * __xe_sriov_pf_debugfs_get_u32 - Get a 32-bit unsigned integer from debugfs
> + * @pf: PF device file descriptor
> + * @vf_num: VF number
> + * @gt_num: GT number
> + * @attr: Debugfs attribute to read
> + * @value: Pointer to store the retrieved value
> + *
> + * Reads a 32-bit unsigned integer from the specified debugfs attribute.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(uint32_t *, get_u32, __igt_sysfs_get_u32)
> +
> +/**
> + * __xe_sriov_pf_debugfs_set_u32 - Set a 32-bit unsigned integer in debugfs
> + * @pf: PF device file descriptor
> + * @vf_num: VF number
> + * @gt_num: GT number
> + * @attr: Debugfs attribute to write to
> + * @value: The value to set
> + *
> + * Writes a 32-bit unsigned integer to the specified debugfs attribute.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(uint32_t, set_u32, __igt_sysfs_set_u32)
> +
> +/**
> + * __xe_sriov_pf_debugfs_get_u64 - Get a 64-bit unsigned integer from debugfs
> + * @pf: PF device file descriptor
> + * @vf_num: VF number
> + * @gt_num: GT number
> + * @attr: Debugfs attribute to read
> + * @value: Pointer to store the retrieved value
> + *
> + * Reads a 64-bit unsigned integer from the specified debugfs attribute.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(uint64_t *, get_u64, __igt_sysfs_get_u64)
> +
> +/**
> + * __xe_sriov_pf_debugfs_set_u64 - Set a 64-bit unsigned integer in debugfs
> + * @pf: PF device file descriptor
> + * @vf_num: VF number
> + * @gt_num: GT number
> + * @attr: Debugfs attribute to write to
> + * @value: The value to set
> + *
> + * Writes a 64-bit unsigned integer to the specified debugfs attribute.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(uint64_t, set_u64, __igt_sysfs_set_u64)
> +
> +/**
> + * __xe_sriov_pf_debugfs_get_boolean - Get a boolean value from debugfs
> + * @pf: PF device file descriptor
> + * @vf_num: VF number
> + * @gt_num: GT number
> + * @attr: Debugfs attribute to read
> + * @value: Pointer to store the retrieved value
> + *
> + * Reads a boolean value from the specified debugfs attribute.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(bool *, get_boolean, __igt_sysfs_get_boolean)
> +
> +/**
> + * __xe_sriov_pf_debugfs_set_boolean - Set a boolean value in debugfs
> + * @pf: PF device file descriptor
> + * @vf_num: VF number
> + * @gt_num: GT number
> + * @attr: Debugfs attribute to write to
> + * @value: The value to set
> + *
> + * Writes a boolean value to the specified debugfs attribute.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +DEFINE_XE_SRIOV_PF_DEBUGFS_FUNC(bool, set_boolean, __igt_sysfs_set_boolean)
> diff --git a/lib/xe/xe_sriov_debugfs.h b/lib/xe/xe_sriov_debugfs.h
> index 856445e76..2db965f9b 100644
> --- a/lib/xe/xe_sriov_debugfs.h
> +++ b/lib/xe/xe_sriov_debugfs.h
> @@ -16,5 +16,23 @@ int xe_sriov_pf_debugfs_read_provisioned_ranges(int pf_fd, enum xe_sriov_shared_
>   						unsigned int gt_id,
>   						struct xe_sriov_provisioned_range **ranges,
>   						unsigned int *nr_ranges);
> +int __xe_sriov_pf_debugfs_get_u32(int pf, unsigned int vf_num,
> +				  unsigned int gt_num, const char *attr,
> +				  uint32_t *value);
> +int __xe_sriov_pf_debugfs_set_u32(int pf, unsigned int vf_num,
> +				  unsigned int gt_num, const char *attr,
> +				  uint32_t value);
> +int __xe_sriov_pf_debugfs_get_u64(int pf, unsigned int vf_num,
> +				  unsigned int gt_num, const char *attr,
> +				  uint64_t *value);
> +int __xe_sriov_pf_debugfs_set_u64(int pf, unsigned int vf_num,
> +				  unsigned int gt_num, const char *attr,
> +				  uint64_t value);
> +int __xe_sriov_pf_debugfs_get_boolean(int pf, unsigned int vf_num,
> +				      unsigned int gt_num, const char *attr,
> +				      bool *value);
> +int __xe_sriov_pf_debugfs_set_boolean(int pf, unsigned int vf_num,
> +				      unsigned int gt_num, const char *attr,
> +				      bool value);
>   
>   #endif /* __XE_SRIOV_DEBUGFS_H__ */

One small nit, but overall looks good:
Reviewed-by: Lukasz Laguna <lukasz.laguna@intel.com>

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