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

Raag Jadav
2026-03-04T16:52:39Z

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 ;)

> 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.

> + 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.

> + 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.

Raag

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