On 14 April 2016 at 11:06, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Wed, Mar 30, 2016 at 11:39:01AM +0100, Emil Velikov wrote: >> On 30 March 2016 at 10:45, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote: >> > Like in >> > >> > commit 0e975980d435d58df2d430d688b8c18778b42218 >> > Author: Peter Antoine <peter.antoine@xxxxxxxxx> >> > Date: Tue Jun 23 08:18:49 2015 +0100 >> > >> > drm: Turn off Legacy Context Functions >> > >> > we need to again make an exception for nouveau, but everyone else >> > really doesn't need this. >> > >> > Cc: Peter Antoine <peter.antoine@xxxxxxxxx> >> > Cc: Ben Skeggs <bskeggs@xxxxxxxxxx> >> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> >> > --- >> > drivers/gpu/drm/drm_bufs.c | 12 ++++++++++++ >> > 1 file changed, 12 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c >> > index d92db7007f62..e8a12a4fd400 100644 >> > --- a/drivers/gpu/drm/drm_bufs.c >> > +++ b/drivers/gpu/drm/drm_bufs.c >> > @@ -396,6 +396,10 @@ int drm_legacy_addmap_ioctl(struct drm_device *dev, void *data, >> > if (!(capable(CAP_SYS_ADMIN) || map->type == _DRM_AGP || map->type == _DRM_SHM)) >> > return -EPERM; >> > >> > + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) && >> > + drm_core_check_feature(dev, DRIVER_MODESET)) >> > + return -EINVAL; >> > + >> Wondering if making this the first check in the function won't be >> better ? We have a handful of places which preemptively check >> DRIVER_MODESET prior to calling drm_legacy functions and similarly >> some (last time I've looked) drm_legacy functions check for >> DRIVER_MODESET. Perhaps we can move all the checking into the >> drm_legacy API alone ? > > This is an ioctl handler, so there's not really any caller we can move > this to. Indeed. One could extract the legacy ioctls into separate table and/or use a function alike drm_ioctl_permit() to generalise things. It will allow clear and obvious separation of things, although the particular above will make things a bit annoying. > In general I'm split, and I just put checks like these wherever > it makes sense, pulling them out into callers if there's an entire pile of > them who all want the same checks. > > In the end I don't think it matters, as long as we dutifully combine > drm_legacy_* with such checks, so that it's _really_ all dead code for > modern drives. I'd suspect that there's a few bits that are missing the check, plus going through might be a bit time consuming. Thus thinking of a 'generic' way to handle things. Just thinking out loud :-) -Emil _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
