| Message ID | 1395228825-28908-1-git-send-email-thellstrom@vmware.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Wed, Mar 19, 2014 at 12:33 PM, Thomas Hellstrom <thellstrom@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@gmail.com> Thanks David > Signed-off-by: Thomas Hellstrom <thellstrom@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
On 03/19/2014 12:45 PM, David Herrmann wrote: > Hi > > On Wed, Mar 19, 2014 at 12:33 PM, Thomas Hellstrom > <thellstrom@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@gmail.com> > > Thanks > David Thanks for reviewing, David. Although it turned out that I won't be needing this, ATM since the TTM object mechanism already has a per file object-to-handle mapping that I could reuse for this. Thanks, Thomas
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; }
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. Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> --- drivers/gpu/drm/drm_vma_manager.c | 22 +++++++++++----------- include/drm/drm_vma_manager.h | 12 ++++++------ 2 files changed, 17 insertions(+), 17 deletions(-)