| Message ID | 1466148814-8194-17-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 17, 2016 at 09:33:34AM +0200, Daniel Vetter wrote: > Also extract drm_auth.h for nicer grouping. > > v2: Nuke the other comments since they don't really explain a lot, and > within the drm core we generally only document functions exported to > drivers: The main audience for these docs are driver writers. > > v3: Limit the exposure of drm_master internals by only including > drm_auth.h where it is neede (Chris). > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 9fa9698fe247..0f8632c93e95 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -47,6 +47,7 @@ > #include <drm/intel-gtt.h> > #include <drm/drm_legacy.h> /* for struct drm_dma_handle */ > #include <drm/drm_gem.h> > +#include <drm/drm_auth.h> I think we should be able to wean this further. -Chris
On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Also extract drm_auth.h for nicer grouping. > > v2: Nuke the other comments since they don't really explain a lot, and > within the drm core we generally only document functions exported to > drivers: The main audience for these docs are driver writers. > > v3: Limit the exposure of drm_master internals by only including > drm_auth.h where it is neede (Chris). > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > Documentation/DocBook/gpu.tmpl | 6 ++++ > drivers/gpu/drm/drm_auth.c | 69 +++++++++++++++++++++---------------- > drivers/gpu/drm/drm_crtc.c | 1 + > drivers/gpu/drm/drm_ioctl.c | 1 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + > include/drm/drmP.h | 30 +--------------- > include/drm/drm_auth.h | 59 +++++++++++++++++++++++++++++++ > include/drm/drm_legacy.h | 2 ++ > 9 files changed, 112 insertions(+), 58 deletions(-) > create mode 100644 include/drm/drm_auth.h > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 94c6bdee8080..b7f6316b7bee 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -3103,6 +3103,12 @@ int num_ioctls;</synopsis> > !Pdrivers/gpu/drm/drm_vma_manager.c getunique and setversion story > </sect1> > <sect1> > + <title>Primary Nodes, DRM Master and Authentication</title> > +!Pdrivers/gpu/drm_auth.c master and authentication > +!Edrivers/gpu/drm_auth.c > +!Einclude/drm/drm_auth.h > + </sect1> > + <sect1> > <title>Render nodes</title> > <para> > DRM core provides multiple character-devices for user-space to use. > diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c > index b4dfa8ab20d7..3774b9964dbe 100644 > --- a/drivers/gpu/drm/drm_auth.c > +++ b/drivers/gpu/drm/drm_auth.c > @@ -32,18 +32,27 @@ > #include "drm_internal.h" > #include "drm_legacy.h" > > -/** > - * drm_getmagic - Get unique magic of a client > - * @dev: DRM device to operate on > - * @data: ioctl data containing the drm_auth object > - * @file_priv: DRM file that performs the operation > +/** DOC: master and authentication > + * > + * struct &drm_master is used to track groups of clients with open > + * primary/legacy device nodes. For every struct &drm_file which at least once s/which at least/which has had at least/ > + * successfully became the device master (either through the SET_MASTER IOCTL, > + * or implicitly through opening the primary device node when no one else is the > + * current master that time) there exists one &drm_master. This is noted in the/ > + * is_master member of &drm_master. All other clients have just a pointer to the s/member of &drm_master/member of &drm_file/ > + * &drm_master they are associated with. > * > - * This looks up the unique magic of the passed client and returns it. If the > - * client did not have a magic assigned, yet, a new one is registered. The magic > - * is stored in the passed drm_auth object. > + * In addition only one &drm_master can be the current master for a &drm_device. > + * It can be switched through the DROP_MASTER and SET_MASTER IOCTL, or > + * implicitly through closing/openeing the primary device node. See also > + * drm_is_current_master(). > * > - * Returns: 0 on success, negative error code on failure. > + * Clients can authenticate against the current master (if it matches their own) > + * using the GETMAGIC and AUTHMAGIC IOCTLs. Together with exchanging masters, > + * this allows controlled access to the device for an entire group of mutually > + * trusted clients. > */ > + > int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) > { > struct drm_auth *auth = data; > @@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) > return ret < 0 ? ret : 0; > } > > -/** > - * drm_authmagic - Authenticate client with a magic > - * @dev: DRM device to operate on > - * @data: ioctl data containing the drm_auth object > - * @file_priv: DRM file that performs the operation > - * > - * This looks up a DRM client by the passed magic and authenticates it. > - * > - * Returns: 0 on success, negative error code on failure. > - */ Why is this and drm_getmagic()'s documetation going away ? Kernel doc isn't restricted to EXPORTED_SYMBOL(s) only, is it ? > int drm_authmagic(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > @@ -126,16 +125,6 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv, > return ret; > } > > -/* > - * drm_new_set_master - Allocate a new master object and become master for the > - * associated master realm. > - * > - * @dev: The associated device. > - * @fpriv: File private identifying the client. > - * > - * This function must be called with dev::master_mutex held. > - * Returns negative error code on failure. Zero on success. > - */ > static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) > { > struct drm_master *old_master; > @@ -288,12 +277,28 @@ out: > mutex_unlock(&dev->master_mutex); > } > > +/** > + * drm_is_current_master - checks whether this master is the current one s/this master is the current one/@fpriv is the current master/ perhaps ? > + * @fpriv: DRM file private > + * > + * Checks whether @fpriv is a master and that it is the current master on its s/a master and that it is the current/the current/ > + * device. This decides whether a client is allowed to run DRM_MASTER IOCTLs. > + * > + * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting > + * - the current master is assumed to own the non-shareable display hardware. > + */ > bool drm_is_current_master(struct drm_file *fpriv) > { > return fpriv->is_master && fpriv->master == fpriv->minor->dev->master; > } > EXPORT_SYMBOL(drm_is_current_master); > > +/** > + * drm_master_get - reference a master pointer > + * @master: struct &drm_master > + * > + * Increments the reference count of @master. Bikeshed: s/@master./@master and returns a pointer to @master./ > + */ > struct drm_master *drm_master_get(struct drm_master *master) > { > kref_get(&master->refcount); > @@ -316,6 +321,12 @@ static void drm_master_destroy(struct kref *kref) > kfree(master); > } > > +/** > + * drm_master_put - unreference and clear a master pointer > + * @master: pointer to a pointer of struct &drm_master > + * > + * This decrements the &drm_master behind @master and sets it to NULL. > + */ > void drm_master_put(struct drm_master **master) > { > kref_put(&(*master)->refcount, drm_master_destroy); > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 81083f98d155..871af372662d 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -39,6 +39,7 @@ > #include <drm/drm_fourcc.h> > #include <drm/drm_modeset_lock.h> > #include <drm/drm_atomic.h> > +#include <drm/drm_auth.h> > > #include "drm_crtc_internal.h" > #include "drm_internal.h" > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index a0c1d172954d..88796a383e40 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -30,6 +30,7 @@ > > #include <drm/drmP.h> > #include <drm/drm_core.h> > +#include <drm/drm_auth.h> > #include "drm_legacy.h" > #include "drm_internal.h" > #include "drm_crtc_internal.h" > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 9fa9698fe247..0f8632c93e95 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -47,6 +47,7 @@ > #include <drm/intel-gtt.h> > #include <drm/drm_legacy.h> /* for struct drm_dma_handle */ > #include <drm/drm_gem.h> > +#include <drm/drm_auth.h> > > #include "i915_params.h" > #include "i915_reg.h" > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > index 1980e2a28265..9a90f824814e 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > @@ -32,6 +32,7 @@ > #include <drm/drmP.h> > #include <drm/vmwgfx_drm.h> > #include <drm/drm_hashtab.h> > +#include <drm/drm_auth.h> > #include <linux/suspend.h> > #include <drm/ttm/ttm_bo_driver.h> > #include <drm/ttm/ttm_object.h> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 761b20332321..d22ba6bf2299 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -86,6 +86,7 @@ struct drm_local_map; > struct drm_device_dma; > struct drm_dma_handle; > struct drm_gem_object; > +struct drm_master; > > struct device_node; > struct videomode; > @@ -373,30 +374,6 @@ struct drm_lock_data { > int idle_has_lock; > }; > > -/** > - * struct drm_master - drm master structure > - * > - * @refcount: Refcount for this master object. > - * @dev: Link back to the DRM device > - * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex. > - * @unique_len: Length of unique field. Protected by drm_global_mutex. > - * @magic_map: Map of used authentication tokens. Protected by struct_mutex. > - * @lock: DRI lock information. > - * @driver_priv: Pointer to driver-private information. > - * > - * Note that master structures are only relevant for the legacy/primary device > - * nodes, hence there can only be one per device, not one per drm_minor. > - */ > -struct drm_master { > - struct kref refcount; > - struct drm_device *dev; > - char *unique; > - int unique_len; > - struct idr magic_map; > - struct drm_lock_data lock; > - void *driver_priv; > -}; > - > /* Flags and return codes for get_vblank_timestamp() driver function. */ > #define DRM_CALLED_FROM_VBLIRQ 1 > #define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0) > @@ -1008,11 +985,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc > return &crtc->dev->vblank[drm_crtc_index(crtc)].queue; > } > > -/* drm_auth.c */ > -struct drm_master *drm_master_get(struct drm_master *master); > -void drm_master_put(struct drm_master **master); > -bool drm_is_current_master(struct drm_file *fpriv); > - > /* drm_drv.c */ > void drm_put_dev(struct drm_device *dev); > void drm_unplug_dev(struct drm_device *dev); > diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h > new file mode 100644 > index 000000000000..610223b0481b > --- /dev/null > +++ b/include/drm/drm_auth.h > @@ -0,0 +1,59 @@ > +/* > + * Internal Header for the Direct Rendering Manager > + * > + * Copyright 2016 Intel Corporation > + * > + * Author: Daniel Vetter <daniel.vetter@ffwll.ch> > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + */ > + > +#ifndef _DRM_AUTH_H_ > +#define _DRM_AUTH_H_ > + > +/** > + * struct drm_master - drm master structure > + * > + * @refcount: Refcount for this master object. > + * @dev: Link back to the DRM device > + * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex. > + * @unique_len: Length of unique field. Protected by drm_global_mutex. > + * @magic_map: Map of used authentication tokens. Protected by struct_mutex. > + * @lock: DRI lock information. > + * @driver_priv: Pointer to driver-private information. > + * > + * Note that master structures are only relevant for the legacy/primary device > + * nodes, hence there can only be one per device, not one per drm_minor. > + */ > +struct drm_master { > + struct kref refcount; > + struct drm_device *dev; > + char *unique; > + int unique_len; > + struct idr magic_map; > + struct drm_lock_data lock; > + void *driver_priv; > +}; > + > +struct drm_master *drm_master_get(struct drm_master *master); > +void drm_master_put(struct drm_master **master); > +bool drm_is_current_master(struct drm_file *fpriv); > + > +#endif > diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h > index a5ef2c7e40f8..cf0e7d89bcdf 100644 > --- a/include/drm/drm_legacy.h > +++ b/include/drm/drm_legacy.h > @@ -1,6 +1,8 @@ > #ifndef __DRM_DRM_LEGACY_H__ > #define __DRM_DRM_LEGACY_H__ > > +#include <drm/drm_auth.h> > + > /* > * Legacy driver interfaces for the Direct Rendering Manager > * > -- > 2.8.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Sat, Jun 18, 2016 at 12:46:38AM +0100, Emil Velikov wrote: > On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > @@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) > > return ret < 0 ? ret : 0; > > } > > > > -/** > > - * drm_authmagic - Authenticate client with a magic > > - * @dev: DRM device to operate on > > - * @data: ioctl data containing the drm_auth object > > - * @file_priv: DRM file that performs the operation > > - * > > - * This looks up a DRM client by the passed magic and authenticates it. > > - * > > - * Returns: 0 on success, negative error code on failure. > > - */ > Why is this and drm_getmagic()'s documetation going away ? Kernel doc > isn't restricted to EXPORTED_SYMBOL(s) only, is it ? No, but imo the documentation for the drm core&helpers should be aimed at driver writers. And driver writers can only use EXPORT_SYMBOL stuff (or from headers), which is why I think it's good to kick out everything else to avoid too much clutter. It's already a daunting thing to get started with a new drm driver. Of course we can keep the comments as normal comments, but for these here I didn't see the value. Also note that this is just for the drm core/helpers. In the i915 driver documentation we instead try to document non-static stuff (since that's exposed to other parts), but just as a rough guideline. Since often our source file split doesn't make that much sense, or is too monolithic. In both cases the goal is to give a useful guideline to users of a piece of code (calling it or otherwise using), not developers changing the implementation details themselves. -Daniel > > > int drm_authmagic(struct drm_device *dev, void *data, > > struct drm_file *file_priv) > > { > > @@ -126,16 +125,6 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv, > > return ret; > > } > > > > -/* > > - * drm_new_set_master - Allocate a new master object and become master for the > > - * associated master realm. > > - * > > - * @dev: The associated device. > > - * @fpriv: File private identifying the client. > > - * > > - * This function must be called with dev::master_mutex held. > > - * Returns negative error code on failure. Zero on success. > > - */ > > static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) > > { > > struct drm_master *old_master; > > @@ -288,12 +277,28 @@ out: > > mutex_unlock(&dev->master_mutex); > > } > > > > +/** > > + * drm_is_current_master - checks whether this master is the current one > s/this master is the current one/@fpriv is the current master/ perhaps ? > > > + * @fpriv: DRM file private > > + * > > + * Checks whether @fpriv is a master and that it is the current master on its > s/a master and that it is the current/the current/ > > > + * device. This decides whether a client is allowed to run DRM_MASTER IOCTLs. > > + * > > + * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting > > + * - the current master is assumed to own the non-shareable display hardware. > > + */ > > bool drm_is_current_master(struct drm_file *fpriv) > > { > > return fpriv->is_master && fpriv->master == fpriv->minor->dev->master; > > } > > EXPORT_SYMBOL(drm_is_current_master); > > > > +/** > > + * drm_master_get - reference a master pointer > > + * @master: struct &drm_master > > + * > > + * Increments the reference count of @master. > Bikeshed: s/@master./@master and returns a pointer to @master./ > > > + */ > > struct drm_master *drm_master_get(struct drm_master *master) > > { > > kref_get(&master->refcount); > > @@ -316,6 +321,12 @@ static void drm_master_destroy(struct kref *kref) > > kfree(master); > > } > > > > +/** > > + * drm_master_put - unreference and clear a master pointer > > + * @master: pointer to a pointer of struct &drm_master > > + * > > + * This decrements the &drm_master behind @master and sets it to NULL. > > + */ > > void drm_master_put(struct drm_master **master) > > { > > kref_put(&(*master)->refcount, drm_master_destroy); > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index 81083f98d155..871af372662d 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -39,6 +39,7 @@ > > #include <drm/drm_fourcc.h> > > #include <drm/drm_modeset_lock.h> > > #include <drm/drm_atomic.h> > > +#include <drm/drm_auth.h> > > > > #include "drm_crtc_internal.h" > > #include "drm_internal.h" > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > > index a0c1d172954d..88796a383e40 100644 > > --- a/drivers/gpu/drm/drm_ioctl.c > > +++ b/drivers/gpu/drm/drm_ioctl.c > > @@ -30,6 +30,7 @@ > > > > #include <drm/drmP.h> > > #include <drm/drm_core.h> > > +#include <drm/drm_auth.h> > > #include "drm_legacy.h" > > #include "drm_internal.h" > > #include "drm_crtc_internal.h" > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 9fa9698fe247..0f8632c93e95 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -47,6 +47,7 @@ > > #include <drm/intel-gtt.h> > > #include <drm/drm_legacy.h> /* for struct drm_dma_handle */ > > #include <drm/drm_gem.h> > > +#include <drm/drm_auth.h> > > > > #include "i915_params.h" > > #include "i915_reg.h" > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > > index 1980e2a28265..9a90f824814e 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h > > @@ -32,6 +32,7 @@ > > #include <drm/drmP.h> > > #include <drm/vmwgfx_drm.h> > > #include <drm/drm_hashtab.h> > > +#include <drm/drm_auth.h> > > #include <linux/suspend.h> > > #include <drm/ttm/ttm_bo_driver.h> > > #include <drm/ttm/ttm_object.h> > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > > index 761b20332321..d22ba6bf2299 100644 > > --- a/include/drm/drmP.h > > +++ b/include/drm/drmP.h > > @@ -86,6 +86,7 @@ struct drm_local_map; > > struct drm_device_dma; > > struct drm_dma_handle; > > struct drm_gem_object; > > +struct drm_master; > > > > struct device_node; > > struct videomode; > > @@ -373,30 +374,6 @@ struct drm_lock_data { > > int idle_has_lock; > > }; > > > > -/** > > - * struct drm_master - drm master structure > > - * > > - * @refcount: Refcount for this master object. > > - * @dev: Link back to the DRM device > > - * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex. > > - * @unique_len: Length of unique field. Protected by drm_global_mutex. > > - * @magic_map: Map of used authentication tokens. Protected by struct_mutex. > > - * @lock: DRI lock information. > > - * @driver_priv: Pointer to driver-private information. > > - * > > - * Note that master structures are only relevant for the legacy/primary device > > - * nodes, hence there can only be one per device, not one per drm_minor. > > - */ > > -struct drm_master { > > - struct kref refcount; > > - struct drm_device *dev; > > - char *unique; > > - int unique_len; > > - struct idr magic_map; > > - struct drm_lock_data lock; > > - void *driver_priv; > > -}; > > - > > /* Flags and return codes for get_vblank_timestamp() driver function. */ > > #define DRM_CALLED_FROM_VBLIRQ 1 > > #define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0) > > @@ -1008,11 +985,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc > > return &crtc->dev->vblank[drm_crtc_index(crtc)].queue; > > } > > > > -/* drm_auth.c */ > > -struct drm_master *drm_master_get(struct drm_master *master); > > -void drm_master_put(struct drm_master **master); > > -bool drm_is_current_master(struct drm_file *fpriv); > > - > > /* drm_drv.c */ > > void drm_put_dev(struct drm_device *dev); > > void drm_unplug_dev(struct drm_device *dev); > > diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h > > new file mode 100644 > > index 000000000000..610223b0481b > > --- /dev/null > > +++ b/include/drm/drm_auth.h > > @@ -0,0 +1,59 @@ > > +/* > > + * Internal Header for the Direct Rendering Manager > > + * > > + * Copyright 2016 Intel Corporation > > + * > > + * Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining a > > + * copy of this software and associated documentation files (the "Software"), > > + * to deal in the Software without restriction, including without limitation > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > > + * and/or sell copies of the Software, and to permit persons to whom the > > + * Software is furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice (including the next > > + * paragraph) shall be included in all copies or substantial portions of the > > + * Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > > + * OTHER DEALINGS IN THE SOFTWARE. > > + */ > > + > > +#ifndef _DRM_AUTH_H_ > > +#define _DRM_AUTH_H_ > > + > > +/** > > + * struct drm_master - drm master structure > > + * > > + * @refcount: Refcount for this master object. > > + * @dev: Link back to the DRM device > > + * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex. > > + * @unique_len: Length of unique field. Protected by drm_global_mutex. > > + * @magic_map: Map of used authentication tokens. Protected by struct_mutex. > > + * @lock: DRI lock information. > > + * @driver_priv: Pointer to driver-private information. > > + * > > + * Note that master structures are only relevant for the legacy/primary device > > + * nodes, hence there can only be one per device, not one per drm_minor. > > + */ > > +struct drm_master { > > + struct kref refcount; > > + struct drm_device *dev; > > + char *unique; > > + int unique_len; > > + struct idr magic_map; > > + struct drm_lock_data lock; > > + void *driver_priv; > > +}; > > + > > +struct drm_master *drm_master_get(struct drm_master *master); > > +void drm_master_put(struct drm_master **master); > > +bool drm_is_current_master(struct drm_file *fpriv); > > + > > +#endif > > diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h > > index a5ef2c7e40f8..cf0e7d89bcdf 100644 > > --- a/include/drm/drm_legacy.h > > +++ b/include/drm/drm_legacy.h > > @@ -1,6 +1,8 @@ > > #ifndef __DRM_DRM_LEGACY_H__ > > #define __DRM_DRM_LEGACY_H__ > > > > +#include <drm/drm_auth.h> > > + > > /* > > * Legacy driver interfaces for the Direct Rendering Manager > > * > > -- > > 2.8.1 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 20 June 2016 at 22:17, Daniel Vetter <daniel@ffwll.ch> wrote: > On Sat, Jun 18, 2016 at 12:46:38AM +0100, Emil Velikov wrote: >> On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> > @@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) >> > return ret < 0 ? ret : 0; >> > } >> > >> > -/** >> > - * drm_authmagic - Authenticate client with a magic >> > - * @dev: DRM device to operate on >> > - * @data: ioctl data containing the drm_auth object >> > - * @file_priv: DRM file that performs the operation >> > - * >> > - * This looks up a DRM client by the passed magic and authenticates it. >> > - * >> > - * Returns: 0 on success, negative error code on failure. >> > - */ >> Why is this and drm_getmagic()'s documetation going away ? Kernel doc >> isn't restricted to EXPORTED_SYMBOL(s) only, is it ? > > No, but imo the documentation for the drm core&helpers should be aimed at > driver writers. And driver writers can only use EXPORT_SYMBOL stuff (or > from headers), which is why I think it's good to kick out everything else > to avoid too much clutter. It's already a daunting thing to get started > with a new drm driver. > > Of course we can keep the comments as normal comments, but for these here > I didn't see the value. > > Also note that this is just for the drm core/helpers. In the i915 driver > documentation we instead try to document non-static stuff (since that's > exposed to other parts), but just as a rough guideline. Since often our > source file split doesn't make that much sense, or is too monolithic. > > In both cases the goal is to give a useful guideline to users of a piece > of code (calling it or otherwise using), not developers changing the > implementation details themselves. Indeed... Having this (and similar internal/implementation details) as the kernel doc will bring volume about something that most devs cannot and should not care about, and is likely confuse them. If at all, one could keep them as normal comments. Don't feel to strongly about them, just wanted to educate myself a bit on the topic (dos and don'ts) Thanks a million ! Emil
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 94c6bdee8080..b7f6316b7bee 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -3103,6 +3103,12 @@ int num_ioctls;</synopsis> !Pdrivers/gpu/drm/drm_vma_manager.c getunique and setversion story </sect1> <sect1> + <title>Primary Nodes, DRM Master and Authentication</title> +!Pdrivers/gpu/drm_auth.c master and authentication +!Edrivers/gpu/drm_auth.c +!Einclude/drm/drm_auth.h + </sect1> + <sect1> <title>Render nodes</title> <para> DRM core provides multiple character-devices for user-space to use. diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index b4dfa8ab20d7..3774b9964dbe 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -32,18 +32,27 @@ #include "drm_internal.h" #include "drm_legacy.h" -/** - * drm_getmagic - Get unique magic of a client - * @dev: DRM device to operate on - * @data: ioctl data containing the drm_auth object - * @file_priv: DRM file that performs the operation +/** DOC: master and authentication + * + * struct &drm_master is used to track groups of clients with open + * primary/legacy device nodes. For every struct &drm_file which at least once + * successfully became the device master (either through the SET_MASTER IOCTL, + * or implicitly through opening the primary device node when no one else is the + * current master that time) there exists one &drm_master. This is noted in the + * is_master member of &drm_master. All other clients have just a pointer to the + * &drm_master they are associated with. * - * This looks up the unique magic of the passed client and returns it. If the - * client did not have a magic assigned, yet, a new one is registered. The magic - * is stored in the passed drm_auth object. + * In addition only one &drm_master can be the current master for a &drm_device. + * It can be switched through the DROP_MASTER and SET_MASTER IOCTL, or + * implicitly through closing/openeing the primary device node. See also + * drm_is_current_master(). * - * Returns: 0 on success, negative error code on failure. + * Clients can authenticate against the current master (if it matches their own) + * using the GETMAGIC and AUTHMAGIC IOCTLs. Together with exchanging masters, + * this allows controlled access to the device for an entire group of mutually + * trusted clients. */ + int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_auth *auth = data; @@ -64,16 +73,6 @@ int drm_getmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) return ret < 0 ? ret : 0; } -/** - * drm_authmagic - Authenticate client with a magic - * @dev: DRM device to operate on - * @data: ioctl data containing the drm_auth object - * @file_priv: DRM file that performs the operation - * - * This looks up a DRM client by the passed magic and authenticates it. - * - * Returns: 0 on success, negative error code on failure. - */ int drm_authmagic(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -126,16 +125,6 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv, return ret; } -/* - * drm_new_set_master - Allocate a new master object and become master for the - * associated master realm. - * - * @dev: The associated device. - * @fpriv: File private identifying the client. - * - * This function must be called with dev::master_mutex held. - * Returns negative error code on failure. Zero on success. - */ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) { struct drm_master *old_master; @@ -288,12 +277,28 @@ out: mutex_unlock(&dev->master_mutex); } +/** + * drm_is_current_master - checks whether this master is the current one + * @fpriv: DRM file private + * + * Checks whether @fpriv is a master and that it is the current master on its + * device. This decides whether a client is allowed to run DRM_MASTER IOCTLs. + * + * Most of the modern IOCTL which require DRM_MASTER are for kernel modesetting + * - the current master is assumed to own the non-shareable display hardware. + */ bool drm_is_current_master(struct drm_file *fpriv) { return fpriv->is_master && fpriv->master == fpriv->minor->dev->master; } EXPORT_SYMBOL(drm_is_current_master); +/** + * drm_master_get - reference a master pointer + * @master: struct &drm_master + * + * Increments the reference count of @master. + */ struct drm_master *drm_master_get(struct drm_master *master) { kref_get(&master->refcount); @@ -316,6 +321,12 @@ static void drm_master_destroy(struct kref *kref) kfree(master); } +/** + * drm_master_put - unreference and clear a master pointer + * @master: pointer to a pointer of struct &drm_master + * + * This decrements the &drm_master behind @master and sets it to NULL. + */ void drm_master_put(struct drm_master **master) { kref_put(&(*master)->refcount, drm_master_destroy); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 81083f98d155..871af372662d 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -39,6 +39,7 @@ #include <drm/drm_fourcc.h> #include <drm/drm_modeset_lock.h> #include <drm/drm_atomic.h> +#include <drm/drm_auth.h> #include "drm_crtc_internal.h" #include "drm_internal.h" diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index a0c1d172954d..88796a383e40 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -30,6 +30,7 @@ #include <drm/drmP.h> #include <drm/drm_core.h> +#include <drm/drm_auth.h> #include "drm_legacy.h" #include "drm_internal.h" #include "drm_crtc_internal.h" diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9fa9698fe247..0f8632c93e95 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -47,6 +47,7 @@ #include <drm/intel-gtt.h> #include <drm/drm_legacy.h> /* for struct drm_dma_handle */ #include <drm/drm_gem.h> +#include <drm/drm_auth.h> #include "i915_params.h" #include "i915_reg.h" diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 1980e2a28265..9a90f824814e 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -32,6 +32,7 @@ #include <drm/drmP.h> #include <drm/vmwgfx_drm.h> #include <drm/drm_hashtab.h> +#include <drm/drm_auth.h> #include <linux/suspend.h> #include <drm/ttm/ttm_bo_driver.h> #include <drm/ttm/ttm_object.h> diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 761b20332321..d22ba6bf2299 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -86,6 +86,7 @@ struct drm_local_map; struct drm_device_dma; struct drm_dma_handle; struct drm_gem_object; +struct drm_master; struct device_node; struct videomode; @@ -373,30 +374,6 @@ struct drm_lock_data { int idle_has_lock; }; -/** - * struct drm_master - drm master structure - * - * @refcount: Refcount for this master object. - * @dev: Link back to the DRM device - * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex. - * @unique_len: Length of unique field. Protected by drm_global_mutex. - * @magic_map: Map of used authentication tokens. Protected by struct_mutex. - * @lock: DRI lock information. - * @driver_priv: Pointer to driver-private information. - * - * Note that master structures are only relevant for the legacy/primary device - * nodes, hence there can only be one per device, not one per drm_minor. - */ -struct drm_master { - struct kref refcount; - struct drm_device *dev; - char *unique; - int unique_len; - struct idr magic_map; - struct drm_lock_data lock; - void *driver_priv; -}; - /* Flags and return codes for get_vblank_timestamp() driver function. */ #define DRM_CALLED_FROM_VBLIRQ 1 #define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0) @@ -1008,11 +985,6 @@ static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc return &crtc->dev->vblank[drm_crtc_index(crtc)].queue; } -/* drm_auth.c */ -struct drm_master *drm_master_get(struct drm_master *master); -void drm_master_put(struct drm_master **master); -bool drm_is_current_master(struct drm_file *fpriv); - /* drm_drv.c */ void drm_put_dev(struct drm_device *dev); void drm_unplug_dev(struct drm_device *dev); diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h new file mode 100644 index 000000000000..610223b0481b --- /dev/null +++ b/include/drm/drm_auth.h @@ -0,0 +1,59 @@ +/* + * Internal Header for the Direct Rendering Manager + * + * Copyright 2016 Intel Corporation + * + * Author: Daniel Vetter <daniel.vetter@ffwll.ch> + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +#ifndef _DRM_AUTH_H_ +#define _DRM_AUTH_H_ + +/** + * struct drm_master - drm master structure + * + * @refcount: Refcount for this master object. + * @dev: Link back to the DRM device + * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex. + * @unique_len: Length of unique field. Protected by drm_global_mutex. + * @magic_map: Map of used authentication tokens. Protected by struct_mutex. + * @lock: DRI lock information. + * @driver_priv: Pointer to driver-private information. + * + * Note that master structures are only relevant for the legacy/primary device + * nodes, hence there can only be one per device, not one per drm_minor. + */ +struct drm_master { + struct kref refcount; + struct drm_device *dev; + char *unique; + int unique_len; + struct idr magic_map; + struct drm_lock_data lock; + void *driver_priv; +}; + +struct drm_master *drm_master_get(struct drm_master *master); +void drm_master_put(struct drm_master **master); +bool drm_is_current_master(struct drm_file *fpriv); + +#endif diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index a5ef2c7e40f8..cf0e7d89bcdf 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -1,6 +1,8 @@ #ifndef __DRM_DRM_LEGACY_H__ #define __DRM_DRM_LEGACY_H__ +#include <drm/drm_auth.h> + /* * Legacy driver interfaces for the Direct Rendering Manager *
Also extract drm_auth.h for nicer grouping. v2: Nuke the other comments since they don't really explain a lot, and within the drm core we generally only document functions exported to drivers: The main audience for these docs are driver writers. v3: Limit the exposure of drm_master internals by only including drm_auth.h where it is neede (Chris). Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/DocBook/gpu.tmpl | 6 ++++ drivers/gpu/drm/drm_auth.c | 69 +++++++++++++++++++++---------------- drivers/gpu/drm/drm_crtc.c | 1 + drivers/gpu/drm/drm_ioctl.c | 1 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 1 + include/drm/drmP.h | 30 +--------------- include/drm/drm_auth.h | 59 +++++++++++++++++++++++++++++++ include/drm/drm_legacy.h | 2 ++ 9 files changed, 112 insertions(+), 58 deletions(-) create mode 100644 include/drm/drm_auth.h