Re: [PATCH 16/16] drm: document drm_auth.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17 June 2016 at 08:33, Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> ---
>  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@xxxxxxxx>
> + *
> + * 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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux