Re: [PATCH v5 07/13] drm/xe/xe_ras: Report device memory errors using SIGID

["Nilawar, Badal" <badal.nilawar@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3197 bytes --]


On 27-10-2026 06:59, Raag Jadav wrote:
> On Tue, Oct 27, 2026 at 12:01:42AM +0530, Nilawar, Badal wrote:
>> On 26-10-2026 18:17, Raag Jadav wrote:
>>> On Fri, Oct 23, 2026 at 10:04:29AM +0530, Badal Nilawar wrote:
>>>> Use xe_log_comp_info helper to report device memory errors using the
>>>> corresponding SIGID.
>>>>
>>>> Example:
>>>> 	xe 0000:03:00.0: [drm] SIGID=202 (020100020008002000300000)
>>>> DEVICE_MEMORY: Double-bit ECC error detected at sw address 0x63000
>>>>
>>>> Reviewed-by: Mallesh Koujalagi<mallesh.koujalagi@intel.com>
>>>> Signed-off-by: Badal Nilawar<badal.nilawar@intel.com>
>>>> Cc: Riana Tauro<riana.tauro@intel.com>
>>>> ---
>>>> v5:
>>>>     - Log unknown memory errors
>>>>     - Add example log in commit message (Raag)
>>>> ---
>>>>    drivers/gpu/drm/xe/xe_ras.c | 20 +++++++++++++++++---
>>>>    1 file changed, 17 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
>>>> index 95e87c635b04..5ebfc6e8dfc7 100644
>>>> --- a/drivers/gpu/drm/xe/xe_ras.c
>>>> +++ b/drivers/gpu/drm/xe/xe_ras.c
>>>> @@ -643,6 +643,7 @@ static u8 handle_soc_internal_errors(struct xe_device *xe, struct xe_ras_error_a
>>>>    static u8 handle_device_memory_errors(struct xe_device *xe, struct xe_ras_error_array *arr)
>>>>    {
>>>>    	struct xe_ras_memory_error *info = (void *)arr->details;
>>>> +	u8 cper_sev = ras_sev_to_cper_sev(arr->counter.common.severity);
>>>>    	int ret;
>>>>    	/*
>>>> @@ -654,17 +655,30 @@ static u8 handle_device_memory_errors(struct xe_device *xe, struct xe_ras_error_
>>>>    	 */
>>>>    	switch (info->category) {
>>>>    	case XE_RAS_MEMORY_POISON:
>>>> -		xe_info(xe, "[RAS]: Poison error detected\n");
>>>> +		xe_log_comp_info(xe, DEVICE_MEMORY, &arr->counter, sizeof(arr->counter),
>>>> +				 "Poison error detected\n");
>>>>    		break;
>>>>    	case XE_RAS_MEMORY_DATA_PARITY:
>>>> -		xe_info(xe, "[RAS]: Data parity error detected\n");
>>>> +		xe_log_comp_info(xe, DEVICE_MEMORY, &arr->counter, sizeof(arr->counter),
>>>> +				 "Data parity error detected\n");
>>>>    		break;
>>>>    	case XE_RAS_MEMORY_DB_ECC:
>>>>    		ret = handle_page_offline(xe, info->sw_address, true);
>>>> -		if (ret)
>>>> +		if (ret) {
>>>> +			xe_log_comp(xe, cper_sev, DEVICE_MEMORY,
>>>> +				    &arr->counter, sizeof(arr->counter),
>>>> +				    "Failed to recover double-bit ECC error at sw address 0x%llx\n",
>>>> +				    info->sw_address);
>>> Isn't handle_page_offline() already logging this? If no, shouldn't we
>>> try to consolidate?
>> It doesn't emit CPER.
> So perhaps convert them into cper instead of duplicating logs?

I'd prefer to keep the CPER emission in handle_device_memory_errors(), 
which is called from xe_ras_process_error().

handle_page_offline() and its sub-functions invoke SIGID at multiple 
places, not all of which require CPER emission. These functions are also 
called outside the error-handling path.

Moving CPER emission into these helpers could therefore result in 
multiple hardware CPER records being emitted for a single error event.

My suggestion to authors of this feature is to clean up SIGID logging in 
handle_page_offline().

Thanks,
Badal

>
> Raag

[-- Attachment #2: Type: text/html, Size: 4479 bytes --]