dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Provide compat ioctl for addfb2.1
@ 2015-07-13 15:56 Tvrtko Ursulin
  2015-07-13 17:18 ` Rob Clark
  2015-07-14  8:15 ` [Intel-gfx] " Daniel Vetter
  0 siblings, 2 replies; 3+ messages in thread
From: Tvrtko Ursulin @ 2015-07-13 15:56 UTC (permalink / raw)
  To: Intel-gfx; +Cc: Daniel Vetter, Daniel Stone, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Frame buffer modifiers extensions provided in;

  commit e3eb3250d84ef97b766312345774367b6a310db8
  Author: Rob Clark <robdclark@gmail.com>
  Date:   Thu Feb 5 14:41:52 2015 +0000

      drm: add support for tiled/compressed/etc modifier in addfb2

Missed the structure packing/alignment problem where 64-bit
members were added after the odd number of 32-bit ones. This
makes the compiler produce structures of different sizes under
32- and 64-bit x86 targets and makes the ioctl need explicit
compat handling.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: Rob Clark <robdclark@gmail.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioc32.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
index aa8bbb460c57..2bea11bfb3ae 100644
--- a/drivers/gpu/drm/drm_ioc32.c
+++ b/drivers/gpu/drm/drm_ioc32.c
@@ -70,6 +70,8 @@
 
 #define DRM_IOCTL_WAIT_VBLANK32		DRM_IOWR(0x3a, drm_wait_vblank32_t)
 
+#define DRM_IOCTL_MODE_ADDFB232		DRM_IOWR(0xb8, drm_mode_fb_cmd232_t)
+
 typedef struct drm_version_32 {
 	int version_major;	  /**< Major version */
 	int version_minor;	  /**< Minor version */
@@ -1016,6 +1018,63 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd,
 	return 0;
 }
 
