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

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

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

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