[PATCH 1/2] RFC drm/xe: Add configfs to enable survivability mode
Aravind Iddamsetty
aravind.iddamsetty at linux.intel.com
Fri Mar 14 16:17:33 UTC 2025
On 14-03-2025 12:54, Riana Tauro wrote:
>
>
> On 3/14/2025 1:22 AM, Lucas De Marchi wrote:
>> On Thu, Mar 13, 2025 at 10:52:36AM -0400, Rodrigo Vivi wrote:
>>> On Thu, Mar 13, 2025 at 11:48:41AM +0530, Riana Tauro wrote:
>>>> Hi Aravind
>>>>
>>>> On 3/10/2025 12:44 PM, Aravind Iddamsetty wrote:
>>>> >
>>>> > On 07-03-2025 19:54, Riana Tauro wrote:
>>>> >
>>>> > Hi Riana,
>>>> >
>>>> > I do think we can achieve the same functionality with module
>>>> param and
>>>> > we needn't reload the driver
>>>> > if we are doing unbind, Since the driver will be loaded event after
>>>> > unbind we can modify the module param
>>>>
>>>> tried this. We can modify the module param using sysfs and bind
>>>> similar to
>>>> configfs.
>>>>
>>>> If we want to configure any other attributes or move existing
>>>> module params
>>>> to device specific then adding configfs seems better.
>>>>
>>>> @Rodrigo thoughts?
>>>
>>> Yeap, if we make the param write-able and document that it is only
>>> checked
>>> at probe we could indeed use the flow
>>>
>>> echo <bdf> > /sys/../xe/unbind
>>> echo <bdf> > /sys/../xe/param/survivability_mode
>>> echo <bdf> > /sys/../xe/bind
>>>
>>> and accomplish the same.
>>>
>>> on the good side:
>>>
>>> So, this is the future prof case. Because if we start seeing cases
>>> where
>>> the device fails at boot without the FW request for the
>>> survivability mode
>>> we might be forced to have a parameter anyway. :/
>>
>> With parameters we have these possibilities:
>>
>> 1) driver is already loaded:
>>
>> echo <bdf> > /sys/../xe/unbind
>> echo <bdf> > /sys/module/xe/parameters/survivability_mode
>> echo <bdf> > /sys/../xe/bind
>>
>> 2) driver is not loaded
>>
>> # put all device in survivability mode
>> modprobe xe survivability_mode=
>>
>> 3) driver not loaded, and only one device in this mode
>>
>> echo 0 > /sys/bus/pci/drivers_autoprobe
>> modprobe xe
>> echo <bdf> > /sys/module/xe/parameters/survivability_mode
>> echo <bdf> > /sys/../xe/bind
>>
>> With configfs we have these possibilities:
>>
>> a) driver is already loaded:
>>
>> echo <bdf> > /sys/../xe/unbind
>> mkdir /sys/kernel/config/xe/0000:03:00.0
>> echo 1 > /sys/kernel/config/xe/0000:03:00.0/survivability_mode
>> echo <bdf> > /sys/../xe/bind
>>
>> b) driver is not loaded
>>
>> # no equivalent option for "all devices in survivability mode"
>> # other than repeating (c) for each since the option is per
>> # device not per module
>>
>> c) driver not loaded, and only one device in this mode
>>
>> echo 0 > /sys/bus/pci/drivers_autoprobe
>> modprobe xe
>> mkdir /sys/kernel/config/xe/0000:03:00.0
>> echo 1 > /sys/kernel/config/xe/0000:03:00.0/survivability_mode
>> echo <bdf> > /sys/../xe/bind
>>
>>>
>>> on the bad side:
>>>
>>> But we were actually aiming to reduce the parameters that we have
>>> because that was getting indiscriminately used and even recommended
>>> by some
>>> distros' doc and wikis.
>>>
>>> Another problem with the write-able param is that people might
>>> expect to
>>>
>>> echo <bdf> > /sys/../xe/param/survivability_mode
>>>
>>> and immediately get the device in the survivability_mode for that
>>> device.
>>> Then we are going to get questions and bug reports that this is not
>>> working.
>>>
>>> But well, perhaps documenting the flow properly in the param
>>> description
>>> might solve this concern.
>>>
>>> Lucas, Thomas, thoughts?
>>
>> Let me add here that we will have to extend configuration to more
>> things. The one I will work once this configfs lands (or even before it)
>> is to allow developers to add WAs (or we could say
>> register-save-restore) dynamically so it's much easier to
>> try/error/recover. For that my plan is something like this:
>>
>> echo 0 > /sys/bus/pci/drivers_autoprobe
>> modprobe xe
>> mkdir /sys/kernel/config/xe/0000:03:00.0
>> cat gt-wa.txt > /sys/kernel/config/xe/0000:03:00.0/gt_wa
>> cat engine-wa.txt > /sys/kernel/config/xe/0000:03:00.0/engine_wa
>> echo <bdf> > /sys/../xe/bind
>>
>> And cleanup my kmod "frontend" protoype to make it simpler[1]:
>>
>> i) no additional configuration:
>>
>> kmod bind --device 0000:03:00.0 xe
>>
>> ii) with configuration stored somewhere:
>>
>> kmod bind --device 0000:03:00.0 \
>> --config <path-to-config-dir|path-to-config-tarball> \
>> xe
>>
>> iii) with configuration in the command line:
>>
>> kmod bind --device 0000:03:00.0 \
>> --config-kv survivability_mode:1 \
>> xe
>>
>> So, I think the configfs approach is more future proof. For a simple
>> switch/panic-button like survivability_mode I wouldn't oppose to have it
>> as a module parameter though. Maybe make the module param read-only
>> and if per-device support is desired, then handle only via configfs?
> yeah survivability mode is per device. Since there is a plan to extend
> configfs for other attributes will go ahead with configfs.
>
> Thank you all for the inputs.
Even if we go ahead with configfs should we support per device, because
thinking from end user or production environment it doesn't
make sense to recover a device from a system while the system is active
in service. In my opinion the entire system will be taken out of
service for maintenance.
Thanks,
Aravind.
>
> Thanks
> Riana
>
>>
>> Lucas De Marchi
>>
>> [1] Typing here without much thought on the actual interface - my
>> prototype hardcodes it for pci, but the kmod command would
>> probably have to consider other buses too.
>>
>>>
>>>>
>>>> Thanks
>>>> Riana
>>>> > and once we bind the device back it can check if the BDF belongs
>>>> to this
>>>> > driver instance and configure the mode accordingly.
>>>> >
>>>> > Thanks,
>>>> > Aravind.
>>>> > > Registers a configfs subsystem called 'xe' to userspace. The
>>>> user can
>>>> > > use this to modify exposed attributes.
>>>> > >
>>>> > > Add survivability mode attribute (config/xe/survivability_mode)
>>>> to the
>>>> > > subsystem to allow the user to specify the card that should enter
>>>> > > survivability mode.
>>>> > >
>>>> > > Signed-off-by: Riana Tauro <riana.tauro at intel.com>
>>>> > > ---
>>>> > > drivers/gpu/drm/xe/Makefile | 1 +
>>>> > > drivers/gpu/drm/xe/xe_configfs.c | 95
>>>> +++++++++++++++++++++++++ +++++++
>>>> > > drivers/gpu/drm/xe/xe_configfs.h | 12 ++++
>>>> > > drivers/gpu/drm/xe/xe_module.c | 5 ++
>>>> > > drivers/gpu/drm/xe/xe_module.h | 1 +
>>>> > > 5 files changed, 114 insertions(+)
>>>> > > create mode 100644 drivers/gpu/drm/xe/xe_configfs.c
>>>> > > create mode 100644 drivers/gpu/drm/xe/xe_configfs.h
>>>> > >
>>>> > > diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/
>>>> Makefile
>>>> > > index 9699b08585f7..3f8c87292cee 100644
>>>> > > --- a/drivers/gpu/drm/xe/Makefile
>>>> > > +++ b/drivers/gpu/drm/xe/Makefile
>>>> > > @@ -28,6 +28,7 @@ $(obj)/generated/%_wa_oob.c $(obj)/generated/
>>>> %_wa_oob.h: $(obj)/xe_gen_wa_oob \
>>>> > > xe-y += xe_bb.o \
>>>> > > xe_bo.o \
>>>> > > xe_bo_evict.o \
>>>> > > + xe_configfs.o \
>>>> > > xe_devcoredump.o \
>>>> > > xe_device.o \
>>>> > > xe_device_sysfs.o \
>>>> > > diff --git a/drivers/gpu/drm/xe/xe_configfs.c
>>>> b/drivers/gpu/drm/ xe/xe_configfs.c
>>>> > > new file mode 100644
>>>> > > index 000000000000..8c5f248e466d
>>>> > > --- /dev/null
>>>> > > +++ b/drivers/gpu/drm/xe/xe_configfs.c
>>>> > > @@ -0,0 +1,95 @@
>>>> > > +// SPDX-License-Identifier: MIT
>>>> > > +/*
>>>> > > + * Copyright © 2025 Intel Corporation
>>>> > > + */
>>>> > > +
>>>> > > +#include <linux/configfs.h>
>>>> > > +#include <linux/init.h>
>>>> > > +#include <linux/module.h>
>>>> > > +
>>>> > > +#include "xe_configfs.h"
>>>> > > +#include "xe_module.h"
>>>> > > +
>>>> > > +/**
>>>> > > + * DOC: Xe Configfs
>>>> > > + *
>>>> > > + * XE KMD registers a configfs subsystem called 'xe'to
>>>> userspace that allows users to modify
>>>> > > + * the exposed attributes.
>>>> > > + *
>>>> > > + * Attributes:
>>>> > > + *
>>>> > > + * config/xe/survivability_mode : Write only attribute that
>>>> allows user to specify the PCI address
>>>> > > + * of the card that has to enter
>>>> survivability mode
>>>> > > + */
>>>> > > +
>>>> > > +void xe_configfs_clear_survivability_mode(void)
>>>> > > +{
>>>> > > + kfree(xe_modparam.survivability_mode);
>>>> > > + xe_modparam.survivability_mode = NULL;
>>>> > > +}
>>>> > > +
>>>> > > +static ssize_t survivability_mode_store(struct config_item
>>>> *item, const char *page, size_t len)
>>>> > > +{
>>>> > > + char *survivability_mode;
>>>> > > + int ret;
>>>> > > + unsigned int domain, bus, slot, function;
>>>> > > +
>>>> > > + ret = sscanf(page, "%04x:%02x:%02x.%x", &domain, &bus,
>>>> &slot, &function);
>>>> > > + if (ret != 4)
>>>> > > + return -EINVAL;
>>>> > > +
>>>> > > + survivability_mode = kstrdup(page, GFP_KERNEL);
>>>> > > + if (!survivability_mode)
>>>> > > + return -ENOMEM;
>>>> > > +
>>>> > > + xe_configfs_clear_survivability_mode();
>>>> > > + xe_modparam.survivability_mode = survivability_mode;
>>>> > > +
>>>> > > + return len;
>>>> > > +}
>>>> > > +
>>>> > > +CONFIGFS_ATTR_WO(, survivability_mode);
>>>> > > +
>>>> > > +static struct configfs_attribute *xe_configfs_attrs[] = {
>>>> > > + &attr_survivability_mode,
>>>> > > + NULL,
>>>> > > +};
>>>> > > +
>>>> > > +static const struct config_item_type xe_config_type = {
>>>> > > + .ct_attrs = xe_configfs_attrs,
>>>> > > + .ct_owner = THIS_MODULE,
>>>> > > +};
>>>> > > +
>>>> > > +static struct configfs_subsystem xe_config_subsys = {
>>>> > > + .su_group = {
>>>> > > + .cg_item = {
>>>> > > + .ci_namebuf = "xe",
>>>> > > + .ci_type = &xe_config_type,
>>>> > > + },
>>>> > > + },
>>>> > > +};
>>>> > > +
>>>> > > +int __init xe_configfs_init(void)
>>>> > > +{
>>>> > > + int ret;
>>>> > > +
>>>> > > + config_group_init(&xe_config_subsys.su_group);
>>>> > > + mutex_init(&xe_config_subsys.su_mutex);
>>>> > > + ret = configfs_register_subsystem(&xe_config_subsys);
>>>> > > + if (ret) {
>>>> > > + pr_err("Error %d while registering subsystem %s\n",
>>>> > > + ret,
>>>> xe_config_subsys.su_group.cg_item.ci_namebuf);
>>>> > > + mutex_destroy(&xe_config_subsys.su_mutex);
>>>> > > + return ret;
>>>> > > + }
>>>> > > +
>>>> > > + return 0;
>>>> > > +}
>>>> > > +
>>>> > > +void __exit xe_configfs_exit(void)
>>>> > > +{
>>>> > > + xe_configfs_clear_survivability_mode();
>>>> > > + configfs_unregister_subsystem(&xe_config_subsys);
>>>> > > + mutex_destroy(&xe_config_subsys.su_mutex);
>>>> > > +}
>>>> > > +
>>>> > > diff --git a/drivers/gpu/drm/xe/xe_configfs.h
>>>> b/drivers/gpu/drm/ xe/xe_configfs.h
>>>> > > new file mode 100644
>>>> > > index 000000000000..491629a2ca53
>>>> > > --- /dev/null
>>>> > > +++ b/drivers/gpu/drm/xe/xe_configfs.h
>>>> > > @@ -0,0 +1,12 @@
>>>> > > +/* SPDX-License-Identifier: MIT */
>>>> > > +/*
>>>> > > + * Copyright © 2025 Intel Corporation
>>>> > > + */
>>>> > > +#ifndef _XE_CONFIGFS_H_
>>>> > > +#define _XE_CONFIGFS_H_
>>>> > > +
>>>> > > +int xe_configfs_init(void);
>>>> > > +void xe_configfs_exit(void);
>>>> > > +void xe_configfs_clear_survivability_mode(void);
>>>> > > +
>>>> > > +#endif
>>>> > > diff --git a/drivers/gpu/drm/xe/xe_module.c
>>>> b/drivers/gpu/drm/xe/ xe_module.c
>>>> > > index 475acdba2b55..15b3cf22193c 100644
>>>> > > --- a/drivers/gpu/drm/xe/xe_module.c
>>>> > > +++ b/drivers/gpu/drm/xe/xe_module.c
>>>> > > @@ -11,6 +11,7 @@
>>>> > > #include <drm/drm_module.h>
>>>> > > #include "xe_drv.h"
>>>> > > +#include "xe_configfs.h"
>>>> > > #include "xe_hw_fence.h"
>>>> > > #include "xe_pci.h"
>>>> > > #include "xe_pm.h"
>>>> > > @@ -91,6 +92,10 @@ static const struct init_funcs init_funcs[] = {
>>>> > > {
>>>> > > .init = xe_check_nomodeset,
>>>> > > },
>>>> > > + {
>>>> > > + .init = xe_configfs_init,
>>>> > > + .exit = xe_configfs_exit,
>>>> > > + },
>>>> > > {
>>>> > > .init = xe_hw_fence_module_init,
>>>> > > .exit = xe_hw_fence_module_exit,
>>>> > > diff --git a/drivers/gpu/drm/xe/xe_module.h
>>>> b/drivers/gpu/drm/xe/ xe_module.h
>>>> > > index 84339e509c80..c238dbee6bc7 100644
>>>> > > --- a/drivers/gpu/drm/xe/xe_module.h
>>>> > > +++ b/drivers/gpu/drm/xe/xe_module.h
>>>> > > @@ -24,6 +24,7 @@ struct xe_modparam {
>>>> > > #endif
>>>> > > int wedged_mode;
>>>> > > u32 svm_notifier_size;
>>>> > > + char *survivability_mode;
>>>> > > };
>>>> > > extern struct xe_modparam xe_modparam;
>>>>
>
More information about the Intel-xe
mailing list