Re: [PATCH v2 2/2] drm/xe/ras: Enable SGUnit PCIe error reporting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unprotected call to pci_save_state() on a shared upstream device can cause a data race.
--

--- Patch [2]: [PATCH v2 2/2] drm/xe/ras: Enable SGUnit PCIe error reporting ---
commit 4e1cc07b97a799fd7dc8e01bed43ae859ed4cc97
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/ras: Enable SGUnit PCIe error reporting

This commit hacks the SGUnit config space to program error reporting bits
in the DevCtl register since it is not AER capable. It also adds a native
AER sanity check for the upstream port.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index d25d25f775310..3118c713c1186 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
[ ... ]
> @@ -279,6 +292,7 @@ static void ras_usp_aer_init(struct xe_device *xe)
>  	/* Unmask Uncorrectable Internal Error */
>  	pci_clear_and_set_config_dword(usp, aer_cap + PCI_ERR_UNCOR_MASK, PCI_ERR_UNC_INTN, 0);
>  
>  	pci_save_state(usp);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
pci_save_state(usp) on a shared upstream device without locking?

Looking at ras_usp_aer_init() in drivers/gpu/drm/xe/xe_ras.c, if multiple
SGUnits (endpoints) under the same upstream switch port are probed or
initialized concurrently (for example, via async probe of multiple GPU tiles
or functions), they will concurrently execute pci_save_state(usp) on the same
shared upstream port without acquiring the device lock.

Could this lead to a data race, writing directly to usp->saved_config_space
and other PM state arrays simultaneously, and corrupt the PCI PM state for the
upstream switch port?

>  	dev_dbg(&usp->dev, "Uncorrectable Internal Errors downgraded and unmasked\n");
>  }
> +MODULE_IMPORT_NS("AER");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825054312.1319951-1-raag.jadav@intel.com?part=2