| Message ID | 20250320172956.168358-15-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Replace xe_hmm with gpusvm | expand |
On Thu, Mar 20, 2025 at 05:30:03PM +0000, Matthew Auld wrote: > Idea is to use this for userptr, where we mostly just need > get/unmap/free pages, plus some kind of common notifier lock at the svm > level (provided by gpusvm). The range itself also maps to a single > notifier, covering the entire range (provided by the user). > So, same comment as in patch #7 [1]: could we drop the idea of a basic GPUSVM and unify SVM and userptr GPUSVM to share the locking? Following that, rather than having wrappers in GPU SVM for drm_gpusvm_basic_range, can we expose the lower-level GPU SVM functions that operate on pages and have wrappers call these in the Xe layer? Again, adding +Himal and Thomas for their opinions. Matt [1] https://patchwork.freedesktop.org/patch/643976/?series=146553&rev=1#comment_1177820 > TODO: needs proper kernel-doc, assuming this change makes sense. > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/drm_gpusvm.c | 138 +++++++++++++++++++++++++++++------ > include/drm/drm_gpusvm.h | 23 ++++++ > 2 files changed, 137 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c > index 2beca5a6dc0a..ca571610214c 100644 > --- a/drivers/gpu/drm/drm_gpusvm.c > +++ b/drivers/gpu/drm/drm_gpusvm.c > @@ -521,6 +521,41 @@ static const struct mmu_interval_notifier_ops drm_gpusvm_notifier_ops = { > .invalidate = drm_gpusvm_notifier_invalidate, > }; > > +static void __drm_gpusvm_init(struct drm_gpusvm *gpusvm, const char *name, > + struct drm_device *drm, struct mm_struct *mm, > + void *device_private_page_owner, > + unsigned long mm_start, unsigned long mm_range, > + unsigned long notifier_size, > + const struct drm_gpusvm_ops *ops, > + const unsigned long *chunk_sizes, int num_chunks) > +{ > + gpusvm->name = name; > + gpusvm->drm = drm; > + gpusvm->mm = mm; > + gpusvm->device_private_page_owner = device_private_page_owner; > + gpusvm->mm_start = mm_start; > + gpusvm->mm_range = mm_range; > + gpusvm->notifier_size = notifier_size; > + gpusvm->ops = ops; > + gpusvm->chunk_sizes = chunk_sizes; > + gpusvm->num_chunks = num_chunks; > + > + if (mm) > + mmgrab(mm); > + gpusvm->root = RB_ROOT_CACHED; > + INIT_LIST_HEAD(&gpusvm->notifier_list); > + > + init_rwsem(&gpusvm->notifier_lock); > + > + fs_reclaim_acquire(GFP_KERNEL); > + might_lock(&gpusvm->notifier_lock); > + fs_reclaim_release(GFP_KERNEL); > + > +#ifdef CONFIG_LOCKDEP > + gpusvm->lock_dep_map = NULL; > +#endif > +} > + > /** > * drm_gpusvm_init() - Initialize the GPU SVM. > * @gpusvm: Pointer to the GPU SVM structure. > @@ -552,35 +587,32 @@ int drm_gpusvm_init(struct drm_gpusvm *gpusvm, > if (!ops->invalidate || !num_chunks) > return -EINVAL; > > - gpusvm->name = name; > - gpusvm->drm = drm; > - gpusvm->mm = mm; > - gpusvm->device_private_page_owner = device_private_page_owner; > - gpusvm->mm_start = mm_start; > - gpusvm->mm_range = mm_range; > - gpusvm->notifier_size = notifier_size; > - gpusvm->ops = ops; > - gpusvm->chunk_sizes = chunk_sizes; > - gpusvm->num_chunks = num_chunks; > - > - mmgrab(mm); > - gpusvm->root = RB_ROOT_CACHED; > - INIT_LIST_HEAD(&gpusvm->notifier_list); > - > - init_rwsem(&gpusvm->notifier_lock); > - > - fs_reclaim_acquire(GFP_KERNEL); > - might_lock(&gpusvm->notifier_lock); > - fs_reclaim_release(GFP_KERNEL); > - > -#ifdef CONFIG_LOCKDEP > - gpusvm->lock_dep_map = NULL; > -#endif > + __drm_gpusvm_init(gpusvm, name, drm, mm, device_private_page_owner, > + mm_start, mm_range, notifier_size, ops, chunk_sizes, > + num_chunks); > > return 0; > } > EXPORT_SYMBOL_GPL(drm_gpusvm_init); > > +static bool drm_gpusvm_is_basic(struct drm_gpusvm *svm) > +{ > + return !svm->mm; > +} > + > +void drm_gpusvm_basic_init(struct drm_gpusvm *gpusvm, const char *name, > + struct drm_device *drm) > +{ > + __drm_gpusvm_init(gpusvm, name, drm, NULL, NULL, 0, 0, 0, NULL, NULL, > + 0); > +} > +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_init); > + > +void drm_gpusvm_basic_fini(struct drm_gpusvm *gpusvm) > +{ > +} > +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_fini); > + > /** > * drm_gpusvm_notifier_find() - Find GPU SVM notifier > * @gpusvm: Pointer to the GPU SVM structure > @@ -1001,6 +1033,27 @@ static void drm_gpusvm_driver_lock_held(struct drm_gpusvm *gpusvm) > } > #endif > > +void drm_gpusvm_basic_range_init(struct drm_gpusvm *svm, > + struct drm_gpusvm_basic_range *range, > + struct mmu_interval_notifier *notifier, > + unsigned long *notifier_seq) > +{ > + WARN_ON(!drm_gpusvm_is_basic(svm)); > + > + range->gpusvm = svm; > + range->notifier = notifier; > + range->notifier_seq = notifier_seq; > + *notifier_seq = LONG_MAX; > + memset(&range->pages, 0, sizeof(range->pages)); > +} > +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_init); > + > +void drm_gpusvm_basic_range_fini(struct drm_gpusvm_basic_range *range) > +{ > + WARN_ON(range->pages.flags.has_dma_mapping); > +} > +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_fini); > + > /** > * drm_gpusvm_range_find_or_insert() - Find or insert GPU SVM range > * @gpusvm: Pointer to the GPU SVM structure > @@ -1176,6 +1229,19 @@ static void drm_gpusvm_free_pages(struct drm_gpusvm *gpusvm, > } > } > > +void drm_gpusvm_basic_range_free_pages(struct drm_gpusvm_basic_range *range) > +{ > + unsigned long npages = > + npages_in_range(range->notifier->interval_tree.start, > + range->notifier->interval_tree.last + 1); > + > + drm_gpusvm_notifier_lock(range->gpusvm); > + __drm_gpusvm_unmap_pages(range->gpusvm, &range->pages, npages); > + drm_gpusvm_free_pages(range->gpusvm, &range->pages); > + drm_gpusvm_notifier_unlock(range->gpusvm); > +} > +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_free_pages); > + > /** > * drm_gpusvm_range_remove() - Remove GPU SVM range > * @gpusvm: Pointer to the GPU SVM structure > @@ -1545,6 +1611,20 @@ int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm, > } > EXPORT_SYMBOL_GPL(drm_gpusvm_range_get_pages); > > +int drm_gpusvm_basic_range_get_pages(struct drm_gpusvm_basic_range *range, > + const struct drm_gpusvm_ctx *ctx) > +{ > + drm_gpusvm_driver_lock_held(range->gpusvm); > + > + return drm_gpusvm_get_pages(range->gpusvm, &range->pages, > + range->notifier->mm, range->notifier, > + range->notifier_seq, > + range->notifier->interval_tree.start, > + range->notifier->interval_tree.last + 1, > + ctx); > +} > +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_get_pages); > + > static void drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm, > unsigned long mm_start, unsigned long mm_end, > struct drm_gpusvm_pages *svm_pages, > @@ -1585,6 +1665,16 @@ void drm_gpusvm_range_unmap_pages(struct drm_gpusvm *gpusvm, > } > EXPORT_SYMBOL_GPL(drm_gpusvm_range_unmap_pages); > > +void drm_gpusvm_basic_range_unmap_pages(struct drm_gpusvm_basic_range *range, > + const struct drm_gpusvm_ctx *ctx) > +{ > + drm_gpusvm_unmap_pages(range->gpusvm, > + range->notifier->interval_tree.start, > + range->notifier->interval_tree.last + 1, > + &range->pages, ctx); > +} > +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_unmap_pages); > + > /** > * drm_gpusvm_migration_unlock_put_page() - Put a migration page > * @page: Pointer to the page to put > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h > index c15c733ef0ad..82c4e58fa84c 100644 > --- a/include/drm/drm_gpusvm.h > +++ b/include/drm/drm_gpusvm.h > @@ -305,6 +305,29 @@ struct drm_gpusvm_ctx { > unsigned int devmem_possible :1; > }; > > +struct drm_gpusvm_basic_range { > + struct drm_gpusvm *gpusvm; > + struct drm_gpusvm_pages pages; > + struct mmu_interval_notifier *notifier; > + unsigned long *notifier_seq; > +}; > + > +void drm_gpusvm_basic_init(struct drm_gpusvm *gpusvm, const char *name, > + struct drm_device *drm); > +void drm_gpusvm_basic_fini(struct drm_gpusvm *gpusvm); > + > +void drm_gpusvm_basic_range_init(struct drm_gpusvm *svm, > + struct drm_gpusvm_basic_range *range, > + struct mmu_interval_notifier *notifier, > + unsigned long *notifier_seq); > +void drm_gpusvm_basic_range_fini(struct drm_gpusvm_basic_range *range); > + > +int drm_gpusvm_basic_range_get_pages(struct drm_gpusvm_basic_range *range, > + const struct drm_gpusvm_ctx *ctx); > +void drm_gpusvm_basic_range_unmap_pages(struct drm_gpusvm_basic_range *range, > + const struct drm_gpusvm_ctx *ctx); > +void drm_gpusvm_basic_range_free_pages(struct drm_gpusvm_basic_range *range); > + > int drm_gpusvm_init(struct drm_gpusvm *gpusvm, > const char *name, struct drm_device *drm, > struct mm_struct *mm, void *device_private_page_owner, > -- > 2.48.1 >
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c index 2beca5a6dc0a..ca571610214c 100644 --- a/drivers/gpu/drm/drm_gpusvm.c +++ b/drivers/gpu/drm/drm_gpusvm.c @@ -521,6 +521,41 @@ static const struct mmu_interval_notifier_ops drm_gpusvm_notifier_ops = { .invalidate = drm_gpusvm_notifier_invalidate, }; +static void __drm_gpusvm_init(struct drm_gpusvm *gpusvm, const char *name, + struct drm_device *drm, struct mm_struct *mm, + void *device_private_page_owner, + unsigned long mm_start, unsigned long mm_range, + unsigned long notifier_size, + const struct drm_gpusvm_ops *ops, + const unsigned long *chunk_sizes, int num_chunks) +{ + gpusvm->name = name; + gpusvm->drm = drm; + gpusvm->mm = mm; + gpusvm->device_private_page_owner = device_private_page_owner; + gpusvm->mm_start = mm_start; + gpusvm->mm_range = mm_range; + gpusvm->notifier_size = notifier_size; + gpusvm->ops = ops; + gpusvm->chunk_sizes = chunk_sizes; + gpusvm->num_chunks = num_chunks; + + if (mm) + mmgrab(mm); + gpusvm->root = RB_ROOT_CACHED; + INIT_LIST_HEAD(&gpusvm->notifier_list); + + init_rwsem(&gpusvm->notifier_lock); + + fs_reclaim_acquire(GFP_KERNEL); + might_lock(&gpusvm->notifier_lock); + fs_reclaim_release(GFP_KERNEL); + +#ifdef CONFIG_LOCKDEP + gpusvm->lock_dep_map = NULL; +#endif +} + /** * drm_gpusvm_init() - Initialize the GPU SVM. * @gpusvm: Pointer to the GPU SVM structure. @@ -552,35 +587,32 @@ int drm_gpusvm_init(struct drm_gpusvm *gpusvm, if (!ops->invalidate || !num_chunks) return -EINVAL; - gpusvm->name = name; - gpusvm->drm = drm; - gpusvm->mm = mm; - gpusvm->device_private_page_owner = device_private_page_owner; - gpusvm->mm_start = mm_start; - gpusvm->mm_range = mm_range; - gpusvm->notifier_size = notifier_size; - gpusvm->ops = ops; - gpusvm->chunk_sizes = chunk_sizes; - gpusvm->num_chunks = num_chunks; - - mmgrab(mm); - gpusvm->root = RB_ROOT_CACHED; - INIT_LIST_HEAD(&gpusvm->notifier_list); - - init_rwsem(&gpusvm->notifier_lock); - - fs_reclaim_acquire(GFP_KERNEL); - might_lock(&gpusvm->notifier_lock); - fs_reclaim_release(GFP_KERNEL); - -#ifdef CONFIG_LOCKDEP - gpusvm->lock_dep_map = NULL; -#endif + __drm_gpusvm_init(gpusvm, name, drm, mm, device_private_page_owner, + mm_start, mm_range, notifier_size, ops, chunk_sizes, + num_chunks); return 0; } EXPORT_SYMBOL_GPL(drm_gpusvm_init); +static bool drm_gpusvm_is_basic(struct drm_gpusvm *svm) +{ + return !svm->mm; +} + +void drm_gpusvm_basic_init(struct drm_gpusvm *gpusvm, const char *name, + struct drm_device *drm) +{ + __drm_gpusvm_init(gpusvm, name, drm, NULL, NULL, 0, 0, 0, NULL, NULL, + 0); +} +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_init); + +void drm_gpusvm_basic_fini(struct drm_gpusvm *gpusvm) +{ +} +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_fini); + /** * drm_gpusvm_notifier_find() - Find GPU SVM notifier * @gpusvm: Pointer to the GPU SVM structure @@ -1001,6 +1033,27 @@ static void drm_gpusvm_driver_lock_held(struct drm_gpusvm *gpusvm) } #endif +void drm_gpusvm_basic_range_init(struct drm_gpusvm *svm, + struct drm_gpusvm_basic_range *range, + struct mmu_interval_notifier *notifier, + unsigned long *notifier_seq) +{ + WARN_ON(!drm_gpusvm_is_basic(svm)); + + range->gpusvm = svm; + range->notifier = notifier; + range->notifier_seq = notifier_seq; + *notifier_seq = LONG_MAX; + memset(&range->pages, 0, sizeof(range->pages)); +} +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_init); + +void drm_gpusvm_basic_range_fini(struct drm_gpusvm_basic_range *range) +{ + WARN_ON(range->pages.flags.has_dma_mapping); +} +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_fini); + /** * drm_gpusvm_range_find_or_insert() - Find or insert GPU SVM range * @gpusvm: Pointer to the GPU SVM structure @@ -1176,6 +1229,19 @@ static void drm_gpusvm_free_pages(struct drm_gpusvm *gpusvm, } } +void drm_gpusvm_basic_range_free_pages(struct drm_gpusvm_basic_range *range) +{ + unsigned long npages = + npages_in_range(range->notifier->interval_tree.start, + range->notifier->interval_tree.last + 1); + + drm_gpusvm_notifier_lock(range->gpusvm); + __drm_gpusvm_unmap_pages(range->gpusvm, &range->pages, npages); + drm_gpusvm_free_pages(range->gpusvm, &range->pages); + drm_gpusvm_notifier_unlock(range->gpusvm); +} +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_free_pages); + /** * drm_gpusvm_range_remove() - Remove GPU SVM range * @gpusvm: Pointer to the GPU SVM structure @@ -1545,6 +1611,20 @@ int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm, } EXPORT_SYMBOL_GPL(drm_gpusvm_range_get_pages); +int drm_gpusvm_basic_range_get_pages(struct drm_gpusvm_basic_range *range, + const struct drm_gpusvm_ctx *ctx) +{ + drm_gpusvm_driver_lock_held(range->gpusvm); + + return drm_gpusvm_get_pages(range->gpusvm, &range->pages, + range->notifier->mm, range->notifier, + range->notifier_seq, + range->notifier->interval_tree.start, + range->notifier->interval_tree.last + 1, + ctx); +} +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_get_pages); + static void drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm, unsigned long mm_start, unsigned long mm_end, struct drm_gpusvm_pages *svm_pages, @@ -1585,6 +1665,16 @@ void drm_gpusvm_range_unmap_pages(struct drm_gpusvm *gpusvm, } EXPORT_SYMBOL_GPL(drm_gpusvm_range_unmap_pages); +void drm_gpusvm_basic_range_unmap_pages(struct drm_gpusvm_basic_range *range, + const struct drm_gpusvm_ctx *ctx) +{ + drm_gpusvm_unmap_pages(range->gpusvm, + range->notifier->interval_tree.start, + range->notifier->interval_tree.last + 1, + &range->pages, ctx); +} +EXPORT_SYMBOL_GPL(drm_gpusvm_basic_range_unmap_pages); + /** * drm_gpusvm_migration_unlock_put_page() - Put a migration page * @page: Pointer to the page to put diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h index c15c733ef0ad..82c4e58fa84c 100644 --- a/include/drm/drm_gpusvm.h +++ b/include/drm/drm_gpusvm.h @@ -305,6 +305,29 @@ struct drm_gpusvm_ctx { unsigned int devmem_possible :1; }; +struct drm_gpusvm_basic_range { + struct drm_gpusvm *gpusvm; + struct drm_gpusvm_pages pages; + struct mmu_interval_notifier *notifier; + unsigned long *notifier_seq; +}; + +void drm_gpusvm_basic_init(struct drm_gpusvm *gpusvm, const char *name, + struct drm_device *drm); +void drm_gpusvm_basic_fini(struct drm_gpusvm *gpusvm); + +void drm_gpusvm_basic_range_init(struct drm_gpusvm *svm, + struct drm_gpusvm_basic_range *range, + struct mmu_interval_notifier *notifier, + unsigned long *notifier_seq); +void drm_gpusvm_basic_range_fini(struct drm_gpusvm_basic_range *range); + +int drm_gpusvm_basic_range_get_pages(struct drm_gpusvm_basic_range *range, + const struct drm_gpusvm_ctx *ctx); +void drm_gpusvm_basic_range_unmap_pages(struct drm_gpusvm_basic_range *range, + const struct drm_gpusvm_ctx *ctx); +void drm_gpusvm_basic_range_free_pages(struct drm_gpusvm_basic_range *range); + int drm_gpusvm_init(struct drm_gpusvm *gpusvm, const char *name, struct drm_device *drm, struct mm_struct *mm, void *device_private_page_owner,
Idea is to use this for userptr, where we mostly just need get/unmap/free pages, plus some kind of common notifier lock at the svm level (provided by gpusvm). The range itself also maps to a single notifier, covering the entire range (provided by the user). TODO: needs proper kernel-doc, assuming this change makes sense. Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Matthew Brost <matthew.brost@intel.com> --- drivers/gpu/drm/drm_gpusvm.c | 138 +++++++++++++++++++++++++++++------ include/drm/drm_gpusvm.h | 23 ++++++ 2 files changed, 137 insertions(+), 24 deletions(-)