Re: [PATCH v2 4/9] drm/xe/xe_hw_error: Handle non-fatal errors routed via direct IRQ

["Tauro, Riana" <riana.tauro@intel.com>]

On 29-10-2026 12:06, Mallesh, Koujalagi wrote:
>
> On 26-10-2026 07:38 pm, Tauro, Riana wrote:
>>
>> On 26-10-2026 16:19, Upadhyay, Tejas wrote:
>>>
>>>> -----Original Message-----
>>>> From: Tauro, Riana <riana.tauro@intel.com>
>>>> Sent: 26 October 2026 11:35
>>>> To: intel-xe@lists.freedesktop.org
>>>> Cc: Tauro, Riana <riana.tauro@intel.com>; Gupta, Anshuman
>>>> <anshuman.gupta@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
>>>> aravind.iddamsetty@linux.intel.com; Nilawar, Badal
>>>> <badal.nilawar@intel.com>; Jadav, Raag <raag.jadav@intel.com>;
>>>> Koppuravuri, Ravi Kishore <ravi.kishore.koppuravuri@intel.com>; 
>>>> Koujalagi,
>>>> Mallesh <mallesh.koujalagi@intel.com>; Upadhyay, Tejas
>>>> <tejas.upadhyay@intel.com>
>>>> Subject: [PATCH v2 4/9] drm/xe/xe_hw_error: Handle non-fatal errors 
>>>> routed
>>>> via direct IRQ
>>>>
>>>> Process, log and recover non-fatal errors routed via direct IRQ.
>>>> Double-bit ecc errors belong to non-fatal error category and are 
>>>> reported via
>>>> irq. For pages that belong to critical BO, wedge the device with 
>>>> bus-reset as
>>>> the recovery action.
>>>>
>>>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/xe/xe_hw_error.c | 37
>>>> ++++++++++++++++++++++++++++----
>>>>   drivers/gpu/drm/xe/xe_ras.c      |  9 +++++++-
>>>>   2 files changed, 41 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_hw_error.c
>>>> b/drivers/gpu/drm/xe/xe_hw_error.c
>>>> index f7799c3c08aa..269ce6a19a80 100644
>>>> --- a/drivers/gpu/drm/xe/xe_hw_error.c
>>>> +++ b/drivers/gpu/drm/xe/xe_hw_error.c
>>>> @@ -5,6 +5,8 @@
>>>>
>>>>   #include <linux/bitmap.h>
>>>>
>>>> +#include <generated/xe_device_wa_oob.h>
>>>> +
>>>>   #include "regs/xe_gsc_regs.h"
>>>>   #include "regs/xe_hw_error_regs.h"
>>>>   #include "regs/xe_irq_regs.h"
>>>> @@ -15,7 +17,9 @@
>>>>   #include "xe_hw_error.h"
>>>>   #include "xe_mmio.h"
>>>>   #include "xe_pm.h"
>>>> +#include "xe_ras.h"
>>>>   #include "xe_survivability_mode.h"
>>>> +#include "xe_wa.h"
>>>>
>>>>   #define GT_HW_ERROR_MAX_ERR_BITS        16
>>>>   #define HEC_UNCORR_FW_ERR_BITS            4
>>>> @@ -166,12 +170,36 @@
>>>> static_assert(ARRAY_SIZE(pvc_master_local_nonfatal_err_reg) ==
>>>> XE_RAS_REG_SIZE);
>>>>
>>>> pvc_master_local_fatal_err_reg : \
>>>>
>>>> pvc_master_local_nonfatal_err_reg)
>>>>
>>>> +static const char *hw_err_to_str(const enum hardware_error hw_err) {
>>>> +    switch (hw_err) {
>>>> +    case HARDWARE_ERROR_CORRECTABLE:
>>>> +        return "Correctable";
>>>> +    case HARDWARE_ERROR_NONFATAL:
>>>> +        return "Non-Fatal";
>>>> +    case HARDWARE_ERROR_FATAL:
>>>> +        return "Fatal";
>>>> +    default:
>>>> +        return "Unknown";
>>>> +    }
>>>> +}
>>>> +
>>>>   static void hw_error_work(struct work_struct *work)  {
>>>>       struct xe_tile *tile = container_of(work, typeof(*tile),
>>>> hw_error_work.work);
>>>>       struct xe_device *xe = tile_to_xe(tile);
>>>>
>>>>       guard(xe_pm_runtime)(xe);
>>>> +
>>>> +    if (xe->info.has_sysctrl && XE_DEVICE_WA(xe, 14028434957)) {
>>> I think you should have all these in 1 patch for 14028434957
>>
>> I can combine 3 and 4.  If i combine all usages, it would be 
>> difficult to review.
>>
>> Thanks
>> Riana
>>
>>>
>>> Tejas
>>>> +        /* If the device is in reset in AER path, skip processing
>>>> hardware errors */
>>>> +        if (xe_device_is_in_reset(xe))
>>>> +            return;
>>>> +
>>>> +        xe_ras_process_errors(xe);
>>>> +        return;
>>>> +    }
>>>> +
>>>>       xe_survivability_mode_runtime_enable(xe);
>>>>   }
>>>>
>>>> @@ -433,12 +461,13 @@ static void hw_error_source_handler(struct 
>>>> xe_tile
>>>> *tile, const enum hardware_er
>>>>           return;
>>>>
>>>>       /*
>>>> -     * Hardware errors are reported through System Controller on the
>>>> platforms that
>>>> -     * support it, and never routed as direct IRQ to SGUnit. So we 
>>>> should
>>>> never be
>>>> -     * here for those platforms.
>>>> +     * Non fatal errors are routed as a direct IRQ on platforms 
>>>> that support
>>>> +     * system controller.
>>>>        */
>>>>       if (xe->info.has_sysctrl) {
>>>> -        drm_err_ratelimited(&xe->drm, HW_ERR "Invalid error
>>>> routing\n");
>>>> +        drm_err_ratelimited(&xe->drm, HW_ERR "%s %s reported\n",
>>>> hw_err_to_str(hw_err),
>>>> +                    severity_str);
>>>> +        schedule_delayed_work(&tile->hw_error_work,
>>>> +msecs_to_jiffies(MSEC_PER_SEC));
>
> hmm, why we are scheduling hw_error_work worker after 1 sec, if we get 
> AER before and after 1 sec, what is impact?

Its a best case scenario that if there is a non-fatal error which is 
accompanied with a fatal at the same time. We will wait
for AER to get triggered to avoid unnecessary handling of non-fatal errors.
  It is not guaranteed that we will see a AER in 1sec because even that 
is a worker.

If we get AER when irq handling is in progress, it will wait for it to 
complete before triggering a reset,

After 1sec is a same as normal scenario of irq followed by AER

Thanks
Riana

>
> Thanks,
>
> -/Mallesh
>
>>>>           return;
>>>>       }
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_ras.c 
>>>> b/drivers/gpu/drm/xe/xe_ras.c index
>>>> db3cbc186a51..e6a0ee468b17 100644
>>>> --- a/drivers/gpu/drm/xe/xe_ras.c
>>>> +++ b/drivers/gpu/drm/xe/xe_ras.c
>>>> @@ -628,8 +628,15 @@ static u8 handle_device_memory_errors(struct
>>>> xe_device *xe, struct xe_ras_error_
>>>>           break;
>>>>       case XE_RAS_MEMORY_DB_ECC:
>>>>           ret = handle_page_offline(xe, info->sw_address, true);
>>>> -        if (ret)
>>>> +        /*
>>>> +         * Double-bit ECC errors are non-fatal errors and are 
>>>> reported
>>>> via IRQ.
>>>> +         * Wedge the device with a bus reset for errors that require
>>>> SBR to recover.
>>>> +         * TODO: Trigger SBR using pci_reset_bus
>>>> +         */
>>>> +        if (ret) {
>>>> +            xe_wedge_declare(xe,
>>>> DRM_WEDGE_RECOVERY_BUS_RESET);
>>>>               return XE_RAS_RECOVERY_ACTION_RESET;
>>>> +        }
>>>>           break;
>>>>       default:
>>>>           return XE_RAS_RECOVERY_ACTION_RESET;
>>>> -- 
>>>> 2.47.1