[RFC 0/2] drm/msm: additions for firmware debugging

[RFC 0/2] drm/msm: additions for firmware debugging

[Rob Clark]

A couple patches aimed in particular at simplifying firmware debugging.

The first patch adds some debugfs to dump out state, as well as a
debugfs file that can be written to trigger GPU reset and firmware
reloading.

The second patch adds a new SUBMIT ioctl flag to allow userspace to
submit cmdstream that is executed from the ringbuffer (RB) instead of
from IB1.  *Normally* this isn't something you want to allow userspace
to do, so it is (a) restricted to CAP_SYS_ADMIN and (b) hidden behind
an additional kconfig option.  But it *is* quite useful for debugging
since it allows you to execute a single command and then dump out the
PFP and/or ME microcontroller registers/state, without having that state
overwritten by handling of the return-from-CP_INDIRECT_BUFFER or other
commands that the kernel normally writes into RB as part of handling of
a SUBMIT.

Compared to hacking up the kernel to submit different commands and
dumping state, these two patches avoid much kernel rebuilding and
rebooting.  Also it makes it much easier to debug commands that have
pointers to other GPU buffers, since it re-uses all the existing SUBMIT
reloc logic.

Maybe I should also add a module_param() to unlock the "sudo" flag on
SUBMIT, or some other mechanism to prevent accidental use?  Suggestions
welcome.

So far, the "sudo" flag is only supported on a5xx.. a4xx and before had
different instruction sets for PFP and ME (which seem to closely match
the PFP and ME instruction sets for r600), and I haven't tried to write
an assembler/disassembler for those yet (and therefore haven't needed
this feature).

In case anyone else wants to play with this, see:

  https://github.com/freedreno/envytools/tree/afuc/afuc

Rob Clark (2):
  drm/msm: add a5xx specific debugfs
  drm/msm: add sudo flag to submit ioctl

 drivers/gpu/drm/msm/Kconfig                |  13 ++
 drivers/gpu/drm/msm/Makefile               |   2 +
 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c  | 188 +++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c      |  66 ++++++++++
 drivers/gpu/drm/msm/adreno/a5xx_gpu.h      |   4 +
 drivers/gpu/drm/msm/adreno/adreno_device.c |   6 +
 drivers/gpu/drm/msm/msm_debugfs.c          |   5 +-
 drivers/gpu/drm/msm/msm_gem.h              |   1 +
 drivers/gpu/drm/msm/msm_gem_submit.c       |   9 ++
 drivers/gpu/drm/msm/msm_gpu.h              |   2 +
 include/uapi/drm/msm_drm.h                 |   2 +
 11 files changed, 297 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c

-- 
2.13.6

[RFC 1/2] drm/msm: add a5xx specific debugfs

[Rob Clark]

Add some debugfs to dump out PFP and ME microcontroller state, as well
as some of the queues (MEQ and ROQ).  Also add a debugfs file to trigger
a GPU reset (and reloading the firmware on next submit).

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/Makefile               |   2 +
 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c  | 188 +++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c      |   1 +
 drivers/gpu/drm/msm/adreno/a5xx_gpu.h      |   4 +
 drivers/gpu/drm/msm/adreno/adreno_device.c |   6 +
 drivers/gpu/drm/msm/msm_debugfs.c          |   5 +-
 drivers/gpu/drm/msm/msm_gpu.h              |   2 +
 7 files changed, 207 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c

diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 92b3844202d2..ebe0c3d0b126 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -62,6 +62,8 @@ msm-y := \
 	msm_ringbuffer.o \
 	msm_submitqueue.o
 
+msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o
+
 msm-$(CONFIG_DRM_FBDEV_EMULATION) += msm_fbdev.o
 msm-$(CONFIG_COMMON_CLK) += mdp/mdp4/mdp4_lvds_pll.o
 msm-$(CONFIG_COMMON_CLK) += hdmi/hdmi_pll_8960.o
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
new file mode 100644
index 000000000000..cef09780ef17
--- /dev/null
+++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
@@ -0,0 +1,188 @@
+/* Copyright (c) 2016-2017 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+
+#include <linux/types.h>
+#include <linux/debugfs.h>
+#include <drm/drm_print.h>
+
+#include "a5xx_gpu.h"
+
+static int pfp_print(struct msm_gpu *gpu, struct drm_printer *p)
+{
+	int i;
+
+	drm_printf(p, "PFP state:\n");
+
+	for (i = 0; i < 36; i++) {
+		gpu_write(gpu, REG_A5XX_CP_PFP_STAT_ADDR, i);
+		drm_printf(p, "  %02x: %08x\n", i,
+			gpu_read(gpu, REG_A5XX_CP_PFP_STAT_DATA));
+	}
+
+	return 0;
+}
+
+static int me_print(struct msm_gpu *gpu, struct drm_printer *p)
+{
+	int i;
+
+	drm_printf(p, "ME state:\n");
+
+	for (i = 0; i < 29; i++) {
+		gpu_write(gpu, REG_A5XX_CP_ME_STAT_ADDR, i);
+		drm_printf(p, "  %02x: %08x\n", i,
+			gpu_read(gpu, REG_A5XX_CP_ME_STAT_DATA));
+	}
+
+	return 0;
+}
+
+static int meq_print(struct msm_gpu *gpu, struct drm_printer *p)
+{
+	int i;
+
+	drm_printf(p, "MEQ state:\n");
+	gpu_write(gpu, REG_A5XX_CP_MEQ_DBG_ADDR, 0);
+
+	for (i = 0; i < 64; i++) {
+		drm_printf(p, "  %02x: %08x\n", i,
+			gpu_read(gpu, REG_A5XX_CP_MEQ_DBG_DATA));
+	}
+
+	return 0;
+}
+
+static int roq_print(struct msm_gpu *gpu, struct drm_printer *p)
+{
+	int i;
+
+	drm_printf(p, "ROQ state:\n");
+	gpu_write(gpu, REG_A5XX_CP_ROQ_DBG_ADDR, 0);
+
+	for (i = 0; i < 512 / 4; i++) {
+		uint32_t val[4];
+		int j;
+		for (j = 0; j < 4; j++)
+			val[j] = gpu_read(gpu, REG_A5XX_CP_ROQ_DBG_DATA);
+		drm_printf(p, "  %02x: %08x %08x %08x %08x\n", i,
+			val[0], val[1], val[2], val[3]);
+	}
+
+	return 0;
+}
+
+static int show(struct seq_file *m, void *arg)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct drm_printer p = drm_seq_file_printer(m);
+	int (*show)(struct msm_gpu *gpu, struct drm_printer *p) =
+		node->info_ent->data;
+
+	return show(priv->gpu, &p);
+}
+
+#define ENT(n) { .name = #n, .show = show, .data = n ##_print }
+static struct drm_info_list a5xx_debugfs_list[] = {
+	ENT(pfp),
+	ENT(me),
+	ENT(meq),
+	ENT(roq),
+};
+
+/* for debugfs files that can be written to, we can't use drm helper: */
+static int
+reset_set(void *data, u64 val)
+{
+	struct drm_device *dev = data;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct msm_gpu *gpu = priv->gpu;
+	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+	struct a5xx_gpu *a5xx_gpu = to_a5xx_gpu(adreno_gpu);
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EINVAL;
+
+	/* TODO do we care about trying to make sure the GPU is idle?
+	 * Since this is just a debug feature limited to CAP_SYS_ADMIN,
+	 * maybe it is fine to let the user keep both pieces if they
+	 * try to reset an active GPU.
+	 */
+
+	mutex_lock(&dev->struct_mutex);
+
+	if (adreno_gpu->pm4) {
+		release_firmware(adreno_gpu->pm4);
+		adreno_gpu->pm4 = NULL;
+	}
+
+	if (adreno_gpu->pfp) {
+		release_firmware(adreno_gpu->pfp);
+		adreno_gpu->pfp = NULL;
+	}
+	if (a5xx_gpu->pm4_bo) {
+		if (a5xx_gpu->pm4_iova)
+			msm_gem_put_iova(a5xx_gpu->pm4_bo, gpu->aspace);
+		drm_gem_object_unreference(a5xx_gpu->pm4_bo);
+		a5xx_gpu->pm4_bo = NULL;
+	}
+
+	if (a5xx_gpu->pfp_bo) {
+		if (a5xx_gpu->pfp_iova)
+			msm_gem_put_iova(a5xx_gpu->pfp_bo, gpu->aspace);
+		drm_gem_object_unreference(a5xx_gpu->pfp_bo);
+		a5xx_gpu->pfp_bo = NULL;
+	}
+
+	gpu->needs_hw_init = true;
+
+	pm_runtime_get_sync(&gpu->pdev->dev);
+	gpu->funcs->recover(gpu);
+
+	pm_runtime_put_sync(&gpu->pdev->dev);
+	mutex_unlock(&dev->struct_mutex);
+
+	return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(reset_fops, NULL, reset_set, "%llx\n");
+
+
+int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor)
+{
+	struct drm_device *dev = minor->dev;
+	struct dentry *ent;
+	int ret;
+
+	if (!minor)
+		return 0;
+
+	ret = drm_debugfs_create_files(a5xx_debugfs_list,
+			ARRAY_SIZE(a5xx_debugfs_list),
+			minor->debugfs_root, minor);
+
+	if (ret) {
+		dev_err(dev->dev, "could not install a5xx_debugfs_list\n");
+		return ret;
+	}
+
+	ent = debugfs_create_file("reset", S_IWUGO,
+		minor->debugfs_root,
+		dev, &reset_fops);
+	if (!ent)
+		return -ENOMEM;
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 63413ecb7a6c..44d3c6a129c9 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1179,6 +1179,7 @@ static const struct adreno_gpu_funcs funcs = {
 		.destroy = a5xx_destroy,
 #ifdef CONFIG_DEBUG_FS
 		.show = a5xx_show,
+		.debugfs_init = a5xx_debugfs_init,
 #endif
 	},
 	.get_timestamp = a5xx_get_timestamp,
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
index 6fb8c2f9b9e4..7d71860c4bee 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.h
@@ -49,6 +49,10 @@ struct a5xx_gpu {
 
 #define to_a5xx_gpu(x) container_of(x, struct a5xx_gpu, base)
 
+#ifdef CONFIG_DEBUG_FS
+int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor);
+#endif
+
 /*
  * In order to do lockless preemption we use a simple state machine to progress
  * through the process.
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 05022ea2a007..4dbb7837d5bb 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -143,6 +143,12 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
 		return NULL;
 	}
 
+	if (gpu->funcs->debugfs_init) {
+		gpu->funcs->debugfs_init(gpu, dev->primary);
+		gpu->funcs->debugfs_init(gpu, dev->render);
+		gpu->funcs->debugfs_init(gpu, dev->control);
+	}
+
 	return gpu;
 }
 
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index 1855182c76ce..ba74cb4f94df 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -161,8 +161,11 @@ int msm_debugfs_init(struct drm_minor *minor)
 		return ret;
 	}
 
-	if (priv->kms->funcs->debugfs_init)
+	if (priv->kms->funcs->debugfs_init) {
 		ret = priv->kms->funcs->debugfs_init(priv->kms, minor);
+		if (ret)
+			return ret;
+	}
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 9c09c05b465f..acddf1fb9b81 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -66,6 +66,8 @@ struct msm_gpu_funcs {
 #ifdef CONFIG_DEBUG_FS
 	/* show GPU status in debugfs: */
 	void (*show)(struct msm_gpu *gpu, struct seq_file *m);
+	/* for generation specific debugfs: */
+	int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor);
 #endif
 };
 
-- 
2.13.6

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[RFC 2/2] drm/msm: add sudo flag to submit ioctl

[Rob Clark]

This flags cause cmdstream to be executed from the ringbuffer (RB)
instead of IB1.  Normally not something you'd ever want to do, but
it is super useful for firmware debugging.

Hidden behind CAP_SYS_ADMIN and a default=n kconfig option, to prevent
it from being used on accident.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/msm/Kconfig           | 13 +++++++
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 65 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/msm/msm_gem.h         |  1 +
 drivers/gpu/drm/msm/msm_gem_submit.c  |  9 +++++
 include/uapi/drm/msm_drm.h            |  2 ++
 5 files changed, 90 insertions(+)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 99d39b2aefa6..733d878e1180 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -28,6 +28,19 @@ config DRM_MSM_REGISTER_LOGGING
 	  that can be parsed by envytools demsm tool.  If enabled, register
 	  logging can be switched on via msm.reglog=y module param.
 
+config DRM_MSM_GPU_SUDO
+	bool "Enable SUDO flag on submits"
+	depends on DRM_MSM && EXPERT
+	default n
+	help
+	  Enable userspace that has CAP_SYS_ADMIN to submit GPU commands
+	  that are run from RB instead of IB1.  This essentially gives
+	  userspace kernel level access, but is useful for firmware
+	  debugging.
+
+	  Only use this if you really know what you are doing.  This should
+	  *not* be enabled for production kernels.  If unsure, say N.
+
 config DRM_MSM_HDMI_HDCP
 	bool "Enable HDMI HDCP support in MSM DRM driver"
 	depends on DRM_MSM && QCOM_SCM
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 44d3c6a129c9..644ef5367423 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -138,6 +138,65 @@ static void a5xx_flush(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
 		gpu_write(gpu, REG_A5XX_CP_RB_WPTR, wptr);
 }
 
