Re: [PATCH v1] drm/xe/pci_error: Allow debugging PCI errors

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning PCI_ERS_RESULT_DISCONNECT does not prevent bus reset on fatal AER errors and causes the PCI core to tear down the device, defeating the patch's debugging goal.
--

--- Patch [1]: [PATCH v1] drm/xe/pci_error: Allow debugging PCI errors ---
commit bd082f56247e27f219ffa1505be8bdbb355ef59d
Author: Raag Jadav <[email protected]>

drm/xe/pci_error: Allow debugging PCI errors

XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET is intended for debugging hangs.
Disconnect the device on AER and prevent bus reset, so that it is available
to the user for debugging.

> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/xe_pci_error.c
> --- a/drivers/gpu/drm/xe/xe_pci_error.c
> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
> @@ -63,6 +63,12 @@ static pci_ers_result_t xe_pci_error_detected(struct pci_dev *pdev, pci_channel_
>  	if (xe_survivability_mode_is_boot_enabled(xe) || xe_device_wedged(xe))
>  		return PCI_ERS_RESULT_DISCONNECT;
>  
> +	/* User wants to debug the error, prevent reset */
> +	if (xe->wedged.mode == XE_WEDGED_MODE_UPON_ANY_HANG_NO_RESET) {
> +		xe_device_declare_wedged(xe);
> +		return PCI_ERS_RESULT_DISCONNECT;
> +	}
> +

[Severity: High]
Does returning PCI_ERS_RESULT_DISCONNECT actually prevent the bus reset
and keep the device available for debugging?

Looking at the PCI core in pcie_do_recovery(), if the error is fatal 
(state == pci_channel_io_frozen), a bus reset is issued unconditionally 
regardless of the result returned by the driver's error_detected() callback:

drivers/pci/pcie/err.c:pcie_do_recovery() {
	...
	status = report_error_detected(dev, state, &pci_ers_result_none);

	if (status == PCI_ERS_RESULT_NEED_RESET ||
	    state == pci_channel_io_frozen) {
		if (reset_subordinates(bridge) != PCI_ERS_RESULT_RECOVERED) {
	...
}

Furthermore, returning PCI_ERS_RESULT_DISCONNECT means the status is not
PCI_ERS_RESULT_RECOVERED. This forces the PCI core to jump to its failure
path, which notifies the driver with pci_channel_io_perm_failure and
subsequently tears down the device.

Will this unintentionally destroy the very state the user is trying
to debug, rather than keeping the device available?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1