Re: [PATCH v12 4/4] drm/xe/ras: Use fault-inject to trigger punit error handler

On 30-07-2026 12:56 pm, Mallesh, Koujalagi wrote:
>
> On 29-07-2026 12:55 am, Rodrigo Vivi wrote:
>> On Fri, Jul 24, 2026 at 03:33:07PM +0530, Mallesh Koujalagi wrote:
>>> Use fault-inject framework to trigger punit_error_handler()
>>> for testing.
>>>
>>> Usage:
>>>    echo 100 > .../inject_punit_error/probability
>>>    echo 1   > .../inject_punit_error/times
>>>
>>> Signed-off-by: Mallesh Koujalagi <[email protected]>
>>> ---
>>>   drivers/gpu/drm/xe/xe_debugfs.c |  4 ++++
>>>   drivers/gpu/drm/xe/xe_debugfs.h |  2 ++
>>>   drivers/gpu/drm/xe/xe_ras.c     | 17 +++++++++++++++++
>>>   3 files changed, 23 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c 
>>> b/drivers/gpu/drm/xe/xe_debugfs.c
>>> index 5a3877fcb0f0..5599d493feca 100644
>>> --- a/drivers/gpu/drm/xe/xe_debugfs.c
>>> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
>>> @@ -42,6 +42,7 @@
>>>     DECLARE_FAULT_ATTR(gt_reset_failure);
>>>   DECLARE_FAULT_ATTR(inject_csc_hw_error);
>>> +DECLARE_FAULT_ATTR(inject_punit_error);
>>>     static bool csc_hw_error_available(struct xe_device *xe)
>>>   {
>>> @@ -62,6 +63,8 @@ static struct {
>>>       { .name = "inject_csc_hw_error",
>>>         .attr = &inject_csc_hw_error,
>>>         .is_visible = csc_hw_error_available },
>>> +    { .name = "inject_punit_error",
>>> +      .attr = &inject_punit_error },
>>>   };
>>>     /*
>>> @@ -76,6 +79,7 @@ bool xe_fault_##name(void)                \
>>>     FAULT_ACTION(gt_reset, gt_reset_failure)
>>>   FAULT_ACTION(csc_hw_error, inject_csc_hw_error)
>>> +FAULT_ACTION(punit_error, inject_punit_error)
>>>     static void xe_fault_inject_debugfs_register(struct xe_device *xe,
>>>                            struct dentry *root)
>>> diff --git a/drivers/gpu/drm/xe/xe_debugfs.h 
>>> b/drivers/gpu/drm/xe/xe_debugfs.h
>>> index cd56f7442b99..dd57914dd4f2 100644
>>> --- a/drivers/gpu/drm/xe/xe_debugfs.h
>>> +++ b/drivers/gpu/drm/xe/xe_debugfs.h
>>> @@ -13,10 +13,12 @@ struct xe_device;
>>>   #ifdef CONFIG_DEBUG_FS
>>>   bool xe_fault_gt_reset(void);
>>>   bool xe_fault_csc_hw_error(void);
>>> +bool xe_fault_punit_error(void);
>>>   void xe_debugfs_register(struct xe_device *xe);
>>>   #else
>>>   static inline bool xe_fault_gt_reset(void) { return false; }
>>>   static inline bool xe_fault_csc_hw_error(void) { return false; }
>>> +static inline bool xe_fault_punit_error(void) { return false; }
>>>   static inline void xe_debugfs_register(struct xe_device *xe) { }
>>>   #endif
>>>   diff --git a/drivers/gpu/drm/xe/xe_ras.c 
>>> b/drivers/gpu/drm/xe/xe_ras.c
>>> index 92b4181026cb..da06c16cf7ed 100644
>>> --- a/drivers/gpu/drm/xe/xe_ras.c
>>> +++ b/drivers/gpu/drm/xe/xe_ras.c
>>> @@ -3,6 +3,7 @@
>>>    * Copyright © 2026 Intel Corporation
>>>    */
>>>   +#include "xe_debugfs.h"
>>>   #include "xe_device.h"
>>>   #include "xe_drm_ras.h"
>>>   #include "xe_pm.h"
>>> @@ -391,6 +392,22 @@ enum xe_ras_recovery_action 
>>> xe_ras_process_errors(struct xe_device *xe)
>>>       size_t rlen;
>>>       int ret;
>>>   +    /*
>>> +     * Only allow the injected PUNIT error once the DRM device is 
>>> registered.
>> That entirely defeats the purpose! :)
>>
>> Sashiko identified this corner case so I asked you to ensure that 
>> this is covered
>> and to prove that you should provide a fault-inject with test case.
>>
>> But then you are skipping exactly the corner case. That makes 
>> absolutely no sense.
>
> You are absolutely right!! Guarding the injection with 
> xe->drm.registered was the wrong approach.
>
> The correct fix is structural, we are splitting xe_ras_init function 
> into xe_ras_init_early() (keep all bookkeeping) and
>
> xe_ras_init() (called after  drm_dev_register in which 
> xe_ras_process_errors now lives).
>
>
> Thanks,
>
> -/Mallesh
>
Sashiko corner case issue is addressed with patch: 
https://gitlab.freedesktop.org/drm/tip/-/commit/20bc4883c7c0e28c3ba6c76ccc279486c349dd3e

Do we still need this patch? Please let me know anything else I need to 
address here.

Thanks,

-/Mallesh

>>> +     * xe_ras_process_errors() also runs during probe (via 
>>> xe_ras_init()), before
>>> +     * drm_dev_register() calls device_add() on the DRM minor's 
>>> kdev. At that
>>> +     * point kdev->kobj.parent is not yet linked into the sysfs 
>>> hierarchy, so
>>> +     * kobject_get_path() returns "/card0" instead of the real 
>>> sysfs path.
>>> +     * drm_dev_wedged_event() would then emit a KOBJ_CHANGE uevent 
>>> with a wrong
>>> +     * DEVPATH that udev cannot resolve, silently dropping the 
>>> event and leaving
>>> +     * the cold-reset recovery broken.
>>> +     */
>>> +    if (xe->drm.registered && xe_fault_punit_error()) {
>>> +        xe_err(xe, "[RAS]: PUNIT error injected\n");
>>> +        punit_error_handler(xe);
>>> +        return XE_RAS_RECOVERY_ACTION_DISCONNECT;
>>> +    }
>>> +
>>>       if (!xe->info.has_sysctrl)
>>>           return XE_RAS_RECOVERY_ACTION_RESET;
>>>   --
>>> 2.48.1
>>>