[PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
Sean Paul
seanpaul at chromium.org
Mon Oct 16 21:03:15 UTC 2017
On Thu, Oct 12, 2017 at 06:56:31PM -0700, Keith Packard wrote:
> drm_mode_create_lease
>
> Creates a lease for a list of drm mode objects, returning an
> fd for the new drm_master and a 64-bit identifier for the lessee
>
> drm_mode_list_lesees
>
> List the identifiers of the lessees for a master file
>
> drm_mode_get_lease
>
> List the leased objects for a master file
>
> drm_mode_revoke_lease
>
> Erase the set of objects managed by a lease.
>
> This should suffice to at least create and query leases.
>
> Changes for v2 as suggested by Daniel Vetter <daniel.vetter at ffwll.ch>:
>
> * query ioctls only query the master associated with
> the provided file.
>
> * 'mask_lease' value has been removed
>
> * change ioctl has been removed.
>
> Changes for v3 suggested in part by Dave Airlie <airlied at gmail.com>
>
> * Add revoke ioctl.
>
> Changes for v4 suggested by Dave Airlie <airlied at gmail.com>
>
> * Expand on the comment about the magic use of &drm_lease_idr_object
> * Pad lease ioctl structures to align on 64-bit boundaries
>
> Changes for v5 suggested by Dave Airlie <airlied at gmail.com>
>
> * Check for non-negative object_id in create_lease to avoid debug
> output from the kernel.
>
> Changes for v5 provided by Dave Airlie <airlied at gmail.com>
>
> * For non-universal planes add primary/cursor planes to lease
>
> If we aren't exposing universal planes to this userspace client,
> and it requests a lease on a crtc, we should implicitly export the
> primary and cursor planes for the crtc.
>
> If the lessee doesn't request universal planes, it will just see
> the crtc, but if it does request them it will then see the plane
> objects as well.
>
> This also moves the object look ups earlier as a side effect, so
> we'd exit the ioctl quicker for non-existant objects.
>
> * Restrict leases to crtc/connector/planes.
>
> This only allows leasing for objects we wish to allow.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> drivers/gpu/drm/drm_ioctl.c | 4 +
> drivers/gpu/drm/drm_lease.c | 318 ++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_mode_object.c | 5 +-
> include/drm/drm_lease.h | 12 ++
> include/drm/drm_mode_object.h | 2 +
> include/uapi/drm/drm.h | 5 +
> include/uapi/drm/drm_mode.h | 66 ++++++++
> 7 files changed, 410 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 9c435a4c0c82..4aafe4802099 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> };
>
> #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index 6c3f2445254c..88c213f9c4ab 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -23,6 +23,8 @@
> #define drm_for_each_lessee(lessee, lessor) \
> list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
>
> +static uint64_t drm_lease_idr_object;
> +
> /**
> * drm_lease_owner - return ancestor owner drm_master
> * @master: drm_master somewhere within tree of lessees and lessors
> @@ -337,3 +339,319 @@ void _drm_lease_revoke(struct drm_master *top)
> }
> }
> }
> +
> +/**
> + * drm_mode_create_lease_ioctl - create a new lease
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_create_lease
> + * @file_priv: the file being manipulated
> + *
> + * The master associated with the specified file will have a lease
> + * created containing the objects specified in the ioctl structure.
> + * A file descriptor will be allocated for that and returned to the
> + * application.
> + */
> +int drm_mode_create_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessor_priv)
> +{
> + struct drm_mode_create_lease *cl = data;
> + size_t object_count;
> + size_t o;
> + int ret = 0;
> + struct idr leases;
> + struct drm_master *lessor = lessor_priv->master;
> + struct drm_master *lessee = NULL;
> + struct file *lessee_file = NULL;
> + struct file *lessor_file = lessor_priv->filp;
> + struct drm_file *lessee_priv;
> + int fd = -1;
> +
> + /* Do not allow sub-leases */
> + if (lessor->lessor)
> + return -EINVAL;
> +
> + object_count = cl->object_count;
> + idr_init(&leases);
> +
> + /* Allocate a file descriptor for the lease */
> + fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK));
> +
> + DRM_DEBUG_LEASE("Creating new lease\n");
> +
> + /* Lookup the mode objects and add their IDs to the lease request */
> + for (o = 0; o < object_count; o++) {
> + __u32 object_id;
> + struct drm_mode_object *obj;
> +
> + if (copy_from_user(&object_id,
> + u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32),
> + sizeof (__u32))) {
> + ret = -EFAULT;
> + goto out_leases;
> + }
> + DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
> +
> + if ((int) object_id < 0) {
> + ret = -EINVAL;
> + goto out_leases;
> + }
> +
> + obj = drm_mode_object_find(dev, lessor_priv, object_id,
> + DRM_MODE_OBJECT_ANY);
> + if (!obj) {
> + ret = -ENOENT;
> + goto out_leases;
> + }
> +
> + /* only allow leasing on crtc/plane/connector objects */
> + if (!drm_mode_object_lease_required(obj->type)) {
> + ret = -EINVAL;
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> +
> + /*
> + * We're using an IDR to hold the set of leased
> + * objects, but we don't need to point at the object's
> + * data structure from the lease as the main crtc_idr
> + * will be used to actually find that. Instead, all we
> + * really want is a 'leased/not-leased' result, for
> + * which any non-NULL pointer will work fine.
> + */
> + ret = idr_alloc(&leases, &drm_lease_idr_object , object_id, object_id + 1, GFP_KERNEL);
nit: space before ,
> + if (ret < 0) {
> + DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
> + object_id, ret);
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> + if (obj->type == DRM_MODE_OBJECT_CRTC && !lessor_priv->universal_planes) {
> + struct drm_crtc *crtc = obj_to_crtc(obj);
> + ret = idr_alloc(&leases, &drm_lease_idr_object, crtc->primary->base.id, crtc->primary->base.id + 1, GFP_KERNEL);
> + if (ret < 0) {
> + DRM_DEBUG_LEASE("Object primary plane %d cannot be inserted into leases (%d)\n",
> + object_id, ret);
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> + if (crtc->cursor) {
> + ret = idr_alloc(&leases, &drm_lease_idr_object, crtc->cursor->base.id, crtc->cursor->base.id + 1, GFP_KERNEL);
> + if (ret < 0) {
> + DRM_DEBUG_LEASE("Object cursor plane %d cannot be inserted into leases (%d)\n",
> + object_id, ret);
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> + }
> + }
> + drm_mode_object_put(obj);
> + }
> +
> + mutex_lock(&dev->master_mutex);
> +
> + DRM_DEBUG_LEASE("Creating lease\n");
> + lessee = drm_lease_create(lessor, &leases);
> +
> + if (IS_ERR(lessee)) {
> + ret = PTR_ERR(lessee);
> + mutex_unlock(&dev->master_mutex);
> + goto out_leases;
> + }
> +
> + /* Clone the lessor file to create a new file for us */
> + DRM_DEBUG_LEASE("Allocating lease file\n");
> + path_get(&lessor_file->f_path);
Please forgive the stupid question, but where is this reference given up?
> + lessee_file = alloc_file(&lessor_file->f_path,
> + lessor_file->f_mode,
> + fops_get(lessor_file->f_inode->i_fop));
> + mutex_unlock(&dev->master_mutex);
> +
> + if (IS_ERR(lessee_file)) {
> + ret = PTR_ERR(lessee_file);
> + goto out_lessee;
> + }
> +
> + /* Initialize the new file for DRM */
> + DRM_DEBUG_LEASE("Initializing the file with %p\n", lessee_file->f_op->open);
> + ret = lessee_file->f_op->open(lessee_file->f_inode, lessee_file);
> + if (ret)
> + goto out_lessee_file;
> +
> + lessee_priv = lessee_file->private_data;
> +
> + /* Change the file to a master one */
> + drm_master_put(&lessee_priv->master);
> + lessee_priv->master = lessee;
> + lessee_priv->is_master = 1;
> + lessee_priv->authenticated = 1;
> +
> + /* Hook up the fd */
> + fd_install(fd, lessee_file);
> +
> + /* Pass fd back to userspace */
> + DRM_DEBUG_LEASE("Returning fd %d id %d\n", fd, lessee->lessee_id);
> + cl->fd = fd;
> + cl->lessee_id = lessee->lessee_id;
> +
> + DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n");
> + return 0;
> +
> +out_lessee_file:
> + fput(lessee_file);
> +
> +out_lessee:
> + drm_master_put(&lessee);
> +
> +out_leases:
> + idr_destroy(&leases);
> + put_unused_fd(fd);
> +
> + DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret);
> + return ret;
> +}
> +
> +/**
> + * drm_mode_list_lessees_ioctl - list lessee ids
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_list_lessees
> + * @lessor_priv: the file being manipulated
> + *
> + * Starting from the master associated with the specified file,
> + * the master with the provided lessee_id is found, and then
> + * an array of lessee ids associated with leases from that master
> + * are returned.
> + */
> +
> +int drm_mode_list_lessees_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessor_priv)
> +{
> + struct drm_mode_list_lessees *arg = data;
> + __u32 __user *lessee_ids = (__u32 __user *) (uintptr_t) (arg->lessees_ptr);
> + __u32 count_lessees = arg->count_lessees;
> + struct drm_master *lessor = lessor_priv->master, *lessee;
> + int count;
> + int ret = 0;
> +
> + DRM_DEBUG_LEASE("List lessees for %d\n", lessor->lessee_id);
> +
> + mutex_lock(&dev->master_mutex);
> +
> + count = 0;
> + drm_for_each_lessee(lessee, lessor) {
> + /* Only list un-revoked leases */
> + if (!idr_is_empty(&lessee->leases)) {
> + if (count_lessees > count) {
> + DRM_DEBUG_LEASE("Add lessee %d\n", lessee->lessee_id);
> + ret = put_user(lessee->lessee_id, lessee_ids + count);
> + if (ret)
> + break;
> + }
> + count++;
> + }
> + }
> +
> + DRM_DEBUG_LEASE("Lessor leases to %d\n", count);
> + if (ret == 0)
> + arg->count_lessees = count;
> +
> + mutex_unlock(&dev->master_mutex);
> +
> + return ret;
> +}
> +
> +/**
> + * drm_mode_get_lease_ioctl - list leased objects
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_get_lease
> + * @file_priv: the file being manipulated
> + *
> + * Return the list of leased objects for the specified lessee
> + */
> +
> +int drm_mode_get_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessee_priv)
> +{
> + struct drm_mode_get_lease *arg = data;
> + __u32 __user *object_ids = (__u32 __user *) (uintptr_t) (arg->objects_ptr);
> + __u32 count_objects = arg->count_objects;
> + struct drm_master *lessee = lessee_priv->master;
> + struct idr *object_idr;
> + int count;
> + void *entry;
> + int object;
> + int ret = 0;
> +
> + DRM_DEBUG_LEASE("get lease for %d\n", lessee->lessee_id);
> +
> + mutex_lock(&dev->master_mutex);
> +
> + if (lessee->lessor == NULL)
> + /* owner can use all objects */
> + object_idr = &lessee->dev->mode_config.crtc_idr;
What about other types of objects?
> + else
> + /* lessee can only use allowed object */
> + object_idr = &lessee->leases;
> +
> + count = 0;
> + idr_for_each_entry(object_idr, entry, object) {
> + if (count_objects > count) {
> + DRM_DEBUG_LEASE("adding object %d\n", object);
> + ret = put_user(object, object_ids + count);
> + if (ret)
> + break;
> + }
> + count++;
> + }
> +
> + DRM_DEBUG("lease holds %d objects\n", count);
> + if (ret == 0)
> + arg->count_objects = count;
> +
> + mutex_unlock(&dev->master_mutex);
> +
> + return ret;
> +}
> +
> +/**
> + * drm_mode_revoke_lease_ioctl - revoke lease
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_revoke_lease
> + * @file_priv: the file being manipulated
> + *
> + * This removes all of the objects from the lease without
> + * actually getting rid of the lease itself; that way all
> + * references to it still work correctly
> + */
> +int drm_mode_revoke_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessor_priv)
> +{
> + struct drm_mode_revoke_lease *arg = data;
> + struct drm_master *lessor = lessor_priv->master;
> + struct drm_master *lessee;
> + int ret = 0;
> +
> + DRM_DEBUG_LEASE("revoke lease for %d\n", arg->lessee_id);
> +
> + mutex_lock(&dev->master_mutex);
> +
> + lessee = _drm_find_lessee(lessor, arg->lessee_id);
> +
> + /* No such lessee */
> + if (!lessee) {
> + ret = -ENOENT;
> + goto fail;
> + }
> +
> + /* Lease is not held by lessor */
> + if (lessee->lessor != lessor) {
> + ret = -EACCES;
> + goto fail;
> + }
> +
> + _drm_lease_revoke(lessee);
> +
> +fail:
> + mutex_unlock(&dev->master_mutex);
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index d1599f36b605..7c8b2698c6a7 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -111,7 +111,7 @@ void drm_mode_object_unregister(struct drm_device *dev,
> * Returns whether the provided type of drm_mode_object must
> * be owned or leased to be used by a process.
> */
> -static bool drm_lease_required(uint32_t type)
> +bool drm_mode_object_lease_required(uint32_t type)
> {
> switch(type) {
> case DRM_MODE_OBJECT_CRTC:
> @@ -136,7 +136,8 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
> if (obj && obj->id != id)
> obj = NULL;
>
> - if (obj && drm_lease_required(obj->type) && !_drm_lease_held(file_priv, obj->id))
> + if (obj && drm_mode_object_lease_required(obj->type) &&
> + !_drm_lease_held(file_priv, obj->id))
> obj = NULL;
>
> if (obj && obj->free_cb) {
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> index 0981631b9aed..d2bcd1ea6cdc 100644
> --- a/include/drm/drm_lease.h
> +++ b/include/drm/drm_lease.h
> @@ -31,4 +31,16 @@ void _drm_lease_revoke(struct drm_master *master);
>
> uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs);
>
> +int drm_mode_create_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> +int drm_mode_list_lessees_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> +int drm_mode_get_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> +int drm_mode_revoke_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> #endif /* _DRM_LEASE_H_ */
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index c8155cb5a932..7ba3913f30b5 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -154,4 +154,6 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
> void drm_object_attach_property(struct drm_mode_object *obj,
> struct drm_property *property,
> uint64_t init_val);
> +
> +bool drm_mode_object_lease_required(uint32_t type);
> #endif
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index a106f6a7a0f9..9c02d2125d07 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -888,6 +888,11 @@ extern "C" {
> #define DRM_IOCTL_SYNCOBJ_RESET DRM_IOWR(0xC4, struct drm_syncobj_array)
> #define DRM_IOCTL_SYNCOBJ_SIGNAL DRM_IOWR(0xC5, struct drm_syncobj_array)
>
> +#define DRM_IOCTL_MODE_CREATE_LEASE DRM_IOWR(0xC6, struct drm_mode_create_lease)
> +#define DRM_IOCTL_MODE_LIST_LESSEES DRM_IOWR(0xC7, struct drm_mode_list_lessees)
> +#define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct drm_mode_get_lease)
> +#define DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct drm_mode_revoke_lease)
> +
> /**
> * Device specific ioctls should only be in their respective headers
> * The device specific ioctl range is from 0x40 to 0x9f.
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 34b6bb34b002..5597a87154e5 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -782,6 +782,72 @@ struct drm_mode_destroy_blob {
> __u32 blob_id;
> };
>
> +/**
> + * Lease mode resources, creating another drm_master.
> + */
> +struct drm_mode_create_lease {
> + /** Pointer to array of object ids (__u32) */
> + __u64 object_ids;
> + /** Number of object ids */
> + __u32 object_count;
> + /** flags for new FD (O_CLOEXEC, etc) */
> + __u32 flags;
> +
> + /** Return: unique identifier for lessee. */
> + __u32 lessee_id;
> + /** Return: file descriptor to new drm_master file */
> + __u32 fd;
> +};
> +
> +/**
> + * List lesses from a drm_master
> + */
> +struct drm_mode_list_lessees {
> + /** Number of lessees.
> + * On input, provides length of the array.
> + * On output, provides total number. No
> + * more than the input number will be written
> + * back, so two calls can be used to get
> + * the size and then the data.
> + */
> + __u32 count_lessees;
> + __u32 pad;
> +
> + /** Pointer to lessees.
> + * pointer to __u64 array of lessee ids
> + */
> + __u64 lessees_ptr;
> +};
> +
> +/**
> + * Get leased objects
> + */
> +struct drm_mode_get_lease {
> + /** Number of leased objects.
> + * On input, provides length of the array.
> + * On output, provides total number. No
> + * more than the input number will be written
> + * back, so two calls can be used to get
> + * the size and then the data.
> + */
> + __u32 count_objects;
> + __u32 pad;
> +
> + /** Pointer to objects.
> + * pointer to __u32 array of object ids
> + */
> + __u64 objects_ptr;
> +};
> +
> +/**
> + * Revoke lease
> + */
> +struct drm_mode_revoke_lease {
> + /** Unique ID of lessee
> + */
> + __u32 lessee_id;
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.15.0.rc0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
More information about the dri-devel
mailing list