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 <[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 > >
