On Tue, 23 Jun 2026 12:17:51 +0100 Akash Goel <[email protected]> wrote:
> Hi Boris > > On 6/23/26 10:53, Boris Brezillon wrote: > > On Tue, 23 Jun 2026 10:24:13 +0100 > > Akash Goel <[email protected]> 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 <[email protected]> > >> --- > >> 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 <[email protected]>
