Dynamic drm_pagemaps and Initial multi-device SVM

In situations where no system memory is migrated to devmem, and in upcoming patches where another GPU is performing the migration to the newly allocated devmem buffer, there is nothing to ensure any ongoing clear to the devmem allocation or async eviction from the devmem allocation is complete.

Address that by passing a struct dma_fence down to the copy functions, and ensure it is waited for before migration is marked complete.

v3:
- New patch.
v4:
- Update the logic used for determining when to wait for the pre_migrate_fence.
- Update the logic used for determining when to warn for the pre_migrate_fence since the scheduler fences apparently can signal out-of-order.

Fixes c5b3eb5a906c ("drm/xe: Add GPUSVM device memory copy vfunc functions")

Cc Matthew Brost <[email protected]>

Cc <[email protected]> # v6.15+

Signed-off-by Thomas Hellström <[email protected]>

---
  drivers/gpu/drm/drm_pagemap.c | 13 ++++---
  drivers/gpu/drm/xe/xe_svm.c   | 67 ++++++++++++++++++++++++++++++-----
  include/drm/drm_pagemap.h     | 17 +++++++--
  3 files changed, 81 insertions(+), 16 deletions(-)

drivers/gpu/drm/drm_pagemap.c +9 -4

index 22c44807e3fe..864a73d019ed 100644
@@ -408,7 +408,8 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
  		drm_pagemap_get_devmem_page(page, zdd);
  	}
 
-	err = ops->copy_to_devmem(pages, pagemap_addr, npages);
+	err = ops->copy_to_devmem(pages, pagemap_addr, npages,
+				  devmem_allocation->pre_migrate_fence);
  	if (err)
  		goto err_finalize;
 

@@ -596,7 +597,7 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation)
  	for (i = 0; i < npages; ++i)
  		pages[i] = migrate_pfn_to_page(src[i]);
 
-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);
  	if (err)
  		goto err_finalize;
 

@@ -732,7 +733,7 @@ static int __drm_pagemap_migrate_to_ram(struct vm_area_struct *vas,
  	for (i = 0; i < npages; ++i)
  		pages[i] = migrate_pfn_to_page(migrate.src[i]);
 
-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);
  	if (err)
  		goto err_finalize;
 

@@ -813,11 +814,14 @@ EXPORT_SYMBOL_GPL(drm_pagemap_pagemap_ops_get);
   * @ops: Pointer to the operations structure for GPU SVM device memory
   * @dpagemap: The struct drm_pagemap we're allocating from.
   * @size: Size of device memory allocation
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
  			     struct device *dev, struct mm_struct *mm,
  			     const struct drm_pagemap_devmem_ops *ops,
-			     struct drm_pagemap *dpagemap, size_t size)
+			     struct drm_pagemap *dpagemap, size_t size,
+			     struct dma_fence *pre_migrate_fence)
  {
  	init_completion(&devmem_allocation->detached);
  	devmem_allocation->dev = dev;

@@ -825,6 +829,7 @@ void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
  	devmem_allocation->ops = ops;
  	devmem_allocation->dpagemap = dpagemap;
  	devmem_allocation->size = size;
+	devmem_allocation->pre_migrate_fence = pre_migrate_fence;
  }
  EXPORT_SYMBOL_GPL(drm_pagemap_devmem_init);
 

drivers/gpu/drm/xe/xe_svm.c +43 -8