+static void a5xx_submit_in_rb(struct msm_gpu *gpu, struct msm_gem_submit *submit,
+	struct msm_file_private *ctx)
+{
+	struct msm_drm_private *priv = gpu->dev->dev_private;
+	struct msm_ringbuffer *ring = submit->ring;
+	struct msm_gem_object *obj;
+	uint32_t *ptr, dwords;
+	unsigned int i;
+
+	for (i = 0; i < submit->nr_cmds; i++) {
+		switch (submit->cmd[i].type) {
+		case MSM_SUBMIT_CMD_IB_TARGET_BUF:
+			break;
+		case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
+			if (priv->lastctx == ctx)
+				break;
+		case MSM_SUBMIT_CMD_BUF:
+			/* copy commands into RB: */
+			obj = submit->bos[submit->cmd[i].idx].obj;
+			dwords = submit->cmd[i].size;
+
+			ptr = msm_gem_get_vaddr(&obj->base);
+
+			/* _get_vaddr() shouldn't fail at this point,
+			 * since we've already mapped it once in
+			 * submit_reloc()
+			 */
+			if (WARN_ON(!ptr))
+				return;
+
+			for (i = 0; i < dwords; i++) {
+				/* normally the OUT_PKTn() would wait
+				 * for space for the packet.  But since
+				 * we just OUT_RING() the whole thing,
+				 * need to call adreno_wait_ring()
+				 * ourself:
+				 */
+				adreno_wait_ring(ring, 1);
+				OUT_RING(ring, ptr[i]);
+			}
+
+			msm_gem_put_vaddr(&obj->base);
+
+			break;
+		}
+	}
+
+	a5xx_flush(gpu, ring);
+	a5xx_preempt_trigger(gpu);
+
+	/* we might not necessarily have a cmd from userspace to
+	 * trigger an event to know that submit has completed, so
+	 * do this manually:
+	 */
+	a5xx_idle(gpu, ring);
+	ring->memptrs->fence = submit->seqno;
+	msm_gpu_retire(gpu);
+}
+
 static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
 	struct msm_file_private *ctx)
 {
@@ -147,6 +206,12 @@ static void a5xx_submit(struct msm_gpu *gpu, struct msm_gem_submit *submit,
 	struct msm_ringbuffer *ring = submit->ring;
 	unsigned int i, ibs = 0;
 
+	if (IS_ENABLED(CONFIG_DRM_MSM_GPU_SUDO) && submit->in_rb) {
+		priv->lastctx = NULL;
+		a5xx_submit_in_rb(gpu, submit, ctx);
+		return;
+	}
+
 	OUT_PKT7(ring, CP_PREEMPT_ENABLE_GLOBAL, 1);
 	OUT_RING(ring, 0x02);
 
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 7087639932a2..dd2f8b419495 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -147,6 +147,7 @@ struct msm_gem_submit {
 	struct pid *pid;    /* submitting process */
 	bool valid;         /* true if no cmdstream patching needed */
 	bool dumped;        /* true if already dumped */
+	bool in_rb;         /* "sudo" mode, copy cmds into RB */
 	struct msm_ringbuffer *ring;
 	unsigned int nr_cmds;
 	unsigned int nr_bos;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index da8187f760ce..c07635afd98f 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -431,6 +431,12 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 	if (MSM_PIPE_FLAGS(args->flags) & ~MSM_SUBMIT_FLAGS)
 		return -EINVAL;
 
+	if (args->flags & MSM_SUBMIT_SUDO) {
+		if (!IS_ENABLED(CONFIG_DRM_MSM_GPU_SUDO) ||
+		    !capable(CAP_SYS_ADMIN))
+			return -EINVAL;
+	}
+
 	queue = msm_submitqueue_get(ctx, args->queueid);
 	if (!queue)
 		return -ENOENT;
@@ -472,6 +478,9 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
+	if (args->flags & MSM_SUBMIT_SUDO)
+		submit->in_rb = true;
+
 	ret = submit_lookup_objects(submit, args, file);
 	if (ret)
 		goto out;
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index bbbaffad772d..c06d0a5bdd80 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -201,10 +201,12 @@ struct drm_msm_gem_submit_bo {
 #define MSM_SUBMIT_NO_IMPLICIT   0x80000000 /* disable implicit sync */
 #define MSM_SUBMIT_FENCE_FD_IN   0x40000000 /* enable input fence_fd */
 #define MSM_SUBMIT_FENCE_FD_OUT  0x20000000 /* enable output fence_fd */
+#define MSM_SUBMIT_SUDO          0x10000000 /* run submitted cmds from RB */
 #define MSM_SUBMIT_FLAGS                ( \
 		MSM_SUBMIT_NO_IMPLICIT   | \
 		MSM_SUBMIT_FENCE_FD_IN   | \
 		MSM_SUBMIT_FENCE_FD_OUT  | \
+		MSM_SUBMIT_SUDO          | \
 		0)
 
 /* Each cmdstream submit consists of a table of buffers involved, and
-- 
2.13.6