linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] drm: Add mode resource leasing
@ 2017-04-01 17:08 Keith Packard
  2017-04-01 17:08 ` [PATCH 1/4] drm: Add new LEASE debug level Keith Packard
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-01 17:08 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie; +Cc: Keith Packard, dri-devel

Here's a first cut of the proposed mode resource leasing code. What
this does is allow an application to create a new drm_master which
"leases" resources from an existing drm_master. This new drm_master
can do whatever it likes with the resources it was granted, including
setting modes, performing page_flip operations etc.

This can be used to run multiple compositors on the same GPU, each
driving its own set of outputs. Examples of this include multi-user
setups and virtual reality environments.

Because setting modes can consume 'hidden' resources within the GPU,
it isn't entirely clear that letting multiple processes perform mode
setting is a good idea or not. A process doing the usual
test/render/commit sequence may find that the commit fails because
some other process consumed necessary resources after the test was
invoked. Daniel Vetter suggested that perhaps some kind of locking
mechanism across this sequence might help, but that can lead to
problems when a process fails to unlock. If someone wants to come up
with a reasonable scheme here, that'd be great.

For now, I'll be working on the assumption that the lease holder will
not set any modes, which will avoid the problematic case described above.

The series is broken into four patches in an attempt to make review a
bit easier.

The trickiest bit of the code was in creating the new drm_master,
which involved allocating a new file and fd and getting those
initialized with the right reference counts on all of the related data
structures. It "seems" to work, but it would be nice if someone with
more experience in that part of the kernel could take a look at
it. That's in the fourth patch.

The third patch hooks the lease checks into the other ioctls; that
could use some review to make sure I didn't miss any needed checks.

-keith

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/4] drm: Add new LEASE debug level
  2017-04-01 17:08 [PATCH 0/4] drm: Add mode resource leasing Keith Packard
@ 2017-04-01 17:08 ` Keith Packard
  2017-04-01 17:08 ` [PATCH 2/4] drm: Add drm_object lease infrastructure Keith Packard
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-01 17:08 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie; +Cc: Keith Packard, dri-devel

Separate out lease debugging from the core.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_drv.c | 3 ++-
 include/drm/drmP.h        | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 6594b4088f11..d4a3612655e3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -57,7 +57,8 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
 "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n"
 "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n"
 "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n"
-"\t\tBit 5 (0x20) will enable VBL messages (vblank code)");
+"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
+"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)");
 module_param_named(debug, drm_debug, int, 0600);
 
 static DEFINE_SPINLOCK(drm_minor_lock);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9c4ee144b5f6..304a22c87999 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -137,6 +137,7 @@ struct dma_buf_attachment;
 #define DRM_UT_ATOMIC		0x10
 #define DRM_UT_VBL		0x20
 #define DRM_UT_STATE		0x40
+#define DRM_UT_LEASE		0x80
 
 /***********************************************************************/
 /** \name DRM template customization defaults */
@@ -251,6 +252,9 @@ struct dma_buf_attachment;
 #define DRM_DEBUG_VBL(fmt, ...)					\
 	drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
 
+#define DRM_DEBUG_LEASE(fmt, ...)					\
+	drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
+
 #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)	\
 ({									\
 	static DEFINE_RATELIMIT_STATE(_rs,				\
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/4] drm: Add drm_object lease infrastructure
  2017-04-01 17:08 [PATCH 0/4] drm: Add mode resource leasing Keith Packard
  2017-04-01 17:08 ` [PATCH 1/4] drm: Add new LEASE debug level Keith Packard
@ 2017-04-01 17:08 ` Keith Packard
  2017-04-02 13:38   ` Daniel Vetter
  2017-04-02 17:51   ` Daniel Vetter
  2017-04-01 17:08 ` [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths Keith Packard
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-01 17:08 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie; +Cc: Keith Packard, dri-devel

This provides new data structures to hold "lease" information about
drm mode setting objects, and provides for creating new drm_masters
which have access to a subset of the available drm resources.

An 'owner' is a drm_master which is not leasing the objects from
another drm_master, and hence 'owns' them. This sits at the top of a
tree of drm_masters.

A 'lessee' is a drm_master which is leasing objects from some other
drm_master. Each lessee holds the set of objects which it is leasing
from the lessor.

A 'lessor' is a drm_master which is leasing objects to another
drm_master.

The set of objects any drm_master 'controls' is limited to the set of
objects it leases (for lessees) or all objects (for owners),
optionally minus the set of objects it has leased to other
drm_masters.

Objects not controlled by a drm_master cannot be modified through the
various state manipulating ioctls, and any state reported back to user
space will be edited to make them appear idle and/or unusable. For
instance, connectors always report 'disconnected', while encoders
report no possible crtcs or clones.

The full list of lessees leasing objects from an owner (either
directly, or indirectly through another lessee), can be searched from
an idr in the drm_master of the owner.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/Makefile    |   3 +-
 drivers/gpu/drm/drm_auth.c  |  22 +-
 drivers/gpu/drm/drm_lease.c | 485 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h          |   1 +
 include/drm/drm_auth.h      |  28 +++
 include/drm/drm_lease.h     |  51 +++++
 6 files changed, 588 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_lease.c
 create mode 100644 include/drm/drm_lease.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index b9ae4280de9d..c2c6d61d30cf 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,8 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_framebuffer.o drm_connector.o drm_blend.o \
 		drm_encoder.o drm_mode_object.o drm_property.o \
 		drm_plane.o drm_color_mgmt.o drm_print.o \
-		drm_dumb_buffers.o drm_mode_config.o
+		drm_dumb_buffers.o drm_mode_config.o \
+		drm_lease.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 6b143514a566..1db4f63860d1 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -31,6 +31,7 @@
 #include <drm/drmP.h>
 #include "drm_internal.h"
 #include "drm_legacy.h"
+#include <drm/drm_lease.h>
 
 /**
  * DOC: master and authentication
@@ -93,7 +94,7 @@ int drm_authmagic(struct drm_device *dev, void *data,
 	return file ? 0 : -EINVAL;
 }
 
-static struct drm_master *drm_master_create(struct drm_device *dev)
+struct drm_master *drm_master_create(struct drm_device *dev)
 {
 	struct drm_master *master;
 
@@ -107,6 +108,14 @@ static struct drm_master *drm_master_create(struct drm_device *dev)
 	idr_init(&master->magic_map);
 	master->dev = dev;
 
+	/* initialize the tree of output resource lessees */
+	master->lessor = NULL;
+	master->lessee_id = 0;
+	INIT_LIST_HEAD(&master->lessees);
+	INIT_LIST_HEAD(&master->lessee_list);
+	idr_init(&master->leases);
+	idr_init(&master->lessee_idr);
+
 	return master;
 }
 
