[PATCH v4 6/6] drm/gpusvm: keep an IOVA mapped range dma address inline

[Honglei Huang <honghuan@amd.com>]

dma_iova_try_alloc() reserves one contiguous IOVA for the whole range and
links each page at the next offset, so the device addresses run
contiguously from entry 0 and one entry describes them all. A 2 MiB range
of 4 KiB pages then drops the same 8 KiB array as a THP backed one.

Fold only when state_offset covers the full range, which proves no device
page was mapped in between, and only single page entries, so the order
kept is 0 and stays true. Widening it instead would tell a consumer to use
a huge page for npages separate CPU pages, which hangs Vega20 on amdgpu.

The kept entry no longer bounds the segment, so skip the unmap walk when
it has nothing to do, keyed off dpagemap rather than the flags, which are
not published yet on the error unwind. Consumers need the same
distinction, so drm_gpusvm_pages_first_dma() returns it alongside the
array from one read of the flags; xe passes it to xe_res_first_dma().

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
 drivers/gpu/drm/drm_gpusvm.c       | 41 +++++++++++++++++++++++-------
 drivers/gpu/drm/xe/xe_pt.c         | 23 +++++++++++------
 drivers/gpu/drm/xe/xe_res_cursor.h |  5 ++--
 drivers/gpu/drm/xe/xe_svm.h        |  8 +++---
 include/drm/drm_gpusvm.h           | 13 +++++++++-
 5 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index 2c7c4c89dc4..b6c9d3a07dc 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1242,7 +1242,7 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
 			.__flags = svm_pages->flags.__flags,
 		};
 		const struct drm_pagemap_addr *addrs =
