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

[Riana Tauro <riana.tauro@intel.com>]

On 1/27/2026 4:53 PM, Mallesh, Koujalagi wrote:
> 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.


No xe is not removed in xe_remove.  xe is also a dev_res.
If you see here in device_release. Remove is first and then the device 
resources are cleared

https://elixir.bootlin.com/linux/v6.18.6/source/drivers/base/dd.c#L1243

Ideally using devres_release_all(dev); would be better but this is not 
exported.

Grouping is the only way i could think of. Any other suggestions are 
welcome.

Thanks
Riana


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