[3/4] drm: Add decoding of i915 ioctls

Submitted by Patrik Jakobsson on June 9, 2015, 11:26 a.m.

Details

Message ID 1433849204-4125-4-git-send-email-patrik.jakobsson@linux.intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Patrik Jakobsson June 9, 2015, 11:26 a.m.
There are more ioctls to add but the ones in this patch are most
commonly used.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 Makefile.am                |   1 +
 defs.h                     |   2 +
 drm.c                      |   6 +
 drm_i915.c                 | 287 +++++++++++++++++++++++++++++++++++++++++++++
 ioctl.c                    |   6 +
 xlat/drm_i915_getparams.in |  28 +++++
 xlat/drm_i915_ioctls.in    |  51 ++++++++
 xlat/drm_i915_setparams.in |   4 +
 8 files changed, 385 insertions(+)
 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

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 50d5140..941e12a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -122,6 +122,7 @@  strace_SOURCES =	\
 	utimes.c	\
 	v4l2.c		\
 	drm.c		\
+	drm_i915.c	\
 	vsprintf.c	\
 	wait.c		\
 	xattr.c
diff --git a/defs.h b/defs.h
index f77330b..e9286a1 100644
--- a/defs.h
+++ b/defs.h
@@ -575,6 +575,8 @@  extern int v4l2_ioctl(struct tcb *, const unsigned int, long);
 extern int drm_is_priv(const unsigned int);
 extern int drm_is_driver(struct tcb *tcp, const char *name);
 extern int drm_ioctl(struct tcb *, const unsigned int, long);
+extern int drm_i915_ioctl(struct tcb *, const unsigned int, long);
+extern int drm_i915_decode_number(struct tcb *, unsigned int);
 
 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
index 56ef98b..fa98fb7 100644
--- a/drm.c
+++ b/drm.c
@@ -84,5 +84,11 @@  int drm_is_driver(struct tcb *tcp, const char *name)
 
 int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
 {
+	/* Check for device specific ioctls */
+	if (drm_is_priv(tcp->u_arg[1])) {
+		if (verbose(tcp) && drm_is_driver(tcp, "i915"))
+			return drm_i915_ioctl(tcp, code, arg);
+	}
+
 	return 0;
 }
diff --git a/drm_i915.c b/drm_i915.c
new file mode 100644
index 0000000..b7b32f7
--- /dev/null
+++ b/drm_i915.c
@@ -0,0 +1,287 @@ 
+/*
+ * 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.h>
+#include <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_decode_number(struct tcb *tcp, unsigned int arg)
+{
+	const char *str = xlookup(drm_i915_ioctls, arg);
+
+	if (str) {
+		tprintf("%s", str);
+		return 1;
+	}
+
+	return 0;
+}
+
+int drm_i915_ioctl(struct tcb *tcp, const unsigned int code, long arg)
+{
+	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/ioctl.c b/ioctl.c
index 690e7aa..f12ded8 100644
--- a/ioctl.c
+++ b/ioctl.c
@@ -184,6 +184,12 @@  hiddev_decode_number(unsigned int arg)
 static int
 drm_decode_number(struct tcb *tcp, unsigned int arg)
 {
+	/* Check for device specific ioctls */
+	if (drm_is_priv(tcp->u_arg[1])) {
+		if (verbose(tcp) && drm_is_driver(tcp, "i915"))
+			return drm_i915_decode_number(tcp, 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_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
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

Comments

On Tue, Jun 09, 2015 at 01:26:43PM +0200, Patrik Jakobsson wrote:
[...]
> +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=");

We use tprints to print regular strings.

> +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;

In this and other ioctl printers that unconditionally return 0 on exit,
wouldn't the caller treat it as an ioctl that hasn't been printed?
On Wed, Jun 10, 2015 at 01:35:35AM +0300, Dmitry V. Levin wrote:
> On Tue, Jun 09, 2015 at 01:26:43PM +0200, Patrik Jakobsson wrote:
> [...]
> > +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=");
> 
> We use tprints to print regular strings.

Fixed in v2

> 
> > +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;
> 
> In this and other ioctl printers that unconditionally return 0 on exit,
> wouldn't the caller treat it as an ioctl that hasn't been printed?

Yes, seems like the exiting phase should return 1 if already handled in the
entering phase. But changing it produces the same output for some reason. Not
sure what's going on here.

> 
> 
> -- 
> ldv
On Wed, Jun 10, 2015 at 02:45:24PM +0200, Patrik Jakobsson wrote:
> On Wed, Jun 10, 2015 at 01:35:35AM +0300, Dmitry V. Levin wrote:
> > On Tue, Jun 09, 2015 at 01:26:43PM +0200, Patrik Jakobsson wrote:
[...]
> > > +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;
> > 
> > In this and other ioctl printers that unconditionally return 0 on exit,
> > wouldn't the caller treat it as an ioctl that hasn't been printed?
> 
> Yes, seems like the exiting phase should return 1 if already handled in the
> entering phase. But changing it produces the same output for some reason. Not
> sure what's going on here.

Isn't tcp->u_arg[2] printed twice, the first time decoded,
and the second time in hex?
On Thu, Jun 11, 2015 at 02:27:12AM +0300, Dmitry V. Levin wrote:
> On Wed, Jun 10, 2015 at 02:45:24PM +0200, Patrik Jakobsson wrote:
> > On Wed, Jun 10, 2015 at 01:35:35AM +0300, Dmitry V. Levin wrote:
> > > On Tue, Jun 09, 2015 at 01:26:43PM +0200, Patrik Jakobsson wrote:
> [...]
> > > > +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;
> > > 
> > > In this and other ioctl printers that unconditionally return 0 on exit,
> > > wouldn't the caller treat it as an ioctl that hasn't been printed?
> > 
> > Yes, seems like the exiting phase should return 1 if already handled in the
> > entering phase. But changing it produces the same output for some reason. Not
> > sure what's going on here.
> 
> Isn't tcp->u_arg[2] printed twice, the first time decoded,
> and the second time in hex?

My mistake, it does print u_arg[2] in hex as well. Luckily they could all be
moved to the exiting phase instead. Fixed in v2.

> 
> 
> -- 
> ldv



> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jun 11, 2015 at 03:34:14PM +0200, Patrik Jakobsson wrote:
> On Thu, Jun 11, 2015 at 02:27:12AM +0300, Dmitry V. Levin wrote:
> > On Wed, Jun 10, 2015 at 02:45:24PM +0200, Patrik Jakobsson wrote:
> > > On Wed, Jun 10, 2015 at 01:35:35AM +0300, Dmitry V. Levin wrote:
> > > > On Tue, Jun 09, 2015 at 01:26:43PM +0200, Patrik Jakobsson wrote:
> > [...]
> > > > > +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;
> > > > 
> > > > In this and other ioctl printers that unconditionally return 0 on exit,
> > > > wouldn't the caller treat it as an ioctl that hasn't been printed?
> > > 
> > > Yes, seems like the exiting phase should return 1 if already handled in the
> > > entering phase. But changing it produces the same output for some reason. Not
> > > sure what's going on here.
> > 
> > Isn't tcp->u_arg[2] printed twice, the first time decoded,
> > and the second time in hex?
> 
> My mistake, it does print u_arg[2] in hex as well. Luckily they could all be
> moved to the exiting phase instead. Fixed in v2.

The common rule is to decode as early as possible, consequently, all
syscall arguments that could be decoded on entering syscall should be
decoded on entering rather than on exiting.  All syscall arguments
of _IOW ioctl commands could be fully decoded on entering syscall.
On Sat, Jun 13, 2015 at 1:48 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Thu, Jun 11, 2015 at 03:34:14PM +0200, Patrik Jakobsson wrote:
>> On Thu, Jun 11, 2015 at 02:27:12AM +0300, Dmitry V. Levin wrote:
>> > On Wed, Jun 10, 2015 at 02:45:24PM +0200, Patrik Jakobsson wrote:
>> > > On Wed, Jun 10, 2015 at 01:35:35AM +0300, Dmitry V. Levin wrote:
>> > > > On Tue, Jun 09, 2015 at 01:26:43PM +0200, Patrik Jakobsson wrote:
>> > [...]
>> > > > > +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;
>> > > >
>> > > > In this and other ioctl printers that unconditionally return 0 on exit,
>> > > > wouldn't the caller treat it as an ioctl that hasn't been printed?
>> > >
>> > > Yes, seems like the exiting phase should return 1 if already handled in the
>> > > entering phase. But changing it produces the same output for some reason. Not
>> > > sure what's going on here.
>> >
>> > Isn't tcp->u_arg[2] printed twice, the first time decoded,
>> > and the second time in hex?
>>
>> My mistake, it does print u_arg[2] in hex as well. Luckily they could all be
>> moved to the exiting phase instead. Fixed in v2.
>
> The common rule is to decode as early as possible, consequently, all
> syscall arguments that could be decoded on entering syscall should be
> decoded on entering rather than on exiting.  All syscall arguments
> of _IOW ioctl commands could be fully decoded on entering syscall.
>

Yes, that makes sense. I'll rework it for v2 of the patch set.

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