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! > Suggested-by: Matthew Brost <[email protected]> Reviewed-by: Matthew Brost <[email protected]> > Signed-off-by: Honglei Huang <[email protected]> > --- > 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 >
