Re: [PATCH 4/5] drm: Make HW_LOCK access functions optional.

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 28, 2015 at 01:29:21PM +0000, Antoine, Peter wrote:
> On Tue, 2015-04-28 at 16:08 +0300, Ville Syrjälä wrote:
> > On Tue, Apr 28, 2015 at 11:29:06AM +0000, Antoine, Peter wrote:
> > > > > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 
> > > > > > 62c40777..367e42f 100644
> > > > > > --- a/include/drm/drmP.h
> > > > > > +++ b/include/drm/drmP.h
> > > > > > @@ -137,17 +137,18 @@ void drm_err(const char *format, ...);  /*@{*/
> > > > > >  
> > > > > >  /* driver capabilities and requirements mask */
> > > > > > -#define DRIVER_USE_AGP     0x1
> > > > > > -#define DRIVER_PCI_DMA     0x8
> > > > > > -#define DRIVER_SG          0x10
> > > > > > -#define DRIVER_HAVE_DMA    0x20
> > > > > > -#define DRIVER_HAVE_IRQ    0x40
> > > > > > -#define DRIVER_IRQ_SHARED  0x80
> > > > > > -#define DRIVER_GEM         0x1000
> > > > > > -#define DRIVER_MODESET     0x2000
> > > > > > -#define DRIVER_PRIME       0x4000
> > > > > > -#define DRIVER_RENDER      0x8000
> > > > > > -#define DRIVER_ATOMIC      0x10000
> > > > > > +#define DRIVER_USE_AGP			0x1
> > > > > > +#define DRIVER_PCI_DMA			0x8
> > > > > > +#define DRIVER_SG			0x10
> > > > > > +#define DRIVER_HAVE_DMA			0x20
> > > > > > +#define DRIVER_HAVE_IRQ			0x40
> > > > > > +#define DRIVER_IRQ_SHARED		0x80
> > > > > > +#define DRIVER_GEM			0x1000
> > > > > > +#define DRIVER_MODESET			0x2000
> > > > > > +#define DRIVER_PRIME			0x4000
> > > > > > +#define DRIVER_RENDER			0x8000
> > > > > > +#define DRIVER_ATOMIC			0x10000
> > > > > > +#define DRIVER_KMS_LEGACY_CONTEXT	0x20000
> > > > > 
> > > > > Why is there KMS in the name?
> > > > > 
> > > > > By suggestion of Daniel.
> > > > > 
> > > > > I was thinking just checking for GEM, but I think there was some
> > > > > gem+dri1 userland for i915 at some point in time. ums and dri1 are
> > > > > now dead as far as i915 is concerned, so in theory it should be fine.
> > > > > But I'm not sure if some other driver might have the same baggage.
> > > > > 
> > > > > Other drivers have the same baggage.
> > > > > 
> > > > > I suppose one option would be to check for MODESET instead. kms+dri1 doesn't sound like an entirely sane combination to me.
> > > > > 
> > > > > Can't use the MODESET as this was how it was turned off in the previous incarnation and was reverted by Dave Airle.
> > > > 
> > > > Reference?
> > > 
> > > From the next commit [5/5] as it is the one that actually turns off the
> > > functions that were turned off before.
> > > 
> > > These changes are based on the two patches:
> > >   commit c21eb21cb50d58e7cbdcb8b9e7ff68b85cfa5095
> > >   Author: Dave Airlie <airlied@redhat.com>
> > > 
> > > And the commit that the above patch reverts:
> > >   commit 7c510133d93dd6f15ca040733ba7b2891ed61fd1
> > >   Author: Daniel Vetter <daniel.vetter@ffwll.ch>

These two commits definitely should be referenced from the commit message,
othwerise no one will understand the history.

> > Looking at ancient libdrm sources makes me think nouveau just used to
> > create and destroy the context, but not actually use it for anything.
> > So nopping out the ioctls should be good enough AFAICS. Or am I missing
> > something?
> > 
> 
> An old version of libdrm that still requires support needs them, it's
> the reason that David Airlie reverted the patch that Daniel did to
> remove the functions. Do they still need support, I don't know? David
> Airlie is on the cc list.
> 
> A discussion was had and this was the way that it was suggested it be
> done. This seems a good half-way house, the actual security holes that
> have been found have been fixed and the functions (that seem to have a
> lot more security issues in them) are turned off for the drivers that
> don't use them, and if a driver does require them, it will be a one line
> change to reintroduce them. Are we carrying code we don't need to
> support, probably.

Iirc it was in the ddx, and it was actually using the mmap code. Leftovers
from ums, but unfortunately X crashes if we take them away. If I recall
correctly nouveau was in staging still, but per Linus staging or not
doesn't matter when distros are shipping with the code already. I did dig
out the details way back out of curiosity, but lost them since then again.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx