Re: [PATCH v2 7/7] drm/xe/configfs: add disable_mem_copy knob

[Matthew Auld <matthew.auld@intel.com>]

On 21/10/2025 03:18, Lucas De Marchi wrote:
> On Mon, Oct 20, 2025 at 01:54:39PM +0100, Matthew Auld wrote:
>> For easier experimentation/comparison allow turning off the newer
>> MEM_COPY path, without needing to apply manual hacks and build a new
>> kernel. This needs to be configured before fully probing the device.
>>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_configfs.c | 65 ++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_configfs.h |  2 +
>> drivers/gpu/drm/xe/xe_pci.c      |  4 +-
>> 3 files changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.c b/drivers/gpu/drm/xe/ 
>> xe_configfs.c
>> index c1419a270fa4..df459daa0f07 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.c
>> +++ b/drivers/gpu/drm/xe/xe_configfs.c
>> @@ -236,6 +236,15 @@
>>  *
>>  * This setting only takes effect when probing the device.
>>  *
>> + * Disable MEM_COPY path
>> + * -----------------------------------------------------
>> + * This config will force the use of the older XY_FAST_COPY 
>> instruction in the migration code.
>> + * Intended only for experimentation.
>> + *
>> + *    # echo 1 > /sys/kernel/config/xe/0000:03:00.0/disable_mem_copy
>> + *
>> + * This setting only takes effect when probing the device.
>> + *
>>  * Remove devices
>>  * ==============
>>  *
>> @@ -264,6 +273,9 @@ struct xe_config_group_device {
>>         struct {
>>             unsigned int max_vfs;
>>         } sriov;
>> +        struct {
>> +            bool disable_mem_copy;
>> +        } migrate;
> 
> so this is migrate.disable_mem_copy
> 
> ...
> 
>> CONFIGFS_ATTR(, ctx_restore_mid_bb);
>> CONFIGFS_ATTR(, ctx_restore_post_bb);
>> +CONFIGFS_ATTR(, disable_mem_copy);
> 
> but the attribute is a misleading "disable_mem_copy".

I was just trying to group the migrate related knobs under one struct 
since there could potentially be more in the future. Should that be 
reflected in the attribute? Should we do migrate_disable_mem_copy or did 
you mean something else here?

> 
> Note that configfs is not a place for hacks so if it's generally useful
> we need to think of a name that is not misleading and will be
> future-proof. This seems more like "the MEM_COPY for migration is a new
> path and we may have bugs or we want to compare them in the short term".
> Did I misunderstand the purpose of the patch?

Yeah, it's likely more a short/medium term thing. Although I did also 
use it to quickly test both paths on one machine. Happy to drop this 
one, if this is not a good fit.

> 
> Lucas De Marchi