Re: [PATCH v2 05/17] drm/xe/pf: Allow change PF and VFs EQ/PT using sysfs

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on wto [2025-paź-28 18:58:19 +0100]:
> On current platforms, in SR-IOV virtualization, the GPU is shared
> between VFs on the time-slice basis. The 'execution quantum' (EQ)
> and 'preemption timeout' (PT) are two main scheduling parameters
> that could be set individually per each VF.
> 
> Add EQ/PT read-write attributes for the PF and all VFs.
> 
> By exposing those two parameters over sysfs, the admin can change
> their default values (infinity) and let the GuC scheduler enforce
> that settings.
> 
>  /sys/bus/pci/drivers/xe/BDF/
>  ├── sriov_admin/
>      ├── pf/
>      │   └── profile
>      │       ├── exec_quantum_ms	[RW] unsigned integer
>      │       └── preempt_timeout_us	[RW] unsigned integer
>      ├── vf1/
>      │   └── profile
>      │       ├── exec_quantum_ms	[RW] unsigned integer
>      │       └── preempt_timeout_us	[RW] unsigned integer
> 
> Writing 0 to these files will set infinity EQ/PT for the VF on all
> tiles/GTs. This is a default value. Writing non-zero integers to
> these files will change EQ/PT to new value (in their respective
> units: msec or usec).
> 
> Reading from these files will return EQ/PT as previously set on
> all tiles/GTs. In case of inconsistent values detected, due to
> errors or low-level configuration done using debugfs, -EUCLEAN
> error will be returned.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> v2: apply EQ/PT under single lock (Lucas)
> ---
>  drivers/gpu/drm/xe/xe_sriov_pf_provision.c | 124 +++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_sriov_pf_provision.h |   8 ++
>  drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c     |  54 ++++++++-
>  3 files changed, 184 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_provision.c b/drivers/gpu/drm/xe/xe_sriov_pf_provision.c
> index 663fb0c045e9..c5b3a6aa67f4 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_pf_provision.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_pf_provision.c
> @@ -152,3 +152,127 @@ int xe_sriov_pf_provision_set_mode(struct xe_device *xe, enum xe_sriov_provision
>  	xe->sriov.pf.provision.mode = mode;
>  	return 0;
>  }
> +
> +/**
> + * xe_sriov_pf_provision_apply_vf_eq() - Change VF's execution quantum.
> + * @xe: the PF &xe_device
> + * @vfid: the VF identifier
> + * @eq: execution quantum in [ms] to set
> + *
> + * Change VF's execution quantum (EQ) provisioning on all tiles/GTs.
> + *
> + * This function can only be called on PF.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_sriov_pf_provision_apply_vf_eq(struct xe_device *xe, unsigned int vfid, u32 eq)
> +{
> +	struct xe_gt *gt;
> +	unsigned int id;
> +	int result = 0;
> +	int err;
> +
> +	guard(mutex)(xe_sriov_pf_master_mutex(xe));
> +
> +	for_each_gt(gt, xe, id) {
> +		err = xe_gt_sriov_pf_config_set_exec_quantum_locked(gt, vfid, eq);
> +		result = result ?: err;
> +	}
> +
> +	return result;
> +}
> +
> +/**
> + * xe_sriov_pf_provision_query_vf_eq() - Query VF's execution quantum.
> + * @xe: the PF &xe_device
> + * @vfid: the VF identifier
> + * @eq: placeholder for the returned execution quantum in [ms]
> + *
> + * Query VF's execution quantum (EQ) provisioning from all tiles/GTs.
> + * If values across tiles/GTs are inconsistent then -EUCLEAN error will be returned.
> + *
> + * This function can only be called on PF.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_sriov_pf_provision_query_vf_eq(struct xe_device *xe, unsigned int vfid, u32 *eq)
> +{
> +	struct xe_gt *gt;
> +	unsigned int id;
> +	int count = 0;
> +	u32 value;
> +
> +	guard(mutex)(xe_sriov_pf_master_mutex(xe));
> +
> +	for_each_gt(gt, xe, id) {
> +		value = xe_gt_sriov_pf_config_get_exec_quantum_locked(gt, vfid);
> +		if (!count++)
> +			*eq = value;
> +		else if (value != *eq)
> +			return -EUCLEAN;

NIT: Perhaps it would be worth adding a dbg log with information about which GT is inconsistent.

> +	}
> +
> +	return !count ? -ENODATA : 0;
> +}
> +
> +/**
> + * xe_sriov_pf_provision_apply_vf_pt() - Change VF's preemption timeout.
> + * @xe: the PF &xe_device
> + * @vfid: the VF identifier
> + * @pt: preemption timeout in [us] to set
> + *
> + * Change VF's preemption timeout (PT) provisioning on all tiles/GTs.
> + *
> + * This function can only be called on PF.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_sriov_pf_provision_apply_vf_pt(struct xe_device *xe, unsigned int vfid, u32 pt)
> +{
> +	struct xe_gt *gt;
> +	unsigned int id;
> +	int result = 0;
> +	int err;
> +
> +	guard(mutex)(xe_sriov_pf_master_mutex(xe));
> +
> +	for_each_gt(gt, xe, id) {
> +		err = xe_gt_sriov_pf_config_set_preempt_timeout_locked(gt, vfid, pt);
> +		result = result ?: err;
> +	}
> +
> +	return result;
> +}
> +
> +/**
> + * xe_sriov_pf_provision_query_vf_pt() - Query VF's preemption timeout.
> + * @xe: the PF &xe_device
> + * @vfid: the VF identifier
> + * @pt: placeholder for the returned preemption timeout in [us]
> + *
> + * Query VF's preemption timeout (PT) provisioning from all tiles/GTs.
> + * If values across tiles/GTs are inconsistent then -EUCLEAN error will be returned.
> + *
> + * This function can only be called on PF.
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int xe_sriov_pf_provision_query_vf_pt(struct xe_device *xe, unsigned int vfid, u32 *pt)
> +{
> +	struct xe_gt *gt;
> +	unsigned int id;
> +	int count = 0;
> +	u32 value;
> +
> +	guard(mutex)(xe_sriov_pf_master_mutex(xe));
> +
> +	for_each_gt(gt, xe, id) {
> +		value = xe_gt_sriov_pf_config_get_preempt_timeout_locked(gt, vfid);
> +		if (!count++)
> +			*pt = value;
> +		else if (value != *pt)
> +			return -EUCLEAN;
> +	}
> +
> +	return !count ? -ENODATA : 0;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_provision.h b/drivers/gpu/drm/xe/xe_sriov_pf_provision.h
> index cf3657a32e90..cb81b5880930 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_pf_provision.h
> +++ b/drivers/gpu/drm/xe/xe_sriov_pf_provision.h
> @@ -6,10 +6,18 @@
>  #ifndef _XE_SRIOV_PF_PROVISION_H_
>  #define _XE_SRIOV_PF_PROVISION_H_
>  
> +#include <linux/types.h>
> +
>  #include "xe_sriov_pf_provision_types.h"
>  
>  struct xe_device;
>  
> +int xe_sriov_pf_provision_apply_vf_eq(struct xe_device *xe, unsigned int vfid, u32 eq);
> +int xe_sriov_pf_provision_query_vf_eq(struct xe_device *xe, unsigned int vfid, u32 *eq);
> +
> +int xe_sriov_pf_provision_apply_vf_pt(struct xe_device *xe, unsigned int vfid, u32 pt);
> +int xe_sriov_pf_provision_query_vf_pt(struct xe_device *xe, unsigned int vfid, u32 *pt);
> +
>  int xe_sriov_pf_provision_vfs(struct xe_device *xe, unsigned int num_vfs);
>  int xe_sriov_pf_unprovision_vfs(struct xe_device *xe, unsigned int num_vfs);
>  
> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c b/drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c
> index 439a0cd02a86..f12d6752e9f1 100644
> --- a/drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c
> @@ -13,6 +13,7 @@
>  #include "xe_sriov.h"
>  #include "xe_sriov_pf.h"
>  #include "xe_sriov_pf_helpers.h"
> +#include "xe_sriov_pf_provision.h"
>  #include "xe_sriov_pf_sysfs.h"
>  #include "xe_sriov_printk.h"
>  
> @@ -23,10 +24,14 @@
>   *     ├── ...
>   *     ├── pf/
>   *     │   ├── ...
> - *     │   └── ...
> + *     │   └── profile
> + *     │       ├── exec_quantum_ms
> + *     │       └── preempt_timeout_us
>   *     ├── vf1/
>   *     │   ├── ...
> - *     │   └── ...
> + *     │   └── profile
> + *     │       ├── exec_quantum_ms
> + *     │       └── preempt_timeout_us
>   *     ├── vf2/
>   *     :
>   *     └── vfN/
> @@ -85,7 +90,52 @@ static const struct attribute_group *xe_sriov_dev_attr_groups[] = {
>  
>  /* and VF-level attributes go here */
>  
> +#define DEFINE_SIMPLE_PROVISIONING_SRIOV_VF_ATTR(NAME, ITEM, TYPE, FORMAT)		\
> +static ssize_t xe_sriov_vf_attr_##NAME##_show(struct xe_device *xe, unsigned int vfid,	\
> +					      char *buf)				\
> +{											\
> +	TYPE value = 0;									\
> +	int err;									\
> +											\
> +	err = xe_sriov_pf_provision_query_vf_##ITEM(xe, vfid, &value);			\
> +	if (err)									\
> +		return err;								\
> +											\
> +	return sysfs_emit(buf, FORMAT, value);						\
> +}											\
> +											\
> +static ssize_t xe_sriov_vf_attr_##NAME##_store(struct xe_device *xe, unsigned int vfid,	\
> +					       const char *buf, size_t count)		\
> +{											\
> +	TYPE value;									\
> +	int err;									\
> +											\
> +	err = kstrto##TYPE(buf, 0, &value);						\
> +	if (err)									\
> +		return err;								\
> +											\
> +	err = xe_sriov_pf_provision_apply_vf_##ITEM(xe, vfid, value);			\
> +	return err ?: count;								\
> +}											\
> +											\
> +static XE_SRIOV_VF_ATTR(NAME)
> +
> +DEFINE_SIMPLE_PROVISIONING_SRIOV_VF_ATTR(exec_quantum_ms, eq, u32, "%u\n");
> +DEFINE_SIMPLE_PROVISIONING_SRIOV_VF_ATTR(preempt_timeout_us, pt, u32, "%u\n");
> +
> +static struct attribute *profile_vf_attrs[] = {
> +	&xe_sriov_vf_attr_exec_quantum_ms.attr,
> +	&xe_sriov_vf_attr_preempt_timeout_us.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group profile_vf_attr_group = {
> +	.name = "profile",
> +	.attrs = profile_vf_attrs,
> +};
> +
>  static const struct attribute_group *xe_sriov_vf_attr_groups[] = {
> +	&profile_vf_attr_group,
>  	NULL
>  };

LGTM:
Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

>  
> -- 
> 2.47.1
> 

--