[PATCH v2 0/5] drm/panthor: Fix partial unmaps, again

[PATCH v2 0/5] drm/panthor: Fix partial unmaps, again

[Boris Brezillon]

The first patch is just a theoretical perf improvement to avoid
unnecessary unmap+map caused by false positive returned by
iova_mapped_as_huge_page(). The second patch is a fix for a real
bug when iova_mapped_as_huge_page() is called on an imported BO.
The last 2 patches are just consolidating the logic to make things
clearer.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Changes in v2:
- Add a commit dropping a redundant panthor_fix_sparse_map_offset() call
- Collect R-bs
- Link to v1: https://patch.msgid.link/20260917-panthor-fix-partial-unmap-v1-0-c7008f15fea3@collabora.com

---
Boris Brezillon (5):
      drm/panthor: Avoid false positives in iova_mapped_as_huge_page()
      drm/panthor: Fix iova_mapped_as_huge_page() for imported BOs
      drm/panthor: Consolidate the is-huge-page-mapping test
      drm/panthor: Actually check huge-page mapping on sparse regions
      drm/panthor: Remove redundant panthor_fix_sparse_map_offset() call

 drivers/gpu/drm/panthor/panthor_mmu.c | 112 +++++++++++++++++++++++-----------
 1 file changed, 77 insertions(+), 35 deletions(-)
---
base-commit: 45585c3aa285854face65293acc95eff73063d6d
change-id: 20260916-panthor-fix-partial-unmap-a52034f21184

Best regards,
--  
Boris Brezillon <boris.brezillon@collabora.com>

[PATCH v2 1/5] drm/panthor: Avoid false positives in iova_mapped_as_huge_page()

[Boris Brezillon]

The check on the folio size is actually moot if the BO offset matching
the VA we're checking huge-mapping for is not 2M aligned as well.
This means that we are sometimes returning true when we shouldn't, which
forces an extra unmap+map to deal with block-mapping splits. It's not
a functional bug per-se, because the unmap+map sequence will restore
things in the state we expect them to be, but it's better to properly
optimize those cases.

Note that we now align the VA on 2M address below it otherwise we can't
check the bo_offset alignment (both physical and virtual address need
to be aligned, in addition to the physically contiguous size being 2M,
which the folio size check ensures).

These changes force us to pass the drm_gpuva that's being unmapped
instead of the new mappings that will be created to cover the left/right
sections we remap. This changes makes the logic a lot easier to reason
about, because it doesn't make sense to how things were mapped by
passing the new mappings that are not yet in place.

Fixes: 8e7460eac786 ("drm/panthor: Support partial unmaps of huge pages")
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Akash Goel <akash.goel@arm.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 9f63a048df61..b0a7033480e6 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2295,15 +2295,29 @@ static int panthor_gpuva_sm_step_map(struct drm_gpuva_op *op, void *priv)
 }
 
 static bool
-iova_mapped_as_huge_page(struct drm_gpuva_op_map *op, u64 addr)
+iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
 {
-	struct panthor_gem_object *bo = to_panthor_bo(op->gem.obj);
+	struct panthor_gem_object *bo = to_panthor_bo(mapping->gem.obj);
+	u64 aligned_va = ALIGN_DOWN(va, SZ_2M);
 	const struct page *pg;
 	pgoff_t bo_offset;
 
-	bo_offset = addr - op->va.addr + op->gem.offset;
+	/* If the 2M-aligned VA is outside the mapping being tested, we know
+	 * it's not a huge map.
+	 */
+	if (aligned_va < mapping->va.addr)
+		return false;
+
+	bo_offset = aligned_va - mapping->va.addr + mapping->gem.offset;
 	pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
 
+	/* In case of shmem backing, we know we can only have a huge mapping
+	 * if the bo_offset is 2M aligned, meaning we can skip the folio size
+	 * check if it's not the case.
+	 */
+	if (!IS_ALIGNED(bo_offset, SZ_2M))
+		return false;
+
 	return folio_size(page_folio(pg)) >= SZ_2M;
 }
 
@@ -2328,7 +2342,7 @@ unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
 	 */
 	if (op->prev && aligned_unmap_start < *unmap_start &&
 	    op->prev->va.addr <= aligned_unmap_start &&
-	    (is_sparse || iova_mapped_as_huge_page(op->prev, *unmap_start))) {
+	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, *unmap_start))) {
 		*unmap_range += *unmap_start - aligned_unmap_start;
 		*unmap_start = aligned_unmap_start;
 	}
@@ -2338,7 +2352,7 @@ unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
 	 */
 	if (op->next && aligned_unmap_end > unmap_end &&
 	    op->next->va.addr + op->next->va.range >= aligned_unmap_end &&
-	    (is_sparse || iova_mapped_as_huge_page(op->next, unmap_end - 1))) {
+	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, unmap_end - 1))) {
 		*unmap_range += aligned_unmap_end - unmap_end;
 	}
 }

-- 
2.55.0

Re: [PATCH v2 1/5] drm/panthor: Avoid false positives in iova_mapped_as_huge_page()

[Steven Price]

