Re: [PATCH] drm/xe/pf: Add max_vfs configfs attribute to control PF mode

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

On 10/3/2025 10:48 PM, Rodrigo Vivi wrote:
> On Fri, Oct 03, 2025 at 01:26:48AM +0200, Michal Wajdeczko wrote:
>> In addition to existing max_vfs modparam, add max_vfs configfs
>> attribute to allow PF configuration on the per-device level.
>> Default config value is still based on the modparam value.
>>
>> 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_configfs.c | 139 +++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/xe/xe_configfs.h |   4 +
>>  drivers/gpu/drm/xe/xe_sriov_pf.c |   3 +
>>  3 files changed, 146 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/xe_configfs.c
>> index 139663423185..464a79c2a903 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.c
>> +++ b/drivers/gpu/drm/xe/xe_configfs.c
>> @@ -18,6 +18,7 @@
>>  #include "xe_hw_engine_types.h"
>>  #include "xe_module.h"
>>  #include "xe_pci_types.h"
>> +#include "xe_sriov_types.h"
>>  
>>  /**
>>   * DOC: Xe Configfs
>> @@ -169,6 +170,32 @@
>>   * Currently this is implemented only for post and mid context restore and
>>   * these attributes can only be set before binding to the device.
>>   *
>> + * Max SR-IOV Virtual Functions
>> + * ----------------------------
>> + *
>> + * This config allows to limit number of the Virtual Functions (VFs) that can
>> + * be managed by the Physical Function (PF) driver, where value 0 disables the
>> + * PF mode (no VFs).
>> + *
>> + * The default max_vfs config value is taken from the max_vfs modparam.
>> + *
>> + * How to enable PF with support with unlimited (up to HW limit) number of VFs::
>> + *
>> + *	# echo unlimited > /sys/kernel/config/xe/0000:00:02.0/sriov/max_vfs
>> + *	# echo 0000:00:02.0 > /sys/bus/pci/drivers/xe/bind
>> + *
>> + * How to enable PF with support up to 3 VFs::
>> + *
>> + *	# echo 3 > /sys/kernel/config/xe/0000:00:02.0/sriov/max_vfs
>> + *	# echo 0000:00:02.0 > /sys/bus/pci/drivers/xe/bind
>> + *
>> + * How to disable PF mode and always run as native::
>> + *
>> + *	# echo 0 > /sys/kernel/config/xe/0000:00:02.0/sriov/max_vfs
>> + *	# echo 0000:00:02.0 > /sys/bus/pci/drivers/xe/bind
>> + *
>> + * This setting only takes effect when probing the device.
>> + *
>>   * Remove devices
>>   * ==============
>>   *
>> @@ -185,6 +212,7 @@ struct wa_bb {
>>  
>>  struct xe_config_group_device {
>>  	struct config_group group;
>> +	struct config_group sriov;
>>  
>>  	struct xe_config_device {
>>  		u64 engines_allowed;
>> @@ -192,23 +220,34 @@ struct xe_config_group_device {
>>  		struct wa_bb ctx_restore_mid_bb[XE_ENGINE_CLASS_MAX];
>>  		bool survivability_mode;
>>  		bool enable_psmi;
>> +		struct {
>> +			unsigned int max_vfs;
>> +		} sriov;
>>  	} config;
>>  
>>  	/* protects attributes */
>>  	struct mutex lock;
>>  	/* matching descriptor */
>>  	const struct xe_device_desc *desc;
>> +	/* tentative SR-IOV mode */
>> +	enum xe_sriov_mode mode;
>>  };
>>  
>>  static const struct xe_config_device device_defaults = {
>>  	.engines_allowed = U64_MAX,
>>  	.survivability_mode = false,
>>  	.enable_psmi = false,
>> +	.sriov = {
>> +		.max_vfs = UINT_MAX,
>> +	},
>>  };
>>  
>>  static void set_device_defaults(struct xe_config_device *config)
>>  {
>>  	*config = device_defaults;
>> +#ifdef CONFIG_PCI_IOV
>> +	config->sriov.max_vfs = xe_modparam.max_vfs;
>> +#endif
>>  }
>>  
>>  struct engine_info {
>> @@ -721,6 +760,68 @@ static const struct config_item_type xe_config_device_type = {
>>  	.ct_owner	= THIS_MODULE,
>>  };
>>  
>> +static ssize_t sriov_max_vfs_show(struct config_item *item, char *page)
>> +{
>> +	struct xe_config_group_device *dev = to_xe_config_group_device(item->ci_parent);
>> +
>> +	guard(mutex)(&dev->lock);
>> +
>> +	if (dev->config.sriov.max_vfs == UINT_MAX)
>> +		return sprintf(page, "%s\n", "unlimited");
>> +	else
>> +		return sprintf(page, "%u\n", dev->config.sriov.max_vfs);
>> +}
>> +
>> +static ssize_t sriov_max_vfs_store(struct config_item *item, const char *page, size_t len)
>> +{
>> +	struct xe_config_group_device *dev = to_xe_config_group_device(item->ci_parent);
>> +	unsigned int max_vfs;
>> +	int ret;
>> +
>> +	guard(mutex)(&dev->lock);
>> +
>> +	if (is_bound(dev))
>> +		return -EBUSY;
>> +
>> +	ret = kstrtouint(page, 0, &max_vfs);
>> +	if (ret) {
>> +		if (!sysfs_streq(page, "unlimited"))
>> +			return ret;
>> +		max_vfs = UINT_MAX;
>> +	}
>> +
>> +	dev->config.sriov.max_vfs = max_vfs;
>> +	return len;
>> +}
>> +
>> +CONFIGFS_ATTR(sriov_, max_vfs);
>> +
>> +static struct configfs_attribute *xe_config_sriov_attrs[] = {
>> +	&sriov_attr_max_vfs,
>> +	NULL,
>> +};
>> +
>> +static bool xe_config_sriov_is_visible(struct config_item *item,
>> +				       struct configfs_attribute *attr, int n)
>> +{
>> +	struct xe_config_group_device *dev = to_xe_config_group_device(item->ci_parent);
>> +
>> +	if (attr == &sriov_attr_max_vfs && dev->mode != XE_SRIOV_MODE_PF)
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>> +static struct configfs_group_operations xe_config_sriov_group_ops = {
>> +	.is_visible	= xe_config_sriov_is_visible,
>> +};
>> +
>> +static const struct config_item_type xe_config_sriov_type = {
>> +	.ct_owner	= THIS_MODULE,
>> +	.ct_group_ops	= &xe_config_sriov_group_ops,
>> +	.ct_attrs	= xe_config_sriov_attrs,
>> +};
>> +
>>  static const struct xe_device_desc *xe_match_desc(struct pci_dev *pdev)
>>  {
>>  	struct device_driver *driver = driver_find("xe", &pci_bus_type);
>> @@ -746,6 +847,7 @@ static struct config_group *xe_config_make_device_group(struct config_group *gro
>>  	unsigned int domain, bus, slot, function;
>>  	struct xe_config_group_device *dev;
>>  	const struct xe_device_desc *match;
>> +	enum xe_sriov_mode mode;
>>  	struct pci_dev *pdev;
>>  	char canonical[16];
>>  	int vfnumber = 0;
>> @@ -762,6 +864,9 @@ static struct config_group *xe_config_make_device_group(struct config_group *gro
>>  		return ERR_PTR(-EINVAL);
>>  
>>  	pdev = pci_get_domain_bus_and_slot(domain, bus, PCI_DEVFN(slot, function));
>> +	mode = pdev ? dev_is_pf(&pdev->dev) ?
>> +		XE_SRIOV_MODE_PF : XE_SRIOV_MODE_NONE : XE_SRIOV_MODE_VF;
>> +
>>  	if (!pdev && function)
>>  		pdev = pci_get_domain_bus_and_slot(domain, bus, PCI_DEVFN(slot, 0));
>>  	if (!pdev && slot)
>> @@ -796,9 +901,15 @@ static struct config_group *xe_config_make_device_group(struct config_group *gro
>>  		return ERR_PTR(-ENOMEM);
>>  
>>  	dev->desc = match;
>> +	dev->mode = match->has_sriov ? mode : XE_SRIOV_MODE_NONE;
>> +
>>  	set_device_defaults(&dev->config);
>>  
>>  	config_group_init_type_name(&dev->group, name, &xe_config_device_type);
>> +	if (dev->mode != XE_SRIOV_MODE_NONE) {
>> +		config_group_init_type_name(&dev->sriov, "sriov", &xe_config_sriov_type);
>> +		configfs_add_default_group(&dev->sriov, &dev->group);
>> +	}
>>  
>>  	mutex_init(&dev->lock);
>>  
>> @@ -988,6 +1099,34 @@ u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev,
>>  	return len;
>>  }
>>  
>> +#ifdef CONFIG_PCI_IOV
>> +/**
>> + * xe_configfs_get_max_vfs() - Get number of VFs that could be managed
>> + * @pdev: the &pci_dev device
>> + *
>> + * Find the configfs group that belongs to the PCI device and return maximum
>> + * number of Virtual Functions (VFs) that could be managed by this device.
>> + * If configfs group is not present, use value of max_vfs module parameter.
>> + *
>> + * Return: maximum number of VFs that could be managed.
>> + */
>> +unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev)
>> +{
>> +	struct xe_config_group_device *dev = find_xe_config_group_device(pdev);
>> +	unsigned int max_vfs;
>> +
>> +	if (!dev)
>> +		return xe_modparam.max_vfs;
>> +
>> +	scoped_guard(mutex, &dev->lock)
>> +		max_vfs = dev->config.sriov.max_vfs;
>> +
>> +	config_group_put(&dev->group);
>> +
>> +	return max_vfs;
>> +}
>> +#endif
>> +
>>  int __init xe_configfs_init(void)
>>  {
>>  	int ret;
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.h b/drivers/gpu/drm/xe/xe_configfs.h
>> index c61e0e47ed94..16a1f578e4fe 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.h
>> +++ b/drivers/gpu/drm/xe/xe_configfs.h
>> @@ -23,6 +23,9 @@ u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev, enum xe_engine_clas
>>  				       const u32 **cs);
>>  u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, enum xe_engine_class,
>>  					const u32 **cs);
>> +#ifdef CONFIG_PCI_IOV
>> +unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev);
>> +#endif
>>  #else
>>  static inline int xe_configfs_init(void) { return 0; }
>>  static inline void xe_configfs_exit(void) { }
>> @@ -34,6 +37,7 @@ static inline u32 xe_configfs_get_ctx_restore_mid_bb(struct pci_dev *pdev, enum
>>  						     const u32 **cs) { return 0; }
>>  static inline u32 xe_configfs_get_ctx_restore_post_bb(struct pci_dev *pdev, enum xe_engine_class,
>>  						      const u32 **cs) { return 0; }
>> +static inline unsigned int xe_configfs_get_max_vfs(struct pci_dev *pdev) { return UINT_MAX; }
>>  #endif
>>  
>>  #endif
>> diff --git a/drivers/gpu/drm/xe/xe_sriov_pf.c b/drivers/gpu/drm/xe/xe_sriov_pf.c
>> index 4698348c010a..428b1b62cf48 100644
>> --- a/drivers/gpu/drm/xe/xe_sriov_pf.c
>> +++ b/drivers/gpu/drm/xe/xe_sriov_pf.c
>> @@ -8,6 +8,7 @@
>>  #include <drm/drm_managed.h>
>>  
>>  #include "xe_assert.h"
>> +#include "xe_configfs.h"
>>  #include "xe_device.h"
>>  #include "xe_gt_sriov_pf.h"
>>  #include "xe_module.h"
>> @@ -19,6 +20,8 @@
>>  
>>  static unsigned int wanted_max_vfs(struct xe_device *xe)
>>  {
>> +	if (IS_ENABLED(CONFIG_CONFIGFS_FS))
>> +		return xe_configfs_get_max_vfs(to_pci_dev(xe->drm.dev));
>>  	return xe_modparam.max_vfs;
> 
> I like the idea of the configfs, but this here is strange.
> shouldn't be better to simply remove the modparam?

well, I just wasn't sure what will be our approach here, since all our
current configfs entries were just new options, never existed before as
modparams, while this max_vfs is the only new one config that previously
was defined as modparam, that some users may still rely on.

so my approach was to use modparam value as initial default configfs value
for all devices, or immediate fallback if configfs is not available

just need a clear statement

note that there are few other modparams that should be promoted to
configfs (like guc/huc firmwares) and I assume the same rule will apply

> 
>>  }
>>  
>> -- 
>> 2.47.1
>>