[RFC 0/2] strace/drm: Add i915 ioctls to strace

[RFC 0/2] strace/drm: Add i915 ioctls to strace

[Patrik Jakobsson]

This patch set aims to make strace more useful when tracing i915 ioctls.
The ioctl type is first checked for being drm and then the driver
backing the opened device is identified by looking at sysfs. Other
drivers than i915 can easily be added.

Only a subset of the i915 ioctls are included. I will extend this patch
set if the approach looks ok. The generic drm ioctls are also missing.

Give it a spin with:
        strace -e trace=ioctl -p `pidof X`

Patrik Jakobsson (2):
  strace/drm: Print extended info for drm and i915 ioctls
  strace/drm: Print args for most common i915 ioctls

 Makefile.am                |   2 +
 defs.h                     |   2 +
 drm.c                      | 104 +++++++++++++++++
 drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
 ioctl.c                    |   5 +
 xlat/drm_i915_getparams.in |  28 +++++
 xlat/drm_i915_ioctls.in    |  51 +++++++++
 xlat/drm_i915_setparams.in |   4 +
 8 files changed, 474 insertions(+)
 create mode 100644 drm.c
 create mode 100644 drm_i915.c
 create mode 100644 xlat/drm_i915_getparams.in
 create mode 100644 xlat/drm_i915_ioctls.in
 create mode 100644 xlat/drm_i915_setparams.in

-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 1/2] strace/drm: Print extended info for drm and i915 ioctls

[Patrik Jakobsson]

This adds a dispatcher for extending drm ioctl debugging info and adds
the i915 ioctls to the xlat framework.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 Makefile.am             |   2 +
 defs.h                  |   2 +
 drm.c                   | 104 ++++++++++++++++++++++++++++++++++++++++++++++++
 drm_i915.c              |  43 ++++++++++++++++++++
 ioctl.c                 |   5 +++
 xlat/drm_i915_ioctls.in |  51 ++++++++++++++++++++++++
 6 files changed, 207 insertions(+)
 create mode 100644 drm.c
 create mode 100644 drm_i915.c
 create mode 100644 xlat/drm_i915_ioctls.in

diff --git a/Makefile.am b/Makefile.am
index 549aebc..941e12a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -121,6 +121,8 @@ strace_SOURCES =	\
 	utime.c		\
 	utimes.c	\
 	v4l2.c		\
+	drm.c		\
+	drm_i915.c	\
 	vsprintf.c	\
 	wait.c		\
 	xattr.c
diff --git a/defs.h b/defs.h
index 77c819c..699e244 100644
--- a/defs.h
+++ b/defs.h
@@ -571,6 +571,8 @@ extern int sock_ioctl(struct tcb *, const unsigned int, long);
 extern int term_ioctl(struct tcb *, const unsigned int, long);
 extern int ubi_ioctl(struct tcb *, const unsigned int, long);
 extern int v4l2_ioctl(struct tcb *, const unsigned int, long);
+extern int drm_ioctl(struct tcb *, const unsigned int, long);
+extern int drm_i915_ioctl(struct tcb *, const unsigned int, long);
 
 extern int tv_nz(const struct timeval *);
 extern int tv_cmp(const struct timeval *, const struct timeval *);
diff --git a/drm.c b/drm.c
new file mode 100644
index 0000000..f47c514
--- /dev/null
+++ b/drm.c
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2015 Intel Corporation
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Authors:
+ *    Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
+ */
+
+#include "defs.h"
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <string.h>
+#include <linux/limits.h>
+#include <stdint.h>
+#include <sys/ioctl.h>
+#include <linux/types.h>
+#include <drm/drm.h>
+#include <drm/i915_drm.h>
+
+#define DRM_MAX_NAME_LEN 128
+
+static int drm_get_driver_name(struct tcb *tcp, char *name, size_t bufsize)
+{
+	char path[PATH_MAX];
+	char link[PATH_MAX];
+	int ret;
+
+	ret = getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1);
+	if (!ret)
+		return ret;
+
+	snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver",
+		 basename(path));
+
+	ret = readlink(link, path, PATH_MAX - 1);
+	if (ret < 0)
+		return ret;
+
+	path[ret] = '\0';
+	strncpy(name, basename(path), bufsize);
+
+	return 0;
+}
+
+static int drm_is_driver(struct tcb *tcp, const char *name)
+{
+	char drv[DRM_MAX_NAME_LEN];
+	int ret;
+
+	ret = drm_get_driver_name(tcp, drv, DRM_MAX_NAME_LEN);
+	if (ret)
+		return 0;
+
+	return strcmp(name, drv) == 0;
+}
+
+int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
+{
+	const struct_ioctlent *iop;
+
+	/* Check for device specific ioctls */
+	if (_IOC_NR(tcp->u_arg[1]) >= DRM_COMMAND_BASE &&
+	    _IOC_NR(tcp->u_arg[1]) < DRM_COMMAND_END) {
+		if (verbose(tcp) && drm_is_driver(tcp, "i915"))
+			return drm_i915_ioctl(tcp, code, arg);
+	}
+
+	if (entering(tcp)) {
+		/* Fall back to default behaviour */
+		iop = ioctl_lookup(tcp->u_arg[1]);
+		if (iop) {
+			tprints(iop->symbol);
+			while ((iop = ioctl_next_match(iop)))
+				tprintf(" or %s", iop->symbol);
+		} else {
+			ioctl_print_code(tcp->u_arg[1]);
+		}
+	}
+
+	return 0;
+}
diff --git a/drm_i915.c b/drm_i915.c
new file mode 100644
index 0000000..860e22b
--- /dev/null
+++ b/drm_i915.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2015 Intel Corporation
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * Authors:
+ *    Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
+ */
+
+#include "defs.h"
+
+#include <drm/drm.h>
+#include <drm/i915_drm.h>
+#include "xlat/drm_i915_ioctls.h"
+
+int drm_i915_ioctl(struct tcb *tcp, const unsigned int code, long arg)
+{
+	if (entering(tcp))
+		printxval(drm_i915_ioctls, code, "I915_IOCTL_???");
+
+	return 0;
+}
diff --git a/ioctl.c b/ioctl.c
index c67d048..225e850 100644
--- a/ioctl.c
+++ b/ioctl.c
@@ -216,6 +216,8 @@ ioctl_decode_command_number(unsigned int arg)
 				return 1;
 			}
 			return 0;
+		case 'd': /* DRM */
+			return 1;
 		default:
 			return 0;
 	}
@@ -252,6 +254,8 @@ ioctl_decode(struct tcb *tcp, unsigned int code, long arg)
 		return ubi_ioctl(tcp, code, arg);
 	case 'V':
 		return v4l2_ioctl(tcp, code, arg);