On 24/09/2026 12:04, Boris Brezillon wrote:
> The check on the folio size is actually moot if the BO offset matching
> the VA we're checking huge-mapping for is not 2M aligned as well.
> This means that we are sometimes returning true when we shouldn't, which
> forces an extra unmap+map to deal with block-mapping splits. It's not
> a functional bug per-se, because the unmap+map sequence will restore
> things in the state we expect them to be, but it's better to properly
> optimize those cases.
> 
> Note that we now align the VA on 2M address below it otherwise we can't
> check the bo_offset alignment (both physical and virtual address need
> to be aligned, in addition to the physically contiguous size being 2M,
> which the folio size check ensures).
> 
> These changes force us to pass the drm_gpuva that's being unmapped
> instead of the new mappings that will be created to cover the left/right
> sections we remap. This changes makes the logic a lot easier to reason
> about, because it doesn't make sense to how things were mapped by
> passing the new mappings that are not yet in place.
> 
> Fixes: 8e7460eac786 ("drm/panthor: Support partial unmaps of huge pages")
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> Reviewed-by: Akash Goel <akash.goel@arm.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 9f63a048df61..b0a7033480e6 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2295,15 +2295,29 @@ static int panthor_gpuva_sm_step_map(struct drm_gpuva_op *op, void *priv)
>  }
>  
>  static bool
> -iova_mapped_as_huge_page(struct drm_gpuva_op_map *op, u64 addr)
> +iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  {
> -	struct panthor_gem_object *bo = to_panthor_bo(op->gem.obj);
> +	struct panthor_gem_object *bo = to_panthor_bo(mapping->gem.obj);
> +	u64 aligned_va = ALIGN_DOWN(va, SZ_2M);
>  	const struct page *pg;
>  	pgoff_t bo_offset;
>  
> -	bo_offset = addr - op->va.addr + op->gem.offset;
> +	/* If the 2M-aligned VA is outside the mapping being tested, we know
> +	 * it's not a huge map.
> +	 */
> +	if (aligned_va < mapping->va.addr)
> +		return false;
> +
> +	bo_offset = aligned_va - mapping->va.addr + mapping->gem.offset;
>  	pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>  
> +	/* In case of shmem backing, we know we can only have a huge mapping
> +	 * if the bo_offset is 2M aligned, meaning we can skip the folio size
> +	 * check if it's not the case.
> +	 */
> +	if (!IS_ALIGNED(bo_offset, SZ_2M))
> +		return false;
> +
>  	return folio_size(page_folio(pg)) >= SZ_2M;
>  }
>  
> @@ -2328,7 +2342,7 @@ unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
>  	 */
>  	if (op->prev && aligned_unmap_start < *unmap_start &&
>  	    op->prev->va.addr <= aligned_unmap_start &&
> -	    (is_sparse || iova_mapped_as_huge_page(op->prev, *unmap_start))) {
> +	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, *unmap_start))) {
>  		*unmap_range += *unmap_start - aligned_unmap_start;
>  		*unmap_start = aligned_unmap_start;
>  	}
> @@ -2338,7 +2352,7 @@ unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
>  	 */
>  	if (op->next && aligned_unmap_end > unmap_end &&
>  	    op->next->va.addr + op->next->va.range >= aligned_unmap_end &&
> -	    (is_sparse || iova_mapped_as_huge_page(op->next, unmap_end - 1))) {
> +	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, unmap_end - 1))) {
>  		*unmap_range += aligned_unmap_end - unmap_end;
>  	}
>  }
> 

[PATCH v2 2/5] drm/panthor: Fix iova_mapped_as_huge_page() for imported BOs

[Boris Brezillon]

Imported BOs have no backing.pages array allocated, leading to a NULL
deref when iova_mapped_as_huge_page() gets called on them.

Implement this check through and sgt walk to reach the position of the
sgt targeted by a VA, and check that the DMA address is properly aligned
and the size remaining in the SG entry is bigger than a huge page.
This is basically matching the logic in vm_map_pages(), with get_pgsize()
being replaced by a simpler test, because we don't care about the pgcount
info.

Reported-by: Akash Goel <akash.goel@arm.com>
Fixes: 8e7460eac786 ("drm/panthor: Support partial unmaps of huge pages")
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Akash Goel <akash.goel@arm.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 44 ++++++++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index b0a7033480e6..6cef954e2cba 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2299,7 +2299,6 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
 {
 	struct panthor_gem_object *bo = to_panthor_bo(mapping->gem.obj);
 	u64 aligned_va = ALIGN_DOWN(va, SZ_2M);
-	const struct page *pg;
 	pgoff_t bo_offset;
 
 	/* If the 2M-aligned VA is outside the mapping being tested, we know
@@ -2309,16 +2308,45 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
 		return false;
 
 	bo_offset = aligned_va - mapping->va.addr + mapping->gem.offset;
-	pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
 
-	/* In case of shmem backing, we know we can only have a huge mapping
-	 * if the bo_offset is 2M aligned, meaning we can skip the folio size
-	 * check if it's not the case.
-	 */
-	if (!IS_ALIGNED(bo_offset, SZ_2M))
+	if (drm_gem_is_imported(&bo->base)) {
+		struct sg_table *sgt = bo->dmap.sgt;
+		struct scatterlist *sgl;
+		unsigned int count;
+
+		/* If this is an imported BO, we have to walk the SGT and
+		 * check the dma address/size alignment to determine if it's
+		 * a huge map or not.
+		 */
+		for_each_sgtable_dma_sg(sgt, sgl, count) {
+			size_t len = sg_dma_len(sgl);
+			dma_addr_t daddr;
+
+			if (len <= bo_offset) {
+				bo_offset -= len;
+				continue;
+			}
+
+			len -= bo_offset;
+			daddr = sg_dma_address(sgl) + bo_offset;
+
+			return IS_ALIGNED(daddr, SZ_2M) &&
+			       len >= SZ_2M;
+		}
+
 		return false;
+	} else {
+		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
 
-	return folio_size(page_folio(pg)) >= SZ_2M;
+		/* In case of shmem backing, we know we can only have a huge mapping
+		 * if the bo_offset is 2M aligned, meaning we can skip the folio size
+		 * check if it's not the case.
+		 */
+		if (!IS_ALIGNED(bo_offset, SZ_2M))
+			return false;
+
+		return folio_size(page_folio(pg)) >= SZ_2M;
+	}
 }
 
 static void

-- 
2.55.0

Re: [PATCH v2 2/5] drm/panthor: Fix iova_mapped_as_huge_page() for imported BOs

[Steven Price]

On 24/09/2026 12:04, Boris Brezillon wrote:
> Imported BOs have no backing.pages array allocated, leading to a NULL
> deref when iova_mapped_as_huge_page() gets called on them.
> 
> Implement this check through and sgt walk to reach the position of the
NIT:                           ^^^
s/and/an/

> sgt targeted by a VA, and check that the DMA address is properly aligned
> and the size remaining in the SG entry is bigger than a huge page.
> This is basically matching the logic in vm_map_pages(), with get_pgsize()
> being replaced by a simpler test, because we don't care about the pgcount
> info.
> 
> Reported-by: Akash Goel <akash.goel@arm.com>
> Fixes: 8e7460eac786 ("drm/panthor: Support partial unmaps of huge pages")
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> Reviewed-by: Akash Goel <akash.goel@arm.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Steven Price <stevne.price@arm.com>

