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

[Matthew Brost <matthew.brost@intel.com>]

On Sat, Sep 05, 2026 at 12:39:56AM +0800, Honglei Huang wrote:
> 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>

Same as last patch, we will need an additional wrapper around
drm_gpusvm_pages_first_dma for xe_svm_ranges to deal with different
Kconfigs but other than LGTM.

Matt

> Signed-off-by: Honglei Huang <honghuan@amd.com>
> ---
>  drivers/gpu/drm/drm_gpusvm.c       | 41 +++++++++++++++++++++++-------
>  drivers/gpu/drm/xe/xe_pt.c         | 24 +++++++++++------
>  drivers/gpu/drm/xe/xe_res_cursor.h |  5 ++--
>  include/drm/drm_gpusvm.h           | 13 +++++++++-
>  4 files changed, 63 insertions(+), 20 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 96ea9735f34..8bc0e801514 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -831,9 +831,13 @@ 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(drm_gpusvm_pages_first_dma(&range->pages), 0,
> -					 xe_svm_range_size(range),
> -					 &curs);
> +			const struct drm_pagemap_addr *addr;
> +			bool contiguous;
> +
> +			addr = drm_gpusvm_pages_first_dma(&range->pages,
> +							  &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 +869,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/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
>