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

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

On Mon, Sep 07, 2026 at 08:18:08PM -0700, Matthew Brost wrote:
> On Sat, Sep 05, 2026 at 09:31:42PM +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().
> > 
> 
> Nice trick for IOVA allocs and variabled sized mappings like userptr.
> Looks good.
> 
> Going to megre entire series to drm-misc-next shortly. Thanks!
> 

So small snafu merging - this series won't cleanly apply drm-misc-next
and won't until 7.4 because of patches in Xe targeted for 7.4. The
series will cleanly apply to drm-xe-next, so if AMD is ok with this, I
can merge there. Or I can work through the (minor) conflicts and merge
this into drm-misc-next (ofc I'll need confirm with the drm-misc
maintainers that this ok too).

Matt

> > Suggested-by: Matthew Brost <matthew.brost@intel.com>
> 
> Reviewed-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
> >