Re: [PATCH v2 7/9] drm/xe/xe_ras: Disable hw error worker when AER handling is in progress

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: uninitialized hw_error_work disabled on VF in xe_pci_error_detected()
--

commit b14238c72b86ed2185123242102a8af0419bfeb1
Author: Riana Tauro <riana.tauro@intel.com>

drm/xe/xe_ras: Disable hw error worker when AER handling is in progress

When an uncorrectable internal AER error occurs on a device with workaround
14028434957, this commit marks the RAS state as using the AER path and
flushes and disables the hardware error worker in xe_pci_error_detected().

> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/xe_pci_error.c
> index 9bf0da3a3d152..b27f8a0f1b170 100644
> --- a/drivers/gpu/drm/xe/xe_pci_error.c
> +++ b/drivers/gpu/drm/xe/xe_pci_error.c

[ ... ]

> @@ -67,6 +91,15 @@ static pci_ers_result_t xe_pci_error_detected(struct pci_dev *pdev, pci_channel_
>  	if (xe_survivability_mode_is_boot_enabled(xe) || xe_wedged(xe))
>  		return PCI_ERS_RESULT_DISCONNECT;
>  
> +	/*
> +	 * If non-fatal error processing is in progress when uncorrectable internal error occurs,
> +	 * flush and disable the worker to prevent further processing, as a reset is imminent.
> +	 */
> +	if (XE_DEVICE_WA(xe, 14028434957) && is_aer_internal_error(pdev)) {
> +		xe_ras_set_aer_path(xe, true);
> +		xe_hw_error_work_disable(xe);
> +	}

[Severity: High]
Can this call xe_hw_error_work_disable() on an uninitialized tile->hw_error_work
when xe_pci_error_detected() runs on an SR-IOV VF?

On Crescent Island, SR-IOV is supported and workaround 14028434957 is
populated via xe_wa_process_device_oob() in xe_device_probe_early(), so
XE_DEVICE_WA(xe, 14028434957) evaluates to true on both PF and VF instances.

However, xe_hw_error_init() skips initializing tile->hw_error_work on VFs:

drivers/gpu/drm/xe/xe_hw_error.c:xe_hw_error_init()
	if (!IS_DGFX(xe) || IS_SRIOV_VF(xe))
		return;

	for_each_tile(tile, xe, id)
		INIT_DELAYED_WORK(&tile->hw_error_work, hw_error_work);

When an uncorrectable internal error occurs on the upstream switch port,
pcie_do_recovery() walks subordinate devices under the bridge and invokes
xe_pci_error_detected() for bound VFs as well as the PF. Because
pci_upstream_bridge() resolves pci_physfn(dev) for VFs,
is_aer_internal_error() returns true on the VF too, and
xe_hw_error_work_disable() then calls disable_delayed_work_sync() on
uninitialized delayed_work structs:

drivers/gpu/drm/xe/xe_hw_error.c:xe_hw_error_work_disable()
	for_each_tile(tile, xe, id)
		disable_delayed_work_sync(&tile->hw_error_work);

Should this path check !IS_SRIOV_VF(xe) (or xe->info.has_sysctrl) before
disabling the worker?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026060459.771448-11-riana.tauro@intel.com?part=7