Re: [PATCH v2 16/17] drm/xe/pf: Allow to stop and reset VF using sysfs

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

On 10/30/2025 2:43 PM, Lucas De Marchi wrote:
> On Tue, Oct 28, 2025 at 06:58:30PM +0100, Michal Wajdeczko wrote:
>> It is expected that VFs activity will be monitored and in some
>> cases admin might want to silence specific VF without killing
>> the VM where it was attached.
>>
>> Add write-only attribute to stop GuC scheduling at VFs level.
>>
>>  /sys/bus/pci/drivers/xe/BDF/
>>  ├── sriov_admin/
>>      ├── vf1/
>>      │   └── stop        [WO] bool
>>      ├── vf2/
>>      │   └── stop        [WO] bool
>>
>> Writing "1" or "y" (or whatever is recognized by the strtobool()
>> function) to this file will trigger the change of the VF state
>> to STOP (GuC will stop servicing the VF). To go back to a READY
>> state (to allow GuC to service this VF again) the VF FLR must be
>> triggered (which can be done by writing 1 to device/reset file).
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> v2: drop reset file (Rodrigo, Lucas)
>> ---
>> drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c | 49 ++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c b/drivers/gpu/drm/xe/xe_sriov_pf_sysfs.c
>> index 360b0ffd9cb4..3a8c488d183c 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_pm.h"
>> #include "xe_sriov.h"
>> #include "xe_sriov_pf.h"
>> +#include "xe_sriov_pf_control.h"
>> #include "xe_sriov_pf_helpers.h"
>> #include "xe_sriov_pf_provision.h"
>> #include "xe_sriov_pf_sysfs.h"
>> @@ -52,6 +53,7 @@ static int emit_choice(char *buf, int choice, const char * const *array, size_t
>>  *     ├── vf1/
>>  *     │   ├── ...
>>  *     │   ├── device -> ../../../BDF.1
>> + *     │   ├── stop
>>  *     │   └── profile
>>  *     │       ├── exec_quantum_ms
>>  *     │       ├── preempt_timeout_us
>> @@ -291,8 +293,55 @@ static const struct attribute_group profile_vf_attr_group = {
>>     .is_visible = profile_vf_attr_is_visible,
>> };
>>
>> +#define DEFINE_SIMPLE_CONTROL_SRIOV_VF_ATTR(NAME)                    \
>> +                                            \
>> +static ssize_t xe_sriov_vf_attr_##NAME##_store(struct xe_device *xe, unsigned int vfid,    \
>> +                           const char *buf, size_t count)        \
>> +{                                            \
>> +    bool yes;                                    \
>> +    int err;                                    \
>> +                                            \
>> +    if (!vfid)                                    \
>> +        return -EPERM;                                \
>> +                                            \
>> +    err = kstrtobool(buf, &yes);                            \
>> +    if (err)                                    \
>> +        return err;                                \
>> +    if (!yes)                                    \
>> +        return count;                                \
>> +                                            \
>> +    err = xe_sriov_pf_control_##NAME##_vf(xe, vfid);                \
>> +    return err ?: count;                                \
>> +}                                            \
>> +                                            \
>> +static XE_SRIOV_VF_ATTR_WO(NAME)
>> +
>> +DEFINE_SIMPLE_CONTROL_SRIOV_VF_ATTR(stop);
>> +
>> +static struct attribute *control_vf_attrs[] = {
>> +    &xe_sriov_vf_attr_stop.attr,
>> +    NULL
>> +};
>> +
>> +static umode_t control_vf_attr_is_visible(struct kobject *kobj,
>> +                      struct attribute *attr, int index)
>> +{
>> +    struct xe_sriov_kobj *vkobj = to_xe_sriov_kobj(kobj);
>> +
>> +    if (vkobj->vfid == PFID)
>> +        return 0;
> 
> I think this will be the case for the entire group, isn't it?
> Should we go ahead and just return SYSFS_GROUP_INVISIBLE so even the dir
> is invisible instead of having an empty dir?

today it can't be empty as at this level there is already a "device"
link and a "profile" dir - both for PF and all VFs.

> 
> Lucas De Marchi
> 
>> +
>> +    return attr->mode;
>> +}
>> +
>> +static const struct attribute_group control_vf_attr_group = {
>> +    .attrs = control_vf_attrs,
>> +    .is_visible = control_vf_attr_is_visible,
>> +};
>> +
>> static const struct attribute_group *xe_sriov_vf_attr_groups[] = {
>>     &profile_vf_attr_group,
>> +    &control_vf_attr_group,
>>     NULL
>> };
>>
>> -- 
>> 2.47.1
>>