index 36634c84d148..2152d20049e4 100644
@@ -483,11 +483,12 @@ static void xe_svm_copy_us_stats_incr(struct xe_gt *gt,
 
  static int xe_svm_copy(struct page **pages,
  		       struct drm_pagemap_addr *pagemap_addr,
-		       unsigned long npages, const enum xe_svm_copy_dir dir)
+		       unsigned long npages, const enum xe_svm_copy_dir dir,
+		       struct dma_fence *pre_migrate_fence)
  {
  	struct xe_vram_region *vr = NULL;
  	struct xe_gt *gt = NULL;
-	struct xe_device *xe;
+	struct xe_device *xe = NULL;
  	struct dma_fence *fence = NULL;
  	unsigned long i;
  #define XE_VRAM_ADDR_INVALID	~0x0ull

@@ -496,6 +497,18 @@ static int xe_svm_copy(struct page **pages,
  	bool sram = dir == XE_SVM_COPY_TO_SRAM;
  	ktime_t start = xe_svm_stats_ktime_get();
 
+	if (pre_migrate_fence && (sram || dma_fence_is_container(pre_migrate_fence))) {
+		/*
+		 * This would typically be a p2p migration from source, or
+		 * a composite fence operation on the destination memory.
+		 * Ensure that any other GPU operation on the destination
+		 * is complete.
+		 */
+		err = dma_fence_wait(pre_migrate_fence, true);
+		if (err)
+			return err;
+	}
+
  	/*
  	 * This flow is complex: it locates physically contiguous device pages,
  	 * derives the starting physical address, and performs a single GPU copy

@@ -632,10 +645,28 @@ static int xe_svm_copy(struct page **pages,
 
  err_out:
  	/* Wait for all copies to complete */
-	if (fence) {
+	if (fence)
  		dma_fence_wait(fence, false);
-		dma_fence_put(fence);
+
+	/*
+	 * If migrating to devmem, we should have pipelined the migration behind
+	 * the pre_migrate_fence. Verify that this is indeed likely. If we
+	 * didn't perform any copying, just wait for the pre_migrate_fence.
+	 */
+	if (!sram && pre_migrate_fence && !dma_fence_is_signaled(pre_migrate_fence)) {
+		if (xe && fence &&
+		    (pre_migrate_fence->context != fence->context ||
+		     dma_fence_is_later(pre_migrate_fence, fence))) {
+			drm_WARN(&xe->drm, true, "Unsignaled pre-migrate fence");
+			drm_warn(&xe->drm, "fence contexts: %llu %llu. container %d\n",
+				 (unsigned long long)fence->context,
+				 (unsigned long long)pre_migrate_fence->context,
+				 dma_fence_is_container(pre_migrate_fence));
+		}
+
+		dma_fence_wait(pre_migrate_fence, false);
  	}
+	dma_fence_put(fence);
 
  	/*
  	 * XXX: We can't derive the GT here (or anywhere in this functions, but

@@ -652,16 +683,20 @@ static int xe_svm_copy(struct page **pages,
 
  static int xe_svm_copy_to_devmem(struct page **pages,
  				 struct drm_pagemap_addr *pagemap_addr,
-				 unsigned long npages)
+				 unsigned long npages,
+				 struct dma_fence *pre_migrate_fence)
  {
-	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_VRAM);
+	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_VRAM,
+			   pre_migrate_fence);
  }
 
  static int xe_svm_copy_to_ram(struct page **pages,
  			      struct drm_pagemap_addr *pagemap_addr,
-			      unsigned long npages)
+			      unsigned long npages,
+			      struct dma_fence *pre_migrate_fence)
  {
-	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_SRAM);
+	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_SRAM,
+			   pre_migrate_fence);
  }
 
  static struct xe_bo *to_xe_bo(struct drm_pagemap_devmem *devmem_allocation)

drivers/gpu/drm/xe/xe_svm.c +14 -1

  }
 
  static u64 block_offset_to_pfn(struct xe_vram_region *vr, u64 offset)

@@ -868,6 +904,7 @@ static int xe_drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  				      unsigned long timeslice_ms)
  {
  	struct xe_vram_region *vr = container_of(dpagemap, typeof(*vr), dpagemap);
+	struct dma_fence *pre_migrate_fence = NULL;
  	struct xe_device *xe = vr->xe;
  	struct device *dev = xe->drm.dev;
  	struct drm_buddy_block *block;

@@ -894,8 +931,20 @@ static int xe_drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  			break;
  		}
 
+		/* Ensure that any clearing or async eviction will complete before migration. */
+		if (!dma_resv_test_signaled(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL)) {
+			err = dma_resv_get_singleton(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL,
+						     &pre_migrate_fence);
+			if (err)
+				dma_resv_wait_timeout(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL,
+						      false, MAX_SCHEDULE_TIMEOUT);
+			else if (pre_migrate_fence)
+				dma_fence_enable_sw_signaling(pre_migrate_fence);
+		}
+
  		drm_pagemap_devmem_init(&bo->devmem_allocation, dev, mm,
-					&dpagemap_devmem_ops, dpagemap, end - start);
+					&dpagemap_devmem_ops, dpagemap, end - start,
+					pre_migrate_fence);
 
  		blocks = &to_xe_ttm_vram_mgr_resource(bo->ttm.resource)->blocks;
  		list_for_each_entry(block, blocks, link)

include/drm/drm_pagemap.h +14 -3

index f6e7e234c089..70a7991f784f 100644
@@ -8,6 +8,7 @@
 
  #define NR_PAGES(order) (1U << (order))
 
+struct dma_fence;
  struct drm_pagemap;
  struct drm_pagemap_zdd;
  struct device;

@@ -174,6 +175,8 @@ struct drm_pagemap_devmem_ops {
  	 * @pages: Pointer to array of device memory pages (destination)
  	 * @pagemap_addr: Pointer to array of DMA information (source)
  	 * @npages: Number of pages to copy
+	 * @pre_migrate_fence: dma-fence to wait for before migration start.
+	 * May be NULL.
  	 *
  	 * Copy pages to device memory. If the order of a @pagemap_addr entry
  	 * is greater than 0, the entry is populated but subsequent entries

@@ -183,13 +186,16 @@ struct drm_pagemap_devmem_ops {
  	 */
  	int (*copy_to_devmem)(struct page **pages,
  			      struct drm_pagemap_addr *pagemap_addr,
-			      unsigned long npages);
+			      unsigned long npages,
+			      struct dma_fence *pre_migrate_fence);
 
  	/**
  	 * @copy_to_ram: Copy to system RAM (required for migration)
  	 * @pages: Pointer to array of device memory pages (source)
  	 * @pagemap_addr: Pointer to array of DMA information (destination)
  	 * @npages: Number of pages to copy
+	 * @pre_migrate_fence: dma-fence to wait for before migration start.
+	 * May be NULL.
  	 *
  	 * Copy pages to system RAM. If the order of a @pagemap_addr entry
  	 * is greater than 0, the entry is populated but subsequent entries

@@ -199,7 +205,8 @@ struct drm_pagemap_devmem_ops {
  	 */
  	int (*copy_to_ram)(struct page **pages,
  			   struct drm_pagemap_addr *pagemap_addr,
-			   unsigned long npages);
+			   unsigned long npages,
+			   struct dma_fence *pre_migrate_fence);
  };
 
  /**

@@ -212,6 +219,8 @@ struct drm_pagemap_devmem_ops {
   * @dpagemap: The struct drm_pagemap of the pages this allocation belongs to.
   * @size: Size of device memory allocation
   * @timeslice_expiration: Timeslice expiration in jiffies
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  struct drm_pagemap_devmem {
  	struct device *dev;

@@ -221,6 +230,7 @@ struct drm_pagemap_devmem {
  	struct drm_pagemap *dpagemap;
  	size_t size;
  	u64 timeslice_expiration;
+	struct dma_fence *pre_migrate_fence;
  };
 
  int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,

@@ -238,7 +248,8 @@ struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page);
  void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
  			     struct device *dev, struct mm_struct *mm,
  			     const struct drm_pagemap_devmem_ops *ops,
-			     struct drm_pagemap *dpagemap, size_t size);
+			     struct drm_pagemap *dpagemap, size_t size,
+			     struct dma_fence *pre_migrate_fence);
 
  int drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  			    unsigned long start, unsigned long end,

In situations where no system memory is migrated to devmem, and in upcoming patches where another GPU is performing the migration to the newly allocated devmem buffer, there is nothing to ensure any ongoing clear to the devmem allocation or async eviction from the devmem allocation is complete.

Address that by passing a struct dma_fence down to the copy functions, and ensure it is waited for before migration is marked complete.

v3:
- New patch.
v4:
- Update the logic used for determining when to wait for the pre_migrate_fence.
- Update the logic used for determining when to warn for the pre_migrate_fence since the scheduler fences apparently can signal out-of-order.

Fixes c5b3eb5a906c ("drm/xe: Add GPUSVM device memory copy vfunc functions")

Cc Matthew Brost <[email protected]>

Cc <[email protected]> # v6.15+

Signed-off-by Thomas Hellström <[email protected]>

---
  drivers/gpu/drm/drm_pagemap.c | 13 ++++---
  drivers/gpu/drm/xe/xe_svm.c   | 67 ++++++++++++++++++++++++++++++-----
  include/drm/drm_pagemap.h     | 17 +++++++--
  3 files changed, 81 insertions(+), 16 deletions(-)

drivers/gpu/drm/drm_pagemap.c +9 -4

index 22c44807e3fe..864a73d019ed 100644
@@ -408,7 +408,8 @@ int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,
  		drm_pagemap_get_devmem_page(page, zdd);
  	}
 
-	err = ops->copy_to_devmem(pages, pagemap_addr, npages);
+	err = ops->copy_to_devmem(pages, pagemap_addr, npages,
+				  devmem_allocation->pre_migrate_fence);
  	if (err)
  		goto err_finalize;
 

@@ -596,7 +597,7 @@ int drm_pagemap_evict_to_ram(struct drm_pagemap_devmem *devmem_allocation)
  	for (i = 0; i < npages; ++i)
  		pages[i] = migrate_pfn_to_page(src[i]);
 
-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);
  	if (err)
  		goto err_finalize;
 

@@ -732,7 +733,7 @@ static int __drm_pagemap_migrate_to_ram(struct vm_area_struct *vas,
  	for (i = 0; i < npages; ++i)
  		pages[i] = migrate_pfn_to_page(migrate.src[i]);
 
-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);
  	if (err)
  		goto err_finalize;
 

@@ -813,11 +814,14 @@ EXPORT_SYMBOL_GPL(drm_pagemap_pagemap_ops_get);
   * @ops: Pointer to the operations structure for GPU SVM device memory
   * @dpagemap: The struct drm_pagemap we're allocating from.
   * @size: Size of device memory allocation
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
  			     struct device *dev, struct mm_struct *mm,
  			     const struct drm_pagemap_devmem_ops *ops,
-			     struct drm_pagemap *dpagemap, size_t size)
+			     struct drm_pagemap *dpagemap, size_t size,
+			     struct dma_fence *pre_migrate_fence)
  {
  	init_completion(&devmem_allocation->detached);
  	devmem_allocation->dev = dev;

@@ -825,6 +829,7 @@ void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
  	devmem_allocation->ops = ops;
  	devmem_allocation->dpagemap = dpagemap;
  	devmem_allocation->size = size;
+	devmem_allocation->pre_migrate_fence = pre_migrate_fence;
  }
  EXPORT_SYMBOL_GPL(drm_pagemap_devmem_init);
 

drivers/gpu/drm/xe/xe_svm.c +3 -2

index 36634c84d148..2152d20049e4 100644
@@ -483,11 +483,12 @@ static void xe_svm_copy_us_stats_incr(struct xe_gt *gt,
 
  static int xe_svm_copy(struct page **pages,
  		       struct drm_pagemap_addr *pagemap_addr,
-		       unsigned long npages, const enum xe_svm_copy_dir dir)
+		       unsigned long npages, const enum xe_svm_copy_dir dir,
+		       struct dma_fence *pre_migrate_fence)
  {
  	struct xe_vram_region *vr = NULL;
  	struct xe_gt *gt = NULL;
-	struct xe_device *xe;
+	struct xe_device *xe = NULL;
  	struct dma_fence *fence = NULL;
  	unsigned long i;
  #define XE_VRAM_ADDR_INVALID	~0x0ull

drivers/gpu/drm/xe/xe_svm.c +54 -7

+		/*
+		 * This would typically be a p2p migration from source, or
+		 * a composite fence operation on the destination memory.
+		 * Ensure that any other GPU operation on the destination
+		 * is complete.
+		 */
+		err = dma_fence_wait(pre_migrate_fence, true);
+		if (err)
+			return err;
+	}
+
  	/*
  	 * This flow is complex: it locates physically contiguous device pages,
  	 * derives the starting physical address, and performs a single GPU copy

@@ -632,10 +645,28 @@ static int xe_svm_copy(struct page **pages,
 
  err_out:
  	/* Wait for all copies to complete */