@@ -189,6 +198,12 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
+	if (file_priv->master->lessor != NULL) {
+		DRM_DEBUG_LEASE("Attempt to set lessee %d as master\n", file_priv->master->lessee_id);
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	ret = drm_set_master(dev, file_priv, false);
 out_unlock:
 	mutex_unlock(&dev->master_mutex);
@@ -310,12 +325,17 @@ static void drm_master_destroy(struct kref *kref)
 	struct drm_master *master = container_of(kref, struct drm_master, refcount);
 	struct drm_device *dev = master->dev;
 
+	drm_lease_destroy(master);
+
 	if (dev->driver->master_destroy)
 		dev->driver->master_destroy(dev, master);
 
 	drm_legacy_master_rmmaps(dev, master);
 
 	idr_destroy(&master->magic_map);
+
+	idr_destroy(&master->lessee_idr);
+
 	kfree(master->unique);
 	kfree(master);
 }
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
new file mode 100644
index 000000000000..782005c7706d
--- /dev/null
+++ b/drivers/gpu/drm/drm_lease.c
@@ -0,0 +1,485 @@
+/*
+ * Copyright © 2017 Keith Packard <keithp@keithp.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <drm/drmP.h>
+#include "drm_internal.h"
+#include "drm_legacy.h"
+#include "drm_crtc_internal.h"
+#include <drm/drm_lease.h>
+#include <drm/drm_auth.h>
+
+#define drm_for_each_lessee(lessee, lessor) \
+	list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
+
+/*
+ * drm_lease_owner - return ancestor owner drm_master
+ * @master: drm_master somewhere within tree of lessees and lessors
+ *
+ * RETURN:
+ *
+ * drm_master at the top of the tree (i.e, with lessor NULL
+ */
+struct drm_master *drm_lease_owner(struct drm_master *master) {
+	while (master->lessor != NULL)
+		master = master->lessor;
+	return master;
+}
+EXPORT_SYMBOL(drm_lease_owner);
+
+/**
+ * _drm_lease_find: Look up an object and check who holds the lease
+ * @master: the master to look in
+ * @id: the id to check
+ *
+ * Check if 'master' holds a lease (or owns) 'id' and has not
+ * sub-leased it to another master. Return value:
+ *
+ *	object		'master' holds lease and has not sub-leased
+ *	-EACCESS	Some other master holds the lease
+ *	-ENOENT		'id' is not a valid DRM object for this device
+ *	-EBUSY		'master' holds lease, but has sub-leased
+ *
+ * Must be called with mode_config.idr_mutex held
+ */
+static struct drm_mode_object *_drm_lease_find(struct drm_master *master, int id)
+{
+	struct drm_master *lessee;
+	struct drm_mode_object *object;
+
+	DRM_DEBUG_LEASE("master %d object %d\n", master->lessee_id, id);
+	BUG_ON(__mutex_owner(&master->dev->mode_config.idr_mutex) != current);
+
+	object = idr_find(&master->dev->mode_config.crtc_idr, id);
+	if (IS_ERR_OR_NULL(object)) {
+		DRM_DEBUG_LEASE("no such object %d\n", id);
+		return ERR_PTR(-ENOENT);
+	}
+
+	if (master->lessor) {
+		/*
+		 * If the lessor is not the owner, then make sure they
+		 * are the current lease holder for the object
+		 */
+		if (idr_find(&master->leases, id) == NULL) {
+			DRM_DEBUG_LEASE("%d is not lessee for %d\n", master->lessee_id, id);
+			return ERR_PTR(-EACCES);
+		}
+	}
+
+	/* Now make sure no other lessee already holds a lease which masks this object */
+	drm_for_each_lessee(lessee, master) {
+		if (lessee->mask_lease && idr_find(&lessee->leases, id) != NULL) {
+			DRM_DEBUG_LEASE("id %d is held by %d\n", id, lessee->lessee_id);
+			return ERR_PTR(-EBUSY);
+		}
+	}
+
+	DRM_DEBUG_LEASE("id %d is held by us\n", id);
+	return object;
+}
+
+struct drm_mode_object *drm_lease_find(struct drm_master *master, int id)
+{
+	struct drm_mode_object *object;
+
+	BUG_ON(__mutex_owner(&master->dev->mode_config.idr_mutex) == current);
+	mutex_lock(&master->dev->mode_config.idr_mutex);
+	object = _drm_lease_find(master, id);
+	mutex_unlock(&master->dev->mode_config.idr_mutex);
+	if (IS_ERR(object))
+		DRM_DEBUG_LEASE("find %d from master %d failed %ld\n", id, master->lessee_id, PTR_ERR(object));
+	else
+		DRM_DEBUG_LEASE("find %d from master %d %p\n", id, master->lessee_id, object);
+	return object;
+}
+EXPORT_SYMBOL(drm_lease_find);
+
+static inline int _drm_lease_check(struct drm_master *master, int id) {
+	struct drm_mode_object *object = _drm_lease_find(master, id);
+	if (IS_ERR(object))
+		return PTR_ERR(object);
+	return 0;
+}
+
+/**
+ * drm_lease_hoder - find current lease holder for a resource
+ * @master: Some drm_master for the device
+ * @id: the resource id
+ *
+ * Walks down the lease holder tree looking for the drm_master who holds
+ * a lease and is not sub-leasing to another drm_master.
+ *
+ * Returns the owner if no lessee holds the resource
+ */
+static struct drm_master *_drm_lease_holder(struct drm_master *master, int id)
+{
+	struct drm_master *lessor = drm_lease_owner(master);
+
+	for (;;) {
+		struct drm_master *lessee;
+		struct drm_master *found;
+
+		found = NULL;
+		drm_for_each_lessee(lessee, lessor) {
+			if (idr_find(&lessee->leases, id) != NULL) {
+				found = lessee;
+				break;
+			}
+		}
+		if (found == NULL)
+			return lessor;
+		lessor = found;
+	}
+}
+
+/**
+ * drm_lessee_is_descendant - check hierarchy structure
+ * @lessee: lessee
+ * @lessor: lessor
+ *
+ * Checks whether 'lessor' is in control of 'lessee' resources, which
+ * happens when lessee is lessor or lessee is a lessee of lessor or
+ * one of lessors lessees (recursively)
+ */
+static bool
+drm_lessee_is_descendant(struct drm_master *lessee, struct drm_master *lessor)
+{
+	/* Walk up the chain to the owner */
+	while (lessee) {
+
+		/* return true if we found lessor on the way up */
+		if (lessee == lessor)
+			return true;
+		lessee = lessee->lessor;
+	}
+
+	/* return false if lessor is not 'above' lessee */
+	return false;
+}
+
+/**
+ * drm_lessee_get - Get a reference on a lessee_id in a lease sub-tree
+ * @top: the top of the sub-tree to search in
+ * @id: the lessee_id to look for
+ */
+static struct drm_master *
+drm_lessee_get(struct drm_master *top, int lessee_id)
+{
+	struct drm_master *owner;
+	struct drm_master *lessee;
+
+	BUG_ON(__mutex_owner(&top->dev->master_mutex) != current);
+
+	DRM_DEBUG_LEASE("%d\n", lessee_id);
+
+	owner = drm_lease_owner(top);
+	if (lessee_id == 0)
+		lessee = owner;
+	else
+		lessee = idr_find(&owner->lessee_idr, lessee_id);
+
+	DRM_DEBUG_LEASE("%d -> %p (top %p)\n", lessee_id, lessee, top);
+	if (!lessee || !drm_lessee_is_descendant(lessee, top)) {
+		DRM_DEBUG_LEASE("null lessee or not descendant\n");
+		return NULL;
+	}
+
+	return drm_master_get(lessee);
+}
+
+enum drm_lease_walk {
+	drm_lease_walk_stop,
+	drm_lease_walk_dont_descend,
+	drm_lease_walk_descend
+};
+
+/**
+ * drm_lease_walk - walk a subtree of leases
+ * @master: the top of the sub-tree to walk
+ * @func: the function to call on each lease
+ * @closure: data to pass to the function
+ */
+static struct drm_master *
+drm_lease_walk(struct drm_master *top,
+	       enum drm_lease_walk (*func)(struct drm_master *lessee, void *closure),
+	       void *closure)
+{
+	struct drm_master *master = top;
+	enum drm_lease_walk result;
+
+	for (;;) {
+		result = (*func)(master, closure);
+		if (result == drm_lease_walk_stop)
+			return master;
+		if (result == drm_lease_walk_descend && !list_empty(&master->lessees)) {
+			master = list_first_entry(&master->lessees, struct drm_master, lessee_list);
+		} else {
+			while (master != top && list_is_last(&master->lessee_list,
+							     &master->lessor->lessees))
+				master = master->lessor;
+			if (master == top)
+				return NULL;
+			master = list_next_entry(master, lessee_list);
+		}
+	}
+}
+
+/**
+ * drm_lease_filter_crtcs - remove unleased crtcs from set
+ * @master: drm_master of requestor
+ * @crtcs: bitmask of crtcs to check
+ */
+uint32_t drm_lease_filter_crtcs(struct drm_master *master, uint32_t crtcs)
+{
+	struct drm_device *dev = master->dev;
+	struct drm_crtc *crtc;
+	int count;
+
+	BUG_ON(__mutex_owner(&master->dev->mode_config.idr_mutex) == current);
+	mutex_lock(&master->dev->mode_config.idr_mutex);
+	count = -1;
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		uint32_t	mask = 1ul << count;
+		count++;
+
+		if ((crtcs & mask) != 0)
+			if (IS_ERR(_drm_lease_find(master, crtc->base.id)))
+				crtcs &= ~mask;
+	}
+	mutex_unlock(&master->dev->mode_config.idr_mutex);
+	return crtcs;
+}
+EXPORT_SYMBOL(drm_lease_filter_crtcs);
+
+/**
+ * drm_lease_filter_encoders - remove unleased encoders from set
+ * @master: drm_master of requestor
+ * @encoders: bitmask of encoders to check
+ */
+uint32_t drm_lease_filter_encoders(struct drm_master *master, uint32_t encoders)
+{
+	struct drm_device *dev = master->dev;
+	struct drm_encoder *encoder;
+	int count;
+
+	BUG_ON(__mutex_owner(&master->dev->mode_config.idr_mutex) == current);
+	mutex_lock(&master->dev->mode_config.idr_mutex);
+	count = -1;
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+		uint32_t	mask = 1ul << count;
+		count++;
+
+		if ((encoders & mask) != 0)
+			if (IS_ERR(_drm_lease_find(master, encoder->base.id)))
+				encoders &= ~mask;
+	}
+	mutex_unlock(&master->dev->mode_config.idr_mutex);
+	return encoders;
+}
+EXPORT_SYMBOL(drm_lease_filter_encoders);
+
+/*
+ * drm_lease_create - create a new drm_master with leased objects
+ * @lessor: lease holder (or owner) of objects
+ * @leases: objects to lease to the new drm_master
+ *
+ * Uses drm_master_create to allocate a new drm_master, then checks to
+ * make sure all of the desired objects can be leased, atomically
+ * leasing them to the new drmmaster.
+ *
+ * 	ERR_PTR(-EACCESS)	some other master holds the title to any object
+ * 	ERR_PTR(-ENOENT)	some object is not a valid DRM object for this device
+ * 	ERR_PTR(-EBUSY)		some other lessee holds title to this object
+ *	ERR_PTR(-EEXIST)	same object specified more than once in the provided list
+ *	ERR_PTR(-ENOMEM)	allocation failed
+ */
+static struct drm_master *drm_lease_create(struct drm_master *lessor, bool mask_lease, struct idr *leases)
+{
+	int error;
+	struct drm_master *lessee;
+	int object;
+	int id;
+	void *entry;
+
+	DRM_DEBUG_LEASE("lessor %d\n", lessor->lessee_id);
+
+	BUG_ON(__mutex_owner(&lessor->dev->master_mutex) != current);
+
+	lessee = drm_master_create(lessor->dev);
+	if (!lessee) {
+		DRM_DEBUG_LEASE("drm_master_create failed\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* Insert the new lessee into the tree */
+
+	id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL);
+	if (id < 0) {
+		DRM_DEBUG_LEASE("lessee_id allocation failed %d\n", id);
+		drm_master_put(&lessee);
+		return ERR_PTR(id);
+	}
+
+	lessee->lessee_id = id;
+	lessee->lessor = drm_master_get(lessor);
+	list_add_tail(&lessee->lessee_list, &lessor->lessees);
+
+	/* Lock the mode object mutex to make the check and allocation atomic */
+
+	mutex_lock(&lessor->dev->mode_config.idr_mutex);
+	idr_for_each_entry(leases, entry, object) {
+		error = _drm_lease_check(lessor, object);
+		if (error) {
+			mutex_unlock(&lessor->dev->mode_config.idr_mutex);
+			drm_master_put(&lessee);
+			DRM_DEBUG_LEASE("_drm_lease_check %d failed %d\n", object, error);
+			return ERR_PTR(error);
+		}
+	}
+
+	/* Move the leases over */
+	lessee->leases = *leases;
+	lessee->mask_lease = mask_lease;
+	idr_init(leases);
+	mutex_unlock(&lessor->dev->mode_config.idr_mutex);
+
+	DRM_DEBUG_LEASE("new lessee %d %p, lessor %d %p\n", lessee->lessee_id, lessee, lessor->lessee_id, lessor);
+
+	return lessee;
+}
+
+static enum drm_lease_walk drm_lease_validate(struct drm_master *lessee, void *v_lessor)
+{
+	struct drm_master *lessor = v_lessor;
+	struct drm_mode_object *entry;
+	int object;
+	int to_remove;
+	bool any_removed = false;
+
+	if (lessee == lessor)
+		return drm_lease_walk_descend;
+
+	for (;;) {
+		to_remove = 0;
+
+		/* it's not safe to call idr_remove inside the idr_for_each_entry loop,
+		 * so find the first invalid object, remove that, and then try again
+		 */
+		idr_for_each_entry(&lessee->leases, entry, object) {
+			if (idr_find(&lessor->leases, object) == NULL) {
+				to_remove = object;
+				break;
+			}
+		}
+		if (to_remove == 0)
+			break;
+		idr_remove(&lessee->leases, to_remove);
+		any_removed = true;
+	}
+	if (any_removed)
+		return drm_lease_walk_descend;
+	else
+		return drm_lease_walk_dont_descend;
+}
+
+/**
+ * drm_lease_change - change the resources held in a specific lease
+ * @lessor: lessor of the resources
+ * @lessee_id: unique id of the lessee
+ * @leases: new set of resources to be held in the lease
+ */
+int
+drm_lease_change(struct drm_master *lessor, int lessee_id, bool mask_lease, struct idr *leases)
+{
+	struct drm_master *lessee;
+	int ret;
+	struct drm_mode_object *entry;
+	int object;
+
+	/* Find the lessee and hold a reference while we mess around */
+	lessee = drm_lessee_get(lessor, lessee_id);
+	if (!lessee)
+		return -ENOENT;
+	if (lessee == lessor)
+		return -EINVAL;
+
+	/*
+	 * Check to make sure the updated lease terms are valid, which
+	 * means that each resource is leased by the lessor and either
+	 * not sub-leased, or leased to the lessee
+	 */
+
+	mutex_lock(&lessor->dev->mode_config.idr_mutex);
+	idr_for_each_entry(leases, entry, object) {
+		entry = _drm_lease_find(lessor, object);
+		if (IS_ERR(entry)) {
+			ret = PTR_ERR(entry);
+			if (ret != -EBUSY ||
+			    _drm_lease_holder(lessor, object) != lessee)
+			{
+				mutex_unlock(&lessor->dev->mode_config.idr_mutex);
+				drm_master_put(&lessee);
+				return ret;
+			}
+		}
+	}
+
+	/* Free the existing leases and move the new ones in */
+	idr_destroy(&lessee->leases);
+	lessee->leases = *leases;
+	lessee->mask_lease = mask_lease;
+	idr_init(leases);
+
+	drm_lease_walk(lessee, drm_lease_validate, lessee);
+
+	mutex_unlock(&lessor->dev->mode_config.idr_mutex);
+
+	drm_master_put(&lessee);
+	return 0;
+}
+
+/**
+ * drm_lease_destroy - a master is going away
+ * @master: the drm_master being destroyed
+ *
+ * Revoke all leases held by this master from it and all lessees
+ * Orphan all lessees. They'll go away when closed.
+ */
+void drm_lease_destroy(struct drm_master *master)
+{
+	struct drm_device *dev = master->dev;
+
+	BUG_ON(__mutex_owner(&dev->master_mutex) != current);
+
+	DRM_DEBUG_LEASE("drm_lease_destroy %d\n", master->lessee_id);
+
+	/* This master is referenced by all lessees, hence it cannot be destroyed
+	 * until all of them have been
+	 */
+
+	BUG_ON(!list_empty(&master->lessees));
+
+	/* Remove this master from the lessee idr in the owner */
+	if (master->lessee_id != 0) {
+		DRM_DEBUG_LEASE("remove master %d from device list of lessees\n", master->lessee_id);
+		idr_remove(&(drm_lease_owner(master)->lessee_idr), master->lessee_id);
+	}
+
+	/* Remove this master from any lessee list it may be on */
+	list_del(&master->lessee_list);
+	if (master->lessor)
+		drm_master_put(&master->lessor);
+
+	DRM_DEBUG_LEASE("drm_lease_destroy done %d\n", master->lessee_id);
+}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 304a22c87999..b468bc9cf318 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -340,6 +340,7 @@ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
 #define DRM_CONTROL_ALLOW 0x8
 #define DRM_UNLOCKED	0x10
 #define DRM_RENDER_ALLOW 0x20
+#define DRM_ANY_MASTER	0x40
 
 struct drm_ioctl_desc {
 	unsigned int cmd;
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index 610223b0481b..e0e2af09d3af 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -50,10 +50,38 @@ struct drm_master {
 	struct idr magic_map;
 	struct drm_lock_data lock;
 	void *driver_priv;
+
+	/* Tree of display resource leases, each of which is a drm_master struct
+	 * All of these get activated simultaneously, so drm_device master points
+	 * at the top of the tree (for which lessor is NULL)
+	 */
+
+	/** Lease holder */
+	struct drm_master *lessor;
+
+	/** id for lessees. Owners always have id 0 */
+	int	lessee_id;
+
+	/** other lessees of the same master */
+	struct list_head lessee_list;
+
+	/** drm_masters leasing from this one */
+	struct list_head lessees;
+
+	/** Objects leased to this drm_master. */
+	struct idr leases;
+
+	/** All lessees under this owner (only used where lessor == NULL) */
+	struct idr lessee_idr;
+
+	/** Indicates that the leased objects should be hidden from the lessor */
+	bool mask_lease;
 };
 
 struct drm_master *drm_master_get(struct drm_master *master);
 void drm_master_put(struct drm_master **master);
 bool drm_is_current_master(struct drm_file *fpriv);
 
+struct drm_master *drm_master_create(struct drm_device *dev);
+
 #endif
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
new file mode 100644
index 000000000000..e02adf3e42fd
--- /dev/null
+++ b/include/drm/drm_lease.h
@@ -0,0 +1,51 @@
+/*
+ * Copyright © 2017 Keith Packard <keithp@keithp.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef _DRM_LEASE_H_
+#define _DRM_LEASE_H_
+
+struct drm_file;
+struct drm_device;
+
+struct drm_master *drm_lease_owner(struct drm_master *master);
+
+struct drm_master *drm_lessee_find(struct drm_master *top, int lessee_id);
+
+void drm_lease_destroy(struct drm_master *lessee);
+
+struct drm_mode_object *drm_lease_find(struct drm_master *master, int id);
+
+/**
+ * drm_lease_check - check drm_mode_object lease status
+ * @master: the drm_master
+ * @id: the object id
+ *
+ * Checks if the specified master holds a lease on the object
+ * and also whether it has been leased to some lessee of the
+ * specified master. Return value:
+ *
+ *	0		'master' holds a lease (or owns) and has not leased
+ *	-EACCESS	Some other master holds the lease
+ *	-ENOENT		'id' is not a valid DRM object for this device
+ *	-EBUSY		'master' holds lease, but has sub-leased
+ */
+
+static inline int drm_lease_check(struct drm_master *master, int id) {
+	struct drm_mode_object *object = drm_lease_find(master, id);
+	if (IS_ERR(object))
+		return PTR_ERR(object);
+	return 0;
+}
+
+#endif /* _DRM_LEASE_H_ */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths
  2017-04-01 17:08 [PATCH 0/4] drm: Add mode resource leasing Keith Packard
  2017-04-01 17:08 ` [PATCH 1/4] drm: Add new LEASE debug level Keith Packard
  2017-04-01 17:08 ` [PATCH 2/4] drm: Add drm_object lease infrastructure Keith Packard
@ 2017-04-01 17:08 ` Keith Packard
  2017-04-02 13:19   ` Daniel Vetter
  2017-04-01 17:08 ` [PATCH 4/4] drm: Add four ioctls for managing drm mode object leases Keith Packard
  2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
  4 siblings, 1 reply; 22+ messages in thread
From: Keith Packard @ 2017-04-01 17:08 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie; +Cc: Keith Packard, dri-devel

Attempts to modify un-leased objects are rejected with an error.
Information returned about unleased objects is modified to make them
appear unusable and/or disconnected.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_atomic.c      |  3 +++
 drivers/gpu/drm/drm_auth.c        |  2 +-
 drivers/gpu/drm/drm_color_mgmt.c  |  3 +++
 drivers/gpu/drm/drm_connector.c   | 52 ++++++++++++++++++++++++++-------------
 drivers/gpu/drm/drm_crtc.c        | 15 ++++++++---
 drivers/gpu/drm/drm_encoder.c     | 18 +++++++++++---
 drivers/gpu/drm/drm_mode_object.c |  3 +++
 drivers/gpu/drm/drm_plane.c       | 36 +++++++++++++++++++++++----
 include/drm/drm_lease.h           |  4 +++
 include/drm/drm_mode_object.h     |  1 +
 10 files changed, 108 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index fdfb1ec17e66..a3cb95f7f1c1 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -2134,6 +2134,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 			goto out;
 		}
 
+		if ((ret = drm_lease_check(file_priv->master, obj->id)) < 0)
+			goto out;
+
 		if (!obj->properties) {
 			drm_mode_object_unreference(obj);
 			ret = -ENOENT;
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 1db4f63860d1..44c99d12f4c1 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -303,7 +303,7 @@ void drm_master_release(struct drm_file *file_priv)
  */
 bool drm_is_current_master(struct drm_file *fpriv)
 {
-	return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
+	return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master;
 }
 EXPORT_SYMBOL(drm_is_current_master);
 
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 6543ebde501a..f8d7a499cf95 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -206,6 +206,9 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
+	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0)
+		goto out;
+
 	if (crtc->funcs->gamma_set == NULL) {
 		ret = -ENOSYS;
 		goto out;
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 7a7019ac9388..a95db57dd966 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1079,6 +1079,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	struct drm_mode_modeinfo u_mode;
 	struct drm_mode_modeinfo __user *mode_ptr;
 	uint32_t __user *encoder_ptr;
+	bool leased;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -1093,9 +1094,16 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
-		if (connector->encoder_ids[i] != 0)
-			encoders_count++;
+	leased = drm_lease_check(file_priv->master, connector->base.id) == 0;
+
+	DRM_DEBUG_LEASE("connector %d leased %s\n", connector->base.id, leased ? "true" : "false");
+
+	if (leased) {
+		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
+			if (connector->encoder_ids[i] != 0 &&
+			    drm_lease_check(file_priv->master, connector->encoder_ids[i]) == 0)
+				encoders_count++;
+	}
 
 	if (out_resp->count_modes == 0) {
 		connector->funcs->fill_modes(connector,
@@ -1104,21 +1112,29 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	}
 
 	/* delayed so we get modes regardless of pre-fill_modes state */
-	list_for_each_entry(mode, &connector->modes, head)
-		if (drm_mode_expose_to_userspace(mode, file_priv))
-			mode_count++;
+	if (leased)
+		list_for_each_entry(mode, &connector->modes, head)
+			if (drm_mode_expose_to_userspace(mode, file_priv))
+				mode_count++;
 
 	out_resp->connector_id = connector->base.id;
 	out_resp->connector_type = connector->connector_type;
 	out_resp->connector_type_id = connector->connector_type_id;
-	out_resp->mm_width = connector->display_info.width_mm;
-	out_resp->mm_height = connector->display_info.height_mm;
-	out_resp->subpixel = connector->display_info.subpixel_order;
-	out_resp->connection = connector->status;
+	if (leased) {
+		out_resp->mm_width = connector->display_info.width_mm;
+		out_resp->mm_height = connector->display_info.height_mm;
+		out_resp->subpixel = connector->display_info.subpixel_order;
+		out_resp->connection = connector->status;
+	} else {
+		out_resp->mm_width = 0;
+		out_resp->mm_height = 0;
+		out_resp->subpixel = 0;
+		out_resp->connection = connector_status_disconnected;
+	}
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	encoder = drm_connector_get_encoder(connector);
-	if (encoder)
+	if (encoder && leased)
 		out_resp->encoder_id = encoder->base.id;
 	else
 		out_resp->encoder_id = 0;
@@ -1145,12 +1161,14 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	}
 	out_resp->count_modes = mode_count;
 
-	ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,
-			(uint32_t __user *)(unsigned long)(out_resp->props_ptr),
-			(uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),
-			&out_resp->count_props);
-	if (ret)
-		goto out;
+	if (leased) {
+		ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,
+						     (uint32_t __user *)(unsigned long)(out_resp->props_ptr),
+						     (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),
+						     &out_resp->count_props);
+		if (ret)
+			goto out;
+	}
 
 	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
 		copied = 0;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e75f62cd8a65..95026ca74568 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -347,6 +347,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
 {
 	struct drm_mode_crtc *crtc_resp = data;
 	struct drm_crtc *crtc;
+	bool leased;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -355,9 +356,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
 	if (!crtc)
 		return -ENOENT;
 
+	leased = drm_lease_check(file_priv->master, crtc->base.id) == 0;
+
+	DRM_DEBUG_LEASE("crtc %d leased %s\n", crtc->base.id, leased ? "true" : "false");
+
 	drm_modeset_lock_crtc(crtc, crtc->primary);
 	crtc_resp->gamma_size = crtc->gamma_size;
-	if (crtc->primary->fb)
+	if (crtc->primary->fb && leased)
 		crtc_resp->fb_id = crtc->primary->fb->base.id;
 	else
 		crtc_resp->fb_id = 0;
@@ -365,7 +370,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
 	if (crtc->state) {
 		crtc_resp->x = crtc->primary->state->src_x >> 16;
 		crtc_resp->y = crtc->primary->state->src_y >> 16;
-		if (crtc->state->enable) {
+		if (crtc->state->enable && leased) {
 			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
 			crtc_resp->mode_valid = 1;
 
@@ -375,7 +380,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
 	} else {
 		crtc_resp->x = crtc->x;
 		crtc_resp->y = crtc->y;
-		if (crtc->enabled) {
+		if (crtc->enabled && leased) {
 			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode);
 			crtc_resp->mode_valid = 1;
 
@@ -529,6 +534,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 	}
 	DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
 
+	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) {
+		DRM_DEBUG_KMS("CRTC lease not held\n");
+		goto out;
+	}
 	if (crtc_req->mode_valid) {
 		/* If we have a mode we need a framebuffer. */
 		/* If we pass -1, set the mode with the currently bound fb */
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 992879f15f23..24d03e13f522 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -201,6 +201,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 	struct drm_mode_get_encoder *enc_resp = data;
 	struct drm_encoder *encoder;
 	struct drm_crtc *crtc;
+	bool leased;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -209,9 +210,13 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 	if (!encoder)
 		return -ENOENT;
 
+	leased = drm_lease_check(file_priv->master, encoder->base.id) == 0;
+
+	DRM_DEBUG_LEASE("encoder %d leased %s\n", encoder->base.id, leased ? "true" : "false");
+
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	crtc = drm_encoder_get_crtc(encoder);
-	if (crtc)
+	if (crtc && leased)
 		enc_resp->crtc_id = crtc->base.id;
 	else
 		enc_resp->crtc_id = 0;
@@ -219,8 +224,15 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 
 	enc_resp->encoder_type = encoder->encoder_type;
 	enc_resp->encoder_id = encoder->base.id;
-	enc_resp->possible_crtcs = encoder->possible_crtcs;
-	enc_resp->possible_clones = encoder->possible_clones;
+	if (leased) {
+		enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master,
+								  encoder->possible_crtcs);
+		enc_resp->possible_clones = drm_lease_filter_encoders(file_priv->master,
+								      encoder->possible_clones);
+	} else {
+		enc_resp->possible_crtcs = 0;
+		enc_resp->possible_clones = 0;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 9f17085b1fdd..9f8559d82a17 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -404,6 +404,9 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 		goto out;
 	}
 
+	if ((ret = drm_lease_check(file_priv->master, arg->obj_id)) != 0)
+		goto out;
+
 	if (!arg_obj->properties)
 		goto out_unref;
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 62b98f386fd1..df811869c1dd 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -383,6 +383,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 	struct drm_mode_get_plane *plane_resp = data;
 	struct drm_plane *plane;
 	uint32_t __user *format_ptr;
+	bool leased;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -391,27 +392,34 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 	if (!plane)
 		return -ENOENT;
 
+	leased = drm_lease_check(file_priv->master, plane->base.id) == 0;
+
 	drm_modeset_lock(&plane->mutex, NULL);
-	if (plane->crtc)
+	if (plane->crtc && leased)
 		plane_resp->crtc_id = plane->crtc->base.id;
 	else
 		plane_resp->crtc_id = 0;
 
-	if (plane->fb)
+	if (plane->fb && leased)
 		plane_resp->fb_id = plane->fb->base.id;
 	else
 		plane_resp->fb_id = 0;
 	drm_modeset_unlock(&plane->mutex);
 
 	plane_resp->plane_id = plane->base.id;
-	plane_resp->possible_crtcs = plane->possible_crtcs;
+	if (leased)
+		plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master,
+								    plane->possible_crtcs);
+	else
+		plane_resp->possible_crtcs = 0;
+
 	plane_resp->gamma_size = 0;
 
 	/*
 	 * This ioctl is called twice, once to determine how much space is
 	 * needed, and the 2nd time to fill it.
 	 */
-	if (plane->format_count &&
+	if (plane->format_count && leased &&
 	    (plane_resp->count_format_types >= plane->format_count)) {
 		format_ptr = (uint32_t __user *)(unsigned long)plane_resp->format_type_ptr;
 		if (copy_to_user(format_ptr,
@@ -420,7 +428,10 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 			return -EFAULT;
 		}
 	}
-	plane_resp->count_format_types = plane->format_count;
+	if (leased)
+		plane_resp->count_format_types = plane->format_count;
+	else
+		plane_resp->count_format_types = 0;
 
 	return 0;
 }
@@ -551,6 +562,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 	struct drm_plane *plane;
 	struct drm_crtc *crtc = NULL;
 	struct drm_framebuffer *fb = NULL;
+	int ret;
 
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
@@ -566,6 +578,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 		return -ENOENT;
 	}
 
+	if ((ret = drm_lease_check(file_priv->master, plane->base.id)) < 0) {
+		DRM_DEBUG_KMS("Plane lease not held: %d error %d\n",
+			      plane->base.id, ret);
+		return ret;
+	}
+
 	if (plane_req->fb_id) {
 		fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
 		if (!fb) {
@@ -687,6 +705,11 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 		DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id);
 		return -ENOENT;
 	}
+	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) {
+		DRM_DEBUG_KMS("CRTC lease not held %d error %d\n",
+			      crtc->base.id, ret);
+		goto out;
+	}
 
 	/*
 	 * If this crtc has a universal cursor plane, call that plane's update
@@ -785,6 +808,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	if (!crtc)
 		return -ENOENT;
 
+	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0)
+		return ret;
+
 	if (crtc->funcs->page_flip_target) {
 		u32 current_vblank;
 		int r;
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
index e02adf3e42fd..8f91fc4226e3 100644
--- a/include/drm/drm_lease.h
+++ b/include/drm/drm_lease.h
@@ -48,4 +48,8 @@ static inline int drm_lease_check(struct drm_master *master, int id) {
 	return 0;
 }
 
+uint32_t drm_lease_filter_crtcs(struct drm_master *master, uint32_t crtcs);
+
+uint32_t drm_lease_filter_encoders(struct drm_master *master, uint32_t encoders);
+
 #endif /* _DRM_LEASE_H_ */
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index 43460b21d112..07830182598b 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -24,6 +24,7 @@
 #define __DRM_MODESET_H__
 
 #include <linux/kref.h>
+#include <drm/drm_lease.h>
 struct drm_object_properties;
 struct drm_property;
 struct drm_device;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/4] drm: Add four ioctls for managing drm mode object leases
  2017-04-01 17:08 [PATCH 0/4] drm: Add mode resource leasing Keith Packard
                   ` (2 preceding siblings ...)
  2017-04-01 17:08 ` [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths Keith Packard
@ 2017-04-01 17:08 ` Keith Packard
  2017-04-02 13:23   ` Daniel Vetter
  2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
  4 siblings, 1 reply; 22+ messages in thread
From: Keith Packard @ 2017-04-01 17:08 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie; +Cc: Keith Packard, dri-devel

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 from a particular lessor

drm_mode_get_lease

	List the leased objects for a particular lessee

drm_mode_change_lease

	Adjust the terms of a lease, adding or removing resources or
	switching from masking to non-masking.

This should suffice to at least create, query and manage available
leases.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_ioctl.c |   4 +
 drivers/gpu/drm/drm_lease.c | 309 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_lease.h     |  12 ++
 include/uapi/drm/drm.h      |   4 +
 include/uapi/drm/drm_mode.h |  78 +++++++++++
 5 files changed, 407 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fed22c2b98b6..0f9e3c0fe2ac 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -636,6 +636,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATOMIC, drm_mode_atomic_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATEPROPBLOB, drm_mode_createblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROYPROPBLOB, drm_mode_destroyblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CHANGE_LEASE, drm_mode_change_lease_ioctl, DRM_ANY_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 782005c7706d..39131860bcd3 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -483,3 +483,312 @@ void drm_lease_destroy(struct drm_master *master)
 
 	DRM_DEBUG_LEASE("drm_lease_destroy done %d\n", master->lessee_id);
 }
+
+/**
+ * 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;
+
+	object_count = cl->object_count;
+	idr_init(&leases);
+
+	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;
+
+		if (copy_from_user(&object_id,
+				   u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32),
+				   sizeof (__u32))) {
+			ret = -EFAULT;
+			goto bail;
+		}
+		DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
+
+		ret = idr_alloc(&leases, (void *) 1, object_id, object_id + 1, GFP_KERNEL);
+		if (ret < 0) {
+			DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
+					object_id, ret);
+			goto bail;
+		}
+	}
+
+	mutex_lock(&dev->master_mutex);
+
+	DRM_DEBUG_LEASE("Creating lease\n");
+	lessee = drm_lease_create(lessor, cl->mask_lease != 0, &leases);
+
+	if (IS_ERR(lessee)) {
+		ret = PTR_ERR(lessee);
+		lessee = NULL;
+		mutex_unlock(&dev->master_mutex);
+		goto bail;
+	}
+
+	/* Clone the lessor file to create a new file for us */
+	DRM_DEBUG_LEASE("Allocating lease file\n");
+	path_get(&lessor_file->f_path);
+	lessee_file = alloc_file(&lessor_file->f_path,
+				 lessor_file->f_mode,
+				 fops_get(lessor_file->f_inode->i_fop));
+
+	if (IS_ERR(lessee_file)) {
+		ret = PTR_ERR(lessee_file);
+		lessee_file = NULL;
+		mutex_unlock(&dev->master_mutex);
+		goto bail;
+	}
+
+	mutex_unlock(&dev->master_mutex);
+
+	/* 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 bail;
+
+	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;
+
+	/* and don't destroy our resources */
+	fd = -1;
+	lessee = NULL;
+	lessee_file = NULL;
+
+	ret = 0;
+
+bail:
+
+	if (lessee_file) {
+		DRM_DEBUG_LEASE("Freeing lessee file\n");
+		fput(lessee_file);
+	}
+
+	if (lessee) {
+		DRM_DEBUG_LEASE("Freeing lessee drm_master\n");
+		drm_master_put(&lessee);
+	}
+
+	idr_destroy(&leases);
+
+	if (fd != -1) {
+		DRM_DEBUG_LEASE("Freeing unused fd\n");
+		put_unused_fd(fd);
+	}
+
+	DRM_DEBUG_LEASE("Return %d from drm_mode_create_lease_ioctl\n", ret);
+	return ret;
+}
+
+/**
+ * drm_mode_list_lessees_ioctl - list lessee ids
+ * @dev: the drm device
+ * @data: pointer to struct drm_mode_create_lease
+ * @file_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 *file_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, *lessee;
+	int count;
+	int ret = 0;
+
+	DRM_DEBUG_LEASE("List lessees for %d\n", arg->lessor_id);
+
+	mutex_lock(&dev->master_mutex);
+
+	/* Search the tree for the requested drm_master */
+	lessor = drm_lessee_get(file_priv->master, arg->lessor_id);
+	if (!lessor) {
+		DRM_DEBUG_LEASE("No such lessor %d\n", arg->lessor_id);
+		mutex_unlock(&dev->master_mutex);
+		return -ENOENT;
+	}
+
+	count = 0;
+	drm_for_each_lessee(lessee, lessor) {
+		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;
+
+	drm_master_put(&lessor);
+
+	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_create_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 *file_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;
+	struct idr *object_idr;
+	int count;
+	void *entry;
+	int object;
+	int ret = 0;
+
+	DRM_DEBUG_LEASE("get lease for %d\n", arg->lessee_id);
+
+	mutex_lock(&dev->master_mutex);
+
+	/* Search the tree for the requested drm_master */
+	lessee = drm_lessee_get(file_priv->master, arg->lessee_id);
+	if (!lessee) {
+		mutex_unlock(&dev->master_mutex);
+		DRM_DEBUG_LEASE("No such lessee %d\n", arg->lessee_id);
+		return -ENOENT;
+	}
+
+	if (lessee->lessor == NULL)
+		object_idr = &lessee->dev->mode_config.crtc_idr;
+	else
+		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;
+
+	drm_master_put(&lessee);
+
+	mutex_unlock(&dev->master_mutex);
+
+	return ret;
+}
+
+/**
+ * drm_mode_change_lease_ioctl - change the objects in a 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_change_lease_ioctl(struct drm_device *dev,
+				void *data, struct drm_file *file_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 = file_priv->master;
+
+	object_count = cl->object_count;
+	idr_init(&leases);
+
+	DRM_DEBUG_LEASE("Changing existing lease\n");
+
+	/* Lookup the mode objects and add their IDs to the lease request */
+	for (o = 0; o < object_count; o++) {
+		__u32 object_id;
+
+		if (copy_from_user(&object_id,
+				   u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32),
+				   sizeof (__u32))) {
+			return -EFAULT;
+		}
+		DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
+
+		ret = idr_alloc(&leases, (void *) 1, object_id, object_id + 1, GFP_KERNEL);
+		if (ret < 0) {
+			DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
+					object_id, ret);
+			return ret;
+		}
+	}
+
+	mutex_lock(&dev->master_mutex);
+
+	DRM_DEBUG_LEASE("Change lease\n");
+
+	ret = drm_lease_change(lessor, cl->lessee_id, cl->mask_lease != 0, &leases);
+
+	mutex_unlock(&dev->master_mutex);
+
+	idr_destroy(&leases);
+
+	DRM_DEBUG_LEASE("Return %d from drm_mode_change_lease_ioctl\n", ret);
+	return ret;
+}
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
index 8f91fc4226e3..367b57698f53 100644
--- a/include/drm/drm_lease.h
+++ b/include/drm/drm_lease.h
@@ -52,4 +52,16 @@ uint32_t drm_lease_filter_crtcs(struct drm_master *master, uint32_t crtcs);
 
 uint32_t drm_lease_filter_encoders(struct drm_master *master, uint32_t encoders);
 
+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_change_lease_ioctl(struct drm_device *dev,
+				void *data, struct drm_file *file_priv);
+
 #endif /* _DRM_LEASE_H_ */
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b2c52843bc70..8c62da23d7a3 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -813,6 +813,10 @@ extern "C" {
 #define DRM_IOCTL_MODE_ATOMIC		DRM_IOWR(0xBC, struct drm_mode_atomic)
 #define DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, struct drm_mode_create_blob)
 #define DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, struct drm_mode_destroy_blob)
+#define DRM_IOCTL_MODE_CREATE_LEASE	DRM_IOWR(0xBF, struct drm_mode_create_lease)
+#define DRM_IOCTL_MODE_LIST_LESSEES	DRM_IOWR(0xC0, struct drm_mode_list_lessees)
+#define DRM_IOCTL_MODE_GET_LEASE	DRM_IOWR(0xC1, struct drm_mode_get_lease)
+#define DRM_IOCTL_MODE_CHANGE_LEASE	DRM_IOWR(0xC2, struct drm_mode_change_lease)
 
 /**
  * Device specific ioctls should only be in their respective headers
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index ce7efe2e8a5e..e6669ada3b10 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -681,6 +681,84 @@ 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;
+	/** whether to hide the leased objects from the lessor */
+	__u32 mask_lease;
+
+	/** 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 {
+
+	/** Identifier of the lessor's lessee_id (0 for owner) */
+	__u32 lessor_id;
+
+	/** 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;
+
+	/** Pointer to lessees.
+	 * pointer to __u64 array of lessee ids
+	 */
+	__u64 lessees_ptr;
+};
+
+/**
+ * Get leased objects for a lessee
+ */
+struct drm_mode_get_lease {
+	/** Identifier of the lessee (0 for owner) */
+	__u32 lessee_id;
+
+	/** 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;
+
+	/** Pointer to objects.
+	 * pointer to __u32 array of object ids
+	 */
+	__u64 objects_ptr;
+};
+
+/**
+ * Change resources leased to another drm_master
+ */
+struct drm_mode_change_lease {
+	/** Pointer to array of object ids (__u32) */
+	__u64 object_ids;
+	/** Number of object ids */
+	__u32 object_count;
+	/** unique identifier for lessee. */
+	__u32 lessee_id;
+	/** whether to hide the leased objects from the lessor */
+	__u32 mask_lease;
+};
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths
  2017-04-01 17:08 ` [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths Keith Packard
@ 2017-04-02 13:19   ` Daniel Vetter
  2017-04-02 16:37     ` Keith Packard
  2017-04-10  1:06     ` Keith Packard
  0 siblings, 2 replies; 22+ messages in thread
From: Daniel Vetter @ 2017-04-02 13:19 UTC (permalink / raw)
  To: Keith Packard; +Cc: linux-kernel, Dave Airlie, dri-devel

On Sat, Apr 01, 2017 at 10:08:40AM -0700, Keith Packard wrote:
> Attempts to modify un-leased objects are rejected with an error.
> Information returned about unleased objects is modified to make them
> appear unusable and/or disconnected.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

Just a merge-technical idea to make this easier to change and less painful
to rebase until we've locked down the semantics properly. And also because
I expect we'll need v2 of this uapi soon or later anyway:

I think it'd be good if we could consolidate all the lease checking into
drm_mode_object_find (respectively __drm_mode_object_find). We'd need to
wire up the fpriv to be able to do that, but we could upstream that patch
right away before anything else. That should take care of most of the
checks in this patch here.

There's a few things on top:
- filtering the various bitmasks. I think you have most, but we could
  perhaps upstream the helpers for these.
- filtering object lists (essentially getresources and getplanes ioctls).
- filtering implicit objects in the legacy ioctl. E.g. page_flip done
  through atomic doesn't just need the CRTC id, but also the id of the
  primary plane plus of the FB_ID atomic property. Similarly for all the
  other legacy ioctls. I think we want to make sure there's no difference
  here in behaviour.

Especially for the last one it might be simplest to outright disallow all
legacy ioctl and require that sub-drm_master nodes only get access to the
read-only GET* ioctl (they get that anyway, even when they're not the
current master), plus atomic. Makes it a _lot_ easier to implement.
Downside is that amdgpu _really_ needs to land atomic asap :-)

> ---
>  drivers/gpu/drm/drm_atomic.c      |  3 +++
>  drivers/gpu/drm/drm_auth.c        |  2 +-
>  drivers/gpu/drm/drm_color_mgmt.c  |  3 +++
>  drivers/gpu/drm/drm_connector.c   | 52 ++++++++++++++++++++++++++-------------
>  drivers/gpu/drm/drm_crtc.c        | 15 ++++++++---
>  drivers/gpu/drm/drm_encoder.c     | 18 +++++++++++---
>  drivers/gpu/drm/drm_mode_object.c |  3 +++
>  drivers/gpu/drm/drm_plane.c       | 36 +++++++++++++++++++++++----
>  include/drm/drm_lease.h           |  4 +++
>  include/drm/drm_mode_object.h     |  1 +
>  10 files changed, 108 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index fdfb1ec17e66..a3cb95f7f1c1 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -2134,6 +2134,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>  			goto out;
>  		}
>  
> +		if ((ret = drm_lease_check(file_priv->master, obj->id)) < 0)
> +			goto out;
> +
>  		if (!obj->properties) {
>  			drm_mode_object_unreference(obj);
>  			ret = -ENOENT;
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 1db4f63860d1..44c99d12f4c1 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -303,7 +303,7 @@ void drm_master_release(struct drm_file *file_priv)
>   */
>  bool drm_is_current_master(struct drm_file *fpriv)
>  {
> -	return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
> +	return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master;

This feels like it should be part of the earlier patch to make
sub-drm_master objects for leasing. In that patch we should also make sure
that all the current docs about drm_master are updated correctly (but
maybe only later on, when we get towards merging this). This change here
is pretty important wrt set/drop_master ioctl behaviour and leases.

Cheers, Daniel

>  }
>  EXPORT_SYMBOL(drm_is_current_master);
>  
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 6543ebde501a..f8d7a499cf95 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -206,6 +206,9 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>  		goto out;
>  	}
>  
> +	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0)
> +		goto out;
> +
>  	if (crtc->funcs->gamma_set == NULL) {
>  		ret = -ENOSYS;
>  		goto out;
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 7a7019ac9388..a95db57dd966 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1079,6 +1079,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	struct drm_mode_modeinfo u_mode;
>  	struct drm_mode_modeinfo __user *mode_ptr;
>  	uint32_t __user *encoder_ptr;
> +	bool leased;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return -EINVAL;
> @@ -1093,9 +1094,16 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  		goto out_unlock;
>  	}
>  
> -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
> -		if (connector->encoder_ids[i] != 0)
> -			encoders_count++;
> +	leased = drm_lease_check(file_priv->master, connector->base.id) == 0;
> +
> +	DRM_DEBUG_LEASE("connector %d leased %s\n", connector->base.id, leased ? "true" : "false");
> +
> +	if (leased) {
> +		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
> +			if (connector->encoder_ids[i] != 0 &&
> +			    drm_lease_check(file_priv->master, connector->encoder_ids[i]) == 0)
> +				encoders_count++;
> +	}
>  
>  	if (out_resp->count_modes == 0) {
>  		connector->funcs->fill_modes(connector,
> @@ -1104,21 +1112,29 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	}
>  
>  	/* delayed so we get modes regardless of pre-fill_modes state */
> -	list_for_each_entry(mode, &connector->modes, head)
> -		if (drm_mode_expose_to_userspace(mode, file_priv))
> -			mode_count++;
> +	if (leased)
> +		list_for_each_entry(mode, &connector->modes, head)
> +			if (drm_mode_expose_to_userspace(mode, file_priv))
> +				mode_count++;
>  
>  	out_resp->connector_id = connector->base.id;
>  	out_resp->connector_type = connector->connector_type;
>  	out_resp->connector_type_id = connector->connector_type_id;
> -	out_resp->mm_width = connector->display_info.width_mm;
> -	out_resp->mm_height = connector->display_info.height_mm;
> -	out_resp->subpixel = connector->display_info.subpixel_order;
> -	out_resp->connection = connector->status;
> +	if (leased) {
> +		out_resp->mm_width = connector->display_info.width_mm;
> +		out_resp->mm_height = connector->display_info.height_mm;
> +		out_resp->subpixel = connector->display_info.subpixel_order;
> +		out_resp->connection = connector->status;
> +	} else {
> +		out_resp->mm_width = 0;
> +		out_resp->mm_height = 0;
> +		out_resp->subpixel = 0;
> +		out_resp->connection = connector_status_disconnected;
> +	}
>  
>  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	encoder = drm_connector_get_encoder(connector);
> -	if (encoder)
> +	if (encoder && leased)
>  		out_resp->encoder_id = encoder->base.id;
>  	else
>  		out_resp->encoder_id = 0;
> @@ -1145,12 +1161,14 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	}
>  	out_resp->count_modes = mode_count;
>  
> -	ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,
> -			(uint32_t __user *)(unsigned long)(out_resp->props_ptr),
> -			(uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),
> -			&out_resp->count_props);
> -	if (ret)
> -		goto out;
> +	if (leased) {
> +		ret = drm_mode_object_get_properties(&connector->base, file_priv->atomic,
> +						     (uint32_t __user *)(unsigned long)(out_resp->props_ptr),
> +						     (uint64_t __user *)(unsigned long)(out_resp->prop_values_ptr),
> +						     &out_resp->count_props);
> +		if (ret)
> +			goto out;
> +	}
>  
>  	if ((out_resp->count_encoders >= encoders_count) && encoders_count) {
>  		copied = 0;
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e75f62cd8a65..95026ca74568 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -347,6 +347,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  {
>  	struct drm_mode_crtc *crtc_resp = data;
>  	struct drm_crtc *crtc;
> +	bool leased;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return -EINVAL;
> @@ -355,9 +356,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  	if (!crtc)
>  		return -ENOENT;
>  
> +	leased = drm_lease_check(file_priv->master, crtc->base.id) == 0;
> +
> +	DRM_DEBUG_LEASE("crtc %d leased %s\n", crtc->base.id, leased ? "true" : "false");
> +
>  	drm_modeset_lock_crtc(crtc, crtc->primary);
>  	crtc_resp->gamma_size = crtc->gamma_size;
> -	if (crtc->primary->fb)
> +	if (crtc->primary->fb && leased)
>  		crtc_resp->fb_id = crtc->primary->fb->base.id;
>  	else
>  		crtc_resp->fb_id = 0;
> @@ -365,7 +370,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  	if (crtc->state) {
>  		crtc_resp->x = crtc->primary->state->src_x >> 16;
>  		crtc_resp->y = crtc->primary->state->src_y >> 16;
> -		if (crtc->state->enable) {
> +		if (crtc->state->enable && leased) {
>  			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->state->mode);
>  			crtc_resp->mode_valid = 1;
>  
> @@ -375,7 +380,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  	} else {
>  		crtc_resp->x = crtc->x;
>  		crtc_resp->y = crtc->y;
> -		if (crtc->enabled) {
> +		if (crtc->enabled && leased) {
>  			drm_mode_convert_to_umode(&crtc_resp->mode, &crtc->mode);
>  			crtc_resp->mode_valid = 1;
>  
> @@ -529,6 +534,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  	}
>  	DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
>  
> +	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) {
> +		DRM_DEBUG_KMS("CRTC lease not held\n");
> +		goto out;
> +	}
>  	if (crtc_req->mode_valid) {
>  		/* If we have a mode we need a framebuffer. */
>  		/* If we pass -1, set the mode with the currently bound fb */
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 992879f15f23..24d03e13f522 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -201,6 +201,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
>  	struct drm_mode_get_encoder *enc_resp = data;
>  	struct drm_encoder *encoder;
>  	struct drm_crtc *crtc;
> +	bool leased;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return -EINVAL;
> @@ -209,9 +210,13 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
>  	if (!encoder)
>  		return -ENOENT;
>  
> +	leased = drm_lease_check(file_priv->master, encoder->base.id) == 0;
> +
> +	DRM_DEBUG_LEASE("encoder %d leased %s\n", encoder->base.id, leased ? "true" : "false");
> +
>  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	crtc = drm_encoder_get_crtc(encoder);
> -	if (crtc)
> +	if (crtc && leased)
>  		enc_resp->crtc_id = crtc->base.id;
>  	else
>  		enc_resp->crtc_id = 0;
> @@ -219,8 +224,15 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
>  
>  	enc_resp->encoder_type = encoder->encoder_type;
>  	enc_resp->encoder_id = encoder->base.id;
> -	enc_resp->possible_crtcs = encoder->possible_crtcs;
> -	enc_resp->possible_clones = encoder->possible_clones;
> +	if (leased) {
> +		enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master,
> +								  encoder->possible_crtcs);
> +		enc_resp->possible_clones = drm_lease_filter_encoders(file_priv->master,
> +								      encoder->possible_clones);
> +	} else {
> +		enc_resp->possible_crtcs = 0;
> +		enc_resp->possible_clones = 0;
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 9f17085b1fdd..9f8559d82a17 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -404,6 +404,9 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
>  		goto out;
>  	}
>  
> +	if ((ret = drm_lease_check(file_priv->master, arg->obj_id)) != 0)
> +		goto out;
> +
>  	if (!arg_obj->properties)
>  		goto out_unref;
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 62b98f386fd1..df811869c1dd 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -383,6 +383,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  	struct drm_mode_get_plane *plane_resp = data;
>  	struct drm_plane *plane;
>  	uint32_t __user *format_ptr;
> +	bool leased;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return -EINVAL;
> @@ -391,27 +392,34 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  	if (!plane)
>  		return -ENOENT;
>  
> +	leased = drm_lease_check(file_priv->master, plane->base.id) == 0;
> +
>  	drm_modeset_lock(&plane->mutex, NULL);
> -	if (plane->crtc)
> +	if (plane->crtc && leased)
>  		plane_resp->crtc_id = plane->crtc->base.id;
>  	else
>  		plane_resp->crtc_id = 0;
>  
> -	if (plane->fb)
> +	if (plane->fb && leased)
>  		plane_resp->fb_id = plane->fb->base.id;
>  	else
>  		plane_resp->fb_id = 0;
>  	drm_modeset_unlock(&plane->mutex);
>  
>  	plane_resp->plane_id = plane->base.id;
> -	plane_resp->possible_crtcs = plane->possible_crtcs;
> +	if (leased)
> +		plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv->master,
> +								    plane->possible_crtcs);
> +	else
> +		plane_resp->possible_crtcs = 0;
> +
>  	plane_resp->gamma_size = 0;
>  
>  	/*
>  	 * This ioctl is called twice, once to determine how much space is
>  	 * needed, and the 2nd time to fill it.
>  	 */
> -	if (plane->format_count &&
> +	if (plane->format_count && leased &&
>  	    (plane_resp->count_format_types >= plane->format_count)) {
>  		format_ptr = (uint32_t __user *)(unsigned long)plane_resp->format_type_ptr;
>  		if (copy_to_user(format_ptr,
> @@ -420,7 +428,10 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
>  			return -EFAULT;
>  		}
>  	}
> -	plane_resp->count_format_types = plane->format_count;
> +	if (leased)
> +		plane_resp->count_format_types = plane->format_count;
> +	else
> +		plane_resp->count_format_types = 0;
>  
>  	return 0;
>  }
> @@ -551,6 +562,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
>  	struct drm_plane *plane;
>  	struct drm_crtc *crtc = NULL;
>  	struct drm_framebuffer *fb = NULL;
> +	int ret;
>  
>  	if (!drm_core_check_feature(dev, DRIVER_MODESET))
>  		return -EINVAL;
> @@ -566,6 +578,12 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
>  		return -ENOENT;
>  	}
>  
> +	if ((ret = drm_lease_check(file_priv->master, plane->base.id)) < 0) {
> +		DRM_DEBUG_KMS("Plane lease not held: %d error %d\n",
> +			      plane->base.id, ret);
> +		return ret;
> +	}
> +
>  	if (plane_req->fb_id) {
>  		fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
>  		if (!fb) {
> @@ -687,6 +705,11 @@ static int drm_mode_cursor_common(struct drm_device *dev,
>  		DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id);
>  		return -ENOENT;
>  	}
> +	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0) {
> +		DRM_DEBUG_KMS("CRTC lease not held %d error %d\n",
> +			      crtc->base.id, ret);
> +		goto out;
> +	}
>  
>  	/*
>  	 * If this crtc has a universal cursor plane, call that plane's update
> @@ -785,6 +808,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  	if (!crtc)
>  		return -ENOENT;
>  
> +	if ((ret = drm_lease_check(file_priv->master, crtc->base.id)) < 0)
> +		return ret;
> +
>  	if (crtc->funcs->page_flip_target) {
>  		u32 current_vblank;
>  		int r;
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> index e02adf3e42fd..8f91fc4226e3 100644
> --- a/include/drm/drm_lease.h
> +++ b/include/drm/drm_lease.h
> @@ -48,4 +48,8 @@ static inline int drm_lease_check(struct drm_master *master, int id) {
>  	return 0;
>  }
>  
> +uint32_t drm_lease_filter_crtcs(struct drm_master *master, uint32_t crtcs);
> +
> +uint32_t drm_lease_filter_encoders(struct drm_master *master, uint32_t encoders);
> +
>  #endif /* _DRM_LEASE_H_ */
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index 43460b21d112..07830182598b 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -24,6 +24,7 @@
>  #define __DRM_MODESET_H__
>  
>  #include <linux/kref.h>
> +#include <drm/drm_lease.h>
>  struct drm_object_properties;
>  struct drm_property;
>  struct drm_device;
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/4] drm: Add four ioctls for managing drm mode object leases
  2017-04-01 17:08 ` [PATCH 4/4] drm: Add four ioctls for managing drm mode object leases Keith Packard
@ 2017-04-02 13:23   ` Daniel Vetter
  2017-04-02 16:44     ` Keith Packard
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2017-04-02 13:23 UTC (permalink / raw)
  To: Keith Packard; +Cc: linux-kernel, Dave Airlie, dri-devel

On Sat, Apr 01, 2017 at 10:08:41AM -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 from a particular lessor
> 
> drm_mode_get_lease
> 
> 	List the leased objects for a particular lessee

Should we just use fd for this? Essentially lessors would be required to
keep track of all their pending leases, and just dup() them over to the
client. Would reduce the uapi scope a bit, and if a lessor can't keep
track of it's leases there's a problem.

> drm_mode_change_lease
> 
> 	Adjust the terms of a lease, adding or removing resources or
> 	switching from masking to non-masking.

For this one here we could pass the fd of the lease as the argument. I
also still think that for v1 we just want revoke and otherwise invariant
leases. Makes things simpler.

> 
> This should suffice to at least create, query and manage available
> leases.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c |   4 +
>  drivers/gpu/drm/drm_lease.c | 309 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_lease.h     |  12 ++
>  include/uapi/drm/drm.h      |   4 +
>  include/uapi/drm/drm_mode.h |  78 +++++++++++
>  5 files changed, 407 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index fed22c2b98b6..0f9e3c0fe2ac 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -636,6 +636,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATOMIC, drm_mode_atomic_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATEPROPBLOB, drm_mode_createblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
>  	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROYPROPBLOB, drm_mode_destroyblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> +	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> +	DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> +	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> +	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CHANGE_LEASE, drm_mode_change_lease_ioctl, DRM_ANY_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 782005c7706d..39131860bcd3 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -483,3 +483,312 @@ void drm_lease_destroy(struct drm_master *master)
>  
>  	DRM_DEBUG_LEASE("drm_lease_destroy done %d\n", master->lessee_id);
>  }
> +
> +/**
> + * 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;
> +
> +	object_count = cl->object_count;
> +	idr_init(&leases);
> +
> +	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;
> +
> +		if (copy_from_user(&object_id,
> +				   u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32),
> +				   sizeof (__u32))) {
> +			ret = -EFAULT;
> +			goto bail;
> +		}
> +		DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
> +
> +		ret = idr_alloc(&leases, (void *) 1, object_id, object_id + 1, GFP_KERNEL);
> +		if (ret < 0) {
> +			DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
> +					object_id, ret);
> +			goto bail;
> +		}
> +	}
> +
> +	mutex_lock(&dev->master_mutex);
> +
> +	DRM_DEBUG_LEASE("Creating lease\n");
> +	lessee = drm_lease_create(lessor, cl->mask_lease != 0, &leases);
> +
> +	if (IS_ERR(lessee)) {
> +		ret = PTR_ERR(lessee);
> +		lessee = NULL;
> +		mutex_unlock(&dev->master_mutex);
> +		goto bail;
> +	}
> +
> +	/* Clone the lessor file to create a new file for us */
> +	DRM_DEBUG_LEASE("Allocating lease file\n");
> +	path_get(&lessor_file->f_path);
> +	lessee_file = alloc_file(&lessor_file->f_path,
> +				 lessor_file->f_mode,
> +				 fops_get(lessor_file->f_inode->i_fop));
> +
> +	if (IS_ERR(lessee_file)) {
> +		ret = PTR_ERR(lessee_file);
> +		lessee_file = NULL;
> +		mutex_unlock(&dev->master_mutex);
> +		goto bail;
> +	}
> +
> +	mutex_unlock(&dev->master_mutex);
> +
> +	/* 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 bail;
> +
> +	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;
> +
> +	/* and don't destroy our resources */
> +	fd = -1;
> +	lessee = NULL;
> +	lessee_file = NULL;
> +
> +	ret = 0;
> +
> +bail:
> +
> +	if (lessee_file) {
> +		DRM_DEBUG_LEASE("Freeing lessee file\n");
> +		fput(lessee_file);
> +	}
> +
> +	if (lessee) {
> +		DRM_DEBUG_LEASE("Freeing lessee drm_master\n");
> +		drm_master_put(&lessee);
> +	}
> +
> +	idr_destroy(&leases);
> +
> +	if (fd != -1) {
> +		DRM_DEBUG_LEASE("Freeing unused fd\n");
> +		put_unused_fd(fd);
> +	}
> +
> +	DRM_DEBUG_LEASE("Return %d from drm_mode_create_lease_ioctl\n", ret);
> +	return ret;
> +}
> +
> +/**
> + * drm_mode_list_lessees_ioctl - list lessee ids
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_create_lease
> + * @file_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 *file_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, *lessee;
> +	int count;
> +	int ret = 0;
> +
> +	DRM_DEBUG_LEASE("List lessees for %d\n", arg->lessor_id);
> +
> +	mutex_lock(&dev->master_mutex);
> +
> +	/* Search the tree for the requested drm_master */
> +	lessor = drm_lessee_get(file_priv->master, arg->lessor_id);
> +	if (!lessor) {
> +		DRM_DEBUG_LEASE("No such lessor %d\n", arg->lessor_id);
> +		mutex_unlock(&dev->master_mutex);
> +		return -ENOENT;
> +	}
> +
> +	count = 0;
> +	drm_for_each_lessee(lessee, lessor) {
> +		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;
> +
> +	drm_master_put(&lessor);
> +
> +	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_create_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 *file_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;
> +	struct idr *object_idr;
> +	int count;
> +	void *entry;
> +	int object;
> +	int ret = 0;
> +
> +	DRM_DEBUG_LEASE("get lease for %d\n", arg->lessee_id);
> +
> +	mutex_lock(&dev->master_mutex);
> +
> +	/* Search the tree for the requested drm_master */
> +	lessee = drm_lessee_get(file_priv->master, arg->lessee_id);
> +	if (!lessee) {
> +		mutex_unlock(&dev->master_mutex);
> +		DRM_DEBUG_LEASE("No such lessee %d\n", arg->lessee_id);
> +		return -ENOENT;
> +	}
> +
> +	if (lessee->lessor == NULL)
> +		object_idr = &lessee->dev->mode_config.crtc_idr;
> +	else
> +		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;
> +
> +	drm_master_put(&lessee);
> +
> +	mutex_unlock(&dev->master_mutex);
> +
> +	return ret;
> +}
> +
> +/**
> + * drm_mode_change_lease_ioctl - change the objects in a 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_change_lease_ioctl(struct drm_device *dev,
> +				void *data, struct drm_file *file_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 = file_priv->master;
> +
> +	object_count = cl->object_count;
> +	idr_init(&leases);
> +
> +	DRM_DEBUG_LEASE("Changing existing lease\n");
> +
> +	/* Lookup the mode objects and add their IDs to the lease request */
> +	for (o = 0; o < object_count; o++) {
> +		__u32 object_id;
> +
> +		if (copy_from_user(&object_id,
> +				   u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32),
> +				   sizeof (__u32))) {
> +			return -EFAULT;
> +		}
> +		DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
> +
> +		ret = idr_alloc(&leases, (void *) 1, object_id, object_id + 1, GFP_KERNEL);
> +		if (ret < 0) {
> +			DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
> +					object_id, ret);
> +			return ret;
> +		}
> +	}
> +
> +	mutex_lock(&dev->master_mutex);
> +
> +	DRM_DEBUG_LEASE("Change lease\n");
> +
> +	ret = drm_lease_change(lessor, cl->lessee_id, cl->mask_lease != 0, &leases);
> +
> +	mutex_unlock(&dev->master_mutex);
> +
> +	idr_destroy(&leases);
> +
> +	DRM_DEBUG_LEASE("Return %d from drm_mode_change_lease_ioctl\n", ret);
> +	return ret;
> +}
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> index 8f91fc4226e3..367b57698f53 100644
> --- a/include/drm/drm_lease.h
> +++ b/include/drm/drm_lease.h
> @@ -52,4 +52,16 @@ uint32_t drm_lease_filter_crtcs(struct drm_master *master, uint32_t crtcs);
>  
>  uint32_t drm_lease_filter_encoders(struct drm_master *master, uint32_t encoders);
>  
> +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_change_lease_ioctl(struct drm_device *dev,
> +				void *data, struct drm_file *file_priv);

