[Intel-xe] [PATCH v2 1/1] drm/xe: Notify Userspace when engine/gt reset fails.

[John Harrison]

On 5/16/2023 04:41, Ghimiray, Himal Prasad wrote:
>> -----Original Message-----
>> From: Rodrigo Vivi <rodrigo.vivi at kernel.org>
>> Sent: 16 May 2023 06:18
>> To: Nilawar, Badal <badal.nilawar at intel.com>
>> Cc: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>; intel-
>> xe at lists.freedesktop.org; Harrison, John C <john.c.harrison at intel.com>;
>> Argenziano, Antonio <antonio.argenziano at intel.com>
>> Subject: Re: [Intel-xe] [PATCH v2 1/1] drm/xe: Notify Userspace when
>> engine/gt reset fails.
>>
>> On Mon, May 15, 2023 at 09:46:47PM +0530, Nilawar, Badal wrote:
>>>
>>> On 11-05-2023 17:13, Ghimiray, Himal Prasad wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Nilawar, Badal <badal.nilawar at intel.com>
>>>>> Sent: 11 May 2023 10:07
>>>>> To: Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>;
>>>>> intel- xe at lists.freedesktop.org
>>>>> Cc: Upadhyay, Tejas <tejas.upadhyay at intel.com>
>>>>> Subject: Re: [Intel-xe] [PATCH v2 1/1] drm/xe: Notify Userspace
>>>>> when engine/gt reset fails.
>>>>>
>>>>> Hi Himal,
>>>>>
>>>>> On 09-05-2023 13:55, Himal Prasad Ghimiray wrote:
[snip]

>>>>>> +
>>>>>>     int xe_guc_engine_reset_failure_handler(struct xe_guc *guc,
>>>>>> u32 *msg,
>>>>> u32 len)
>>>>>>     {
>>>>>>     	struct xe_device *xe = guc_to_xe(guc);
>>>>>> +	struct xe_hw_engine *hwe;
>>>>>>     	u8 guc_class, instance;
>>>>>> +	u16 engine_class;
>>>>>>     	u32 reason;
>>>>>>
>>>>>>     	if (unlikely(len != 3)) {
>>>>>> @@ -1608,6 +1625,12 @@ int
>>>>>> xe_guc_engine_reset_failure_handler(struct
>>>>> xe_guc *guc, u32 *msg, u32 len)
>>>>>>     	drm_err(&xe->drm, "GuC engine reset request failed on
>> %d:%d
>>>>> because 0x%08X",
>>>>>>     		guc_class, instance, reason);
>>>>>>
>>>>>> +	/* Notify userspace about engine reset failure */
>>>>>> +	engine_class = xe_guc_class_to_engine_class(guc_class);
>>>>>> +	hwe = xe_gt_hw_engine(guc_to_gt(guc), engine_class,
>> instance,
>>>>>> +false);The The instance here is considered as physical instance
>>>>>> +of hwe but while
>>>>> submitting contexts kmd passes logical instance to guc. With that
>>>>> I assume guc might be passing logical instance or for this
>>>>> notification it is passing physical instance?
>>>> AFAIK for reset failure notification the instance passed by guc in g2h
>> payload is physical instance .
>>>> @Harrison, John C Can you please confirm ?
>>> Spoke to @Antonio, he confirmed that to notify engine reset failure
>>> guc is reporting physical instance to host.
>>>
>>> Reviewed-by: Badal Nilawar <badal.nilawar at intel.com>
>> I don't believe this is what we want here still. At least not from the sysman
>> perspective.
>>
>> After guc reset fails, we will try to reset the GT ourselves and if we succeed
>> we don't need to request user space interference.
>>
>> That should only happen if we also fail our gt_reset after the guc_reset
>> failed.
>>
>> That would be the similar point on when i915 gets wedged?
>> We probably also need the wedge point here before we can issue this
>> notification?
> Hi Rodrigo,
> You are right, In case of engine reset failure the driver tries to do the  gt reset
> and if gt reset fails there are two notifications one for engine failure and another for gt
> reset failure. The changes to send uevent in case of gt failure are added in xe_uevent_gt_reset_failure.
>
> Level0 UMD does sends the RESET_REQUIRED uevent in case of only gt reset failure.
> Engine reset failure notification is not used for this purpose, it is to provide information that engine reset was required and it failed.
>
>> Anyway, we also need IGT for the uevent itself.
> The IGT for uevent listener will be helpful only if we can inject errors in such a way that they lead to engine/gt reset failure.
> I am unable to find a mechanism to do so. Can you provide some input on the same ?
> BR
> Himal
It should not be physically possible to inject an engine reset failure. 
The only way to do so would be via a hardware exploit. Same for a GT 
reset failure.

Bad user code can trigger an engine reset. Absolutely nothing should be 
able to make that reset fail. If it does, then the hardware is broken.

John.