On 02.09.25 16:52, Srinivasan Shanmugam wrote:
> Enable userspace to obtain a handle to the kernel-owned MMIO_REMAP
> singleton when AMDGPU_GEM_DOMAIN_MMIO_REMAP is requested via
> amdgpu_gem_create_ioctl().
> 
> Validate the fixed 4K constraint: if PAGE_SIZE > AMDGPU_GPU_PAGE_SIZE
> return -EINVAL; when provided, size and alignment must equal
> AMDGPU_GPU_PAGE_SIZE.
> 
> If the singleton BO is not available, return -ENODEV.
> 
> v2:
> - Drop READ_ONCE() on adev->mmio_remap.bo (use a plain pointer load).
>   The pointer is set `bo = adev->mmio_remap.bo;` ie., The pointer is
>   written once during init and not changed while IOCTLs run. There’s no
>   concurrent writer in this execution path, so a normal read is safe.
>   (Alex)
> 
> v3:
> - Drop early -EINVAL for AMDGPU_GEM_DOMAIN_MMIO_REMAP; let the
>   MMIO_REMAP fast-path (For MMIO_REMAP, if asked, we don’t allocate a
>   new BO — we just check size/alignment, grab the one pre-made BO,
>   return a handle) handle it and return the singleton handle.
> 
> v4:
>  - Return -EOPNOTSUPP if the singleton isn’t available; drop PAGE_SIZE
>    check from IOCTL; inline the MMIO_REMAP fast-path and keep
>    size/alignment validation there. (Christian)
> 
> v5:
>  - Reword comments (no “===”), explain why the singleton is returned.
>  - Pass &args->in to amdgpu_gem_get_mmio_remap_handle() (drop local
>    ‘size’) (Christian)
> 
> Cc: Christian König <christian.koe...@amd.com>
> Suggested-by: Alex Deucher <alexander.deuc...@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmu...@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 58 ++++++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index d3c369742124..7676eafbedbf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -424,6 +424,47 @@ const struct drm_gem_object_funcs 
> amdgpu_gem_object_funcs = {
>       .vm_ops = &amdgpu_gem_vm_ops,
>  };
>  
> +/**
> + * amdgpu_gem_get_mmio_remap_handle - Create a GEM handle for the MMIO_REMAP 
> BO
> + * @file_priv: DRM file of the caller
> + * @adev: amdgpu device
> + * @in: GEM create input parameters from userspace (size/alignment fields 
> may be unset (0))
> + * @handle: returned GEM handle for userspace (output)
> + *
> + * Creates a GEM handle to the kernel-owned singleton MMIO_REMAP buffer 
> object
> + * (adev->rmmio_remap.bo). The BO is expected to be allocated during TTM init
> + * when the hardware exposes a remap base and PAGE_SIZE <= 4K.
> + *
> + * Although @in can specify size or alignment, this BO is fixed and unique;
> + * those fields are only validated, not used for allocation.
> + *
> + * drm_gem_handle_create() acquires the handle reference, which will be 
> dropped
> + * by GEM_CLOSE in userspace.
> + *
> + * Returns 0 on success,
> + *         -EOPNOTSUPP if the singleton BO is not available on this system,
> + *         or a negative errno from drm_gem_handle_create() / validation.
> + */
> +static int amdgpu_gem_get_mmio_remap_handle(struct drm_file *file_priv,
> +                                         struct amdgpu_device *adev,
> +                                         const struct 
> drm_amdgpu_gem_create_in *in,
> +                                         u32 *handle)
> +{
> +     struct amdgpu_bo *bo = adev->rmmio_remap.bo;
> +
> +     if (!bo)
> +             return -EOPNOTSUPP;
> +
> +     /* MMIO_REMAP is a fixed 4K page; enforce only if userspace specified 
> them. */
> +     if (in->bo_size != AMDGPU_GPU_PAGE_SIZE)
> +             return -EINVAL;

> +     if (in->alignment != AMDGPU_GPU_PAGE_SIZE)
> +             return -EINVAL;

You misunderstood me on teams :(

Only the size must be exactly AMDGPU_GPU_PAGE_SIZE. The alignment can also be 
smaller than that, just not larger.

In other words the check here is probably best written as "if (in->alignment <= 
AMDGPU_GPU_PAGE_SIZE)".

Apart from that the patch is Reviewed-by: Christian König 
<christian.koe...@amd.com>.

Regards,
Christian.

> +
> +     /* drm_gem_handle_create() gets the ref; GEM_CLOSE will drop it */
> +     return drm_gem_handle_create(file_priv, &bo->tbo.base, handle);
> +}
> +
>  /*
>   * GEM ioctls.
>   */
> @@ -465,8 +506,21 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void 
> *data,
>       /* always clear VRAM */
>       flags |= AMDGPU_GEM_CREATE_VRAM_CLEARED;
>  
> -     if (args->in.domains & AMDGPU_GEM_DOMAIN_MMIO_REMAP)
> -             return -EINVAL;
> +     /*
> +      * MMIO_REMAP fast-path:
> +      * For AMDGPU_GEM_DOMAIN_MMIO_REMAP we expose a single global, fixed 4K 
> page
> +      * via one shared BO. Don’t allocate a new BO; return a handle to that 
> singleton.
> +      */
> +     if (args->in.domains & AMDGPU_GEM_DOMAIN_MMIO_REMAP) {
> +             r = amdgpu_gem_get_mmio_remap_handle(filp, adev,
> +                                                  &args->in,
> +                                                  &handle);
> +             if (r)
> +                     return r;
> +
> +             args->out.handle = handle;
> +             return 0;
> +     }
>  
>       /* create a gem object to contain this object in */
>       if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |

Reply via email to