Re: [PATCH v3 17/23] drm/xe: Report 'device wedged' errors using SIGID

On 07-08-2026 03:54 pm, Michal Wajdeczko wrote:
>
> On 8/7/2026 11:56 AM, Mallesh, Koujalagi wrote:
>> On 30-07-2026 08:51 pm, Michal Wajdeczko wrote:
>>> Report 'device wedged' error using xe_log_err_fatal() helper.
>>> Use -EIO as the error cause, as nothing else is provided here.
>>> The hints about the recovery method and bug filling are now
>> Typo 'filing'
>>> separated as it only needs to be printed once.
>>>
>>> Signed-off-by: Michal Wajdeczko <[email protected]>
>>> Cc: Rodrigo Vivi <[email protected]>
>>> Cc: Aravind Iddamsetty <[email protected]>
>>> Cc: Mallesh Koujalagi <[email protected]>
>>> ---
>>>    drivers/gpu/drm/xe/xe_device.c | 16 ++++++++++------
>>>    1 file changed, 10 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>>> index 4eed9a251e65..b1e9bcd9f5b4 100644
>>> --- a/drivers/gpu/drm/xe/xe_device.c
>>> +++ b/drivers/gpu/drm/xe/xe_device.c
>>> @@ -48,6 +48,7 @@
>>>    #include "xe_i2c.h"
>>>    #include "xe_irq.h"
>>>    #include "xe_late_bind_fw.h"
>>> +#include "xe_log.h"
>>>    #include "xe_mmio.h"
>>>    #include "xe_module.h"
>>>    #include "xe_nvm.h"
>>> @@ -1396,6 +1397,9 @@ void xe_device_set_wedged_method(struct xe_device *xe, unsigned long method)
>>>        xe->wedged.method = method;
>>>    }
>>>    +#define WEDGED_URL    "https://docs.kernel.org/gpu/drm-uapi.html#device-wedging"
>>> +#define XE_BUG_URL    "https://gitlab.freedesktop.org/drm/xe/kernel/issues/new"
>> We can directly used url string, instead of define as macro, since we are not using anywhere else. OR
> URLs change from time to time
> IMO it is easier to change a dedicated macro then full message,
> that will remain unchanged otherwise

In that case, I would prefer keeping macros, even if they are currently 
used only once, it's good for readability and future updates.

>> Please make scope to file only.
> hmm?
>
> we can try to promote WEDGED_URL to drm/drm_device.h as DRM_WEDGE_URL_DOC
> and move XE_BUG_URL to xe/xe_drv.h, but I don't want to this right now

Longer term, I agree it could make sense to move those macros to a more 
common location, e.g

a generic DRM_WEDGE_URL in drm_device.h and XE_BUF_URL in xe_drv.h. I 
would like keep this change focused

rather than refactoring the patch.

>>> +
>>>    /**
>>>     * xe_device_declare_wedged - Declare device wedged
>>>     * @xe: xe device instance
>>> @@ -1427,12 +1431,12 @@ void xe_device_declare_wedged(struct xe_device *xe)
>>>        if (!atomic_xchg(&xe->wedged.flag, 1)) {
>>>            xe->needs_flr_on_fini = true;
>>>            xe_pm_runtime_get_noresume(xe);
>>> -        drm_err(&xe->drm,
>>> -            "CRITICAL: Xe has declared device %s as wedged.\n"
>>> -            "IOCTLs and executions are blocked.\n"
>>> -            "For recovery procedure, refer to https://docs.kernel.org/gpu/drm-uapi.html#device-wedging\n"
>>> -            "Please file a _new_ bug report at https://gitlab.freedesktop.org/drm/xe/kernel/issues/new\n",
>>> -            dev_name(xe->drm.dev));
>>> +
>>> +        xe_log_err_fatal(xe, WEDGED, -EIO, "Device declared wedged!\n");
>> Using -EIO is right? since we called this function with difference location (since we are not propagating errors), we may have different errors.
>>
>> which may be misleading with -EIO.
> true, but there is no errno available at this point
> and most callers don't use any errno at all

That's  a fair point.

-EIO was chosen as a generic one, but there is no meaning errno 
available at the point where the device is declared

wedged and most callers don't propagate that.

> we can change that to -ENOTRECOVERABLE, which was introduced
> recently to indicate a WEDGED state, but then it will be
> little redundant
This would be arguable be more accurate right. We can remove log message 
right? since errno itself explainable.
>
> or
>
> change the xe_device_declare_wedged signature and require
> errno to be assigned by the callers (but that would be
> a separate task/patch from this example
>
Agreed! I'd view that as a separate cleanup/refactoring task.

OR

xe_device struct we can keep sigid_errno as member we can use it 
whenever required rather than passing arguments. any thoughts?

Thanks,

-/Mallesh

>>
>> Thanks,
>>
>> -/Mallesh
>>
>>> +        xe_err_once(xe, "IOCTLs and executions are now blocked!\n"
>>> +                "For recovery procedure, refer to %s\n"
>>> +                "Please file a _new_ bug report at %s\n",
>>> +                WEDGED_URL, XE_BUG_URL);
>>>        }
>>>          for_each_gt(gt, xe, id)