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

[Akash Goel <akash.goel@arm.com>]

Hi Boris

On 6/23/26 10:53, Boris Brezillon wrote:
> On Tue, 23 Jun 2026 10:24:13 +0100
> Akash Goel <akash.goel@arm.com> 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@arm.com>
>> ---
>>   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.

Please can you kindly clarify.

Best regards
Akash



> --->8---
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> index 5e735995f80d..28844e1e2f81 100644
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2301,6 +2301,10 @@ iova_mapped_as_huge_page(struct drm_gpuva_op_map *op, u64 addr)
>          const struct page *pg;
>          pgoff_t bo_offset;
>   
> +       /* Can happen if the BO is evicted. */
> +       if (!bo->backing.pages)
> +               return false;
> +
>          bo_offset = addr - op->va.addr + op->gem.offset;
>          pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>   
>