[3/4] drm/i915: Add i915_gpu_state_unsafe debugfs entry

Commit Message

Mika Kuoppala Dec. 9, 2014, 4:04 p.m. UTC

This is a similar to 'i915_gpu_state' but more dangerous
as it doesn't try to lock nor idle the gpu before grabbing
the state. But the obvious advantages are that one can
inspect the current running gpu state and also get a state
dump even if mutex is hold.

As we don't want a unsuspecting user to trip into this by
accident, there needs to be write into the debugfs node
before any unsafe capturing can take place.

Signed-off-by: Mika Kuoppala <[e-mail]>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 55 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h     |  4 +++
 2 files changed, 59 insertions(+)

Comments

Chris Wilson Dec. 9, 2014, 9 p.m. UTC | #1

On Tue, Dec 09, 2014 at 06:04:33PM +0200, Mika Kuoppala wrote:
> This is a similar to 'i915_gpu_state' but more dangerous
> as it doesn't try to lock nor idle the gpu before grabbing
> the state. But the obvious advantages are that one can
> inspect the current running gpu state and also get a state
> dump even if mutex is hold.
> 
> As we don't want a unsuspecting user to trip into this by
> accident, there needs to be write into the debugfs node
> before any unsafe capturing can take place.

It shouldn't be called unsafe. It is precisely the mode in which the
regular error capture runs and so the kernel is required to be robust.
So just document that i915_gpu_state runs asynchronously to any client
and the gpu, and caveat emptor.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 30f56f3..5480cf5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -970,6 +970,60 @@  static const struct file_operations i915_gpu_state_fops = {
 	.release = i915_gpu_state_release,
 };
 
+static int i915_gpu_state_open_unsafe(struct inode *inode, struct file *file)
+{
+	struct drm_device *dev = inode->i_private;
+	struct i915_error_state_file_priv *state_priv;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	state_priv = kzalloc(sizeof(*state_priv), GFP_KERNEL);
+	if (!state_priv)
+		return -ENOMEM;
+
+	state_priv->dev = dev;
+
+	if (dev_priv->gpu_error.allow_unsafe_state_capture) {
+		state_priv->error = i915_gpu_state_capture(dev);
+		if (state_priv->error == NULL) {
+			kfree(state_priv);
+			return -ENOMEM;
+		}
+	}
+
+	file->private_data = state_priv;
+	return 0;
+}
+
+static ssize_t
+i915_gpu_state_write_unsafe(struct file *filp,
+			    const char __user *ubuf,
+			    size_t cnt,
+			    loff_t *ppos)
+{
+	struct i915_error_state_file_priv *error_priv = filp->private_data;
+	struct drm_device *dev = error_priv->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->gpu_error.allow_unsafe_state_capture = !
+		dev_priv->gpu_error.allow_unsafe_state_capture;
+
+	if (dev_priv->gpu_error.allow_unsafe_state_capture)
+		DRM_DEBUG_DRIVER("allowing unsafe, non locked gpu state dumps\n");
+	else
+		DRM_DEBUG_DRIVER("denying unsafe, non locked gpu state dumps\n");
+
+	return cnt;
+}
+
+static const struct file_operations i915_gpu_state_unsafe_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_gpu_state_open_unsafe,
+	.read = i915_gpu_state_read,
+	.write = i915_gpu_state_write_unsafe,
+	.llseek = default_llseek,
+	.release = i915_gpu_state_release,
+};
+
 static int i915_error_state_open(struct inode *inode, struct file *file)
 {
 	struct drm_device *dev = inode->i_private;
@@ -4410,6 +4464,7 @@  static const struct i915_debugfs_files {
 	{"i915_gem_drop_caches", &i915_drop_caches_fops},
 	{"i915_error_state", &i915_error_state_fops},
 	{"i915_gpu_state", &i915_gpu_state_fops},
+	{"i915_gpu_state_unsafe", &i915_gpu_state_unsafe_fops},
 	{"i915_next_seqno", &i915_next_seqno_fops},
 	{"i915_display_crc_ctl", &i915_display_crc_ctl_fops},
 	{"i915_pri_wm_latency", &i915_pri_wm_latency_fops},
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ca487b4..452f93f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1278,6 +1278,10 @@  struct i915_gpu_error {
 
 	/* Used to prevent gem_check_wedged returning -EAGAIN during gpu reset   */
 	bool reload_in_reset;
+
+	/* Safety mechanism to prevent unsuspecting user to obtain nonlocked
+	 * capture by accident */
+	bool allow_unsafe_state_capture;
 };
 
 enum modeset_restore {