Non-driver functions please into drm-internal.h, we don't want to let
drivers even see this :-)

Cheers, Daniel

> +
>  #endif /* _DRM_LEASE_H_ */
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b2c52843bc70..8c62da23d7a3 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -813,6 +813,10 @@ extern "C" {
>  #define DRM_IOCTL_MODE_ATOMIC		DRM_IOWR(0xBC, struct drm_mode_atomic)
>  #define DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, struct drm_mode_create_blob)
>  #define DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, struct drm_mode_destroy_blob)
> +#define DRM_IOCTL_MODE_CREATE_LEASE	DRM_IOWR(0xBF, struct drm_mode_create_lease)
> +#define DRM_IOCTL_MODE_LIST_LESSEES	DRM_IOWR(0xC0, struct drm_mode_list_lessees)
> +#define DRM_IOCTL_MODE_GET_LEASE	DRM_IOWR(0xC1, struct drm_mode_get_lease)
> +#define DRM_IOCTL_MODE_CHANGE_LEASE	DRM_IOWR(0xC2, struct drm_mode_change_lease)
>  
>  /**
>   * Device specific ioctls should only be in their respective headers
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index ce7efe2e8a5e..e6669ada3b10 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -681,6 +681,84 @@ 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;
> +	/** whether to hide the leased objects from the lessor */
> +	__u32 mask_lease;
> +
> +	/** 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 {
> +
> +	/** Identifier of the lessor's lessee_id (0 for owner) */
> +	__u32 lessor_id;
> +
> +	/** 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;
> +
> +	/** Pointer to lessees.
> +	 * pointer to __u64 array of lessee ids
> +	 */
> +	__u64 lessees_ptr;
> +};
> +
> +/**
> + * Get leased objects for a lessee
> + */
> +struct drm_mode_get_lease {
> +	/** Identifier of the lessee (0 for owner) */
> +	__u32 lessee_id;
> +
> +	/** 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;
> +
> +	/** Pointer to objects.
> +	 * pointer to __u32 array of object ids
> +	 */
> +	__u64 objects_ptr;
> +};
> +
> +/**
> + * Change resources leased to another drm_master
> + */
> +struct drm_mode_change_lease {
> +	/** Pointer to array of object ids (__u32) */
> +	__u64 object_ids;
> +	/** Number of object ids */
> +	__u32 object_count;
> +	/** unique identifier for lessee. */
> +	__u32 lessee_id;
> +	/** whether to hide the leased objects from the lessor */
> +	__u32 mask_lease;
> +};
> +
>  #if defined(__cplusplus)
>  }
>  #endif
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/4] drm: Add drm_object lease infrastructure
  2017-04-01 17:08 ` [PATCH 2/4] drm: Add drm_object lease infrastructure Keith Packard
@ 2017-04-02 13:38   ` Daniel Vetter
  2017-04-02 16:31     ` Keith Packard
  2017-04-02 17:51   ` Daniel Vetter
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2017-04-02 13:38 UTC (permalink / raw)
  To: Keith Packard; +Cc: linux-kernel, Dave Airlie, dri-devel