Although there's a minor style issue below if you do respin (see below).

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 44 ++++++++++++++++++++++++++++-------
>  1 file changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index b0a7033480e6..6cef954e2cba 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2299,7 +2299,6 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  {
>  	struct panthor_gem_object *bo = to_panthor_bo(mapping->gem.obj);
>  	u64 aligned_va = ALIGN_DOWN(va, SZ_2M);
> -	const struct page *pg;
>  	pgoff_t bo_offset;
>  
>  	/* If the 2M-aligned VA is outside the mapping being tested, we know
> @@ -2309,16 +2308,45 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  		return false;
>  
>  	bo_offset = aligned_va - mapping->va.addr + mapping->gem.offset;
> -	pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>  
> -	/* In case of shmem backing, we know we can only have a huge mapping
> -	 * if the bo_offset is 2M aligned, meaning we can skip the folio size
> -	 * check if it's not the case.
> -	 */
> -	if (!IS_ALIGNED(bo_offset, SZ_2M))
> +	if (drm_gem_is_imported(&bo->base)) {
> +		struct sg_table *sgt = bo->dmap.sgt;
> +		struct scatterlist *sgl;
> +		unsigned int count;
> +
> +		/* If this is an imported BO, we have to walk the SGT and
> +		 * check the dma address/size alignment to determine if it's
> +		 * a huge map or not.
> +		 */
> +		for_each_sgtable_dma_sg(sgt, sgl, count) {
> +			size_t len = sg_dma_len(sgl);
> +			dma_addr_t daddr;
> +
> +			if (len <= bo_offset) {
> +				bo_offset -= len;
> +				continue;
> +			}
> +
> +			len -= bo_offset;
> +			daddr = sg_dma_address(sgl) + bo_offset;
> +
> +			return IS_ALIGNED(daddr, SZ_2M) &&
> +			       len >= SZ_2M;
> +		}
> +
>  		return false;
> +	} else {

This 'else' branch isn't needed - since the above branch returns early.
Dropping the below back out of the braces would make the diff easier to
read - effectively the drm_gem_is_imported() branch is just added and
the rest of the code is left as is.

Thanks,
Steve

> +		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>  
> -	return folio_size(page_folio(pg)) >= SZ_2M;
> +		/* In case of shmem backing, we know we can only have a huge mapping
> +		 * if the bo_offset is 2M aligned, meaning we can skip the folio size
> +		 * check if it's not the case.
> +		 */
> +		if (!IS_ALIGNED(bo_offset, SZ_2M))
> +			return false;
> +
> +		return folio_size(page_folio(pg)) >= SZ_2M;
> +	}
>  }
>  
>  static void
> 

[PATCH v2 3/5] drm/panthor: Consolidate the is-huge-page-mapping test

[Boris Brezillon]

Right now the logic to determine whether a given VA in the drm_gpuva
being unmapped is a huge page mapping or not is scattered
in two functions: unmap_hugepage_align() and
iova_mapped_as_huge_page(). This makes it harder to reason about the
logic being implemented for very little gain (some simple checks being
done twice), so let's consolidate all the checks related to huge page
mapping testing in iova_mapped_as_huge_page() and leave
unmap_hugepage_align() as a simple user of this helper that aligns the
area to unmap based on the return of iova_mapped_as_huge_page().

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Akash Goel <akash.goel@arm.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 57 +++++++++++++++++------------------
 1 file changed, 28 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 6cef954e2cba..d2897099763e 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2301,10 +2301,11 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
 	u64 aligned_va = ALIGN_DOWN(va, SZ_2M);
 	pgoff_t bo_offset;
 
-	/* If the 2M-aligned VA is outside the mapping being tested, we know
-	 * it's not a huge map.
+	/* If the 2M section being tested is crossing the mapping boundary
+	 * we know it's not a huge map.
 	 */
-	if (aligned_va < mapping->va.addr)
+	if (aligned_va < mapping->va.addr ||
+	    aligned_va + SZ_2M > mapping->va.addr + mapping->va.range)
 		return false;
 
 	bo_offset = aligned_va - mapping->va.addr + mapping->gem.offset;
@@ -2337,10 +2338,21 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
 		return false;
 	} else {
 		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
+		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
+		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
 
-		/* In case of shmem backing, we know we can only have a huge mapping
-		 * if the bo_offset is 2M aligned, meaning we can skip the folio size
-		 * check if it's not the case.
+		/* If the unmapped VMA stands for a sparse mapping, always
+		 * assume the backing storage is a THP, since the overhead of
+		 * unmapping 2MiB worth of 4KiB pages and remapping some of
+		 * them is offset by the logic of working out whether it's
+		 * the opposite case right below.
+		 */
+		if (is_sparse)
+			return true;
+
+		/* In case of shmem backing, we know we can only have a huge
+		 * mapping if the bo_offset is 2M aligned, meaning we can skip
+		 * the folio size check if it's not the case.
 		 */
 		if (!IS_ALIGNED(bo_offset, SZ_2M))
 			return false;
@@ -2353,36 +2365,23 @@ static void
 unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
 		     u64 *unmap_start, u64 *unmap_range)
 {
-	struct panthor_vma *unmap_vma = container_of(op->unmap->va, struct panthor_vma, base);
-	bool is_sparse = unmap_vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
-	u64 aligned_unmap_start, aligned_unmap_end, unmap_end;
-
-	unmap_end = *unmap_start + *unmap_range;
-	aligned_unmap_start = ALIGN_DOWN(*unmap_start, SZ_2M);
-	aligned_unmap_end = ALIGN(unmap_end, SZ_2M);
+	u64 unmap_end = *unmap_start + *unmap_range;
 
 	/* If we're dealing with a huge page, make sure the unmap region is
-	 * aligned on the start of the page. If the unmapped VMA stands for
-	 * a sparse mapping, always assume the backing storage is a THP, since
-	 * the overhead of unmapping 2MiB worth of 4KiB pages and remapping
-	 * some of them is offset by the logic of working out whether it's
-	 * the opposite case right below. This also holds true for op->next.
+	 * aligned on the start of the page.
 	 */
-	if (op->prev && aligned_unmap_start < *unmap_start &&
-	    op->prev->va.addr <= aligned_unmap_start &&
-	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, *unmap_start))) {
-		*unmap_range += *unmap_start - aligned_unmap_start;
-		*unmap_start = aligned_unmap_start;
-	}
+	if (op->prev && !IS_ALIGNED(*unmap_start, SZ_2M) &&
+	    iova_mapped_as_huge_page(op->unmap->va, *unmap_start))
+		*unmap_start = ALIGN_DOWN(*unmap_start, SZ_2M);
 
 	/* If we're dealing with a huge page, make sure the unmap region is
 	 * aligned on the end of the page.
 	 */