+	case 'd':
+		return drm_ioctl(tcp, code, arg);
 	case '=':
 		return ptp_ioctl(tcp, code, arg);
 #ifdef HAVE_LINUX_INPUT_H
@@ -284,6 +288,7 @@ ioctl_decode(struct tcb *tcp, unsigned int code, long arg)
  *   d	sys/des.h			(possible overlap)
  *   d	vax/dkio.h			(possible overlap)
  *   d	vaxuba/rxreg.h			(possible overlap)
+ *   d  drm/drm.h
  *   f	sys/filio.h
  *   g	sunwindow/win_ioctl.h		-no overlap-
  *   g	sunwindowdev/winioctl.c		!no manifest constant! -no overlap-
diff --git a/xlat/drm_i915_ioctls.in b/xlat/drm_i915_ioctls.in
new file mode 100644
index 0000000..21c3397
--- /dev/null
+++ b/xlat/drm_i915_ioctls.in
@@ -0,0 +1,51 @@
+/* Unfortunately i915 collides with other DRM drivers so we must specify these */
+DRM_IOCTL_I915_INIT
+DRM_IOCTL_I915_FLUSH
+DRM_IOCTL_I915_FLIP
+DRM_IOCTL_I915_BATCHBUFFER
+DRM_IOCTL_I915_IRQ_EMIT
+DRM_IOCTL_I915_IRQ_WAIT
+DRM_IOCTL_I915_GETPARAM
+DRM_IOCTL_I915_SETPARAM
+DRM_IOCTL_I915_ALLOC
+DRM_IOCTL_I915_FREE
+DRM_IOCTL_I915_INIT_HEAP
+DRM_IOCTL_I915_CMDBUFFER
+DRM_IOCTL_I915_DESTROY_HEAP
+DRM_IOCTL_I915_SET_VBLANK_PIPE
+DRM_IOCTL_I915_GET_VBLANK_PIPE
+DRM_IOCTL_I915_VBLANK_SWAP
+DRM_IOCTL_I915_HWS_ADDR
+DRM_IOCTL_I915_GEM_INIT
+DRM_IOCTL_I915_GEM_EXECBUFFER
+DRM_IOCTL_I915_GEM_EXECBUFFER2
+DRM_IOCTL_I915_GEM_PIN
+DRM_IOCTL_I915_GEM_UNPIN
+DRM_IOCTL_I915_GEM_BUSY
+DRM_IOCTL_I915_GEM_SET_CACHING
+DRM_IOCTL_I915_GEM_GET_CACHING
+DRM_IOCTL_I915_GEM_THROTTLE
+DRM_IOCTL_I915_GEM_ENTERVT
+DRM_IOCTL_I915_GEM_LEAVEVT
+DRM_IOCTL_I915_GEM_CREATE
+DRM_IOCTL_I915_GEM_PREAD
+DRM_IOCTL_I915_GEM_PWRITE
+DRM_IOCTL_I915_GEM_MMAP
+DRM_IOCTL_I915_GEM_MMAP_GTT
+DRM_IOCTL_I915_GEM_SET_DOMAIN
+DRM_IOCTL_I915_GEM_SW_FINISH
+DRM_IOCTL_I915_GEM_SET_TILING
+DRM_IOCTL_I915_GEM_GET_TILING
+DRM_IOCTL_I915_GEM_GET_APERTURE
+DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID
+DRM_IOCTL_I915_GEM_MADVISE
+DRM_IOCTL_I915_OVERLAY_PUT_IMAGE
+DRM_IOCTL_I915_OVERLAY_ATTRS
+DRM_IOCTL_I915_SET_SPRITE_COLORKEY
+DRM_IOCTL_I915_GET_SPRITE_COLORKEY
+DRM_IOCTL_I915_GEM_WAIT
+DRM_IOCTL_I915_GEM_CONTEXT_CREATE
+DRM_IOCTL_I915_GEM_CONTEXT_DESTROY
+DRM_IOCTL_I915_REG_READ
+DRM_IOCTL_I915_GET_RESET_STATS
+DRM_IOCTL_I915_GEM_USERPTR
-- 
2.1.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC 2/2] strace/drm: Print args for most common i915 ioctls

[Patrik Jakobsson]

Signed-off-by: Patrik Jakobsson <patrik.jakobsson-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drm_i915.c                 | 235 +++++++++++++++++++++++++++++++++++++++++++++
 xlat/drm_i915_getparams.in |  28 ++++++
 xlat/drm_i915_setparams.in |   4 +
 3 files changed, 267 insertions(+)
 create mode 100644 xlat/drm_i915_getparams.in
 create mode 100644 xlat/drm_i915_setparams.in

diff --git a/drm_i915.c b/drm_i915.c
index 860e22b..a93f01f 100644
--- a/drm_i915.c
+++ b/drm_i915.c
@@ -33,11 +33,246 @@
 #include <drm/drm.h>
 #include <drm/i915_drm.h>
 #include "xlat/drm_i915_ioctls.h"
