From: Christian König <[email protected]>
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.
This keeps the GEM create path simpler and avoids exposing internal
allocation details to userspace.
v2 (Srini):
- Centralize global BO ID to BO mapping into a helper.
- Return -EOPNOTSUPP if the requested global BO is not available.
- Allow args->value == 0 for AMDGPU_GEM_OP_OPEN_GLOBAL so callers that
only need a handle can skip the create_info copy_to_user().
- Clarify the input/output semantics of the handle field for OPEN_GLOBAL
in the UAPI documentation.
- Avoid potential NULL dereference for MMIO_REMAP on unsupported
hardware. (David)
Fixes: dd2bf86d1383 ("drm/amdgpu/uapi: Introduce AMDGPU_GEM_DOMAIN_MMIO_REMAP")
Cc: Alex Deucher <[email protected]>
Cc: Christian König <[email protected]>
Cc: Leo Liu <[email protected]>
Cc: Ruijing Dong <[email protected]>
Cc: David (Ming Qiang) Wu <[email protected]>
Signed-off-by: Christian König <[email protected]>
Signed-off-by: Srinivasan Shanmugam <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 44 +++++++++++++++++++++----
include/uapi/drm/amdgpu_drm.h | 11 ++++++-
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9b81a6677f90..a381b8ad9d29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -965,25 +965,44 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void
*data,
return r;
}
+static struct amdgpu_bo *
+amdgpu_get_global_bo(struct amdgpu_device *adev, u32 id)
+{
+ switch (id) {
+ case AMDGPU_GEM_GLOBAL_MMIO_REMAP:
+ return adev->rmmio_remap.bo;
+ default:
+ return NULL;
+ }
+}
+
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) {
+ robj = amdgpu_get_global_bo(drm_to_adev(dev), args->handle);
+ if (!robj)
+ return -EOPNOTSUPP;
+ 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,17 +1021,27 @@ 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);
+ void __user *out = NULL;
info.bo_size = robj->tbo.base.size;
info.alignment = robj->tbo.page_alignment << PAGE_SHIFT;
info.domains = robj->preferred_domains;
info.domain_flags = robj->flags;
drm_exec_fini(&exec);
+ /*
+ * For OPEN_GLOBAL, allow args->value == 0 when the caller
+ * only wants a handle and does not need the create_info.
+ */
+ if (args->op == AMDGPU_GEM_OP_OPEN_GLOBAL && !args->value)
+ break;
+
+ out = u64_to_user_ptr(args->value);
if (copy_to_user(out, &info, sizeof(info)))
r = -EFAULT;
+
break;
}
case AMDGPU_GEM_OP_SET_PLACEMENT:
@@ -1096,6 +1125,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 9680548ee41b..283410a19dbd 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,13 @@ 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_*.
+ *
+ * For AMDGPU_GEM_OP_OPEN_GLOBAL:
+ * - input: handle = AMDGPU_GEM_GLOBAL_* ID
+ * - output: handle = per-file GEM handle to that global BO
+ */
__u32 handle;
/** AMDGPU_GEM_OP_* */
__u32 op;
--
2.34.1
