Re: [PATCH 03/14] drm/xe/pf: Allow change PF and VFs EQ/PT using sysfs

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 10/27/2025 6:27 PM, Lucas De Marchi wrote:
> On Mon, Oct 20, 2025 at 08:24:03PM +0200, Michal Wajdeczko wrote:
>> 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>
>> ---
>> drivers/gpu/drm/xe/xe_sriov_pf_provision.c | 116 +++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_sriov_pf_provision.h |   8 ++
>> drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c     |  54 +++++++++-
>> 3 files changed, 176 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..f2e14103c9aa 100644
>> --- a/drivers/gpu/drm/xe/xe_sriov_pf_provision.c
>> +++ b/drivers/gpu/drm/xe/xe_sriov_pf_provision.c
>> @@ -152,3 +152,119 @@ 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;
>> +
>> +    for_each_gt(gt, xe, id) {
>> +        err = xe_gt_sriov_pf_config_set_exec_quantum(gt, vfid, eq);
> 
> 
> I find it weird that a race on multiple writes would basically give
> undeterministic results for the device. We are locking/releaseing
> xe_gt_sriov_pf_master_mutex on the gt, but shouldn't we syncronize
> multiple writes at the device level?

but we still expose more granular attributes over debugfs, so even if
we would be successful in this protected round here, the actual
configuration could became "unclean" soon anyway, so I'm not sure there
is any benefit trying to add this extra protection

> 
>> +        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.
> 
> or if you had multiple userspace components trying to set the value
> creating the race above.

yes, this "inconsistency" can be mostly introduced by the userspace and
concurrency / race is not only sysfs vs sysfs but also sysfs vs debugfs

> 
> Lucas De Marchi