Re: [PATCH v3 3/5] drm/xe/bo: Add a bo remove callback

[Matthew Auld <matthew.auld@intel.com>]

On 25/03/2025 09:07, Thomas Hellström wrote:
> On Tue, 2025-03-25 at 09:02 +0000, Matthew Auld wrote:
>> On 24/03/2025 16:54, Thomas Hellström wrote:
>>> On device unbind, migrate exported bos, including pagemap bos to
>>> system. This allows importers to take proper action without
>>> disruption. In particular, SVM clients on remote devices may
>>> continue as if nothing happened, and can chose a different
>>> placement.
>>>
>>> The evict_flags() placement is chosen in such a way that bos that
>>> aren't exported are purged.
>>>
>>> For pinned bos, we unmap DMA, but their pages are not freed yet
>>> since we can't be 100% sure they are not accessed.
>>>
>>> All pinned external bos (not just the VRAM ones) are put on the
>>> pinned.external list with this patch. But this only affects the
>>> xe_bo_pci_dev_remove_pinned() function since !VRAM bos are
>>> ignored by the suspend / resume functionality. As a follow-up we
>>> could look at removing the suspend / resume iteration over
>>> pinned external bos since we currently don't allow pinning
>>> external bos in VRAM, and other external bos don't need any
>>> special treatment at suspend / resume.
>>>
>>> v2:
>>> - Address review comments. (Matthew Auld).
>>> v3:
>>> - Don't introduce an external_evicted list (Matthew Auld)
>>> - Add a discussion around suspend / resume behaviour to the
>>>     commit message.
>>> - Formatting fixes.
>>>
>>> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>>
>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>>
> 
> Actually, there is a CI failure on LNL indicating that the pinned
> kernel-bo dma-maps are actually needed at devm-managed release.

Hmm, do you have a link? The failure I see looks to be more probe 
related? Once we do unplug(), outside the special evict all we do here 
we should pretty much not need dma-maps?

What about moving the evict_all into a well placed devm action during 
probe? Basically at the point at which we think it is reasonable to get 
rid of the dma-maps? Or is that what you mean below?

> 
> I'm in the process on testing this out on LNL, and if so I'll drop
> these dma-unmaps and we'd continue down the route of ensuring that
> these subsystems are indeed devm_ managed and not drmm_ managed.
> 
> Thanks,
> Thomas
> 
> 
>