Re: [PATCH] drm/panthor: Fix NPD issue on partial unmap of an evicted BO
From: Akash Goel
Date: Tue Jun 23 2026 - 08:42:36 EST
On 6/23/26 13:09, Boris Brezillon wrote:
On Tue, 23 Jun 2026 12:17:51 +0100
Akash Goel <akash.goel@xxxxxxx> wrote:
Hi Boris
On 6/23/26 10:53, Boris Brezillon wrote:
On Tue, 23 Jun 2026 10:24:13 +0100
Akash Goel <akash.goel@xxxxxxx> wrote:
This commit fixes the NULL pointer dereference issue that would have
happened on the split of GPU mapping due to partial unmap of an evicted
BO. There is a logic to handle the partial unmap of huge pages when the
GPU mapping is split. That logic was not being completely skipped for
the VMA of an evicted BO and that resulted in a NPD possibility for the
'bo->backing.pages' pointer, which is set to NULL when pages of a
BO are released on eviction.
Fixes: 8e7460eac786 ("drm/panthor: Support partial unmaps of huge pages")
Signed-off-by: Akash Goel <akash.goel@xxxxxxx>
---
drivers/gpu/drm/panthor/panthor_mmu.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 31cc57029c12..285e7b9bc100 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2358,20 +2358,20 @@ static int panthor_gpuva_sm_step_remap(struct drm_gpuva_op *op,
*/
panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags);
- /*
- * ARM IOMMU page table management code disallows partial unmaps of huge pages,
- * so when a partial unmap is requested, we must first unmap the entire huge
- * page and then remap the difference between the huge page minus the requested
- * unmap region. Calculating the right start address and range for the expanded
- * unmap operation is the responsibility of the following function.
- */
- unmap_hugepage_align(&op->remap, &unmap_start, &unmap_range);
-
- /* If the range changed, we might have to lock a wider region to guarantee
- * atomicity. panthor_vm_lock_region() bails out early if the new region
- * is already part of the locked region, so no need to do this check here.
- */
if (!unmap_vma->evicted) {
+ /*
+ * ARM IOMMU page table management code disallows partial unmaps of huge pages,
+ * so when a partial unmap is requested, we must first unmap the entire huge
+ * page and then remap the difference between the huge page minus the requested
+ * unmap region. Calculating the right start address and range for the expanded
+ * unmap operation is the responsibility of the following function.
+ */
+ unmap_hugepage_align(&op->remap, &unmap_start, &unmap_range);
+
+ /* If the range changed, we might have to lock a wider region to guarantee
+ * atomicity. panthor_vm_lock_region() bails out early if the new region
+ * is already part of the locked region, so no need to do this check here.
+ */
panthor_vm_lock_region(vm, unmap_start, unmap_range);
panthor_vm_unmap_pages(vm, unmap_start, unmap_range);
}
I think we want something like that instead, so we can keep the
2M alignment for sparse mappings which go recently introduced.
Thanks for the suggestion. But sorry I didn't get it.
I see that the patching of 'op->remap.next->gem.offset' would still be
done with my change.
panthor_fix_sparse_map_offset(op->remap.next, unmap_vma->flags);
if (!unmap_vma->evicted) {
unmap_hugepage_align(&op->remap, &unmap_start,
IIUC, the 2M alignment is done to avoid a potential partial unmap of 2M
page. But if the VMA is in evicted state then already the unmap would
have happened for the whole virtual range covered by the VMA.
Nah, you're correct, the patching of the drm_gpuva is independent of the
adjusted unmap range, so we should be good even if we don't adjust this
range for evicted sparse mappings. Sorry for the noise.
No worries. Thanks for confirming.
Since I had a closer look at the code, sorry I have another doubt.
Do we really need the call to 'panthor_fix_sparse_map_offset()' in the following code block ?. The 'op->remap.next->gem.offset' would already have been patched before.
if (op->remap.next) {
u64 addr = op->remap.next->va.addr;
u64 size = unmap_start + unmap_range - op->remap.next->va.addr;
if (!unmap_vma->evicted && size > 0) {
struct drm_gpuva_op_map map_op = {
.va.addr = addr,
.va.range = size,
.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);
Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
Sorry I realized that indentation needs to be fixed in my patch.
Will send a v2 and ad your r-b tag.
Best regards
Akash