All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Hellstrom <thellstrom@vmware.com>
To: "emil.l.velikov@gmail.com" <emil.l.velikov@gmail.com>
Cc: "kernel@collabora.com" <kernel@collabora.com>,
	Linux-graphics-maintainer <Linux-graphics-maintainer@vmware.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check
Date: Fri, 24 May 2019 22:39:53 +0000	[thread overview]
Message-ID: <27cb3f6d2002edcf45a4d50e6cef0854ba80766a.camel@vmware.com> (raw)
In-Reply-To: <20190524152648.GD8938@arch-x1c3>

Hi, Emil

On Fri, 2019-05-24 at 16:26 +0100, Emil Velikov wrote:
> On 2019/05/24, Thomas Hellstrom wrote:
> > On Fri, 2019-05-24 at 13:14 +0100, Emil Velikov wrote:
> > > On 2019/05/23, Thomas Hellstrom wrote:
> > > > Hi, Emil,
> > > > 
> > > > On Wed, 2019-05-22 at 17:41 +0100, Emil Velikov wrote:
> > > > > From: Emil Velikov <emil.velikov@collabora.com>
> > > > > 
> > > > > Drop the custom ioctl io encoding check - core drm does it
> > > > > for
> > > > > us.
> > > > 
> > > > I fail to see where the core does this, or do I miss something?
> > > 
> > > drm_ioctl() allows for the encoding to be changed and attributes
> > > that
> > > only the
> > > appropriate size is copied in/out of the kernel.
> > > 
> > > Technically the function is more relaxed relative to the vmwgfx
> > > check, yet
> > > seems perfectly reasonable.
> > > 
> > > Is there any corner-case that isn't but should be handled in
> > > drm_ioctl()?
> > 
> > I'd like to turn the question around and ask whether there's a
> > reason
> > we should relax the vmwgfx test? In the past it has trapped quite a
> > few
> > user-space errors.
> > 
> The way I see it either:
>  - the check, as-is, is unnessesary, or
>  - it is needed, and we should do something equivalent for all of DRM
> 
> We had a very long brainstorming session with a colleague and we
> could not see
> any cases where this would cause a problem. If you recall anything
> concrete
> please let me know - I would be more than happy to take a closer
> look.

If you have a good reason to drop an ioctl sanity check, I'd be
perfectly happy to do it. To me, a good reason even includes "I have a
non-open-source customer having problems with this check" because of
reason etc. etc. as long as I have a way to evaluate those reasons and
determine if they're valid or not. "No other drm driver nor the core is
doing this" is NOT a valid reason to me. In particular if the check is
not affecting performance. So unless you provide additional reasons to
drop this check, it's a solid NAK from my side.

Thanks,
Thomas


> 
> Thanks
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2019-05-24 22:40 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-22 16:41 [PATCH 1/5] vmwgfx: drop empty lastclose stub Emil Velikov
2019-05-22 16:41 ` [PATCH 2/5] drm/vmgfx: kill off unused init_mutex Emil Velikov
2019-05-23  6:48   ` Thomas Hellstrom
2019-05-22 16:41 ` [PATCH 3/5] drm/vmwgfx: use core drm to extend/check vmw_execbuf_ioctl Emil Velikov
2019-05-22 19:01   ` Thomas Hellstrom
2019-05-22 19:09     ` Daniel Vetter
2019-05-24  6:05       ` Thomas Hellstrom
2019-05-24  7:46         ` Daniel Vetter
2019-05-24 10:53           ` Emil Velikov
2019-05-24 10:56             ` Thomas Hellstrom
2019-05-23  8:52   ` Thomas Hellstrom
2019-05-22 16:41 ` [PATCH 4/5] drm/vmwgfx: remove custom ioctl io encoding check Emil Velikov
2019-05-23  6:44   ` Thomas Hellstrom
2019-05-24 12:14     ` Emil Velikov
2019-05-24 13:04       ` Thomas Hellstrom
2019-05-24 15:26         ` Emil Velikov
2019-05-24 22:39           ` Thomas Hellstrom [this message]
2019-05-25  8:25             ` Thomas Hellstrom
2019-05-27  9:08               ` Emil Velikov
2019-05-27 11:34                 ` Thomas Hellstrom
2019-05-27 12:35                   ` Emil Velikov
2019-05-27 13:44                     ` Thomas Hellstrom
2019-05-27 15:27                       ` Emil Velikov
2019-05-27 15:50                         ` Thomas Hellstrom
2019-05-27 16:36                           ` Emil Velikov
2019-05-22 16:41 ` [PATCH 5/5] drm: make drm_ioctl_permit() internal Emil Velikov
2019-05-23  6:47 ` [PATCH 1/5] vmwgfx: drop empty lastclose stub Thomas Hellstrom

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27cb3f6d2002edcf45a4d50e6cef0854ba80766a.camel@vmware.com \
    --to=thellstrom@vmware.com \
    --cc=Linux-graphics-maintainer@vmware.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.l.velikov@gmail.com \
    --cc=kernel@collabora.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.