-	if (fence) {
+	if (fence)
  		dma_fence_wait(fence, false);
-		dma_fence_put(fence);
+
+	/*
+	 * If migrating to devmem, we should have pipelined the migration behind
+	 * the pre_migrate_fence. Verify that this is indeed likely. If we
+	 * didn't perform any copying, just wait for the pre_migrate_fence.
+	 */
+	if (!sram && pre_migrate_fence && !dma_fence_is_signaled(pre_migrate_fence)) {
+		if (xe && fence &&
+		    (pre_migrate_fence->context != fence->context ||
+		     dma_fence_is_later(pre_migrate_fence, fence))) {
+			drm_WARN(&xe->drm, true, "Unsignaled pre-migrate fence");
+			drm_warn(&xe->drm, "fence contexts: %llu %llu. container %d\n",
+				 (unsigned long long)fence->context,
+				 (unsigned long long)pre_migrate_fence->context,
+				 dma_fence_is_container(pre_migrate_fence));
+		}
+
+		dma_fence_wait(pre_migrate_fence, false);
  	}
+	dma_fence_put(fence);
 
  	/*
  	 * XXX: We can't derive the GT here (or anywhere in this functions, but

@@ -652,16 +683,20 @@ static int xe_svm_copy(struct page **pages,
 
  static int xe_svm_copy_to_devmem(struct page **pages,
  				 struct drm_pagemap_addr *pagemap_addr,
-				 unsigned long npages)
+				 unsigned long npages,
+				 struct dma_fence *pre_migrate_fence)
  {
-	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_VRAM);
+	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_VRAM,
+			   pre_migrate_fence);
  }
 
  static int xe_svm_copy_to_ram(struct page **pages,
  			      struct drm_pagemap_addr *pagemap_addr,
-			      unsigned long npages)
+			      unsigned long npages,
+			      struct dma_fence *pre_migrate_fence)
  {
-	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_SRAM);
+	return xe_svm_copy(pages, pagemap_addr, npages, XE_SVM_COPY_TO_SRAM,
+			   pre_migrate_fence);
  }
 
  static struct xe_bo *to_xe_bo(struct drm_pagemap_devmem *devmem_allocation)

@@ -676,6 +711,7 @@ static void xe_svm_devmem_release(struct drm_pagemap_devmem *devmem_allocation)
 
  	xe_bo_put_async(bo);
  	xe_pm_runtime_put(xe);
+	dma_fence_put(devmem_allocation->pre_migrate_fence);
  }
 
  static u64 block_offset_to_pfn(struct xe_vram_region *vr, u64 offset)

@@ -868,6 +904,7 @@ static int xe_drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  				      unsigned long timeslice_ms)
  {
  	struct xe_vram_region *vr = container_of(dpagemap, typeof(*vr), dpagemap);
+	struct dma_fence *pre_migrate_fence = NULL;
  	struct xe_device *xe = vr->xe;
  	struct device *dev = xe->drm.dev;
  	struct drm_buddy_block *block;

@@ -894,8 +931,20 @@ static int xe_drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  			break;
  		}
 
+		/* Ensure that any clearing or async eviction will complete before migration. */
+		if (!dma_resv_test_signaled(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL)) {
+			err = dma_resv_get_singleton(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL,
+						     &pre_migrate_fence);
+			if (err)
+				dma_resv_wait_timeout(bo->ttm.base.resv, DMA_RESV_USAGE_KERNEL,
+						      false, MAX_SCHEDULE_TIMEOUT);
+			else if (pre_migrate_fence)
+				dma_fence_enable_sw_signaling(pre_migrate_fence);
+		}
+
  		drm_pagemap_devmem_init(&bo->devmem_allocation, dev, mm,
-					&dpagemap_devmem_ops, dpagemap, end - start);
+					&dpagemap_devmem_ops, dpagemap, end - start,
+					pre_migrate_fence);
 
  		blocks = &to_xe_ttm_vram_mgr_resource(bo->ttm.resource)->blocks;
  		list_for_each_entry(block, blocks, link)

include/drm/drm_pagemap.h +14 -3

index f6e7e234c089..70a7991f784f 100644
@@ -8,6 +8,7 @@
 
  #define NR_PAGES(order) (1U << (order))
 
+struct dma_fence;
  struct drm_pagemap;
  struct drm_pagemap_zdd;
  struct device;

@@ -174,6 +175,8 @@ struct drm_pagemap_devmem_ops {
  	 * @pages: Pointer to array of device memory pages (destination)
  	 * @pagemap_addr: Pointer to array of DMA information (source)
  	 * @npages: Number of pages to copy
+	 * @pre_migrate_fence: dma-fence to wait for before migration start.
+	 * May be NULL.
  	 *
  	 * Copy pages to device memory. If the order of a @pagemap_addr entry
  	 * is greater than 0, the entry is populated but subsequent entries

@@ -183,13 +186,16 @@ struct drm_pagemap_devmem_ops {
  	 */
  	int (*copy_to_devmem)(struct page **pages,
  			      struct drm_pagemap_addr *pagemap_addr,
-			      unsigned long npages);
+			      unsigned long npages,
+			      struct dma_fence *pre_migrate_fence);
 
  	/**
  	 * @copy_to_ram: Copy to system RAM (required for migration)
  	 * @pages: Pointer to array of device memory pages (source)
  	 * @pagemap_addr: Pointer to array of DMA information (destination)
  	 * @npages: Number of pages to copy
+	 * @pre_migrate_fence: dma-fence to wait for before migration start.
+	 * May be NULL.
  	 *
  	 * Copy pages to system RAM. If the order of a @pagemap_addr entry
  	 * is greater than 0, the entry is populated but subsequent entries

@@ -199,7 +205,8 @@ struct drm_pagemap_devmem_ops {
  	 */
  	int (*copy_to_ram)(struct page **pages,
  			   struct drm_pagemap_addr *pagemap_addr,
-			   unsigned long npages);
+			   unsigned long npages,
+			   struct dma_fence *pre_migrate_fence);
  };
 
  /**

@@ -212,6 +219,8 @@ struct drm_pagemap_devmem_ops {
   * @dpagemap: The struct drm_pagemap of the pages this allocation belongs to.
   * @size: Size of device memory allocation
   * @timeslice_expiration: Timeslice expiration in jiffies
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  struct drm_pagemap_devmem {
  	struct device *dev;

@@ -221,6 +230,7 @@ struct drm_pagemap_devmem {
  	struct drm_pagemap *dpagemap;
  	size_t size;
  	u64 timeslice_expiration;
+	struct dma_fence *pre_migrate_fence;
  };
 
  int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,

@@ -238,7 +248,8 @@ struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page);
  void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
  			     struct device *dev, struct mm_struct *mm,
  			     const struct drm_pagemap_devmem_ops *ops,
-			     struct drm_pagemap *dpagemap, size_t size);
+			     struct drm_pagemap *dpagemap, size_t size,
+			     struct dma_fence *pre_migrate_fence);
 
  int drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  			    unsigned long start, unsigned long end,

Quoted 127 lines · Thomas Hellström

In situations where no system memory is migrated to devmem, and in upcoming patches where another GPU is performing the migration to the newly allocated devmem buffer, there is nothing to ensure any ongoing clear to the devmem allocation or async eviction from the devmem allocation is complete.

Address that by passing a struct dma_fence down to the copy functions, and ensure it is waited for before migration is marked complete.

v3:
- New patch.
v4:
- Update the logic used for determining when to wait for the pre_migrate_fence.
- Update the logic used for determining when to warn for the pre_migrate_fence since the scheduler fences apparently can signal out-of-order.

Fixes c5b3eb5a906c ("drm/xe: Add GPUSVM device memory copy vfunc

functions")

Cc Matthew Brost <[email protected]>

Cc <[email protected]> # v6.15+

Signed-off-by Thomas Hellström <[email protected]>

---
  drivers/gpu/drm/drm_pagemap.c | 13 ++++---
  drivers/gpu/drm/xe/xe_svm.c   | 67
++++++++++++++++++++++++++++++-----
  include/drm/drm_pagemap.h     | 17 +++++++--

  3 files changed, 81 insertions(+), 16 deletions(-)

drivers/gpu/drm/drm_pagemap.c +0 -0

index 22c44807e3fe..864a73d019ed 100644
@@ -408,7 +408,8 @@ int drm_pagemap_migrate_to_devmem(struct
 drm_pagemap_devmem *devmem_allocation,
   		drm_pagemap_get_devmem_page(page, zdd);
   	}

drivers/gpu/drm/drm_pagemap.c +2 -1

-	err = ops->copy_to_devmem(pages, pagemap_addr, npages);
+	err = ops->copy_to_devmem(pages, pagemap_addr, npages,
+				  devmem_allocation-

Quoted 1 line

pre_migrate_fence);

   if (err)
   goto err_finalize;

drivers/gpu/drm/drm_pagemap.c +0 -0

@@ -596,7 +597,7 @@ int drm_pagemap_evict_to_ram(struct
 drm_pagemap_devmem *devmem_allocation)
   	for (i = 0; i < npages; ++i)
   		pages[i] = migrate_pfn_to_page(src[i]);

drivers/gpu/drm/drm_pagemap.c +1 -1

-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);

   if (err)
   goto err_finalize;

drivers/gpu/drm/drm_pagemap.c +0 -0

@@ -732,7 +733,7 @@ static int __drm_pagemap_migrate_to_ram(struct
 vm_area_struct *vas,
   	for (i = 0; i < npages; ++i)
   		pages[i] = migrate_pfn_to_page(migrate.src[i]);

drivers/gpu/drm/drm_pagemap.c +1 -1

-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);

   if (err)
   goto err_finalize;

drivers/gpu/drm/drm_pagemap.c +0 -0

@@ -813,11 +814,14 @@
 EXPORT_SYMBOL_GPL(drm_pagemap_pagemap_ops_get);
    * @ops: Pointer to the operations structure for GPU SVM device

memory
   * @dpagemap: The struct drm_pagemap we're allocating from.
   * @size: Size of device memory allocation
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
        struct device *dev, struct mm_struct *mm,
        const struct drm_pagemap_devmem_ops *ops,

drivers/gpu/drm/drm_pagemap.c +0 -1

-			     struct drm_pagemap *dpagemap, size_t

size)

drivers/gpu/drm/drm_pagemap.c +1 -0

+			     struct drm_pagemap *dpagemap, size_t

size,

drivers/gpu/drm/drm_pagemap.c +1 -0

+			     struct dma_fence *pre_migrate_fence)

  { init_completion(&devmem_allocation->detached); devmem_allocation->dev = dev;

drivers/gpu/drm/drm_pagemap.c +1 -0

@@ -825,6 +829,7 @@ void drm_pagemap_devmem_init(struct
 drm_pagemap_devmem *devmem_allocation,
   	devmem_allocation->ops = ops;
   	devmem_allocation->dpagemap = dpagemap;
   	devmem_allocation->size = size;
+	devmem_allocation->pre_migrate_fence = pre_migrate_fence;
   }
   EXPORT_SYMBOL_GPL(drm_pagemap_devmem_init);

drivers/gpu/drm/xe/xe_svm.c +0 -0

index 36634c84d148..2152d20049e4 100644
@@ -483,11 +483,12 @@ static void xe_svm_copy_us_stats_incr(struct
 xe_gt *gt,

  static int xe_svm_copy(struct page **pages, struct drm_pagemap_addr *pagemap_addr,

drivers/gpu/drm/xe/xe_svm.c +0 -1

-		       unsigned long npages, const enum

xe_svm_copy_dir dir)

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		       unsigned long npages, const enum

xe_svm_copy_dir dir,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		       struct dma_fence *pre_migrate_fence)

  {
   struct xe_vram_region *vr = NULL;
   struct xe_gt *gt = NULL;

drivers/gpu/drm/xe/xe_svm.c +1 -1

-	struct xe_device *xe;
+	struct xe_device *xe = NULL;

   struct dma_fence *fence = NULL;
   unsigned long i;
  #define XE_VRAM_ADDR_INVALID ~0x0ull

drivers/gpu/drm/xe/xe_svm.c +0 -0

@@ -496,6 +497,18 @@ static int xe_svm_copy(struct page **pages,
   	bool sram = dir == XE_SVM_COPY_TO_SRAM;
   	ktime_t start = xe_svm_stats_ktime_get();

Quoted 3 lines

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	if (pre_migrate_fence && (sram ||

dma_fence_is_container(pre_migrate_fence))) {

Patch LGTM. Nit, Moving sram check for p2p migration from source here makes better sense with [Patch 22] drm/pagemap: Support source migration
over interconnect

Quoted 250 lines

drivers/gpu/drm/xe/xe_svm.c +2 -0

+		/*
+		 * This would typically be a p2p migration from

source, or

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		 * a composite fence operation on the destination

memory.

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		 * Ensure that any other GPU operation on the

destination

drivers/gpu/drm/xe/xe_svm.c +7 -0

+		 * is complete.
+		 */
+		err = dma_fence_wait(pre_migrate_fence, true);
+		if (err)
+			return err;
+	}
+

   /*
   * This flow is complex: it locates physically contiguous device pages,
   * derives the starting physical address, and performs a single GPU copy

drivers/gpu/drm/xe/xe_svm.c +0 -0

@@ -632,10 +645,28 @@ static int xe_svm_copy(struct page **pages,

  err_out:
   /* Wait for all copies to complete */

drivers/gpu/drm/xe/xe_svm.c +1 -1

-	if (fence) {
+	if (fence)

   dma_fence_wait(fence, false);

drivers/gpu/drm/xe/xe_svm.c +3 -1

-		dma_fence_put(fence);
+
+	/*
+	 * If migrating to devmem, we should have pipelined the

migration behind

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	 * the pre_migrate_fence. Verify that this is indeed

likely. If we

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	 * didn't perform any copying, just wait for the

pre_migrate_fence.

drivers/gpu/drm/xe/xe_svm.c +2 -0

+	 */
+	if (!sram && pre_migrate_fence &&

!dma_fence_is_signaled(pre_migrate_fence)) {

drivers/gpu/drm/xe/xe_svm.c +2 -0

+		if (xe && fence &&
+		    (pre_migrate_fence->context != fence->context

||

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		     dma_fence_is_later(pre_migrate_fence,

fence))) {

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			drm_WARN(&xe->drm, true, "Unsignaled pre-

migrate fence");

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			drm_warn(&xe->drm, "fence contexts: %llu

%llu. container %d\n",

drivers/gpu/drm/xe/xe_svm.c +1 -0

+				 (unsigned long long)fence-

Quoted 1 line

context,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+				 (unsigned long

long)pre_migrate_fence->context,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+				

dma_fence_is_container(pre_migrate_fence));

drivers/gpu/drm/xe/xe_svm.c +3 -0

+		}
+
+		dma_fence_wait(pre_migrate_fence, false);

   }

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	dma_fence_put(fence);

   /*
   * XXX: We can't derive the GT here (or anywhere in this functions, but

drivers/gpu/drm/xe/xe_svm.c +0 -0

@@ -652,16 +683,20 @@ static int xe_svm_copy(struct page **pages,

  static int xe_svm_copy_to_devmem(struct page **pages, struct drm_pagemap_addr *pagemap_addr,

drivers/gpu/drm/xe/xe_svm.c +2 -1

-				 unsigned long npages)
+				 unsigned long npages,
+				 struct dma_fence

*pre_migrate_fence)
  {

drivers/gpu/drm/xe/xe_svm.c +0 -1

-	return xe_svm_copy(pages, pagemap_addr, npages,

XE_SVM_COPY_TO_VRAM);

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	return xe_svm_copy(pages, pagemap_addr, npages,

XE_SVM_COPY_TO_VRAM,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			   pre_migrate_fence);

  }

  static int xe_svm_copy_to_ram(struct page **pages, struct drm_pagemap_addr *pagemap_addr,

drivers/gpu/drm/xe/xe_svm.c +2 -1

-			      unsigned long npages)
+			      unsigned long npages,
+			      struct dma_fence *pre_migrate_fence)

  {

drivers/gpu/drm/xe/xe_svm.c +0 -1

-	return xe_svm_copy(pages, pagemap_addr, npages,

XE_SVM_COPY_TO_SRAM);

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	return xe_svm_copy(pages, pagemap_addr, npages,

XE_SVM_COPY_TO_SRAM,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			   pre_migrate_fence);

  }

  static struct xe_bo *to_xe_bo(struct drm_pagemap_devmem *devmem_allocation)

drivers/gpu/drm/xe/xe_svm.c +0 -0

@@ -676,6 +711,7 @@ static void xe_svm_devmem_release(struct
 drm_pagemap_devmem *devmem_allocation)

   xe_bo_put_async(bo);
   xe_pm_runtime_put(xe);

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	dma_fence_put(devmem_allocation->pre_migrate_fence);

  }

  static u64 block_offset_to_pfn(struct xe_vram_region *vr, u64 offset)

drivers/gpu/drm/xe/xe_svm.c +1 -0

@@ -868,6 +904,7 @@ static int xe_drm_pagemap_populate_mm(struct
 drm_pagemap *dpagemap,
   				      unsigned long timeslice_ms)
   {
   	struct xe_vram_region *vr = container_of(dpagemap,
 typeof(*vr), dpagemap);
+	struct dma_fence *pre_migrate_fence = NULL;
   	struct xe_device *xe = vr->xe;

   struct device *dev = xe->drm.dev;
   struct drm_buddy_block *block;

drivers/gpu/drm/xe/xe_svm.c +0 -0

@@ -894,8 +931,20 @@ static int xe_drm_pagemap_populate_mm(struct
 drm_pagemap *dpagemap,
   			break;
   		}

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		/* Ensure that any clearing or async eviction will

complete before migration. */

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		if (!dma_resv_test_signaled(bo->ttm.base.resv,

DMA_RESV_USAGE_KERNEL)) {

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			err = dma_resv_get_singleton(bo-

Quoted 1 line

ttm.base.resv, DMA_RESV_USAGE_KERNEL,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+						    

&pre_migrate_fence);

drivers/gpu/drm/xe/xe_svm.c +2 -0

+			if (err)
+				dma_resv_wait_timeout(bo-

Quoted 1 line

ttm.base.resv, DMA_RESV_USAGE_KERNEL,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+						      false,

MAX_SCHEDULE_TIMEOUT);

drivers/gpu/drm/xe/xe_svm.c +2 -0

+			else if (pre_migrate_fence)
+				dma_fence_enable_sw_signaling(pre_

migrate_fence);

drivers/gpu/drm/xe/xe_svm.c +2 -0

+		}
+

   drm_pagemap_devmem_init(&bo->devmem_allocation, dev, mm,

drivers/gpu/drm/xe/xe_svm.c +0 -1

-					&dpagemap_devmem_ops,

dpagemap, end - start);

drivers/gpu/drm/xe/xe_svm.c +1 -0

+					&dpagemap_devmem_ops,

dpagemap, end - start,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+					pre_migrate_fence);

   blocks = &to_xe_ttm_vram_mgr_resource(bo-

Quoted 1 line

ttm.resource)->blocks;

   list_for_each_entry(block, blocks, link)

include/drm/drm_pagemap.h +0 -0

index f6e7e234c089..70a7991f784f 100644
@@ -8,6 +8,7 @@

  #define NR_PAGES(order) (1U << (order))

+struct dma_fence;
  struct drm_pagemap;
  struct drm_pagemap_zdd;
  struct device;

include/drm/drm_pagemap.h +1 -0

@@ -174,6 +175,8 @@ struct drm_pagemap_devmem_ops {
   	 * @pages: Pointer to array of device memory pages
 (destination)
   	 * @pagemap_addr: Pointer to array of DMA information
 (source)
   	 * @npages: Number of pages to copy
+	 * @pre_migrate_fence: dma-fence to wait for before

migration start.

include/drm/drm_pagemap.h +1 -0

+	 * May be NULL.

   *
   * Copy pages to device memory. If the order of a @pagemap_addr entry
   * is greater than 0, the entry is populated but subsequent entries

include/drm/drm_pagemap.h +0 -0

@@ -183,13 +186,16 @@ struct drm_pagemap_devmem_ops {
   	 */
   	int (*copy_to_devmem)(struct page **pages,

         struct drm_pagemap_addr *pagemap_addr,

include/drm/drm_pagemap.h +2 -1

-			      unsigned long npages);
+			      unsigned long npages,
+			      struct dma_fence

*pre_migrate_fence);

   /**
   * @copy_to_ram: Copy to system RAM (required for migration)
   * @pages: Pointer to array of device memory pages (source)
   * @pagemap_addr: Pointer to array of DMA information (destination)
   * @npages: Number of pages to copy

include/drm/drm_pagemap.h +1 -0

+	 * @pre_migrate_fence: dma-fence to wait for before

migration start.

include/drm/drm_pagemap.h +1 -0

+	 * May be NULL.

   *
   * Copy pages to system RAM. If the order of a @pagemap_addr entry
   * is greater than 0, the entry is populated but subsequent entries

include/drm/drm_pagemap.h +2 -1

@@ -199,7 +205,8 @@ struct drm_pagemap_devmem_ops {
   	 */
   	int (*copy_to_ram)(struct page **pages,
   			   struct drm_pagemap_addr *pagemap_addr,
-			   unsigned long npages);
+			   unsigned long npages,
+			   struct dma_fence *pre_migrate_fence);
   };

  /**

include/drm/drm_pagemap.h +0 -0

@@ -212,6 +219,8 @@ struct drm_pagemap_devmem_ops {
    * @dpagemap: The struct drm_pagemap of the pages this allocation

belongs to.
   * @size: Size of device memory allocation
   * @timeslice_expiration: Timeslice expiration in jiffies
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  struct drm_pagemap_devmem {
   struct device *dev;

include/drm/drm_pagemap.h +1 -0

@@ -221,6 +230,7 @@ struct drm_pagemap_devmem {
   	struct drm_pagemap *dpagemap;
   	size_t size;
   	u64 timeslice_expiration;
+	struct dma_fence *pre_migrate_fence;
   };

  int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem *devmem_allocation,

include/drm/drm_pagemap.h +0 -0

@@ -238,7 +248,8 @@ struct drm_pagemap
 *drm_pagemap_page_to_dpagemap(struct page *page);

  void drm_pagemap_devmem_init(struct drm_pagemap_devmem *devmem_allocation,
        struct device *dev, struct mm_struct *mm,
        const struct drm_pagemap_devmem_ops *ops,

include/drm/drm_pagemap.h +0 -1

-			     struct drm_pagemap *dpagemap, size_t

size);

include/drm/drm_pagemap.h +1 -0

+			     struct drm_pagemap *dpagemap, size_t

size,

include/drm/drm_pagemap.h +1 -0

+			     struct dma_fence *pre_migrate_fence);

  int drm_pagemap_populate_mm(struct drm_pagemap *dpagemap, unsigned long start, unsigned long end,

Hi, Himal,

Quoted 131 lines · Himal Prasad

Quoted 128 lines · Thomas Hellström

In situations where no system memory is migrated to devmem, and in upcoming patches where another GPU is performing the migration to the newly allocated devmem buffer, there is nothing to ensure any ongoing clear to the devmem allocation or async eviction from the devmem allocation is complete.

Address that by passing a struct dma_fence down to the copy functions, and ensure it is waited for before migration is marked complete.

v3:
- New patch.
v4:
- Update the logic used for determining when to wait for the pre_migrate_fence.
- Update the logic used for determining when to warn for the pre_migrate_fence since the scheduler fences apparently can signal out-of-order.

Fixes c5b3eb5a906c ("drm/xe: Add GPUSVM device memory copy vfunc

functions")

Cc Matthew Brost <[email protected]>

Cc <[email protected]> # v6.15+

Signed-off-by Thomas Hellström <[email protected]>

---
   drivers/gpu/drm/drm_pagemap.c | 13 ++++---
   drivers/gpu/drm/xe/xe_svm.c   | 67
++++++++++++++++++++++++++++++-----
   include/drm/drm_pagemap.h     | 17 +++++++--
   3 files changed, 81 insertions(+), 16 deletions(-)

drivers/gpu/drm/drm_pagemap.c +2 -1

index 22c44807e3fe..864a73d019ed 100644
@@ -408,7 +408,8 @@ int drm_pagemap_migrate_to_devmem(struct
 drm_pagemap_devmem *devmem_allocation,
   		drm_pagemap_get_devmem_page(page, zdd);
   	}
 
-	err = ops->copy_to_devmem(pages, pagemap_addr, npages);
+	err = ops->copy_to_devmem(pages, pagemap_addr, npages,
+				  devmem_allocation-
 > pre_migrate_fence);
   	if (err)

   goto err_finalize;

drivers/gpu/drm/drm_pagemap.c +2 -2

@@ -596,7 +597,7 @@ int drm_pagemap_evict_to_ram(struct
 drm_pagemap_devmem *devmem_allocation)
   	for (i = 0; i < npages; ++i)
   		pages[i] = migrate_pfn_to_page(src[i]);
 
-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);
   	if (err)
   		goto err_finalize;

@@ -732,7 +733,7 @@ static int __drm_pagemap_migrate_to_ram(struct
 vm_area_struct *vas,
   	for (i = 0; i < npages; ++i)
   		pages[i] = migrate_pfn_to_page(migrate.src[i]);
 
-	err = ops->copy_to_ram(pages, pagemap_addr, npages);
+	err = ops->copy_to_ram(pages, pagemap_addr, npages, NULL);
   	if (err)
   		goto err_finalize;

@@ -813,11 +814,14 @@
 EXPORT_SYMBOL_GPL(drm_pagemap_pagemap_ops_get);
    * @ops: Pointer to the operations structure for GPU SVM device

memory
   * @dpagemap: The struct drm_pagemap we're allocating from.
   * @size: Size of device memory allocation
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  void drm_pagemap_devmem_init(struct drm_pagemap_devmem
*devmem_allocation,
        struct device *dev, struct mm_struct
*mm,
        const struct drm_pagemap_devmem_ops
*ops,

drivers/gpu/drm/drm_pagemap.c +0 -1

-			     struct drm_pagemap *dpagemap, size_t

size)

drivers/gpu/drm/drm_pagemap.c +1 -0

+			     struct drm_pagemap *dpagemap, size_t

size,

drivers/gpu/drm/drm_pagemap.c +2 -0

+			     struct dma_fence *pre_migrate_fence)
   {
   	init_completion(&devmem_allocation->detached);
   	devmem_allocation->dev = dev;

@@ -825,6 +829,7 @@ void drm_pagemap_devmem_init(struct
 drm_pagemap_devmem *devmem_allocation,
   	devmem_allocation->ops = ops;
   	devmem_allocation->dpagemap = dpagemap;
   	devmem_allocation->size = size;
+	devmem_allocation->pre_migrate_fence = pre_migrate_fence;
   }
   EXPORT_SYMBOL_GPL(drm_pagemap_devmem_init);

drivers/gpu/drm/xe/xe_svm.c +3 -2

index 36634c84d148..2152d20049e4 100644
@@ -483,11 +483,12 @@ static void xe_svm_copy_us_stats_incr(struct
 xe_gt *gt,
 
   static int xe_svm_copy(struct page **pages,
   		       struct drm_pagemap_addr *pagemap_addr,
-		       unsigned long npages, const enum
 xe_svm_copy_dir dir)
+		       unsigned long npages, const enum
 xe_svm_copy_dir dir,
+		       struct dma_fence *pre_migrate_fence)
   {
   	struct xe_vram_region *vr = NULL;
   	struct xe_gt *gt = NULL;
-	struct xe_device *xe;
+	struct xe_device *xe = NULL;

   struct dma_fence *fence = NULL;
   unsigned long i;
  #define XE_VRAM_ADDR_INVALID ~0x0ull

drivers/gpu/drm/xe/xe_svm.c +0 -0

@@ -496,6 +497,18 @@ static int xe_svm_copy(struct page **pages,
   	bool sram = dir == XE_SVM_COPY_TO_SRAM;
   	ktime_t start = xe_svm_stats_ktime_get();
 

Quoted 7 lines

drivers/gpu/drm/xe/xe_svm.c static int xe_svm_copy(struct page **pages, · line 501

Quoted 2 lines

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	if (pre_migrate_fence && (sram ||
 dma_fence_is_container(pre_migrate_fence))) {

Patch LGTM. Nit, Moving sram check for p2p migration from source here makes better sense with [Patch 22] drm/pagemap: Support source migration
over interconnect

Just to make sure I get this right, You're suggesting moving the sram check above and the comment below about source migration to the source migration patch (22), right? If so, Yeah that makes sense.

Thomas

Quoted 253 lines

Quoted 252 lines

drivers/gpu/drm/xe/xe_svm.c +2 -0

+		/*
+		 * This would typically be a p2p migration from

source, or

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		 * a composite fence operation on the destination

memory.

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		 * Ensure that any other GPU operation on the

destination

drivers/gpu/drm/xe/xe_svm.c +7 -0

+		 * is complete.
+		 */
+		err = dma_fence_wait(pre_migrate_fence, true);
+		if (err)
+			return err;
+	}
+
   	/*
   	 * This flow is complex: it locates physically contiguous

device pages,
   * derives the starting physical address, and performs a
single GPU copy

drivers/gpu/drm/xe/xe_svm.c +4 -2

@@ -632,10 +645,28 @@ static int xe_svm_copy(struct page **pages,
 
   err_out:
   	/* Wait for all copies to complete */
-	if (fence) {
+	if (fence)
   		dma_fence_wait(fence, false);
-		dma_fence_put(fence);
+
+	/*
+	 * If migrating to devmem, we should have pipelined the

migration behind

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	 * the pre_migrate_fence. Verify that this is indeed

likely. If we

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	 * didn't perform any copying, just wait for the

pre_migrate_fence.

drivers/gpu/drm/xe/xe_svm.c +2 -0

+	 */
+	if (!sram && pre_migrate_fence &&

!dma_fence_is_signaled(pre_migrate_fence)) {

drivers/gpu/drm/xe/xe_svm.c +2 -0

+		if (xe && fence &&
+		    (pre_migrate_fence->context != fence->context

||

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		     dma_fence_is_later(pre_migrate_fence,

fence))) {

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			drm_WARN(&xe->drm, true, "Unsignaled pre-

migrate fence");

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			drm_warn(&xe->drm, "fence contexts: %llu

%llu. container %d\n",

drivers/gpu/drm/xe/xe_svm.c +1 -0

+				 (unsigned long long)fence-

Quoted 1 line

context,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+				 (unsigned long

long)pre_migrate_fence->context,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+				

dma_fence_is_container(pre_migrate_fence));

drivers/gpu/drm/xe/xe_svm.c +4 -0

+		}
+
+		dma_fence_wait(pre_migrate_fence, false);
   	}
+	dma_fence_put(fence);
 
   	/*
   	 * XXX: We can't derive the GT here (or anywhere in this

functions, but

drivers/gpu/drm/xe/xe_svm.c +6 -4

@@ -652,16 +683,20 @@ static int xe_svm_copy(struct page **pages,
 
   static int xe_svm_copy_to_devmem(struct page **pages,
   				 struct drm_pagemap_addr
 *pagemap_addr,
-				 unsigned long npages)
+				 unsigned long npages,
+				 struct dma_fence
 *pre_migrate_fence)
   {
-	return xe_svm_copy(pages, pagemap_addr, npages,
 XE_SVM_COPY_TO_VRAM);
+	return xe_svm_copy(pages, pagemap_addr, npages,
 XE_SVM_COPY_TO_VRAM,
+			   pre_migrate_fence);
   }
 
   static int xe_svm_copy_to_ram(struct page **pages,
   			      struct drm_pagemap_addr
 *pagemap_addr,
-			      unsigned long npages)
+			      unsigned long npages,
+			      struct dma_fence *pre_migrate_fence)
   {
-	return xe_svm_copy(pages, pagemap_addr, npages,

XE_SVM_COPY_TO_SRAM);

drivers/gpu/drm/xe/xe_svm.c +1 -0

+	return xe_svm_copy(pages, pagemap_addr, npages,

XE_SVM_COPY_TO_SRAM,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			   pre_migrate_fence);
   }
 
   static struct xe_bo *to_xe_bo(struct drm_pagemap_devmem

*devmem_allocation)

drivers/gpu/drm/xe/xe_svm.c +1 -0

@@ -676,6 +711,7 @@ static void xe_svm_devmem_release(struct
 drm_pagemap_devmem *devmem_allocation)
 
   	xe_bo_put_async(bo);
   	xe_pm_runtime_put(xe);
+	dma_fence_put(devmem_allocation->pre_migrate_fence);
   }
 

  static u64 block_offset_to_pfn(struct xe_vram_region *vr, u64
offset)

drivers/gpu/drm/xe/xe_svm.c +1 -0

@@ -868,6 +904,7 @@ static int xe_drm_pagemap_populate_mm(struct
 drm_pagemap *dpagemap,
   				      unsigned long timeslice_ms)
   {
   	struct xe_vram_region *vr = container_of(dpagemap,
 typeof(*vr), dpagemap);
+	struct dma_fence *pre_migrate_fence = NULL;
   	struct xe_device *xe = vr->xe;

   struct device *dev = xe->drm.dev;
   struct drm_buddy_block *block;

drivers/gpu/drm/xe/xe_svm.c +1 -0

@@ -894,8 +931,20 @@ static int xe_drm_pagemap_populate_mm(struct
 drm_pagemap *dpagemap,
   			break;
   		}
 
+		/* Ensure that any clearing or async eviction will

complete before migration. */

drivers/gpu/drm/xe/xe_svm.c +1 -0

+		if (!dma_resv_test_signaled(bo->ttm.base.resv,

DMA_RESV_USAGE_KERNEL)) {

drivers/gpu/drm/xe/xe_svm.c +1 -0

+			err = dma_resv_get_singleton(bo-

Quoted 1 line

ttm.base.resv, DMA_RESV_USAGE_KERNEL,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+						

&pre_migrate_fence);

drivers/gpu/drm/xe/xe_svm.c +2 -0

+			if (err)
+				dma_resv_wait_timeout(bo-

Quoted 1 line

ttm.base.resv, DMA_RESV_USAGE_KERNEL,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+						      false,

MAX_SCHEDULE_TIMEOUT);

drivers/gpu/drm/xe/xe_svm.c +2 -0

+			else if (pre_migrate_fence)
+				dma_fence_enable_sw_signaling(pre_

migrate_fence);

drivers/gpu/drm/xe/xe_svm.c +2 -0

+		}
+
   		drm_pagemap_devmem_init(&bo->devmem_allocation,

dev, mm,

drivers/gpu/drm/xe/xe_svm.c +0 -1

-					&dpagemap_devmem_ops,

dpagemap, end - start);

drivers/gpu/drm/xe/xe_svm.c +1 -0

+					&dpagemap_devmem_ops,

dpagemap, end - start,

drivers/gpu/drm/xe/xe_svm.c +1 -0

+					pre_migrate_fence);
 
   		blocks = &to_xe_ttm_vram_mgr_resource(bo-

Quoted 1 line

ttm.resource)->blocks;

   list_for_each_entry(block, blocks, link)

include/drm/drm_pagemap.h +2 -0

index f6e7e234c089..70a7991f784f 100644
@@ -8,6 +8,7 @@
 
   #define NR_PAGES(order) (1U << (order))
 
+struct dma_fence;
   struct drm_pagemap;
   struct drm_pagemap_zdd;
   struct device;

@@ -174,6 +175,8 @@ struct drm_pagemap_devmem_ops {
   	 * @pages: Pointer to array of device memory pages
 (destination)
   	 * @pagemap_addr: Pointer to array of DMA information
 (source)
   	 * @npages: Number of pages to copy
+	 * @pre_migrate_fence: dma-fence to wait for before

migration start.

include/drm/drm_pagemap.h +1 -0

+	 * May be NULL.
   	 *
   	 * Copy pages to device memory. If the order of a

@pagemap_addr entry
   * is greater than 0, the entry is populated but
subsequent entries

include/drm/drm_pagemap.h +2 -1

@@ -183,13 +186,16 @@ struct drm_pagemap_devmem_ops {
   	 */
   	int (*copy_to_devmem)(struct page **pages,
   			      struct drm_pagemap_addr
 *pagemap_addr,
-			      unsigned long npages);
+			      unsigned long npages,
+			      struct dma_fence
 *pre_migrate_fence);
 
   	/**
   	 * @copy_to_ram: Copy to system RAM (required for
 migration)
   	 * @pages: Pointer to array of device memory pages
 (source)
   	 * @pagemap_addr: Pointer to array of DMA information

(destination)
   * @npages: Number of pages to copy

include/drm/drm_pagemap.h +1 -0

+	 * @pre_migrate_fence: dma-fence to wait for before

migration start.

include/drm/drm_pagemap.h +1 -0

+	 * May be NULL.
   	 *
   	 * Copy pages to system RAM. If the order of a

@pagemap_addr entry
   * is greater than 0, the entry is populated but
subsequent entries

include/drm/drm_pagemap.h +2 -1

@@ -199,7 +205,8 @@ struct drm_pagemap_devmem_ops {
   	 */
   	int (*copy_to_ram)(struct page **pages,
   			   struct drm_pagemap_addr *pagemap_addr,
-			   unsigned long npages);
+			   unsigned long npages,
+			   struct dma_fence *pre_migrate_fence);
   };
 
   /**

@@ -212,6 +219,8 @@ struct drm_pagemap_devmem_ops {
    * @dpagemap: The struct drm_pagemap of the pages this allocation

belongs to.
   * @size: Size of device memory allocation
   * @timeslice_expiration: Timeslice expiration in jiffies
+ * @pre_migrate_fence: Fence to wait for or pipeline behind before migration starts.
+ * (May be NULL).
   */
  struct drm_pagemap_devmem {
   struct device *dev;

include/drm/drm_pagemap.h +1 -0

@@ -221,6 +230,7 @@ struct drm_pagemap_devmem {
   	struct drm_pagemap *dpagemap;
   	size_t size;
   	u64 timeslice_expiration;
+	struct dma_fence *pre_migrate_fence;
   };
 
   int drm_pagemap_migrate_to_devmem(struct drm_pagemap_devmem

*devmem_allocation,

include/drm/drm_pagemap.h +0 -1

@@ -238,7 +248,8 @@ struct drm_pagemap
 *drm_pagemap_page_to_dpagemap(struct page *page);
   void drm_pagemap_devmem_init(struct drm_pagemap_devmem
 *devmem_allocation,
   			     struct device *dev, struct mm_struct
 *mm,
   			     const struct drm_pagemap_devmem_ops
 *ops,
-			     struct drm_pagemap *dpagemap, size_t

size);

include/drm/drm_pagemap.h +1 -0

+			     struct drm_pagemap *dpagemap, size_t

size,

include/drm/drm_pagemap.h +1 -0

+			     struct dma_fence *pre_migrate_fence);
 
   int drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
   			    unsigned long start, unsigned long

end,

With the end goal of being able to free unused pagemaps and allocate them on demand, add a refcount to struct drm_pagemap, remove the xe embedded drm_pagemap, allocating and freeing it explicitly.

v2:
- Make the drm_pagemap pointer in drm_gpusvm_pages reference-counted. v3:
- Call drm_pagemap_get() before drm_pagemap_put() in drm_gpusvm_pages (Himal Prasad Ghimiray)

Signed-off-by Thomas Hellström <[email protected]>

Reviewed-by Matthew Brost <[email protected]> #v1

---
  drivers/gpu/drm/drm_gpusvm.c       |  3 ++
  drivers/gpu/drm/drm_pagemap.c      | 51 ++++++++++++++++++++++++++++++
  drivers/gpu/drm/xe/xe_svm.c        | 26 ++++++++++-----
  drivers/gpu/drm/xe/xe_vram_types.h |  2 +-
  include/drm/drm_pagemap.h          | 36 +++++++++++++++++++++
  5 files changed, 109 insertions(+), 9 deletions(-)

drivers/gpu/drm/drm_gpusvm.c +3 -0

index 39c8c50401dd..4c7474a331bc 100644
@@ -1038,6 +1038,7 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
  		flags.has_dma_mapping = false;
  		WRITE_ONCE(svm_pages->flags.__flags, flags.__flags);
 
+		drm_pagemap_put(svm_pages->dpagemap);
  		svm_pages->dpagemap = NULL;
  	}
  }

