On 02.09.25 05:29, 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)
>
> Cc: Christian König <[email protected]>
> Suggested-by: Alex Deucher <[email protected]>
> Signed-off-by: Srinivasan Shanmugam <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 56 ++++++++++++++++++++++++-
> 1 file changed, 54 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..8781b2e16f54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -424,6 +424,45 @@ 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
> + * @req_size: size requested by userspace (0 means “unspecified”)
> + * @req_align: alignment requested by userspace (0 means “unspecified”)
> + * @handle: returned userspace GEM handle (out)
> + *
> + * 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.
> + *
> + * 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,
> + u64 req_size, u64 req_align,
> + u32 *handle)
> +{
> + struct amdgpu_bo *bo = adev->rmmio_remap.bo;
> +
> + if (!bo)
> + return -EOPNOTSUPP;
> +
> + /* Enforce fixed 4K constraints when explicitly provided by userspace */
> + if (req_size && req_size != AMDGPU_GPU_PAGE_SIZE)
> + return -EINVAL;
> + if (req_align && req_align != AMDGPU_GPU_PAGE_SIZE)
> + return -EINVAL;
> +
> + /* 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 +504,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 (HDP flush) fast-path ===
Please don't use === in comments.
> + * If userspace asks for the MMIO_REMAP domain, don't allocate a new BO.
> + * Return a handle to the singleton BO created at ttm init.
That explains what is done but not why. Maybe use something like:
For AMDGPU_GEM_DOMAIN_MMIO_REMAP we only have a single global page and always
use the same BO to represent it.
> + */
> + if (args->in.domains & AMDGPU_GEM_DOMAIN_MMIO_REMAP) {
> + r = amdgpu_gem_get_mmio_remap_handle(filp, adev,
> + size, args->in.alignment,
Maybe just pass in args instead of size and alignment separately.
We only have size as local variable because we used to have a workaround here.
That should probably be removed at some point.
Regards,
Christian.
> + &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 |
