Re: [PATCH v4 23/32] drm/xe/survivability: Report 'Runtime Mode enabled' status using SIGID

On 8/14/2026 12:27 PM, Tauro, Riana wrote:
> 
> On 14-08-2026 15:48, Michal Wajdeczko wrote:
>>
>> On 8/14/2026 9:06 AM, Tauro, Riana wrote:
>>>
>>> On 13-08-2026 18:16, Michal Wajdeczko wrote:
>>>> On 8/13/2026 1:40 PM, Mallesh, Koujalagi wrote:
>>>>> On 13-08-2026 12:44 am, Michal Wajdeczko wrote:
>>>>>> Report 'Runtime Mode' status using various xe_log() helpers.
>>>>>>
>>>>>> Signed-off-by: Michal Wajdeczko <[email protected]>
>>>>>> Cc: Rodrigo Vivi <[email protected]>
>>>>>> Cc: Riana Tauro <[email protected]>
>>>>>> Cc: Aravind Iddamsetty <[email protected]>
>>>>>> Cc: Mallesh Koujalagi <[email protected]>
>>>>>> ---
>>>>>>    drivers/gpu/drm/xe/xe_survivability_mode.c | 10 +++++++---
>>>>>>    1 file changed, 7 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c
>>>>>> index ebd288986c11..5d3315ac735c 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_survivability_mode.c
>>>>>> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
>>>>>> @@ -18,6 +18,7 @@
>>>>>>    #include "xe_mmio.h"
>>>>>>    #include "xe_nvm.h"
>>>>>>    #include "xe_pcode_api.h"
>>>>>> +#include "xe_printk.h"
>>>>>>    #include "xe_vsec.h"
>>>>>>      /**
>>>>>> @@ -405,7 +406,7 @@ void xe_survivability_mode_runtime_enable(struct xe_device *xe)
>>>>>>        struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>>>>>          if (!IS_DGFX(xe) || IS_SRIOV_VF(xe) || xe->info.platform < XE_BATTLEMAGE) {
>>>>>> -        dev_err(&pdev->dev, "Runtime Survivability Mode not supported\n");
>>>>>> +        xe_log_err(xe, SURVIVABILITY, -EOPNOTSUPP, "Runtime Mode not supported!\n");
>>>>>>            return;
>>>>>>        }
>>>>>>    @@ -413,11 +414,14 @@ void xe_survivability_mode_runtime_enable(struct xe_device *xe)
>>>>>>        create_survivability_sysfs(pdev);
>>>>>>          survivability->type = XE_SURVIVABILITY_TYPE_RUNTIME;
>>>>>> -    dev_err(&pdev->dev, "Runtime Survivability mode enabled\n");
>>>>>> +    xe_log_info(xe, SURVIVABILITY, "Runtime Mode enabled!\n");
>>> This should be xe_log_err
>> do you want to include any specific errno here (like -ESTALE)
>> or just use 0 to do not include it in the message?
> 
> Can we skip errorno for now based on comment in the last patch?

ok

> @Rodrigo I was thinking we could add the detailed error information from scratch registers here
> in a future patch but will need to come up with some format
> Let me know your thoughts?

btw, is this something more than what the log_survivability_info()
is currently doing ? we log that with SIGID already (but not
always)

> 
>> asking now, as I don't want to respin the whole series
>> after I make a wrong decision based on dice roll
>>
>> having an extra errno in the dmesg/CPER along with SIGID will
>> likely help us write better documentation with resolutions
> 
> 
> For survivability, UMD's need to check sysfs always before taking a decision
> of recovery as we have multiple recovery methods.

btw, there is a chance that sysfs creation will fail.
maybe in such case we should unconditionally call
log_survivability_info() to collect all info at
least in the dmesg?

> We cannot have generic recovery documentation based on error no here.

it looks that providing valuable resolution based just on
the SIGID alone will be even more challenging ;)

> 
> Thanks
> Riana
> 
> 
>>
>>>>>>          xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_VENDOR);
>>>>>>        xe_device_declare_wedged(xe);
>>>>>> -    dev_err(&pdev->dev, "Firmware flash required, Please refer to the userspace documentation for more details!\n");
>>>>>> +
>>>>>> +    xe_log_info(xe, SURVIVABILITY, "Firmware flash required!\n");
>>>>> IMO, we need to use xe_log_err (recoverable) here instead of xe_log_info.
>>> Same here.
>> ditto
>>
>>>>> Already mentioned in original changes [1], runtime firmware errors cause device unstable:
>>>>>
>>>>> [1] https://gitlab.freedesktop.org/drm/tip/-/commit/a2ca0633a0fef925a0d8125d8f3e4495a5ecb310
>>>> but in recent comment [2] you said that Arch team wanted that to be logged as info rather than fatal
>>>>
>>>> [2] https://patchwork.freedesktop.org/patch/743324/?series=171022&rev=3#comment_1373731
>>>>
>>>> @Riana, @Rodrigo, @Aravind ?
>>>>
>>>> I need a decider here
>>>>
>>>>> As I know xe_survivability_mode_runtime_enable function need to carry errno as parameter to pass into xe_log_err,
>>>>>
>>>>> however we need to create separate refactor patch for that in future.
>>> Why do we need error no here? It should be 0.
>>>
>>> Thanks
>>> Riana
>>>
>>>
>>>>>
>>>>> With above changes:
>>>>>
>>>>> Reviewed-by: Mallesh Koujalagi <[email protected]>
>>>>>
>>>>>> +    xe_info(xe, "Please refer to the userspace documentation for more details how to flash the firmware on %s!\n",
>>>>>> +        xe->info.platform_name);
>>>>>>    }
>>>>>>      static void log_survivability_info(struct xe_device *xe)