+typedef struct drm_mode_fb_cmd232 {
+	u32 fb_id;
+	u32 width;
+	u32 height;
+	u32 pixel_format;
+	u32 flags;
+	u32 handles[4];
+	u32 pitches[4];
+	u32 offsets[4];
+	u64 modifier[4];
+} __attribute__((packed)) drm_mode_fb_cmd232_t;
+
+static int compat_drm_mode_addfb2(struct file *file, unsigned int cmd,
+				  unsigned long arg)
+{
+	drm_mode_fb_cmd232_t __user *argp = (void __user *)arg;
+	struct drm_mode_fb_cmd232 req32;
+	struct drm_mode_fb_cmd2 __user *req64;
+	int i;
+	int err;
+
+	if (copy_from_user(&req32, argp, sizeof(req32)))
+		return -EFAULT;
+
+	req64 = compat_alloc_user_space(sizeof(*req64));
+
+	if (!access_ok(VERIFY_WRITE, req64, sizeof(*req64))
+	    || __put_user(req32.width, &req64->width)
+	    || __put_user(req32.height, &req64->height)
+	    || __put_user(req32.pixel_format, &req64->pixel_format)
+	    || __put_user(req32.flags, &req64->flags))
+		return -EFAULT;
+
+	for (i = 0; i < 4; i++) {
+		if (__put_user(req32.handles[i], &req64->handles[i]))
+			return -EFAULT;
+		if (__put_user(req32.pitches[i], &req64->pitches[i]))
+			return -EFAULT;
+		if (__put_user(req32.offsets[i], &req64->offsets[i]))
+			return -EFAULT;
+		if (__put_user(req32.modifier[i], &req64->modifier[i]))
+			return -EFAULT;
+	}
+
+	err = drm_ioctl(file, DRM_IOCTL_MODE_ADDFB2, (unsigned long)req64);
+	if (err)
+		return err;
+
+	if (__get_user(req32.fb_id, &req64->fb_id))
+		return -EFAULT;
+
+	if (copy_to_user(argp, &req32, sizeof(req32)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static drm_ioctl_compat_t *drm_compat_ioctls[] = {
 	[DRM_IOCTL_NR(DRM_IOCTL_VERSION32)] = compat_drm_version,
 	[DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE32)] = compat_drm_getunique,
@@ -1048,6 +1107,7 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = {
 	[DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw,
 #endif
 	[DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank,
+	[DRM_IOCTL_NR(DRM_IOCTL_MODE_ADDFB232)] = compat_drm_mode_addfb2,
 };
 
 /**
-- 
2.4.2

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm: Provide compat ioctl for addfb2.1
  2015-07-13 15:56 [PATCH] drm: Provide compat ioctl for addfb2.1 Tvrtko Ursulin
@ 2015-07-13 17:18 ` Rob Clark
  2015-07-14  8:15 ` [Intel-gfx] " Daniel Vetter
  1 sibling, 0 replies; 3+ messages in thread
From: Rob Clark @ 2015-07-13 17:18 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: Daniel Vetter, Intel Graphics Development, Daniel Stone,
	dri-devel@lists.freedesktop.org, Tvrtko Ursulin

btw, I mentioned on irc, but someone *really* ought to go through the
ioclt structs and split each struct member into it's own line, so
things like '__u32 width, height' aren't so sneaky ;-)

On Mon, Jul 13, 2015 at 11:56 AM, Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Frame buffer modifiers extensions provided in;
>
>   commit e3eb3250d84ef97b766312345774367b6a310db8
>   Author: Rob Clark <robdclark@gmail.com>
>   Date:   Thu Feb 5 14:41:52 2015 +0000
>
>       drm: add support for tiled/compressed/etc modifier in addfb2
>
> Missed the structure packing/alignment problem where 64-bit
> members were added after the odd number of 32-bit ones. This
> makes the compiler produce structures of different sizes under
> 32- and 64-bit x86 targets and makes the ioctl need explicit
> compat handling.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_ioc32.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index aa8bbb460c57..2bea11bfb3ae 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -70,6 +70,8 @@
>
>  #define DRM_IOCTL_WAIT_VBLANK32                DRM_IOWR(0x3a, drm_wait_vblank32_t)
>
> +#define DRM_IOCTL_MODE_ADDFB232                DRM_IOWR(0xb8, drm_mode_fb_cmd232_t)
> +
>  typedef struct drm_version_32 {
>         int version_major;        /**< Major version */
>         int version_minor;        /**< Minor version */
> @@ -1016,6 +1018,63 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd,
>         return 0;
>  }
>
> +typedef struct drm_mode_fb_cmd232 {
> +       u32 fb_id;
> +       u32 width;
> +       u32 height;
> +       u32 pixel_format;
> +       u32 flags;
> +       u32 handles[4];
> +       u32 pitches[4];
> +       u32 offsets[4];
> +       u64 modifier[4];
> +} __attribute__((packed)) drm_mode_fb_cmd232_t;
> +
> +static int compat_drm_mode_addfb2(struct file *file, unsigned int cmd,
> +                                 unsigned long arg)
> +{
> +       drm_mode_fb_cmd232_t __user *argp = (void __user *)arg;
> +       struct drm_mode_fb_cmd232 req32;
> +       struct drm_mode_fb_cmd2 __user *req64;
> +       int i;
> +       int err;
> +
> +       if (copy_from_user(&req32, argp, sizeof(req32)))
> +               return -EFAULT;
> +
> +       req64 = compat_alloc_user_space(sizeof(*req64));
> +
> +       if (!access_ok(VERIFY_WRITE, req64, sizeof(*req64))
> +           || __put_user(req32.width, &req64->width)
> +           || __put_user(req32.height, &req64->height)
> +           || __put_user(req32.pixel_format, &req64->pixel_format)
> +           || __put_user(req32.flags, &req64->flags))
> +               return -EFAULT;
> +
> +       for (i = 0; i < 4; i++) {
> +               if (__put_user(req32.handles[i], &req64->handles[i]))
> +                       return -EFAULT;
> +               if (__put_user(req32.pitches[i], &req64->pitches[i]))
> +                       return -EFAULT;
> +               if (__put_user(req32.offsets[i], &req64->offsets[i]))
> +                       return -EFAULT;
> +               if (__put_user(req32.modifier[i], &req64->modifier[i]))
> +                       return -EFAULT;
> +       }
> +
> +       err = drm_ioctl(file, DRM_IOCTL_MODE_ADDFB2, (unsigned long)req64);
> +       if (err)
> +               return err;
> +
> +       if (__get_user(req32.fb_id, &req64->fb_id))
> +               return -EFAULT;
> +
> +       if (copy_to_user(argp, &req32, sizeof(req32)))
> +               return -EFAULT;
> +
> +       return 0;
> +}
> +
>  static drm_ioctl_compat_t *drm_compat_ioctls[] = {
>         [DRM_IOCTL_NR(DRM_IOCTL_VERSION32)] = compat_drm_version,
>         [DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE32)] = compat_drm_getunique,
> @@ -1048,6 +1107,7 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = {
>         [DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw,
>  #endif
>         [DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank,
> +       [DRM_IOCTL_NR(DRM_IOCTL_MODE_ADDFB232)] = compat_drm_mode_addfb2,
>  };
>
>  /**
> --
> 2.4.2
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Intel-gfx] [PATCH] drm: Provide compat ioctl for addfb2.1
  2015-07-13 15:56 [PATCH] drm: Provide compat ioctl for addfb2.1 Tvrtko Ursulin
  2015-07-13 17:18 ` Rob Clark
@ 2015-07-14  8:15 ` Daniel Vetter
  1 sibling, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2015-07-14  8:15 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx, Daniel Stone, dri-devel

On Mon, Jul 13, 2015 at 04:56:53PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Frame buffer modifiers extensions provided in;
> 
>   commit e3eb3250d84ef97b766312345774367b6a310db8
>   Author: Rob Clark <robdclark@gmail.com>
>   Date:   Thu Feb 5 14:41:52 2015 +0000
> 
>       drm: add support for tiled/compressed/etc modifier in addfb2
> 
> Missed the structure packing/alignment problem where 64-bit
> members were added after the odd number of 32-bit ones. This
> makes the compiler produce structures of different sizes under
> 32- and 64-bit x86 targets and makes the ioctl need explicit
> compat handling.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_ioc32.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index aa8bbb460c57..2bea11bfb3ae 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -70,6 +70,8 @@
>  
>  #define DRM_IOCTL_WAIT_VBLANK32		DRM_IOWR(0x3a, drm_wait_vblank32_t)
>  
> +#define DRM_IOCTL_MODE_ADDFB232		DRM_IOWR(0xb8, drm_mode_fb_cmd232_t)
> +
>  typedef struct drm_version_32 {
>  	int version_major;	  /**< Major version */
>  	int version_minor;	  /**< Minor version */
> @@ -1016,6 +1018,63 @@ static int compat_drm_wait_vblank(struct file *file, unsigned int cmd,
>  	return 0;
>  }
>  
> +typedef struct drm_mode_fb_cmd232 {

Please no typedefs for structs in new code. drm_ioc32.c has lots of them
simply because it's old. Also the patch needs cc: stable. With that

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +	u32 fb_id;
> +	u32 width;
> +	u32 height;
> +	u32 pixel_format;
> +	u32 flags;
> +	u32 handles[4];
> +	u32 pitches[4];
> +	u32 offsets[4];
> +	u64 modifier[4];
> +} __attribute__((packed)) drm_mode_fb_cmd232_t;
> +
> +static int compat_drm_mode_addfb2(struct file *file, unsigned int cmd,
> +				  unsigned long arg)
> +{
> +	drm_mode_fb_cmd232_t __user *argp = (void __user *)arg;
> +	struct drm_mode_fb_cmd232 req32;
> +	struct drm_mode_fb_cmd2 __user *req64;
> +	int i;
> +	int err;
> +
> +	if (copy_from_user(&req32, argp, sizeof(req32)))
> +		return -EFAULT;
> +
> +	req64 = compat_alloc_user_space(sizeof(*req64));
> +
> +	if (!access_ok(VERIFY_WRITE, req64, sizeof(*req64))
> +	    || __put_user(req32.width, &req64->width)
> +	    || __put_user(req32.height, &req64->height)
> +	    || __put_user(req32.pixel_format, &req64->pixel_format)
> +	    || __put_user(req32.flags, &req64->flags))
> +		return -EFAULT;
> +
> +	for (i = 0; i < 4; i++) {
> +		if (__put_user(req32.handles[i], &req64->handles[i]))
> +			return -EFAULT;
> +		if (__put_user(req32.pitches[i], &req64->pitches[i]))
> +			return -EFAULT;
> +		if (__put_user(req32.offsets[i], &req64->offsets[i]))
> +			return -EFAULT;
> +		if (__put_user(req32.modifier[i], &req64->modifier[i]))
> +			return -EFAULT;
> +	}
> +
> +	err = drm_ioctl(file, DRM_IOCTL_MODE_ADDFB2, (unsigned long)req64);
> +	if (err)
> +		return err;
> +
> +	if (__get_user(req32.fb_id, &req64->fb_id))
> +		return -EFAULT;
> +
> +	if (copy_to_user(argp, &req32, sizeof(req32)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
>  static drm_ioctl_compat_t *drm_compat_ioctls[] = {
>  	[DRM_IOCTL_NR(DRM_IOCTL_VERSION32)] = compat_drm_version,
>  	[DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE32)] = compat_drm_getunique,
> @@ -1048,6 +1107,7 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = {
>  	[DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw,
>  #endif
>  	[DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank,
> +	[DRM_IOCTL_NR(DRM_IOCTL_MODE_ADDFB232)] = compat_drm_mode_addfb2,
>  };
>  
>  /**
> -- 
> 2.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-07-14  8:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-13 15:56 [PATCH] drm: Provide compat ioctl for addfb2.1 Tvrtko Ursulin
2015-07-13 17:18 ` Rob Clark
2015-07-14  8:15 ` [Intel-gfx] " Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox