Re: [PATCH v10 5/6] drm/panthor: Support sparse mappings

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 30 Apr 2026 09:57:34 +0200
Boris Brezillon <boris.brezillon@collabora.com> wrote:

> On Wed, 29 Apr 2026 19:32:17 +0100
> Adrián Larumbe <adrian.larumbe@collabora.com> wrote:
> 
> > Allow UM to bind sparsely populated memory regions by cyclically mapping
> > virtual ranges over a kernel-allocated dummy BO. This alternative is
> > preferable to the old method of handling sparseness in the UMD, because it
> > relied on the creation of a buffer object to the same end, despite the fact
> > Vulkan sparse resources don't need to be backed by a driver BO.
> > 
> > The choice of backing sparsely-bound regions with a Panhtor BO was made so
> > as to profit from the existing shrinker reclaim code. That way no special
> > treatment must be given to the dummy sparse BOs when reclaiming memory, as
> > would be the case if we had chosen a raw kernel page implementation.
> > 
> > A new dummy BO is allocated per open file context, because even though the
> > Vulkan spec mandates that writes into sparsely bound regions must be
> > discarded, our implementation is still a workaround over the fact Mali CSF
> > GPUs cannot support this behaviour on the hardware level, so writes still
> > make it into the backing BO. If we had a global one, then it could be a
> > venue for information leaks between file contexts, which should never
> > happen in DRM.
> > 
> > Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> > Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> > ---
> >  drivers/gpu/drm/panthor/panthor_gem.c |  18 +++
> >  drivers/gpu/drm/panthor/panthor_gem.h |   2 +
> >  drivers/gpu/drm/panthor/panthor_mmu.c | 159 ++++++++++++++++++++++----
> >  include/uapi/drm/panthor_drm.h        |  12 ++
> >  4 files changed, 170 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> > index 13295d7a593d..c798ac2963e1 100644
> > --- a/drivers/gpu/drm/panthor/panthor_gem.c
> > +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> > @@ -1345,6 +1345,24 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
> >  	return ERR_PTR(ret);
> >  }
> >  
> > +/**
> > + * panthor_dummy_bo_create() - Create a Panthor BO meant to back sparse bindings.
> > + * @ptdev: Device.
> > + *
> > + * Return: A valid pointer in case of success, an ERR_PTR() otherwise.
> > + */
> > +struct panthor_gem_object *
> > +panthor_dummy_bo_create(struct panthor_device *ptdev)
> > +{
> > +	/* Since even when the DRM device's mount point has enabled THP we have no guarantee
> > +	 * that drm_gem_get_pages() will return a single 2MiB PMD, and also we cannot be sure
> > +	 * that the 2MiB won't be reclaimed and re-allocated later on as 4KiB chunks, it doesn't
> > +	 * make sense to pre-populate this object's page array, nor to fall back on a BO size
> > +	 * of 4KiB. Sticking to a dummy object size of 2MiB lets us keep things simple for now.
> > +	 */
> > +	return panthor_gem_create(&ptdev->base, SZ_2M, DRM_PANTHOR_BO_NO_MMAP, NULL, 0);
> > +}
> > +
> >  static bool can_swap(void)
> >  {
> >  	return get_nr_swap_pages() > 0;
> > diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
> > index ae0491d0b121..8639c2fa08e6 100644
> > --- a/drivers/gpu/drm/panthor/panthor_gem.h
> > +++ b/drivers/gpu/drm/panthor/panthor_gem.h
> > @@ -315,6 +315,8 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
> >  
> >  void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo);
> >  
> > +struct panthor_gem_object *panthor_dummy_bo_create(struct panthor_device *ptdev);
> > +
> >  #ifdef CONFIG_DEBUG_FS
> >  void panthor_gem_debugfs_init(struct drm_minor *minor);
> >  #endif
> > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> > index f54a60cd0ec4..9257afd6adc9 100644
> > --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> > @@ -112,6 +112,17 @@ struct panthor_mmu {
> >  struct panthor_vm_pool {
> >  	/** @xa: Array used for VM handle tracking. */
> >  	struct xarray xa;
> > +
> > +	/**
> > +	 * @dummy: Dummy object used for sparse mappings
> > +	 *
> > +	 * Sparse bindings map virtual address ranges onto a dummy
> > +	 * BO in a modulo fashion. Even though sparse writes are meant
> > +	 * to be discarded and reads undefined, writes are still reflected
> > +	 * in the dummy buffer. That means we must keep a dummy object per
> > +	 * file context, to avoid data leaks between them.
> > +	 */
> > +	struct panthor_gem_object *dummy;
> >  };
> >  
> >  /**
> > @@ -391,6 +402,16 @@ struct panthor_vm {
> >  		 */
> >  		struct list_head lru_node;
> >  	} reclaim;
> > +
> > +	/**
> > +	 * @dummy: Dummy object used for sparse mappings.
> > +	 *
> > +	 * VM's must keep a reference to the file context-wide dummy BO because
> > +	 * they can outlive the file context, which includes the VM pool holding
> > +	 * the original dummy BO reference.
> > +	 *  
> 
> nit: Drop the extra blank line.
> 
> > +	 */
> > +	struct panthor_gem_object *dummy;
> >  };
> >  
> >  /**
> > @@ -1020,6 +1041,30 @@ panthor_vm_map_pages(struct panthor_vm *vm, u64 iova, int prot,
> >  	return 0;
> >  }
> >  
> > +static int
> > +panthor_vm_map_sparse(struct panthor_vm *vm, u64 iova, int prot,
> > +		      struct sg_table *sgt, u64 size)
> > +{
> > +	u64 mapped = 0;
> > +	int ret;
> > +
> > +	while (mapped < size) {
> > +		u64 addr = iova + mapped;
> > +		u32 chunk_size = min(size - mapped, SZ_2M - (addr & (SZ_2M - 1)));
> > +
> > +		ret = panthor_vm_map_pages(vm, addr, prot,
> > +					   sgt, 0, chunk_size);
> > +		if (ret) {
> > +			panthor_vm_unmap_pages(vm, iova, mapped);
> > +			return ret;
> > +		}
> > +
> > +		mapped += chunk_size;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int flags_to_prot(u32 flags)
> >  {
> >  	int prot = 0;
> > @@ -1262,6 +1307,7 @@ static int panthor_vm_op_ctx_prealloc_pts(struct panthor_vm_op_ctx *op_ctx)
> >  	(DRM_PANTHOR_VM_BIND_OP_MAP_READONLY | \
> >  	 DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC | \
> >  	 DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED | \
> > +	 DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE | \
> >  	 DRM_PANTHOR_VM_BIND_OP_TYPE_MASK)
> >  
> >  static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
> > @@ -1269,6 +1315,7 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
> >  					 struct panthor_gem_object *bo,
> >  					 const struct drm_panthor_vm_bind_op *op)
> >  {
> > +	bool is_sparse = op->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
> >  	struct drm_gpuvm_bo *preallocated_vm_bo;
> >  	struct sg_table *sgt = NULL;
> >  	int ret;
> > @@ -1280,8 +1327,21 @@ static int panthor_vm_prepare_map_op_ctx(struct panthor_vm_op_ctx *op_ctx,
> >  	    (op->flags & DRM_PANTHOR_VM_BIND_OP_TYPE_MASK) != DRM_PANTHOR_VM_BIND_OP_TYPE_MAP)
> >  		return -EINVAL;
> >  
> > -	/* Make sure the VA and size are in-bounds. */
> > -	if (op->size > bo->base.size || op->bo_offset > bo->base.size - op->size)
> > +	/* uAPI mandates sparsely bound regions must not be executable. */
> > +	if (is_sparse && !(op->flags & DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC))
> > +		return -EINVAL;
> > +
> > +	/* For non-sparse, make sure the VA and size are in-bounds.
> > +	 * For sparse, this is not applicable, because the dummy BO is
> > +	 * repeatedly mapped over a potentially wider VA range.
> > +	 */
> > +	if (!is_sparse && (op->size > bo->base.size || op->bo_offset > bo->base.size - op->size))
> > +		return -EINVAL;
> > +
> > +	/* For sparse, we don't expect any user BO, the BO we get passed
> > +	 * is the dummy BO attached to the VM pool.
> > +	 */
> > +	if (is_sparse && (op->bo_handle || op->bo_offset))
> >  		return -EINVAL;
> >  
> >  	/* If the BO has an exclusive VM attached, it can't be mapped to other VMs. */
> > @@ -1543,6 +1603,9 @@ int panthor_vm_pool_create_vm(struct panthor_device *ptdev,
> >  		return ret;
> >  	}
> >  
> > +	drm_gem_object_get(&pool->dummy->base);
> > +	vm->dummy = pool->dummy;
> > +
> >  	args->user_va_range = kernel_va_start;
> >  	return id;
> >  }
> > @@ -1634,6 +1697,7 @@ void panthor_vm_pool_destroy(struct panthor_file *pfile)
> >  	xa_for_each(&pfile->vms->xa, i, vm)
> >  		panthor_vm_destroy(vm);
> >  
> > +	drm_gem_object_put(&pfile->vms->dummy->base);
> >  	xa_destroy(&pfile->vms->xa);
> >  	kfree(pfile->vms);
> >  }
> > @@ -1651,6 +1715,13 @@ int panthor_vm_pool_create(struct panthor_file *pfile)
> >  		return -ENOMEM;
> >  
> >  	xa_init_flags(&pfile->vms->xa, XA_FLAGS_ALLOC1);
> > +
> > +	pfile->vms->dummy = panthor_dummy_bo_create(pfile->ptdev);
> > +	if (IS_ERR(pfile->vms->dummy)) {
> > +		kfree(pfile->vms);
> > +		return PTR_ERR(pfile->vms->dummy);
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > @@ -1987,6 +2058,9 @@ static void panthor_vm_free(struct drm_gpuvm *gpuvm)
> >  
> >  	free_io_pgtable_ops(vm->pgtbl_ops);
> >  
> > +	if (vm->dummy)
> > +		drm_gem_object_put(&vm->dummy->base);
> > +
> >  	drm_mm_takedown(&vm->mm);
> >  	kfree(vm);
> >  }
> > @@ -2146,7 +2220,23 @@ static void panthor_vma_init(struct panthor_vma *vma, u32 flags)
> >  #define PANTHOR_VM_MAP_FLAGS \
> >  	(DRM_PANTHOR_VM_BIND_OP_MAP_READONLY | \
> >  	 DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC | \
> > -	 DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED)
> > +	 DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED | \
> > +	 DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE)
> > +
> > +static int
> > +panthor_vm_exec_map_op(struct panthor_vm *vm, u32 flags,
> > +		       const struct drm_gpuva_op_map *op)
> > +{
> > +	struct panthor_gem_object *bo = to_panthor_bo(op->gem.obj);
> > +	int prot = flags_to_prot(flags);
> > +
> > +	if (flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE)
> > +		return panthor_vm_map_sparse(vm, op->va.addr, prot,
> > +					     bo->dmap.sgt, op->va.range);
> > +
> > +	return panthor_vm_map_pages(vm, op->va.addr, prot, bo->dmap.sgt,
> > +				    op->gem.offset, op->va.range);
> > +}
> >  
> >  static int panthor_gpuva_sm_step_map(struct drm_gpuva_op *op, void *priv)
> >  {
> > @@ -2160,9 +2250,7 @@ static int panthor_gpuva_sm_step_map(struct drm_gpuva_op *op, void *priv)
> >  
> >  	panthor_vma_init(vma, op_ctx->flags & PANTHOR_VM_MAP_FLAGS);
> >  
> > -	ret = panthor_vm_map_pages(vm, op->map.va.addr, flags_to_prot(vma->flags),
> > -				   op_ctx->map.bo->dmap.sgt, op->map.gem.offset,
> > -				   op->map.va.range);
> > +	ret = panthor_vm_exec_map_op(vm, vma->flags, &op->map);
> >  	if (ret) {
> >  		panthor_vm_op_ctx_return_vma(op_ctx, vma);
> >  		return ret;
> > @@ -2178,13 +2266,16 @@ static int panthor_gpuva_sm_step_map(struct drm_gpuva_op *op, void *priv)
> >  }
> >  
> >  static bool
> > -iova_mapped_as_huge_page(struct drm_gpuva_op_map *op, u64 addr)
> > +iova_mapped_as_huge_page(struct drm_gpuva_op_map *op, u64 addr, bool is_sparse)
> >  {
> >  	struct panthor_gem_object *bo = to_panthor_bo(op->gem.obj);
> >  	const struct page *pg;
> >  	pgoff_t bo_offset;
> >  
> > -	bo_offset = addr - op->va.addr + op->gem.offset;
> > +	/* Per-VM Dummy BO in sparse mappings is always 2MiB, so checking the
> > +	 * size of the very first page is enough.
> > +	 */
> > +	bo_offset = !is_sparse ? addr - op->va.addr + op->gem.offset : 0;
> >  	pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
> >  
> >  	return folio_size(page_folio(pg)) >= SZ_2M;
> > @@ -2194,6 +2285,8 @@ static void
> >  unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
> >  		     u64 *unmap_start, u64 *unmap_range)
> >  {
> > +	struct panthor_vma *unmap_vma = container_of(op->unmap->va, struct panthor_vma, base);
> > +	bool is_sparse = unmap_vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
> >  	u64 aligned_unmap_start, aligned_unmap_end, unmap_end;
> >  
> >  	unmap_end = *unmap_start + *unmap_range;
> > @@ -2205,7 +2298,7 @@ unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
> >  	 */
> >  	if (op->prev && aligned_unmap_start < *unmap_start &&
> >  	    op->prev->va.addr <= aligned_unmap_start &&
> > -	    iova_mapped_as_huge_page(op->prev, *unmap_start)) {
> > +	    (iova_mapped_as_huge_page(op->prev, *unmap_start, is_sparse))) {  
> 
> Actually, I think this could be:
> 	    
> 	    (is_sparse || iova_mapped_as_huge_page(op->prev, *unmap_start)) {
> 
> such that we always end up with sparse mappings starting at offset=0 on
> the dummy GEM, even if those mappings are not 2M aligned (see below for
> more reasons to keep this thing accurate).
> 
> >  		*unmap_range += *unmap_start - aligned_unmap_start;
> >  		*unmap_start = aligned_unmap_start;
> >  	}
> > @@ -2215,7 +2308,7 @@ unmap_hugepage_align(const struct drm_gpuva_op_remap *op,
> >  	 */
> >  	if (op->next && aligned_unmap_end > unmap_end &&
> >  	    op->next->va.addr + op->next->va.range >= aligned_unmap_end &&
> > -	    iova_mapped_as_huge_page(op->next, unmap_end - 1)) {
> > +	    iova_mapped_as_huge_page(op->next, unmap_end - 1, is_sparse)) {
> >  		*unmap_range += aligned_unmap_end - unmap_end;
> >  	}

If we want all sparse mappings to start at offset 0, we'll also need
something like:

	if (op->next && aligned_unmap_end > unmap_end) {
		/* If this is a sparse mapping, we always unmap everything
		 * up to the next 2M boundary, just so we have the guarantee
		 * the new unaligned mapping starts at offset=0 of the dummy
		 * GEM.
		 */
		if (is_sparse) {
			u64 new_unmap_end = min(op->next->va.addr + op->next->va.range,
						aligned_unmap_end);

			*unmap_range += new_unmap_end - unmap_end;
		} else if (op->next->va.addr + op->next->va.range >= aligned_unmap_end &&
			   iova_mapped_as_huge_page(op->next, unmap_end - 1)) {
			*unmap_range += aligned_unmap_end - unmap_end;
		}	
	}

But it looks like we're accumulating special cases for SPARSE,
so I'm wondering if we're not better off setting
drm_gpuva::gem.offset to iova & (SZ_2M - 1) at this point,
and adding all these GEM offset adjustment machinery behind
some helpers that would check the VM_BIND_MAP flags.

> >  }