On Sat, Apr 01, 2017 at 10:08:39AM -0700, Keith Packard wrote:
> This provides new data structures to hold "lease" information about
> drm mode setting objects, and provides for creating new drm_masters
> which have access to a subset of the available drm resources.
> 
> An 'owner' is a drm_master which is not leasing the objects from
> another drm_master, and hence 'owns' them. This sits at the top of a
> tree of drm_masters.
> 
> A 'lessee' is a drm_master which is leasing objects from some other
> drm_master. Each lessee holds the set of objects which it is leasing
> from the lessor.
> 
> A 'lessor' is a drm_master which is leasing objects to another
> drm_master.
> 
> The set of objects any drm_master 'controls' is limited to the set of
> objects it leases (for lessees) or all objects (for owners),
> optionally minus the set of objects it has leased to other
> drm_masters.

Still not sure we want to restrict objects on the lessor side. Feels like
unecessary complexity (i.e. more bugs in kernel, that's never good), and
at best only needed for lessors who can't keep track of stuff. I'm also
not sure whether we really want sub-leases in v1, that's easy to add later
on, but for now just complicates stuff. Main compositor should be a full
master, VR can be the first lease level, we don't need more I think for
now?

> Objects not controlled by a drm_master cannot be modified through the
> various state manipulating ioctls, and any state reported back to user
> space will be edited to make them appear idle and/or unusable. For
> instance, connectors always report 'disconnected', while encoders
> report no possible crtcs or clones.
> 
> The full list of lessees leasing objects from an owner (either
> directly, or indirectly through another lessee), can be searched from
> an idr in the drm_master of the owner.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

[snip]

> diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
> index 610223b0481b..e0e2af09d3af 100644
> --- a/include/drm/drm_auth.h
> +++ b/include/drm/drm_auth.h
> @@ -50,10 +50,38 @@ struct drm_master {
>  	struct idr magic_map;
>  	struct drm_lock_data lock;
>  	void *driver_priv;
> +
> +	/* Tree of display resource leases, each of which is a drm_master struct
> +	 * All of these get activated simultaneously, so drm_device master points

&drm_device.master to do a reference in kernel-doc. Please feed this to
kernel-doc in general and make sure the links all point at the right
stuff, and it's all parsed.

Cheers, Daniel


> +	 * at the top of the tree (for which lessor is NULL)
> +	 */
> +
> +	/** Lease holder */

	/** @lessor: Lease holder. */

> +	struct drm_master *lessor;
> +
> +	/** id for lessees. Owners always have id 0 */
> +	int	lessee_id;
> +
> +	/** other lessees of the same master */
> +	struct list_head lessee_list;
> +
> +	/** drm_masters leasing from this one */
> +	struct list_head lessees;
> +
> +	/** Objects leased to this drm_master. */
> +	struct idr leases;
> +
> +	/** All lessees under this owner (only used where lessor == NULL) */
> +	struct idr lessee_idr;
> +
> +	/** Indicates that the leased objects should be hidden from the lessor */
> +	bool mask_lease;
>  };
>  
>  struct drm_master *drm_master_get(struct drm_master *master);
>  void drm_master_put(struct drm_master **master);
>  bool drm_is_current_master(struct drm_file *fpriv);
>  
> +struct drm_master *drm_master_create(struct drm_device *dev);
> +
>  #endif
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> new file mode 100644
> index 000000000000..e02adf3e42fd
> --- /dev/null
> +++ b/include/drm/drm_lease.h
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright © 2017 Keith Packard <keithp@keithp.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +#ifndef _DRM_LEASE_H_
> +#define _DRM_LEASE_H_
> +
> +struct drm_file;
> +struct drm_device;
> +
> +struct drm_master *drm_lease_owner(struct drm_master *master);
> +
> +struct drm_master *drm_lessee_find(struct drm_master *top, int lessee_id);
> +
> +void drm_lease_destroy(struct drm_master *lessee);
> +
> +struct drm_mode_object *drm_lease_find(struct drm_master *master, int id);
> +
> +/**
> + * drm_lease_check - check drm_mode_object lease status
> + * @master: the drm_master
> + * @id: the object id
> + *
> + * Checks if the specified master holds a lease on the object
> + * and also whether it has been leased to some lessee of the
> + * specified master. Return value:
> + *
> + *	0		'master' holds a lease (or owns) and has not leased
> + *	-EACCESS	Some other master holds the lease
> + *	-ENOENT		'id' is not a valid DRM object for this device
> + *	-EBUSY		'master' holds lease, but has sub-leased
> + */
> +
> +static inline int drm_lease_check(struct drm_master *master, int id) {
> +	struct drm_mode_object *object = drm_lease_find(master, id);
> +	if (IS_ERR(object))
> +		return PTR_ERR(object);
> +	return 0;
> +}
> +
> +#endif /* _DRM_LEASE_H_ */
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/4] drm: Add drm_object lease infrastructure
  2017-04-02 13:38   ` Daniel Vetter
@ 2017-04-02 16:31     ` Keith Packard
  2017-04-10  5:16       ` Michel Dänzer
  0 siblings, 1 reply; 22+ messages in thread
From: Keith Packard @ 2017-04-02 16:31 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-kernel, Dave Airlie, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> Still not sure we want to restrict objects on the lessor side. Feels like
> unecessary complexity (i.e. more bugs in kernel, that's never good), and
> at best only needed for lessors who can't keep track of stuff.

It's been useful when hacking existing code, and will help catch
application bugs. Limiting access to what you actually need always seems
like good practice to me.

> I'm also not sure whether we really want sub-leases in v1, that's easy
> to add later on, but for now just complicates stuff. Main compositor
> should be a full master, VR can be the first lease level, we don't
> need more I think for now?

We've discussed how leases might be used to implement multi-user
support, so offering sub-leases means that environment could also
support leasing resources out from the users session.

We also just don't know how useful it might be until we explore the
space a bit more. Given that it takes years to get new features into
distributions, I tend to error on the side of generality.

I think a key requirement for acceptance would be a set of robust tests,
something I haven't started writing yet.

>> +	/* Tree of display resource leases, each of which is a drm_master struct
>> +	 * All of these get activated simultaneously, so drm_device master points
>
> &drm_device.master to do a reference in kernel-doc. Please feed this to
> kernel-doc in general and make sure the links all point at the right
> stuff, and it's all parsed.

Thanks, will do.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths
  2017-04-02 13:19   ` Daniel Vetter