@@ -1434,6 +1435,8 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 
  	if (pagemap) {
  		flags.has_devmem_pages = true;
+		drm_pagemap_get(dpagemap);
+		drm_pagemap_put(svm_pages->dpagemap);
  		svm_pages->dpagemap = dpagemap;
  	}
 

drivers/gpu/drm/drm_pagemap.c +51 -0

index 864a73d019ed..e82a38df72a6 100644
@@ -539,6 +539,57 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct vm_area_struct *vas,
  	return -ENOMEM;
  }
 
+static void drm_pagemap_release(struct kref *ref)
+{
+	struct drm_pagemap *dpagemap = container_of(ref, typeof(*dpagemap), ref);
+
+	kfree(dpagemap);
+}
+
+/**
+ * drm_pagemap_create() - Create a struct drm_pagemap.
+ * @dev: Pointer to a struct device providing the device-private memory.
+ * @pagemap: Pointer to a pre-setup struct dev_pagemap providing the struct pages.
+ * @ops: Pointer to the struct drm_pagemap_ops.
+ *
+ * Allocate and initialize a struct drm_pagemap.
+ *
+ * Return: A refcounted pointer to a struct drm_pagemap on success.
+ * Error pointer on error.
+ */
+struct drm_pagemap *
+drm_pagemap_create(struct device *dev,
+		   struct dev_pagemap *pagemap,
+		   const struct drm_pagemap_ops *ops)
+{
+	struct drm_pagemap *dpagemap = kzalloc(sizeof(*dpagemap), GFP_KERNEL);
+
+	if (!dpagemap)
+		return ERR_PTR(-ENOMEM);
+
+	kref_init(&dpagemap->ref);
+	dpagemap->dev = dev;
+	dpagemap->ops = ops;
+	dpagemap->pagemap = pagemap;
+
+	return dpagemap;
+}
+EXPORT_SYMBOL(drm_pagemap_create);
+
+/**
+ * drm_pagemap_put() - Put a struct drm_pagemap reference
+ * @dpagemap: Pointer to a struct drm_pagemap object.
+ *
+ * Puts a struct drm_pagemap reference and frees the drm_pagemap object
+ * if the refount reaches zero.
+ */
+void drm_pagemap_put(struct drm_pagemap *dpagemap)
+{
+	if (likely(dpagemap))
+		kref_put(&dpagemap->ref, drm_pagemap_release);
+}
+EXPORT_SYMBOL(drm_pagemap_put);
+
  /**
   * drm_pagemap_evict_to_ram() - Evict GPU SVM range to RAM
   * @devmem_allocation: Pointer to the device memory allocation

drivers/gpu/drm/xe/xe_svm.c +18 -8

index 2152d20049e4..e4036f428dc0 100644
@@ -903,7 +903,7 @@ static int xe_drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  				      struct mm_struct *mm,
  				      unsigned long timeslice_ms)
  {
-	struct xe_vram_region *vr = container_of(dpagemap, typeof(*vr), dpagemap);
+	struct xe_vram_region *vr = container_of(dpagemap->pagemap, typeof(*vr), pagemap);
  	struct dma_fence *pre_migrate_fence = NULL;
  	struct xe_device *xe = vr->xe;
  	struct device *dev = xe->drm.dev;

@@ -1427,7 +1427,7 @@ u8 xe_svm_ranges_zap_ptes_in_range(struct xe_vm *vm, u64 start, u64 end)
 
  static struct drm_pagemap *tile_local_pagemap(struct xe_tile *tile)
  {
-	return &tile->mem.vram->dpagemap;
+	return tile->mem.vram->dpagemap;
  }
 
  /**

@@ -1537,6 +1537,15 @@ int xe_devm_add(struct xe_tile *tile, struct xe_vram_region *vr)
  		return ret;
  	}
 
+	vr->dpagemap = drm_pagemap_create(dev, &vr->pagemap,
+					  &xe_drm_pagemap_ops);
+	if (IS_ERR(vr->dpagemap)) {
+		drm_err(&xe->drm, "Failed to create drm_pagemap tile %d memory: %pe\n",
+			tile->id, vr->dpagemap);
+		ret = PTR_ERR(vr->dpagemap);
+		goto out_no_dpagemap;
+	}
+
  	vr->pagemap.type = MEMORY_DEVICE_PRIVATE;
  	vr->pagemap.range.start = res->start;
  	vr->pagemap.range.end = res->end;

@@ -1544,22 +1553,23 @@ int xe_devm_add(struct xe_tile *tile, struct xe_vram_region *vr)
  	vr->pagemap.ops = drm_pagemap_pagemap_ops_get();
  	vr->pagemap.owner = xe_svm_devm_owner(xe);
  	addr = devm_memremap_pages(dev, &vr->pagemap);
-
-	vr->dpagemap.dev = dev;
-	vr->dpagemap.ops = &xe_drm_pagemap_ops;
-
  	if (IS_ERR(addr)) {
-		devm_release_mem_region(dev, res->start, resource_size(res));
  		ret = PTR_ERR(addr);
  		drm_err(&xe->drm, "Failed to remap tile %d memory, errno %pe\n",
  			tile->id, ERR_PTR(ret));
-		return ret;
+		goto out_failed_memremap;
  	}
  	vr->hpa_base = res->start;
 
  	drm_dbg(&xe->drm, "Added tile %d memory [%llx-%llx] to devm, remapped to %pr\n",
  		tile->id, vr->io_start, vr->io_start + vr->usable_size, res);
  	return 0;
+
+out_failed_memremap:
+	drm_pagemap_put(vr->dpagemap);
+out_no_dpagemap:
+	devm_release_mem_region(dev, res->start, resource_size(res));
+	return ret;
  }
  #else
  int xe_svm_alloc_vram(struct xe_tile *tile,

drivers/gpu/drm/xe/xe_vram_types.h +1 -1

index 83772dcbf1af..c0d2c5ee8c10 100644
@@ -72,7 +72,7 @@ struct xe_vram_region {
  	 * @dpagemap: The struct drm_pagemap of the ZONE_DEVICE memory
  	 * pages of this tile.
  	 */
