Re: [PATCH 3/8] drm/xe/xe_pci_error: Group all devres to release them on PCIe slot reset

["Mallesh, Koujalagi" <mallesh.koujalagi@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3299 bytes --]

Hi Riana,

On 22-01-2026 03:36 pm, Riana Tauro wrote:
> Add devres grouping to handle device resource cleanup during
> PCI error recovery.
>
> Secondary Bus Reset (SBR) is triggered by PCI core when the
> error_detected/mmio_enabled callbacks return PCI_ERS_RESULT_NEED_RESET.
>
> Once SBR is complete, the slot_reset callback is triggered. SBR wipes
> out all device memory requiring XE KMD to perform a device removal and
> reprobe.
> Calling xe_pci_remove() alone does not free the devres allocated.
> Since there are no exported functions to release all devres, group the
> devres allocations and release the entire group during slot reset to
> ensure proper cleanup.
>
> Cc: Matthew Brost<matthew.brost@intel.com>
> Cc: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com>
> Signed-off-by: Riana Tauro<riana.tauro@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_device.c       | 7 +++++++
>   drivers/gpu/drm/xe/xe_device_types.h | 3 +++
>   drivers/gpu/drm/xe/xe_pci_error.c    | 1 +
>   3 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 16fc6da01357..0cf6480b8aad 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -440,6 +440,7 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
>   				   const struct pci_device_id *ent)
>   {
>   	struct xe_device *xe;
> +	void *devres_id;
>   	int err;
>   
>   	xe_display_driver_set_hooks(&driver);
> @@ -448,10 +449,16 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
>   	if (err)
>   		return ERR_PTR(err);
>   
> +	devres_id = devres_open_group(&pdev->dev, NULL, GFP_KERNEL);
> +	if (!devres_id)
> +		return ERR_PTR(-ENOMEM);
> +
>   	xe = devm_drm_dev_alloc(&pdev->dev, &driver, struct xe_device, drm);
>   	if (IS_ERR(xe))
>   		return xe;
>   
> +	xe->devres_group_id = devres_id;
> +
>   	err = ttm_device_init(&xe->ttm, &xe_ttm_funcs, xe->drm.dev,
>   			      xe->drm.anon_inode->i_mapping,
>   			      xe->drm.vma_offset_manager, 0);
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 2d140463dc5e..3a19e9b5dfae 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -672,6 +672,9 @@ struct xe_device {
>   	/** @in_recovery: Indicates if device is in recovery */
>   	atomic_t in_recovery;
>   
> +	/** @devres_group_id: id for devres group */
> +	void *devres_group_id;
> +
>   	/** @bo_device: Struct to control async free of BOs */
>   	struct xe_bo_dev {
>   		/** @bo_device.async_free: Free worker */
> diff --git a/drivers/gpu/drm/xe/xe_pci_error.c b/drivers/gpu/drm/xe/xe_pci_error.c
> index a3cc01afa179..0960aa5861bc 100644
> --- a/drivers/gpu/drm/xe/xe_pci_error.c
> +++ b/drivers/gpu/drm/xe/xe_pci_error.c
> @@ -65,6 +65,7 @@ static pci_ers_result_t xe_pci_error_slot_reset(struct pci_dev *pdev)
>   	 */
>   	pdev->driver->remove(pdev);
>   	xe_device_clear_in_recovery(xe);
> +	devres_release_group(&pdev->dev, xe->devres_group_id);

We see use after free issue. In pdev->driver->remove(pdev); call xe 
structure is removed. We can handle devres_group_id by assigning locally 
and release it.

Thanks,

-/Mallesh


>   
>   	if (!pdev->driver->probe(pdev, ent))
>   		return PCI_ERS_RESULT_RECOVERED;

[-- Attachment #2: Type: text/html, Size: 4099 bytes --]