Commit 866e43b9 authored by Rob Clark's avatar Rob Clark
Browse files

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: default avatarRob Clark <robdclark@chromium.org>
Reviewed-by: default avatarAkhil P Oommen <quic_akhilpo@quicinc.com>
Patchwork: https://patchwork.freedesktop.org/patch/630578/
parent 19dbdbfb

drivers/gpu/drm/msm/adreno/adreno_gpu.c

+11 −10
Original line number Diff line number Diff line
@@ -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);

	}

}

drivers/gpu/drm/msm/msm_drv.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

drivers/gpu/drm/msm/msm_drv.h

+7 −0
Original line number Diff line number Diff line
@@ -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__)

drivers/gpu/drm/msm/msm_gem_submit.c

+27 −37
Original line number Diff line number Diff line
@@ -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",

			ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned cmdstream buffer size: %u\n",

					   submit_cmd.size);

			ret = -EINVAL;

			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",

		return SUBMIT_ERROR(EINVAL, submit, "invalid buffer index: %u (out of %u)\n",

				    idx, submit->nr_bos);

		return -EINVAL;

	}



	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",

			ret = SUBMIT_ERROR(EINVAL, submit, "non-aligned reloc offset: %u\n",

					   submit_reloc.submit_offset);

			ret = -EINVAL;

			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;

	}

drivers/gpu/drm/msm/msm_submitqueue.c

+1 −1
Original line number Diff line number Diff line
@@ -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;