Re: [PATCH v2 08/11] drm/xe/xe_ras: Add support for Uncorrectable Core-Compute errors

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

On 3/4/2026 10:22 PM, Raag Jadav wrote:
> On Mon, Mar 02, 2026 at 03:52:03PM +0530, Riana Tauro wrote:
>> Uncorrectable Core-Compute errors are classified into Global and Local
>> errors.
>>
>> Global error is an error that affects the entire device requiring a
>> reset. This type of error is not isolated. When an AER is reported and
>> error_detected is invoked return PCI_ERS_RESULT_NEED_RESET.
>>
>> A Local error is confined to a specific component or context like a
>> engine. These errors can be contained and recovered by resetting
>> only the affected part without distrupting the rest of the device.
>>
>> Upon detection of an Uncorrectable Local Core-Compute error, an AER is
>> generated and GuC is notified of the error. The KMD then sets
>> the context as non-runnable and initiates an engine reset.
>> (TODO: GuC <->KMD communication for the error).
> TODOs are more useful in the code, so we can actually find them ;)


Yeah but the code will not be in xe_ras.  I will add a comment on why we 
return recovered
for local uncorrectable GT error.

>
>> Since the error is contained and recovered, PCI error handling
>> callback returns PCI_ERS_RESULT_RECOVERED.
> ...
>
>> +enum xe_ras_recovery_action xe_ras_process_errors(struct xe_device *xe)
>> +{
>> +	struct xe_sysctrl_mailbox_command command = {0};
>> +	struct xe_ras_get_error_response response;
>> +	enum xe_ras_recovery_action final_action;
>> +	size_t rlen;
>> +	int ret;
>> +
>> +	/* Default action */
>> +	final_action = XE_RAS_RECOVERY_ACTION_RECOVERED;
>> +
>> +	if (!xe->info.has_sysctrl)
>> +		return XE_RAS_RECOVERY_ACTION_RESET;
>> +
>> +	xe_ras_prepare_sysctrl_command(&command, XE_SYSCTRL_CMD_GET_SOC_ERROR, NULL, 0,
>> +				       &response, sizeof(response));
>> +
>> +	do {
>> +		memset(&response, 0, sizeof(response));
>> +		rlen = 0;
>> +
>> +		ret = xe_sysctrl_send_command(xe, &command, &rlen);
>> +		if (ret || !rlen) {
> We'd probably want them to be separate cases so we know what actually
> happened. Besides, I think !rlen is redundant here since you're already
> handling it below.
Sure will fix this.
>
>> +			xe_err(xe, "[RAS]: Sysctrl error ret %d\n", ret);
>> +			goto err;
>> +		}
>> +
>> +		if (rlen != sizeof(response)) {
>> +			xe_err(xe, "[RAS]: Sysctrl response does not match len!!\n");
> I'd print rlen as well.
>
>> +			goto err;
>> +		}
>> +
>> +		if (response.num_errors > XE_RAS_NUM_ERROR_ARR) {
> I'd handle this as part of for loop below so we atleast have the chance
> to recover based on initial errors.
yeah makes sense. Will add it as part of the loop
>
>> +			xe_err(xe, "[RAS]: Number of errors out of bound (%d)\n",
>> +			       XE_RAS_NUM_ERROR_ARR);
>> +			goto err;
>> +		}
>> +
>> +		for (int i = 0; i < response.num_errors; i++) {
> 		for (int i = 0; i < response.num_errors && i < XE_RAS_NUM_ERROR_ARR; i++)
>
>> +			struct xe_ras_error_array arr = response.error_arr[i];
>> +			enum xe_ras_recovery_action action;
>> +			struct xe_ras_error_class error_class;
>> +			u8 component;
>> +
>> +			error_class = arr.error_class;
>> +			component = error_class.common.component;
>> +
>> +			switch (component) {
>> +			case XE_RAS_COMPONENT_CORE_COMPUTE:
>> +				action = handle_compute_errors(xe, &arr);
>> +				break;
>> +			default:
>> +				xe_err(xe, "[RAS]: Unknown error component %u\n", component);
>> +				break;
>> +			}
>> +
>> +			/*
>> +			 * Retain the highest severity action. Process and log all errors
>> +			 * and then take appropriate recovery action
> Punctuations.
>
>> +			 */
>> +			if (action > final_action)
>> +				final_action = action;
>> +		}
>> +
>> +	} while (response.additional_errors);
> I know we're not NASA but I'd try to have some timeout instead of blindly
> trusting the hardware.

additional_errors is a indication. So will be 0/1.
Timeout is already present in sysctrl_send_command. So unnecessary here 
again.

Also before every sysctrl command we are setting response to 0.

Thanks
Riana


>
> Raag
>
>> +	return final_action;
>> +
>> +err:
>> +	return XE_RAS_RECOVERY_ACTION_RESET;
>> +}