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

["Antoine, Peter" <peter.antoine@intel.com>]

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>
> 
> 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.

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