-	struct drm_pagemap dpagemap;
+	struct drm_pagemap *dpagemap;
  	/**
  	 * @hpa_base: base host physical address
  	 *

include/drm/drm_pagemap.h +36 -0

index 70a7991f784f..093e7199c44b 100644
@@ -130,11 +130,15 @@ struct drm_pagemap_ops {
   * struct drm_pagemap: Additional information for a struct dev_pagemap
   * used for device p2p handshaking.
   * @ops: The struct drm_pagemap_ops.
+ * @ref: Reference count.
   * @dev: The struct drevice owning the device-private memory.
+ * @pagemap: Pointer to the underlying dev_pagemap.
   */
  struct drm_pagemap {
  	const struct drm_pagemap_ops *ops;
+	struct kref ref;
  	struct device *dev;
+	struct dev_pagemap *pagemap;
  };
 
  struct drm_pagemap_devmem;

@@ -209,6 +213,37 @@ struct drm_pagemap_devmem_ops {
  			   struct dma_fence *pre_migrate_fence);
  };
 
+struct drm_pagemap *drm_pagemap_create(struct device *dev,
+				       struct dev_pagemap *pagemap,
+				       const struct drm_pagemap_ops *ops);
+
+#if IS_ENABLED(CONFIG_DRM_GPUSVM)
+
+void drm_pagemap_put(struct drm_pagemap *dpagemap);
+
+#else
+
+static inline void drm_pagemap_put(struct drm_pagemap *dpagemap)
+{
+}
+
+#endif /* IS_ENABLED(CONFIG_DRM_GPUSVM) */
+
+/**
+ * drm_pagemap_get() - Obtain a reference on a struct drm_pagemap
+ * @dpagemap: Pointer to the struct drm_pagemap.
+ *
+ * Return: Pointer to the struct drm_pagemap.
+ */
+static inline struct drm_pagemap *
+drm_pagemap_get(struct drm_pagemap *dpagemap)
+{
+	if (likely(dpagemap))
+		kref_get(&dpagemap->ref);
+
+	return dpagemap;
+}
+
  /**
   * struct drm_pagemap_devmem - Structure representing a GPU SVM device memory allocation
   *

@@ -257,3 +292,4 @@ int drm_pagemap_populate_mm(struct drm_pagemap *dpagemap,
  			    unsigned long timeslice_ms);
 
  #endif
+