Re: [RFC PATCH 2/3] drm/amdgpu: add AMDGPU_GEM_OP_OPEN_GLOBAL

["Wu, David" <davidwu2@amd.com>]

On 12/2/2025 10:12 AM, Christian König wrote:
> Instead of abusing the create IOCTL to open global BO add a new
> AMDGPU_GEM_OP_OPEN_GLOBAL functionality.
>
> The new AMDGPU_GEM_OP_OPEN_GLOBAL functionality expects an enum which tells
> it which global BO to open and copies the information about the BO to
> userspace similar to the AMDGPU_GEM_OP_GET_GEM_CREATE_INFO operation.
>
> The advantage is that we don't start overloading the create IOCTL with
> tons of special cases and opening the global BOs doesn't requires knowing
> the exact size and parameters of it in userspace any more.
>
> Heavily WIP and only compile tested.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 26 ++++++++++++++++++++-----
>   include/uapi/drm/amdgpu_drm.h           |  5 ++++-
>   2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9b81a6677f90..9e9b94dcb699 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -968,22 +968,34 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
>   int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>   			struct drm_file *filp)
>   {
> +	struct amdgpu_fpriv *fpriv = filp->driver_priv;
>   	struct drm_amdgpu_gem_op *args = data;
>   	struct drm_gem_object *gobj;
>   	struct amdgpu_vm_bo_base *base;
>   	struct amdgpu_bo *robj;
>   	struct drm_exec exec;
> -	struct amdgpu_fpriv *fpriv = filp->driver_priv;
>   	int r;
>   
>   	if (args->padding)
>   		return -EINVAL;
>   
> -	gobj = drm_gem_object_lookup(filp, args->handle);
> -	if (!gobj)
> -		return -ENOENT;
> +	if (args->op == AMDGPU_GEM_OP_OPEN_GLOBAL) {
> +		switch (args->handle) {
> +		case AMDGPU_GEM_GLOBAL_MMIO_REMAP:
> +			robj = drm_to_adev(dev)->rmmio_remap.bo;

rmmio_remap.bo could be null - so robj should be checked and return error instead.

David

> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		gobj = &robj->tbo.base;
> +		drm_gem_object_get(gobj);
> +	} else {
> +		gobj = drm_gem_object_lookup(filp, args->handle);
> +		if (!gobj)
> +			return -ENOENT;
>   
> -	robj = gem_to_amdgpu_bo(gobj);
> +		robj = gem_to_amdgpu_bo(gobj);
> +	}
>   
>   	drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT |
>   			  DRM_EXEC_IGNORE_DUPLICATES, 0);
> @@ -1002,6 +1014,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>   	}
>   
>   	switch (args->op) {
> +	case AMDGPU_GEM_OP_OPEN_GLOBAL:
>   	case AMDGPU_GEM_OP_GET_GEM_CREATE_INFO: {
>   		struct drm_amdgpu_gem_create_in info;
>   		void __user *out = u64_to_user_ptr(args->value);
> @@ -1096,6 +1109,9 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
>   		r = -EINVAL;
>   	}
>   
> +	if (!r && args->op == AMDGPU_GEM_OP_OPEN_GLOBAL)
> +		r = drm_gem_handle_create(filp, gobj, &args->handle);
> +
>   	drm_gem_object_put(gobj);
>   	return r;
>   out_exec:
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index c1336ed4ff75..6927c864a6d1 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -807,6 +807,9 @@ union drm_amdgpu_wait_fences {
>   #define AMDGPU_GEM_OP_GET_GEM_CREATE_INFO	0
>   #define AMDGPU_GEM_OP_SET_PLACEMENT		1
>   #define AMDGPU_GEM_OP_GET_MAPPING_INFO		2
> +#define AMDGPU_GEM_OP_OPEN_GLOBAL		3
> +
> +#define AMDGPU_GEM_GLOBAL_MMIO_REMAP		0
>   
>   struct drm_amdgpu_gem_vm_entry {
>   	/* Start of mapping (in bytes) */
> @@ -824,7 +827,7 @@ struct drm_amdgpu_gem_vm_entry {
>   
>   /* Sets or returns a value associated with a buffer. */
>   struct drm_amdgpu_gem_op {
> -	/** GEM object handle */
> +	/** GEM object handle or AMDGPU_GEM_GLOBAL_* */
>   	__u32	handle;
>   	/** AMDGPU_GEM_OP_* */
>   	__u32	op;