[Intel-gfx] [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