Re: [PATCH v2 11/11] drm/xe/xe_pci_error: Process errors in mmio_enabled

Mallesh, Koujalagi
2026-03-12T08:09:05Z

On 11-03-2026 08:09 pm, Riana Tauro wrote:
> Hi Mallesh
>
> On 3/11/2026 12:40 PM, Mallesh, Koujalagi wrote:
>>
>> On 02-03-2026 03:52 pm, Riana Tauro wrote:
>>> Query system controller when any non fatal error occurs to check
>>> the type of the error, contain and recover.
>>>
>>> The system controller is queried in the mmio_enabled callback.
>>>
>>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>>> ---
>>> v2: use ras recovery enum (Raag)
>>> ---
>>>   drivers/gpu/drm/xe/xe_pci_error.c | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/
>>> xe_pci_error.c
>>> index ba62868f00d4..35bc5b8cee99 100644
>>> --- a/drivers/gpu/drm/xe/xe_pci_error.c
>>> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
>>> @@ -9,6 +9,7 @@
>>>   #include "xe_device.h"
>>>   #include "xe_gt.h"
>>>   #include "xe_pci.h"
>>> +#include "xe_ras.h"
>>>   #include "xe_uc.h"
>>>   static void xe_pci_error_handling(struct pci_dev *pdev)
>>> @@ -26,6 +27,12 @@ static void xe_pci_error_handling(struct pci_dev
>>> *pdev)
>>>       pci_disable_device(pdev);
>>>   }
>> Explain the mapping rationale
>
> Do you mean commit message?
> In the code the naming is self explanatory

Not commit message. Add  a brief code comments
(ras_recovery_action_to_pci_result) about map RAS recovery action to PCI
AER results.

For example DISCONNECT: Require system intervention (FW update, cold reset)

Since we called PCI DISCONNECT when CSC Firmware errors that need a
flash update.

Thanks

-/Mallesh

>
>>> +static pci_ers_result_t ras_recovery_action_to_pci_result[] = {
>> The mapping array should be constant, never modified.
>>> +    [XE_RAS_RECOVERY_ACTION_RECOVERED] = PCI_ERS_RESULT_RECOVERED,
>>> +    [XE_RAS_RECOVERY_ACTION_RESET] = PCI_ERS_RESULT_NEED_RESET,
>>> +    [XE_RAS_RECOVERY_ACTION_DISCONNECT] = PCI_ERS_RESULT_DISCONNECT,
>>> +};
>>> +
>>>   static pci_ers_result_t xe_pci_error_detected(struct pci_dev
>>> *pdev, pci_channel_state_t state)
>>>   {
>>>       struct xe_device *xe = pdev_to_xe_device(pdev);
>>> @@ -50,9 +57,13 @@ static pci_ers_result_t
>>> xe_pci_error_detected(struct pci_dev *pdev, pci_channel_
>>>   static pci_ers_result_t xe_pci_error_mmio_enabled(struct pci_dev
>>> *pdev)
>>>   {
>>> +    struct xe_device *xe = pdev_to_xe_device(pdev);
>>> +    enum xe_ras_recovery_action action;
>> Add action_max @ end of enum to validate.
>
> Answered below, but for consistency i can add max
>
>>> +
>>>       dev_err(&pdev->dev, "Xe Pci error recovery: MMIO enabled\n");
>>> +    action = xe_ras_process_errors(xe);
>> What will happen, if RAS processing takes significant time?
>
> System controller times out after a duration. Refer patch
> https://patchwork.freedesktop.org/patch/710637/?series=159554&rev=10
>
> If the ret is negative in process_errors
>
> +        ret = xe_sysctrl_send_command(xe, &command, &rlen);
> +        if (ret || !rlen) {
> +            xe_err(xe, "[RAS]: Sysctrl error ret %d\n", ret);
> +            goto err;
> +        }
> +
>
> We request for a reset assuming a SC error.
>
>
>>> -    return PCI_ERS_RESULT_NEED_RESET;
>>
>> Use array bound check before using action to avoid out of bounds
>> array access
>
> The action here comes from a xe_ras_process_errors which also returns
> enum. This action is explicitly sent and is not coming from user
> or firmware.
>
> Adding a check here seems unnecessary.
>
> Thanks
> Riana
>
>>
>> Thanks,
>>
>> -/Mallesh
>>
>>> +    return ras_recovery_action_to_pci_result[action];
>>>   }
>>>   static pci_ers_result_t xe_pci_error_slot_reset(struct pci_dev *pdev)
>