On Tue, Apr 04, 2017 at 02:27:26PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@xxxxxxxxxx>
>
> This allows us to create sync files with different semantics,
> and clearly define the interoperation between them it also
> provides flags to allow for tweaks on those semantics.
>
> This provides a validation interface for drivers that accept
> types from userspace so they can return EINVAL instead of ENOMEM.
>
> This provides an ioctl for userspace to retrieve the type/flags
> of an object it may recieve from somewhere else.
>
> Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
Since you've bothered to type the docs for the ioctl structs too, please
squash in the below (and double-check that kernel-doc is still happy):
diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
index 31671b469627..375848c590ce 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -163,3 +163,8 @@ DMA Fence uABI/Sync File
.. kernel-doc:: include/linux/sync_file.h
:internal:
+Sync File IOCTL Definitions
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/uapi/linux/sync_file.h
+ :internal:
With that: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
One open question: Do we expect the current sync_file_get_fence to only
ever work for a FENCE type sync_file? Might be good to clarify that in the
kernel-doc for sync_file_get_fence().
-Daniel
> ---
> drivers/dma-buf/sw_sync.c | 2 +-
> drivers/dma-buf/sync_file.c | 62 +++++++++++++++++++++++++++++++++++++++---
> drivers/gpu/drm/drm_atomic.c | 2 +-
> include/linux/sync_file.h | 9 +++++-
> include/uapi/linux/sync_file.h | 27 ++++++++++++++++++
> 5 files changed, 95 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 69c5ff3..1c47de6 100644
> --- a/drivers/dma-buf/sw_sync.c
> +++ b/drivers/dma-buf/sw_sync.c
> @@ -315,7 +315,7 @@ static long sw_sync_ioctl_create_fence(struct sync_timeline *obj,
> goto err;
> }
>
> - sync_file = sync_file_create(&pt->base);
> + sync_file = sync_file_create(&pt->base, SYNC_FILE_TYPE_FENCE, 0);
> dma_fence_put(&pt->base);
> if (!sync_file) {
> err = -ENOMEM;
> diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
> index 2321035..b33af9d 100644
> --- a/drivers/dma-buf/sync_file.c
> +++ b/drivers/dma-buf/sync_file.c
> @@ -28,9 +28,32 @@
>
> static const struct file_operations sync_file_fops;
>
> -static struct sync_file *sync_file_alloc(void)
> +/**
> + * sync_file_validate_type_flags - validate type/flags for support
> + * @type: type of sync file object
> + * @flags: flags to sync object.
> + *
> + * Validates the flags are correct so userspace can get a more
> + * detailed error type.
> + */
> +int sync_file_validate_type_flags(uint32_t type, uint32_t flags)
> +{
> + if (flags)
> + return -EINVAL;
> + if (type != SYNC_FILE_TYPE_FENCE)
> + return -EINVAL;
> + return 0;
> +}
> +EXPORT_SYMBOL(sync_file_validate_type_flags);
> +
> +static struct sync_file *sync_file_alloc(uint32_t type, uint32_t flags)
> {
> struct sync_file *sync_file;
> + int ret;
> +
> + ret = sync_file_validate_type_flags(type, flags);
> + if (ret)
> + return NULL;
>
> sync_file = kzalloc(sizeof(*sync_file), GFP_KERNEL);
> if (!sync_file)
> @@ -47,6 +70,8 @@ static struct sync_file *sync_file_alloc(void)
>
> INIT_LIST_HEAD(&sync_file->cb.node);
>
> + sync_file->type = type;
> + sync_file->flags = flags;
> return sync_file;
>
> err:
> @@ -72,11 +97,13 @@ static void fence_check_cb_func(struct dma_fence *f, struct dma_fence_cb *cb)
> * sync_file can be released with fput(sync_file->file). Returns the
> * sync_file or NULL in case of error.
> */
> -struct sync_file *sync_file_create(struct dma_fence *fence)
> +struct sync_file *sync_file_create(struct dma_fence *fence,
> + uint32_t type,
> + uint32_t flags)
> {
> struct sync_file *sync_file;
>
> - sync_file = sync_file_alloc();
> + sync_file = sync_file_alloc(type, flags);
> if (!sync_file)
> return NULL;
>
> @@ -200,7 +227,10 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,
> struct dma_fence **fences, **nfences, **a_fences, **b_fences;
> int i, i_a, i_b, num_fences, a_num_fences, b_num_fences;
>
> - sync_file = sync_file_alloc();
> + if (a->type != b->type)
> + return NULL;
> +
> + sync_file = sync_file_alloc(a->type, a->flags);
> if (!sync_file)
> return NULL;
>
> @@ -437,6 +467,27 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
> return ret;
> }
>
> +static long sync_file_ioctl_type(struct sync_file *sync_file,
> + unsigned long arg)
> +{
> + struct sync_file_type type;
> + int ret;
> + if (copy_from_user(&type, (void __user *)arg, sizeof(type)))
> + return -EFAULT;
> +
> + if (type.flags || type.type)
> + return -EINVAL;
> +
> + type.type = sync_file->type;
> + type.flags = sync_file->flags;
> +
> + if (copy_to_user((void __user *)arg, &type, sizeof(type)))
> + ret = -EFAULT;
> + else
> + ret = 0;
> + return ret;
> +}
> +
> static long sync_file_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> @@ -449,6 +500,9 @@ static long sync_file_ioctl(struct file *file, unsigned int cmd,
> case SYNC_IOC_FILE_INFO:
> return sync_file_ioctl_fence_info(sync_file, arg);
>
> + case SYNC_IOC_TYPE:
> + return sync_file_ioctl_type(sync_file, arg);
> +
> default:
> return -ENOTTY;
> }
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index a567310..bb5a740 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1917,7 +1917,7 @@ static int setup_out_fence(struct drm_out_fence_state *fence_state,
> if (put_user(fence_state->fd, fence_state->out_fence_ptr))
> return -EFAULT;
>
> - fence_state->sync_file = sync_file_create(fence);
> + fence_state->sync_file = sync_file_create(fence, SYNC_FILE_TYPE_FENCE, 0);
> if (!fence_state->sync_file)
> return -ENOMEM;
>
> diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
> index 3e3ab84..ede4182 100644
> --- a/include/linux/sync_file.h
> +++ b/include/linux/sync_file.h
> @@ -20,6 +20,8 @@
> #include <linux/spinlock.h>
> #include <linux/dma-fence.h>
> #include <linux/dma-fence-array.h>
> +#include <uapi/linux/sync_file.h>
> +
>
> /**
> * struct sync_file - sync file to export to the userspace
> @@ -30,6 +32,8 @@
> * @wq: wait queue for fence signaling
> * @fence: fence with the fences in the sync_file
> * @cb: fence callback information
> + * @type: sync file type
> + * @flags: flags used to create sync file
> */
> struct sync_file {
> struct file *file;
> @@ -43,11 +47,14 @@ struct sync_file {
>
> struct dma_fence *fence;
> struct dma_fence_cb cb;
> + uint32_t type;
> + uint32_t flags;
> };
>
> #define POLL_ENABLED DMA_FENCE_FLAG_USER_BITS
>
> -struct sync_file *sync_file_create(struct dma_fence *fence);
> +int sync_file_validate_type_flags(uint32_t type, uint32_t flags);
> +struct sync_file *sync_file_create(struct dma_fence *fence, uint32_t type, uint32_t flags);
> struct dma_fence *sync_file_get_fence(int fd);
>
> #endif /* _LINUX_SYNC_H */
> diff --git a/include/uapi/linux/sync_file.h b/include/uapi/linux/sync_file.h
> index 5b287d6..f439cda 100644
> --- a/include/uapi/linux/sync_file.h
> +++ b/include/uapi/linux/sync_file.h
> @@ -69,6 +69,26 @@ struct sync_file_info {
> #define SYNC_IOC_MAGIC '>'
>
> /**
> + * DOC: SYNC_FILE_TYPE_FENCE - fence sync file object
> + *
> + * This sync file is a wrapper around a dma fence or a dma fence array.
> + * It can be merged with another fence sync file object to create a new
> + * merged object.
> + * The fence backing this object cannot be replaced.
> + * This is useful for shared fences.
> + */
> +#define SYNC_FILE_TYPE_FENCE 0
> +
> +/**
> + * struct sync_file_type - data returned from sync file type ioctl
> + * @type: sync_file type
> + * @flags: sync_file creation flags
> + */
> +struct sync_file_type {
> + __u32 type;
> + __u32 flags;
> +};
> +/**
> * Opcodes 0, 1 and 2 were burned during a API change to avoid users of the
> * old API to get weird errors when trying to handling sync_files. The API
> * change happened during the de-stage of the Sync Framework when there was
> @@ -94,4 +114,11 @@ struct sync_file_info {
> */
> #define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 4, struct sync_file_info)
>
> +/**
> + * DOC: SYNC_IOC_TYPE - get creation type and flags of sync_file.
> + *
> + * Takes a struct sync_file_type. Returns the created values of type and flags.
> + */
> +#define SYNC_IOC_TYPE _IOWR(SYNC_IOC_MAGIC, 5, struct sync_file_type)
> +
> #endif /* _UAPI_LINUX_SYNC_H */
> --
> 2.9.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
