Re: [PATCH v1] drm/xe/guc: Allow GuC CT for wedged device

["Laguna, Lukasz" <lukasz.laguna@intel.com>]

On 8/25/2026 14:49, Raag Jadav wrote:
> On Tue, Aug 25, 2026 at 12:53:35PM +0200, Laguna, Lukasz wrote:
>> On 8/25/2026 12:04, Raag Jadav wrote:
>>> On Tue, Aug 25, 2026 at 11:39:44AM +0200, Laguna, Lukasz wrote:
>>>> On 8/21/2026 14:21, Raag Jadav wrote:
>>>>> Commit 50fa9acac26f ("drm/xe/guc: distinguish wedged from recoverable
>>>>> cancellation") introduced distinguishable error codes for g2h failure
>>>>> cases, but also blocked GuC CT for wedged device. This is problematic
>>>>> in cases where we want to prevent user from accessing the device but
>>>>> also keep GuC CT functioning on temporarily wedged device. First user
>>>>> of such requirement is PCIe FLR handling where we require uC firmware
>>>>> loading while the device is temporarily wedged.
>>>>>
>>>>> Fixes: 50fa9acac26f ("drm/xe/guc: distinguish wedged from recoverable cancellation")
>>>>> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
>>>>> ---
>>>>>     drivers/gpu/drm/xe/xe_guc_ct.c | 8 --------
>>>>>     1 file changed, 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
>>>>> index 5c4733da385c..97e38147effd 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
>>>>> @@ -1062,11 +1062,6 @@ static int __guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action,
>>>>>     	xe_gt_assert(gt, g2h_len || !num_g2h);
>>>>>     	lockdep_assert_held(&ct->lock);
>>>>> -	if (xe_device_wedged(ct_to_xe(ct))) {
>>>>> -		ret = -ENOTRECOVERABLE;
>>>>> -		goto out;
>>>>> -	}
>>>>> -
>>>>>     	if (unlikely(ct->ctbs.h2g.info.broken)) {
>>>>>     		ret = -EPIPE;
>>>>>     		goto out;
>>>>> @@ -1813,9 +1808,6 @@ static int g2h_read(struct xe_guc_ct *ct, u32 *msg, bool fast_path)
>>>>>     	xe_gt_assert(gt, xe_guc_ct_initialized(ct));
>>>>>     	lockdep_assert_held(&ct->fast_lock);
>>>>> -	if (xe_device_wedged(xe))
>>>>> -		return -ENOTRECOVERABLE;
>>>>> -
>>>>>     	if (ct->state == XE_GUC_CT_STATE_DISABLED)
>>>>>     		return -ENODEV;
>>>> There's also third instance in guc_ct_send_recv().
>>>>
>>>> Shouldn't we distinguish between temporary and permanent wedge here rather
>>>> than removing the checks entirely?
>>> I thought of adding a xe_device_wedged_perm() but this would create
>>> confusion with existing xe_device_wedged() regardling it's usage.
>>> So perhaps we need a better name? Or a better idea? Open to suggestions.
>> What about xe_device_needs_recovery()?
>> In this case, we should also change the ENOTRECOVERABLE to something else,
>> maybe ENOLINK or ESHUTDOWN?
> Or make it the other way around? Perhaps xe_device_not_recoverable()?

I was also thinking about it, but then I thought that it's too heavy as 
the device can be recovered with some recovery method.

Now I see that it's a consistent terminology in the driver for 
permanently wedged device, so yeah, I think we can go with 
xe_device_not_recoverable() and leave the
-ENOTRECOVERABLE error.

Lukasz

> Raag
>
>>>>> And it looks like it should be a part of the FLR series. This patch doesn't
>>>> make sense without this series, and FLR series doesn't work without it.
>>>> I'm also not sure if it deserves the fixes tag, it wasn't broken at the
>>>> time.
>>> Agree, will add it as part of FLR series once we have some conclusion.
>>>
>>> Raag