Re: [PATCH v5 07/11] drm/gpuvm: Ensure correctness of unmap/remaps of repeated regions

["Danilo Krummrich" <dakr@kernel.org>]

On Fri Mar 13, 2026 at 4:09 PM CET, Adrián Larumbe wrote:
> When an unmap or map operation that leads to a remap intersects with a
> GPU VA that spans over a repeated range, the newly spawned VAs must
> preserve the repeated property, ie, VA's range must be a multiple of
> gem.range, and also the VA's start address must be on a gem.range
> boundary. When this doesn't hold, disallow such operations and notify
> UM with an invalid argument error.
>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@collabora.com>
> ---
>  drivers/gpu/drm/drm_gpuvm.c | 67 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_gpuvm.h     |  7 +++-
>  2 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> index ca7445f767fc..80750119221d 100644
> --- a/drivers/gpu/drm/drm_gpuvm.c
> +++ b/drivers/gpu/drm/drm_gpuvm.c
> @@ -2462,6 +2462,65 @@ static int validate_map_request(struct drm_gpuvm *gpuvm,
>  	return 0;
>  }
>  
> +static int
> +validate_repeated_unmap_request(struct drm_gpuvm *gpuvm,
> +				u64 req_addr, u64 req_end)

I think the name is misleading, this is called from both __drm_gpuvm_sm_map()
and __drm_gpuvm_sm_unmap().

> +{
> +	struct drm_gpuva *first, *last, *va;
> +	u64 multiple;
> +
> +	if (!(gpuvm->flags & DRM_GPUVM_HAS_REPEAT_MAPS))
> +		return 0;
> +
> +	/* Find the first and last VAs the map request intersects with */
> +	first = last = NULL;
> +	drm_gpuvm_for_each_va_range(va, gpuvm, req_addr, req_end) {
> +		if (!first)
> +			first = va;
> +		last = va;
> +	}

Can't this be covered with two calls to drm_gpuva_find_first()? I.e. for the
last you can call drm_gpuva_find_first(gpuvm, req_end - 1, 1), as you are only
interested if it spans across the req_end boundary.

This way this becomes O(log N) rather than O(N), where N can actually be very
large.

> +
> +	if (!first)
> +		return 0;
> +
> +	if (first->flags & DRM_GPUVA_REPEAT) {
> +		u64 addr = first->va.addr;
> +		u64 range = first->va.range;
> +		u64 end = addr + range;
> +
> +		drm_WARN_ON(gpuvm->drm, first->gem.repeat_range == 0);

How can this ever happen? Why do we need to check it here? If that's invalid
reject it when creating the VA instead.

> +
> +		if (addr < req_addr) {
> +			multiple = req_addr;
> +			if (do_div(multiple, first->gem.repeat_range))
> +				return -EINVAL;
> +		}
> +
> +		if (end > req_end) {
> +			multiple = req_end;
> +			if (do_div(multiple, first->gem.repeat_range))
> +				return -EINVAL;
> +			return 0;
> +		}
> +	}
> +
> +	if ((first != last) && (last->flags & DRM_GPUVA_REPEAT)) {
> +		u64 addr = last->va.addr;
> +		u64 range = last->va.range;
> +		u64 end = addr + range;
> +
> +		drm_WARN_ON(last->vm->drm, last->gem.repeat_range == 0);
> +
> +		if (end > req_end) {
> +			multiple = req_end;
> +			if (do_div(multiple, last->gem.repeat_range))
> +				return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}