Re: [PATCH] drm/panthor: Fix NPD issue on partial unmap of an evicted BO

[Boris Brezillon]

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.

Reviewed-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>