-	if (op->next && aligned_unmap_end > unmap_end &&
-	    op->next->va.addr + op->next->va.range >= aligned_unmap_end &&
-	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, unmap_end - 1))) {
-		*unmap_range += aligned_unmap_end - unmap_end;
-	}
+	if (op->next && !IS_ALIGNED(unmap_end, SZ_2M) &&
+	    iova_mapped_as_huge_page(op->unmap->va, unmap_end - 1))
+		unmap_end = ALIGN(unmap_end, SZ_2M);
+
+	*unmap_range = unmap_end - *unmap_start;
 }
 
 static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,

-- 
2.55.0

Re: [PATCH v2 3/5] drm/panthor: Consolidate the is-huge-page-mapping test

[Steven Price]

On 24/09/2026 12:04, Boris Brezillon wrote:
> Right now the logic to determine whether a given VA in the drm_gpuva
> being unmapped is a huge page mapping or not is scattered
> in two functions: unmap_hugepage_align() and
> iova_mapped_as_huge_page(). This makes it harder to reason about the
> logic being implemented for very little gain (some simple checks being
> done twice), so let's consolidate all the checks related to huge page
> mapping testing in iova_mapped_as_huge_page() and leave
> unmap_hugepage_align() as a simple user of this helper that aligns the
> area to unmap based on the return of iova_mapped_as_huge_page().
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> Reviewed-by: Akash Goel <akash.goel@arm.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 57 +++++++++++++++++------------------
>  1 file changed, 28 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 6cef954e2cba..d2897099763e 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2301,10 +2301,11 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  	u64 aligned_va = ALIGN_DOWN(va, SZ_2M);
>  	pgoff_t bo_offset;
>  
> -	/* If the 2M-aligned VA is outside the mapping being tested, we know
> -	 * it's not a huge map.
> +	/* If the 2M section being tested is crossing the mapping boundary
> +	 * we know it's not a huge map.
>  	 */
> -	if (aligned_va < mapping->va.addr)
> +	if (aligned_va < mapping->va.addr ||
> +	    aligned_va + SZ_2M > mapping->va.addr + mapping->va.range)
>  		return false;
>  
>  	bo_offset = aligned_va - mapping->va.addr + mapping->gem.offset;
> @@ -2337,10 +2338,21 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  		return false;
>  	} else {
>  		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
> +		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
> +		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
>  
> -		/* In case of shmem backing, we know we can only have a huge mapping
> -		 * if the bo_offset is 2M aligned, meaning we can skip the folio size
> -		 * check if it's not the case.
> +		/* If the unmapped VMA stands for a sparse mapping, always
> +		 * assume the backing storage is a THP, since the overhead of
> +		 * unmapping 2MiB worth of 4KiB pages and remapping some of
> +		 * them is offset by the logic of working out whether it's
> +		 * the opposite case right below.
> +		 */
> +		if (is_sparse)
> +			return true;
> +
> +		/* In case of shmem backing, we know we can only have a huge
> +		 * mapping if the bo_offset is 2M aligned, meaning we can skip
> +		 * the folio size check if it's not the case.
>  		 */
>  		if (!IS_ALIGNED(bo_offset, SZ_2M))
>  			return false;
> @@ -2353,36 +2365,23 @@ static void
>  unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
>  		     u64 *unmap_start, u64 *unmap_range)
>  {
> -	struct panthor_vma *unmap_vma = container_of(op->unmap->va, struct panthor_vma, base);
> -	bool is_sparse = unmap_vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
> -	u64 aligned_unmap_start, aligned_unmap_end, unmap_end;
> -
> -	unmap_end = *unmap_start + *unmap_range;
> -	aligned_unmap_start = ALIGN_DOWN(*unmap_start, SZ_2M);
> -	aligned_unmap_end = ALIGN(unmap_end, SZ_2M);
> +	u64 unmap_end = *unmap_start + *unmap_range;
>  
>  	/* If we're dealing with a huge page, make sure the unmap region is
> -	 * aligned on the start of the page. If the unmapped VMA stands for
> -	 * a sparse mapping, always assume the backing storage is a THP, since
> -	 * the overhead of unmapping 2MiB worth of 4KiB pages and remapping
> -	 * some of them is offset by the logic of working out whether it's
> -	 * the opposite case right below. This also holds true for op->next.
> +	 * aligned on the start of the page.
>  	 */
> -	if (op->prev && aligned_unmap_start < *unmap_start &&
> -	    op->prev->va.addr <= aligned_unmap_start &&
> -	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, *unmap_start))) {
> -		*unmap_range += *unmap_start - aligned_unmap_start;
> -		*unmap_start = aligned_unmap_start;
> -	}
> +	if (op->prev && !IS_ALIGNED(*unmap_start, SZ_2M) &&
> +	    iova_mapped_as_huge_page(op->unmap->va, *unmap_start))
> +		*unmap_start = ALIGN_DOWN(*unmap_start, SZ_2M);
>  
>  	/* If we're dealing with a huge page, make sure the unmap region is
>  	 * aligned on the end of the page.
>  	 */
> -	if (op->next && aligned_unmap_end > unmap_end &&
> -	    op->next->va.addr + op->next->va.range >= aligned_unmap_end &&
> -	    (is_sparse || iova_mapped_as_huge_page(op->unmap->va, unmap_end - 1))) {
> -		*unmap_range += aligned_unmap_end - unmap_end;
> -	}
> +	if (op->next && !IS_ALIGNED(unmap_end, SZ_2M) &&
> +	    iova_mapped_as_huge_page(op->unmap->va, unmap_end - 1))
> +		unmap_end = ALIGN(unmap_end, SZ_2M);
> +
> +	*unmap_range = unmap_end - *unmap_start;
>  }
>  
>  static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
> 

