On Mon Jul 7, 2025 at 7:04 PM CEST, Caterina Shablia wrote:
> From: Asahi Lina <lina+ker...@asahilina.net>
>
> To be able to support "fake sparse" mappings without relying on GPU page
> fault handling, drivers may need to create large (e.g. 4GiB) mappings of
> the same page repeatedly (or same range of pages). Doing this through
> individual mappings would be very wasteful. This can be handled better
> by using a flag on map creation, but to do it safely, drm_gpuvm needs to
> be aware of this special case.
>
> Add a flag that signals that a given mapping is a page mapping, which is
> repeated all over the entire requested VA range. This tweaks the
> sm_map() logic to treat the GEM offsets differently when mappings are
> a repeated ones so they are not incremented as they would be with regular
> mappings.
>
> The size of the GEM portion to repeat is passed through
> drm_gpuva::gem::range. Most of the time it will be a page size, but
> it can be bigger as long as it's less that drm_gpuva::va::range, and
> drm_gpuva::gem::range is a multiple of drm_gpuva::va::range.

Should be "as long as it's less that drm_gpuva::va::range, and
drm_gpuva::va::range is a multiple of  drm_gpuva::gem::range".

I also think this feature deserves its own section in the global GPUVM
documentation -- please add a corresponding paragraph.

> +static int check_map_req(struct drm_gpuvm *gpuvm,

Let's call this validate_map_request().

> +                      const struct drm_gpuvm_map_req *req)
> +{
> +     if (unlikely(!drm_gpuvm_range_valid(gpuvm, req->va.addr, 
> req->va.range)))
> +             return -EINVAL;
> +
> +     if (req->flags & DRM_GPUVA_REPEAT) {
> +             u64 va_range = req->va.range;
> +
> +             /* For a repeated mapping, GEM range must be > 0
> +              * and a multiple of the VA range.
> +              */
> +             if (unlikely(!req->gem.range ||
> +                          va_range < req->gem.range ||
> +                          do_div(va_range, req->gem.range)))
> +                     return -EINVAL;
> +     }
> +
> +     return 0;
> +}
> +
>  static int
>  __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>                  const struct drm_gpuvm_ops *ops, void *priv,
> @@ -2137,6 +2179,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>       struct drm_gpuva reqva = {
>               .va.addr = req->va.addr,
>               .va.range = req->va.range,
> +             .gem.range = req->gem.range,
>               .gem.offset = req->gem.offset,
>               .gem.obj = req->gem.obj,
>               .flags = req->flags,
> @@ -2144,7 +2187,8 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>       u64 req_end = req->va.addr + req->va.range;
>       int ret;
>  
> -     if (unlikely(!drm_gpuvm_range_valid(gpuvm, req->va.addr, 
> req->va.range)))
> +     ret = check_map_req(gpuvm, req);
> +     if (unlikely(ret))
>               return -EINVAL;
>  
>       drm_gpuvm_for_each_va_range_safe(va, next, gpuvm, req->va.addr, 
> req_end) {
> @@ -2175,7 +2219,8 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>                                       .va.addr = req_end,
>                                       .va.range = range - req->va.range,
>                                       .gem.obj = obj,
> -                                     .gem.offset = offset + req->va.range,
> +                                     .gem.range = va->gem.range,
> +                                     .gem.offset = offset,

Why change this from offset + req->va.range to just offset?

Same for similar other changes below.

Also it seems that we need to update the documentation which shows all potential
cases when calling __drm_gpuvm_sm_map() [1].

[1] https://docs.kernel.org/gpu/drm-mm.html#split-and-merge

>                                       .flags = va->flags,
>                               };
>                               struct drm_gpuva_op_unmap u = {
> @@ -2183,6 +2228,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>                                       .keep = merge,
>                               };
>  
> +                             if (!(va->flags & DRM_GPUVA_REPEAT))
> +                                     n.gem.offset += req->va.range;
> +
>                               ret = op_remap_cb(ops, priv, NULL, &n, &u);
>                               if (ret)
>                                       return ret;
> @@ -2194,6 +2242,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>                               .va.addr = addr,
>                               .va.range = ls_range,
>                               .gem.obj = obj,
> +                             .gem.range = va->gem.range,
>                               .gem.offset = offset,
>                               .flags = va->flags,
>                       };
> @@ -2220,11 +2269,14 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>                                       .va.addr = req_end,
>                                       .va.range = end - req_end,
>                                       .gem.obj = obj,
> -                                     .gem.offset = offset + ls_range +
> -                                                   req->va.range,
> +                                     .gem.range = va->gem.range,
> +                                     .gem.offset = offset,
>                                       .flags = va->flags,
>                               };
>  
> +                             if (!(va->flags & DRM_GPUVA_REPEAT))
> +                                     n.gem.offset += ls_range + 
> req->va.range;
> +
>                               ret = op_remap_cb(ops, priv, &p, &n, &u);
>                               if (ret)
>                                       return ret;
> @@ -2250,7 +2302,8 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>                                       .va.addr = req_end,
>                                       .va.range = end - req_end,
>                                       .gem.obj = obj,
> -                                     .gem.offset = offset + req_end - addr,
> +                                     .gem.range = va->gem.range,
> +                                     .gem.offset = offset,
>                                       .flags = va->flags,
>                               };
>                               struct drm_gpuva_op_unmap u = {
> @@ -2258,6 +2311,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>                                       .keep = merge,
>                               };
>  
> +                             if (!(va->flags & DRM_GPUVA_REPEAT))
> +                                     n.gem.offset += req_end - addr;
> +
>                               ret = op_remap_cb(ops, priv, NULL, &n, &u);
>                               if (ret)
>                                       return ret;
> @@ -2294,6 +2350,7 @@ __drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm,
>                       prev.va.addr = addr;
>                       prev.va.range = req_addr - addr;
>                       prev.gem.obj = obj;
> +                     prev.gem.range = va->gem.range;
>                       prev.gem.offset = offset;
>                       prev.flags = va->flags;
>  
> @@ -2304,7 +2361,10 @@ __drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm,
>                       next.va.addr = req_end;
>                       next.va.range = end - req_end;
>                       next.gem.obj = obj;
> -                     next.gem.offset = offset + (req_end - addr);
> +                     prev.gem.range = va->gem.range;
> +                     next.gem.offset = offset;
> +                     if (!(va->flags & DRM_GPUVA_REPEAT))
> +                             next.gem.offset += req_end - addr;
>                       next.flags = va->flags;
>  
>                       next_split = true;
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index f77a89e791f1..629e8508f99f 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -56,10 +56,19 @@ enum drm_gpuva_flags {
>        */
>       DRM_GPUVA_SPARSE = (1 << 1),
>  
> +     /**
> +      * @DRM_GPUVA_REPEAT:
> +      *
> +      * Flag indicating that the &drm_gpuva is a mapping of a GEM
> +      * portion repeated multiple times to fill the virtual address

"of a GEM object with a certain range that is repeated multiple times to ..."

Reply via email to