Re: [PATCH v4 7/8] drm/xe/madvise: Block imported and exported dma-bufs

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2026-01-30 at 13:52 +0530, Yadav, Arvind wrote:
> 
> On 23-01-2026 19:01, Thomas Hellström wrote:
> > On Tue, 2026-01-20 at 09:51 -0800, Matthew Brost wrote:
> > > On Tue, Jan 20, 2026 at 11:38:53AM +0530, Arvind Yadav wrote:
> > > > Prevent marking imported or exported dma-bufs as purgeable.
> > > > External devices may be accessing these buffers without our
> > > > knowledge, making purging unsafe.
> > > > 
> > > > Check drm_gem_is_imported() for buffers created by other
> > > > drivers and obj->dma_buf for buffers exported to other
> > > > drivers. Silently skip these BOs during madvise processing.
> > > > 
> > > > This follows drm_gem_shmem's purgeable implementation and
> > > > prevents data corruption from purging actively-used shared
> > > > buffers.
> > > > 
> > > > v3:
> > > >     - Addresses review feedback from Matt Roper about handling
> > > >       imported/exported BOs correctly in the purgeable BO
> > > >       implementation.
> > > > 
> > > > v4:
> > > >     - Check should be add to xe_vm_madvise_purgeable_bo.
> > > > 
> > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > @Thomas - couple questions below here I need a 2nd opinion on.
> > > 
> > > > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > > > Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/xe/xe_vm_madvise.c | 33
> > > > ++++++++++++++++++++++++++++++
> > > >   1 file changed, 33 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > b/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > index 27b6ad65b314..5808fef89777 100644
> > > > --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > @@ -180,6 +180,31 @@ static void madvise_pat_index(struct
> > > > xe_device
> > > > *xe, struct xe_vm *vm,
> > > >   	}
> > > >   }
> > > >   
> > > > +/**
> > > > + * xe_bo_is_external_dmabuf() - Check if BO is imported or
> > > > exported dma-buf
> > > > + * @bo: Buffer object
> > > > + *
> > > > + * Prevent marking imported or exported dma-bufs as purgeable.
> > > > + * External devices may be accessing these buffers without our
> > > > + * knowledge, making purging unsafe.
> > > > + *
> > > > + * Return: true if BO is imported or exported, false otherwise
> > > > + */
> > > > +static bool xe_bo_is_external_dmabuf(struct xe_bo *bo)
> > > > +{
> > > @Thomas
> > > 
> > > Should we have this check more generic? e.g., if a BO is not tied
> > > to
> > > a
> > > VM, we don't allow purablity to be set?
> > I think if NEO is going to implement this in one form or another,
> > we
> > need to support also external bos as long as they're not exported,
> > since NEO refuses to use local bos.
> 
> 
> You're right, and I believe the current implementation should support
> that.
> 
> To clarify: "external" here refers only to dma-buf sharing (imported
> or
> exported), not to placement. System-memory BOs that NEO typically
> uses
> remain fully eligible for purging.
> 
> The xe_bo_is_dmabuf_shared() check specifically targets:
>    - Imported dma-bufs (we don't own backing store)
>    - Exported dma-bufs (external devices may have active mappings)
> 
> Regular Xe-owned system BOs can be marked DONTNEED and purged under
> memory
> pressure. We only skip cases where the BO is shared via dma-buf,
> following
> the same pattern as drm_gem_shmem_is_purgeable().
> 
> Please let me know if I've misunderstood the NEO requirements.

No it sounds right. We have
*local BOs, those that Matt refers to as tied to a VM. They are not
sharable.
*external BOs. Those may or may not be exported as dma-bufs. NEO uses
only these, and they need to be purgeable as well, as long as they are
not exported or imported dma-bufs.

Thanks,
Thomas



> 
> 
> > > > +	struct drm_gem_object *obj = &bo->ttm.base;
> > > > +
> > > > +	/* Imported from another driver */
> > > > +	if (drm_gem_is_imported(obj))
> > > > +		return true;
> > > > +
> > > > +	/* Exported to another driver */
> > > > +	if (obj->dma_buf)
> > > > +		return true;
> > > > +
> > > > +	return false;
> > > > +}
> > > > +
> > > >   /**
> > > >    * xe_bo_all_vmas_dontneed() - Check if all VMAs of a BO are
> > > > marked DONTNEED
> > > >    * @bo: Buffer object
> > > > @@ -200,6 +225,10 @@ static bool xe_bo_all_vmas_dontneed(struct
> > > > xe_bo *bo)
> > > >   
> > > >   	dma_resv_assert_held(bo->ttm.base.resv);
> > > >   
> > > > +	/* External dma-bufs cannot be purgeable */
> > > > +	if (xe_bo_is_external_dmabuf(bo))
> > > > +		return false;
> > > > +
> > > >   	drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
> > > >   		drm_gpuvm_bo_for_each_va(gpuva, vm_bo) {
> > > >   			struct xe_vma *vma =
> > > > gpuva_to_vma(gpuva);
> > > > @@ -277,6 +306,10 @@ static bool
> > > > xe_vm_madvise_purgeable_bo(struct
> > > > xe_device *xe, struct xe_vm *vm,
> > > >   		/* BO must be locked before modifying madv
> > > > state
> > > > */
> > > >   		xe_bo_assert_held(bo);
> > > >   
> > > > +		/* Skip external dma-bufs */
> > > > +		if (xe_bo_is_external_dmabuf(bo))
> > > > +			continue;
> > > @Thomas
> > > 
> > > I think instead of silently continuing here we fail the IOCTL
> > > with
> > > -EINVAL?
> > > 
> > > What do you think?
> > Hmm. If we view the export as yet another map, then IMO silently
> > continuing would probably be the best option and consistent with
> > sharing across VMs.
> Noted, I'll keep the silent skip behavior for dma-buf shared BOs to 
> match the "unanimous DONTNEED" model.
> > 
> > Finally I wonder if we should update the purgeable state on final
> > dma-
> > buf attach.
> 
> 
> Good point. Currently the check only happens at madvise time, so
> there's 
> a window where a BO marked DONTNEED could later be exported.
> Would it make sense to hook into the export path to force transition
> to 
> WILLNEED? That would make the "export as implicit mapping" model
> complete,
> though it adds complexity. Let me know your preference.
> 
> > 
> > Floating an idea here:
> > 
> > Could the code be simplified if we maintain a map_count and a
> > purgeable_count on each bo? Bo is purgeable iff map_count ==
> > purgeable_count. vmas and dma-buf attachments are included in
> > map_count.
> 
> 
> That's good idea and would avoid VMA iteration. The tradeoff is 
> maintaining correct counts across all VMA and dma-buf lifecycle
> paths.
> 
> For this series, would you prefer I pursue the counter-based refactor
> now, or is the current iteration approach acceptable
> with counters as a future optimization? I'm happy to go either
> direction 
> based on your feedback.
> 
> 
> Thanks,
> Arvind
> 
> > 
> > Thanks,
> > Thomas
> > 
> > 
> > > Matt
> > > 
> > > > +
> > > >   		/*
> > > >   		 * Once purged, always purged. Cannot
> > > > transition
> > > > back to WILLNEED.
> > > >   		 * This matches i915 semantics where purged
> > > > BOs
> > > > are permanently invalid.
> > > > -- 
> > > > 2.43.0
> > > >