Re: [PATCH 3/3] drm/xe/configfs: Don't expose survivability_mode if not applicable

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

On 9/2/2025 7:25 PM, Summers, Stuart wrote:
> On Tue, 2025-09-02 at 15:17 +0200, Michal Wajdeczko wrote:
>> The survivability_mode attribute is applicable only for DGFX and
>> platforms newer than BATTLEMAGE. Use .is_visible() hook to hide
>> this attribute when above conditions are not met. Remove code that
>> was trying to fix such configuration during the runtime.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Cc: Riana Tauro <riana.tauro@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_configfs.c           | 24 ++++++--------------
>> --
>>  drivers/gpu/drm/xe/xe_configfs.h           |  2 --
>>  drivers/gpu/drm/xe/xe_survivability_mode.c | 11 +---------
>>  3 files changed, 7 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.c
>> b/drivers/gpu/drm/xe/xe_configfs.c
>> index 43f000260752..0337811864cd 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.c
>> +++ b/drivers/gpu/drm/xe/xe_configfs.c
>> @@ -369,7 +369,12 @@ static bool xe_config_device_is_visible(struct
>> config_item *item,
>>  {
>>         struct xe_config_group_device *dev =
>> to_xe_config_group_device(item);
>>  
>> -       return dev->desc; /* shall be always true */
>> +       if (attr == &attr_survivability_mode) {
>> +               if (!dev->desc->is_dgfx || dev->desc->platform <
>> XE_BATTLEMAGE)
>> +                       return false;
>> +       }
>> +
>> +       return true;
> 
> Why change the return here? Can we either leave this as dev->desc or
> otherwise use return true for the initial implementation (previous
> patch)?

the reason is simple: in patch 2/3 I just wanted to show how to obtain
the device xe device desc here to make some decisions based on that.

with plain "return true" in 2/3 I would have to mark desc var as "maybe_unused"

and OTOH leaving "return desc" here in 3/3 is redundant since we already might have de-referenced the pointer

so this one extra line in diff was IMO minimal cost to have two separate patches
(as I didn't want to combine survivability_mode change with introduction of is_visible)

> 
> Everything else in the series looks good to me. This does seem like a
> better way to approach this.

thanks,

> 
> Thanks,
> Stuart
> 
>>  }
>>  
>>  static struct configfs_group_operations xe_config_device_group_ops =
>> {
>> @@ -558,23 +563,6 @@ bool xe_configfs_get_survivability_mode(struct
>> pci_dev *pdev)
>>         return mode;
>>  }
>>  
>> -/**
>> - * xe_configfs_clear_survivability_mode - clear configfs
>> survivability mode
>> - * @pdev: pci device
>> - */
>> -void xe_configfs_clear_survivability_mode(struct pci_dev *pdev)
>> -{
>> -       struct xe_config_group_device *dev =
>> find_xe_config_group_device(pdev);
>> -
>> -       if (!dev)
>> -               return;
>> -
>> -       guard(mutex)(&dev->lock);
>> -       dev->config.survivability_mode = 0;
>> -
>> -       config_group_put(&dev->group);
>> -}
>> -
>>  /**
>>   * xe_configfs_get_engines_allowed - get engine allowed mask from
>> configfs
>>   * @pdev: pci device
>> diff --git a/drivers/gpu/drm/xe/xe_configfs.h
>> b/drivers/gpu/drm/xe/xe_configfs.h
>> index 58c8c3164000..1402e863b71c 100644
>> --- a/drivers/gpu/drm/xe/xe_configfs.h
>> +++ b/drivers/gpu/drm/xe/xe_configfs.h
>> @@ -15,7 +15,6 @@ int xe_configfs_init(void);
>>  void xe_configfs_exit(void);
>>  void xe_configfs_check_device(struct pci_dev *pdev);
>>  bool xe_configfs_get_survivability_mode(struct pci_dev *pdev);
>> -void xe_configfs_clear_survivability_mode(struct pci_dev *pdev);
>>  u64 xe_configfs_get_engines_allowed(struct pci_dev *pdev);
>>  bool xe_configfs_get_psmi_enabled(struct pci_dev *pdev);
>>  #else
>> @@ -23,7 +22,6 @@ static inline int xe_configfs_init(void) { return
>> 0; }
>>  static inline void xe_configfs_exit(void) { }
>>  static inline void xe_configfs_check_device(struct pci_dev *pdev) {
>> }
>>  static inline bool xe_configfs_get_survivability_mode(struct pci_dev
>> *pdev) { return false; }
>> -static inline void xe_configfs_clear_survivability_mode(struct
>> pci_dev *pdev) { }
>>  static inline u64 xe_configfs_get_engines_allowed(struct pci_dev
>> *pdev) { return U64_MAX; }
>>  static inline bool xe_configfs_get_psmi_enabled(struct pci_dev
>> *pdev) { return false; }
>>  #endif
>> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c
>> b/drivers/gpu/drm/xe/xe_survivability_mode.c
>> index 79426ea46861..19a1732e33d4 100644
>> --- a/drivers/gpu/drm/xe/xe_survivability_mode.c
>> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
>> @@ -287,19 +287,10 @@ bool xe_survivability_mode_is_requested(struct
>> xe_device *xe)
>>         u32 data;
>>         bool survivability_mode;
>>  
>> -       if (!IS_DGFX(xe) || IS_SRIOV_VF(xe))
>> +       if (!IS_DGFX(xe) || IS_SRIOV_VF(xe) || xe->info.platform <
>> XE_BATTLEMAGE)
>>                 return false;
>>  
>>         survivability_mode =
>> xe_configfs_get_survivability_mode(pdev);
>> -
>> -       if (xe->info.platform < XE_BATTLEMAGE) {
>> -               if (survivability_mode) {
>> -                       dev_err(&pdev->dev, "Survivability Mode is
>> not supported on this card\n");
>> -                       xe_configfs_clear_survivability_mode(pdev);
>> -               }
>> -               return false;
>> -       }
>> -
>>         /* Enable survivability mode if set via configfs */
>>         if (survivability_mode)
>>                 return true;
>