[PATCH RFC] drm: Make the drm_vma_manager authentication mechanism a bit more versatile

[David Herrmann]

Hi

On Wed, Mar 19, 2014 at 12:33 PM, Thomas Hellstrom
<thellstrom at vmware.com> wrote:
> By handing the authentication mechanism tokens in the form of const void
> pointers instead of struct file pointers, drivers are free to provide whatever
> file pointers they find appropriate. The pointers are never dereferenced in
> the drm_vma_manager so that shouldn't be a problem.
>
> As an example, the vmwgfx driver would like to provide struct ttm_object_file
> pointers.

I guess a "struct file*" pointer in tmm_object_file would be easier,
but if that reverse dependency is not wanted, I'm fine with that. Just
make sure you don't use drm_vma_node_verify_access for TTM directly as
TTM passes in a file pointer.

Reviewed-by: David Herrmann <dh.herrmann at gmail.com>

Thanks
David

> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
> ---
>  drivers/gpu/drm/drm_vma_manager.c |   22 +++++++++++-----------
>  include/drm/drm_vma_manager.h     |   12 ++++++------
>  2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
> index 63b4712..93676c1 100644
> --- a/drivers/gpu/drm/drm_vma_manager.c
> +++ b/drivers/gpu/drm/drm_vma_manager.c
> @@ -291,24 +291,24 @@ EXPORT_SYMBOL(drm_vma_offset_remove);
>  /**
>   * drm_vma_node_allow - Add open-file to list of allowed users
>   * @node: Node to modify
> - * @filp: Open file to add
> + * @filp: Authentication token (typically pointer to open file) to add
>   *
> - * Add @filp to the list of allowed open-files for this node. If @filp is
> - * already on this list, the ref-count is incremented.
> + * Add @filp to the list of allowed authentication tokens for this node.
> + * If @filp is already on this list, the ref-count is incremented.
>   *
>   * The list of allowed-users is preserved across drm_vma_offset_add() and
>   * drm_vma_offset_remove() calls. You may even call it if the node is currently
>   * not added to any offset-manager.
>   *
> - * You must remove all open-files the same number of times as you added them
> - * before destroying the node. Otherwise, you will leak memory.
> + * You must remove all authentication tokens the same number of times as you
> + * added them before destroying the node. Otherwise, you will leak memory.
>   *
>   * This is locked against concurrent access internally.
>   *
>   * RETURNS:
>   * 0 on success, negative error code on internal failure (out-of-mem)
>   */
> -int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp)
> +int drm_vma_node_allow(struct drm_vma_offset_node *node, const void *filp)
>  {
>         struct rb_node **iter;
>         struct rb_node *parent = NULL;
> @@ -360,7 +360,7 @@ EXPORT_SYMBOL(drm_vma_node_allow);
>  /**
>   * drm_vma_node_revoke - Remove open-file from list of allowed users
>   * @node: Node to modify
> - * @filp: Open file to remove
> + * @filp: Authentication token to remove
>   *
>   * Decrement the ref-count of @filp in the list of allowed open-files on @node.
>   * If the ref-count drops to zero, remove @filp from the list. You must call
> @@ -370,7 +370,7 @@ EXPORT_SYMBOL(drm_vma_node_allow);
>   *
>   * If @filp is not on the list, nothing is done.
>   */
> -void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp)
> +void drm_vma_node_revoke(struct drm_vma_offset_node *node, const void *filp)
>  {
>         struct drm_vma_offset_file *entry;
>         struct rb_node *iter;
> @@ -400,10 +400,10 @@ EXPORT_SYMBOL(drm_vma_node_revoke);
>  /**
>   * drm_vma_node_is_allowed - Check whether an open-file is granted access
>   * @node: Node to check
> - * @filp: Open-file to check for
> + * @filp: Authentication token to check for
>   *
>   * Search the list in @node whether @filp is currently on the list of allowed
> - * open-files (see drm_vma_node_allow()).
> + * authentication tokens (see drm_vma_node_allow()).
>   *
>   * This is locked against concurrent access internally.
>   *
> @@ -411,7 +411,7 @@ EXPORT_SYMBOL(drm_vma_node_revoke);
>   * true iff @filp is on the list
>   */
>  bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
> -                            struct file *filp)
> +                            const void *filp)
>  {
>         struct drm_vma_offset_file *entry;
>         struct rb_node *iter;
> diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
> index c18a593..d7ff1e5 100644
> --- a/include/drm/drm_vma_manager.h
> +++ b/include/drm/drm_vma_manager.h
> @@ -33,7 +33,7 @@
>
>  struct drm_vma_offset_file {
>         struct rb_node vm_rb;
> -       struct file *vm_filp;
> +       const void *vm_filp;
>         unsigned long vm_count;
>  };
>
> @@ -65,10 +65,10 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
>  void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
>                            struct drm_vma_offset_node *node);
>
> -int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp);
> -void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp);
> +int drm_vma_node_allow(struct drm_vma_offset_node *node, const void *filp);
> +void drm_vma_node_revoke(struct drm_vma_offset_node *node, const void *filp);
>  bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
> -                            struct file *filp);
> +                            const void *filp);
>
>  /**
>   * drm_vma_offset_exact_lookup() - Look up node by exact address
> @@ -239,7 +239,7 @@ static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
>  /**
>   * drm_vma_node_verify_access() - Access verification helper for TTM
>   * @node: Offset node
> - * @filp: Open-file
> + * @filp: Authentication token. Typically pointer to open file
>   *
>   * This checks whether @filp is granted access to @node. It is the same as
>   * drm_vma_node_is_allowed() but suitable as drop-in helper for TTM
> @@ -249,7 +249,7 @@ static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
>   * 0 if access is granted, -EACCES otherwise.
>   */
>  static inline int drm_vma_node_verify_access(struct drm_vma_offset_node *node,
> -                                            struct file *filp)
> +                                            const void *filp)
>  {
>         return drm_vma_node_is_allowed(node, filp) ? 0 : -EACCES;
>  }
> --
> 1.7.10.4