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 <[email protected]> Signed-off-by: Honglei Huang <[email protected]> --- 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