-			drm_gpusvm_pages_first_dma(svm_pages);
+			drm_gpusvm_pages_first_dma(svm_pages, NULL);
 		bool use_iova = dma_use_iova(&svm_pages->state);
 
 		/*
@@ -1259,7 +1259,15 @@ static void __drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
 			dma_iova_free(dev, &svm_pages->state);
 		}
 
-		for (i = 0, j = 0; i < npages; j++) {
+		/*
+		 * With IOVA and no device page the unlink above tore every
+		 * entry down, and that is also when the range may be folded
+		 * to one entry, which must not be walked per entry. dpagemap
+		 * is set before the first device_map(), so it is also right
+		 * on the error path, where the flags are not published yet.
+		 */
+		for (i = 0, j = 0;
+		     (!use_iova || dpagemap) && i < npages; j++) {
 			const struct drm_pagemap_addr *addr = &addrs[j];
 
 			if (addr->proto == DRM_INTERCONNECT_SYSTEM) {
@@ -1491,17 +1499,32 @@ static bool drm_gpusvm_pages_valid_unlocked(struct drm_gpusvm *gpusvm,
 
 /**
  * drm_gpusvm_pages_inlinable() - Whether the dma address can be inlined
+ * @svm_pages: The SVM pages instance that was just mapped
  * @nentries: Number of entries the mapping loop produced
+ * @npages: Number of pages in the CPU range
  *
- * A THP maps as one huge page, so the whole range needs a single device
- * address: the dma_addr array can be freed and the address kept inline,
- * which is where the memory saving comes from.
+ * A THP maps as one huge page, and an IOVA reservation links every page of
+ * the range at the next offset, so the device addresses run contiguously from
+ * entry 0. Either way one entry describes the whole range, so the dma_addr
+ * array can be freed and the address kept inline.
+ *
+ * state_offset advances only on the IOVA branch, so reaching the full range
+ * length proves no device page was mapped in between. Only single page
+ * entries fold, so the order kept is 0 and describes the range truthfully.
+ * Larger chunks, several huge pages among them, stay an array that is
+ * already short and that a consumer places with one PTE each.
  *
  * Return: True if the mapping fits in a single drm_pagemap_addr.
  */
-static bool drm_gpusvm_pages_inlinable(unsigned long nentries)
+static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
+				       unsigned long nentries,
+				       unsigned long npages)
 {
-	return nentries == 1;
+	if (nentries == 1)
+		return true;
+
+	return nentries == npages && dma_use_iova(&svm_pages->state) &&
+	       svm_pages->state_offset == npages * PAGE_SIZE;
 }
 
 /**
@@ -1656,7 +1679,7 @@ static int drm_gpusvm_dma_map_pages(struct drm_gpusvm *gpusvm,
 	if (pagemap)
 		flags.has_devmem_pages = true;
 
-	if (drm_gpusvm_pages_inlinable(j)) {
+	if (drm_gpusvm_pages_inlinable(svm_pages, j, npages)) {
 		struct drm_pagemap_addr addr = svm_pages->dma_addr[0];
 
 		kvfree(svm_pages->dma_addr);
@@ -1772,7 +1795,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 
 	if (map_dma) {
 		for (p = 0; p < num_pages; ++p) {
-			if (drm_gpusvm_pages_first_dma(&svm_pages[p]))
+			if (drm_gpusvm_pages_first_dma(&svm_pages[p], NULL))
 				continue;
 			svm_pages[p].dma_addr =
 				kvzalloc_objs(*svm_pages[p].dma_addr, npages);
diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index fa4b29da0b6..7fb2fe1f826 100644
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -831,9 +831,12 @@ 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(xe_svm_range_first_dma(range), 0,
-					 xe_svm_range_size(range),
-					 &curs);
+			const struct drm_pagemap_addr *addr;
+			bool contiguous;
+
+			addr = xe_svm_range_first_dma(range, &contiguous);
+			xe_res_first_dma(addr, 0, xe_svm_range_size(range),
+					 contiguous, &curs);
 			xe_svm_range_debug(range, "BIND PREPARE - MIXED");
 		} else {
 			xe_assert(xe, false);
@@ -865,11 +868,15 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
 		xe_bo_assert_held(bo);
 
 	if (!xe_vma_is_null(vma) && !range && !is_purged) {
-		if (xe_vma_is_userptr(vma))
-			xe_res_first_dma(drm_gpusvm_pages_first_dma
-					 (&to_userptr_vma(vma)->userptr.pages),
-					 0, xe_vma_size(vma), &curs);
-		else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
+		if (xe_vma_is_userptr(vma)) {
+			const struct drm_pagemap_addr *addr;
+			bool contiguous;
+
+			addr = drm_gpusvm_pages_first_dma(&to_userptr_vma(vma)->userptr.pages,
+							  &contiguous);
+			xe_res_first_dma(addr, 0, xe_vma_size(vma), contiguous,
+					 &curs);
+		} else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
 			xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
 				     xe_vma_size(vma), &curs);
 		else
diff --git a/drivers/gpu/drm/xe/xe_res_cursor.h b/drivers/gpu/drm/xe/xe_res_cursor.h
index 0522caafd89..c3a037e5f34 100644
--- a/drivers/gpu/drm/xe/xe_res_cursor.h
+++ b/drivers/gpu/drm/xe/xe_res_cursor.h
@@ -233,12 +233,13 @@ static inline void xe_res_first_sg(const struct sg_table *sg,
  * @dma_addr: struct drm_pagemap_addr array to walk
  * @start: Start of the range
  * @size: Size of the range
+ * @contiguous: Whether one entry describes the whole range
  * @cur: cursor object to initialize
  *
  * Start walking over the range of allocations between @start and @size.
  */
 static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
-				    u64 start, u64 size,
+				    u64 start, u64 size, bool contiguous,
 				    struct xe_res_cursor *cur)
 {
 	XE_WARN_ON(!dma_addr);
@@ -248,7 +249,7 @@ static inline void xe_res_first_dma(const struct drm_pagemap_addr *dma_addr,
 	cur->node = NULL;
 	cur->start = start;
 	cur->remaining = size;
-	cur->dma_seg_size = PAGE_SIZE << dma_addr->order;
+	cur->dma_seg_size = contiguous ? start + size : PAGE_SIZE << dma_addr->order;
 	cur->dma_start = 0;
 	cur->size = 0;
 	cur->dma_addr = dma_addr;
diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
index 7eb80d4d6db..2ef4ef026cc 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -223,13 +223,14 @@ static inline unsigned long xe_svm_range_size(struct xe_svm_range *range)
 /**
  * xe_svm_range_first_dma() - Resolve the device address array of a SVM range
  * @range: SVM range
+ * @contiguous: Where to store whether one entry spans the whole range
  *
  * Return: Pointer to the first device address, NULL if none is populated.
  */
 static inline const struct drm_pagemap_addr *
-xe_svm_range_first_dma(struct xe_svm_range *range)
+xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
 {
-	return drm_gpusvm_pages_first_dma(&range->pages);
+	return drm_gpusvm_pages_first_dma(&range->pages, contiguous);
 }
 
 void xe_svm_flush(struct xe_vm *vm);
@@ -449,8 +450,9 @@ static inline bool xe_svm_range_is_removed(struct xe_svm_range *range)
 }
 
 static inline const struct drm_pagemap_addr *
-xe_svm_range_first_dma(struct xe_svm_range *range)
+xe_svm_range_first_dma(struct xe_svm_range *range, bool *contiguous)
 {
+	*contiguous = false;
 	return NULL;
 }
 
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index aaad5c9b510..9e35584812f 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -380,12 +380,19 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
 /**
  * drm_gpusvm_pages_first_dma() - Resolve the device address array
  * @svm_pages: Pointer to the drm_gpusvm_pages.
+ * @contiguous: Where to store whether one entry spans the whole range, or NULL
  *
  * drm_gpusvm_pages use unions to optimize the storage of DMA addresses,
  * this function abstracts the access to the first device address. The driver
  * should use this helper instead of reading dma_addr directly to prevent
  * array out of bounds access.
  *
+ * @contiguous comes from the same read of the flags as the array itself, so a
+ * caller cannot see the two disagree and walk past that single entry into the
+ * fields behind it. When it is set the length comes from the range rather than
+ * from the order. The order still states what one PTE may cover: the range
+ * length for a huge page, PAGE_SIZE for an IOVA mapped range of single pages.
+ *
  * Only get_pages() and the free path switch between the two union members.
  * Both hold the notifier lock for read, so taking that lock does not stop
  * them; callers need the driver lock that does, which every reader of the
@@ -396,13 +403,17 @@ static inline void drm_gpusvm_init_pages(struct drm_gpusvm_pages *svm_pages,
  * Return: Pointer to the first device address, NULL if none is populated.
  */
 static inline const struct drm_pagemap_addr *
-drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages)
+drm_gpusvm_pages_first_dma(const struct drm_gpusvm_pages *svm_pages,
+			   bool *contiguous)
 {
 	struct drm_gpusvm_pages_flags flags = {
 		/* READ_ONCE pairs with the WRITE_ONCE of the flag writers */
 		.__flags = READ_ONCE(svm_pages->flags.__flags),
 	};
 
+	if (contiguous)
+		*contiguous = flags.inline_dma_mapping;
+
 	if (flags.inline_dma_mapping)
 		return &svm_pages->inline_addr;
 
-- 
2.34.1