[PATCH v2 4/5] drm/panthor: Actually check huge-page mapping on sparse regions

[Boris Brezillon]

With the recent changes to iova_mapped_as_huge_page(), the check for
huge-page mapping of sparse BOs is actually simple:

- for a sparse mapping, we know the BO offset any VA in this regions is
  va & (SZ_2M - 1)
- the VA we're searching the BO offset for is the 2M-aligned
  aligned_va value

This guarantees that the BO offset to check is always zero in that case.

This is simple enough to let the code check if page 0 is a huge page
and save the unmap+map dance when the dummy BO is not backed by a
a huge page. So let's do that and kill the comment that says it's too
complicated.

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Akash Goel <akash.goel@arm.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index d2897099763e..01564d250adf 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2337,18 +2337,18 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
 
 		return false;
 	} else {
-		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
 		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
 		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
+		const struct page *pg;
 
-		/* If the unmapped VMA stands for a sparse mapping, always
-		 * assume the backing storage is a THP, since the overhead of
-		 * unmapping 2MiB worth of 4KiB pages and remapping some of
-		 * them is offset by the logic of working out whether it's
-		 * the opposite case right below.
+		/* BO offset on a sparse mapping is chosen so that 2M-aligned
+		 * VAs point to the start of the BO. Since aligned_va (the
+		 * address we check huge-page against) is 2M-aligned, the BO
+		 * offset is guaranteed to be zero.
+		 * Check panthor_fix_sparse_map_offset() for more details.
 		 */
 		if (is_sparse)
-			return true;
+			bo_offset = 0;
 
 		/* In case of shmem backing, we know we can only have a huge
 		 * mapping if the bo_offset is 2M aligned, meaning we can skip
@@ -2357,6 +2357,7 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
 		if (!IS_ALIGNED(bo_offset, SZ_2M))
 			return false;
 
+		pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
 		return folio_size(page_folio(pg)) >= SZ_2M;
 	}
 }

-- 
2.55.0

Re: [PATCH v2 4/5] drm/panthor: Actually check huge-page mapping on sparse regions

[Adrián Larumbe]

On 24.09.2026 13:04, Boris Brezillon wrote:
> With the recent changes to iova_mapped_as_huge_page(), the check for
> huge-page mapping of sparse BOs is actually simple:
> 
> - for a sparse mapping, we know the BO offset any VA in this regions is
>   va & (SZ_2M - 1)
> - the VA we're searching the BO offset for is the 2M-aligned
>   aligned_va value
> 
> This guarantees that the BO offset to check is always zero in that case.
> 
> This is simple enough to let the code check if page 0 is a huge page
> and save the unmap+map dance when the dummy BO is not backed by a
> a huge page. So let's do that and kill the comment that says it's too
> complicated.

I've been thinking about the kind of IGT test I could write to validate this series. Other than maybe
expanding igt_panthor_vm_bind_sparse() so that it can take an offset argument, I cannot think of anything
else, because whether the dummy BO's backing pages form a THP or unmapping a subset of a sparse VA causes
unmap boundaries to be expanded is transparent to UM.

Maybe adding a specific module param that the IGT test would trigger to show debug information relevant
to the above? But it seems a bit of an overkill to me.

> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> Reviewed-by: Akash Goel <akash.goel@arm.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index d2897099763e..01564d250adf 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2337,18 +2337,18 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  
>  		return false;
>  	} else {
> -		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>  		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
>  		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
> +		const struct page *pg;
>  
> -		/* If the unmapped VMA stands for a sparse mapping, always
> -		 * assume the backing storage is a THP, since the overhead of
> -		 * unmapping 2MiB worth of 4KiB pages and remapping some of
> -		 * them is offset by the logic of working out whether it's
> -		 * the opposite case right below.
> +		/* BO offset on a sparse mapping is chosen so that 2M-aligned
> +		 * VAs point to the start of the BO. Since aligned_va (the
> +		 * address we check huge-page against) is 2M-aligned, the BO
> +		 * offset is guaranteed to be zero.
> +		 * Check panthor_fix_sparse_map_offset() for more details.
>  		 */
>  		if (is_sparse)
> -			return true;
> +			bo_offset = 0;
>  
>  		/* In case of shmem backing, we know we can only have a huge
>  		 * mapping if the bo_offset is 2M aligned, meaning we can skip
> @@ -2357,6 +2357,7 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  		if (!IS_ALIGNED(bo_offset, SZ_2M))
>  			return false;
>  
> +		pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>  		return folio_size(page_folio(pg)) >= SZ_2M;
>  	}
>  }
> 
> -- 
> 2.55.0


Adrian Larumbe

Re: [PATCH v2 4/5] drm/panthor: Actually check huge-page mapping on sparse regions

[Steven Price]

On 24/09/2026 12:04, Boris Brezillon wrote:
> With the recent changes to iova_mapped_as_huge_page(), the check for
> huge-page mapping of sparse BOs is actually simple:
> 
> - for a sparse mapping, we know the BO offset any VA in this regions is
>   va & (SZ_2M - 1)
> - the VA we're searching the BO offset for is the 2M-aligned
>   aligned_va value
> 
> This guarantees that the BO offset to check is always zero in that case.
> 
> This is simple enough to let the code check if page 0 is a huge page
> and save the unmap+map dance when the dummy BO is not backed by a
> a huge page. So let's do that and kill the comment that says it's too
> complicated.
> 
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> Reviewed-by: Akash Goel <akash.goel@arm.com>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

In itself I can't see anything wrong with this change, so:

Reviewed-by: Steven Price <steven.price@arm.com>

However...

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index d2897099763e..01564d250adf 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2337,18 +2337,18 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  
>  		return false;
>  	} else {
> -		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>  		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
>  		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
> +		const struct page *pg;
>  
> -		/* If the unmapped VMA stands for a sparse mapping, always
> -		 * assume the backing storage is a THP, since the overhead of
> -		 * unmapping 2MiB worth of 4KiB pages and remapping some of
> -		 * them is offset by the logic of working out whether it's
> -		 * the opposite case right below.
> +		/* BO offset on a sparse mapping is chosen so that 2M-aligned
> +		 * VAs point to the start of the BO. Since aligned_va (the
> +		 * address we check huge-page against) is 2M-aligned, the BO
> +		 * offset is guaranteed to be zero.
> +		 * Check panthor_fix_sparse_map_offset() for more details.
>  		 */
>  		if (is_sparse)
> -			return true;
> +			bo_offset = 0;
>  
>  		/* In case of shmem backing, we know we can only have a huge
>  		 * mapping if the bo_offset is 2M aligned, meaning we can skip
> @@ -2357,6 +2357,7 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>  		if (!IS_ALIGNED(bo_offset, SZ_2M))
>  			return false;
>  
> +		pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>  		return folio_size(page_folio(pg)) >= SZ_2M;

... this seems like it could be problematic. On the mapping side we use
the scatter list to decide whether the region is huge page mapped or
not. The scatter list code can merge segments that are contiguous (see
pages_are_mergeable()), so if we have a region which has small folios we
fail this check even though the pages might have been mapped as huge pages.

This is a problem on the non-sparse path as well (hence not really
related to this patch). I'm not really sure how to test this though - I
may well have overlooked something here.

Thanks,
Steve

>  	}
>  }
> 

Re: [PATCH v2 4/5] drm/panthor: Actually check huge-page mapping on sparse regions

[Boris Brezillon]

On Mon, 5 Oct 2026 12:03:26 +0100
Steven Price <steven.price@arm.com> wrote:

> On 24/09/2026 12:04, Boris Brezillon wrote:
> > With the recent changes to iova_mapped_as_huge_page(), the check for
> > huge-page mapping of sparse BOs is actually simple:
> > 
> > - for a sparse mapping, we know the BO offset any VA in this regions is
> >   va & (SZ_2M - 1)
> > - the VA we're searching the BO offset for is the 2M-aligned
> >   aligned_va value
> > 
> > This guarantees that the BO offset to check is always zero in that case.
> > 
> > This is simple enough to let the code check if page 0 is a huge page
> > and save the unmap+map dance when the dummy BO is not backed by a
> > a huge page. So let's do that and kill the comment that says it's too
> > complicated.
> > 
> > Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > Reviewed-by: Akash Goel <akash.goel@arm.com>
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>  
> 
> In itself I can't see anything wrong with this change, so:
> 
> Reviewed-by: Steven Price <steven.price@arm.com>
> 
> However...
> 
> > ---
> >  drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> > index d2897099763e..01564d250adf 100644
> > --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> > @@ -2337,18 +2337,18 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
> >  
> >  		return false;
> >  	} else {
> > -		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
> >  		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
> >  		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
> > +		const struct page *pg;
> >  
> > -		/* If the unmapped VMA stands for a sparse mapping, always
> > -		 * assume the backing storage is a THP, since the overhead of
> > -		 * unmapping 2MiB worth of 4KiB pages and remapping some of
> > -		 * them is offset by the logic of working out whether it's
> > -		 * the opposite case right below.
> > +		/* BO offset on a sparse mapping is chosen so that 2M-aligned
> > +		 * VAs point to the start of the BO. Since aligned_va (the
> > +		 * address we check huge-page against) is 2M-aligned, the BO
> > +		 * offset is guaranteed to be zero.
> > +		 * Check panthor_fix_sparse_map_offset() for more details.
> >  		 */
> >  		if (is_sparse)
> > -			return true;
> > +			bo_offset = 0;
> >  
> >  		/* In case of shmem backing, we know we can only have a huge
> >  		 * mapping if the bo_offset is 2M aligned, meaning we can skip
> > @@ -2357,6 +2357,7 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
> >  		if (!IS_ALIGNED(bo_offset, SZ_2M))
> >  			return false;
> >  
> > +		pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
> >  		return folio_size(page_folio(pg)) >= SZ_2M;  
> 
> ... this seems like it could be problematic. On the mapping side we use
> the scatter list to decide whether the region is huge page mapped or
> not. The scatter list code can merge segments that are contiguous (see
> pages_are_mergeable()), so if we have a region which has small folios we
> fail this check even though the pages might have been mapped as huge pages.
> 
> This is a problem on the non-sparse path as well (hence not really
> related to this patch). I'm not really sure how to test this though - I
> may well have overlooked something here.

So, this is based on the assumption that shmem backing is allocated
with the buddy allocator, and because of how this allocator splits
bigger order blocks to service smaller allocations, it's my
understanding that two consecutive folios of the same size/order can't
be physically contiguous.

Re: [PATCH v2 4/5] drm/panthor: Actually check huge-page mapping on sparse regions

[Steven Price]

On 05/10/2026 12:53, Boris Brezillon wrote:
> On Mon, 5 Oct 2026 12:03:26 +0100
> Steven Price <steven.price@arm.com> wrote:
> 
>> On 24/09/2026 12:04, Boris Brezillon wrote:
>>> With the recent changes to iova_mapped_as_huge_page(), the check for
>>> huge-page mapping of sparse BOs is actually simple:
>>>
>>> - for a sparse mapping, we know the BO offset any VA in this regions is
>>>   va & (SZ_2M - 1)
>>> - the VA we're searching the BO offset for is the 2M-aligned
>>>   aligned_va value
>>>
>>> This guarantees that the BO offset to check is always zero in that case.
>>>
>>> This is simple enough to let the code check if page 0 is a huge page
>>> and save the unmap+map dance when the dummy BO is not backed by a
>>> a huge page. So let's do that and kill the comment that says it's too
>>> complicated.
>>>
>>> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
>>> Reviewed-by: Akash Goel <akash.goel@arm.com>
>>> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>  
>>
>> In itself I can't see anything wrong with this change, so:
>>
>> Reviewed-by: Steven Price <steven.price@arm.com>
>>
>> However...
>>
>>> ---
>>>  drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++-------
>>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
>>> index d2897099763e..01564d250adf 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
>>> @@ -2337,18 +2337,18 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>>>  
>>>  		return false;
>>>  	} else {
>>> -		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>>>  		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
>>>  		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
>>> +		const struct page *pg;
>>>  
>>> -		/* If the unmapped VMA stands for a sparse mapping, always
>>> -		 * assume the backing storage is a THP, since the overhead of
>>> -		 * unmapping 2MiB worth of 4KiB pages and remapping some of
>>> -		 * them is offset by the logic of working out whether it's
>>> -		 * the opposite case right below.
>>> +		/* BO offset on a sparse mapping is chosen so that 2M-aligned
>>> +		 * VAs point to the start of the BO. Since aligned_va (the
>>> +		 * address we check huge-page against) is 2M-aligned, the BO
>>> +		 * offset is guaranteed to be zero.
>>> +		 * Check panthor_fix_sparse_map_offset() for more details.
>>>  		 */
>>>  		if (is_sparse)
>>> -			return true;
>>> +			bo_offset = 0;
>>>  
>>>  		/* In case of shmem backing, we know we can only have a huge
>>>  		 * mapping if the bo_offset is 2M aligned, meaning we can skip
>>> @@ -2357,6 +2357,7 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>>>  		if (!IS_ALIGNED(bo_offset, SZ_2M))
>>>  			return false;
>>>  
>>> +		pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>>>  		return folio_size(page_folio(pg)) >= SZ_2M;  
>>
>> ... this seems like it could be problematic. On the mapping side we use
>> the scatter list to decide whether the region is huge page mapped or
>> not. The scatter list code can merge segments that are contiguous (see
>> pages_are_mergeable()), so if we have a region which has small folios we
>> fail this check even though the pages might have been mapped as huge pages.
>>
>> This is a problem on the non-sparse path as well (hence not really
>> related to this patch). I'm not really sure how to test this though - I
>> may well have overlooked something here.
> 
> So, this is based on the assumption that shmem backing is allocated
> with the buddy allocator, and because of how this allocator splits
> bigger order blocks to service smaller allocations, it's my
> understanding that two consecutive folios of the same size/order can't
> be physically contiguous.

Yes, you'd expect the buddy allocator to combine the folios back into a
larger one if they were contiguous. I guess we should be safe, at least
for now. I still feel it's unnecessarily fragile and complex trying to
work out whether we've mapped as a huge page or not. But I don't
actually have a better solution at the moment, and this series is at
least improving things. I'll put it on my todo list to look at later.

Thanks,
Steve

Re: [PATCH v2 4/5] drm/panthor: Actually check huge-page mapping on sparse regions

[Boris Brezillon]

On Mon, 5 Oct 2026 16:31:09 +0100
Steven Price <steven.price@arm.com> wrote:

> On 05/10/2026 12:53, Boris Brezillon wrote:
> > On Mon, 5 Oct 2026 12:03:26 +0100
> > Steven Price <steven.price@arm.com> wrote:
> >   
> >> On 24/09/2026 12:04, Boris Brezillon wrote:  
> >>> With the recent changes to iova_mapped_as_huge_page(), the check for
> >>> huge-page mapping of sparse BOs is actually simple:
> >>>
> >>> - for a sparse mapping, we know the BO offset any VA in this regions is
> >>>   va & (SZ_2M - 1)
> >>> - the VA we're searching the BO offset for is the 2M-aligned
> >>>   aligned_va value
> >>>
> >>> This guarantees that the BO offset to check is always zero in that case.
> >>>
> >>> This is simple enough to let the code check if page 0 is a huge page
> >>> and save the unmap+map dance when the dummy BO is not backed by a
> >>> a huge page. So let's do that and kill the comment that says it's too
> >>> complicated.
> >>>
> >>> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> >>> Reviewed-by: Akash Goel <akash.goel@arm.com>
> >>> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>    
> >>
> >> In itself I can't see anything wrong with this change, so:
> >>
> >> Reviewed-by: Steven Price <steven.price@arm.com>
> >>
> >> However...
> >>  
> >>> ---
> >>>  drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++-------
> >>>  1 file changed, 8 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> >>> index d2897099763e..01564d250adf 100644
> >>> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> >>> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> >>> @@ -2337,18 +2337,18 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
> >>>  
> >>>  		return false;
> >>>  	} else {
> >>> -		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
> >>>  		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
> >>>  		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
> >>> +		const struct page *pg;
> >>>  
> >>> -		/* If the unmapped VMA stands for a sparse mapping, always
> >>> -		 * assume the backing storage is a THP, since the overhead of
> >>> -		 * unmapping 2MiB worth of 4KiB pages and remapping some of
> >>> -		 * them is offset by the logic of working out whether it's
> >>> -		 * the opposite case right below.
> >>> +		/* BO offset on a sparse mapping is chosen so that 2M-aligned
> >>> +		 * VAs point to the start of the BO. Since aligned_va (the
> >>> +		 * address we check huge-page against) is 2M-aligned, the BO
> >>> +		 * offset is guaranteed to be zero.
> >>> +		 * Check panthor_fix_sparse_map_offset() for more details.
> >>>  		 */
> >>>  		if (is_sparse)
> >>> -			return true;
> >>> +			bo_offset = 0;
> >>>  
> >>>  		/* In case of shmem backing, we know we can only have a huge
> >>>  		 * mapping if the bo_offset is 2M aligned, meaning we can skip
> >>> @@ -2357,6 +2357,7 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
> >>>  		if (!IS_ALIGNED(bo_offset, SZ_2M))
> >>>  			return false;
> >>>  
> >>> +		pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
> >>>  		return folio_size(page_folio(pg)) >= SZ_2M;    
> >>
> >> ... this seems like it could be problematic. On the mapping side we use
> >> the scatter list to decide whether the region is huge page mapped or
> >> not. The scatter list code can merge segments that are contiguous (see
> >> pages_are_mergeable()), so if we have a region which has small folios we
> >> fail this check even though the pages might have been mapped as huge pages.
> >>
> >> This is a problem on the non-sparse path as well (hence not really
> >> related to this patch). I'm not really sure how to test this though - I
> >> may well have overlooked something here.  
> > 
> > So, this is based on the assumption that shmem backing is allocated
> > with the buddy allocator, and because of how this allocator splits
> > bigger order blocks to service smaller allocations, it's my
> > understanding that two consecutive folios of the same size/order can't
> > be physically contiguous.  
> 
> Yes, you'd expect the buddy allocator to combine the folios back into a
> larger one if they were contiguous. I guess we should be safe, at least
> for now. I still feel it's unnecessarily fragile and complex trying to
> work out whether we've mapped as a huge page or not. But I don't
> actually have a better solution at the moment, and this series is at
> least improving things. I'll put it on my todo list to look at later.

One way to make that consistent would be to have the same logic on the
map_pages() side, where we'd use the pages array to check for physical
contiguity in addition to the dma_addr/size-based checks we already
have. This being said, this might be moot if we consider moving to
our own PT implementation with full sub-tree updates (see how [1]
doesn't have a get_pgsize() anymore). In the meantime, I can add a
comment explaining the assumptions at play here.

[1]https://gitlab.freedesktop.org/bbrezillon/linux/-/commits/b4/panthor-part-ways-with-iopgtbl?ref_type=heads

[PATCH v2 5/5] drm/panthor: Remove redundant panthor_fix_sparse_map_offset() call

[Boris Brezillon]

op->remap.next->gem.offset is adjusted twice in
panthor_gpuva_sm_step_remap() (once on the remap.next object, and once on
the local map_op). Let's do it only once, and move this adjusment closer
to the place it matters.

Reported-by: Akash Goel <akash.goel@arm.com>
Closes: https://lore.kernel.org/dri-devel/b85679e3-5ca2-4d36-8675-2b968d97cb5b@arm.com/
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 01564d250adf..d8fb29ace5c4 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2397,11 +2397,6 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 
 	drm_gpuva_op_remap_to_unmap_range(&op->remap, &unmap_start, &unmap_range);
 
-	/* op->remap.prev's BO offset is always the same as the unmap va's, but
-	 * that of op->remap.next must be adjusted so as to remain < SZ_2M
-	 */
-	panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags);
-
 	if (!unmap_vma->evicted) {
 		/*
 		 * ARM IOMMU page table management code disallows partial unmaps of huge pages,
@@ -2447,6 +2442,12 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 		u64 addr = op->remap.next->va.addr;
 		u64 size = unmap_start + unmap_range - op->remap.next->va.addr;
 
+		/* op->remap.prev's BO offset is always the same as the unmap
+		 * va's, but that of op->remap.next must be adjusted so as to
+		 * remain < SZ_2M
+		 */
+		panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags);
+
 		if (!unmap_vma->evicted && size > 0) {
 			struct drm_gpuva_op_map map_op = {
 				.va.addr = addr,
@@ -2454,7 +2455,6 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
 				.gem.obj = op->remap.next->gem.obj,
 				.gem.offset = op->remap.next->gem.offset,
 			};
-			panthor_fix_sparse_map_offset(&map_op, unmap_vma->flags);
 
 			ret = panthor_vm_exec_map_op(vm, unmap_vma->flags, &map_op);
 			if (ret)

-- 
2.55.0

Re: [PATCH v2 5/5] drm/panthor: Remove redundant panthor_fix_sparse_map_offset() call

[Steven Price]

On 24/09/2026 12:04, Boris Brezillon wrote:
> op->remap.next->gem.offset is adjusted twice in
> panthor_gpuva_sm_step_remap() (once on the remap.next object, and once on
> the local map_op). Let's do it only once, and move this adjusment closer
> to the place it matters.
> 
> Reported-by: Akash Goel <akash.goel@arm.com>
> Closes: https://lore.kernel.org/dri-devel/b85679e3-5ca2-4d36-8675-2b968d97cb5b@arm.com/
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  drivers/gpu/drm/panthor/panthor_mmu.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 01564d250adf..d8fb29ace5c4 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2397,11 +2397,6 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
>  
>  	drm_gpuva_op_remap_to_unmap_range(&op->remap, &unmap_start, &unmap_range);
>  
> -	/* op->remap.prev's BO offset is always the same as the unmap va's, but
> -	 * that of op->remap.next must be adjusted so as to remain < SZ_2M
> -	 */
> -	panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags);
> -
>  	if (!unmap_vma->evicted) {
>  		/*
>  		 * ARM IOMMU page table management code disallows partial unmaps of huge pages,
> @@ -2447,6 +2442,12 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
>  		u64 addr = op->remap.next->va.addr;
>  		u64 size = unmap_start + unmap_range - op->remap.next->va.addr;
>  
> +		/* op->remap.prev's BO offset is always the same as the unmap
> +		 * va's, but that of op->remap.next must be adjusted so as to
> +		 * remain < SZ_2M
> +		 */
> +		panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags);
> +
>  		if (!unmap_vma->evicted && size > 0) {
>  			struct drm_gpuva_op_map map_op = {
>  				.va.addr = addr,
> @@ -2454,7 +2455,6 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
>  				.gem.obj = op->remap.next->gem.obj,
>  				.gem.offset = op->remap.next->gem.offset,
>  			};
> -			panthor_fix_sparse_map_offset(&map_op, unmap_vma->flags);
>  
>  			ret = panthor_vm_exec_map_op(vm, unmap_vma->flags, &map_op);
>  			if (ret)
> 

Re: [PATCH v2 0/5] drm/panthor: Fix partial unmaps, again

[Adrián Larumbe]

On Thu, 24 Sep 2026 13:04:37 +0200, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> drm/panthor: Fix partial unmaps, again
> 
> The first patch is just a theoretical perf improvement to avoid
> unnecessary unmap+map caused by false positive returned by
> iova_mapped_as_huge_page(). The second patch is a fix for a real
> bug when iova_mapped_as_huge_page() is called on an imported BO.
> The last 2 patches are just consolidating the logic to make things
> clearer.
> 
> [...]

Reviewed-by: Adrián Larumbe <adrian.larumbe@collabora.com>

-- 
Adrián Larumbe <adrian.larumbe@collabora.com>