| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160415081808.GU2510@phenom.ffwll.local>
Date: Fri, 15 Apr 2016 10:18:08 +0200
From: Daniel Vetter <daniel@...ll.ch>
To: Gustavo Padovan <gustavo@...ovan.org>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Daniel Stone <daniels@...labora.com>,
Arve Hjønnevåg <arve@...roid.com>,
Riley Andrews <riandrews@...roid.com>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Rob Clark <robdclark@...il.com>,
Greg Hackmann <ghackmann@...gle.com>,
John Harrison <John.C.Harrison@...el.com>,
laurent.pinchart@...asonboard.com, seanpaul@...gle.com,
marcheu@...gle.com, m.chehab@...sung.com,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Gustavo Padovan <gustavo.padovan@...labora.co.uk>
Subject: Re: [RFC 8/8] drm/fence: add out-fences support
On Thu, Apr 14, 2016 at 06:29:41PM -0700, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
>
> Support DRM out-fences creating a sync_file with a fence for each crtc
> update with the DRM_MODE_ATOMIC_OUT_FENCE flag.
>
> We then send an struct drm_out_fences array with the out-fences fds back in
> the drm_atomic_ioctl() as an out arg in the out_fences_ptr field.
>
> struct drm_out_fences {
> __u32 crtc_id;
> __u32 fd;
> };
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@...labora.co.uk>
> ---
> drivers/gpu/drm/drm_atomic.c | 109 +++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/drm_atomic_helper.c | 1 +
> include/drm/drm_crtc.h | 3 +
> include/uapi/drm/drm_mode.h | 7 +++
> 4 files changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 0b95526..af6e051 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1560,6 +1560,103 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>
> +static int drm_atomic_get_out_fences(struct drm_device *dev,
> + struct drm_atomic_state *state,
> + uint32_t __user *out_fences_ptr,
> + uint64_t count_out_fences,
> + uint64_t user_data)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> + struct drm_out_fences *out_fences;
> + struct sync_file **sync_file;
> + int num_fences = 0;
> + int i, ret;
> +
> + out_fences = kcalloc(count_out_fences, sizeof(*out_fences),
> + GFP_KERNEL);
> + if (!out_fences)
> + return -ENOMEM;
> +
> + sync_file = kcalloc(count_out_fences, sizeof(*sync_file),
> + GFP_KERNEL);
> + if (!sync_file) {
> + kfree(out_fences);
> + return -ENOMEM;
> + }
> +
> + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> + struct drm_pending_vblank_event *e;
> + struct fence *fence;
> + char name[32];
> + int fd;
> +
> + fence = sync_timeline_create_fence(crtc->timeline,
> + crtc->fence_seqno);
> + if (!fence) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + snprintf(name, sizeof(name), "crtc-%d_%lu",
> + drm_crtc_index(crtc), crtc->fence_seqno++);
> +
> + sync_file[i] = sync_file_create(name, fence);
> + if(!sync_file[i]) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + fd = get_unused_fd_flags(O_CLOEXEC);
> + if (fd < 0) {
> + ret = fd;
> + goto out;
> + }
> +
> + sync_file_install(sync_file[i], fd);
> +
> + if (crtc_state->event) {
> + crtc_state->event->base.fence = fence;
> + } else {
> + e = create_vblank_event(dev, NULL, fence, user_data);
> + if (!e) {
> + put_unused_fd(fd);
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + crtc_state->event = e;
> + }
> + if (num_fences > count_out_fences) {
> + put_unused_fd(fd);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + fence_get(fence);
> +
> + out_fences[num_fences].crtc_id = crtc->base.id;
> + out_fences[num_fences].fd = fd;
> + num_fences++;
> + }
> +
> + if (copy_to_user(out_fences_ptr, out_fences,
> + num_fences * sizeof(*out_fences))) {
> + ret = -EFAULT;
> + goto out;
> + }
> +
> + return 0;
> +
> +out:
> + for (i = 0 ; i < count_out_fences ; i++) {
> + if (sync_file[i])
> + sync_file_put(sync_file[i]);
> + }
> +
> + return ret;
> +}
> +
> int drm_mode_atomic_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
> @@ -1568,6 +1665,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> uint32_t __user *count_props_ptr = (uint32_t __user *)(unsigned long)(arg->count_props_ptr);
> uint32_t __user *props_ptr = (uint32_t __user *)(unsigned long)(arg->props_ptr);
> uint64_t __user *prop_values_ptr = (uint64_t __user *)(unsigned long)(arg->prop_values_ptr);
> + uint32_t __user *out_fences_ptr = (uint32_t __user *)(unsigned long)(arg->out_fences_ptr);
> unsigned int copied_objs, copied_props;
> struct drm_atomic_state *state;
> struct drm_modeset_acquire_ctx ctx;
> @@ -1601,7 +1699,8 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>
> /* can't test and expect an event at the same time. */
> if ((arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) &&
> - (arg->flags & DRM_MODE_PAGE_FLIP_EVENT))
> + (arg->flags & (DRM_MODE_PAGE_FLIP_EVENT
> + | DRM_MODE_ATOMIC_OUT_FENCE)))
> return -EINVAL;
>
> drm_modeset_acquire_init(&ctx, 0);
> @@ -1693,6 +1792,14 @@ retry:
> }
> }
>
> + if (arg->flags & DRM_MODE_ATOMIC_OUT_FENCE) {
OUT_FENCE and TEST_ONLY probably don't make sense, and need to be
rejected. Needs a testcase, too.
> + ret = drm_atomic_get_out_fences(dev, state, out_fences_ptr,
> + arg->count_out_fences,
> + arg->user_data);
> + if (ret < 0)
> + goto out;
> + }
> +
> if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
If anything fails below this point we need to clean up the sync_file/fd
mess. Might be easier to first create sync_file objects only, and only
install the fd once atomic has succeeded. You probably want to reserve the
fd slots beforehand though.
That means a bunch more per-crtc state in drm_atomic_state. We should
probably take all the per-crtc pointers and throw them into a small
struct, to avoid allocating individual arrays for everything. So
struct drm_atomic_state_per_crtc {
struct drm_crtc *crtc;
struct drm_crtc_state *state;
struct sync_file *sync_file;
int fd;
};
Sorry if this means a bit a sprawling upfront refactor :(
Wrt testcase: Need evil ones that pass invalid pointer to out_fences_ptr
or too small array size in count_out_fences and all that ofc.
-Daniel
> /*
> * Unlike commit, check_only does not clean up state.
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 6ed8339..15ba3a8 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -31,6 +31,7 @@
> #include <drm/drm_crtc_helper.h>
> #include <drm/drm_atomic_helper.h>
> #include <linux/fence.h>
> +#include <linux/sync_file.h>
>
> /**
> * DOC: overview
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 7934178..53e4e71 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -717,6 +717,7 @@ struct drm_crtc_funcs {
> * @properties: property tracking for this CRTC
> * @state: current atomic state for this CRTC
> * @timeline: sync timeline for fence sigalling
> + * @fence_seqno: seqno variable to create fences
> * @acquire_ctx: per-CRTC implicit acquire context used by atomic drivers for
> * legacy IOCTLs
> *
> @@ -773,7 +774,9 @@ struct drm_crtc {
>
> struct drm_crtc_state *state;
>
> + /* for out-fences */
> struct sync_timeline *timeline;
> + unsigned long fence_seqno;
>
> /*
> * For legacy crtc IOCTLs so that atomic drivers can get at the locking
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 39905cc..4cdcd22 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -592,6 +592,11 @@ struct drm_mode_destroy_dumb {
> DRM_MODE_ATOMIC_ALLOW_MODESET |\
> DRM_MODE_ATOMIC_OUT_FENCE)
>
> +struct drm_out_fences {
> + __u32 crtc_id;
> + __u32 fd;
> +};
> +
> struct drm_mode_atomic {
> __u32 flags;
> __u32 count_objs;
> @@ -601,6 +606,8 @@ struct drm_mode_atomic {
> __u64 prop_values_ptr;
> __u64 reserved;
> __u64 user_data;
> + __u64 count_out_fences;
> + __u64 out_fences_ptr;
> };
>
> /**
> --
> 2.5.5
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists