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

Tauro, Riana
2026-03-31T16:24:48Z

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;
>> +}