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

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The driver improperly imports the 'CXL' module namespace to bypass symbol export restrictions for pcie_aer_is_native().

Pre-existing issues:
- [High] The driver directly modifies the PCI configuration space of an unowned upstream switch port (USP) and calls pci_save_state() on it without synchronization.
--

--- Patch [1]: [PATCH v1] drm/xe/ras: Enable SGUnit PCIe error reporting ---
commit e3840dec09cb480f65642a26c3093d383b1db319
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/ras: Enable SGUnit PCIe error reporting

Enable SGUnit PCIe error reporting by programming its config space for
upstream error propagation. Adds a native AER sanity check for the upstream
port before programming SGUnit registers.

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index d25d25f775310..9979fe5b4ceb0 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 does modifying the PCI
configuration space of an unowned upstream switch port (USP) and saving
its state here in ras_usp_aer_init() cause data races?

If multiple GPU endpoints are probed concurrently on the same upstream
port, bypassing the core PCI layer to modify an unowned bridge and calling
pci_save_state(usp) without first acquiring pci_dev_lock(usp) could blindly
overwrite the saved state array and corrupt PM state restorations managed
by the PCI core.

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

[Severity: Medium]
Is it appropriate to spoof the CXL namespace here to access
pcie_aer_is_native()?

Since pcie_aer_is_native() is explicitly exported to the CXL namespace to
restrict its use to Compute Express Link drivers, masquerading as a CXL
driver to circumvent this boundary seems to violate kernel namespace
protection principles.

If non-CXL endpoint drivers legitimately require generic access to AER
native status, could the PCI core API be made generic instead?

>  
>  static void ras_send_error_event(struct xe_device *xe, u8 severity, u8 component)

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