drm/msm: UAPI error reporting

Debugging incorrect UAPI usage tends to be a bit painful, so add a
helper macro to make it easier to add debug logging which can be enabled
at runtime via drm.debug.

Signed-off-by: Rob Clark <[email protected]>
Reviewed-by: Akhil P Oommen <[email protected]>
Patchwork: https://patchwork.freedesktop.org/patch/630578/

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index c7454b6cfb7f752a23bd74b67d0eadbd02ac089b..1238f326597808eb28b4c6822cbd41a26e555eb9 100644 (file)
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -310,10 +310,11 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
                     uint32_t param, uint64_t *value, uint32_t *len)
 {
        struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+       struct drm_device *drm = gpu->dev;
 
        /* No pointer params yet */
        if (*len != 0)
-               return -EINVAL;
+               return UERR(EINVAL, drm, "invalid len");
 
        switch (param) {
        case MSM_PARAM_GPU_ID:
@@ -365,12 +366,12 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
                return 0;
        case MSM_PARAM_VA_START:
                if (ctx->aspace == gpu->aspace)
-                       return -EINVAL;
+                       return UERR(EINVAL, drm, "requires per-process pgtables");
                *value = ctx->aspace->va_start;
                return 0;
        case MSM_PARAM_VA_SIZE:
                if (ctx->aspace == gpu->aspace)
-                       return -EINVAL;
+                       return UERR(EINVAL, drm, "requires per-process pgtables");
                *value = ctx->aspace->va_size;
                return 0;
        case MSM_PARAM_HIGHEST_BANK_BIT:
@@ -389,14 +390,15 @@ int adreno_get_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
                *value = adreno_gpu->uche_trap_base;
                return 0;
        default:
-               DBG("%s: invalid param: %u", gpu->name, param);
-               return -EINVAL;
+               return UERR(EINVAL, drm, "%s: invalid param: %u", gpu->name, param);
        }
 }
 
 int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
                     uint32_t param, uint64_t value, uint32_t len)
 {
+       struct drm_device *drm = gpu->dev;
+
        switch (param) {
        case MSM_PARAM_COMM:
        case MSM_PARAM_CMDLINE:
@@ -404,11 +406,11 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
                 * that should be a reasonable upper bound
                 */
                if (len > PAGE_SIZE)
-                       return -EINVAL;
+                       return UERR(EINVAL, drm, "invalid len");
                break;
        default:
                if (len != 0)
-                       return -EINVAL;
+                       return UERR(EINVAL, drm, "invalid len");
        }
 
        switch (param) {
@@ -437,11 +439,10 @@ int adreno_set_param(struct msm_gpu *gpu, struct msm_file_private *ctx,
        }
        case MSM_PARAM_SYSPROF:
                if (!capable(CAP_SYS_ADMIN))
-                       return -EPERM;
+                       return UERR(EPERM, drm, "invalid permissions");
                return msm_file_private_set_sysprof(ctx, gpu, value);
        default:
-               DBG("%s: invalid param: %u", gpu->name, param);
-               return -EINVAL;
+               return UERR(EINVAL, drm, "%s: invalid param: %u", gpu->name, param);
        }
 }
 

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index c2dd8ef6d6dc69edf499e7aab250277aa92f462b..2aefb8becda0f193dc5cf80f98d62162451c9599 100644 (file)
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -538,7 +538,7 @@ static int msm_ioctl_gem_info_set_iova(struct drm_device *dev,
 
        /* Only supported if per-process address space is supported: */
        if (priv->gpu->aspace == ctx->aspace)
-               return -EOPNOTSUPP;
+               return UERR(EOPNOTSUPP, dev, "requires per-process pgtables");
 
        if (should_fail(&fail_gem_iova, obj->size))
                return -ENOMEM;

diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index d8c9a1b192632d3e29ff125bd7bb2d0bb491275d..fee31680a6d540d87cada67342bd5bee721662df 100644 (file)
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -28,6 +28,7 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/display/drm_dsc.h>
 #include <drm/msm_drm.h>
@@ -506,6 +507,12 @@ void msm_hrtimer_work_init(struct msm_hrtimer_work *work,
                           clockid_t clock_id,
                           enum hrtimer_mode mode);
 
+/* Helper for returning a UABI error with optional logging which can make
+ * it easier for userspace to understand what it is doing wrong.
+ */
+#define UERR(err, drm, fmt, ...) \
+       ({ DRM_DEV_DEBUG_DRIVER((drm)->dev, fmt, ##__VA_ARGS__); -(err); })
+
 #define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
 #define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
 

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index f775638d239a5c6e0c3fe6109230d725fcc74fdf..dee470403036849d43d27075585309c4ddff1ed3 100644 (file)
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -20,8 +20,8 @@
 /* For userspace errors, use DRM_UT_DRIVER.. so that userspace can enable
  * error msgs for debugging, but we don't spam dmesg by default
  */
-#define SUBMIT_ERROR(submit, fmt, ...) \
-       DRM_DEV_DEBUG_DRIVER((submit)->dev->dev, fmt, ##__VA_ARGS__)
+#define SUBMIT_ERROR(err, submit, fmt, ...) \
+       UERR(err, (submit)->dev, fmt, ##__VA_ARGS__)
 
 /*
  * Cmdstream submission:
@@ -142,8 +142,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
 
                if ((submit_bo.flags & ~MSM_SUBMIT_BO_FLAGS) ||
                        !(submit_bo.flags & MANDATORY_FLAGS)) {
-                       SUBMIT_ERROR(submit, "invalid flags: %x\n", submit_bo.flags);
-                       ret = -EINVAL;
+                       ret = SUBMIT_ERROR(EINVAL, submit, "invalid flags: %x\n", submit_bo.flags);
                        i = 0;
                        goto out;
                }
@@ -162,8 +161,7 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
                 */
                obj = idr_find(&file->object_idr, submit->bos[i].handle);
                if (!obj) {
-                       SUBMIT_ERROR(submit, "invalid handle %u at index %u\n", submit->bos[i].handle, i);
-                       ret = -EINVAL;
+                       ret = SUBMIT_ERROR(EINVAL, submit, "invalid handle %u at index %u\n", submit->bos[i].handle, i);
                        goto out_unlock;
                }
 
@@ -206,14 +204,12 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
                case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
                        break;
                default:
-                       SUBMIT_ERROR(submit, "invalid type: %08x\n", submit_cmd.type);
-                       return -EINVAL;
+                       return SUBMIT_ERROR(EINVAL, submit, "invalid type: %08x\n", submit_cmd.type);
                }
 
                if (submit_cmd.size % 4) {
-                       SUBMIT_ERROR(submit, "non-aligned cmdstream buffer size: %u\n",
-                                    submit_cmd.size);
-                       ret = -EINVAL;
+                       ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned cmdstream buffer size: %u\n",
+                                          submit_cmd.size);
                        goto out;
                }
 
@@ -371,9 +367,8 @@ static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
                struct drm_gem_object **obj, uint64_t *iova)
 {
        if (idx >= submit->nr_bos) {
-               SUBMIT_ERROR(submit, "invalid buffer index: %u (out of %u)\n",
-                            idx, submit->nr_bos);
-               return -EINVAL;
+               return SUBMIT_ERROR(EINVAL, submit, "invalid buffer index: %u (out of %u)\n",
+                                   idx, submit->nr_bos);
        }
 
        if (obj)
@@ -392,10 +387,8 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
        uint32_t *ptr;
        int ret = 0;
 
-       if (offset % 4) {
-               SUBMIT_ERROR(submit, "non-aligned cmdstream buffer: %u\n", offset);
-               return -EINVAL;
-       }
+       if (offset % 4)
+               return SUBMIT_ERROR(EINVAL, submit, "non-aligned cmdstream buffer: %u\n", offset);
 
        /* For now, just map the entire thing.  Eventually we probably
         * to do it page-by-page, w/ kmap() if not vmap()d..
@@ -414,9 +407,8 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
                uint64_t iova;
 
                if (submit_reloc.submit_offset % 4) {
-                       SUBMIT_ERROR(submit, "non-aligned reloc offset: %u\n",
-                                    submit_reloc.submit_offset);
-                       ret = -EINVAL;
+                       ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned reloc offset: %u\n",
+                                          submit_reloc.submit_offset);
                        goto out;
                }
 
@@ -425,8 +417,7 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
 
                if ((off >= (obj->size / 4)) ||
                                (off < last_offset)) {
-                       SUBMIT_ERROR(submit, "invalid offset %u at reloc %u\n", off, i);
-                       ret = -EINVAL;
+                       ret = SUBMIT_ERROR(EINVAL, submit, "invalid offset %u at reloc %u\n", off, i);
                        goto out;
                }
 
@@ -513,12 +504,12 @@ static struct drm_syncobj **msm_parse_deps(struct msm_gem_submit *submit,
 
                if (syncobj_desc.point &&
                    !drm_core_check_feature(submit->dev, DRIVER_SYNCOBJ_TIMELINE)) {
-                       ret = -EOPNOTSUPP;
+                       ret = SUBMIT_ERROR(EOPNOTSUPP, submit, "syncobj timeline unsupported");
                        break;
                }
 
                if (syncobj_desc.flags & ~MSM_SUBMIT_SYNCOBJ_FLAGS) {
-                       ret = -EINVAL;
+                       ret = -SUBMIT_ERROR(EINVAL, submit, "invalid syncobj flags: %x", syncobj_desc.flags);
                        break;
                }
 
@@ -531,7 +522,7 @@ static struct drm_syncobj **msm_parse_deps(struct msm_gem_submit *submit,
                        syncobjs[i] =
                                drm_syncobj_find(file, syncobj_desc.handle);
                        if (!syncobjs[i]) {
-                               ret = -EINVAL;
+                               ret = SUBMIT_ERROR(EINVAL, submit, "invalid syncobj handle: %u", i);
                                break;
                        }
                }
@@ -588,14 +579,14 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev,
                post_deps[i].point = syncobj_desc.point;
 
                if (syncobj_desc.flags) {
-                       ret = -EINVAL;
+                       ret = UERR(EINVAL, dev, "invalid syncobj flags");
                        break;
                }
 
                if (syncobj_desc.point) {
                        if (!drm_core_check_feature(dev,
                                                    DRIVER_SYNCOBJ_TIMELINE)) {
-                               ret = -EOPNOTSUPP;
+                               ret = UERR(EOPNOTSUPP, dev, "syncobj timeline unsupported");
                                break;
                        }
 
@@ -609,7 +600,7 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev,
                post_deps[i].syncobj =
                        drm_syncobj_find(file, syncobj_desc.handle);
                if (!post_deps[i].syncobj) {
-                       ret = -EINVAL;
+                       ret = UERR(EINVAL, dev, "invalid syncobj handle");
                        break;
                }
        }
@@ -677,10 +668,10 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
         * be more clever to dispatch to appropriate gpu module:
         */
        if (MSM_PIPE_ID(args->flags) != MSM_PIPE_3D0)
-               return -EINVAL;
+               return UERR(EINVAL, dev, "invalid pipe");
 
        if (MSM_PIPE_FLAGS(args->flags) & ~MSM_SUBMIT_FLAGS)
-               return -EINVAL;
+               return UERR(EINVAL, dev, "invalid flags");
 
        if (args->flags & MSM_SUBMIT_SUDO) {
                if (!IS_ENABLED(CONFIG_DRM_MSM_GPU_SUDO) ||
@@ -724,7 +715,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
                in_fence = sync_file_get_fence(args->fence_fd);
 
                if (!in_fence) {
-                       ret = -EINVAL;
+                       ret = UERR(EINVAL, dev, "invalid in-fence");
                        goto out_unlock;
                }
 
@@ -788,8 +779,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 
                if (!submit->cmd[i].size ||
                    (size_add(submit->cmd[i].size, submit->cmd[i].offset) > obj->size / 4)) {
-                       SUBMIT_ERROR(submit, "invalid cmdstream size: %u\n", submit->cmd[i].size * 4);
-                       ret = -EINVAL;
+                       ret = UERR(EINVAL, dev, "invalid cmdstream size: %u\n",
+                                  submit->cmd[i].size * 4);
                        goto out;
                }
 
@@ -799,8 +790,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
                        continue;
 
                if (!gpu->allow_relocs) {
-                       SUBMIT_ERROR(submit, "relocs not allowed\n");
-                       ret = -EINVAL;
+                       ret = UERR(EINVAL, dev, "relocs not allowed\n");
                        goto out;
                }
 
@@ -826,7 +816,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
                        (!args->fence || idr_find(&queue->fence_idr, args->fence))) {
                spin_unlock(&queue->idr_lock);
                idr_preload_end();
-               ret = -EINVAL;
+               ret = UERR(EINVAL, dev, "invalid in-fence-sn");
                goto out;
        }
 

diff --git a/drivers/gpu/drm/msm/msm_submitqueue.c b/drivers/gpu/drm/msm/msm_submitqueue.c
index 2fc3eaf81f4461990d48065f67bf5477787708e9..7fed1de63b5d9e20df88db8d9ca6ea45ec1a2846 100644 (file)
--- a/drivers/gpu/drm/msm/msm_submitqueue.c
+++ b/drivers/gpu/drm/msm/msm_submitqueue.c
@@ -18,7 +18,7 @@ int msm_file_private_set_sysprof(struct msm_file_private *ctx,
 
        switch (sysprof) {
        default:
-               return -EINVAL;
+               return UERR(EINVAL, gpu->dev, "Invalid sysprof: %d", sysprof);
        case 2:
                pm_runtime_get_sync(&gpu->pdev->dev);
                fallthrough;