On 6/23/26 13:09, Boris Brezillon wrote:
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.
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 <[email protected]>
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