@ 2017-04-02 16:37     ` Keith Packard
  2017-04-03  7:49       ` Daniel Vetter
  2017-04-10  1:06     ` Keith Packard
  1 sibling, 1 reply; 22+ messages in thread
From: Keith Packard @ 2017-04-02 16:37 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-kernel, Dave Airlie, dri-devel

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> I think it'd be good if we could consolidate all the lease checking into
> drm_mode_object_find (respectively __drm_mode_object_find). We'd need to
> wire up the fpriv to be able to do that, but we could upstream that patch
> right away before anything else. That should take care of most of the
> checks in this patch here.

That's a good idea.

> There's a few things on top:
> - filtering the various bitmasks. I think you have most, but we could
>   perhaps upstream the helpers for these.

Yeah, would be nice to get hooks in place soon to avoid rebase
adventures later. I guess that would involve shipping a stub drm_lease.h
for now?

> - filtering object lists (essentially getresources and getplanes ioctls).
> - filtering implicit objects in the legacy ioctl. E.g. page_flip done
>   through atomic doesn't just need the CRTC id, but also the id of the
>   primary plane plus of the FB_ID atomic property. Similarly for all the
>   other legacy ioctls. I think we want to make sure there's no difference
>   here in behaviour.

Oh, all of the implicit resource access from the legacy ioctls. Yeah,
that will take a bit of research to identify all of them.

> Especially for the last one it might be simplest to outright disallow all
> legacy ioctl and require that sub-drm_master nodes only get access to the
> read-only GET* ioctl (they get that anyway, even when they're not the
> current master), plus atomic. Makes it a _lot_ easier to implement.
> Downside is that amdgpu _really_ needs to land atomic asap :-)

I'd like to avoid that particular dependency as amdgpu is something of a
requirement for this particular project...

I'll get started fixing the lease checking stuff to try and centralize it.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/4] drm: Add four ioctls for managing drm mode object leases
  2017-04-02 13:23   ` Daniel Vetter
@ 2017-04-02 16:44     ` Keith Packard
  0 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-02 16:44 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-kernel, Dave Airlie, dri-devel

[-- Attachment #1: Type: text/plain, Size: 2194 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> Should we just use fd for this? Essentially lessors would be required to
> keep track of all their pending leases, and just dup() them over to the
> client. Would reduce the uapi scope a bit, and if a lessor can't keep
> track of it's leases there's a problem.

Right now, the lease terminates when the fd closes. If the lessor is
holding the fd open, it would have to have some out-of-band indication
that the lessee was done so that it could close its copy of the fd.

However, it's not clear what value these query APIs have for the lessor
in any case; surely it can remember what resources are in each lease
without needing a kernel API. For the lessee, this API provides a way of
learning about changes to the lease. That would mean getting rid of the
lessee id in the requests, which seems good.

I added them mostly to make checking the code easier during development;
it caught a nasty bug where I was smashing the lessor file_priv with the
lessee due to poor choice of variable names in the kernel code.

I was thinking this morning that the lessee would like to know not only
the ID but also the type of each resources so that it could figure out
how to use the lease.

I'll rewrite these to only return the resources for the fd provided, but
to return both id and type.

For listing lessees, I don't see value in allowing the lessor to look at
sub-leases made by the lessee, so I'll remove the lessor id from that
request and have it just return the list of lessees for the current lessor.

> For this one here we could pass the fd of the lease as the argument. I
> also still think that for v1 we just want revoke and otherwise invariant
> leases. Makes things simpler.

Again, that means that lessor would need to hold the lease fd open,
which makes it hard to know when the lessee is done. However, there's no
reason the lessor should be in control of the lessee's sub-leases, so
again, I propose to restrict the scope of lessees to those directly
under the lessor.

> Non-driver functions please into drm-internal.h, we don't want to let
> drivers even see this :-)

Thanks!

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/4] drm: Add drm_object lease infrastructure
  2017-04-01 17:08 ` [PATCH 2/4] drm: Add drm_object lease infrastructure Keith Packard
  2017-04-02 13:38   ` Daniel Vetter
@ 2017-04-02 17:51   ` Daniel Vetter
  2017-04-02 19:59     ` Keith Packard
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2017-04-02 17:51 UTC (permalink / raw)
  To: Keith Packard; +Cc: linux-kernel, Dave Airlie, dri-devel

On Sat, Apr 01, 2017 at 10:08:39AM -0700, Keith Packard wrote:
> +	BUG_ON(__mutex_owner(&master->dev->mode_config.idr_mutex) != current);

Forgot to reply on this:

lockdep_assert_held + enable lockdep.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/4] drm: Add drm_object lease infrastructure
  2017-04-02 17:51   ` Daniel Vetter
@ 2017-04-02 19:59     ` Keith Packard
  0 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-02 19:59 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-kernel, Dave Airlie, dri-devel

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Sat, Apr 01, 2017 at 10:08:39AM -0700, Keith Packard wrote:
>> +	BUG_ON(__mutex_owner(&master->dev->mode_config.idr_mutex) != current);
>
> Forgot to reply on this:
>
> lockdep_assert_held + enable lockdep.

Thanks. Will fix.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths
  2017-04-02 16:37     ` Keith Packard
@ 2017-04-03  7:49       ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2017-04-03  7:49 UTC (permalink / raw)
  To: Keith Packard; +Cc: Daniel Vetter, linux-kernel, Dave Airlie, dri-devel

On Sun, Apr 02, 2017 at 09:37:16AM -0700, Keith Packard wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
> > I think it'd be good if we could consolidate all the lease checking into
> > drm_mode_object_find (respectively __drm_mode_object_find). We'd need to
> > wire up the fpriv to be able to do that, but we could upstream that patch
> > right away before anything else. That should take care of most of the
> > checks in this patch here.
> 
> That's a good idea.
> 
> > There's a few things on top:
> > - filtering the various bitmasks. I think you have most, but we could
> >   perhaps upstream the helpers for these.
> 
> Yeah, would be nice to get hooks in place soon to avoid rebase
> adventures later. I guess that would involve shipping a stub drm_lease.h
> for now?

I think we should see how far wiring drm_file *fpriv through the lookup
functions gets us, but yeah we probably need a stub drm_lease.h for
filtering. For the interface maybe pass a drm_file *fpriv for all the
filtering functions, and deref into fpriv->master or whatever on the
implementation. That gives us all the freedom to filter however we want
really.

> > - filtering object lists (essentially getresources and getplanes ioctls).
> > - filtering implicit objects in the legacy ioctl. E.g. page_flip done
> >   through atomic doesn't just need the CRTC id, but also the id of the
> >   primary plane plus of the FB_ID atomic property. Similarly for all the
> >   other legacy ioctls. I think we want to make sure there's no difference
> >   here in behaviour.
> 
> Oh, all of the implicit resource access from the legacy ioctls. Yeah,
> that will take a bit of research to identify all of them.
> 
> > Especially for the last one it might be simplest to outright disallow all
> > legacy ioctl and require that sub-drm_master nodes only get access to the
> > read-only GET* ioctl (they get that anyway, even when they're not the
> > current master), plus atomic. Makes it a _lot_ easier to implement.
> > Downside is that amdgpu _really_ needs to land atomic asap :-)
> 
> I'd like to avoid that particular dependency as amdgpu is something of a
> requirement for this particular project...

Figured so :-) Otoh I've heard that atomic for amdgpu is happening for
real now. Although Harry told me I need to wait a few more weeks before
they're ready to present the rewritten stuff and get my feedback ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths
  2017-04-02 13:19   ` Daniel Vetter
  2017-04-02 16:37     ` Keith Packard
@ 2017-04-10  1:06     ` Keith Packard
  1 sibling, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-10  1:06 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: linux-kernel, Dave Airlie, dri-devel

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> I think it'd be good if we could consolidate all the lease checking into
> drm_mode_object_find (respectively __drm_mode_object_find). We'd need to
> wire up the fpriv to be able to do that, but we could upstream that patch
> right away before anything else. That should take care of most of the
> checks in this patch here.

Hrm. Without some major changes, the effect of this will be to have the
lessee see only the objects which it holds a lease on. I think that's OK
as the lessor will be doing object filtering for its clients, however
it's not what Dave and I discussed, so I just want to make him aware
that the kernel is going to be simpler than originally planned. We had
originally discussed having the lessee see the un-leased objects, but to
have them reported with different status.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/4] drm: Add drm_object lease infrastructure
  2017-04-02 16:31     ` Keith Packard
@ 2017-04-10  5:16       ` Michel Dänzer
  0 siblings, 0 replies; 22+ messages in thread
From: Michel Dänzer @ 2017-04-10  5:16 UTC (permalink / raw)
  To: Keith Packard, Daniel Vetter; +Cc: Dave Airlie, linux-kernel, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1107 bytes --]

On 03/04/17 01:31 AM, Keith Packard wrote:
> Daniel Vetter <daniel@ffwll.ch> writes:
> 
>> I'm also not sure whether we really want sub-leases in v1, that's easy
>> to add later on, but for now just complicates stuff. Main compositor
>> should be a full master, VR can be the first lease level, we don't
>> need more I think for now?
> 
> We've discussed how leases might be used to implement multi-user
> support, so offering sub-leases means that environment could also
> support leasing resources out from the users session.
> 
> We also just don't know how useful it might be until we explore the
> space a bit more.

It should only be added upstream once it's clear that it's useful.


> Given that it takes years to get new features into distributions, I
> tend to error on the side of generality.

Why would it take years? Distros seem to generally use the latest
upstream kernel release available at release/freeze.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 224 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 0/5] drm: Add mode resource leasing [v2]
  2017-04-01 17:08 [PATCH 0/4] drm: Add mode resource leasing Keith Packard
                   ` (3 preceding siblings ...)
  2017-04-01 17:08 ` [PATCH 4/4] drm: Add four ioctls for managing drm mode object leases Keith Packard
@ 2017-04-29  6:06 ` Keith Packard
  2017-04-29  6:06   ` [PATCH 1/5] drm: Pass struct drm_file * to __drm_mode_object_find Keith Packard
                     ` (4 more replies)
  4 siblings, 5 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-29  6:06 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel

Here's a second try at mode resource leasing. The differences from v1
are mostly deleting functionality that isn't currently useful.

There are no more sub-leases; there's the owner, the owner is the only
lessor and so the only one who can create leases and hand those out.

The lessor can now manipulate all of the resources without
restriction. It's up to user space to enforce whatever access control
it wants.

The lessee can no longer see any resources other than those in the
lease. This was a bit tricky as there are various indices into the
reported arrays of objects in the form of masks.

The query ioctls no longer take lessor or lessee ids; you can query
the leases you have granted or query the list of resources in your own
lease. The X server uses the former to figure out when lessees close
their DRM file and terminate a lease. As I write this, I'm wondering
if that latter operation is actually useful though. If not, we can rip
that ioctl out easily enough.

As suggested by Daniel Vetter, the first patch changes the API to
__drm_mode_object_find to include a relevant struct drm_file * pointer
so that function can perform any necessary per-file access
control. When there isn't a suitable pointer, NULL is passed instead,
which is a proxy for the origin DRM master. For leases, this means
that no lease permissions checking is done. This is why there are now
5 patches instead of the original 4.

Thanks again to everyone who commented on the mailing list or on IRC;
I'm pretty happy with the current functionality; I've got kmscube
leasing resources from the X server this evening.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/5] drm: Pass struct drm_file * to  __drm_mode_object_find
  2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
@ 2017-04-29  6:06   ` Keith Packard
  2017-04-29  6:06   ` [PATCH 2/5] drm: Add new LEASE debug level Keith Packard
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-29  6:06 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel

This will allow __drm_mode_object_file to be extended to perform
access control checks based on the file in use.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c   | 16 ++++++++--------
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c         |  4 ++--
 drivers/gpu/drm/ast/ast_mode.c                   |  2 +-
 drivers/gpu/drm/bochs/bochs_kms.c                |  2 +-
 drivers/gpu/drm/cirrus/cirrus_mode.c             |  2 +-
 drivers/gpu/drm/drm_atomic.c                     |  8 ++++----
 drivers/gpu/drm/drm_atomic_helper.c              |  2 +-
 drivers/gpu/drm/drm_color_mgmt.c                 |  4 ++--
 drivers/gpu/drm/drm_connector.c                  |  2 +-
 drivers/gpu/drm/drm_crtc.c                       |  8 ++++----
 drivers/gpu/drm/drm_crtc_internal.h              |  1 +
 drivers/gpu/drm/drm_encoder.c                    |  2 +-
 drivers/gpu/drm/drm_framebuffer.c                |  9 +++++----
 drivers/gpu/drm/drm_mode_object.c                | 10 ++++++----
 drivers/gpu/drm/drm_plane.c                      | 14 +++++++-------
 drivers/gpu/drm/drm_property.c                   |  6 +++---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c             |  2 +-
 drivers/gpu/drm/i915/intel_overlay.c             |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c              |  2 +-
 drivers/gpu/drm/mgag200/mgag200_mode.c           |  2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c      |  4 ++--
 drivers/gpu/drm/radeon/r100.c                    |  2 +-
 drivers/gpu/drm/radeon/r600_cs.c                 |  2 +-
 drivers/gpu/drm/radeon/radeon_connectors.c       | 16 ++++++++--------
 drivers/gpu/drm/udl/udl_connector.c              |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c            |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c              |  2 +-
 include/drm/drm_connector.h                      |  3 ++-
 include/drm/drm_crtc.h                           |  5 +++--
 include/drm/drm_encoder.h                        |  3 ++-
 include/drm/drm_framebuffer.h                    |  1 +
 include/drm/drm_mode_object.h                    |  2 ++
 include/drm/drm_plane.h                          |  3 ++-
 include/drm/drm_property.h                       |  3 ++-
 35 files changed, 83 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
