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]>
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 | 22 +++++++++++++++-------
drivers/gpu/drm/xe/xe_svm.h | 6 ++++--
drivers/gpu/drm/xe/xe_userptr.c | 3 +--
include/drm/drm_gpusvm.h | 19 +++++++++++++++++++
6 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index d6407c94cdd..7af535853c3 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 2669ff5ee74..e82b0d8fab1 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 05578e187d2..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
@@ -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..33ae39ac856 100644
--- a/drivers/gpu/drm/xe/xe_userptr.c
+++ b/drivers/gpu/drm/xe/xe_userptr.c
@@ -402,8 +402,7 @@ int xe_userptr_setup(struct xe_userptr_vma *uvma, unsigned
long start,
if (err)
return err;
- userptr->pages.notifier_seq = LONG_MAX;
- userptr->pages.drm = &vm->xe->drm;
+ drm_gpusvm_init_pages(&userptr->pages, &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
