On Thu, Jun 18, 2026 at 04:09:00PM +0800, Honglei Huang wrote: > From: Honglei Huang <[email protected]> > > With drm_gpusvm_pages now self contained, make xe stop relying > on the drm_gpusvm_range pages and take responsibility for the page > lifecycle on the driver side. > > Driver side (xe): > > - Embed struct drm_gpusvm_pages in xe_svm_range and route all > xe accesses through it instead of range->base.pages. > - Initialise the embedded pages via drm_gpusvm_init_pages(), which > binds the owning &xe->drm up front, and take over the page > lifecycle: xe_svm_range_get_pages() calls drm_gpusvm_get_pages() > directly; the notifier event_end and xe_svm_range_free() paths > drive unmap/free on the embedded pages object. > - Convert the open-coded userptr pages init in xe_userptr_setup() > to the same drm_gpusvm_init_pages() helper. > - Switch xe_svm_range_pages_valid() to drm_gpusvm_pages_valid(). > > Framework side (drm_gpusvm): > > - Add a small inline drm_gpusvm_init_pages() helper that records the > owning drm_device and initialises the per-pages state, giving > drivers a single hook to extend. > - Export drm_gpusvm_pages_valid() to let driver owned pages > can query mapping state without going through a range. > - Lifecycle change: drm_gpusvm_range_remove() no longer *triggers* > unmap/free of the embedded pages. The unmap/free logic itself stays > in the framework -- drm_gpusvm_free_pages() still performs the DMA > unmap (as an idempotent backstop) and frees the dma_addr array -- > but the driver now owns *when* it runs, since the driver owns the > drm_gpusvm_pages object. > > Side effect / contract: a driver that owns a drm_gpusvm_pages is now > responsible for its lifecycle: drm_gpusvm_init_pages() before first > use, and drm_gpusvm_free_pages() when the owner goes away. Xe does the > latter from its ops->range_free callback, which the framework invokes > once the range refcount drops to zero in drm_gpusvm_range_remove(). > The timely DMA unmap for the IOMMU security model still happens in the > notifier invalidate path via drm_gpusvm_unmap_pages(); the unmap inside > drm_gpusvm_free_pages() is only a backstop for pages that were never > invalidated. > > Suggested-by: Matthew Brost <[email protected]>
Reviewed-by: Matthew Brost <[email protected]> > Signed-off-by: Honglei Huang <[email protected]> > --- > drivers/gpu/drm/drm_gpusvm.c | 14 ++++++++------ > drivers/gpu/drm/xe/xe_pt.c | 2 +- > drivers/gpu/drm/xe/xe_svm.c | 24 ++++++++++++++++-------- > drivers/gpu/drm/xe/xe_svm.h | 6 ++++-- > drivers/gpu/drm/xe/xe_userptr.c | 5 ++--- > include/drm/drm_gpusvm.h | 19 +++++++++++++++++++ > 6 files changed, 50 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c > index 155e3c2e703..85fbadc9716 100644 > --- a/drivers/gpu/drm/drm_gpusvm.c > +++ b/drivers/gpu/drm/drm_gpusvm.c > @@ -1228,12 +1228,15 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_free_pages); > * This function removes the specified GPU SVM range and also removes the > parent > * GPU SVM notifier if no more ranges remain in the notifier. The caller must > * hold a lock to protect range and notifier removal. > + * > + * This function does not unmap or free the drm_gpusvm_pages; the driver owns > + * that lifecycle and is expected to release them from its > + * &drm_gpusvm_ops.range_free callback (invoked once the range refcount drops > + * to zero via drm_gpusvm_range_put() below). > */ > void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm, > struct drm_gpusvm_range *range) > { > - unsigned long npages = npages_in_range(drm_gpusvm_range_start(range), > - drm_gpusvm_range_end(range)); > struct drm_gpusvm_notifier *notifier; > > drm_gpusvm_driver_lock_held(gpusvm); > @@ -1245,8 +1248,6 @@ void drm_gpusvm_range_remove(struct drm_gpusvm *gpusvm, > return; > > drm_gpusvm_notifier_lock(gpusvm); > - __drm_gpusvm_unmap_pages(gpusvm, &range->pages, npages); > - __drm_gpusvm_free_pages(gpusvm, &range->pages); > __drm_gpusvm_range_remove(notifier, range); > drm_gpusvm_notifier_unlock(gpusvm); > > @@ -1325,13 +1326,14 @@ EXPORT_SYMBOL_GPL(drm_gpusvm_range_put); > * > * Return: True if GPU SVM range has valid pages, False otherwise > */ > -static bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm, > - struct drm_gpusvm_pages *svm_pages) > +bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm, > + struct drm_gpusvm_pages *svm_pages) > { > lockdep_assert_held(&gpusvm->notifier_lock); > > return svm_pages->flags.has_devmem_pages || > svm_pages->flags.has_dma_mapping; > } > +EXPORT_SYMBOL_GPL(drm_gpusvm_pages_valid); > > /** > * drm_gpusvm_range_pages_valid() - GPU SVM range pages valid > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c > index 0959e0e88a1..4a8af0e934c 100644 > --- a/drivers/gpu/drm/xe/xe_pt.c > +++ b/drivers/gpu/drm/xe/xe_pt.c > @@ -758,7 +758,7 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma, > return -EAGAIN; > } > if (xe_svm_range_has_dma_mapping(range)) { > - xe_res_first_dma(range->base.pages.dma_addr, 0, > + xe_res_first_dma(range->pages.dma_addr, 0, > xe_svm_range_size(range), > &curs); > xe_svm_range_debug(range, "BIND PREPARE - MIXED"); > diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c > index 63da149f3b7..77af0a8de63 100644 > --- a/drivers/gpu/drm/xe/xe_svm.c > +++ b/drivers/gpu/drm/xe/xe_svm.c > @@ -66,7 +66,7 @@ static bool xe_svm_range_in_vram(struct xe_svm_range *range) > > struct drm_gpusvm_pages_flags flags = { > /* Pairs with WRITE_ONCE in drm_gpusvm.c */ > - .__flags = READ_ONCE(range->base.pages.flags.__flags), > + .__flags = READ_ONCE(range->pages.flags.__flags), > }; > > return flags.has_devmem_pages; > @@ -96,7 +96,7 @@ static struct xe_vm *range_to_vm(struct drm_gpusvm_range *r) > (r__)->base.gpusvm, \ > xe_svm_range_in_vram((r__)) ? 1 : 0, \ > xe_svm_range_has_vram_binding((r__)) ? 1 : 0, \ > - (r__)->base.pages.notifier_seq, \ > + (r__)->pages.notifier_seq, \ > xe_svm_range_start((r__)), xe_svm_range_end((r__)), \ > xe_svm_range_size((r__))) > > @@ -115,6 +115,7 @@ xe_svm_range_alloc(struct drm_gpusvm *gpusvm) > return NULL; > > INIT_LIST_HEAD(&range->garbage_collector_link); > + drm_gpusvm_init_pages(&range->pages, &gpusvm_to_vm(gpusvm)->xe->drm); > xe_vm_get(gpusvm_to_vm(gpusvm)); > > return &range->base; > @@ -122,8 +123,10 @@ xe_svm_range_alloc(struct drm_gpusvm *gpusvm) > > static void xe_svm_range_free(struct drm_gpusvm_range *range) > { > + drm_gpusvm_free_pages(range->gpusvm, &(to_xe_range(range)->pages), > + drm_gpusvm_range_size(range) >> PAGE_SHIFT); > xe_vm_put(range_to_vm(range)); > - kfree(range); > + kfree(to_xe_range(range)); > } > > static void > @@ -134,7 +137,7 @@ xe_svm_garbage_collector_add_range(struct xe_vm *vm, > struct xe_svm_range *range, > > range_debug(range, "GARBAGE COLLECTOR ADD"); > > - drm_gpusvm_range_set_unmapped(&range->base, &range->base.pages, 1, > + drm_gpusvm_range_set_unmapped(&range->base, &range->pages, 1, > mmu_range); > > spin_lock(&vm->svm.garbage_collector.lock); > @@ -209,7 +212,8 @@ xe_svm_range_notifier_event_end(struct xe_vm *vm, struct > drm_gpusvm_range *r, > > xe_svm_assert_in_notifier(vm); > > - drm_gpusvm_range_unmap_pages(&vm->svm.gpusvm, r, &ctx); > + drm_gpusvm_unmap_pages(&vm->svm.gpusvm, &(to_xe_range(r)->pages), > + drm_gpusvm_range_size(r) >> PAGE_SHIFT, &ctx); > if (!xe_vm_is_closed(vm) && mmu_range->event == MMU_NOTIFY_UNMAP) > xe_svm_garbage_collector_add_range(vm, to_xe_range(r), > mmu_range); > @@ -953,7 +957,7 @@ void xe_svm_fini(struct xe_vm *vm) > static bool xe_svm_range_has_pagemap_locked(const struct xe_svm_range *range, > const struct drm_pagemap *dpagemap) > { > - return range->base.pages.dpagemap == dpagemap; > + return range->pages.dpagemap == dpagemap; > } > > static bool xe_svm_range_has_pagemap(struct xe_svm_range *range, > @@ -1018,7 +1022,7 @@ bool xe_svm_range_validate(struct xe_vm *vm, > if (dpagemap) > ret = ret && xe_svm_range_has_pagemap_locked(range, dpagemap); > else > - ret = ret && !range->base.pages.dpagemap; > + ret = ret && !range->pages.dpagemap; > > xe_svm_notifier_unlock(vm); > > @@ -1508,7 +1512,11 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct > xe_svm_range *range, > { > int err = 0; > > - err = drm_gpusvm_range_get_pages(&vm->svm.gpusvm, &range->base, ctx); > + err = drm_gpusvm_get_pages(&vm->svm.gpusvm, &range->pages, > + vm->svm.gpusvm.mm, > + &range->base.notifier->notifier, > + drm_gpusvm_range_start(&range->base), > + drm_gpusvm_range_end(&range->base), ctx); > if (err == -EOPNOTSUPP) { > range_debug(range, "PAGE FAULT - EVICT PAGES"); > drm_gpusvm_range_evict(&vm->svm.gpusvm, &range->base); > diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h > index b7b8eeacf19..1423ab2f1d6 100644 > --- a/drivers/gpu/drm/xe/xe_svm.h > +++ b/drivers/gpu/drm/xe/xe_svm.h > @@ -31,6 +31,8 @@ struct xe_vram_region; > struct xe_svm_range { > /** @base: base drm_gpusvm_range */ > struct drm_gpusvm_range base; > + /** @pages: Page/DMA mapping state for this range (single drm_device). > */ > + struct drm_gpusvm_pages pages; > /** > * @garbage_collector_link: Link into VM's garbage collect SVM range > * list. Protected by VM's garbage collect lock. > @@ -74,7 +76,7 @@ struct xe_pagemap { > */ > static inline bool xe_svm_range_pages_valid(struct xe_svm_range *range) > { > - return drm_gpusvm_range_pages_valid(range->base.gpusvm, &range->base); > + return drm_gpusvm_pages_valid(range->base.gpusvm, &range->pages); > } > > int xe_devm_add(struct xe_tile *tile, struct xe_vram_region *vr); > @@ -132,7 +134,7 @@ void *xe_svm_private_page_owner(struct xe_vm *vm, bool > force_smem); > static inline bool xe_svm_range_has_dma_mapping(struct xe_svm_range *range) > { > lockdep_assert_held(&range->base.gpusvm->notifier_lock); > - return range->base.pages.flags.has_dma_mapping; > + return range->pages.flags.has_dma_mapping; > } > > /** > diff --git a/drivers/gpu/drm/xe/xe_userptr.c b/drivers/gpu/drm/xe/xe_userptr.c > index 1b540e62af6..06da9725a4a 100644 > --- a/drivers/gpu/drm/xe/xe_userptr.c > +++ b/drivers/gpu/drm/xe/xe_userptr.c > @@ -396,15 +396,14 @@ int xe_userptr_setup(struct xe_userptr_vma *uvma, > unsigned long start, > INIT_LIST_HEAD(&userptr->invalidate_link); > INIT_LIST_HEAD(&userptr->repin_link); > > + drm_gpusvm_init_pages(&userptr->pages, &vm->xe->drm); > + > err = mmu_interval_notifier_insert(&userptr->notifier, current->mm, > start, range, > &vma_userptr_notifier_ops); > if (err) > return err; > > - userptr->pages.notifier_seq = LONG_MAX; > - userptr->pages.drm = &vm->xe->drm; > - > return 0; > } > > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h > index 842353afb27..e32d3bcb47b 100644 > --- a/include/drm/drm_gpusvm.h > +++ b/include/drm/drm_gpusvm.h > @@ -310,6 +310,9 @@ void drm_gpusvm_range_put(struct drm_gpusvm_range *range); > bool drm_gpusvm_range_pages_valid(struct drm_gpusvm *gpusvm, > struct drm_gpusvm_range *range); > > +bool drm_gpusvm_pages_valid(struct drm_gpusvm *gpusvm, > + struct drm_gpusvm_pages *svm_pages); > + > int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm, > struct drm_gpusvm_range *range, > const struct drm_gpusvm_ctx *ctx); > @@ -350,6 +353,22 @@ void drm_gpusvm_free_pages(struct drm_gpusvm *gpusvm, > struct drm_gpusvm_pages *svm_pages, > unsigned long npages); > > +/** > + * drm_gpusvm_init_pages() - Initialize a freshly allocated drm_gpusvm_pages > + * @svm_pages: Pointer to the drm_gpusvm_pages to initialize. > + * @drm: The DRM device that will own DMA mappings for this pages object. > + * > + * Drivers that embed one or more drm_gpusvm_pages in their own range > + * structure must call this once on each pages instance after allocation, > + * before the first drm_gpusvm_get_pages() / unmap / free. > + */ > +static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages, > + struct drm_device *drm) > +{ > + svm_pages->drm = drm; > + svm_pages->notifier_seq = LONG_MAX; > +} > + > /** > * enum drm_gpusvm_scan_result - Scan result from the drm_gpusvm_scan_mm() > function. > * @DRM_GPUSVM_SCAN_UNPOPULATED: At least one page was not present or > inaccessible. > -- > 2.34.1 >