index 8d1cf2d3e663..a6c17353e57d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
@@ -231,7 +231,7 @@ amdgpu_connector_update_scratch_regs(struct drm_connector *connector,
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev,
+		encoder = drm_encoder_find(connector->dev, NULL,
 					connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
@@ -256,7 +256,7 @@ amdgpu_connector_find_encoder(struct drm_connector *connector,
 	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
 		if (connector->encoder_ids[i] == 0)
 			break;
-		encoder = drm_encoder_find(connector->dev,
+		encoder = drm_encoder_find(connector->dev, NULL,
 					connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
@@ -374,7 +374,7 @@ amdgpu_connector_best_single_encoder(struct drm_connector *connector)
 
 	/* pick the encoder ids */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
@@ -1079,7 +1079,7 @@ amdgpu_connector_dvi_detect(struct drm_connector *connector, bool force)
 			if (connector->encoder_ids[i] == 0)
 				break;
 
-			encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]);
+			encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
 			if (!encoder)
 				continue;
 
@@ -1136,7 +1136,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]);
+		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
 
@@ -1155,7 +1155,7 @@ amdgpu_connector_dvi_encoder(struct drm_connector *connector)
 	/* then check use digitial */
 	/* pick the first one */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
@@ -1296,7 +1296,7 @@ u16 amdgpu_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev,
+		encoder = drm_encoder_find(connector->dev, NULL,
 					connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
@@ -1325,7 +1325,7 @@ static bool amdgpu_connector_encoder_is_hbr2(struct drm_connector *connector)
 	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
 		if (connector->encoder_ids[i] == 0)
 			break;
-		encoder = drm_encoder_find(connector->dev,
+		encoder = drm_encoder_find(connector->dev, NULL,
 					connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index e9a176891e13..2f5e30d00b1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -362,7 +362,7 @@ dce_virtual_encoder(struct drm_connector *connector)
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]);
+		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
 
@@ -372,7 +372,7 @@ dce_virtual_encoder(struct drm_connector *connector)
 
 	/* pick the first one */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index e26c98f51eb4..6871d8b22b61 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -690,7 +690,7 @@ static struct drm_encoder *ast_best_single_encoder(struct drm_connector *connect
 	int enc_id = connector->encoder_ids[0];
 	/* pick the encoder ids */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index d5e63eff357b..d728c8015fbb 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -212,7 +212,7 @@ bochs_connector_best_encoder(struct drm_connector *connector)
 	int enc_id = connector->encoder_ids[0];
 	/* pick the encoder ids */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 9a4a27c1afd2..3a0282141edd 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -494,7 +494,7 @@ static struct drm_encoder *cirrus_connector_best_encoder(struct drm_connector
 	int enc_id = connector->encoder_ids[0];
 	/* pick the encoder ids */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index fdfb1ec17e66..06a3a309b733 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -738,7 +738,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
 	struct drm_mode_config *config = &dev->mode_config;
 
 	if (property == config->prop_fb_id) {
-		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val);
+		struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, NULL, val);
 		drm_atomic_set_fb_for_plane(state, fb);
 		if (fb)
 			drm_framebuffer_unreference(fb);
@@ -754,7 +754,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane,
 			return -EINVAL;
 
 	} else if (property == config->prop_crtc_id) {
-		struct drm_crtc *crtc = drm_crtc_find(dev, val);
+		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val);
 		return drm_atomic_set_crtc_for_plane(state, crtc);
 	} else if (property == config->prop_crtc_x) {
 		state->crtc_x = U642I64(val);
@@ -1079,7 +1079,7 @@ int drm_atomic_connector_set_property(struct drm_connector *connector,
 	struct drm_mode_config *config = &dev->mode_config;
 
 	if (property == config->prop_crtc_id) {
-		struct drm_crtc *crtc = drm_crtc_find(dev, val);
+		struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val);
 		return drm_atomic_set_crtc_for_connector(state, crtc);
 	} else if (property == config->dpms_property) {
 		/* setting DPMS property requires special handling, which
@@ -2128,7 +2128,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
 			goto out;
 		}
 
-		obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY);
+		obj = drm_mode_object_find(dev, file_priv, obj_id, DRM_MODE_OBJECT_ANY);
 		if (!obj) {
 			ret = -ENOENT;
 			goto out;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 4594477dee00..b660175136ed 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2901,7 +2901,7 @@ struct drm_encoder *
 drm_atomic_helper_best_encoder(struct drm_connector *connector)
 {
 	WARN_ON(connector->encoder_ids[1]);
-	return drm_encoder_find(connector->dev, connector->encoder_ids[0]);
+	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
 }
 EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
 
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 6543ebde501a..a9ef1e3624c0 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -200,7 +200,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	drm_modeset_lock_all(dev);
-	crtc = drm_crtc_find(dev, crtc_lut->crtc_id);
+	crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id);
 	if (!crtc) {
 		ret = -ENOENT;
 		goto out;
@@ -272,7 +272,7 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	drm_modeset_lock_all(dev);
-	crtc = drm_crtc_find(dev, crtc_lut->crtc_id);
+	crtc = drm_crtc_find(dev, file_priv, crtc_lut->crtc_id);
 	if (!crtc) {
 		ret = -ENOENT;
 		goto out;
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 7a7019ac9388..670c20d5660c 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1087,7 +1087,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 
 	mutex_lock(&dev->mode_config.mutex);
 
-	connector = drm_connector_lookup(dev, out_resp->connector_id);
+	connector = drm_connector_lookup(dev, file_priv, out_resp->connector_id);
 	if (!connector) {
 		ret = -ENOENT;
 		goto out_unlock;
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e75f62cd8a65..ff06284b5d48 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -351,7 +351,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	crtc = drm_crtc_find(dev, crtc_resp->crtc_id);
+	crtc = drm_crtc_find(dev, file_priv, crtc_resp->crtc_id);
 	if (!crtc)
 		return -ENOENT;
 
@@ -521,7 +521,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 		return -ERANGE;
 
 	drm_modeset_lock_all(dev);
-	crtc = drm_crtc_find(dev, crtc_req->crtc_id);
+	crtc = drm_crtc_find(dev, file_priv, crtc_req->crtc_id);
 	if (!crtc) {
 		DRM_DEBUG_KMS("Unknown CRTC ID %d\n", crtc_req->crtc_id);
 		ret = -ENOENT;
@@ -542,7 +542,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 			/* Make refcounting symmetric with the lookup path. */
 			drm_framebuffer_reference(fb);
 		} else {
-			fb = drm_framebuffer_lookup(dev, crtc_req->fb_id);
+			fb = drm_framebuffer_lookup(dev, file_priv, crtc_req->fb_id);
 			if (!fb) {
 				DRM_DEBUG_KMS("Unknown FB ID%d\n",
 						crtc_req->fb_id);
@@ -627,7 +627,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
 				goto out;
 			}
 
-			connector = drm_connector_lookup(dev, out_id);
+			connector = drm_connector_lookup(dev, file_priv, out_id);
 			if (!connector) {
 				DRM_DEBUG_KMS("Connector id %d unknown\n",
 						out_id);
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index cdf6860c9d22..627b297cfe4e 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -108,6 +108,7 @@ void drm_mode_object_register(struct drm_device *dev,
 int drm_mode_object_get(struct drm_device *dev,
 			struct drm_mode_object *obj, uint32_t obj_type);
 struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
+					       struct drm_file *file_priv,
 					       uint32_t id, uint32_t type);
 void drm_mode_object_unregister(struct drm_device *dev,
 				struct drm_mode_object *object);
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 992879f15f23..dbaedd4e12e6 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -205,7 +205,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	encoder = drm_encoder_find(dev, enc_resp->encoder_id);
+	encoder = drm_encoder_find(dev, file_priv, enc_resp->encoder_id);
 	if (!encoder)
 		return -ENOENT;
 
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index cbf0c893f426..97c579da3278 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -357,7 +357,7 @@ int drm_mode_rmfb(struct drm_device *dev,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	fb = drm_framebuffer_lookup(dev, *id);
+	fb = drm_framebuffer_lookup(dev, file_priv, *id);
 	if (!fb)
 		return -ENOENT;
 
@@ -426,7 +426,7 @@ int drm_mode_getfb(struct drm_device *dev,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	fb = drm_framebuffer_lookup(dev, r->fb_id);
+	fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
 	if (!fb)
 		return -ENOENT;
 
@@ -491,7 +491,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	fb = drm_framebuffer_lookup(dev, r->fb_id);
+	fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
 	if (!fb)
 		return -ENOENT;
 
@@ -661,12 +661,13 @@ EXPORT_SYMBOL(drm_framebuffer_init);
  * again, using @drm_framebuffer_unreference.
  */
 struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
+					       struct drm_file *file_priv,
 					       uint32_t id)
 {
 	struct drm_mode_object *obj;
 	struct drm_framebuffer *fb = NULL;
 
-	obj = __drm_mode_object_find(dev, id, DRM_MODE_OBJECT_FB);
+	obj = __drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_FB);
 	if (obj)
 		fb = obj_to_fb(obj);
 	return fb;
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 9f17085b1fdd..daddc7640139 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -108,6 +108,7 @@ void drm_mode_object_unregister(struct drm_device *dev,
 }
 
 struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
+					       struct drm_file *file_priv,
 					       uint32_t id, uint32_t type)
 {
 	struct drm_mode_object *obj = NULL;
@@ -130,7 +131,7 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
 
 /**
  * drm_mode_object_find - look up a drm object with static lifetime
- * @dev: drm device
+ * @file_priv: drm file
  * @id: id of the mode object
  * @type: type of the mode object
  *
@@ -139,11 +140,12 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
  * by callind drm_mode_object_unreference().
  */
 struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
+		struct drm_file *file_priv,
 		uint32_t id, uint32_t type)
 {
 	struct drm_mode_object *obj = NULL;
 
-	obj = __drm_mode_object_find(dev, id, type);
+	obj = __drm_mode_object_find(dev, file_priv, id, type);
 	return obj;
 }
 EXPORT_SYMBOL(drm_mode_object_find);
@@ -350,7 +352,7 @@ int drm_mode_obj_get_properties_ioctl(struct drm_device *dev, void *data,
 
 	drm_modeset_lock_all(dev);
 
-	obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
+	obj = drm_mode_object_find(dev, file_priv, arg->obj_id, arg->obj_type);
 	if (!obj) {
 		ret = -ENOENT;
 		goto out;
@@ -398,7 +400,7 @@ int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
 
 	drm_modeset_lock_all(dev);
 
-	arg_obj = drm_mode_object_find(dev, arg->obj_id, arg->obj_type);
+	arg_obj = drm_mode_object_find(dev, file_priv, arg->obj_id, arg->obj_type);
 	if (!arg_obj) {
 		ret = -ENOENT;
 		goto out;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 62b98f386fd1..819f74616f34 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -387,7 +387,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		return -EINVAL;
 
-	plane = drm_plane_find(dev, plane_resp->plane_id);
+	plane = drm_plane_find(dev, file_priv, plane_resp->plane_id);
 	if (!plane)
 		return -ENOENT;
 
@@ -559,7 +559,7 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 	 * First, find the plane, crtc, and fb objects.  If not available,
 	 * we don't bother to call the driver.
 	 */
-	plane = drm_plane_find(dev, plane_req->plane_id);
+	plane = drm_plane_find(dev, file_priv, plane_req->plane_id);
 	if (!plane) {
 		DRM_DEBUG_KMS("Unknown plane ID %d\n",
 			      plane_req->plane_id);
@@ -567,14 +567,14 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 	}
 
 	if (plane_req->fb_id) {
-		fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
+		fb = drm_framebuffer_lookup(dev, file_priv, plane_req->fb_id);
 		if (!fb) {
 			DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
 				      plane_req->fb_id);
 			return -ENOENT;
 		}
 
-		crtc = drm_crtc_find(dev, plane_req->crtc_id);
+		crtc = drm_crtc_find(dev, file_priv, plane_req->crtc_id);
 		if (!crtc) {
 			DRM_DEBUG_KMS("Unknown crtc ID %d\n",
 				      plane_req->crtc_id);
@@ -682,7 +682,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 	if (!req->flags || (~DRM_MODE_CURSOR_FLAGS & req->flags))
 		return -EINVAL;
 
-	crtc = drm_crtc_find(dev, req->crtc_id);
+	crtc = drm_crtc_find(dev, file_priv, req->crtc_id);
 	if (!crtc) {
 		DRM_DEBUG_KMS("Unknown CRTC ID %d\n", req->crtc_id);
 		return -ENOENT;
@@ -781,7 +781,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip)
 		return -EINVAL;
 
-	crtc = drm_crtc_find(dev, page_flip->crtc_id);
+	crtc = drm_crtc_find(dev, file_priv, page_flip->crtc_id);
 	if (!crtc)
 		return -ENOENT;
 
@@ -834,7 +834,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
-	fb = drm_framebuffer_lookup(dev, page_flip->fb_id);
+	fb = drm_framebuffer_lookup(dev, file_priv, page_flip->fb_id);
 	if (!fb) {
 		ret = -ENOENT;
 		goto out;
diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c
index 24be69d29964..984ad6cff242 100644
--- a/drivers/gpu/drm/drm_property.c
+++ b/drivers/gpu/drm/drm_property.c
@@ -452,7 +452,7 @@ int drm_mode_getproperty_ioctl(struct drm_device *dev,
 		return -EINVAL;
 
 	drm_modeset_lock_all(dev);
-	property = drm_property_find(dev, out_resp->prop_id);
+	property = drm_property_find(dev, file_priv, out_resp->prop_id);
 	if (!property) {
 		ret = -ENOENT;
 		goto done;
@@ -648,7 +648,7 @@ struct drm_property_blob *drm_property_lookup_blob(struct drm_device *dev,
 	struct drm_mode_object *obj;
 	struct drm_property_blob *blob = NULL;
 
-	obj = __drm_mode_object_find(dev, id, DRM_MODE_OBJECT_BLOB);
+	obj = __drm_mode_object_find(dev, NULL, id, DRM_MODE_OBJECT_BLOB);
 	if (obj)
 		blob = obj_to_blob(obj);
 	return blob;
@@ -888,7 +888,7 @@ bool drm_property_change_valid_get(struct drm_property *property,
 		if (value == 0)
 			return true;
 
-		*ref = __drm_mode_object_find(property->dev, value,
+		*ref = __drm_mode_object_find(property->dev, NULL, value,
 					      property->values[0]);
 		return *ref != NULL;
 	}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 12a18557c5fd..ffb7a731572e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -36,7 +36,7 @@ static int hibmc_connector_mode_valid(struct drm_connector *connector,
 static struct drm_encoder *
 hibmc_connector_best_encoder(struct drm_connector *connector)
 {
-	return drm_encoder_find(connector->dev, connector->encoder_ids[0]);
+	return drm_encoder_find(connector->dev, NULL, connector->encoder_ids[0]);
 }
 
 static const struct drm_connector_helper_funcs
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 891c86aef99d..06a9a42f3cfe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15352,7 +15352,7 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
 	struct drm_crtc *drmmode_crtc;
 	struct intel_crtc *crtc;
 
-	drmmode_crtc = drm_crtc_find(dev, pipe_from_crtc_id->crtc_id);
+	drmmode_crtc = drm_crtc_find(dev, file, pipe_from_crtc_id->crtc_id);
 	if (!drmmode_crtc)
 		return -ENOENT;
 
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index e589e17876dc..5395a7f43d74 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1119,7 +1119,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
 	if (!params)
 		return -ENOMEM;
 
-	drmmode_crtc = drm_crtc_find(dev, put_image_rec->crtc_id);
+	drmmode_crtc = drm_crtc_find(dev, file_priv, put_image_rec->crtc_id);
 	if (!drmmode_crtc) {
 		ret = -ENOENT;
 		goto out_free;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 242a73e66d82..61d5a82c2793 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -946,7 +946,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 	    set->flags & I915_SET_COLORKEY_DESTINATION)
 		return -EINVAL;
 
-	plane = drm_plane_find(dev, set->plane_id);
+	plane = drm_plane_find(dev, file_priv, set->plane_id);
 	if (!plane || plane->type != DRM_PLANE_TYPE_OVERLAY)
 		return -ENOENT;
 
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 3a03ac4045d8..49c22f622fa0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1654,7 +1654,7 @@ static struct drm_encoder *mga_connector_best_encoder(struct drm_connector
 	int enc_id = connector->encoder_ids[0];
 	/* pick the encoder ids */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 947c200655b4..eb08545d4e1f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -372,7 +372,7 @@ find_encoder(struct drm_connector *connector, int type)
 		if (!id)
 			break;
 
-		enc = drm_encoder_find(dev, id);
+		enc = drm_encoder_find(dev, NULL, id);
 		if (!enc)
 			continue;
 		nv_encoder = nouveau_encoder(enc);
@@ -440,7 +440,7 @@ nouveau_connector_ddc_detect(struct drm_connector *connector)
 		if (id == 0)
 			break;
 
-		encoder = drm_encoder_find(dev, id);
+		encoder = drm_encoder_find(dev, NULL, id);
 		if (!encoder)
 			continue;
 		nv_encoder = nouveau_encoder(encoder);
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index f5e84f4b58e6..94fd4523881c 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -1460,7 +1460,7 @@ int r100_cs_packet_parse_vline(struct radeon_cs_parser *p)
 	header = radeon_get_ib_value(p, h_idx);
 	crtc_id = radeon_get_ib_value(p, h_idx + 5);
 	reg = R100_CP_PACKET0_GET_REG(header);
-	crtc = drm_crtc_find(p->rdev->ddev, crtc_id);
+	crtc = drm_crtc_find(p->rdev->ddev, p->filp, crtc_id);
 	if (!crtc) {
 		DRM_ERROR("cannot find crtc %d\n", crtc_id);
 		return -ENOENT;
diff --git a/drivers/gpu/drm/radeon/r600_cs.c b/drivers/gpu/drm/radeon/r600_cs.c
index 595a19736458..c1a418debcd8 100644
--- a/drivers/gpu/drm/radeon/r600_cs.c
+++ b/drivers/gpu/drm/radeon/r600_cs.c
@@ -887,7 +887,7 @@ int r600_cs_common_vline_parse(struct radeon_cs_parser *p,
 	crtc_id = radeon_get_ib_value(p, h_idx + 2 + 7 + 1);
 	reg = R600_CP_PACKET0_GET_REG(header);
 
-	crtc = drm_crtc_find(p->rdev->ddev, crtc_id);
+	crtc = drm_crtc_find(p->rdev->ddev, p->filp, crtc_id);
 	if (!crtc) {
 		DRM_ERROR("cannot find crtc %d\n", crtc_id);
 		return -ENOENT;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 27affbde058c..5d4b6d46575e 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -263,7 +263,7 @@ radeon_connector_update_scratch_regs(struct drm_connector *connector, enum drm_c
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev,
+		encoder = drm_encoder_find(connector->dev, NULL,
 					   connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
@@ -290,7 +290,7 @@ static struct drm_encoder *radeon_find_encoder(struct drm_connector *connector,
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]);
+		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
 
@@ -404,7 +404,7 @@ static struct drm_encoder *radeon_best_single_encoder(struct drm_connector *conn
 	int enc_id = connector->encoder_ids[0];
 	/* pick the encoder ids */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
@@ -1365,7 +1365,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 			if (connector->encoder_ids[i] == 0)
 				break;
 
-			encoder = drm_encoder_find(connector->dev,
+			encoder = drm_encoder_find(connector->dev, NULL,
 						   connector->encoder_ids[i]);
 			if (!encoder)
 				continue;
@@ -1451,7 +1451,7 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]);
+		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
 
@@ -1470,7 +1470,7 @@ static struct drm_encoder *radeon_dvi_encoder(struct drm_connector *connector)
 	/* then check use digitial */
 	/* pick the first one */
 	if (enc_id)
-		return drm_encoder_find(connector->dev, enc_id);
+		return drm_encoder_find(connector->dev, NULL, enc_id);
 	return NULL;
 }
 
@@ -1617,7 +1617,7 @@ u16 radeon_connector_encoder_get_dp_bridge_encoder_id(struct drm_connector *conn
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]);
+		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
 
@@ -1646,7 +1646,7 @@ static bool radeon_connector_encoder_is_hbr2(struct drm_connector *connector)
 		if (connector->encoder_ids[i] == 0)
 			break;
 
-		encoder = drm_encoder_find(connector->dev, connector->encoder_ids[i]);
+		encoder = drm_encoder_find(connector->dev, NULL, connector->encoder_ids[i]);
 		if (!encoder)
 			continue;
 
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index d2f57c52f7db..59de243e5ffc 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -105,7 +105,7 @@ static struct drm_encoder*
 udl_best_single_encoder(struct drm_connector *connector)
 {
 	int enc_id = connector->encoder_ids[0];
-	return drm_encoder_find(connector->dev, enc_id);
+	return drm_encoder_find(connector->dev, NULL, enc_id);
 }
 
 static int udl_connector_set_property(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index b8c6a03c8c54..bf360d718621 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -288,7 +288,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
 
 	drm_modeset_lock_all(dev);
 
-	fb = drm_framebuffer_lookup(dev, arg->fb_id);
+	fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
 	if (!fb) {
 		DRM_ERROR("Invalid framebuffer id.\n");
 		ret = -ENOENT;
@@ -371,7 +371,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
 
 	drm_modeset_lock_all(dev);
 
-	fb = drm_framebuffer_lookup(dev, arg->fb_id);
+	fb = drm_framebuffer_lookup(dev, file_priv, arg->fb_id);
 	if (!fb) {
 		DRM_ERROR("Invalid framebuffer id.\n");
 		ret = -ENOENT;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index e7daf59bac80..f3dfe496d30c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1168,7 +1168,7 @@ int vmw_kms_cursor_bypass_ioctl(struct drm_device *dev, void *data,
 		return 0;
 	}
 
-	crtc = drm_crtc_find(dev, arg->crtc_id);
+	crtc = drm_crtc_find(dev, file_priv, arg->crtc_id);
 	if (!crtc) {
 		ret = -ENOENT;
 		goto out;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 045a97cbeba2..16bb601d1f05 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -772,10 +772,11 @@ static inline unsigned drm_connector_index(struct drm_connector *connector)
  * add takes a reference to it.
  */
 static inline struct drm_connector *drm_connector_lookup(struct drm_device *dev,
+		struct drm_file *file_priv,
 		uint32_t id)
 {
 	struct drm_mode_object *mo;
-	mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_CONNECTOR);
+	mo = drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_CONNECTOR);
 	return mo ? obj_to_connector(mo) : NULL;
 }
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 946672f97e1e..d12d2f653e1c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -833,10 +833,11 @@ int drm_mode_set_config_internal(struct drm_mode_set *set);
 
 /* Helpers */
 static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
-	uint32_t id)
+		struct drm_file *file_priv,
+		uint32_t id)
 {
 	struct drm_mode_object *mo;
-	mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_CRTC);
+	mo = drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_CRTC);
 	return mo ? obj_to_crtc(mo) : NULL;
 }
 
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index c7438ff0d609..0520962b7b22 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -213,11 +213,12 @@ static inline bool drm_encoder_crtc_ok(struct drm_encoder *encoder,
  * drm_mode_object_find().
  */
 static inline struct drm_encoder *drm_encoder_find(struct drm_device *dev,
+						   struct drm_file *file_priv,
 						   uint32_t id)
 {
 	struct drm_mode_object *mo;
 
-	mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_ENCODER);
+	mo = drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_ENCODER);
 
 	return mo ? obj_to_encoder(mo) : NULL;
 }
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index 1ddfa2928802..dfbd8a087128 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -212,6 +212,7 @@ int drm_framebuffer_init(struct drm_device *dev,
 			 struct drm_framebuffer *fb,
 			 const struct drm_framebuffer_funcs *funcs);
 struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
+					       struct drm_file *file_priv,
 					       uint32_t id);
 void drm_framebuffer_remove(struct drm_framebuffer *fb);
 void drm_framebuffer_cleanup(struct drm_framebuffer *fb);
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index 43460b21d112..0181fc99c418 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -27,6 +27,7 @@
 struct drm_object_properties;
 struct drm_property;
 struct drm_device;
+struct drm_file;
 
 /**
  * struct drm_mode_object - base structure for modeset objects
@@ -108,6 +109,7 @@ struct drm_object_properties {
 	}
 
 struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
+					     struct drm_file *file_priv,
 					     uint32_t id, uint32_t type);
 void drm_mode_object_reference(struct drm_mode_object *obj);
 void drm_mode_object_unreference(struct drm_mode_object *obj);
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index db3bbdeb36d5..a3cdaab7d028 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -554,10 +554,11 @@ int drm_mode_plane_set_obj_prop(struct drm_plane *plane,
  * drm_mode_object_find().
  */
 static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
+		struct drm_file *file_priv,
 		uint32_t id)
 {
 	struct drm_mode_object *mo;
-	mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_PLANE);
+	mo = drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_PLANE);
 	return mo ? obj_to_plane(mo) : NULL;
 }
 
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 43c4b6a2046d..1d4d99eac348 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -285,10 +285,11 @@ void drm_property_unreference_blob(struct drm_property_blob *blob);
  * This function looks up the property object specified by id and returns it.
  */
 static inline struct drm_property *drm_property_find(struct drm_device *dev,
+						     struct drm_file *file_priv,
 						     uint32_t id)
 {
 	struct drm_mode_object *mo;
-	mo = drm_mode_object_find(dev, id, DRM_MODE_OBJECT_PROPERTY);
+	mo = drm_mode_object_find(dev, file_priv, id, DRM_MODE_OBJECT_PROPERTY);
 	return mo ? obj_to_property(mo) : NULL;
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/5] drm: Add new LEASE debug level
  2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
  2017-04-29  6:06   ` [PATCH 1/5] drm: Pass struct drm_file * to __drm_mode_object_find Keith Packard
@ 2017-04-29  6:06   ` Keith Packard
  2017-04-29  6:07   ` [PATCH 3/5] drm: Add drm_object lease infrastructure [v2] Keith Packard
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-29  6:06 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel

Separate out lease debugging from the core.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_drv.c | 3 ++-
 include/drm/drmP.h        | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 6594b4088f11..d4a3612655e3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -57,7 +57,8 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
 "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n"
 "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n"
 "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n"
-"\t\tBit 5 (0x20) will enable VBL messages (vblank code)");
+"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
+"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)");
 module_param_named(debug, drm_debug, int, 0600);
 
 static DEFINE_SPINLOCK(drm_minor_lock);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9c4ee144b5f6..304a22c87999 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -137,6 +137,7 @@ struct dma_buf_attachment;
 #define DRM_UT_ATOMIC		0x10
 #define DRM_UT_VBL		0x20
 #define DRM_UT_STATE		0x40
+#define DRM_UT_LEASE		0x80
 
 /***********************************************************************/
 /** \name DRM template customization defaults */
@@ -251,6 +252,9 @@ struct dma_buf_attachment;
 #define DRM_DEBUG_VBL(fmt, ...)					\
 	drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
 
+#define DRM_DEBUG_LEASE(fmt, ...)					\
+	drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
+
 #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...)	\
 ({									\
 	static DEFINE_RATELIMIT_STATE(_rs,				\
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/5] drm: Add drm_object lease infrastructure [v2]
  2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
  2017-04-29  6:06   ` [PATCH 1/5] drm: Pass struct drm_file * to __drm_mode_object_find Keith Packard
  2017-04-29  6:06   ` [PATCH 2/5] drm: Add new LEASE debug level Keith Packard
@ 2017-04-29  6:07   ` Keith Packard
  2017-04-29  6:07   ` [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v2] Keith Packard
  2017-04-29  6:07   ` [PATCH 5/5] drm: Add three ioctls for managing drm mode object leases [v2] Keith Packard
  4 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-29  6:07 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel

This provides new data structures to hold "lease" information about
drm mode setting objects, and provides for creating new drm_masters
which have access to a subset of the available drm resources.

An 'owner' is a drm_master which is not leasing the objects from
another drm_master, and hence 'owns' them.

A 'lessee' is a drm_master which is leasing objects from some other
drm_master. Each lessee holds the set of objects which it is leasing
from the lessor.

A 'lessor' is a drm_master which is leasing objects to another
drm_master. This is the same as the owner in the current code.

The set of objects any drm_master 'controls' is limited to the set of
objects it leases (for lessees) or all objects (for owners).

Objects not controlled by a drm_master cannot be modified through the
various state manipulating ioctls, and any state reported back to user
space will be edited to make them appear idle and/or unusable. For
instance, connectors always report 'disconnected', while encoders
report no possible crtcs or clones.

The full list of lessees leasing objects from an owner (either
directly, or indirectly through another lessee), can be searched from
an idr in the drm_master of the owner.

Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:

* Sub-leasing has been disabled.

* BUG_ON for lock checking replaced with lockdep_assert_held

* 'change' ioctl has been removed.

* Leased objects can always be controlled by the lessor; the
  'mask_lease' flag has been removed

* Checking for leased status has been simplified, replacing
  the drm_lease_check function with drm_lease_held.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/Makefile      |   3 +-
 drivers/gpu/drm/drm_auth.c    |  22 ++-
 drivers/gpu/drm/drm_lease.c   | 313 ++++++++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h            |   1 +
 include/drm/drm_auth.h        |  20 +++
 include/drm/drm_lease.h       |  34 +++++
 include/drm/drm_mode_object.h |   1 +
 7 files changed, 392 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_lease.c
 create mode 100644 include/drm/drm_lease.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index b9ae4280de9d..c2c6d61d30cf 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,8 @@ drm-y       :=	drm_auth.o drm_bufs.o drm_cache.o \
 		drm_framebuffer.o drm_connector.o drm_blend.o \
 		drm_encoder.o drm_mode_object.o drm_property.o \
 		drm_plane.o drm_color_mgmt.o drm_print.o \
-		drm_dumb_buffers.o drm_mode_config.o
+		drm_dumb_buffers.o drm_mode_config.o \
+		drm_lease.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 6b143514a566..1db4f63860d1 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -31,6 +31,7 @@
 #include <drm/drmP.h>
 #include "drm_internal.h"
 #include "drm_legacy.h"
+#include <drm/drm_lease.h>
 
 /**
  * DOC: master and authentication
@@ -93,7 +94,7 @@ int drm_authmagic(struct drm_device *dev, void *data,
 	return file ? 0 : -EINVAL;
 }
 
-static struct drm_master *drm_master_create(struct drm_device *dev)
+struct drm_master *drm_master_create(struct drm_device *dev)
 {
 	struct drm_master *master;
 
@@ -107,6 +108,14 @@ static struct drm_master *drm_master_create(struct drm_device *dev)
 	idr_init(&master->magic_map);
 	master->dev = dev;
 
+	/* initialize the tree of output resource lessees */
+	master->lessor = NULL;
+	master->lessee_id = 0;
+	INIT_LIST_HEAD(&master->lessees);
+	INIT_LIST_HEAD(&master->lessee_list);
+	idr_init(&master->leases);
+	idr_init(&master->lessee_idr);
+
 	return master;
 }
 
@@ -189,6 +198,12 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data,
 		goto out_unlock;
 	}
 
+	if (file_priv->master->lessor != NULL) {
+		DRM_DEBUG_LEASE("Attempt to set lessee %d as master\n", file_priv->master->lessee_id);
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	ret = drm_set_master(dev, file_priv, false);
 out_unlock:
 	mutex_unlock(&dev->master_mutex);
@@ -310,12 +325,17 @@ static void drm_master_destroy(struct kref *kref)
 	struct drm_master *master = container_of(kref, struct drm_master, refcount);
 	struct drm_device *dev = master->dev;
 
+	drm_lease_destroy(master);
+
 	if (dev->driver->master_destroy)
 		dev->driver->master_destroy(dev, master);
 
 	drm_legacy_master_rmmaps(dev, master);
 
 	idr_destroy(&master->magic_map);
+
+	idr_destroy(&master->lessee_idr);
+
 	kfree(master->unique);
 	kfree(master);
 }
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
new file mode 100644
index 000000000000..12828b8c0bff
--- /dev/null
+++ b/drivers/gpu/drm/drm_lease.c
@@ -0,0 +1,313 @@
+/*
+ * Copyright © 2017 Keith Packard <keithp@keithp.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include <drm/drmP.h>
+#include "drm_internal.h"
+#include "drm_legacy.h"
+#include "drm_crtc_internal.h"
+#include <drm/drm_lease.h>
+#include <drm/drm_auth.h>
+#include <drm/drm_crtc_helper.h>
+
+#define drm_for_each_lessee(lessee, lessor) \
+	list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
+
+/*
+ * drm_lease_owner - return ancestor owner drm_master
+ * @master: drm_master somewhere within tree of lessees and lessors
+ *
+ * RETURN:
+ *
+ * drm_master at the top of the tree (i.e, with lessor NULL
+ */
+struct drm_master *drm_lease_owner(struct drm_master *master) {
+	while (master->lessor != NULL)
+		master = master->lessor;
+	return master;
+}
+EXPORT_SYMBOL(drm_lease_owner);
+
+/**
+ * _drm_lease_held_master - check to see if an object is leased (or owned) by master
+ * @master: the master to check the lease status of
+ * @id: the id to check
+ *
+ * Checks if the specified master holds a lease on the object. Return
+ * value:
+ *
+ *	true		'master' holds a lease on (or owns) the object
+ *	false		'master' does not hold a lease.
+ */
+static int _drm_lease_held_master(struct drm_master *master, int id) {
+	lockdep_assert_held(&master->dev->mode_config.idr_mutex);
+	if (master->lessor)
+		return idr_find(&master->leases, id) != NULL;
+	return true;
+}
+
+/**
+ * _drm_has_leased - check to see if an object has been leased
+ * @master: the master to check the lease status of
+ * @id: the id to check
+ *
+ * Checks if any lessee of 'master' holds a lease on 'id'. Return
+ * value:
+ *
+ *	true		Some lessee holds a lease on the object.
+ *	false		No lessee has a lease on the object.
+ */
+static bool _drm_has_leased(struct drm_master *master, int id) {
+	struct drm_master *lessee;
+
+	drm_for_each_lessee(lessee, master) {
+		if (idr_find(&master->leases, id) != NULL)
+			return true;
+	}
+	return false;
+}
+
+/**
+ * _drm_lease_held - check drm_mode_object lease status (idr_mutex held)
+ * @master: the drm_master
+ * @id: the object id
+ *
+ * Checks if the specified master holds a lease on the object. Return
+ * value:
+ *
+ *	true		'master' holds a lease on (or owns) the object
+ *	false		'master' does not hold a lease.
+ */
+bool _drm_lease_held(struct drm_file *file_priv, int id)
+{
+	struct drm_master *master;
+	bool ret;
+
+	if (file_priv == NULL)
+		return true;
+
+	master = file_priv->master;
+	ret = _drm_lease_held_master(master, id);
+	return ret;
+}
+EXPORT_SYMBOL(_drm_lease_held);
+
+/**
+ * drm_lease_held - check drm_mode_object lease status (idr_mutex not held)
+ * @master: the drm_master
+ * @id: the object id
+ *
+ * Checks if the specified master holds a lease on the object. Return
+ * value:
+ *
+ *	true		'master' holds a lease on (or owns) the object
+ *	false		'master' does not hold a lease.
+ */
+bool drm_lease_held(struct drm_file *file_priv, int id)
+{
+	struct drm_master *master;
+	bool ret;
+
+	if (file_priv == NULL)
+		return true;
+
+	master = file_priv->master;
+	mutex_lock(&master->dev->mode_config.idr_mutex);
+	ret = _drm_lease_held_master(master, id);
+	mutex_unlock(&master->dev->mode_config.idr_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(drm_lease_held);
+
+/**
+ * drm_lease_filter_crtcs - restricted crtc set to leased values
+ * @file_priv: requestor file
+ * @crtcs: bitmask of crtcs to check
+ *
+ * Reconstructs a crtc mask based on the crtcs which are visible
+ * through the specified file.
+ */
+uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs_in)
+{
+	struct drm_master *master = file_priv->master;
+	struct drm_device *dev = master->dev;
+	struct drm_crtc *crtc;
+	int count_in, count_out;
+	uint32_t crtcs_out = 0;
+
+	count_in = count_out = 0;
+	mutex_lock(&master->dev->mode_config.idr_mutex);
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		if (_drm_lease_held_master(master, crtc->base.id)) {
+			uint32_t	mask_in = 1ul << count_in;
+			if ((crtcs_in & mask_in) != 0) {
+				uint32_t	mask_out = 1ul << count_out;
+				crtcs_out |= mask_out;
+			}
+			count_out++;
+		}
+		count_in++;
+	}
+	mutex_unlock(&master->dev->mode_config.idr_mutex);
+	return crtcs_out;
+}
+EXPORT_SYMBOL(drm_lease_filter_crtcs);
+
+/**
+ * drm_lease_filter_encoders - restrict encoder set to leased values
+ * @file_priv: requestor file
+ * @encoders: bitmask of encoders to check
+ *
+ * Reconstructs an encoder mask based on the encoders which are
+ * visible through the specified file.
+ */
+uint32_t drm_lease_filter_encoders(struct drm_file *file_priv, uint32_t encoders_in)
+{
+	struct drm_master *master = file_priv->master;
+	struct drm_device *dev = master->dev;
+	struct drm_encoder *encoder;
+	int count_in, count_out;
+	uint32_t encoders_out = 0;
+
+	count_in = count_out = 0;
+	mutex_lock(&master->dev->mode_config.idr_mutex);
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
+		if (_drm_lease_held_master(master, encoder->base.id)) {
+			uint32_t	mask_in = 1ul << count_in;
+			if ((encoders_in & mask_in) != 0) {
+				uint32_t	mask_out = 1ul << count_out;
+				encoders_out |= mask_out;
+			}
+			count_out++;
+		}
+		count_in++;
+	}
+	mutex_unlock(&master->dev->mode_config.idr_mutex);
+	return encoders_out;
+}
+EXPORT_SYMBOL(drm_lease_filter_encoders);
+
+/*
+ * drm_lease_create - create a new drm_master with leased objects
+ * @lessor: lease holder (or owner) of objects
+ * @leases: objects to lease to the new drm_master
+ *
+ * Uses drm_master_create to allocate a new drm_master, then checks to
+ * make sure all of the desired objects can be leased, atomically
+ * leasing them to the new drmmaster.
+ *
+ * 	ERR_PTR(-EACCESS)	some other master holds the title to any object
+ * 	ERR_PTR(-ENOENT)	some object is not a valid DRM object for this device
+ * 	ERR_PTR(-EBUSY)		some other lessee holds title to this object
+ *	ERR_PTR(-EEXIST)	same object specified more than once in the provided list
+ *	ERR_PTR(-ENOMEM)	allocation failed
+ */
+static struct drm_master *drm_lease_create(struct drm_master *lessor, struct idr *leases)
+{
+	struct drm_device *dev = lessor->dev;
+	int error;
+	struct drm_master *lessee;
+	int object;
+	int id;
+	void *entry;
+
+	DRM_DEBUG_LEASE("lessor %d\n", lessor->lessee_id);
+
+	lockdep_assert_held(&lessor->dev->master_mutex);
+
+	lessee = drm_master_create(lessor->dev);
+	if (!lessee) {
+		DRM_DEBUG_LEASE("drm_master_create failed\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* Insert the new lessee into the tree */
+	id = idr_alloc(&(drm_lease_owner(lessor)->lessee_idr), lessee, 1, 0, GFP_KERNEL);
+	if (id < 0) {
+		error = id;
+		goto out_lessee;
+	}
+
+	lessee->lessee_id = id;
+	lessee->lessor = drm_master_get(lessor);
+	list_add_tail(&lessee->lessee_list, &lessor->lessees);
+
+	/* Lock the mode object mutex to make the check and allocation atomic */
+	mutex_lock(&lessor->dev->mode_config.idr_mutex);
+	idr_for_each_entry(leases, entry, object) {
+		error = 0;
+		if (!idr_find(&dev->mode_config.crtc_idr, object))
+			error = -ENOENT;
+		else if (!_drm_lease_held_master(lessor, object))
+			error = -EACCES;
+		else if (_drm_has_leased(lessor, object))
+			error = -EBUSY;
+
+		if (error != 0) {
+			DRM_DEBUG_LEASE("object %d failed %d\n", object, error);
+			goto out_unlock;
+		}
+	}
+
+	/* Move the leases over */
+	lessee->leases = *leases;
+	mutex_unlock(&lessor->dev->mode_config.idr_mutex);
+	DRM_DEBUG_LEASE("new lessee %d %p, lessor %d %p\n", lessee->lessee_id, lessee, lessor->lessee_id, lessor);
+	return lessee;
+
+out_unlock:
+	mutex_unlock(&lessor->dev->mode_config.idr_mutex);
+
+out_lessee:
+	drm_master_put(&lessee);
+	return ERR_PTR(error);
+}
+
+/**
+ * drm_lease_destroy - a master is going away
+ * @master: the drm_master being destroyed
+ *
+ * All lessees will have been destroyed as they
+ * hold a reference on their lessor. Notify any
+ * lessor for this master so that it can check
+ * the list of lessees.
+ */
+void drm_lease_destroy(struct drm_master *master)
+{
+	struct drm_device *dev = master->dev;
+
+	lockdep_assert_held(&dev->master_mutex);
+
+	DRM_DEBUG_LEASE("drm_lease_destroy %d\n", master->lessee_id);
+
+	/* This master is referenced by all lessees, hence it cannot be destroyed
+	 * until all of them have been
+	 */
+	WARN_ON(!list_empty(&master->lessees));
+
+	/* Remove this master from the lessee idr in the owner */
+	if (master->lessee_id != 0) {
+		DRM_DEBUG_LEASE("remove master %d from device list of lessees\n", master->lessee_id);
+		idr_remove(&(drm_lease_owner(master)->lessee_idr), master->lessee_id);
+	}
+
+	/* Remove this master from any lessee list it may be on */
+	list_del(&master->lessee_list);
+	if (master->lessor) {
+		/* Tell the master to check the lessee list */
+		drm_sysfs_hotplug_event(dev);
+		drm_master_put(&master->lessor);
+	}
+
+	DRM_DEBUG_LEASE("drm_lease_destroy done %d\n", master->lessee_id);
+}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 304a22c87999..b468bc9cf318 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -340,6 +340,7 @@ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
 #define DRM_CONTROL_ALLOW 0x8
 #define DRM_UNLOCKED	0x10
 #define DRM_RENDER_ALLOW 0x20
+#define DRM_ANY_MASTER	0x40
 
 struct drm_ioctl_desc {
 	unsigned int cmd;
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index 610223b0481b..3db333f889a2 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -38,6 +38,12 @@
  * @magic_map: Map of used authentication tokens. Protected by struct_mutex.
  * @lock: DRI lock information.
  * @driver_priv: Pointer to driver-private information.
+ * @lessor: Lease holder
+ * @lessee_id: id for lessees. Owners always have id 0
+ * @lessee_list: other lessees of the same master
+ * @lessees: drm_masters leasing from this one
+ * @leases: Objects leased to this drm_master.
+ * @lessee_idr: All lessees under this owner (only used where lessor == NULL)
  *
  * Note that master structures are only relevant for the legacy/primary device
  * nodes, hence there can only be one per device, not one per drm_minor.
@@ -50,10 +56,24 @@ struct drm_master {
 	struct idr magic_map;
 	struct drm_lock_data lock;
 	void *driver_priv;
+
+	/* Tree of display resource leases, each of which is a drm_master struct
+	 * All of these get activated simultaneously, so drm_device master points
+	 * at the top of the tree (for which lessor is NULL)
+	 */
+
+	struct drm_master *lessor;
+	int	lessee_id;
+	struct list_head lessee_list;
+	struct list_head lessees;
+	struct idr leases;
+	struct idr lessee_idr;
 };
 
 struct drm_master *drm_master_get(struct drm_master *master);
 void drm_master_put(struct drm_master **master);
 bool drm_is_current_master(struct drm_file *fpriv);
 
+struct drm_master *drm_master_create(struct drm_device *dev);
+
 #endif
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
new file mode 100644
index 000000000000..da4e70ef58a8
--- /dev/null
+++ b/include/drm/drm_lease.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright © 2017 Keith Packard <keithp@keithp.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef _DRM_LEASE_H_
+#define _DRM_LEASE_H_
+
+struct drm_file;
+struct drm_device;
+struct drm_master;
+
+struct drm_master *drm_lease_owner(struct drm_master *master);
+
+void drm_lease_destroy(struct drm_master *lessee);
+
+bool drm_lease_held(struct drm_file *file_priv, int id);
+
+bool _drm_lease_held(struct drm_file *file_priv, int id);
+
+uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs);
+
+uint32_t drm_lease_filter_encoders(struct drm_file *file_priv, uint32_t encoders);
+
+#endif /* _DRM_LEASE_H_ */
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index 0181fc99c418..b1573ac325df 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -24,6 +24,7 @@
 #define __DRM_MODESET_H__
 
 #include <linux/kref.h>
+#include <drm/drm_lease.h>
 struct drm_object_properties;
 struct drm_property;
 struct drm_device;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v2]
  2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
                     ` (2 preceding siblings ...)
  2017-04-29  6:07   ` [PATCH 3/5] drm: Add drm_object lease infrastructure [v2] Keith Packard
@ 2017-04-29  6:07   ` Keith Packard
  2017-04-29  6:07   ` [PATCH 5/5] drm: Add three ioctls for managing drm mode object leases [v2] Keith Packard
  4 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-29  6:07 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel

Attempts to modify un-leased objects are rejected with an error.
Information returned about unleased objects is modified to make them
appear unusable and/or disconnected.

Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:

With the change in the __drm_mode_object_find API to pass the
file_priv along, we can now centralize most of the lease-based access
checks in that function.

A few places skip that API and require in-line checks.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_auth.c        |  2 +-
 drivers/gpu/drm/drm_connector.c   |  8 +++---
 drivers/gpu/drm/drm_encoder.c     |  8 +++---
 drivers/gpu/drm/drm_mode_config.c | 52 ++++++++++++++++++++++++---------------
 drivers/gpu/drm/drm_mode_object.c | 22 +++++++++++++++++
 drivers/gpu/drm/drm_plane.c       |  6 +++--
 6 files changed, 69 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 1db4f63860d1..44c99d12f4c1 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -303,7 +303,7 @@ void drm_master_release(struct drm_file *file_priv)
  */
 bool drm_is_current_master(struct drm_file *fpriv)
 {
-	return fpriv->is_master && fpriv->master == fpriv->minor->dev->master;
+	return fpriv->is_master && drm_lease_owner(fpriv->master) == fpriv->minor->dev->master;
 }
 EXPORT_SYMBOL(drm_is_current_master);
 
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 670c20d5660c..dbf34f08363b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1094,7 +1094,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	}
 
 	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
-		if (connector->encoder_ids[i] != 0)
+		if (connector->encoder_ids[i] != 0 &&
+		    drm_lease_held(file_priv, connector->encoder_ids[i]))
 			encoders_count++;
 
 	if (out_resp->count_modes == 0) {
@@ -1118,7 +1119,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	encoder = drm_connector_get_encoder(connector);
-	if (encoder)
+	if (encoder && drm_lease_held(file_priv, encoder->base.id))
 		out_resp->encoder_id = encoder->base.id;
 	else
 		out_resp->encoder_id = 0;
@@ -1156,7 +1157,8 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 		copied = 0;
 		encoder_ptr = (uint32_t __user *)(unsigned long)(out_resp->encoders_ptr);
 		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-			if (connector->encoder_ids[i] != 0) {
+			if (connector->encoder_ids[i] != 0 &&
+			    drm_lease_held(file_priv, connector->encoder_ids[i])) {
 				if (put_user(connector->encoder_ids[i],
 					     encoder_ptr + copied)) {
 					ret = -EFAULT;
diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index dbaedd4e12e6..512d0b6cb7d2 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -211,7 +211,7 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	crtc = drm_encoder_get_crtc(encoder);
-	if (crtc)
+	if (crtc && drm_lease_held(file_priv, crtc->base.id))
 		enc_resp->crtc_id = crtc->base.id;
 	else
 		enc_resp->crtc_id = 0;
@@ -219,8 +219,10 @@ int drm_mode_getencoder(struct drm_device *dev, void *data,
 
 	enc_resp->encoder_type = encoder->encoder_type;
 	enc_resp->encoder_id = encoder->base.id;
-	enc_resp->possible_crtcs = encoder->possible_crtcs;
-	enc_resp->possible_clones = encoder->possible_clones;
+	enc_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv,
+							  encoder->possible_crtcs);
+	enc_resp->possible_clones = drm_lease_filter_encoders(file_priv,
+							      encoder->possible_clones);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 2735a5847ffa..bb6b64e594b7 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -131,14 +131,20 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
 	/* mode_config.mutex protects the connector list against e.g. DP MST
 	 * connector hot-adding. CRTC/Plane lists are invariant. */
 	mutex_lock(&dev->mode_config.mutex);
-	drm_for_each_crtc(crtc, dev)
-		crtc_count++;
+	drm_for_each_crtc(crtc, dev) {
+		if (drm_lease_held(file_priv, crtc->base.id))
+			crtc_count++;
+	}
 
-	drm_for_each_connector(connector, dev)
-		connector_count++;
+	drm_for_each_connector(connector, dev) {
+		if (drm_lease_held(file_priv, connector->base.id))
+			connector_count++;
+	}
 
-	drm_for_each_encoder(encoder, dev)
-		encoder_count++;
+	drm_for_each_encoder(encoder, dev) {
+		if (drm_lease_held(file_priv, encoder->base.id))
+			encoder_count++;
+	}
 
 	card_res->max_height = dev->mode_config.max_height;
 	card_res->min_height = dev->mode_config.min_height;
@@ -150,11 +156,13 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
 		copied = 0;
 		crtc_id = (uint32_t __user *)(unsigned long)card_res->crtc_id_ptr;
 		drm_for_each_crtc(crtc, dev) {
-			if (put_user(crtc->base.id, crtc_id + copied)) {
-				ret = -EFAULT;
-				goto out;
+			if (drm_lease_held(file_priv, crtc->base.id)) {
+				if (put_user(crtc->base.id, crtc_id + copied)) {
+					ret = -EFAULT;
+					goto out;
+				}
+				copied++;
 			}
-			copied++;
 		}
 	}
 	card_res->count_crtcs = crtc_count;
@@ -164,12 +172,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
 		copied = 0;
 		encoder_id = (uint32_t __user *)(unsigned long)card_res->encoder_id_ptr;
 		drm_for_each_encoder(encoder, dev) {
-			if (put_user(encoder->base.id, encoder_id +
-				     copied)) {
-				ret = -EFAULT;
-				goto out;
+			if (drm_lease_held(file_priv, encoder->base.id)) {
+				if (put_user(encoder->base.id, encoder_id +
+					     copied)) {
+					ret = -EFAULT;
+					goto out;
+				}
+				copied++;
 			}
-			copied++;
 		}
 	}
 	card_res->count_encoders = encoder_count;
@@ -179,12 +189,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
 		copied = 0;
 		connector_id = (uint32_t __user *)(unsigned long)card_res->connector_id_ptr;
 		drm_for_each_connector(connector, dev) {
-			if (put_user(connector->base.id,
-				     connector_id + copied)) {
-				ret = -EFAULT;
-				goto out;
+			if (drm_lease_held(file_priv, connector->base.id)) {
+				if (put_user(connector->base.id,
+					     connector_id + copied)) {
+					ret = -EFAULT;
+					goto out;
+				}
+				copied++;
 			}
-			copied++;
 		}
 	}
 	card_res->count_connectors = connector_count;
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index daddc7640139..9763787492cc 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -107,6 +107,25 @@ void drm_mode_object_unregister(struct drm_device *dev,
 	mutex_unlock(&dev->mode_config.idr_mutex);
 }
 
+/**
+ * drm_lease_required - check types which must be leased to be used
+ * @type: type of object
+ *
+ * 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)
+{
+	switch(type) {
+	case DRM_MODE_OBJECT_CRTC:
+	case DRM_MODE_OBJECT_CONNECTOR:
+	case DRM_MODE_OBJECT_ENCODER:
+		return true;
+	default:
+		return false;
+	}
+}
+
 struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
 					       struct drm_file *file_priv,
 					       uint32_t id, uint32_t type)
@@ -120,6 +139,9 @@ 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))
+		obj = NULL;
+
 	if (obj && obj->free_cb) {
 		if (!kref_get_unless_zero(&obj->refcount))
 			obj = NULL;
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 819f74616f34..c9c687cdc747 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -392,7 +392,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 		return -ENOENT;
 
 	drm_modeset_lock(&plane->mutex, NULL);
-	if (plane->crtc)
+	if (plane->crtc && drm_lease_held(file_priv, plane->crtc->base.id))
 		plane_resp->crtc_id = plane->crtc->base.id;
 	else
 		plane_resp->crtc_id = 0;
@@ -404,7 +404,9 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
 	drm_modeset_unlock(&plane->mutex);
 
 	plane_resp->plane_id = plane->base.id;
-	plane_resp->possible_crtcs = plane->possible_crtcs;
+	plane_resp->possible_crtcs = drm_lease_filter_crtcs(file_priv,
+							    plane->possible_crtcs);
+
 	plane_resp->gamma_size = 0;
 
 	/*
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/5] drm: Add three ioctls for managing drm mode object leases [v2]
  2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
                     ` (3 preceding siblings ...)
  2017-04-29  6:07   ` [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v2] Keith Packard
@ 2017-04-29  6:07   ` Keith Packard
  4 siblings, 0 replies; 22+ messages in thread
From: Keith Packard @ 2017-04-29  6:07 UTC (permalink / raw)
  To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel

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

This should suffice to at least create and query leases.

Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:

 * query ioctls only query the master associated with
   the provided file.

 * 'mask_lease' value has been removed

 * change ioctl has been removed.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_ioctl.c |   3 +
 drivers/gpu/drm/drm_lease.c | 220 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_lease.h     |   9 ++
 include/uapi/drm/drm.h      |   3 +
 include/uapi/drm/drm_mode.h |  55 +++++++++++
 5 files changed, 290 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fed22c2b98b6..5189a32ac3e0 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -636,6 +636,9 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_ATOMIC, drm_mode_atomic_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATEPROPBLOB, drm_mode_createblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROYPROPBLOB, drm_mode_destroyblob_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_ANY_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_ANY_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 12828b8c0bff..20a7b869824e 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -311,3 +311,223 @@ void drm_lease_destroy(struct drm_master *master)
 
 	DRM_DEBUG_LEASE("drm_lease_destroy done %d\n", master->lessee_id);
 }
+
+/**
+ * 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;
+
+		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);
+
+		ret = idr_alloc(&leases, (void *) 1, object_id, object_id + 1, GFP_KERNEL);
+		if (ret < 0) {
+			DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
+					object_id, ret);
+			goto out_leases;
+		}
+	}
+
+	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);
+	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_create_lease
+ * @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) {
+		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_create_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;
+	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;
+}
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
index da4e70ef58a8..18dcb7baa826 100644
--- a/include/drm/drm_lease.h
+++ b/include/drm/drm_lease.h
@@ -31,4 +31,13 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs);
 
 uint32_t drm_lease_filter_encoders(struct drm_file *file_priv, uint32_t encoders);
 
+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);
+
 #endif /* _DRM_LEASE_H_ */
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b2c52843bc70..ab30fd6210f4 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -813,6 +813,9 @@ extern "C" {
 #define DRM_IOCTL_MODE_ATOMIC		DRM_IOWR(0xBC, struct drm_mode_atomic)
 #define DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, struct drm_mode_create_blob)
 #define DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, struct drm_mode_destroy_blob)
+#define DRM_IOCTL_MODE_CREATE_LEASE	DRM_IOWR(0xBF, struct drm_mode_create_lease)
+#define DRM_IOCTL_MODE_LIST_LESSEES	DRM_IOWR(0xC0, struct drm_mode_list_lessees)
+#define DRM_IOCTL_MODE_GET_LEASE	DRM_IOWR(0xC1, struct drm_mode_get_lease)
 
 /**
  * Device specific ioctls should only be in their respective headers
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index ce7efe2e8a5e..6371e39b6863 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -681,6 +681,61 @@ 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;
+
+	/** 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;
+
+	/** Pointer to objects.
+	 * pointer to __u32 array of object ids
+	 */
+	__u64 objects_ptr;
+};
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2017-04-29  6:08 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-01 17:08 [PATCH 0/4] drm: Add mode resource leasing Keith Packard
2017-04-01 17:08 ` [PATCH 1/4] drm: Add new LEASE debug level Keith Packard
2017-04-01 17:08 ` [PATCH 2/4] drm: Add drm_object lease infrastructure Keith Packard
2017-04-02 13:38   ` Daniel Vetter
2017-04-02 16:31     ` Keith Packard
2017-04-10  5:16       ` Michel Dänzer
2017-04-02 17:51   ` Daniel Vetter
2017-04-02 19:59     ` Keith Packard
2017-04-01 17:08 ` [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths Keith Packard
2017-04-02 13:19   ` Daniel Vetter
2017-04-02 16:37     ` Keith Packard
2017-04-03  7:49       ` Daniel Vetter
2017-04-10  1:06     ` Keith Packard
2017-04-01 17:08 ` [PATCH 4/4] drm: Add four ioctls for managing drm mode object leases Keith Packard
2017-04-02 13:23   ` Daniel Vetter
2017-04-02 16:44     ` Keith Packard
2017-04-29  6:06 ` [PATCH 0/5] drm: Add mode resource leasing [v2] Keith Packard
2017-04-29  6:06   ` [PATCH 1/5] drm: Pass struct drm_file * to __drm_mode_object_find Keith Packard
2017-04-29  6:06   ` [PATCH 2/5] drm: Add new LEASE debug level Keith Packard
2017-04-29  6:07   ` [PATCH 3/5] drm: Add drm_object lease infrastructure [v2] Keith Packard
2017-04-29  6:07   ` [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v2] Keith Packard
2017-04-29  6:07   ` [PATCH 5/5] drm: Add three ioctls for managing drm mode object leases [v2] Keith Packard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).