+#include "xlat/drm_i915_getparams.h"
+#include "xlat/drm_i915_setparams.h"
+
+static int i915_getparam(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_getparam param;
+	int value;
+
+	if (entering(tcp) || umove(tcp, arg, &param))
+		return 0;
+	if (umove(tcp, (long)param.value, &value))
+		return 0;
+
+	tprintf(", {param=");
+	printxval(drm_i915_getparams, param.param, "I915_PARAM_???");
+	tprintf(", value=");
+	switch (param.param) {
+	case I915_PARAM_CHIPSET_ID:
+		tprintf("0x%04x", value);
+		break;
+	default:
+		tprintf("%d", value);
+	}
+	tprintf("}");
+
+	return 1;
+}
+
+static int i915_setparam(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_setparam param;
+
+	if (exiting(tcp) || umove(tcp, arg, &param))
+		return 0;
+
+	tprintf(", {param=");
+	printxval(drm_i915_setparams, param.param, "I915_PARAM_???");
+	tprintf(", value=%d}", param.value);
+
+	return 1;
+}
+
+static int i915_gem_execbuffer2(struct tcb *tcp, const unsigned int code,
+				long arg)
+{
+	struct drm_i915_gem_execbuffer2 eb;
+
+	if (exiting(tcp) || umove(tcp, arg, &eb))
+		return 0;
+
+	tprintf(", {buffers_ptr=%p, buffer_count=%u, "
+		"batch_start_offset=%x, batch_len=%u, DR1=%u, DR4=%u, "
+		"num_cliprects=%u, cliprects_ptr=%p, flags=0x%Lx}",
+		(void *)eb.buffers_ptr, eb.buffer_count, eb.batch_start_offset,
+		eb.batch_len, eb.DR1, eb.DR4, eb.num_cliprects,
+		(void *)eb.cliprects_ptr, eb.flags);
+
+	return 1;
+}
+
+static int i915_gem_busy(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_busy busy;
+
+	if (entering(tcp) || umove(tcp, arg, &busy))
+		return 0;
+
+	tprintf(", {handle=%u, busy=%c, ring=%u}",
+		busy.handle, (busy.busy & 0x1) ? 'Y' : 'N', (busy.busy >> 16));
+
+	return 1;
+}
+
+static int i915_gem_create(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_create create;
+
+	if (entering(tcp) || umove(tcp, arg, &create))
+		return 0;
+
+	tprintf(", {size=%Lu, handle=%u}", create.size, create.handle);
+
+	return 1;
+}
+
+static int i915_gem_pread(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_pread pr;
+
+	if (exiting(tcp) || umove(tcp, arg, &pr))
+		return 0;
+
+	tprintf(", {handle=%u, offset=%Lu, size=%Lu, data_ptr=%p}",
+		pr.handle, pr.offset, pr.size, (void *)pr.data_ptr);
+
+	return 1;
+}
+
+static int i915_gem_pwrite(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_pwrite pw;
+
+	if (exiting(tcp) || umove(tcp, arg, &pw))
+		return 0;
+
+	tprintf(", {handle=%u, offset=%Lu, size=%Lu, data_ptr=%p}",
+		pw.handle, pw.offset, pw.size, (void *)pw.data_ptr);
+
+	return 1;
+}
+
+static int i915_gem_mmap(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_mmap mmap;
+
+	if (entering(tcp) || umove(tcp, arg, &mmap))
+		return 0;
+
+	tprintf(", {handle=%u, offset=%Lu, size=%Lu, addr_ptr=%p}",
+		mmap.handle, mmap.offset, mmap.size, (void *)mmap.addr_ptr);
+
+	return 1;
+}
+
+static int i915_gem_mmap_gtt(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_mmap_gtt mmap;
+
+	if (entering(tcp) || umove(tcp, arg, &mmap))
+		return 0;
+
+	tprintf(", {handle=%u, offset=%Lu}", mmap.handle, mmap.offset);
+
+	return 1;
+}
+
+static int i915_gem_set_domain(struct tcb *tcp, const unsigned int code,
+			       long arg)
+{
+	struct drm_i915_gem_set_domain dom;
+
+	if (entering(tcp) || umove(tcp, arg, &dom))
+		return 0;
+
+	tprintf(", {handle=%u, read_domains=%x, write_domain=%x}",
+		dom.handle, dom.read_domains, dom.write_domain);
+
+	return 1;
+}
+
+static int i915_gem_madvise(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_madvise madv;
+
+	if (entering(tcp) || umove(tcp, arg, &madv))
+		return 0;
+
+	tprintf(", {handle=%u, madv=%u, retained=%u}",
+		madv.handle, madv.madv, madv.retained);
+
+	return 1;
+}
+
+static int i915_gem_get_tiling(struct tcb *tcp, const unsigned int code,
+			       long arg)
+{
+	struct drm_i915_gem_get_tiling tiling;
+
+	if (entering(tcp) || umove(tcp, arg, &tiling))
+		return 0;
+
+	tprintf(", {handle=%u, tiling_mode=%u, swizzle_mode=%u}",
+		tiling.handle, tiling.tiling_mode, tiling.swizzle_mode);
+
+	return 1;
+}
+
+static int i915_gem_set_tiling(struct tcb *tcp, const unsigned int code,
+			       long arg)
+{
+	struct drm_i915_gem_set_tiling tiling;
+
+	if (entering(tcp) || umove(tcp, arg, &tiling))
+		return 0;
+
+	tprintf(", {handle=%u, tiling_mode=%u, stride=%u, swizzle_mode=%u}",
+		tiling.handle, tiling.tiling_mode, tiling.stride,
+		tiling.swizzle_mode);
+
+	return 1;
+}
+
+static int i915_gem_userptr(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_i915_gem_userptr uptr;
+
+	if (entering(tcp) || umove(tcp, arg, &uptr))
+		return 0;
+
+	tprintf(", {user_ptr=%p, user_size=%Lu, flags=0x%x, handle=%u}",
+		(void *)uptr.user_ptr, uptr.user_size, uptr.flags, uptr.handle);
+
+	return 1;
+}
 
 int drm_i915_ioctl(struct tcb *tcp, const unsigned int code, long arg)
 {
 	if (entering(tcp))
 		printxval(drm_i915_ioctls, code, "I915_IOCTL_???");
 
+	switch (code) {
+	case DRM_IOCTL_I915_GETPARAM:
+		return i915_getparam(tcp, code, arg);
+	case DRM_IOCTL_I915_SETPARAM:
+		return i915_setparam(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_EXECBUFFER2:
+		return i915_gem_execbuffer2(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_BUSY:
+		return i915_gem_busy(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_CREATE:
+		return i915_gem_create(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_PREAD:
+		return i915_gem_pread(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_PWRITE:
+		return i915_gem_pwrite(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_MMAP:
+		return i915_gem_mmap(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_MMAP_GTT:
+		return i915_gem_mmap_gtt(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_SET_DOMAIN:
+		return i915_gem_set_domain(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_MADVISE:
+		return i915_gem_madvise(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_GET_TILING:
+		return i915_gem_get_tiling(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_SET_TILING:
+		return i915_gem_set_tiling(tcp, code, arg);
+	case DRM_IOCTL_I915_GEM_USERPTR:
+		return i915_gem_userptr(tcp, code, arg);
+	}
+
 	return 0;
 }
diff --git a/xlat/drm_i915_getparams.in b/xlat/drm_i915_getparams.in
new file mode 100644
index 0000000..15275cb
--- /dev/null
+++ b/xlat/drm_i915_getparams.in
@@ -0,0 +1,28 @@
+I915_PARAM_IRQ_ACTIVE
+I915_PARAM_ALLOW_BATCHBUFFER
+I915_PARAM_LAST_DISPATCH
+I915_PARAM_CHIPSET_ID
+I915_PARAM_HAS_GEM
+I915_PARAM_NUM_FENCES_AVAIL
+I915_PARAM_HAS_OVERLAY
+I915_PARAM_HAS_PAGEFLIPPING
+I915_PARAM_HAS_EXECBUF2
+I915_PARAM_HAS_BSD
+I915_PARAM_HAS_BLT
+I915_PARAM_HAS_RELAXED_FENCING
+I915_PARAM_HAS_COHERENT_RINGS
+I915_PARAM_HAS_EXEC_CONSTANTS
+I915_PARAM_HAS_RELAXED_DELTA
+I915_PARAM_HAS_GEN7_SOL_RESET
+I915_PARAM_HAS_LLC
+I915_PARAM_HAS_ALIASING_PPGTT
+I915_PARAM_HAS_WAIT_TIMEOUT
+I915_PARAM_HAS_SEMAPHORES
+I915_PARAM_HAS_PRIME_VMAP_FLUSH
+I915_PARAM_HAS_VEBOX
+I915_PARAM_HAS_SECURE_BATCHES
+I915_PARAM_HAS_PINNED_BATCHES
+I915_PARAM_HAS_EXEC_NO_RELOC
+I915_PARAM_HAS_EXEC_HANDLE_LUT
+I915_PARAM_HAS_WT
+I915_PARAM_CMD_PARSER_VERSION
diff --git a/xlat/drm_i915_setparams.in b/xlat/drm_i915_setparams.in
new file mode 100644
index 0000000..d93d2ea
--- /dev/null
+++ b/xlat/drm_i915_setparams.in
@@ -0,0 +1,4 @@
+I915_SETPARAM_USE_MI_BATCHBUFFER_START
+I915_SETPARAM_TEX_LRU_LOG_GRANULARITY
+I915_SETPARAM_ALLOW_BATCHBUFFER
+I915_SETPARAM_NUM_USED_FENCES
-- 
2.1.0


------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y

Re: [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Jesse Barnes]

On 05/06/2015 07:48 AM, Patrik Jakobsson wrote:
> This patch set aims to make strace more useful when tracing i915 ioctls.
> The ioctl type is first checked for being drm and then the driver
> backing the opened device is identified by looking at sysfs. Other
> drivers than i915 can easily be added.
> 
> Only a subset of the i915 ioctls are included. I will extend this patch
> set if the approach looks ok. The generic drm ioctls are also missing.
> 
> Give it a spin with:
>         strace -e trace=ioctl -p `pidof X`
> 
> Patrik Jakobsson (2):
>   strace/drm: Print extended info for drm and i915 ioctls
>   strace/drm: Print args for most common i915 ioctls
> 
>  Makefile.am                |   2 +
>  defs.h                     |   2 +
>  drm.c                      | 104 +++++++++++++++++
>  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
>  ioctl.c                    |   5 +
>  xlat/drm_i915_getparams.in |  28 +++++
>  xlat/drm_i915_ioctls.in    |  51 +++++++++
>  xlat/drm_i915_setparams.in |   4 +
>  8 files changed, 474 insertions(+)
>  create mode 100644 drm.c
>  create mode 100644 drm_i915.c
>  create mode 100644 xlat/drm_i915_getparams.in
>  create mode 100644 xlat/drm_i915_ioctls.in
>  create mode 100644 xlat/drm_i915_setparams.in

Yeah, this looks pretty cool to me.  I'm not familiar with strace
internals, but the split and extensible design looks reasonable; should
make it easy to add other drivers and such in the future.

Who on the strace side can pick this up?

Thanks,
Jesse

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Patrik Jakobsson]

On Thu, May 07, 2015 at 08:37:40AM -0700, Jesse Barnes wrote:
> On 05/06/2015 07:48 AM, Patrik Jakobsson wrote:
> > This patch set aims to make strace more useful when tracing i915 ioctls.
> > The ioctl type is first checked for being drm and then the driver
> > backing the opened device is identified by looking at sysfs. Other
> > drivers than i915 can easily be added.
> > 
> > Only a subset of the i915 ioctls are included. I will extend this patch
> > set if the approach looks ok. The generic drm ioctls are also missing.
> > 
> > Give it a spin with:
> >         strace -e trace=ioctl -p `pidof X`
> > 
> > Patrik Jakobsson (2):
> >   strace/drm: Print extended info for drm and i915 ioctls
> >   strace/drm: Print args for most common i915 ioctls
> > 
> >  Makefile.am                |   2 +
> >  defs.h                     |   2 +
> >  drm.c                      | 104 +++++++++++++++++
> >  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> >  ioctl.c                    |   5 +
> >  xlat/drm_i915_getparams.in |  28 +++++
> >  xlat/drm_i915_ioctls.in    |  51 +++++++++
> >  xlat/drm_i915_setparams.in |   4 +
> >  8 files changed, 474 insertions(+)
> >  create mode 100644 drm.c
> >  create mode 100644 drm_i915.c
> >  create mode 100644 xlat/drm_i915_getparams.in
> >  create mode 100644 xlat/drm_i915_ioctls.in
> >  create mode 100644 xlat/drm_i915_setparams.in
> 
> Yeah, this looks pretty cool to me.  I'm not familiar with strace
> internals, but the split and extensible design looks reasonable; should
> make it easy to add other drivers and such in the future.
> 
> Who on the strace side can pick this up?

Hi Dmitry

Can I bother you with a review?

Thanks
Patrik

> 
> Thanks,
> Jesse
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Gabriel Laskar]

On Wed,  6 May 2015 16:48:01 +0200
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:

> This patch set aims to make strace more useful when tracing i915 ioctls.
> The ioctl type is first checked for being drm and then the driver
> backing the opened device is identified by looking at sysfs. Other
> drivers than i915 can easily be added.
> 
> Only a subset of the i915 ioctls are included. I will extend this patch
> set if the approach looks ok. The generic drm ioctls are also missing.
> 
> Give it a spin with:
>         strace -e trace=ioctl -p `pidof X`
> 
> Patrik Jakobsson (2):
>   strace/drm: Print extended info for drm and i915 ioctls
>   strace/drm: Print args for most common i915 ioctls
> 
>  Makefile.am                |   2 +
>  defs.h                     |   2 +
>  drm.c                      | 104 +++++++++++++++++
>  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
>  ioctl.c                    |   5 +
>  xlat/drm_i915_getparams.in |  28 +++++
>  xlat/drm_i915_ioctls.in    |  51 +++++++++
>  xlat/drm_i915_setparams.in |   4 +
>  8 files changed, 474 insertions(+)
>  create mode 100644 drm.c
>  create mode 100644 drm_i915.c
>  create mode 100644 xlat/drm_i915_getparams.in
>  create mode 100644 xlat/drm_i915_ioctls.in
>  create mode 100644 xlat/drm_i915_setparams.in
> 

This is a great start! We need this kind of decoding. Do you plan to
add also the generic drm ioctl decoding?

Some issues though:

* The way you avoid the ioctl request decoding is quite hard to follow,
  but it seems that you don't have much of a choice, except that in
  drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it
  needs some work here, to allow a simpler code path. Maybe this would
  be clearer if the decoding/drm_get_driver_name, etc… was in
  ioctl_decode_command_number(). Also, with the actual code, if you are
  on i915 with an invalid ioctl number, it will be printed as
  "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This
  will also add an inconsistent result depending whether /sys is
  mounted or not.
* This does not compile on my system (archlinux), because drm.h lives
  in libdrm/drm.h and not in drm/drm.h, I know it is an rfc, but this
  needs to use pkg-config in order to know where libdrm headers are.


#include <fcntl.h>
#include <sys/ioctl.h>
#include <unistd.h>

#include <drm.h>
#include <i915_drm.h>

int main()
{
	int fd = open("/dev/dri/card0", O_RDWR);
	ioctl(fd, DRM_IOW(DRM_COMMAND_BASE + DRM_I915_FLUSH, int), 0);
	return 0;
}

$ CFLAGS=-I/usr/include/libdrm make i915-ioctls
cc -I/usr/include/libdrm    i915-ioctls.c   -o i915-ioctls
$ sudo strace -e ioctl ./i915-ioctls 
ioctl(3, _IOC(_IOC_WRITE, 0x64, 0x41, 0x04), 0) = 0
+++ exited with 0 +++
$ sudo ~/source/strace/strace -e ioctl ./i915-ioctls 
ioctl(3, 0x40046441 /* I915_IOCTL_??? */, 0) = 0
+++ exited with 0 +++


-- 
Gabriel Laskar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Patrik Jakobsson]

On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote:
> On Wed,  6 May 2015 16:48:01 +0200
> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> 
> > This patch set aims to make strace more useful when tracing i915 ioctls.
> > The ioctl type is first checked for being drm and then the driver
> > backing the opened device is identified by looking at sysfs. Other
> > drivers than i915 can easily be added.
> > 
> > Only a subset of the i915 ioctls are included. I will extend this patch
> > set if the approach looks ok. The generic drm ioctls are also missing.
> > 
> > Give it a spin with:
> >         strace -e trace=ioctl -p `pidof X`
> > 
> > Patrik Jakobsson (2):
> >   strace/drm: Print extended info for drm and i915 ioctls
> >   strace/drm: Print args for most common i915 ioctls
> > 
> >  Makefile.am                |   2 +
> >  defs.h                     |   2 +
> >  drm.c                      | 104 +++++++++++++++++
> >  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> >  ioctl.c                    |   5 +
> >  xlat/drm_i915_getparams.in |  28 +++++
> >  xlat/drm_i915_ioctls.in    |  51 +++++++++
> >  xlat/drm_i915_setparams.in |   4 +
> >  8 files changed, 474 insertions(+)
> >  create mode 100644 drm.c
> >  create mode 100644 drm_i915.c
> >  create mode 100644 xlat/drm_i915_getparams.in
> >  create mode 100644 xlat/drm_i915_ioctls.in
> >  create mode 100644 xlat/drm_i915_setparams.in
> > 
> 
> This is a great start! We need this kind of decoding. Do you plan to
> add also the generic drm ioctl decoding?

Thanks for the review. Yes, my plan is to add generic drm ioctls as well.

> 
> Some issues though:
> 
> * The way you avoid the ioctl request decoding is quite hard to follow,
>   but it seems that you don't have much of a choice, except that in
>   drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it
>   needs some work here, to allow a simpler code path. Maybe this would
>   be clearer if the decoding/drm_get_driver_name, etc… was in
>   ioctl_decode_command_number(). Also, with the actual code, if you are
>   on i915 with an invalid ioctl number, it will be printed as
>   "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This
>   will also add an inconsistent result depending whether /sys is
>   mounted or not.

Yes, moving it to ioctl_decode_command_number() makes sense. I'll do that.
And I'll make the output consistent and skip the I915_IOCTL_???. It comes with
the drawback of possibly duplicated entries when doing the lookup even though we
know we're talking to i915, but it is still nicer than _???.

> * This does not compile on my system (archlinux), because drm.h lives
>   in libdrm/drm.h and not in drm/drm.h, I know it is an rfc, but this
>   needs to use pkg-config in order to know where libdrm headers are.

In theory libdrm is not needed since the headers are available in uapi, so I
tried to avoid adding an additional dependency. But since distros do not
guarantee the existance of <drm/drm.h> it might be better to use libdrm. I'll
look into that.

Thanks
Patrik

> 
> 
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <unistd.h>
> 
> #include <drm.h>
> #include <i915_drm.h>
> 
> int main()
> {
> 	int fd = open("/dev/dri/card0", O_RDWR);
> 	ioctl(fd, DRM_IOW(DRM_COMMAND_BASE + DRM_I915_FLUSH, int), 0);
> 	return 0;
> }
> 
> $ CFLAGS=-I/usr/include/libdrm make i915-ioctls
> cc -I/usr/include/libdrm    i915-ioctls.c   -o i915-ioctls
> $ sudo strace -e ioctl ./i915-ioctls 
> ioctl(3, _IOC(_IOC_WRITE, 0x64, 0x41, 0x04), 0) = 0
> +++ exited with 0 +++
> $ sudo ~/source/strace/strace -e ioctl ./i915-ioctls 
> ioctl(3, 0x40046441 /* I915_IOCTL_??? */, 0) = 0
> +++ exited with 0 +++
> 
> 
> -- 
> Gabriel Laskar

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Re: [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Gabriel Laskar]

On Mon, 11 May 2015 15:54:24 +0200
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:

> On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote:
> > On Wed,  6 May 2015 16:48:01 +0200
> > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> > 
> > > This patch set aims to make strace more useful when tracing i915 ioctls.
> > > The ioctl type is first checked for being drm and then the driver
> > > backing the opened device is identified by looking at sysfs. Other
> > > drivers than i915 can easily be added.
> > > 
> > > Only a subset of the i915 ioctls are included. I will extend this patch
> > > set if the approach looks ok. The generic drm ioctls are also missing.
> > > 
> > > Give it a spin with:
> > >         strace -e trace=ioctl -p `pidof X`
> > > 
> > > Patrik Jakobsson (2):
> > >   strace/drm: Print extended info for drm and i915 ioctls
> > >   strace/drm: Print args for most common i915 ioctls
> > > 
> > >  Makefile.am                |   2 +
> > >  defs.h                     |   2 +
> > >  drm.c                      | 104 +++++++++++++++++
> > >  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> > >  ioctl.c                    |   5 +
> > >  xlat/drm_i915_getparams.in |  28 +++++
> > >  xlat/drm_i915_ioctls.in    |  51 +++++++++
> > >  xlat/drm_i915_setparams.in |   4 +
> > >  8 files changed, 474 insertions(+)
> > >  create mode 100644 drm.c
> > >  create mode 100644 drm_i915.c
> > >  create mode 100644 xlat/drm_i915_getparams.in
> > >  create mode 100644 xlat/drm_i915_ioctls.in
> > >  create mode 100644 xlat/drm_i915_setparams.in
> > > 
> > 
> > This is a great start! We need this kind of decoding. Do you plan to
> > add also the generic drm ioctl decoding?
> 
> Thanks for the review. Yes, my plan is to add generic drm ioctls as well.
> 
> > 
> > Some issues though:
> > 
> > * The way you avoid the ioctl request decoding is quite hard to follow,
> >   but it seems that you don't have much of a choice, except that in
> >   drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it
> >   needs some work here, to allow a simpler code path. Maybe this would
> >   be clearer if the decoding/drm_get_driver_name, etc… was in
> >   ioctl_decode_command_number(). Also, with the actual code, if you are
> >   on i915 with an invalid ioctl number, it will be printed as
> >   "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This
> >   will also add an inconsistent result depending whether /sys is
> >   mounted or not.
> 
> Yes, moving it to ioctl_decode_command_number() makes sense. I'll do that.
> And I'll make the output consistent and skip the I915_IOCTL_???. It comes with
> the drawback of possibly duplicated entries when doing the lookup even though we
> know we're talking to i915, but it is still nicer than _???.

If you call all the request decoding code from
ioctl_decode_command_number() you will still be able to determine if
you are on i915, and write the correct request, but the fallback code
will be no longer duplicated. You will have to call xlookup() instead of
printxval(), in order to be able to know when the decoding fail though.

> > * This does not compile on my system (archlinux), because drm.h lives
> >   in libdrm/drm.h and not in drm/drm.h, I know it is an rfc, but this
> >   needs to use pkg-config in order to know where libdrm headers are.
> 
> In theory libdrm is not needed since the headers are available in uapi, so I
> tried to avoid adding an additional dependency. But since distros do not
> guarantee the existance of <drm/drm.h> it might be better to use libdrm. I'll
> look into that.

It seems that on archlinux, they remove intentionally[1] the headers for
drm. For others, I can't say.

[1]: https://projects.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/linux-api-headers#n40

-- 
Gabriel Laskar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Patrik Jakobsson]

On Mon, May 11, 2015 at 08:08:19PM +0200, Gabriel Laskar wrote:
> On Mon, 11 May 2015 15:54:24 +0200
> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> 
> > On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote:
> > > On Wed,  6 May 2015 16:48:01 +0200
> > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> > > 
> > > > This patch set aims to make strace more useful when tracing i915 ioctls.
> > > > The ioctl type is first checked for being drm and then the driver
> > > > backing the opened device is identified by looking at sysfs. Other
> > > > drivers than i915 can easily be added.
> > > > 
> > > > Only a subset of the i915 ioctls are included. I will extend this patch
> > > > set if the approach looks ok. The generic drm ioctls are also missing.
> > > > 
> > > > Give it a spin with:
> > > >         strace -e trace=ioctl -p `pidof X`
> > > > 
> > > > Patrik Jakobsson (2):
> > > >   strace/drm: Print extended info for drm and i915 ioctls
> > > >   strace/drm: Print args for most common i915 ioctls
> > > > 
> > > >  Makefile.am                |   2 +
> > > >  defs.h                     |   2 +
> > > >  drm.c                      | 104 +++++++++++++++++
> > > >  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> > > >  ioctl.c                    |   5 +
> > > >  xlat/drm_i915_getparams.in |  28 +++++
> > > >  xlat/drm_i915_ioctls.in    |  51 +++++++++
> > > >  xlat/drm_i915_setparams.in |   4 +
> > > >  8 files changed, 474 insertions(+)
> > > >  create mode 100644 drm.c
> > > >  create mode 100644 drm_i915.c
> > > >  create mode 100644 xlat/drm_i915_getparams.in
> > > >  create mode 100644 xlat/drm_i915_ioctls.in
> > > >  create mode 100644 xlat/drm_i915_setparams.in
> > > > 
> > > 
> > > This is a great start! We need this kind of decoding. Do you plan to
> > > add also the generic drm ioctl decoding?
> > 
> > Thanks for the review. Yes, my plan is to add generic drm ioctls as well.
> > 
> > > 
> > > Some issues though:
> > > 
> > > * The way you avoid the ioctl request decoding is quite hard to follow,
> > >   but it seems that you don't have much of a choice, except that in
> > >   drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it
> > >   needs some work here, to allow a simpler code path. Maybe this would
> > >   be clearer if the decoding/drm_get_driver_name, etc… was in
> > >   ioctl_decode_command_number(). Also, with the actual code, if you are
> > >   on i915 with an invalid ioctl number, it will be printed as
> > >   "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This
> > >   will also add an inconsistent result depending whether /sys is
> > >   mounted or not.
> > 
> > Yes, moving it to ioctl_decode_command_number() makes sense. I'll do that.
> > And I'll make the output consistent and skip the I915_IOCTL_???. It comes with
> > the drawback of possibly duplicated entries when doing the lookup even though we
> > know we're talking to i915, but it is still nicer than _???.
> 
> If you call all the request decoding code from
> ioctl_decode_command_number() you will still be able to determine if
> you are on i915, and write the correct request, but the fallback code
> will be no longer duplicated. You will have to call xlookup() instead of
> printxval(), in order to be able to know when the decoding fail though.

Unfortunately I cannot check for i915 in _decode_command_number since I don't
have the full tcb context (cannot figure out the path, etc.). That's probably
why I had to duplicate the decoding in the first place. The only other solution
I see is to detect i915 early and store it globally somewhere but that is rather
nasty. Not sure how to do this in a better way. What am I missing?

> 
> > > * This does not compile on my system (archlinux), because drm.h lives
> > >   in libdrm/drm.h and not in drm/drm.h, I know it is an rfc, but this
> > >   needs to use pkg-config in order to know where libdrm headers are.
> > 
> > In theory libdrm is not needed since the headers are available in uapi, so I
> > tried to avoid adding an additional dependency. But since distros do not
> > guarantee the existance of <drm/drm.h> it might be better to use libdrm. I'll
> > look into that.
> 
> It seems that on archlinux, they remove intentionally[1] the headers for
> drm. For others, I can't say.
> 
> [1]: https://projects.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/linux-api-headers#n40

Yes, it's prefered to use the headers from libdrm. It's actually not a bad thing
that Arch Linux enforces this. I'll just have to add libdrm as a dependency for
drm ioctl decoding.

> 
> -- 
> Gabriel Laskar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Gabriel Laskar]

On Tue, 12 May 2015 14:35:28 +0200
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:

> On Mon, May 11, 2015 at 08:08:19PM +0200, Gabriel Laskar wrote:
> > On Mon, 11 May 2015 15:54:24 +0200
> > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> > 
> > > On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote:
> > > > On Wed,  6 May 2015 16:48:01 +0200
> > > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> > > > 
> > > > > This patch set aims to make strace more useful when tracing i915 ioctls.
> > > > > The ioctl type is first checked for being drm and then the driver
> > > > > backing the opened device is identified by looking at sysfs. Other
> > > > > drivers than i915 can easily be added.
> > > > > 
> > > > > Only a subset of the i915 ioctls are included. I will extend this patch
> > > > > set if the approach looks ok. The generic drm ioctls are also missing.
> > > > > 
> > > > > Give it a spin with:
> > > > >         strace -e trace=ioctl -p `pidof X`
> > > > > 
> > > > > Patrik Jakobsson (2):
> > > > >   strace/drm: Print extended info for drm and i915 ioctls
> > > > >   strace/drm: Print args for most common i915 ioctls
> > > > > 
> > > > >  Makefile.am                |   2 +
> > > > >  defs.h                     |   2 +
> > > > >  drm.c                      | 104 +++++++++++++++++
> > > > >  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> > > > >  ioctl.c                    |   5 +
> > > > >  xlat/drm_i915_getparams.in |  28 +++++
> > > > >  xlat/drm_i915_ioctls.in    |  51 +++++++++
> > > > >  xlat/drm_i915_setparams.in |   4 +
> > > > >  8 files changed, 474 insertions(+)
> > > > >  create mode 100644 drm.c
> > > > >  create mode 100644 drm_i915.c
> > > > >  create mode 100644 xlat/drm_i915_getparams.in
> > > > >  create mode 100644 xlat/drm_i915_ioctls.in
> > > > >  create mode 100644 xlat/drm_i915_setparams.in
> > > > > 
> > > > 
> > > > This is a great start! We need this kind of decoding. Do you plan to
> > > > add also the generic drm ioctl decoding?
> > > 
> > > Thanks for the review. Yes, my plan is to add generic drm ioctls as well.
> > > 
> > > > 
> > > > Some issues though:
> > > > 
> > > > * The way you avoid the ioctl request decoding is quite hard to follow,
> > > >   but it seems that you don't have much of a choice, except that in
> > > >   drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it
> > > >   needs some work here, to allow a simpler code path. Maybe this would
> > > >   be clearer if the decoding/drm_get_driver_name, etc… was in
> > > >   ioctl_decode_command_number(). Also, with the actual code, if you are
> > > >   on i915 with an invalid ioctl number, it will be printed as
> > > >   "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This
> > > >   will also add an inconsistent result depending whether /sys is
> > > >   mounted or not.
> > > 
> > > Yes, moving it to ioctl_decode_command_number() makes sense. I'll do that.
> > > And I'll make the output consistent and skip the I915_IOCTL_???. It comes with
> > > the drawback of possibly duplicated entries when doing the lookup even though we
> > > know we're talking to i915, but it is still nicer than _???.
> > 
> > If you call all the request decoding code from
> > ioctl_decode_command_number() you will still be able to determine if
> > you are on i915, and write the correct request, but the fallback code
> > will be no longer duplicated. You will have to call xlookup() instead of
> > printxval(), in order to be able to know when the decoding fail though.
> 
> Unfortunately I cannot check for i915 in _decode_command_number since I don't
> have the full tcb context (cannot figure out the path, etc.). That's probably
> why I had to duplicate the decoding in the first place. The only other solution
> I see is to detect i915 early and store it globally somewhere but that is rather
> nasty. Not sure how to do this in a better way. What am I missing?

Oh yes, I see. We can add tcb context to ioctl_decode_command_number(),
it should not be a problem. We didn't have the need for it for the
moment, that's all.

Dmitry? What do you think of this?

-- 
Gabriel Laskar

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Re: [Intel-gfx] [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Dmitry V. Levin]

On Tue, May 12, 2015 at 07:37:59PM +0200, Gabriel Laskar wrote:
> On Tue, 12 May 2015 14:35:28 +0200, Patrik Jakobsson wrote:
> > On Mon, May 11, 2015 at 08:08:19PM +0200, Gabriel Laskar wrote:
> > > On Mon, 11 May 2015 15:54:24 +0200, Patrik Jakobsson wrote:
> > > > On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote:
> > > > > On Wed,  6 May 2015 16:48:01 +0200, Patrik Jakobsson wrote:
> > > > > 
> > > > > > This patch set aims to make strace more useful when tracing i915 ioctls.
> > > > > > The ioctl type is first checked for being drm and then the driver
> > > > > > backing the opened device is identified by looking at sysfs. Other
> > > > > > drivers than i915 can easily be added.
> > > > > > 
> > > > > > Only a subset of the i915 ioctls are included. I will extend this patch
> > > > > > set if the approach looks ok. The generic drm ioctls are also missing.
> > > > > > 
> > > > > > Give it a spin with:
> > > > > >         strace -e trace=ioctl -p `pidof X`
> > > > > > 
> > > > > > Patrik Jakobsson (2):
> > > > > >   strace/drm: Print extended info for drm and i915 ioctls
> > > > > >   strace/drm: Print args for most common i915 ioctls
> > > > > > 
> > > > > >  Makefile.am                |   2 +
> > > > > >  defs.h                     |   2 +
> > > > > >  drm.c                      | 104 +++++++++++++++++
> > > > > >  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  ioctl.c                    |   5 +
> > > > > >  xlat/drm_i915_getparams.in |  28 +++++
> > > > > >  xlat/drm_i915_ioctls.in    |  51 +++++++++
> > > > > >  xlat/drm_i915_setparams.in |   4 +
> > > > > >  8 files changed, 474 insertions(+)
> > > > > >  create mode 100644 drm.c
> > > > > >  create mode 100644 drm_i915.c
> > > > > >  create mode 100644 xlat/drm_i915_getparams.in
> > > > > >  create mode 100644 xlat/drm_i915_ioctls.in
> > > > > >  create mode 100644 xlat/drm_i915_setparams.in
> > > > > > 
> > > > > 
> > > > > This is a great start! We need this kind of decoding. Do you plan to
> > > > > add also the generic drm ioctl decoding?
> > > > 
> > > > Thanks for the review. Yes, my plan is to add generic drm ioctls as well.
> > > > 
> > > > > 
> > > > > Some issues though:
> > > > > 
> > > > > * The way you avoid the ioctl request decoding is quite hard to follow,
> > > > >   but it seems that you don't have much of a choice, except that in
> > > > >   drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it
> > > > >   needs some work here, to allow a simpler code path. Maybe this would
> > > > >   be clearer if the decoding/drm_get_driver_name, etc… was in
> > > > >   ioctl_decode_command_number(). Also, with the actual code, if you are
> > > > >   on i915 with an invalid ioctl number, it will be printed as
> > > > >   "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This
> > > > >   will also add an inconsistent result depending whether /sys is
> > > > >   mounted or not.
> > > > 
> > > > Yes, moving it to ioctl_decode_command_number() makes sense. I'll do that.
> > > > And I'll make the output consistent and skip the I915_IOCTL_???. It comes with
> > > > the drawback of possibly duplicated entries when doing the lookup even though we
> > > > know we're talking to i915, but it is still nicer than _???.
> > > 
> > > If you call all the request decoding code from
> > > ioctl_decode_command_number() you will still be able to determine if
> > > you are on i915, and write the correct request, but the fallback code
> > > will be no longer duplicated. You will have to call xlookup() instead of
> > > printxval(), in order to be able to know when the decoding fail though.
> > 
> > Unfortunately I cannot check for i915 in _decode_command_number since I don't
> > have the full tcb context (cannot figure out the path, etc.). That's probably
> > why I had to duplicate the decoding in the first place. The only other solution
> > I see is to detect i915 early and store it globally somewhere but that is rather
> > nasty. Not sure how to do this in a better way. What am I missing?
> 
> Oh yes, I see. We can add tcb context to ioctl_decode_command_number(),
> it should not be a problem. We didn't have the need for it for the
> moment, that's all.
> 
> Dmitry? What do you think of this?

I had no chance to have a look at the code, but anyway, passing
tcb context down to ioctl_decode_command_number() shouldn't be a problem
at all.


-- 
ldv

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Re: [RFC 0/2] strace/drm: Add i915 ioctls to strace

[Patrik Jakobsson]

On Wed, May 13, 2015 at 01:10:17AM +0300, Dmitry V. Levin wrote:
> On Tue, May 12, 2015 at 07:37:59PM +0200, Gabriel Laskar wrote:
> > On Tue, 12 May 2015 14:35:28 +0200, Patrik Jakobsson wrote:
> > > On Mon, May 11, 2015 at 08:08:19PM +0200, Gabriel Laskar wrote:
> > > > On Mon, 11 May 2015 15:54:24 +0200, Patrik Jakobsson wrote:
> > > > > On Mon, May 11, 2015 at 12:50:36PM +0200, Gabriel Laskar wrote:
> > > > > > On Wed,  6 May 2015 16:48:01 +0200, Patrik Jakobsson wrote:
> > > > > > 
> > > > > > > This patch set aims to make strace more useful when tracing i915 ioctls.
> > > > > > > The ioctl type is first checked for being drm and then the driver
> > > > > > > backing the opened device is identified by looking at sysfs. Other
> > > > > > > drivers than i915 can easily be added.
> > > > > > > 
> > > > > > > Only a subset of the i915 ioctls are included. I will extend this patch
> > > > > > > set if the approach looks ok. The generic drm ioctls are also missing.
> > > > > > > 
> > > > > > > Give it a spin with:
> > > > > > >         strace -e trace=ioctl -p `pidof X`
> > > > > > > 
> > > > > > > Patrik Jakobsson (2):
> > > > > > >   strace/drm: Print extended info for drm and i915 ioctls
> > > > > > >   strace/drm: Print args for most common i915 ioctls
> > > > > > > 
> > > > > > >  Makefile.am                |   2 +
> > > > > > >  defs.h                     |   2 +
> > > > > > >  drm.c                      | 104 +++++++++++++++++
> > > > > > >  drm_i915.c                 | 278 +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  ioctl.c                    |   5 +
> > > > > > >  xlat/drm_i915_getparams.in |  28 +++++
> > > > > > >  xlat/drm_i915_ioctls.in    |  51 +++++++++
> > > > > > >  xlat/drm_i915_setparams.in |   4 +
> > > > > > >  8 files changed, 474 insertions(+)
> > > > > > >  create mode 100644 drm.c
> > > > > > >  create mode 100644 drm_i915.c
> > > > > > >  create mode 100644 xlat/drm_i915_getparams.in
> > > > > > >  create mode 100644 xlat/drm_i915_ioctls.in
> > > > > > >  create mode 100644 xlat/drm_i915_setparams.in
> > > > > > > 
> > > > > > 
> > > > > > This is a great start! We need this kind of decoding. Do you plan to
> > > > > > add also the generic drm ioctl decoding?
> > > > > 
> > > > > Thanks for the review. Yes, my plan is to add generic drm ioctls as well.
> > > > > 
> > > > > > 
> > > > > > Some issues though:
> > > > > > 
> > > > > > * The way you avoid the ioctl request decoding is quite hard to follow,
> > > > > >   but it seems that you don't have much of a choice, except that in
> > > > > >   drm_ioctl(), the code from SYS_FUNC(ioctl) is duplicated. It seems it
> > > > > >   needs some work here, to allow a simpler code path. Maybe this would
> > > > > >   be clearer if the decoding/drm_get_driver_name, etc… was in
> > > > > >   ioctl_decode_command_number(). Also, with the actual code, if you are
> > > > > >   on i915 with an invalid ioctl number, it will be printed as
> > > > > >   "I915_IOCTL_???" and not "_IOC(...)" (see below for an example.) This
> > > > > >   will also add an inconsistent result depending whether /sys is
> > > > > >   mounted or not.
> > > > > 
> > > > > Yes, moving it to ioctl_decode_command_number() makes sense. I'll do that.
> > > > > And I'll make the output consistent and skip the I915_IOCTL_???. It comes with
> > > > > the drawback of possibly duplicated entries when doing the lookup even though we
> > > > > know we're talking to i915, but it is still nicer than _???.
> > > > 
> > > > If you call all the request decoding code from
> > > > ioctl_decode_command_number() you will still be able to determine if
> > > > you are on i915, and write the correct request, but the fallback code
> > > > will be no longer duplicated. You will have to call xlookup() instead of
> > > > printxval(), in order to be able to know when the decoding fail though.
> > > 
> > > Unfortunately I cannot check for i915 in _decode_command_number since I don't
> > > have the full tcb context (cannot figure out the path, etc.). That's probably
> > > why I had to duplicate the decoding in the first place. The only other solution
> > > I see is to detect i915 early and store it globally somewhere but that is rather
> > > nasty. Not sure how to do this in a better way. What am I missing?
> > 
> > Oh yes, I see. We can add tcb context to ioctl_decode_command_number(),
> > it should not be a problem. We didn't have the need for it for the
> > moment, that's all.
> > 
> > Dmitry? What do you think of this?
> 
> I had no chance to have a look at the code, but anyway, passing
> tcb context down to ioctl_decode_command_number() shouldn't be a problem
> at all.

Sounds good. Then I'll use ioctl_decode_command_number as the dispatcher.

Thanks
Patrik

> 
> 
> -- 
> ldv
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx