[PATCH 0/4] drm: Add decoding for DRM/KMS and i915 ioctls

[PATCH 0/4] drm: Add decoding for DRM/KMS and i915 ioctls

[Patrik Jakobsson]

This set of patches adds a dispatcher for handling DRM ioctls. The
kernel headers for DRM might not be available on all distributions
so we depend on libdrm for those. If libdrm is not available we fall
back on the kernel headers. Since DRM drivers share the same range of
private ioctl numbers I've added a function for detecting the driver
based on it's name.

Patrik Jakobsson (4):
  drm: Add config for detecting libdrm
  drm: Add dispatcher and driver identification for DRM
  drm: Add decoding of i915 ioctls
  drm: Add decoding of DRM and KMS ioctls

 Makefile.am                |   2 +
 configure.ac               |   4 +
 defs.h                     |   8 +-
 drm.c                      | 613 +++++++++++++++++++++++++++++++++++++++++++++
 drm_i915.c                 | 287 +++++++++++++++++++++
 io.c                       |   2 +-
 ioctl.c                    |  19 +-
 xlat/drm_i915_getparams.in |  28 +++
 xlat/drm_i915_ioctls.in    |  51 ++++
 xlat/drm_i915_setparams.in |   4 +
 10 files changed, 1015 insertions(+), 3 deletions(-)
 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

[PATCH 1/4] drm: Add config for detecting libdrm

[Patrik Jakobsson]

Use pkg-config to try to find libdrm. If that fails use the standard
include directory for kernel drm headers in /usr/include/drm.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 configure.ac | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/configure.ac b/configure.ac
index d829e18..ea3b6e6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -837,6 +837,10 @@ fi
 AM_CONDITIONAL([USE_LIBUNWIND], [test "x$use_libunwind" = xyes])
 AC_MSG_RESULT([$use_libunwind])
 
+PKG_CHECK_MODULES([libdrm], [libdrm],
+	[CPPFLAGS="$CPPFLAGS $libdrm_CFLAGS"],
+	[CPPFLAGS="$CPPFLAGS -I/usr/include/drm"])
+
 if test "$arch" = mips && test "$no_create" != yes; then
 	mkdir -p linux/mips
 	if $srcdir/linux/mips/genstub.sh linux/mips; then
-- 
2.1.0

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

[PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Patrik Jakobsson]

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 Makefile.am |  1 +
 defs.h      |  6 ++++-
 drm.c       | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 io.c        |  2 +-
 ioctl.c     | 13 ++++++++-
 5 files changed, 107 insertions(+), 3 deletions(-)
 create mode 100644 drm.c

diff --git a/Makefile.am b/Makefile.am
index 549aebc..50d5140 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -121,6 +121,7 @@ strace_SOURCES =	\
 	utime.c		\
 	utimes.c	\
 	v4l2.c		\
+	drm.c		\
 	vsprintf.c	\
 	wait.c		\
 	xattr.c
diff --git a/defs.h b/defs.h
index 77c819c..f77330b 100644
--- a/defs.h
+++ b/defs.h
@@ -559,7 +559,7 @@ extern const struct_ioctlent *ioctl_lookup(const unsigned int);
 extern const struct_ioctlent *ioctl_next_match(const struct_ioctlent *);
 extern void ioctl_print_code(const unsigned int);
 extern int ioctl_decode(struct tcb *, const unsigned int, long);
-extern int ioctl_decode_command_number(const unsigned int);
+extern int ioctl_decode_command_number(struct tcb *, const unsigned int);
 extern int block_ioctl(struct tcb *, const unsigned int, long);
 extern int evdev_ioctl(struct tcb *, const unsigned int, long);
 extern int loop_ioctl(struct tcb *, const unsigned int, long);
@@ -572,6 +572,10 @@ 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_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 tv_nz(const struct timeval *);
 extern int tv_cmp(const struct timeval *, const struct timeval *);
 extern double tv_float(const struct timeval *);
diff --git a/drm.c b/drm.c
new file mode 100644
index 0000000..56ef98b
--- /dev/null
+++ b/drm.c
@@ -0,0 +1,88 @@
+/*
+ * 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.h>
+
+#define DRM_MAX_NAME_LEN 128
+
+inline int drm_is_priv(const unsigned int num)
+{
+	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
+		_IOC_NR(num) < DRM_COMMAND_END);
+}
+
+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;
+}
+
+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)
+{
+	return 0;
+}
diff --git a/io.c b/io.c
index 30ed578..6810a45 100644
--- a/io.c
+++ b/io.c
@@ -391,7 +391,7 @@ SYS_FUNC(ioctl)
 	if (entering(tcp)) {
 		printfd(tcp, tcp->u_arg[0]);
 		tprints(", ");
-		if (!ioctl_decode_command_number(tcp->u_arg[1])) {
+		if (!ioctl_decode_command_number(tcp, tcp->u_arg[1])) {
 			iop = ioctl_lookup(tcp->u_arg[1]);
 			if (iop) {
 				tprints(iop->symbol);
diff --git a/ioctl.c b/ioctl.c
index c67d048..690e7aa 100644
--- a/ioctl.c
+++ b/ioctl.c
@@ -181,8 +181,14 @@ hiddev_decode_number(unsigned int arg)
 	return 0;
 }
 
+static int
+drm_decode_number(struct tcb *tcp, unsigned int arg)
+{
+	return 0;
+}
+
 int
-ioctl_decode_command_number(unsigned int arg)
+ioctl_decode_command_number(struct tcb *tcp, unsigned int arg)
 {
 	switch (_IOC_TYPE(arg)) {
 		case 'E':
@@ -216,6 +222,8 @@ ioctl_decode_command_number(unsigned int arg)
 				return 1;
 			}
 			return 0;
+		case 'd':
+			return drm_decode_number(tcp, arg);
 		default:
 			return 0;
 	}
@@ -252,6 +260,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 +294,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-
-- 
2.1.0

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

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Gabriel Laskar]

On Tue,  9 Jun 2015 13:26:42 +0200
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:

> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> ---
>  Makefile.am |  1 +
>  defs.h      |  6 ++++-
>  drm.c       | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  io.c        |  2 +-
>  ioctl.c     | 13 ++++++++-
>  5 files changed, 107 insertions(+), 3 deletions(-)
>  create mode 100644 drm.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 549aebc..50d5140 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -121,6 +121,7 @@ strace_SOURCES =	\
>  	utime.c		\
>  	utimes.c	\
>  	v4l2.c		\
> +	drm.c		\
>  	vsprintf.c	\
>  	wait.c		\
>  	xattr.c

If I remember correctly source files were sorted in alphabetic order.
Same remark for drm_i915.c in the next patch.

(This is nitpicking)

> diff --git a/defs.h b/defs.h
> index 77c819c..f77330b 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -559,7 +559,7 @@ extern const struct_ioctlent *ioctl_lookup(const unsigned int);
>  extern const struct_ioctlent *ioctl_next_match(const struct_ioctlent *);
>  extern void ioctl_print_code(const unsigned int);
>  extern int ioctl_decode(struct tcb *, const unsigned int, long);
> -extern int ioctl_decode_command_number(const unsigned int);
> +extern int ioctl_decode_command_number(struct tcb *, const unsigned int);
>  extern int block_ioctl(struct tcb *, const unsigned int, long);
>  extern int evdev_ioctl(struct tcb *, const unsigned int, long);
>  extern int loop_ioctl(struct tcb *, const unsigned int, long);
> @@ -572,6 +572,10 @@ 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_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 tv_nz(const struct timeval *);
>  extern int tv_cmp(const struct timeval *, const struct timeval *);
>  extern double tv_float(const struct timeval *);
> diff --git a/drm.c b/drm.c
> new file mode 100644
> index 0000000..56ef98b
> --- /dev/null
> +++ b/drm.c
> @@ -0,0 +1,88 @@
> +/*
> + * 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.h>

If I remember correctly, headers are mostly sorted in strace files also

> +
> +#define DRM_MAX_NAME_LEN 128
> +
> +inline int drm_is_priv(const unsigned int num)
> +{
> +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> +		_IOC_NR(num) < DRM_COMMAND_END);
> +}
> +
> +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;
> +}
> +
> +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)
> +{
> +	return 0;
> +}
> diff --git a/io.c b/io.c
> index 30ed578..6810a45 100644
> --- a/io.c
> +++ b/io.c
> @@ -391,7 +391,7 @@ SYS_FUNC(ioctl)
>  	if (entering(tcp)) {
>  		printfd(tcp, tcp->u_arg[0]);
>  		tprints(", ");
> -		if (!ioctl_decode_command_number(tcp->u_arg[1])) {
> +		if (!ioctl_decode_command_number(tcp, tcp->u_arg[1])) {
>  			iop = ioctl_lookup(tcp->u_arg[1]);
>  			if (iop) {
>  				tprints(iop->symbol);
> diff --git a/ioctl.c b/ioctl.c
> index c67d048..690e7aa 100644
> --- a/ioctl.c
> +++ b/ioctl.c
> @@ -181,8 +181,14 @@ hiddev_decode_number(unsigned int arg)
>  	return 0;
>  }
>  
> +static int
> +drm_decode_number(struct tcb *tcp, unsigned int arg)
> +{
> +	return 0;
> +}
> +

If you push drm_decode_number() inside drm.c you will have less
function to export, and avoir polluting to much defs.h.

For the other *_decode_number(), there was no files with the specific
argument decoding attached to it, so it maked sense at that time to not
have separated files for each of them (this makes me think that for
evdev ioctls, we could move that part inside evdev.c.)

>  int
> -ioctl_decode_command_number(unsigned int arg)
> +ioctl_decode_command_number(struct tcb *tcp, unsigned int arg)
>  {
>  	switch (_IOC_TYPE(arg)) {
>  		case 'E':
> @@ -216,6 +222,8 @@ ioctl_decode_command_number(unsigned int arg)
>  				return 1;
>  			}
>  			return 0;
> +		case 'd':
> +			return drm_decode_number(tcp, arg);
>  		default:
>  			return 0;
>  	}
> @@ -252,6 +260,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 +294,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-



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

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Patrik Jakobsson]

On Tue, Jun 09, 2015 at 03:51:10PM +0200, Gabriel Laskar wrote:
> On Tue,  9 Jun 2015 13:26:42 +0200
> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> 
> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > ---
> >  Makefile.am |  1 +
> >  defs.h      |  6 ++++-
> >  drm.c       | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  io.c        |  2 +-
> >  ioctl.c     | 13 ++++++++-
> >  5 files changed, 107 insertions(+), 3 deletions(-)
> >  create mode 100644 drm.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 549aebc..50d5140 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -121,6 +121,7 @@ strace_SOURCES =	\
> >  	utime.c		\
> >  	utimes.c	\
> >  	v4l2.c		\
> > +	drm.c		\
> >  	vsprintf.c	\
> >  	wait.c		\
> >  	xattr.c
> 
> If I remember correctly source files were sorted in alphabetic order.
> Same remark for drm_i915.c in the next patch.
> 
> (This is nitpicking)

Ah yes, totally unacceptable ;). Will fix.

> 
> > diff --git a/defs.h b/defs.h
> > index 77c819c..f77330b 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -559,7 +559,7 @@ extern const struct_ioctlent *ioctl_lookup(const unsigned int);
> >  extern const struct_ioctlent *ioctl_next_match(const struct_ioctlent *);
> >  extern void ioctl_print_code(const unsigned int);
> >  extern int ioctl_decode(struct tcb *, const unsigned int, long);
> > -extern int ioctl_decode_command_number(const unsigned int);
> > +extern int ioctl_decode_command_number(struct tcb *, const unsigned int);
> >  extern int block_ioctl(struct tcb *, const unsigned int, long);
> >  extern int evdev_ioctl(struct tcb *, const unsigned int, long);
> >  extern int loop_ioctl(struct tcb *, const unsigned int, long);
> > @@ -572,6 +572,10 @@ 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_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 tv_nz(const struct timeval *);
> >  extern int tv_cmp(const struct timeval *, const struct timeval *);
> >  extern double tv_float(const struct timeval *);
> > diff --git a/drm.c b/drm.c
> > new file mode 100644
> > index 0000000..56ef98b
> > --- /dev/null
> > +++ b/drm.c
> > @@ -0,0 +1,88 @@
> > +/*
> > + * 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.h>
> 
> If I remember correctly, headers are mostly sorted in strace files also

I'll take a look to see if I can find what pattern to stick with here.

> 
> > +
> > +#define DRM_MAX_NAME_LEN 128
> > +
> > +inline int drm_is_priv(const unsigned int num)
> > +{
> > +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> > +		_IOC_NR(num) < DRM_COMMAND_END);
> > +}
> > +
> > +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;
> > +}
> > +
> > +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)
> > +{
> > +	return 0;
> > +}
> > diff --git a/io.c b/io.c
> > index 30ed578..6810a45 100644
> > --- a/io.c
> > +++ b/io.c
> > @@ -391,7 +391,7 @@ SYS_FUNC(ioctl)
> >  	if (entering(tcp)) {
> >  		printfd(tcp, tcp->u_arg[0]);
> >  		tprints(", ");
> > -		if (!ioctl_decode_command_number(tcp->u_arg[1])) {
> > +		if (!ioctl_decode_command_number(tcp, tcp->u_arg[1])) {
> >  			iop = ioctl_lookup(tcp->u_arg[1]);
> >  			if (iop) {
> >  				tprints(iop->symbol);
> > diff --git a/ioctl.c b/ioctl.c
> > index c67d048..690e7aa 100644
> > --- a/ioctl.c
> > +++ b/ioctl.c
> > @@ -181,8 +181,14 @@ hiddev_decode_number(unsigned int arg)
> >  	return 0;
> >  }
> >  
> > +static int
> > +drm_decode_number(struct tcb *tcp, unsigned int arg)
> > +{
> > +	return 0;
> > +}
> > +
> 
> If you push drm_decode_number() inside drm.c you will have less
> function to export, and avoir polluting to much defs.h.
> 
> For the other *_decode_number(), there was no files with the specific
> argument decoding attached to it, so it maked sense at that time to not
> have separated files for each of them (this makes me think that for
> evdev ioctls, we could move that part inside evdev.c.)
> 

I agree, will do.

> >  int
> > -ioctl_decode_command_number(unsigned int arg)
> > +ioctl_decode_command_number(struct tcb *tcp, unsigned int arg)
> >  {
> >  	switch (_IOC_TYPE(arg)) {
> >  		case 'E':
> > @@ -216,6 +222,8 @@ ioctl_decode_command_number(unsigned int arg)
> >  				return 1;
> >  			}
> >  			return 0;
> > +		case 'd':
> > +			return drm_decode_number(tcp, arg);
> >  		default:
> >  			return 0;
> >  	}
> > @@ -252,6 +260,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 +294,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-
> 
> 
> 
> -- 
> Gabriel Laskar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 2550 bytes --]

On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
[...]
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -121,6 +121,7 @@ strace_SOURCES =	\
>  	utime.c		\
>  	utimes.c	\
>  	v4l2.c		\
> +	drm.c		\
>  	vsprintf.c	\
>  	wait.c		\
>  	xattr.c

Starting with v4.7-166-g7ae73a9, we keep strace_SOURCES list sorted.

> --- /dev/null
> +++ b/drm.c
[...]
> +#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.h>

No need to include header files already included by "defs.h".
Most likely no need to include <linux/limits.h> or <linux/types.h>.

> +#define DRM_MAX_NAME_LEN 128
> +
> +inline int drm_is_priv(const unsigned int num)
> +{
> +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> +		_IOC_NR(num) < DRM_COMMAND_END);
> +}
> +
> +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;
> +}
> +
> +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;
> +}

This interface will result to several getfdpath() calls per
ioctl_decode().  If the only purpose of drm_is_driver() is to help finding
the most appropriate function, let's create a table of pairs
{driver name, function} and pass this table to a function that will do a
single getfdpath() call, calculate the driver name, and choose the right
function from the table.

[...]
> @@ -284,6 +294,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-

This is a history, no need to patch it.


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 195 bytes --]

_______________________________________________
Strace-devel mailing list
Strace-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/strace-devel

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Patrik Jakobsson]

On Wed, Jun 10, 2015 at 01:14:20AM +0300, Dmitry V. Levin wrote:
> On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
> [...]
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -121,6 +121,7 @@ strace_SOURCES =	\
> >  	utime.c		\
> >  	utimes.c	\
> >  	v4l2.c		\
> > +	drm.c		\
> >  	vsprintf.c	\
> >  	wait.c		\
> >  	xattr.c
> 
> Starting with v4.7-166-g7ae73a9, we keep strace_SOURCES list sorted.

Fixed in v2

> 
> > --- /dev/null
> > +++ b/drm.c
> [...]
> > +#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.h>
> 
> No need to include header files already included by "defs.h".
> Most likely no need to include <linux/limits.h> or <linux/types.h>.

Fixed in v2

> 
> > +#define DRM_MAX_NAME_LEN 128
> > +
> > +inline int drm_is_priv(const unsigned int num)
> > +{
> > +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> > +		_IOC_NR(num) < DRM_COMMAND_END);
> > +}
> > +
> > +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;
> > +}
> > +
> > +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;
> > +}
> 
> This interface will result to several getfdpath() calls per
> ioctl_decode().  If the only purpose of drm_is_driver() is to help finding
> the most appropriate function, let's create a table of pairs
> {driver name, function} and pass this table to a function that will do a
> single getfdpath() call, calculate the driver name, and choose the right
> function from the table.

Yes I was thinking the same thing but it's a bit tricky. What I need is:
fd -> path -> driver name. And fd -> path could change between ioctls. It is not
a likely scenario but it's possible. I could get rid of the extra call in
drm_decode_number() if I put it back in drm_ioctl as in my RFC. I could also
optimize path -> driver name with a table but I don't know how expensive those
calls actually are. Not sure what would be the best solution here.

> 
> [...]
> > @@ -284,6 +294,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-
> 
> This is a history, no need to patch it.

Fixed in v2

> 
> 
> -- 
> ldv


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

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 2876 bytes --]

On Wed, Jun 10, 2015 at 01:52:33PM +0200, Patrik Jakobsson wrote:
> On Wed, Jun 10, 2015 at 01:14:20AM +0300, Dmitry V. Levin wrote:
> > On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
[...]
> > > +#define DRM_MAX_NAME_LEN 128
> > > +
> > > +inline int drm_is_priv(const unsigned int num)
> > > +{
> > > +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> > > +		_IOC_NR(num) < DRM_COMMAND_END);
> > > +}
> > > +
> > > +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;
> > > +}
> > > +
> > > +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;
> > > +}
> > 
> > This interface will result to several getfdpath() calls per
> > ioctl_decode().  If the only purpose of drm_is_driver() is to help finding
> > the most appropriate function, let's create a table of pairs
> > {driver name, function} and pass this table to a function that will do a
> > single getfdpath() call, calculate the driver name, and choose the right
> > function from the table.
> 
> Yes I was thinking the same thing but it's a bit tricky. What I need is:
> fd -> path -> driver name. And fd -> path could change between ioctls. It is not
> a likely scenario but it's possible. I could get rid of the extra call in
> drm_decode_number() if I put it back in drm_ioctl as in my RFC. I could also
> optimize path -> driver name with a table but I don't know how expensive those
> calls actually are. Not sure what would be the best solution here.

drm_get_driver_name() is quite expensive as it does two readlink syscalls,
so it should be called at most once per ioctl_decode().

Another method to achieve this is to change drm_get_driver_name() to return
basename(path) instead of return code, so that drm_ioctl() would call it
once and pass the result to strcmp calls:

int
drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
{
	if (verbose(tcp) && drm_is_priv(code)) {
		const char *driver = drm_get_driver_name(tcp);
		if (!driver)
			return 0;
		if (!strcmp(driver, "i915"))
			return drm_i915_ioctl(tcp, code, arg);
	}
	return 0;
}


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Patrik Jakobsson]

On Thu, Jun 11, 2015 at 02:26:59AM +0300, Dmitry V. Levin wrote:
> On Wed, Jun 10, 2015 at 01:52:33PM +0200, Patrik Jakobsson wrote:
> > On Wed, Jun 10, 2015 at 01:14:20AM +0300, Dmitry V. Levin wrote:
> > > On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
> [...]
> > > > +#define DRM_MAX_NAME_LEN 128
> > > > +
> > > > +inline int drm_is_priv(const unsigned int num)
> > > > +{
> > > > +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> > > > +		_IOC_NR(num) < DRM_COMMAND_END);
> > > > +}
> > > > +
> > > > +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;
> > > > +}
> > > > +
> > > > +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;
> > > > +}
> > > 
> > > This interface will result to several getfdpath() calls per
> > > ioctl_decode().  If the only purpose of drm_is_driver() is to help finding
> > > the most appropriate function, let's create a table of pairs
> > > {driver name, function} and pass this table to a function that will do a
> > > single getfdpath() call, calculate the driver name, and choose the right
> > > function from the table.
> > 
> > Yes I was thinking the same thing but it's a bit tricky. What I need is:
> > fd -> path -> driver name. And fd -> path could change between ioctls. It is not
> > a likely scenario but it's possible. I could get rid of the extra call in
> > drm_decode_number() if I put it back in drm_ioctl as in my RFC. I could also
> > optimize path -> driver name with a table but I don't know how expensive those
> > calls actually are. Not sure what would be the best solution here.
> 
> drm_get_driver_name() is quite expensive as it does two readlink syscalls,
> so it should be called at most once per ioctl_decode().
> 
> Another method to achieve this is to change drm_get_driver_name() to return
> basename(path) instead of return code, so that drm_ioctl() would call it
> once and pass the result to strcmp calls:
> 
> int
> drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> {
> 	if (verbose(tcp) && drm_is_priv(code)) {
> 		const char *driver = drm_get_driver_name(tcp);
> 		if (!driver)
> 			return 0;
> 		if (!strcmp(driver, "i915"))
> 			return drm_i915_ioctl(tcp, code, arg);
> 	}
> 	return 0;
> }

I misunderstood you. As you say, we shouldn't do a drm_get_driver_name() more
than once (no matter how many drm drivers we are looking for) so your suggestion
above would fix that. I was thinking about how to get rid of the extra call in
drm_decode_number() (if we could somehow squash them together). But that would
make things rather ugly. If ok with you we could just have the same approach in
drm_decode_number() as above and live with the fact that we get two calls to
drm_get_driver_name() per DRM device specific ioctl. One for drm_decode_number()
and one for drm_ioctl().

> 
> 
> -- 
> ldv



> ------------------------------------------------------------------------------

> _______________________________________________
> Strace-devel mailing list
> Strace-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel

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

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 4140 bytes --]

On Thu, Jun 11, 2015 at 04:11:49PM +0200, Patrik Jakobsson wrote:
> On Thu, Jun 11, 2015 at 02:26:59AM +0300, Dmitry V. Levin wrote:
> > On Wed, Jun 10, 2015 at 01:52:33PM +0200, Patrik Jakobsson wrote:
> > > On Wed, Jun 10, 2015 at 01:14:20AM +0300, Dmitry V. Levin wrote:
> > > > On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
> > [...]
> > > > > +#define DRM_MAX_NAME_LEN 128
> > > > > +
> > > > > +inline int drm_is_priv(const unsigned int num)
> > > > > +{
> > > > > +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> > > > > +		_IOC_NR(num) < DRM_COMMAND_END);
> > > > > +}
> > > > > +
> > > > > +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;
> > > > > +}
> > > > > +
> > > > > +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;
> > > > > +}
> > > > 
> > > > This interface will result to several getfdpath() calls per
> > > > ioctl_decode().  If the only purpose of drm_is_driver() is to help finding
> > > > the most appropriate function, let's create a table of pairs
> > > > {driver name, function} and pass this table to a function that will do a
> > > > single getfdpath() call, calculate the driver name, and choose the right
> > > > function from the table.
> > > 
> > > Yes I was thinking the same thing but it's a bit tricky. What I need is:
> > > fd -> path -> driver name. And fd -> path could change between ioctls. It is not
> > > a likely scenario but it's possible. I could get rid of the extra call in
> > > drm_decode_number() if I put it back in drm_ioctl as in my RFC. I could also
> > > optimize path -> driver name with a table but I don't know how expensive those
> > > calls actually are. Not sure what would be the best solution here.
> > 
> > drm_get_driver_name() is quite expensive as it does two readlink syscalls,
> > so it should be called at most once per ioctl_decode().
> > 
> > Another method to achieve this is to change drm_get_driver_name() to return
> > basename(path) instead of return code, so that drm_ioctl() would call it
> > once and pass the result to strcmp calls:
> > 
> > int
> > drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> > {
> > 	if (verbose(tcp) && drm_is_priv(code)) {
> > 		const char *driver = drm_get_driver_name(tcp);
> > 		if (!driver)
> > 			return 0;
> > 		if (!strcmp(driver, "i915"))
> > 			return drm_i915_ioctl(tcp, code, arg);
> > 	}
> > 	return 0;
> > }
> 
> I misunderstood you. As you say, we shouldn't do a drm_get_driver_name() more
> than once (no matter how many drm drivers we are looking for) so your suggestion
> above would fix that. I was thinking about how to get rid of the extra call in
> drm_decode_number() (if we could somehow squash them together). But that would
> make things rather ugly. If ok with you we could just have the same approach in
> drm_decode_number() as above and live with the fact that we get two calls to
> drm_get_driver_name() per DRM device specific ioctl. One for drm_decode_number()
> and one for drm_ioctl().

This way we would end up with three drm_get_driver_name() calls per ioctl:
twice on entering syscall and once on exiting.  Maybe we could cache
result of the first of these three calls somewhere?


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Patrik Jakobsson]

On Sat, Jun 13, 2015 at 1:41 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Thu, Jun 11, 2015 at 04:11:49PM +0200, Patrik Jakobsson wrote:
>> On Thu, Jun 11, 2015 at 02:26:59AM +0300, Dmitry V. Levin wrote:
>> > On Wed, Jun 10, 2015 at 01:52:33PM +0200, Patrik Jakobsson wrote:
>> > > On Wed, Jun 10, 2015 at 01:14:20AM +0300, Dmitry V. Levin wrote:
>> > > > On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
>> > [...]
>> > > > > +#define DRM_MAX_NAME_LEN 128
>> > > > > +
>> > > > > +inline int drm_is_priv(const unsigned int num)
>> > > > > +{
>> > > > > +     return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
>> > > > > +             _IOC_NR(num) < DRM_COMMAND_END);
>> > > > > +}
>> > > > > +
>> > > > > +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;
>> > > > > +}
>> > > > > +
>> > > > > +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;
>> > > > > +}
>> > > >
>> > > > This interface will result to several getfdpath() calls per
>> > > > ioctl_decode().  If the only purpose of drm_is_driver() is to help finding
>> > > > the most appropriate function, let's create a table of pairs
>> > > > {driver name, function} and pass this table to a function that will do a
>> > > > single getfdpath() call, calculate the driver name, and choose the right
>> > > > function from the table.
>> > >
>> > > Yes I was thinking the same thing but it's a bit tricky. What I need is:
>> > > fd -> path -> driver name. And fd -> path could change between ioctls. It is not
>> > > a likely scenario but it's possible. I could get rid of the extra call in
>> > > drm_decode_number() if I put it back in drm_ioctl as in my RFC. I could also
>> > > optimize path -> driver name with a table but I don't know how expensive those
>> > > calls actually are. Not sure what would be the best solution here.
>> >
>> > drm_get_driver_name() is quite expensive as it does two readlink syscalls,
>> > so it should be called at most once per ioctl_decode().
>> >
>> > Another method to achieve this is to change drm_get_driver_name() to return
>> > basename(path) instead of return code, so that drm_ioctl() would call it
>> > once and pass the result to strcmp calls:
>> >
>> > int
>> > drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
>> > {
>> >     if (verbose(tcp) && drm_is_priv(code)) {
>> >             const char *driver = drm_get_driver_name(tcp);
>> >             if (!driver)
>> >                     return 0;
>> >             if (!strcmp(driver, "i915"))
>> >                     return drm_i915_ioctl(tcp, code, arg);
>> >     }
>> >     return 0;
>> > }
>>
>> I misunderstood you. As you say, we shouldn't do a drm_get_driver_name() more
>> than once (no matter how many drm drivers we are looking for) so your suggestion
>> above would fix that. I was thinking about how to get rid of the extra call in
>> drm_decode_number() (if we could somehow squash them together). But that would
>> make things rather ugly. If ok with you we could just have the same approach in
>> drm_decode_number() as above and live with the fact that we get two calls to
>> drm_get_driver_name() per DRM device specific ioctl. One for drm_decode_number()
>> and one for drm_ioctl().
>
> This way we would end up with three drm_get_driver_name() calls per ioctl:
> twice on entering syscall and once on exiting.  Maybe we could cache
> result of the first of these three calls somewhere?
>

How about adding a "void *private" field to struct tcb. That way any
syscall can store additional data across the life of the tcb.

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

Re: [Intel-gfx] [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 657 bytes --]

On Sun, Jun 14, 2015 at 01:12:45PM +0200, Patrik Jakobsson wrote:
[...]
> How about adding a "void *private" field to struct tcb. That way any
> syscall can store additional data across the life of the tcb.

We can add a field to struct tcb, but its semantics wrt memory management
should be strictly defined.  For example, who is responsible for memory
deallocation, what droptcb() should do about this field, etc.

In this case, we probably need to store a pointer to a string dynamically
allocated in drm_get_driver_name() (on entering syscall) which is
automatically deallocated later in trace_syscall_exiting() and droptcb().


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 79 bytes --]

------------------------------------------------------------------------------

[-- Attachment #3: Type: text/plain, Size: 195 bytes --]

_______________________________________________
Strace-devel mailing list
Strace-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/strace-devel

Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 761 bytes --]

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

The return code of getfdpath() is essentially the return code of
readlink(), so the check should be the same as after readlink() below.

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

-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

[Patrik Jakobsson]

There are more ioctls to add but the ones in this patch are most
commonly used.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 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

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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
+ */
+
+#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
-- 
2.1.0


------------------------------------------------------------------------------

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

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 780 bytes --]

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?


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

[Patrik Jakobsson]

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


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

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

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 886 bytes --]

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?


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

[Patrik Jakobsson]

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

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

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

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 1516 bytes --]

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.


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

[Patrik Jakobsson]

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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm: Add decoding of DRM and KMS ioctls

[Patrik Jakobsson]

This patch adds many of the DRM and KMS ioctls. The rest can be added as
needed.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drm.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 519 insertions(+)

diff --git a/drm.c b/drm.c
index fa98fb7..e550c34 100644
--- a/drm.c
+++ b/drm.c
@@ -82,6 +82,468 @@ int drm_is_driver(struct tcb *tcp, const char *name)
 	return strcmp(name, drv) == 0;
 }
 
+static int drm_version(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_version ver;
+	char *name, *date, *desc;
+	int ret;
+
+	if (entering(tcp) || umove(tcp, arg, &ver))
+		return 0;
+
+	name = calloc(ver.name_len + 1, 1);
+	if (!name)
+		return 0;
+	ret = umovestr(tcp, (long)ver.name, ver.name_len, name);
+	if (ret < 0)
+		goto free_name;
+
+	date = calloc(ver.date_len + 1, 1);
+	if (!date)
+		goto free_name;
+	ret = umovestr(tcp, (long)ver.date, ver.date_len, date);
+	if (ret < 0)
+		goto free_date;
+
+	desc = calloc(ver.desc_len + 1, 1);
+	if (!desc)
+		goto free_date;
+	ret = umovestr(tcp, (long)ver.desc, ver.desc_len, desc);
+	if (ret < 0)
+		goto free_desc;
+
+	tprintf(", {version_major=%d, version_minor=%d, version_patchlevel=%d, "
+		"name_len=%lu, name=%s, date_len=%lu, date=%s, "
+		"desc_len=%lu, desc=%s}",
+		ver.version_major, ver.version_minor, ver.version_patchlevel,
+		ver.name_len, name, ver.date_len, date, ver.desc_len,
+		desc);
+
+free_desc:
+	free(desc);
+free_date:
+	free(date);
+free_name:
+	free(name);
+
+	if (ret < 0)
+		return 0;
+
+	return 1;
+}
+
+static int drm_get_unique(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_unique unique;
+	char *str;
+	int ret;
+
+	if (entering(tcp) || umove(tcp, arg, &unique))
+		return 0;
+
+	str = calloc(unique.unique_len + 1, 1);
+	if (!str)
+		return 0;
+	ret = umovestr(tcp, (long)unique.unique, unique.unique_len, str);
+
+	tprintf(", {unique_len=%lu, unique=%s}", unique.unique_len, str);
+
+	free(str);
+
+	if (ret < 0)
+		return 0;
+
+	return 1;
+}
+
+static int drm_get_magic(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_auth auth;
+
+	if (entering(tcp) || umove(tcp, arg, &auth))
+		return 0;
+
+	tprintf(", {magic=%u}", auth.magic);
+
+	return 1;
+}
+
+static int drm_wait_vblank(struct tcb *tcp, const unsigned int code, long arg)
+{
+	union drm_wait_vblank vblank;
+
+	if (umove(tcp, arg, &vblank))
+		return 0;
+
+	if (entering(tcp)) {
+		tprintf(", {request={type=%u, sequence=%u, signal=%lu}",
+			vblank.request.type, vblank.request.sequence,
+			vblank.request.signal);
+	} else {
+		tprintf(", reply={type=%u, sequence=%u, tval_sec=%ld, tval_usec=%ld}}",
+			vblank.reply.type, vblank.reply.sequence,
+			vblank.reply.tval_sec, vblank.reply.tval_usec);
+	}
+
+	return 1;
+}
+
+static int drm_mode_get_resources(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_card_res res;
+
+	if (entering(tcp) || umove(tcp, arg, &res))
+		return 0;
+
+	tprintf(", {fb_id_ptr=%p, crtc_id_ptr=%p, connector_id_ptr=%p, "
+		"encoder_id_ptr=%p, count_fbs=%u, count_crtcs=%u, "
+		"count_connectors=%u, count_encoders=%u, min_width=%u, "
+		"max_width=%u, min_height=%u, max_height=%u}",
+		(void *)res.fb_id_ptr, (void *)res.crtc_id_ptr,
+		(void *)res.connector_id_ptr, (void *)res.encoder_id_ptr,
+		res.count_fbs, res.count_crtcs, res.count_connectors,
+		res.count_encoders, res.min_width, res.max_width,
+		res.min_height, res.max_height);
+
+	return 1;
+}
+
+static void drm_mode_print_modeinfo(struct drm_mode_modeinfo *info)
+{
+	tprintf("clock=%u, hdisplay=%hu, hsync_start=%hu, hsync_end=%hu, "
+		"htotal=%hu, hskew=%hu, vdisplay=%hu, vsync_start=%hu, "
+		"vsync_end=%hu, vtotal=%hu, vscan=%hu, vrefresh=%u, "
+		"flags=0x%x, type=%u, name=%s", info->clock, info->hdisplay,
+		info->hsync_start, info->hsync_end, info->htotal, info->hskew,
+		info->vdisplay, info->vsync_start, info->vsync_end,
+		info->vtotal, info->vscan, info->vrefresh, info->flags,
+		info->type, info->name);
+}
+
+static int drm_mode_get_crtc(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_crtc crtc;
+
+	if (entering(tcp) || umove(tcp, arg, &crtc))
+		return 0;
+
+	tprintf(", {set_connectors_ptr=%p, count_connectors=%u, crtc_id=%u, "
+		"fb_id=%u, x=%u, y=%u, gamma_size=%u, mode_valid=%u, mode={",
+		(void *)crtc.set_connectors_ptr, crtc.count_connectors,
+		crtc.crtc_id, crtc.fb_id, crtc.x, crtc.y, crtc.gamma_size,
+		crtc.mode_valid);
+
+	drm_mode_print_modeinfo(&crtc.mode);
+	tprintf("}}");
+
+	return 1;
+}
+
+static int drm_mode_set_crtc(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_crtc crtc;
+
+	if (entering(tcp) || umove(tcp, arg, &crtc))
+		return 0;
+
+	tprintf(", {set_connectors_ptr=%p, count_connectors=%u, crtc_id=%u, "
+		"fb_id=%u, x=%u, y=%u, gamma_size=%u, mode_valid=%u, mode={",
+		(void *)crtc.set_connectors_ptr, crtc.count_connectors,
+		crtc.crtc_id, crtc.fb_id, crtc.x, crtc.y, crtc.gamma_size,
+		crtc.mode_valid);
+
+	drm_mode_print_modeinfo(&crtc.mode);
+	tprintf("}}");
+
+	return 1;
+}
+
+static int drm_mode_cursor(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_cursor cursor;
+
+	if (entering(tcp) || umove(tcp, arg, &cursor))
+		return 0;
+
+	tprintf(", {flags=0x%x, crtc_id=%u, x=%d, y=%d, width=%u, height=%u, "
+		"handle=%u}", cursor.flags, cursor.crtc_id, cursor.x, cursor.y,
+		cursor.width, cursor.height, cursor.handle);
+
+	return 1;
+}
+
+static int drm_mode_cursor2(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_cursor2 cursor;
+
+	if (entering(tcp) || umove(tcp, arg, &cursor))
+		return 0;
+
+	tprintf(", {flags=0x%x, crtc_id=%u, x=%d, y=%d, width=%u, height=%u, "
+		"handle=%u, hot_x=%d, hot_y=%d}", cursor.flags, cursor.crtc_id,
+		cursor.x, cursor.y, cursor.width, cursor.height, cursor.handle,
+		cursor.hot_x, cursor.hot_y);
+
+	return 1;
+}
+
+static int drm_mode_get_gamma(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_crtc_lut lut;
+
+	if (entering(tcp) || umove(tcp, arg, &lut))
+		return 0;
+
+	/* We don't print the entire table, just the pointers */
+	tprintf(", {crtc_id=%u, gamma_size=%u, red=%p, green=%p, blue=%p}",
+		lut.crtc_id, lut.gamma_size, (void *)lut.red, (void *)lut.green,
+		(void *)lut.blue);
+
+	return 1;
+}
+
+
+static int drm_mode_set_gamma(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_crtc_lut lut;
+
+	if (entering(tcp) || umove(tcp, arg, &lut))
+		return 0;
+
+	/* We don't print the entire table, just the rgb pointers */
+	tprintf(", {crtc_id=%u, gamma_size=%u, red=%p, green=%p, blue=%p}",
+		lut.crtc_id, lut.gamma_size, (void *)lut.red, (void *)lut.green,
+		(void *)lut.blue);
+
+	return 1;
+}
+
+static int drm_mode_get_encoder(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_get_encoder enc;
+
+	if (entering(tcp) || umove(tcp, arg, &enc))
+		return 0;
+
+	/* TODO: Print name of encoder type */
+	tprintf(", {encoder_id=%u, encoder_type=%u, crtc_id=%u, "
+		"possible_crtcs=0x%x, possible_clones=0x%x}", enc.encoder_id,
+		enc.encoder_type, enc.crtc_id, enc.possible_crtcs,
+		enc.possible_clones);
+
+	return 1;
+}
+
+static int drm_mode_get_connector(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_get_connector con;
+
+	if (entering(tcp) || umove(tcp, arg, &con))
+		return 0;
+
+	/* We could be very verbose here but keep is simple for now */
+	tprintf(", {encoders_ptr=%p, modes_ptr=%p, props_ptr=%p, "
+		"prop_values_ptr=%p, count_modes=%u, count_props=%u, "
+		"count_encoders=%u, encoder_id=%u, connector_id=%u, "
+		"connector_type=%u, connector_type_id=%u, connection=%u, "
+		"mm_width=%u, mm_height=%u, subpixel=%u}",
+		(void *)con.encoders_ptr, (void *)con.modes_ptr,
+		(void *)con.props_ptr, (void *)con.prop_values_ptr,
+		con.count_modes, con.count_props, con.count_encoders,
+		con.encoder_id, con.connector_id, con.connector_type,
+		con.connector_type_id, con.connection, con.mm_width,
+		con.mm_height, con.subpixel);
+
+	return 1;
+}
+
+static int drm_mode_attachmode(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_mode_cmd cmd;
+
+	if (entering(tcp) || umove(tcp, arg, &cmd))
+		return 0;
+
+	tprintf(", {connector_id=%u, mode={", cmd.connector_id);
+	drm_mode_print_modeinfo(&cmd.mode);
+	tprintf("}");
+
+	return 1;
+}
+
+static int drm_mode_detachmode(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_mode_cmd cmd;
+
+	if (entering(tcp) || umove(tcp, arg, &cmd))
+		return 0;
+
+	tprintf(", {connector_id=%u, mode={", cmd.connector_id);
+	drm_mode_print_modeinfo(&cmd.mode);
+	tprintf("}");
+
+	return 1;
+}
+
+static int drm_mode_get_property(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_get_property prop;
+
+	if (entering(tcp) || umove(tcp, arg, &prop))
+		return 0;
+
+	tprintf(", {values_ptr=%p, enum_blob_ptr=%p, prop_id=%u, flags=0x%x, "
+		"name=%s, count_values=%u, count_enum_blobs=%u}",
+		(void *)prop.values_ptr, (void *)prop.enum_blob_ptr,
+		prop.prop_id, prop.flags, prop.name, prop.count_values,
+		prop.count_enum_blobs);
+
+	return 1;
+}
+
+static int drm_mode_set_property(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_connector_set_property prop;
+
+	if (entering(tcp) || umove(tcp, arg, &prop))
+		return 0;
+
+	tprintf(", {value=%Lu, prop_id=%u, connector_id=%u}", prop.value,
+		prop.prop_id, prop.connector_id);
+
+	return 1;
+}
+
+static int drm_mode_get_prop_blob(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_get_blob blob;
+
+	if (entering(tcp) || umove(tcp, arg, &blob))
+		return 0;
+
+	tprintf(", {blob_id=%u, length=%u, data=%p}", blob.blob_id, blob.length,
+		(void *)blob.data);
+
+	return 1;
+}
+
+static int drm_mode_add_fb(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_fb_cmd cmd;
+
+	if (entering(tcp) || umove(tcp, arg, &cmd))
+		return 0;
+
+	tprintf(", {fb_id=%u, width=%u, height=%u, pitch=%u, bpp=%u, depth=%u, "
+		"handle=%u}", cmd.fb_id, cmd.width, cmd.height, cmd.pitch,
+		cmd.bpp, cmd.depth, cmd.handle);
+
+	return 1;
+}
+
+static int drm_mode_get_fb(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_fb_cmd cmd;
+
+	if (entering(tcp) || umove(tcp, arg, &cmd))
+		return 0;
+
+	tprintf(", {fb_id=%u, width=%u, height=%u, pitch=%u, bpp=%u, depth=%u, "
+		"handle=%u}", cmd.fb_id, cmd.width, cmd.height, cmd.pitch,
+		cmd.bpp, cmd.depth, cmd.handle);
+
+	return 1;
+}
+
+static int drm_mode_rm_fb(struct tcb *tcp, const unsigned int code, long arg)
+{
+	unsigned int handle;
+
+	if (entering(tcp) || umove(tcp, arg, &handle))
+		return 0;
+
+	tprintf(", %u", handle);
+
+	return 1;
+}
+
+static int drm_mode_page_flip(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_crtc_page_flip flip;
+
+	if (entering(tcp) || umove(tcp, arg, &flip))
+		return 0;
+
+	tprintf(", {crtc_id=%u, fb_id=%u, flags=0x%x, user_data=0x%Lx}",
+		flip.crtc_id, flip.fb_id, flip.flags, flip.user_data);
+
+	return 1;
+}
+
+static int drm_mode_dirty_fb(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_fb_dirty_cmd cmd;
+
+	if (entering(tcp) || umove(tcp, arg, &cmd))
+		return 0;
+
+	tprintf(", {fb_id=%u, flags=0x%x, color=0x%x, num_clips=%u, "
+		"clips_ptr=%p}", cmd.fb_id, cmd.flags, cmd.color, cmd.num_clips,
+		(void *)cmd.clips_ptr);
+
+	return 1;
+}
+
+static int drm_mode_create_dumb(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_create_dumb dumb;
+
+	if (entering(tcp) || umove(tcp, arg, &dumb))
+		return 0;
+
+	tprintf(", {height=%u, width=%u, bpp=%u, flags=0x%x, handle=%u, "
+		"pitch=%u, size=%Lu}", dumb.height, dumb.width, dumb.bpp,
+		dumb.flags, dumb.handle, dumb.pitch, dumb.size);
+
+	return 1;
+}
+
+static int drm_mode_map_dumb(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_map_dumb dumb;
+
+	if (entering(tcp) || umove(tcp, arg, &dumb))
+		return 0;
+
+	tprintf(", {handle=%u, offset=%Lu}", dumb.handle, dumb.offset);
+
+	return 1;
+}
+
+static int drm_mode_destroy_dumb(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_mode_destroy_dumb dumb;
+
+	if (entering(tcp) || umove(tcp, arg, &dumb))
+		return 0;
+
+	tprintf(", {handle=%u}", dumb.handle);
+
+	return 1;
+}
+
+static int drm_gem_close(struct tcb *tcp, const unsigned int code, long arg)
+{
+	struct drm_gem_close close;
+
+	if (entering(tcp) || umove(tcp, arg, &close))
+		return 0;
+
+	tprintf(", {handle=%u}", close.handle);
+
+	return 1;
+}
+
 int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
 {
 	/* Check for device specific ioctls */
@@ -90,5 +552,62 @@ int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
 			return drm_i915_ioctl(tcp, code, arg);
 	}
 
+	switch (code) {
+	case DRM_IOCTL_VERSION:
+		return drm_version(tcp, code, arg);
+	case DRM_IOCTL_GET_UNIQUE:
+		return drm_get_unique(tcp, code, arg);
+	case DRM_IOCTL_GET_MAGIC:
+		return drm_get_magic(tcp, code, arg);
+	case DRM_IOCTL_WAIT_VBLANK:
+		return drm_wait_vblank(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETRESOURCES:
+		return drm_mode_get_resources(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETCRTC:
+		return drm_mode_get_crtc(tcp, code, arg);
+	case DRM_IOCTL_MODE_SETCRTC:
+		return drm_mode_set_crtc(tcp, code, arg);
+	case DRM_IOCTL_MODE_CURSOR:
+		return drm_mode_cursor(tcp, code, arg);
+	case DRM_IOCTL_MODE_CURSOR2:
+		return drm_mode_cursor2(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETGAMMA:
+		return drm_mode_get_gamma(tcp, code, arg);
+	case DRM_IOCTL_MODE_SETGAMMA:
+		return drm_mode_set_gamma(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETENCODER:
+		return drm_mode_get_encoder(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETCONNECTOR:
+		return drm_mode_get_connector(tcp, code, arg);
+	case DRM_IOCTL_MODE_ATTACHMODE:
+		return drm_mode_attachmode(tcp, code, arg);
+	case DRM_IOCTL_MODE_DETACHMODE:
+		return drm_mode_detachmode(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETPROPERTY:
+		return drm_mode_get_property(tcp, code, arg);
+	case DRM_IOCTL_MODE_SETPROPERTY:
+		return drm_mode_set_property(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETPROPBLOB:
+		return drm_mode_get_prop_blob(tcp, code, arg);
+	case DRM_IOCTL_MODE_GETFB:
+		return drm_mode_get_fb(tcp, code, arg);
+	case DRM_IOCTL_MODE_ADDFB:
+		return drm_mode_add_fb(tcp, code, arg);
+	case DRM_IOCTL_MODE_RMFB:
+		return drm_mode_rm_fb(tcp, code, arg);
+	case DRM_IOCTL_MODE_PAGE_FLIP:
+		return drm_mode_page_flip(tcp, code, arg);
+	case DRM_IOCTL_MODE_DIRTYFB:
+		return drm_mode_dirty_fb(tcp, code, arg);
+	case DRM_IOCTL_MODE_CREATE_DUMB:
+		return drm_mode_create_dumb(tcp, code, arg);
+	case DRM_IOCTL_MODE_MAP_DUMB:
+		return drm_mode_map_dumb(tcp, code, arg);
+	case DRM_IOCTL_MODE_DESTROY_DUMB:
+		return drm_mode_destroy_dumb(tcp, code, arg);
+	case DRM_IOCTL_GEM_CLOSE:
+		return drm_gem_close(tcp, code, arg);
+	}
+
 	return 0;
 }
-- 
2.1.0

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

Re: [PATCH 4/4] drm: Add decoding of DRM and KMS ioctls

[Gabriel Laskar]

On Tue,  9 Jun 2015 13:26:44 +0200
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:

> This patch adds many of the DRM and KMS ioctls. The rest can be added as
> needed.
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> ---
>  drm.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 519 insertions(+)
> 
> diff --git a/drm.c b/drm.c
> index fa98fb7..e550c34 100644
> --- a/drm.c
> +++ b/drm.c
> @@ -82,6 +82,468 @@ int drm_is_driver(struct tcb *tcp, const char *name)
>  	return strcmp(name, drv) == 0;
>  }
>  
> +static int drm_version(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_version ver;
> +	char *name, *date, *desc;
> +	int ret;
> +
> +	if (entering(tcp) || umove(tcp, arg, &ver))
> +		return 0;
> +
> +	name = calloc(ver.name_len + 1, 1);

We have some wrappers for that now, you can call xcalloc(), but it will
die if this does not work. Your version have the advantage of not kill
strace, and just not decode the argument.

> +	if (!name)
> +		return 0;
> +	ret = umovestr(tcp, (long)ver.name, ver.name_len, name);
> +	if (ret < 0)
> +		goto free_name;
> +
> +	date = calloc(ver.date_len + 1, 1);
> +	if (!date)
> +		goto free_name;
> +	ret = umovestr(tcp, (long)ver.date, ver.date_len, date);
> +	if (ret < 0)
> +		goto free_date;
> +
> +	desc = calloc(ver.desc_len + 1, 1);
> +	if (!desc)
> +		goto free_date;
> +	ret = umovestr(tcp, (long)ver.desc, ver.desc_len, desc);
> +	if (ret < 0)
> +		goto free_desc;
> +
> +	tprintf(", {version_major=%d, version_minor=%d, version_patchlevel=%d, "
> +		"name_len=%lu, name=%s, date_len=%lu, date=%s, "
> +		"desc_len=%lu, desc=%s}",
> +		ver.version_major, ver.version_minor, ver.version_patchlevel,
> +		ver.name_len, name, ver.date_len, date, ver.desc_len,
> +		desc);
> +
> +free_desc:
> +	free(desc);
> +free_date:
> +	free(date);
> +free_name:
> +	free(name);
> +
> +	if (ret < 0)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int drm_get_unique(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_unique unique;
> +	char *str;
> +	int ret;
> +
> +	if (entering(tcp) || umove(tcp, arg, &unique))
> +		return 0;
> +
> +	str = calloc(unique.unique_len + 1, 1);
> +	if (!str)
> +		return 0;
> +	ret = umovestr(tcp, (long)unique.unique, unique.unique_len, str);
> +

There is a check missing here

> +	tprintf(", {unique_len=%lu, unique=%s}", unique.unique_len, str);
> +
> +	free(str);
> +
> +	if (ret < 0)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int drm_get_magic(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_auth auth;
> +
> +	if (entering(tcp) || umove(tcp, arg, &auth))
> +		return 0;
> +
> +	tprintf(", {magic=%u}", auth.magic);
> +
> +	return 1;
> +}
> +
> +static int drm_wait_vblank(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	union drm_wait_vblank vblank;
> +
> +	if (umove(tcp, arg, &vblank))
> +		return 0;
> +
> +	if (entering(tcp)) {
> +		tprintf(", {request={type=%u, sequence=%u, signal=%lu}",
> +			vblank.request.type, vblank.request.sequence,
> +			vblank.request.signal);
> +	} else {
> +		tprintf(", reply={type=%u, sequence=%u, tval_sec=%ld, tval_usec=%ld}}",
> +			vblank.reply.type, vblank.reply.sequence,
> +			vblank.reply.tval_sec, vblank.reply.tval_usec);
> +	}
> +
> +	return 1;
> +}
> +
> +static int drm_mode_get_resources(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_card_res res;
> +
> +	if (entering(tcp) || umove(tcp, arg, &res))
> +		return 0;
> +
> +	tprintf(", {fb_id_ptr=%p, crtc_id_ptr=%p, connector_id_ptr=%p, "
> +		"encoder_id_ptr=%p, count_fbs=%u, count_crtcs=%u, "
> +		"count_connectors=%u, count_encoders=%u, min_width=%u, "
> +		"max_width=%u, min_height=%u, max_height=%u}",
> +		(void *)res.fb_id_ptr, (void *)res.crtc_id_ptr,
> +		(void *)res.connector_id_ptr, (void *)res.encoder_id_ptr,
> +		res.count_fbs, res.count_crtcs, res.count_connectors,
> +		res.count_encoders, res.min_width, res.max_width,
> +		res.min_height, res.max_height);
> +
> +	return 1;
> +}
> +
> +static void drm_mode_print_modeinfo(struct drm_mode_modeinfo *info)
> +{
> +	tprintf("clock=%u, hdisplay=%hu, hsync_start=%hu, hsync_end=%hu, "
> +		"htotal=%hu, hskew=%hu, vdisplay=%hu, vsync_start=%hu, "
> +		"vsync_end=%hu, vtotal=%hu, vscan=%hu, vrefresh=%u, "
> +		"flags=0x%x, type=%u, name=%s", info->clock, info->hdisplay,
> +		info->hsync_start, info->hsync_end, info->htotal, info->hskew,
> +		info->vdisplay, info->vsync_start, info->vsync_end,
> +		info->vtotal, info->vscan, info->vrefresh, info->flags,
> +		info->type, info->name);
> +}
> +
> +static int drm_mode_get_crtc(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_crtc crtc;
> +
> +	if (entering(tcp) || umove(tcp, arg, &crtc))
> +		return 0;
> +
> +	tprintf(", {set_connectors_ptr=%p, count_connectors=%u, crtc_id=%u, "
> +		"fb_id=%u, x=%u, y=%u, gamma_size=%u, mode_valid=%u, mode={",
> +		(void *)crtc.set_connectors_ptr, crtc.count_connectors,
> +		crtc.crtc_id, crtc.fb_id, crtc.x, crtc.y, crtc.gamma_size,
> +		crtc.mode_valid);
> +
> +	drm_mode_print_modeinfo(&crtc.mode);
> +	tprintf("}}");
> +
> +	return 1;
> +}
> +
> +static int drm_mode_set_crtc(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_crtc crtc;
> +
> +	if (entering(tcp) || umove(tcp, arg, &crtc))
> +		return 0;
> +
> +	tprintf(", {set_connectors_ptr=%p, count_connectors=%u, crtc_id=%u, "
> +		"fb_id=%u, x=%u, y=%u, gamma_size=%u, mode_valid=%u, mode={",
> +		(void *)crtc.set_connectors_ptr, crtc.count_connectors,
> +		crtc.crtc_id, crtc.fb_id, crtc.x, crtc.y, crtc.gamma_size,
> +		crtc.mode_valid);
> +
> +	drm_mode_print_modeinfo(&crtc.mode);
> +	tprintf("}}");
> +
> +	return 1;
> +}
> +
> +static int drm_mode_cursor(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_cursor cursor;
> +
> +	if (entering(tcp) || umove(tcp, arg, &cursor))
> +		return 0;
> +
> +	tprintf(", {flags=0x%x, crtc_id=%u, x=%d, y=%d, width=%u, height=%u, "
> +		"handle=%u}", cursor.flags, cursor.crtc_id, cursor.x, cursor.y,
> +		cursor.width, cursor.height, cursor.handle);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_cursor2(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_cursor2 cursor;
> +
> +	if (entering(tcp) || umove(tcp, arg, &cursor))
> +		return 0;
> +
> +	tprintf(", {flags=0x%x, crtc_id=%u, x=%d, y=%d, width=%u, height=%u, "
> +		"handle=%u, hot_x=%d, hot_y=%d}", cursor.flags, cursor.crtc_id,
> +		cursor.x, cursor.y, cursor.width, cursor.height, cursor.handle,
> +		cursor.hot_x, cursor.hot_y);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_get_gamma(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_crtc_lut lut;
> +
> +	if (entering(tcp) || umove(tcp, arg, &lut))
> +		return 0;
> +
> +	/* We don't print the entire table, just the pointers */
> +	tprintf(", {crtc_id=%u, gamma_size=%u, red=%p, green=%p, blue=%p}",
> +		lut.crtc_id, lut.gamma_size, (void *)lut.red, (void *)lut.green,
> +		(void *)lut.blue);
> +
> +	return 1;
> +}
> +
> +
> +static int drm_mode_set_gamma(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_crtc_lut lut;
> +
> +	if (entering(tcp) || umove(tcp, arg, &lut))
> +		return 0;
> +
> +	/* We don't print the entire table, just the rgb pointers */
> +	tprintf(", {crtc_id=%u, gamma_size=%u, red=%p, green=%p, blue=%p}",
> +		lut.crtc_id, lut.gamma_size, (void *)lut.red, (void *)lut.green,
> +		(void *)lut.blue);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_get_encoder(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_get_encoder enc;
> +
> +	if (entering(tcp) || umove(tcp, arg, &enc))
> +		return 0;
> +
> +	/* TODO: Print name of encoder type */
> +	tprintf(", {encoder_id=%u, encoder_type=%u, crtc_id=%u, "
> +		"possible_crtcs=0x%x, possible_clones=0x%x}", enc.encoder_id,
> +		enc.encoder_type, enc.crtc_id, enc.possible_crtcs,
> +		enc.possible_clones);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_get_connector(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_get_connector con;
> +
> +	if (entering(tcp) || umove(tcp, arg, &con))
> +		return 0;
> +
> +	/* We could be very verbose here but keep is simple for now */
> +	tprintf(", {encoders_ptr=%p, modes_ptr=%p, props_ptr=%p, "
> +		"prop_values_ptr=%p, count_modes=%u, count_props=%u, "
> +		"count_encoders=%u, encoder_id=%u, connector_id=%u, "
> +		"connector_type=%u, connector_type_id=%u, connection=%u, "
> +		"mm_width=%u, mm_height=%u, subpixel=%u}",
> +		(void *)con.encoders_ptr, (void *)con.modes_ptr,
> +		(void *)con.props_ptr, (void *)con.prop_values_ptr,
> +		con.count_modes, con.count_props, con.count_encoders,
> +		con.encoder_id, con.connector_id, con.connector_type,
> +		con.connector_type_id, con.connection, con.mm_width,
> +		con.mm_height, con.subpixel);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_attachmode(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_mode_cmd cmd;
> +
> +	if (entering(tcp) || umove(tcp, arg, &cmd))
> +		return 0;
> +
> +	tprintf(", {connector_id=%u, mode={", cmd.connector_id);
> +	drm_mode_print_modeinfo(&cmd.mode);
> +	tprintf("}");
> +
> +	return 1;
> +}
> +
> +static int drm_mode_detachmode(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_mode_cmd cmd;
> +
> +	if (entering(tcp) || umove(tcp, arg, &cmd))
> +		return 0;
> +
> +	tprintf(", {connector_id=%u, mode={", cmd.connector_id);
> +	drm_mode_print_modeinfo(&cmd.mode);
> +	tprintf("}");
> +
> +	return 1;
> +}
> +
> +static int drm_mode_get_property(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_get_property prop;
> +
> +	if (entering(tcp) || umove(tcp, arg, &prop))
> +		return 0;
> +
> +	tprintf(", {values_ptr=%p, enum_blob_ptr=%p, prop_id=%u, flags=0x%x, "
> +		"name=%s, count_values=%u, count_enum_blobs=%u}",
> +		(void *)prop.values_ptr, (void *)prop.enum_blob_ptr,
> +		prop.prop_id, prop.flags, prop.name, prop.count_values,
> +		prop.count_enum_blobs);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_set_property(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_connector_set_property prop;
> +
> +	if (entering(tcp) || umove(tcp, arg, &prop))
> +		return 0;
> +
> +	tprintf(", {value=%Lu, prop_id=%u, connector_id=%u}", prop.value,
> +		prop.prop_id, prop.connector_id);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_get_prop_blob(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_get_blob blob;
> +
> +	if (entering(tcp) || umove(tcp, arg, &blob))
> +		return 0;
> +
> +	tprintf(", {blob_id=%u, length=%u, data=%p}", blob.blob_id, blob.length,
> +		(void *)blob.data);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_add_fb(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_fb_cmd cmd;
> +
> +	if (entering(tcp) || umove(tcp, arg, &cmd))
> +		return 0;
> +
> +	tprintf(", {fb_id=%u, width=%u, height=%u, pitch=%u, bpp=%u, depth=%u, "
> +		"handle=%u}", cmd.fb_id, cmd.width, cmd.height, cmd.pitch,
> +		cmd.bpp, cmd.depth, cmd.handle);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_get_fb(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_fb_cmd cmd;
> +
> +	if (entering(tcp) || umove(tcp, arg, &cmd))
> +		return 0;
> +
> +	tprintf(", {fb_id=%u, width=%u, height=%u, pitch=%u, bpp=%u, depth=%u, "
> +		"handle=%u}", cmd.fb_id, cmd.width, cmd.height, cmd.pitch,
> +		cmd.bpp, cmd.depth, cmd.handle);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_rm_fb(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	unsigned int handle;
> +
> +	if (entering(tcp) || umove(tcp, arg, &handle))
> +		return 0;
> +
> +	tprintf(", %u", handle);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_page_flip(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_crtc_page_flip flip;
> +
> +	if (entering(tcp) || umove(tcp, arg, &flip))
> +		return 0;
> +
> +	tprintf(", {crtc_id=%u, fb_id=%u, flags=0x%x, user_data=0x%Lx}",
> +		flip.crtc_id, flip.fb_id, flip.flags, flip.user_data);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_dirty_fb(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_fb_dirty_cmd cmd;
> +
> +	if (entering(tcp) || umove(tcp, arg, &cmd))
> +		return 0;
> +
> +	tprintf(", {fb_id=%u, flags=0x%x, color=0x%x, num_clips=%u, "
> +		"clips_ptr=%p}", cmd.fb_id, cmd.flags, cmd.color, cmd.num_clips,
> +		(void *)cmd.clips_ptr);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_create_dumb(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_create_dumb dumb;
> +
> +	if (entering(tcp) || umove(tcp, arg, &dumb))
> +		return 0;
> +
> +	tprintf(", {height=%u, width=%u, bpp=%u, flags=0x%x, handle=%u, "
> +		"pitch=%u, size=%Lu}", dumb.height, dumb.width, dumb.bpp,
> +		dumb.flags, dumb.handle, dumb.pitch, dumb.size);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_map_dumb(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_map_dumb dumb;
> +
> +	if (entering(tcp) || umove(tcp, arg, &dumb))
> +		return 0;
> +
> +	tprintf(", {handle=%u, offset=%Lu}", dumb.handle, dumb.offset);
> +
> +	return 1;
> +}
> +
> +static int drm_mode_destroy_dumb(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_mode_destroy_dumb dumb;
> +
> +	if (entering(tcp) || umove(tcp, arg, &dumb))
> +		return 0;
> +
> +	tprintf(", {handle=%u}", dumb.handle);
> +
> +	return 1;
> +}
> +
> +static int drm_gem_close(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_gem_close close;
> +
> +	if (entering(tcp) || umove(tcp, arg, &close))
> +		return 0;
> +
> +	tprintf(", {handle=%u}", close.handle);
> +
> +	return 1;
> +}
> +
>  int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
>  {
>  	/* Check for device specific ioctls */
> @@ -90,5 +552,62 @@ int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
>  			return drm_i915_ioctl(tcp, code, arg);
>  	}
>  
> +	switch (code) {
> +	case DRM_IOCTL_VERSION:
> +		return drm_version(tcp, code, arg);
> +	case DRM_IOCTL_GET_UNIQUE:
> +		return drm_get_unique(tcp, code, arg);
> +	case DRM_IOCTL_GET_MAGIC:
> +		return drm_get_magic(tcp, code, arg);
> +	case DRM_IOCTL_WAIT_VBLANK:
> +		return drm_wait_vblank(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETRESOURCES:
> +		return drm_mode_get_resources(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETCRTC:
> +		return drm_mode_get_crtc(tcp, code, arg);
> +	case DRM_IOCTL_MODE_SETCRTC:
> +		return drm_mode_set_crtc(tcp, code, arg);
> +	case DRM_IOCTL_MODE_CURSOR:
> +		return drm_mode_cursor(tcp, code, arg);
> +	case DRM_IOCTL_MODE_CURSOR2:
> +		return drm_mode_cursor2(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETGAMMA:
> +		return drm_mode_get_gamma(tcp, code, arg);
> +	case DRM_IOCTL_MODE_SETGAMMA:
> +		return drm_mode_set_gamma(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETENCODER:
> +		return drm_mode_get_encoder(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETCONNECTOR:
> +		return drm_mode_get_connector(tcp, code, arg);
> +	case DRM_IOCTL_MODE_ATTACHMODE:
> +		return drm_mode_attachmode(tcp, code, arg);
> +	case DRM_IOCTL_MODE_DETACHMODE:
> +		return drm_mode_detachmode(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETPROPERTY:
> +		return drm_mode_get_property(tcp, code, arg);
> +	case DRM_IOCTL_MODE_SETPROPERTY:
> +		return drm_mode_set_property(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETPROPBLOB:
> +		return drm_mode_get_prop_blob(tcp, code, arg);
> +	case DRM_IOCTL_MODE_GETFB:
> +		return drm_mode_get_fb(tcp, code, arg);
> +	case DRM_IOCTL_MODE_ADDFB:
> +		return drm_mode_add_fb(tcp, code, arg);
> +	case DRM_IOCTL_MODE_RMFB:
> +		return drm_mode_rm_fb(tcp, code, arg);
> +	case DRM_IOCTL_MODE_PAGE_FLIP:
> +		return drm_mode_page_flip(tcp, code, arg);
> +	case DRM_IOCTL_MODE_DIRTYFB:
> +		return drm_mode_dirty_fb(tcp, code, arg);
> +	case DRM_IOCTL_MODE_CREATE_DUMB:
> +		return drm_mode_create_dumb(tcp, code, arg);
> +	case DRM_IOCTL_MODE_MAP_DUMB:
> +		return drm_mode_map_dumb(tcp, code, arg);
> +	case DRM_IOCTL_MODE_DESTROY_DUMB:
> +		return drm_mode_destroy_dumb(tcp, code, arg);
> +	case DRM_IOCTL_GEM_CLOSE:
> +		return drm_gem_close(tcp, code, arg);
> +	}
> +
>  	return 0;
>  }



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

Re: [PATCH 4/4] drm: Add decoding of DRM and KMS ioctls

[Patrik Jakobsson]

On Tue, Jun 09, 2015 at 03:51:08PM +0200, Gabriel Laskar wrote:
> On Tue,  9 Jun 2015 13:26:44 +0200
> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> 
> > This patch adds many of the DRM and KMS ioctls. The rest can be added as
> > needed.
> > 
> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > ---
> >  drm.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 519 insertions(+)
> > 
> > diff --git a/drm.c b/drm.c
> > index fa98fb7..e550c34 100644
> > --- a/drm.c
> > +++ b/drm.c
> > @@ -82,6 +82,468 @@ int drm_is_driver(struct tcb *tcp, const char *name)
> >  	return strcmp(name, drv) == 0;
> >  }
> >  
> > +static int drm_version(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_version ver;
> > +	char *name, *date, *desc;
> > +	int ret;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &ver))
> > +		return 0;
> > +
> > +	name = calloc(ver.name_len + 1, 1);
> 
> We have some wrappers for that now, you can call xcalloc(), but it will
> die if this does not work. Your version have the advantage of not kill
> strace, and just not decode the argument.
> 

If ok I'll keep it as calloc? As you say, dying here is not really neccessary.

> > +	if (!name)
> > +		return 0;
> > +	ret = umovestr(tcp, (long)ver.name, ver.name_len, name);
> > +	if (ret < 0)
> > +		goto free_name;
> > +
> > +	date = calloc(ver.date_len + 1, 1);
> > +	if (!date)
> > +		goto free_name;
> > +	ret = umovestr(tcp, (long)ver.date, ver.date_len, date);
> > +	if (ret < 0)
> > +		goto free_date;
> > +
> > +	desc = calloc(ver.desc_len + 1, 1);
> > +	if (!desc)
> > +		goto free_date;
> > +	ret = umovestr(tcp, (long)ver.desc, ver.desc_len, desc);
> > +	if (ret < 0)
> > +		goto free_desc;
> > +
> > +	tprintf(", {version_major=%d, version_minor=%d, version_patchlevel=%d, "
> > +		"name_len=%lu, name=%s, date_len=%lu, date=%s, "
> > +		"desc_len=%lu, desc=%s}",
> > +		ver.version_major, ver.version_minor, ver.version_patchlevel,
> > +		ver.name_len, name, ver.date_len, date, ver.desc_len,
> > +		desc);
> > +
> > +free_desc:
> > +	free(desc);
> > +free_date:
> > +	free(date);
> > +free_name:
> > +	free(name);
> > +
> > +	if (ret < 0)
> > +		return 0;
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_get_unique(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_unique unique;
> > +	char *str;
> > +	int ret;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &unique))
> > +		return 0;
> > +
> > +	str = calloc(unique.unique_len + 1, 1);
> > +	if (!str)
> > +		return 0;
> > +	ret = umovestr(tcp, (long)unique.unique, unique.unique_len, str);
> > +
> 
> There is a check missing here

Oops, will fix.

> 
> > +	tprintf(", {unique_len=%lu, unique=%s}", unique.unique_len, str);
> > +
> > +	free(str);
> > +
> > +	if (ret < 0)
> > +		return 0;
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_get_magic(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_auth auth;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &auth))
> > +		return 0;
> > +
> > +	tprintf(", {magic=%u}", auth.magic);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_wait_vblank(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	union drm_wait_vblank vblank;
> > +
> > +	if (umove(tcp, arg, &vblank))
> > +		return 0;
> > +
> > +	if (entering(tcp)) {
> > +		tprintf(", {request={type=%u, sequence=%u, signal=%lu}",
> > +			vblank.request.type, vblank.request.sequence,
> > +			vblank.request.signal);
> > +	} else {
> > +		tprintf(", reply={type=%u, sequence=%u, tval_sec=%ld, tval_usec=%ld}}",
> > +			vblank.reply.type, vblank.reply.sequence,
> > +			vblank.reply.tval_sec, vblank.reply.tval_usec);
> > +	}
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_get_resources(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_card_res res;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &res))
> > +		return 0;
> > +
> > +	tprintf(", {fb_id_ptr=%p, crtc_id_ptr=%p, connector_id_ptr=%p, "
> > +		"encoder_id_ptr=%p, count_fbs=%u, count_crtcs=%u, "
> > +		"count_connectors=%u, count_encoders=%u, min_width=%u, "
> > +		"max_width=%u, min_height=%u, max_height=%u}",
> > +		(void *)res.fb_id_ptr, (void *)res.crtc_id_ptr,
> > +		(void *)res.connector_id_ptr, (void *)res.encoder_id_ptr,
> > +		res.count_fbs, res.count_crtcs, res.count_connectors,
> > +		res.count_encoders, res.min_width, res.max_width,
> > +		res.min_height, res.max_height);
> > +
> > +	return 1;
> > +}
> > +
> > +static void drm_mode_print_modeinfo(struct drm_mode_modeinfo *info)
> > +{
> > +	tprintf("clock=%u, hdisplay=%hu, hsync_start=%hu, hsync_end=%hu, "
> > +		"htotal=%hu, hskew=%hu, vdisplay=%hu, vsync_start=%hu, "
> > +		"vsync_end=%hu, vtotal=%hu, vscan=%hu, vrefresh=%u, "
> > +		"flags=0x%x, type=%u, name=%s", info->clock, info->hdisplay,
> > +		info->hsync_start, info->hsync_end, info->htotal, info->hskew,
> > +		info->vdisplay, info->vsync_start, info->vsync_end,
> > +		info->vtotal, info->vscan, info->vrefresh, info->flags,
> > +		info->type, info->name);
> > +}
> > +
> > +static int drm_mode_get_crtc(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_crtc crtc;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &crtc))
> > +		return 0;
> > +
> > +	tprintf(", {set_connectors_ptr=%p, count_connectors=%u, crtc_id=%u, "
> > +		"fb_id=%u, x=%u, y=%u, gamma_size=%u, mode_valid=%u, mode={",
> > +		(void *)crtc.set_connectors_ptr, crtc.count_connectors,
> > +		crtc.crtc_id, crtc.fb_id, crtc.x, crtc.y, crtc.gamma_size,
> > +		crtc.mode_valid);
> > +
> > +	drm_mode_print_modeinfo(&crtc.mode);
> > +	tprintf("}}");
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_set_crtc(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_crtc crtc;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &crtc))
> > +		return 0;
> > +
> > +	tprintf(", {set_connectors_ptr=%p, count_connectors=%u, crtc_id=%u, "
> > +		"fb_id=%u, x=%u, y=%u, gamma_size=%u, mode_valid=%u, mode={",
> > +		(void *)crtc.set_connectors_ptr, crtc.count_connectors,
> > +		crtc.crtc_id, crtc.fb_id, crtc.x, crtc.y, crtc.gamma_size,
> > +		crtc.mode_valid);
> > +
> > +	drm_mode_print_modeinfo(&crtc.mode);
> > +	tprintf("}}");
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_cursor(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_cursor cursor;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &cursor))
> > +		return 0;
> > +
> > +	tprintf(", {flags=0x%x, crtc_id=%u, x=%d, y=%d, width=%u, height=%u, "
> > +		"handle=%u}", cursor.flags, cursor.crtc_id, cursor.x, cursor.y,
> > +		cursor.width, cursor.height, cursor.handle);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_cursor2(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_cursor2 cursor;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &cursor))
> > +		return 0;
> > +
> > +	tprintf(", {flags=0x%x, crtc_id=%u, x=%d, y=%d, width=%u, height=%u, "
> > +		"handle=%u, hot_x=%d, hot_y=%d}", cursor.flags, cursor.crtc_id,
> > +		cursor.x, cursor.y, cursor.width, cursor.height, cursor.handle,
> > +		cursor.hot_x, cursor.hot_y);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_get_gamma(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_crtc_lut lut;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &lut))
> > +		return 0;
> > +
> > +	/* We don't print the entire table, just the pointers */
> > +	tprintf(", {crtc_id=%u, gamma_size=%u, red=%p, green=%p, blue=%p}",
> > +		lut.crtc_id, lut.gamma_size, (void *)lut.red, (void *)lut.green,
> > +		(void *)lut.blue);
> > +
> > +	return 1;
> > +}
> > +
> > +
> > +static int drm_mode_set_gamma(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_crtc_lut lut;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &lut))
> > +		return 0;
> > +
> > +	/* We don't print the entire table, just the rgb pointers */
> > +	tprintf(", {crtc_id=%u, gamma_size=%u, red=%p, green=%p, blue=%p}",
> > +		lut.crtc_id, lut.gamma_size, (void *)lut.red, (void *)lut.green,
> > +		(void *)lut.blue);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_get_encoder(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_get_encoder enc;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &enc))
> > +		return 0;
> > +
> > +	/* TODO: Print name of encoder type */
> > +	tprintf(", {encoder_id=%u, encoder_type=%u, crtc_id=%u, "
> > +		"possible_crtcs=0x%x, possible_clones=0x%x}", enc.encoder_id,
> > +		enc.encoder_type, enc.crtc_id, enc.possible_crtcs,
> > +		enc.possible_clones);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_get_connector(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_get_connector con;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &con))
> > +		return 0;
> > +
> > +	/* We could be very verbose here but keep is simple for now */
> > +	tprintf(", {encoders_ptr=%p, modes_ptr=%p, props_ptr=%p, "
> > +		"prop_values_ptr=%p, count_modes=%u, count_props=%u, "
> > +		"count_encoders=%u, encoder_id=%u, connector_id=%u, "
> > +		"connector_type=%u, connector_type_id=%u, connection=%u, "
> > +		"mm_width=%u, mm_height=%u, subpixel=%u}",
> > +		(void *)con.encoders_ptr, (void *)con.modes_ptr,
> > +		(void *)con.props_ptr, (void *)con.prop_values_ptr,
> > +		con.count_modes, con.count_props, con.count_encoders,
> > +		con.encoder_id, con.connector_id, con.connector_type,
> > +		con.connector_type_id, con.connection, con.mm_width,
> > +		con.mm_height, con.subpixel);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_attachmode(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_mode_cmd cmd;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &cmd))
> > +		return 0;
> > +
> > +	tprintf(", {connector_id=%u, mode={", cmd.connector_id);
> > +	drm_mode_print_modeinfo(&cmd.mode);
> > +	tprintf("}");
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_detachmode(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_mode_cmd cmd;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &cmd))
> > +		return 0;
> > +
> > +	tprintf(", {connector_id=%u, mode={", cmd.connector_id);
> > +	drm_mode_print_modeinfo(&cmd.mode);
> > +	tprintf("}");
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_get_property(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_get_property prop;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &prop))
> > +		return 0;
> > +
> > +	tprintf(", {values_ptr=%p, enum_blob_ptr=%p, prop_id=%u, flags=0x%x, "
> > +		"name=%s, count_values=%u, count_enum_blobs=%u}",
> > +		(void *)prop.values_ptr, (void *)prop.enum_blob_ptr,
> > +		prop.prop_id, prop.flags, prop.name, prop.count_values,
> > +		prop.count_enum_blobs);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_set_property(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_connector_set_property prop;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &prop))
> > +		return 0;
> > +
> > +	tprintf(", {value=%Lu, prop_id=%u, connector_id=%u}", prop.value,
> > +		prop.prop_id, prop.connector_id);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_get_prop_blob(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_get_blob blob;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &blob))
> > +		return 0;
> > +
> > +	tprintf(", {blob_id=%u, length=%u, data=%p}", blob.blob_id, blob.length,
> > +		(void *)blob.data);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_add_fb(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_fb_cmd cmd;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &cmd))
> > +		return 0;
> > +
> > +	tprintf(", {fb_id=%u, width=%u, height=%u, pitch=%u, bpp=%u, depth=%u, "
> > +		"handle=%u}", cmd.fb_id, cmd.width, cmd.height, cmd.pitch,
> > +		cmd.bpp, cmd.depth, cmd.handle);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_get_fb(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_fb_cmd cmd;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &cmd))
> > +		return 0;
> > +
> > +	tprintf(", {fb_id=%u, width=%u, height=%u, pitch=%u, bpp=%u, depth=%u, "
> > +		"handle=%u}", cmd.fb_id, cmd.width, cmd.height, cmd.pitch,
> > +		cmd.bpp, cmd.depth, cmd.handle);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_rm_fb(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	unsigned int handle;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &handle))
> > +		return 0;
> > +
> > +	tprintf(", %u", handle);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_page_flip(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_crtc_page_flip flip;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &flip))
> > +		return 0;
> > +
> > +	tprintf(", {crtc_id=%u, fb_id=%u, flags=0x%x, user_data=0x%Lx}",
> > +		flip.crtc_id, flip.fb_id, flip.flags, flip.user_data);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_dirty_fb(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_fb_dirty_cmd cmd;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &cmd))
> > +		return 0;
> > +
> > +	tprintf(", {fb_id=%u, flags=0x%x, color=0x%x, num_clips=%u, "
> > +		"clips_ptr=%p}", cmd.fb_id, cmd.flags, cmd.color, cmd.num_clips,
> > +		(void *)cmd.clips_ptr);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_create_dumb(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_create_dumb dumb;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &dumb))
> > +		return 0;
> > +
> > +	tprintf(", {height=%u, width=%u, bpp=%u, flags=0x%x, handle=%u, "
> > +		"pitch=%u, size=%Lu}", dumb.height, dumb.width, dumb.bpp,
> > +		dumb.flags, dumb.handle, dumb.pitch, dumb.size);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_map_dumb(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_map_dumb dumb;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &dumb))
> > +		return 0;
> > +
> > +	tprintf(", {handle=%u, offset=%Lu}", dumb.handle, dumb.offset);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_mode_destroy_dumb(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_mode_destroy_dumb dumb;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &dumb))
> > +		return 0;
> > +
> > +	tprintf(", {handle=%u}", dumb.handle);
> > +
> > +	return 1;
> > +}
> > +
> > +static int drm_gem_close(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_gem_close close;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &close))
> > +		return 0;
> > +
> > +	tprintf(", {handle=%u}", close.handle);
> > +
> > +	return 1;
> > +}
> > +
> >  int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> >  {
> >  	/* Check for device specific ioctls */
> > @@ -90,5 +552,62 @@ int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> >  			return drm_i915_ioctl(tcp, code, arg);
> >  	}
> >  
> > +	switch (code) {
> > +	case DRM_IOCTL_VERSION:
> > +		return drm_version(tcp, code, arg);
> > +	case DRM_IOCTL_GET_UNIQUE:
> > +		return drm_get_unique(tcp, code, arg);
> > +	case DRM_IOCTL_GET_MAGIC:
> > +		return drm_get_magic(tcp, code, arg);
> > +	case DRM_IOCTL_WAIT_VBLANK:
> > +		return drm_wait_vblank(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETRESOURCES:
> > +		return drm_mode_get_resources(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETCRTC:
> > +		return drm_mode_get_crtc(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_SETCRTC:
> > +		return drm_mode_set_crtc(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_CURSOR:
> > +		return drm_mode_cursor(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_CURSOR2:
> > +		return drm_mode_cursor2(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETGAMMA:
> > +		return drm_mode_get_gamma(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_SETGAMMA:
> > +		return drm_mode_set_gamma(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETENCODER:
> > +		return drm_mode_get_encoder(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETCONNECTOR:
> > +		return drm_mode_get_connector(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_ATTACHMODE:
> > +		return drm_mode_attachmode(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_DETACHMODE:
> > +		return drm_mode_detachmode(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETPROPERTY:
> > +		return drm_mode_get_property(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_SETPROPERTY:
> > +		return drm_mode_set_property(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETPROPBLOB:
> > +		return drm_mode_get_prop_blob(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_GETFB:
> > +		return drm_mode_get_fb(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_ADDFB:
> > +		return drm_mode_add_fb(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_RMFB:
> > +		return drm_mode_rm_fb(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_PAGE_FLIP:
> > +		return drm_mode_page_flip(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_DIRTYFB:
> > +		return drm_mode_dirty_fb(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_CREATE_DUMB:
> > +		return drm_mode_create_dumb(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_MAP_DUMB:
> > +		return drm_mode_map_dumb(tcp, code, arg);
> > +	case DRM_IOCTL_MODE_DESTROY_DUMB:
> > +		return drm_mode_destroy_dumb(tcp, code, arg);
> > +	case DRM_IOCTL_GEM_CLOSE:
> > +		return drm_gem_close(tcp, code, arg);
> > +	}
> > +
> >  	return 0;
> >  }
> 
> 
> 
> -- 
> Gabriel Laskar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm: Add decoding of DRM and KMS ioctls

[Gabriel Laskar]

On Tue, 9 Jun 2015 16:29:31 +0200
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:

> On Tue, Jun 09, 2015 at 03:51:08PM +0200, Gabriel Laskar wrote:
> > On Tue,  9 Jun 2015 13:26:44 +0200
> > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> > 
> > > This patch adds many of the DRM and KMS ioctls. The rest can be added as
> > > needed.
> > > 
> > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > > ---
> > >  drm.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 519 insertions(+)
> > > 
> > > diff --git a/drm.c b/drm.c
> > > index fa98fb7..e550c34 100644
> > > --- a/drm.c
> > > +++ b/drm.c
> > > @@ -82,6 +82,468 @@ int drm_is_driver(struct tcb *tcp, const char *name)
> > >  	return strcmp(name, drv) == 0;
> > >  }
> > >  
> > > +static int drm_version(struct tcb *tcp, const unsigned int code, long arg)
> > > +{
> > > +	struct drm_version ver;
> > > +	char *name, *date, *desc;
> > > +	int ret;
> > > +
> > > +	if (entering(tcp) || umove(tcp, arg, &ver))
> > > +		return 0;
> > > +
> > > +	name = calloc(ver.name_len + 1, 1);
> > 
> > We have some wrappers for that now, you can call xcalloc(), but it will
> > die if this does not work. Your version have the advantage of not kill
> > strace, and just not decode the argument.
> > 
> 
> If ok I'll keep it as calloc? As you say, dying here is not really neccessary.

Yeah, that was mostly me thinking out loud. Imho, I would keep the
calloc(), but I don't know what Dmitry will prefer. Maybe for
consistency, it should be better to have an xcalloc().

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

Re: [PATCH 4/4] drm: Add decoding of DRM and KMS ioctls

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 2072 bytes --]

On Tue, Jun 09, 2015 at 04:38:40PM +0200, Gabriel Laskar wrote:
> On Tue, 9 Jun 2015 16:29:31 +0200
> Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> 
> > On Tue, Jun 09, 2015 at 03:51:08PM +0200, Gabriel Laskar wrote:
> > > On Tue,  9 Jun 2015 13:26:44 +0200
> > > Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> > > 
> > > > This patch adds many of the DRM and KMS ioctls. The rest can be added as
> > > > needed.
> > > > 
> > > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > > > ---
> > > >  drm.c | 519 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 519 insertions(+)
> > > > 
> > > > diff --git a/drm.c b/drm.c
> > > > index fa98fb7..e550c34 100644
> > > > --- a/drm.c
> > > > +++ b/drm.c
> > > > @@ -82,6 +82,468 @@ int drm_is_driver(struct tcb *tcp, const char *name)
> > > >  	return strcmp(name, drv) == 0;
> > > >  }
> > > >  
> > > > +static int drm_version(struct tcb *tcp, const unsigned int code, long arg)
> > > > +{
> > > > +	struct drm_version ver;
> > > > +	char *name, *date, *desc;
> > > > +	int ret;
> > > > +
> > > > +	if (entering(tcp) || umove(tcp, arg, &ver))
> > > > +		return 0;
> > > > +
> > > > +	name = calloc(ver.name_len + 1, 1);
> > > 
> > > We have some wrappers for that now, you can call xcalloc(), but it will
> > > die if this does not work. Your version have the advantage of not kill
> > > strace, and just not decode the argument.
> > > 
> > 
> > If ok I'll keep it as calloc? As you say, dying here is not really neccessary.
> 
> Yeah, that was mostly me thinking out loud. Imho, I would keep the
> calloc(), but I don't know what Dmitry will prefer. Maybe for
> consistency, it should be better to have an xcalloc().

The general rule is to call xcalloc() if die_out_of_memory() is the best
way to handle the OOM condition.  strace should neither attempt to
allocate arbitrary large chunks of memory specified by user input
nor die when such allocation requests fail.


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 4/4] drm: Add decoding of DRM and KMS ioctls

[Dmitry V. Levin]

[-- Attachment #1.1: Type: text/plain, Size: 691 bytes --]

On Tue, Jun 09, 2015 at 01:26:44PM +0200, Patrik Jakobsson wrote:
[...]
> +static int drm_version(struct tcb *tcp, const unsigned int code, long arg)
> +{
> +	struct drm_version ver;
> +	char *name, *date, *desc;
> +	int ret;
> +
> +	if (entering(tcp) || umove(tcp, arg, &ver))
> +		return 0;
> +
> +	name = calloc(ver.name_len + 1, 1);
> +	if (!name)
> +		return 0;
> +	ret = umovestr(tcp, (long)ver.name, ver.name_len, name);
> +	if (ret < 0)
> +		goto free_name;

No need to allocate (arbitrary sized chunk of) memory just to fetch
a string that has to be printed.  There is a function called printstr()
to print such strings in a properly quoted form.


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 4/4] drm: Add decoding of DRM and KMS ioctls

[Patrik Jakobsson]

On Wed, Jun 10, 2015 at 01:46:53AM +0300, Dmitry V. Levin wrote:
> On Tue, Jun 09, 2015 at 01:26:44PM +0200, Patrik Jakobsson wrote:
> [...]
> > +static int drm_version(struct tcb *tcp, const unsigned int code, long arg)
> > +{
> > +	struct drm_version ver;
> > +	char *name, *date, *desc;
> > +	int ret;
> > +
> > +	if (entering(tcp) || umove(tcp, arg, &ver))
> > +		return 0;
> > +
> > +	name = calloc(ver.name_len + 1, 1);
> > +	if (!name)
> > +		return 0;
> > +	ret = umovestr(tcp, (long)ver.name, ver.name_len, name);
> > +	if (ret < 0)
> > +		goto free_name;
> 
> No need to allocate (arbitrary sized chunk of) memory just to fetch
> a string that has to be printed.  There is a function called printstr()
> to print such strings in a properly quoted form.
> 

Yes, that's much nicer. Fixed in v2.

> 
> -- 
> ldv


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

Re: [PATCH 0/4] drm: Add decoding for DRM/KMS and i915 ioctls

[Gabriel Laskar]

On Tue,  9 Jun 2015 13:26:40 +0200
Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:

> This set of patches adds a dispatcher for handling DRM ioctls. The
> kernel headers for DRM might not be available on all distributions
> so we depend on libdrm for those. If libdrm is not available we fall
> back on the kernel headers. Since DRM drivers share the same range of
> private ioctl numbers I've added a function for detecting the driver
> based on it's name.
> 
> Patrik Jakobsson (4):
>   drm: Add config for detecting libdrm
>   drm: Add dispatcher and driver identification for DRM
>   drm: Add decoding of i915 ioctls
>   drm: Add decoding of DRM and KMS ioctls
> 
>  Makefile.am                |   2 +
>  configure.ac               |   4 +
>  defs.h                     |   8 +-
>  drm.c                      | 613 +++++++++++++++++++++++++++++++++++++++++++++
>  drm_i915.c                 | 287 +++++++++++++++++++++
>  io.c                       |   2 +-
>  ioctl.c                    |  19 +-
>  xlat/drm_i915_getparams.in |  28 +++
>  xlat/drm_i915_ioctls.in    |  51 ++++
>  xlat/drm_i915_setparams.in |   4 +
>  10 files changed, 1015 insertions(+), 3 deletions(-)
>  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
> 

Nice work!

This looks good to me. I have just tested it a little.

You should rebase to master, it does not apply for the moment, and add
the Changelog info in your commits (looks at the log, the global
Changelog is extracted from the commit messages).

Other little remarks are inlines in the patches.

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

Re: [PATCH 0/4] drm: Add decoding for DRM/KMS and i915 ioctls

[Patrik Jakobsson]

On Tue, Jun 09, 2015 at 03:51:05PM +0200, Gabriel Laskar wrote:
> On Tue,  9 Jun 2015 13:26:40 +0200
> Patrik Jakobsson <patrik.jakobsson-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> 
> > This set of patches adds a dispatcher for handling DRM ioctls. The
> > kernel headers for DRM might not be available on all distributions
> > so we depend on libdrm for those. If libdrm is not available we fall
> > back on the kernel headers. Since DRM drivers share the same range of
> > private ioctl numbers I've added a function for detecting the driver
> > based on it's name.
> > 
> > Patrik Jakobsson (4):
> >   drm: Add config for detecting libdrm
> >   drm: Add dispatcher and driver identification for DRM
> >   drm: Add decoding of i915 ioctls
> >   drm: Add decoding of DRM and KMS ioctls
> > 
> >  Makefile.am                |   2 +
> >  configure.ac               |   4 +
> >  defs.h                     |   8 +-
> >  drm.c                      | 613 +++++++++++++++++++++++++++++++++++++++++++++
> >  drm_i915.c                 | 287 +++++++++++++++++++++
> >  io.c                       |   2 +-
> >  ioctl.c                    |  19 +-
> >  xlat/drm_i915_getparams.in |  28 +++
> >  xlat/drm_i915_ioctls.in    |  51 ++++
> >  xlat/drm_i915_setparams.in |   4 +
> >  10 files changed, 1015 insertions(+), 3 deletions(-)
> >  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
> > 
> 
> Nice work!
> 
> This looks good to me. I have just tested it a little.
> 
> You should rebase to master, it does not apply for the moment, and add
> the Changelog info in your commits (looks at the log, the global
> Changelog is extracted from the commit messages).
> 
> Other little remarks are inlines in the patches.

Thanks for the review. I'll rebase and add the changelog.

Will send out a v2 of the series tomorrow.

Cheers
Patrik
> 
> -- 
> Gabriel Laskar

------------------------------------------------------------------------------