* [PATCH 0/5]: drm: Add drm mode object leases
@ 2017-10-13 1:56 Keith Packard
2017-10-13 1:56 ` [PATCH 1/5] drm/plane: drop num_overlay_planes (v2) Keith Packard
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Keith Packard @ 2017-10-13 1:56 UTC (permalink / raw)
To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel
New since last time:
* Don't lease encoders
* Do lease planes
* Automatically lease primary and cursor planes for
apps which don't set universal_planes
* Restrict leases to only contain objects which
are actually leasable (connectors, crtcs and planes)
* Drop the patch which changes permissions on get resources
ioctls
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/5] drm/plane: drop num_overlay_planes (v2)
2017-10-13 1:56 [PATCH 0/5]: drm: Add drm mode object leases Keith Packard
@ 2017-10-13 1:56 ` Keith Packard
2017-10-16 19:23 ` Sean Paul
2017-10-13 1:56 ` [PATCH 2/5] drm: Add new LEASE debug level Keith Packard
` (4 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Keith Packard @ 2017-10-13 1:56 UTC (permalink / raw)
To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel
From: Dave Airlie <airlied@redhat.com>
In order to implement plane leasing we need to count things,
just make the code consistent with the counting code currently
used for counting crtcs/encoders/connectors and drop the need
for num_overlay_planes.
v2: don't forget to assign plane_ptr. (keithp)
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_mode_config.c | 1 -
drivers/gpu/drm/drm_plane.c | 46 ++++++++++++++-------------------------
include/drm/drm_mode_config.h | 13 -----------
3 files changed, 16 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 74f6ff5df656..919e78d45ab0 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -385,7 +385,6 @@ void drm_mode_config_init(struct drm_device *dev)
dev->mode_config.num_connector = 0;
dev->mode_config.num_crtc = 0;
dev->mode_config.num_encoder = 0;
- dev->mode_config.num_overlay_plane = 0;
dev->mode_config.num_total_plane = 0;
}
EXPORT_SYMBOL(drm_mode_config_init);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 574fd1475214..c10869bad729 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -241,8 +241,6 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
list_add_tail(&plane->head, &config->plane_list);
plane->index = config->num_total_plane++;
- if (plane->type == DRM_PLANE_TYPE_OVERLAY)
- config->num_overlay_plane++;
drm_object_attach_property(&plane->base,
config->plane_type_property,
@@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
list_del(&plane->head);
dev->mode_config.num_total_plane--;
- if (plane->type == DRM_PLANE_TYPE_OVERLAY)
- dev->mode_config.num_overlay_plane--;
WARN_ON(plane->state && !plane->funcs->atomic_destroy_state);
if (plane->state && plane->funcs->atomic_destroy_state)
@@ -462,43 +458,33 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data,
struct drm_mode_config *config;
struct drm_plane *plane;
uint32_t __user *plane_ptr;
- int copied = 0;
- unsigned num_planes;
+ int count = 0;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
config = &dev->mode_config;
-
- if (file_priv->universal_planes)
- num_planes = config->num_total_plane;
- else
- num_planes = config->num_overlay_plane;
+ plane_ptr = u64_to_user_ptr(plane_resp->plane_id_ptr);
/*
* This ioctl is called twice, once to determine how much space is
* needed, and the 2nd time to fill it.
*/
- if (num_planes &&
- (plane_resp->count_planes >= num_planes)) {
- plane_ptr = (uint32_t __user *)(unsigned long)plane_resp->plane_id_ptr;
-
- /* Plane lists are invariant, no locking needed. */
- drm_for_each_plane(plane, dev) {
- /*
- * Unless userspace set the 'universal planes'
- * capability bit, only advertise overlays.
- */
- if (plane->type != DRM_PLANE_TYPE_OVERLAY &&
- !file_priv->universal_planes)
- continue;
-
- if (put_user(plane->base.id, plane_ptr + copied))
- return -EFAULT;
- copied++;
- }
+ drm_for_each_plane(plane, dev) {
+ /*
+ * Unless userspace set the 'universal planes'
+ * capability bit, only advertise overlays.
+ */
+ if (plane->type != DRM_PLANE_TYPE_OVERLAY &&
+ !file_priv->universal_planes)
+ continue;
+
+ if (count < config->num_total_plane &&
+ put_user(plane->base.id, plane_ptr + count))
+ return -EFAULT;
+ count++;
}
- plane_resp->count_planes = num_planes;
+ plane_resp->count_planes = count;
return 0;
}
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..0b4ac2ebc610 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -429,19 +429,6 @@ struct drm_mode_config {
*/
struct list_head encoder_list;
- /**
- * @num_overlay_plane:
- *
- * Number of overlay planes on this device, excluding primary and cursor
- * planes.
- *
- * Track number of overlay planes separately from number of total
- * planes. By default we only advertise overlay planes to userspace; if
- * userspace sets the "universal plane" capability bit, we'll go ahead
- * and expose all planes. This is invariant over the lifetime of a
- * device and hence doesn't need any locks.
- */
- int num_overlay_plane;
/**
* @num_total_plane:
*
--
2.15.0.rc0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] drm: Add new LEASE debug level
2017-10-13 1:56 [PATCH 0/5]: drm: Add drm mode object leases Keith Packard
2017-10-13 1:56 ` [PATCH 1/5] drm/plane: drop num_overlay_planes (v2) Keith Packard
@ 2017-10-13 1:56 ` Keith Packard
2017-10-16 19:24 ` Sean Paul
2017-10-13 1:56 ` [PATCH 3/5] drm: Add drm_object lease infrastructure [v4] Keith Packard
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Keith Packard @ 2017-10-13 1:56 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 c0292e5d7281..a934fd5e7e55 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 7277783a4ff0..59be1232d005 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -136,6 +136,7 @@ struct pci_controller;
#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 */
@@ -250,6 +251,9 @@ struct pci_controller;
#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.15.0.rc0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
2017-10-13 1:56 [PATCH 0/5]: drm: Add drm mode object leases Keith Packard
2017-10-13 1:56 ` [PATCH 1/5] drm/plane: drop num_overlay_planes (v2) Keith Packard
2017-10-13 1:56 ` [PATCH 2/5] drm: Add new LEASE debug level Keith Packard
@ 2017-10-13 1:56 ` Keith Packard
2017-10-16 19:44 ` Sean Paul
2017-10-13 1:56 ` [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3] Keith Packard
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Keith Packard @ 2017-10-13 1:56 UTC (permalink / raw)
To: linux-kernel, Dave Airlie, Daniel Vetter; +Cc: Keith Packard, dri-devel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=\x0e, Size: 18743 bytes --]
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.
Changes in v3, some suggested by Dave Airlie <airlied@gmail.com>
* Add revocation. This allows leases to be effectively revoked by
removing all of the objects they have access to. The lease itself
hangs around as it's hanging off a file.
* Free the leases IDR when the master is destroyed
* _drm_lease_held should look at lessees, not lessor
* Allow non-master files to check for lease status
Changes in v4, suggested by Dave Airlie <airlied@gmail.com>
* Formatting and whitespace changes
Signed-off-by: Keith Packard <keithp@keithp.com>
---
drivers/gpu/drm/Makefile | 2 +-
drivers/gpu/drm/drm_auth.c | 29 +++-
drivers/gpu/drm/drm_lease.c | 339 ++++++++++++++++++++++++++++++++++++++++++
include/drm/drm_auth.h | 20 +++
include/drm/drm_lease.h | 36 +++++
include/drm/drm_mode_object.h | 1 +
6 files changed, 425 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 a3fdc5a68dff..81ff79336623 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -17,7 +17,7 @@ drm-y := drm_auth.o drm_bufs.o drm_cache.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_vblank.o \
- drm_syncobj.o
+ drm_syncobj.o drm_lease.o
drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 7ff697389d74..541177c71d51 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);
@@ -270,6 +285,13 @@ void drm_master_release(struct drm_file *file_priv)
if (dev->master == file_priv->master)
drm_drop_master(dev, file_priv);
out:
+ if (file_priv->is_master) {
+ /* Revoke any leases held by this or lessees, but only if
+ * this is the "real" master
+ */
+ _drm_lease_revoke(master);
+ }
+
/* drop the master reference held by the file priv */
if (file_priv->master)
drm_master_put(&file_priv->master);
@@ -310,12 +332,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->leases);
+ 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..6c3f2445254c
--- /dev/null
+++ b/drivers/gpu/drm/drm_lease.c
@@ -0,0 +1,339 @@
+/*
+ * 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_find_lessee - find lessee by id
+ * @master: drm_master of lessor
+ * @id: lessee_id
+ *
+ * RETURN:
+ *
+ * drm_master of the lessee if valid, NULL otherwise
+ */
+
+static struct drm_master*
+_drm_find_lessee(struct drm_master *master, int lessee_id)
+{
+ return idr_find(&drm_lease_owner(master)->lessee_idr, lessee_id);
+}
+
+/**
+ * _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 (_drm_lease_held_master(lessee, id))
+ 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)
+{
+ if (file_priv == NULL || file_priv->master == NULL)
+ return true;
+
+ return _drm_lease_held_master(file_priv->master, id);
+}
+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 || file_priv->master == 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;
+ struct drm_device *dev;
+ struct drm_crtc *crtc;
+ int count_in, count_out;
+ uint32_t crtcs_out = 0;
+
+ if (file_priv == NULL || file_priv->master == NULL)
+ return crtcs_in;
+
+ master = file_priv->master;
+ dev = master->dev;
+
+ 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_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);
+}
+
+/**
+ * _drm_lease_revoke - revoke access to all leased objects
+ * @master: the master losing its lease
+ */
+
+void _drm_lease_revoke(struct drm_master *top)
+{
+ int object;
+ void *entry;
+ struct drm_master *master = top;
+
+ /*
+ * Walk the tree starting at 'top' emptying all leases. Because
+ * the tree is fully connected, we can do this without recursing
+ */
+ for (;;) {
+ DRM_DEBUG_LEASE("revoke leases for %p %d\n", master, master->lessee_id);
+
+ /* Evacuate the lease */
+ idr_for_each_entry(&master->leases, entry, object)
+ idr_remove(&master->leases, object);
+
+ /* Depth-first list walk */
+
+ /* Down */
+ if (!list_empty(&master->lessees)) {
+ master = list_first_entry(&master->lessees, struct drm_master, lessee_list);
+ } else {
+ /* Up */
+ while (master != top && master == list_last_entry(&master->lessor->lessees, struct drm_master, lessee_list))
+ master = master->lessor;
+
+ if (master == top)
+ break;
+
+ /* Over */
+ master = list_entry(master->lessee_list.next, struct drm_master, lessee_list);
+ }
+ }
+}
diff --git a/include/drm/drm_auth.h b/include/drm/drm_auth.h
index 81a40c2a9a3e..4b7fc22d3fc9 100644
--- a/include/drm/drm_auth.h
+++ b/include/drm/drm_auth.h
@@ -52,6 +52,12 @@ struct drm_lock_data {
* @dev: Link back to the DRM device
* @lock: DRI1 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.
@@ -76,10 +82,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..a49667db1d6d
--- /dev/null
+++ b/include/drm/drm_lease.h
@@ -0,0 +1,36 @@
+/*
+ * 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);
+
+void _drm_lease_revoke(struct drm_master *master);
+
+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 b2f920b518e3..c8155cb5a932 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.15.0.rc0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3]
2017-10-13 1:56 [PATCH 0/5]: drm: Add drm mode object leases Keith Packard
` (2 preceding siblings ...)
2017-10-13 1:56 ` [PATCH 3/5] drm: Add drm_object lease infrastructure [v4] Keith Packard
@ 2017-10-13 1:56 ` Keith Packard
2017-10-16 20:34 ` Sean Paul
2017-10-13 1:56 ` [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6] Keith Packard
2017-10-16 9:13 ` [PATCH 0/5]: drm: Add drm mode object leases Daniel Vetter
5 siblings, 1 reply; 16+ messages in thread
From: Keith Packard @ 2017-10-13 1:56 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.
Changes for v3 provided by Dave Airlie <airlied@redhat.com>
* remove support for leasing encoders.
* add support for leasing planes.
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_auth.c | 2 +-
drivers/gpu/drm/drm_encoder.c | 5 +++--
drivers/gpu/drm/drm_mode_config.c | 22 +++++++++++++---------
drivers/gpu/drm/drm_mode_object.c | 22 ++++++++++++++++++++++
drivers/gpu/drm/drm_plane.c | 18 +++++++++++-------
drivers/gpu/drm/drm_vblank.c | 18 ++++++++++++++++--
include/drm/drm_lease.h | 2 --
7 files changed, 66 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 541177c71d51..bab26b477738 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -310,7 +310,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_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 43f644844b83..59e0ebe733f8 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -226,7 +226,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;
@@ -234,7 +234,8 @@ 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_crtcs = drm_lease_filter_crtcs(file_priv,
+ encoder->possible_crtcs);
enc_resp->possible_clones = encoder->possible_clones;
return 0;
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 919e78d45ab0..cda8bfab6d3b 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -122,10 +122,12 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
count = 0;
crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr);
drm_for_each_crtc(crtc, dev) {
- if (count < card_res->count_crtcs &&
- put_user(crtc->base.id, crtc_id + count))
- return -EFAULT;
- count++;
+ if (drm_lease_held(file_priv, crtc->base.id)) {
+ if (count < card_res->count_crtcs &&
+ put_user(crtc->base.id, crtc_id + count))
+ return -EFAULT;
+ count++;
+ }
}
card_res->count_crtcs = count;
@@ -143,12 +145,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
count = 0;
connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
drm_for_each_connector_iter(connector, &conn_iter) {
- if (count < card_res->count_connectors &&
- put_user(connector->base.id, connector_id + count)) {
- drm_connector_list_iter_end(&conn_iter);
- return -EFAULT;
+ if (drm_lease_held(file_priv, connector->base.id)) {
+ if (count < card_res->count_connectors &&
+ put_user(connector->base.id, connector_id + count)) {
+ drm_connector_list_iter_end(&conn_iter);
+ return -EFAULT;
+ }
+ count++;
}
- count++;
}
card_res->count_connectors = count;
drm_connector_list_iter_end(&conn_iter);
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index 240a05d91a53..d1599f36b605 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -104,6 +104,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_PLANE:
+ 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)
@@ -117,6 +136,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 c10869bad729..20401baf5909 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -479,10 +479,12 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data,
!file_priv->universal_planes)
continue;
- if (count < config->num_total_plane &&
- put_user(plane->base.id, plane_ptr + count))
- return -EFAULT;
- count++;
+ if (drm_lease_held(file_priv, plane->base.id)) {
+ if (count < config->num_total_plane &&
+ put_user(plane->base.id, plane_ptr + count))
+ return -EFAULT;
+ count++;
+ }
}
plane_resp->count_planes = count;
@@ -504,9 +506,9 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
return -ENOENT;
drm_modeset_lock(&plane->mutex, NULL);
- if (plane->state && plane->state->crtc)
+ if (plane->state && plane->state->crtc && drm_lease_held(file_priv, plane->state->crtc->base.id))
plane_resp->crtc_id = plane->state->crtc->base.id;
- else if (!plane->state && plane->crtc)
+ else if (!plane->state && 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;
@@ -520,7 +522,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;
/*
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index deb080f62a29..4e4569679f44 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1447,10 +1447,12 @@ static void drm_wait_vblank_reply(struct drm_device *dev, unsigned int pipe,
int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
+ struct drm_crtc *crtc;
struct drm_vblank_crtc *vblank;
union drm_wait_vblank *vblwait = data;
int ret;
u64 req_seq, seq;
+ unsigned int pipe_index;
unsigned int flags, pipe, high_pipe;
if (!dev->irq_enabled)
@@ -1472,9 +1474,21 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
flags = vblwait->request.type & _DRM_VBLANK_FLAGS_MASK;
high_pipe = (vblwait->request.type & _DRM_VBLANK_HIGH_CRTC_MASK);
if (high_pipe)
- pipe = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
+ pipe_index = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
else
- pipe = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
+ pipe_index = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
+
+ /* Convert lease-relative crtc index into global crtc index */
+ pipe = 0;
+ drm_for_each_crtc(crtc, dev) {
+ if (drm_lease_held(file_priv, crtc->base.id)) {
+ if (pipe_index == 0)
+ break;
+ pipe_index--;
+ }
+ pipe++;
+ }
+
if (pipe >= dev->num_crtcs)
return -EINVAL;
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
index a49667db1d6d..0981631b9aed 100644
--- a/include/drm/drm_lease.h
+++ b/include/drm/drm_lease.h
@@ -31,6 +31,4 @@ void _drm_lease_revoke(struct drm_master *master);
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_ */
--
2.15.0.rc0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
2017-10-13 1:56 [PATCH 0/5]: drm: Add drm mode object leases Keith Packard
` (3 preceding siblings ...)
2017-10-13 1:56 ` [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3] Keith Packard
@ 2017-10-13 1:56 ` Keith Packard
2017-10-16 21:03 ` Sean Paul
2017-10-16 9:13 ` [PATCH 0/5]: drm: Add drm mode object leases Daniel Vetter
5 siblings, 1 reply; 16+ messages in thread
From: Keith Packard @ 2017-10-13 1:56 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
drm_mode_revoke_lease
Erase the set of objects managed by a lease.
This should suffice to at least create and query leases.
Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:
* query ioctls only query the master associated with
the provided file.
* 'mask_lease' value has been removed
* change ioctl has been removed.
Changes for v3 suggested in part by Dave Airlie <airlied@gmail.com>
* Add revoke ioctl.
Changes for v4 suggested by Dave Airlie <airlied@gmail.com>
* Expand on the comment about the magic use of &drm_lease_idr_object
* Pad lease ioctl structures to align on 64-bit boundaries
Changes for v5 suggested by Dave Airlie <airlied@gmail.com>
* Check for non-negative object_id in create_lease to avoid debug
output from the kernel.
Changes for v5 provided by Dave Airlie <airlied@gmail.com>
* For non-universal planes add primary/cursor planes to lease
If we aren't exposing universal planes to this userspace client,
and it requests a lease on a crtc, we should implicitly export the
primary and cursor planes for the crtc.
If the lessee doesn't request universal planes, it will just see
the crtc, but if it does request them it will then see the plane
objects as well.
This also moves the object look ups earlier as a side effect, so
we'd exit the ioctl quicker for non-existant objects.
* Restrict leases to crtc/connector/planes.
This only allows leasing for objects we wish to allow.
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
---
drivers/gpu/drm/drm_ioctl.c | 4 +
drivers/gpu/drm/drm_lease.c | 318 ++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_mode_object.c | 5 +-
include/drm/drm_lease.h | 12 ++
include/drm/drm_mode_object.h | 2 +
include/uapi/drm/drm.h | 5 +
include/uapi/drm/drm_mode.h | 66 ++++++++
7 files changed, 410 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 9c435a4c0c82..4aafe4802099 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+ DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
};
#define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 6c3f2445254c..88c213f9c4ab 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -23,6 +23,8 @@
#define drm_for_each_lessee(lessee, lessor) \
list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
+static uint64_t drm_lease_idr_object;
+
/**
* drm_lease_owner - return ancestor owner drm_master
* @master: drm_master somewhere within tree of lessees and lessors
@@ -337,3 +339,319 @@ void _drm_lease_revoke(struct drm_master *top)
}
}
}
+
+/**
+ * drm_mode_create_lease_ioctl - create a new lease
+ * @dev: the drm device
+ * @data: pointer to struct drm_mode_create_lease
+ * @file_priv: the file being manipulated
+ *
+ * The master associated with the specified file will have a lease
+ * created containing the objects specified in the ioctl structure.
+ * A file descriptor will be allocated for that and returned to the
+ * application.
+ */
+int drm_mode_create_lease_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *lessor_priv)
+{
+ struct drm_mode_create_lease *cl = data;
+ size_t object_count;
+ size_t o;
+ int ret = 0;
+ struct idr leases;
+ struct drm_master *lessor = lessor_priv->master;
+ struct drm_master *lessee = NULL;
+ struct file *lessee_file = NULL;
+ struct file *lessor_file = lessor_priv->filp;
+ struct drm_file *lessee_priv;
+ int fd = -1;
+
+ /* Do not allow sub-leases */
+ if (lessor->lessor)
+ return -EINVAL;
+
+ object_count = cl->object_count;
+ idr_init(&leases);
+
+ /* Allocate a file descriptor for the lease */
+ fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK));
+
+ DRM_DEBUG_LEASE("Creating new lease\n");
+
+ /* Lookup the mode objects and add their IDs to the lease request */
+ for (o = 0; o < object_count; o++) {
+ __u32 object_id;
+ struct drm_mode_object *obj;
+
+ if (copy_from_user(&object_id,
+ u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32),
+ sizeof (__u32))) {
+ ret = -EFAULT;
+ goto out_leases;
+ }
+ DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
+
+ if ((int) object_id < 0) {
+ ret = -EINVAL;
+ goto out_leases;
+ }
+
+ obj = drm_mode_object_find(dev, lessor_priv, object_id,
+ DRM_MODE_OBJECT_ANY);
+ if (!obj) {
+ ret = -ENOENT;
+ goto out_leases;
+ }
+
+ /* only allow leasing on crtc/plane/connector objects */
+ if (!drm_mode_object_lease_required(obj->type)) {
+ ret = -EINVAL;
+ drm_mode_object_put(obj);
+ goto out_leases;
+ }
+
+ /*
+ * We're using an IDR to hold the set of leased
+ * objects, but we don't need to point at the object's
+ * data structure from the lease as the main crtc_idr
+ * will be used to actually find that. Instead, all we
+ * really want is a 'leased/not-leased' result, for
+ * which any non-NULL pointer will work fine.
+ */
+ ret = idr_alloc(&leases, &drm_lease_idr_object , object_id, object_id + 1, GFP_KERNEL);
+ if (ret < 0) {
+ DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
+ object_id, ret);
+ drm_mode_object_put(obj);
+ goto out_leases;
+ }
+ if (obj->type == DRM_MODE_OBJECT_CRTC && !lessor_priv->universal_planes) {
+ struct drm_crtc *crtc = obj_to_crtc(obj);
+ ret = idr_alloc(&leases, &drm_lease_idr_object, crtc->primary->base.id, crtc->primary->base.id + 1, GFP_KERNEL);
+ if (ret < 0) {
+ DRM_DEBUG_LEASE("Object primary plane %d cannot be inserted into leases (%d)\n",
+ object_id, ret);
+ drm_mode_object_put(obj);
+ goto out_leases;
+ }
+ if (crtc->cursor) {
+ ret = idr_alloc(&leases, &drm_lease_idr_object, crtc->cursor->base.id, crtc->cursor->base.id + 1, GFP_KERNEL);
+ if (ret < 0) {
+ DRM_DEBUG_LEASE("Object cursor plane %d cannot be inserted into leases (%d)\n",
+ object_id, ret);
+ drm_mode_object_put(obj);
+ goto out_leases;
+ }
+ }
+ }
+ drm_mode_object_put(obj);
+ }
+
+ mutex_lock(&dev->master_mutex);
+
+ DRM_DEBUG_LEASE("Creating lease\n");
+ lessee = drm_lease_create(lessor, &leases);
+
+ if (IS_ERR(lessee)) {
+ ret = PTR_ERR(lessee);
+ mutex_unlock(&dev->master_mutex);
+ goto out_leases;
+ }
+
+ /* Clone the lessor file to create a new file for us */
+ DRM_DEBUG_LEASE("Allocating lease file\n");
+ path_get(&lessor_file->f_path);
+ lessee_file = alloc_file(&lessor_file->f_path,
+ lessor_file->f_mode,
+ fops_get(lessor_file->f_inode->i_fop));
+ mutex_unlock(&dev->master_mutex);
+
+ if (IS_ERR(lessee_file)) {
+ ret = PTR_ERR(lessee_file);
+ goto out_lessee;
+ }
+
+ /* Initialize the new file for DRM */
+ DRM_DEBUG_LEASE("Initializing the file with %p\n", lessee_file->f_op->open);
+ ret = lessee_file->f_op->open(lessee_file->f_inode, lessee_file);
+ if (ret)
+ goto out_lessee_file;
+
+ lessee_priv = lessee_file->private_data;
+
+ /* Change the file to a master one */
+ drm_master_put(&lessee_priv->master);
+ lessee_priv->master = lessee;
+ lessee_priv->is_master = 1;
+ lessee_priv->authenticated = 1;
+
+ /* Hook up the fd */
+ fd_install(fd, lessee_file);
+
+ /* Pass fd back to userspace */
+ DRM_DEBUG_LEASE("Returning fd %d id %d\n", fd, lessee->lessee_id);
+ cl->fd = fd;
+ cl->lessee_id = lessee->lessee_id;
+
+ DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n");
+ return 0;
+
+out_lessee_file:
+ fput(lessee_file);
+
+out_lessee:
+ drm_master_put(&lessee);
+
+out_leases:
+ idr_destroy(&leases);
+ put_unused_fd(fd);
+
+ DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret);
+ return ret;
+}
+
+/**
+ * drm_mode_list_lessees_ioctl - list lessee ids
+ * @dev: the drm device
+ * @data: pointer to struct drm_mode_list_lessees
+ * @lessor_priv: the file being manipulated
+ *
+ * Starting from the master associated with the specified file,
+ * the master with the provided lessee_id is found, and then
+ * an array of lessee ids associated with leases from that master
+ * are returned.
+ */
+
+int drm_mode_list_lessees_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *lessor_priv)
+{
+ struct drm_mode_list_lessees *arg = data;
+ __u32 __user *lessee_ids = (__u32 __user *) (uintptr_t) (arg->lessees_ptr);
+ __u32 count_lessees = arg->count_lessees;
+ struct drm_master *lessor = lessor_priv->master, *lessee;
+ int count;
+ int ret = 0;
+
+ DRM_DEBUG_LEASE("List lessees for %d\n", lessor->lessee_id);
+
+ mutex_lock(&dev->master_mutex);
+
+ count = 0;
+ drm_for_each_lessee(lessee, lessor) {
+ /* Only list un-revoked leases */
+ if (!idr_is_empty(&lessee->leases)) {
+ if (count_lessees > count) {
+ DRM_DEBUG_LEASE("Add lessee %d\n", lessee->lessee_id);
+ ret = put_user(lessee->lessee_id, lessee_ids + count);
+ if (ret)
+ break;
+ }
+ count++;
+ }
+ }
+
+ DRM_DEBUG_LEASE("Lessor leases to %d\n", count);
+ if (ret == 0)
+ arg->count_lessees = count;
+
+ mutex_unlock(&dev->master_mutex);
+
+ return ret;
+}
+
+/**
+ * drm_mode_get_lease_ioctl - list leased objects
+ * @dev: the drm device
+ * @data: pointer to struct drm_mode_get_lease
+ * @file_priv: the file being manipulated
+ *
+ * Return the list of leased objects for the specified lessee
+ */
+
+int drm_mode_get_lease_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *lessee_priv)
+{
+ struct drm_mode_get_lease *arg = data;
+ __u32 __user *object_ids = (__u32 __user *) (uintptr_t) (arg->objects_ptr);
+ __u32 count_objects = arg->count_objects;
+ struct drm_master *lessee = lessee_priv->master;
+ struct idr *object_idr;
+ int count;
+ void *entry;
+ int object;
+ int ret = 0;
+
+ DRM_DEBUG_LEASE("get lease for %d\n", lessee->lessee_id);
+
+ mutex_lock(&dev->master_mutex);
+
+ if (lessee->lessor == NULL)
+ /* owner can use all objects */
+ object_idr = &lessee->dev->mode_config.crtc_idr;
+ else
+ /* lessee can only use allowed object */
+ object_idr = &lessee->leases;
+
+ count = 0;
+ idr_for_each_entry(object_idr, entry, object) {
+ if (count_objects > count) {
+ DRM_DEBUG_LEASE("adding object %d\n", object);
+ ret = put_user(object, object_ids + count);
+ if (ret)
+ break;
+ }
+ count++;
+ }
+
+ DRM_DEBUG("lease holds %d objects\n", count);
+ if (ret == 0)
+ arg->count_objects = count;
+
+ mutex_unlock(&dev->master_mutex);
+
+ return ret;
+}
+
+/**
+ * drm_mode_revoke_lease_ioctl - revoke lease
+ * @dev: the drm device
+ * @data: pointer to struct drm_mode_revoke_lease
+ * @file_priv: the file being manipulated
+ *
+ * This removes all of the objects from the lease without
+ * actually getting rid of the lease itself; that way all
+ * references to it still work correctly
+ */
+int drm_mode_revoke_lease_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *lessor_priv)
+{
+ struct drm_mode_revoke_lease *arg = data;
+ struct drm_master *lessor = lessor_priv->master;
+ struct drm_master *lessee;
+ int ret = 0;
+
+ DRM_DEBUG_LEASE("revoke lease for %d\n", arg->lessee_id);
+
+ mutex_lock(&dev->master_mutex);
+
+ lessee = _drm_find_lessee(lessor, arg->lessee_id);
+
+ /* No such lessee */
+ if (!lessee) {
+ ret = -ENOENT;
+ goto fail;
+ }
+
+ /* Lease is not held by lessor */
+ if (lessee->lessor != lessor) {
+ ret = -EACCES;
+ goto fail;
+ }
+
+ _drm_lease_revoke(lessee);
+
+fail:
+ mutex_unlock(&dev->master_mutex);
+
+ return ret;
+}
diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
index d1599f36b605..7c8b2698c6a7 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -111,7 +111,7 @@ void drm_mode_object_unregister(struct drm_device *dev,
* Returns whether the provided type of drm_mode_object must
* be owned or leased to be used by a process.
*/
-static bool drm_lease_required(uint32_t type)
+bool drm_mode_object_lease_required(uint32_t type)
{
switch(type) {
case DRM_MODE_OBJECT_CRTC:
@@ -136,7 +136,8 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
if (obj && obj->id != id)
obj = NULL;
- if (obj && drm_lease_required(obj->type) && !_drm_lease_held(file_priv, obj->id))
+ if (obj && drm_mode_object_lease_required(obj->type) &&
+ !_drm_lease_held(file_priv, obj->id))
obj = NULL;
if (obj && obj->free_cb) {
diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
index 0981631b9aed..d2bcd1ea6cdc 100644
--- a/include/drm/drm_lease.h
+++ b/include/drm/drm_lease.h
@@ -31,4 +31,16 @@ void _drm_lease_revoke(struct drm_master *master);
uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs);
+int drm_mode_create_lease_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+
+int drm_mode_list_lessees_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+
+int drm_mode_get_lease_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+
+int drm_mode_revoke_lease_ioctl(struct drm_device *dev,
+ void *data, struct drm_file *file_priv);
+
#endif /* _DRM_LEASE_H_ */
diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
index c8155cb5a932..7ba3913f30b5 100644
--- a/include/drm/drm_mode_object.h
+++ b/include/drm/drm_mode_object.h
@@ -154,4 +154,6 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
void drm_object_attach_property(struct drm_mode_object *obj,
struct drm_property *property,
uint64_t init_val);
+
+bool drm_mode_object_lease_required(uint32_t type);
#endif
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index a106f6a7a0f9..9c02d2125d07 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -888,6 +888,11 @@ extern "C" {
#define DRM_IOCTL_SYNCOBJ_RESET DRM_IOWR(0xC4, struct drm_syncobj_array)
#define DRM_IOCTL_SYNCOBJ_SIGNAL DRM_IOWR(0xC5, struct drm_syncobj_array)
+#define DRM_IOCTL_MODE_CREATE_LEASE DRM_IOWR(0xC6, struct drm_mode_create_lease)
+#define DRM_IOCTL_MODE_LIST_LESSEES DRM_IOWR(0xC7, struct drm_mode_list_lessees)
+#define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct drm_mode_get_lease)
+#define DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct drm_mode_revoke_lease)
+
/**
* Device specific ioctls should only be in their respective headers
* The device specific ioctl range is from 0x40 to 0x9f.
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 34b6bb34b002..5597a87154e5 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -782,6 +782,72 @@ struct drm_mode_destroy_blob {
__u32 blob_id;
};
+/**
+ * Lease mode resources, creating another drm_master.
+ */
+struct drm_mode_create_lease {
+ /** Pointer to array of object ids (__u32) */
+ __u64 object_ids;
+ /** Number of object ids */
+ __u32 object_count;
+ /** flags for new FD (O_CLOEXEC, etc) */
+ __u32 flags;
+
+ /** Return: unique identifier for lessee. */
+ __u32 lessee_id;
+ /** Return: file descriptor to new drm_master file */
+ __u32 fd;
+};
+
+/**
+ * List lesses from a drm_master
+ */
+struct drm_mode_list_lessees {
+ /** Number of lessees.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+ __u32 count_lessees;
+ __u32 pad;
+
+ /** Pointer to lessees.
+ * pointer to __u64 array of lessee ids
+ */
+ __u64 lessees_ptr;
+};
+
+/**
+ * Get leased objects
+ */
+struct drm_mode_get_lease {
+ /** Number of leased objects.
+ * On input, provides length of the array.
+ * On output, provides total number. No
+ * more than the input number will be written
+ * back, so two calls can be used to get
+ * the size and then the data.
+ */
+ __u32 count_objects;
+ __u32 pad;
+
+ /** Pointer to objects.
+ * pointer to __u32 array of object ids
+ */
+ __u64 objects_ptr;
+};
+
+/**
+ * Revoke lease
+ */
+struct drm_mode_revoke_lease {
+ /** Unique ID of lessee
+ */
+ __u32 lessee_id;
+};
+
#if defined(__cplusplus)
}
#endif
--
2.15.0.rc0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5]: drm: Add drm mode object leases
2017-10-13 1:56 [PATCH 0/5]: drm: Add drm mode object leases Keith Packard
` (4 preceding siblings ...)
2017-10-13 1:56 ` [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6] Keith Packard
@ 2017-10-16 9:13 ` Daniel Vetter
2017-10-16 17:52 ` Keith Packard
5 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2017-10-16 9:13 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
On Thu, Oct 12, 2017 at 06:56:26PM -0700, Keith Packard wrote:
> New since last time:
>
> * Don't lease encoders
> * Do lease planes
> * Automatically lease primary and cursor planes for
> apps which don't set universal_planes
> * Restrict leases to only contain objects which
> are actually leasable (connectors, crtcs and planes)
> * Drop the patch which changes permissions on get resources
> ioctls
A bit lacking time to do an in-depth review, but all my major concerns
seem to be addressed. On the series.
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5]: drm: Add drm mode object leases
2017-10-16 9:13 ` [PATCH 0/5]: drm: Add drm mode object leases Daniel Vetter
@ 2017-10-16 17:52 ` Keith Packard
0 siblings, 0 replies; 16+ messages in thread
From: Keith Packard @ 2017-10-16 17:52 UTC (permalink / raw)
To: Daniel Vetter; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
[-- Attachment #1: Type: text/plain, Size: 438 bytes --]
Daniel Vetter <daniel@ffwll.ch> writes:
> A bit lacking time to do an in-depth review, but all my major concerns
> seem to be addressed. On the series.
Thanks much.
I sent out a note over the weekend about how we can make the Vulkan API
work without the permissions change in the kernel. We need to provide
access to a DRM master and can choose to either use the X server's or
pass a master FD in explicitly.
--
-keith
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/5] drm/plane: drop num_overlay_planes (v2)
2017-10-13 1:56 ` [PATCH 1/5] drm/plane: drop num_overlay_planes (v2) Keith Packard
@ 2017-10-16 19:23 ` Sean Paul
0 siblings, 0 replies; 16+ messages in thread
From: Sean Paul @ 2017-10-16 19:23 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
On Thu, Oct 12, 2017 at 06:56:27PM -0700, Keith Packard wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> In order to implement plane leasing we need to count things,
> just make the code consistent with the counting code currently
> used for counting crtcs/encoders/connectors and drop the need
> for num_overlay_planes.
>
> v2: don't forget to assign plane_ptr. (keithp)
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/drm_mode_config.c | 1 -
> drivers/gpu/drm/drm_plane.c | 46 ++++++++++++++-------------------------
> include/drm/drm_mode_config.h | 13 -----------
> 3 files changed, 16 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 74f6ff5df656..919e78d45ab0 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -385,7 +385,6 @@ void drm_mode_config_init(struct drm_device *dev)
> dev->mode_config.num_connector = 0;
> dev->mode_config.num_crtc = 0;
> dev->mode_config.num_encoder = 0;
> - dev->mode_config.num_overlay_plane = 0;
> dev->mode_config.num_total_plane = 0;
> }
> EXPORT_SYMBOL(drm_mode_config_init);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 574fd1475214..c10869bad729 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -241,8 +241,6 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
>
> list_add_tail(&plane->head, &config->plane_list);
> plane->index = config->num_total_plane++;
> - if (plane->type == DRM_PLANE_TYPE_OVERLAY)
> - config->num_overlay_plane++;
>
> drm_object_attach_property(&plane->base,
> config->plane_type_property,
> @@ -353,8 +351,6 @@ void drm_plane_cleanup(struct drm_plane *plane)
>
> list_del(&plane->head);
> dev->mode_config.num_total_plane--;
> - if (plane->type == DRM_PLANE_TYPE_OVERLAY)
> - dev->mode_config.num_overlay_plane--;
>
> WARN_ON(plane->state && !plane->funcs->atomic_destroy_state);
> if (plane->state && plane->funcs->atomic_destroy_state)
> @@ -462,43 +458,33 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data,
> struct drm_mode_config *config;
> struct drm_plane *plane;
> uint32_t __user *plane_ptr;
> - int copied = 0;
> - unsigned num_planes;
> + int count = 0;
>
> if (!drm_core_check_feature(dev, DRIVER_MODESET))
> return -EINVAL;
>
> config = &dev->mode_config;
> -
> - if (file_priv->universal_planes)
> - num_planes = config->num_total_plane;
> - else
> - num_planes = config->num_overlay_plane;
> + plane_ptr = u64_to_user_ptr(plane_resp->plane_id_ptr);
>
> /*
> * This ioctl is called twice, once to determine how much space is
> * needed, and the 2nd time to fill it.
> */
> - if (num_planes &&
> - (plane_resp->count_planes >= num_planes)) {
> - plane_ptr = (uint32_t __user *)(unsigned long)plane_resp->plane_id_ptr;
> -
> - /* Plane lists are invariant, no locking needed. */
> - drm_for_each_plane(plane, dev) {
> - /*
> - * Unless userspace set the 'universal planes'
> - * capability bit, only advertise overlays.
> - */
> - if (plane->type != DRM_PLANE_TYPE_OVERLAY &&
> - !file_priv->universal_planes)
> - continue;
> -
> - if (put_user(plane->base.id, plane_ptr + copied))
> - return -EFAULT;
> - copied++;
> - }
> + drm_for_each_plane(plane, dev) {
> + /*
> + * Unless userspace set the 'universal planes'
> + * capability bit, only advertise overlays.
> + */
> + if (plane->type != DRM_PLANE_TYPE_OVERLAY &&
> + !file_priv->universal_planes)
> + continue;
> +
> + if (count < config->num_total_plane &&
> + put_user(plane->base.id, plane_ptr + count))
> + return -EFAULT;
> + count++;
> }
> - plane_resp->count_planes = num_planes;
> + plane_resp->count_planes = count;
>
> return 0;
> }
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 1b37368416c8..0b4ac2ebc610 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -429,19 +429,6 @@ struct drm_mode_config {
> */
> struct list_head encoder_list;
>
> - /**
> - * @num_overlay_plane:
> - *
> - * Number of overlay planes on this device, excluding primary and cursor
> - * planes.
> - *
> - * Track number of overlay planes separately from number of total
> - * planes. By default we only advertise overlay planes to userspace; if
> - * userspace sets the "universal plane" capability bit, we'll go ahead
> - * and expose all planes. This is invariant over the lifetime of a
> - * device and hence doesn't need any locks.
> - */
> - int num_overlay_plane;
> /**
> * @num_total_plane:
> *
> --
> 2.15.0.rc0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] drm: Add new LEASE debug level
2017-10-13 1:56 ` [PATCH 2/5] drm: Add new LEASE debug level Keith Packard
@ 2017-10-16 19:24 ` Sean Paul
0 siblings, 0 replies; 16+ messages in thread
From: Sean Paul @ 2017-10-16 19:24 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
On Thu, Oct 12, 2017 at 06:56:28PM -0700, Keith Packard wrote:
> Separate out lease debugging from the core.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
> 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 c0292e5d7281..a934fd5e7e55 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 7277783a4ff0..59be1232d005 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -136,6 +136,7 @@ struct pci_controller;
> #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 */
> @@ -250,6 +251,9 @@ struct pci_controller;
> #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.15.0.rc0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
2017-10-13 1:56 ` [PATCH 3/5] drm: Add drm_object lease infrastructure [v4] Keith Packard
@ 2017-10-16 19:44 ` Sean Paul
2017-10-16 20:42 ` Keith Packard
0 siblings, 1 reply; 16+ messages in thread
From: Sean Paul @ 2017-10-16 19:44 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
On Thu, Oct 12, 2017 at 06:56:29PM -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.
>
> 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.
>
> Changes in v3, some suggested by Dave Airlie <airlied@gmail.com>
>
> * Add revocation. This allows leases to be effectively revoked by
> removing all of the objects they have access to. The lease itself
> hangs around as it's hanging off a file.
>
> * Free the leases IDR when the master is destroyed
>
> * _drm_lease_held should look at lessees, not lessor
>
> * Allow non-master files to check for lease status
>
> Changes in v4, suggested by Dave Airlie <airlied@gmail.com>
>
> * Formatting and whitespace changes
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
> drivers/gpu/drm/Makefile | 2 +-
> drivers/gpu/drm/drm_auth.c | 29 +++-
> drivers/gpu/drm/drm_lease.c | 339 ++++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_auth.h | 20 +++
> include/drm/drm_lease.h | 36 +++++
> include/drm/drm_mode_object.h | 1 +
> 6 files changed, 425 insertions(+), 2 deletions(-)
> create mode 100644 drivers/gpu/drm/drm_lease.c
> create mode 100644 include/drm/drm_lease.h
>
<snip />
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> new file mode 100644
> index 000000000000..6c3f2445254c
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_lease.c
<snip />
> +
> +/**
> + * _drm_lease_revoke - revoke access to all leased objects
Can you add "(idr_mutex held)" like you have in _drm_lease_held()?
> + * @master: the master losing its lease
s/master/top/
> + */
> +
> +void _drm_lease_revoke(struct drm_master *top)
> +{
> + int object;
> + void *entry;
> + struct drm_master *master = top;
> +
lockdep_assert_held(&top->dev->mode_config.idr_mutex);
With these nits fixed,
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> + /*
> + * Walk the tree starting at 'top' emptying all leases. Because
> + * the tree is fully connected, we can do this without recursing
> + */
> + for (;;) {
> + DRM_DEBUG_LEASE("revoke leases for %p %d\n", master, master->lessee_id);
> +
> + /* Evacuate the lease */
> + idr_for_each_entry(&master->leases, entry, object)
> + idr_remove(&master->leases, object);
> +
> + /* Depth-first list walk */
> +
> + /* Down */
> + if (!list_empty(&master->lessees)) {
> + master = list_first_entry(&master->lessees, struct drm_master, lessee_list);
> + } else {
> + /* Up */
> + while (master != top && master == list_last_entry(&master->lessor->lessees, struct drm_master, lessee_list))
> + master = master->lessor;
> +
> + if (master == top)
> + break;
> +
> + /* Over */
> + master = list_entry(master->lessee_list.next, struct drm_master, lessee_list);
> + }
> + }
> +}
<snip />
> --
> 2.15.0.rc0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3]
2017-10-13 1:56 ` [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3] Keith Packard
@ 2017-10-16 20:34 ` Sean Paul
0 siblings, 0 replies; 16+ messages in thread
From: Sean Paul @ 2017-10-16 20:34 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
On Thu, Oct 12, 2017 at 06:56:30PM -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.
>
> 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.
>
> Changes for v3 provided by Dave Airlie <airlied@redhat.com>
>
> * remove support for leasing encoders.
> * add support for leasing planes.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/drm_auth.c | 2 +-
> drivers/gpu/drm/drm_encoder.c | 5 +++--
> drivers/gpu/drm/drm_mode_config.c | 22 +++++++++++++---------
> drivers/gpu/drm/drm_mode_object.c | 22 ++++++++++++++++++++++
> drivers/gpu/drm/drm_plane.c | 18 +++++++++++-------
> drivers/gpu/drm/drm_vblank.c | 18 ++++++++++++++++--
> include/drm/drm_lease.h | 2 --
> 7 files changed, 66 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
> index 541177c71d51..bab26b477738 100644
> --- a/drivers/gpu/drm/drm_auth.c
> +++ b/drivers/gpu/drm/drm_auth.c
> @@ -310,7 +310,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_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index 43f644844b83..59e0ebe733f8 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -226,7 +226,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;
> @@ -234,7 +234,8 @@ 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_crtcs = drm_lease_filter_crtcs(file_priv,
> + encoder->possible_crtcs);
> enc_resp->possible_clones = encoder->possible_clones;
>
> return 0;
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 919e78d45ab0..cda8bfab6d3b 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -122,10 +122,12 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
> count = 0;
> crtc_id = u64_to_user_ptr(card_res->crtc_id_ptr);
> drm_for_each_crtc(crtc, dev) {
> - if (count < card_res->count_crtcs &&
> - put_user(crtc->base.id, crtc_id + count))
> - return -EFAULT;
> - count++;
> + if (drm_lease_held(file_priv, crtc->base.id)) {
> + if (count < card_res->count_crtcs &&
> + put_user(crtc->base.id, crtc_id + count))
> + return -EFAULT;
> + count++;
> + }
> }
> card_res->count_crtcs = count;
>
> @@ -143,12 +145,14 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
> count = 0;
> connector_id = u64_to_user_ptr(card_res->connector_id_ptr);
> drm_for_each_connector_iter(connector, &conn_iter) {
> - if (count < card_res->count_connectors &&
> - put_user(connector->base.id, connector_id + count)) {
> - drm_connector_list_iter_end(&conn_iter);
> - return -EFAULT;
> + if (drm_lease_held(file_priv, connector->base.id)) {
> + if (count < card_res->count_connectors &&
> + put_user(connector->base.id, connector_id + count)) {
> + drm_connector_list_iter_end(&conn_iter);
> + return -EFAULT;
> + }
> + count++;
> }
> - count++;
> }
> card_res->count_connectors = count;
> drm_connector_list_iter_end(&conn_iter);
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index 240a05d91a53..d1599f36b605 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -104,6 +104,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_PLANE:
> + 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)
> @@ -117,6 +136,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 c10869bad729..20401baf5909 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -479,10 +479,12 @@ int drm_mode_getplane_res(struct drm_device *dev, void *data,
> !file_priv->universal_planes)
> continue;
>
> - if (count < config->num_total_plane &&
> - put_user(plane->base.id, plane_ptr + count))
> - return -EFAULT;
> - count++;
> + if (drm_lease_held(file_priv, plane->base.id)) {
> + if (count < config->num_total_plane &&
> + put_user(plane->base.id, plane_ptr + count))
> + return -EFAULT;
> + count++;
> + }
> }
> plane_resp->count_planes = count;
>
> @@ -504,9 +506,9 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
> return -ENOENT;
>
> drm_modeset_lock(&plane->mutex, NULL);
> - if (plane->state && plane->state->crtc)
> + if (plane->state && plane->state->crtc && drm_lease_held(file_priv, plane->state->crtc->base.id))
> plane_resp->crtc_id = plane->state->crtc->base.id;
> - else if (!plane->state && plane->crtc)
> + else if (!plane->state && 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;
> @@ -520,7 +522,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;
>
> /*
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index deb080f62a29..4e4569679f44 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1447,10 +1447,12 @@ static void drm_wait_vblank_reply(struct drm_device *dev, unsigned int pipe,
> int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
> {
> + struct drm_crtc *crtc;
> struct drm_vblank_crtc *vblank;
> union drm_wait_vblank *vblwait = data;
> int ret;
> u64 req_seq, seq;
> + unsigned int pipe_index;
> unsigned int flags, pipe, high_pipe;
>
> if (!dev->irq_enabled)
> @@ -1472,9 +1474,21 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
> flags = vblwait->request.type & _DRM_VBLANK_FLAGS_MASK;
> high_pipe = (vblwait->request.type & _DRM_VBLANK_HIGH_CRTC_MASK);
> if (high_pipe)
> - pipe = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
> + pipe_index = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT;
> else
> - pipe = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
> + pipe_index = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
> +
> + /* Convert lease-relative crtc index into global crtc index */
> + pipe = 0;
> + drm_for_each_crtc(crtc, dev) {
> + if (drm_lease_held(file_priv, crtc->base.id)) {
> + if (pipe_index == 0)
> + break;
> + pipe_index--;
> + }
> + pipe++;
> + }
> +
> if (pipe >= dev->num_crtcs)
> return -EINVAL;
>
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> index a49667db1d6d..0981631b9aed 100644
> --- a/include/drm/drm_lease.h
> +++ b/include/drm/drm_lease.h
> @@ -31,6 +31,4 @@ void _drm_lease_revoke(struct drm_master *master);
>
> 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_ */
> --
> 2.15.0.rc0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
2017-10-16 19:44 ` Sean Paul
@ 2017-10-16 20:42 ` Keith Packard
2017-10-16 21:05 ` Sean Paul
0 siblings, 1 reply; 16+ messages in thread
From: Keith Packard @ 2017-10-16 20:42 UTC (permalink / raw)
To: Sean Paul; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 131 bytes --]
Sean Paul <seanpaul@chromium.org> writes:
> With these nits fixed,
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
Like this?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-drm-Mark-functions-requiring-idr_mutex.-Add-lockdep-.patch --]
[-- Type: text/x-diff, Size: 2063 bytes --]
From 0aa52dd5a0873831c79c14942075354c041e5bed Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Mon, 16 Oct 2017 13:41:20 -0700
Subject: [PATCH] drm: Mark functions requiring idr_mutex. Add lockdep to
_drm_lease_revoke
Reasonable suggestions by Sean Paul.
Signed-off-by: Keith Packard <keithp@keithp.com>
---
drivers/gpu/drm/drm_lease.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index 88c213f9c4ab..20694c77a2de 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -58,7 +58,9 @@ _drm_find_lessee(struct drm_master *master, int lessee_id)
}
/**
- * _drm_lease_held_master - check to see if an object is leased (or owned) by master
+ * _drm_lease_held_master - check to see if an object is leased (or
+ * owned) by master (idr_mutex held)
+ *
* @master: the master to check the lease status of
* @id: the id to check
*
@@ -77,7 +79,7 @@ static int _drm_lease_held_master(struct drm_master *master, int id)
}
/**
- * _drm_has_leased - check to see if an object has been leased
+ * _drm_has_leased - check to see if an object has been leased (idr mutex held)
* @master: the master to check the lease status of
* @id: the id to check
*
@@ -300,8 +302,8 @@ void drm_lease_destroy(struct drm_master *master)
}
/**
- * _drm_lease_revoke - revoke access to all leased objects
- * @master: the master losing its lease
+ * _drm_lease_revoke - revoke access to all leased objects (idr_mutex held)
+ * @top: the master losing its lease
*/
void _drm_lease_revoke(struct drm_master *top)
@@ -310,6 +312,7 @@ void _drm_lease_revoke(struct drm_master *top)
void *entry;
struct drm_master *master = top;
+ lockdep_assert_held(&top->dev->mode_config.idr_mutex);
/*
* Walk the tree starting at 'top' emptying all leases. Because
* the tree is fully connected, we can do this without recursing
--
2.15.0.rc0
[-- Attachment #1.3: Type: text/plain, Size: 17 bytes --]
--
-keith
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
2017-10-13 1:56 ` [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6] Keith Packard
@ 2017-10-16 21:03 ` Sean Paul
2017-10-16 21:31 ` Keith Packard
0 siblings, 1 reply; 16+ messages in thread
From: Sean Paul @ 2017-10-16 21:03 UTC (permalink / raw)
To: Keith Packard; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
On Thu, Oct 12, 2017 at 06:56:31PM -0700, Keith Packard wrote:
> drm_mode_create_lease
>
> Creates a lease for a list of drm mode objects, returning an
> fd for the new drm_master and a 64-bit identifier for the lessee
>
> drm_mode_list_lesees
>
> List the identifiers of the lessees for a master file
>
> drm_mode_get_lease
>
> List the leased objects for a master file
>
> drm_mode_revoke_lease
>
> Erase the set of objects managed by a lease.
>
> This should suffice to at least create and query leases.
>
> Changes for v2 as suggested by Daniel Vetter <daniel.vetter@ffwll.ch>:
>
> * query ioctls only query the master associated with
> the provided file.
>
> * 'mask_lease' value has been removed
>
> * change ioctl has been removed.
>
> Changes for v3 suggested in part by Dave Airlie <airlied@gmail.com>
>
> * Add revoke ioctl.
>
> Changes for v4 suggested by Dave Airlie <airlied@gmail.com>
>
> * Expand on the comment about the magic use of &drm_lease_idr_object
> * Pad lease ioctl structures to align on 64-bit boundaries
>
> Changes for v5 suggested by Dave Airlie <airlied@gmail.com>
>
> * Check for non-negative object_id in create_lease to avoid debug
> output from the kernel.
>
> Changes for v5 provided by Dave Airlie <airlied@gmail.com>
>
> * For non-universal planes add primary/cursor planes to lease
>
> If we aren't exposing universal planes to this userspace client,
> and it requests a lease on a crtc, we should implicitly export the
> primary and cursor planes for the crtc.
>
> If the lessee doesn't request universal planes, it will just see
> the crtc, but if it does request them it will then see the plane
> objects as well.
>
> This also moves the object look ups earlier as a side effect, so
> we'd exit the ioctl quicker for non-existant objects.
>
> * Restrict leases to crtc/connector/planes.
>
> This only allows leasing for objects we wish to allow.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
> drivers/gpu/drm/drm_ioctl.c | 4 +
> drivers/gpu/drm/drm_lease.c | 318 ++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_mode_object.c | 5 +-
> include/drm/drm_lease.h | 12 ++
> include/drm/drm_mode_object.h | 2 +
> include/uapi/drm/drm.h | 5 +
> include/uapi/drm/drm_mode.h | 66 ++++++++
> 7 files changed, 410 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 9c435a4c0c82..4aafe4802099 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -665,6 +665,10 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_REVOKE_LEASE, drm_mode_revoke_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> };
>
> #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index 6c3f2445254c..88c213f9c4ab 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -23,6 +23,8 @@
> #define drm_for_each_lessee(lessee, lessor) \
> list_for_each_entry((lessee), &(lessor)->lessees, lessee_list)
>
> +static uint64_t drm_lease_idr_object;
> +
> /**
> * drm_lease_owner - return ancestor owner drm_master
> * @master: drm_master somewhere within tree of lessees and lessors
> @@ -337,3 +339,319 @@ void _drm_lease_revoke(struct drm_master *top)
> }
> }
> }
> +
> +/**
> + * drm_mode_create_lease_ioctl - create a new lease
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_create_lease
> + * @file_priv: the file being manipulated
> + *
> + * The master associated with the specified file will have a lease
> + * created containing the objects specified in the ioctl structure.
> + * A file descriptor will be allocated for that and returned to the
> + * application.
> + */
> +int drm_mode_create_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessor_priv)
> +{
> + struct drm_mode_create_lease *cl = data;
> + size_t object_count;
> + size_t o;
> + int ret = 0;
> + struct idr leases;
> + struct drm_master *lessor = lessor_priv->master;
> + struct drm_master *lessee = NULL;
> + struct file *lessee_file = NULL;
> + struct file *lessor_file = lessor_priv->filp;
> + struct drm_file *lessee_priv;
> + int fd = -1;
> +
> + /* Do not allow sub-leases */
> + if (lessor->lessor)
> + return -EINVAL;
> +
> + object_count = cl->object_count;
> + idr_init(&leases);
> +
> + /* Allocate a file descriptor for the lease */
> + fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK));
> +
> + DRM_DEBUG_LEASE("Creating new lease\n");
> +
> + /* Lookup the mode objects and add their IDs to the lease request */
> + for (o = 0; o < object_count; o++) {
> + __u32 object_id;
> + struct drm_mode_object *obj;
> +
> + if (copy_from_user(&object_id,
> + u64_to_user_ptr(cl->object_ids) + o * sizeof (__u32),
> + sizeof (__u32))) {
> + ret = -EFAULT;
> + goto out_leases;
> + }
> + DRM_DEBUG_LEASE("Adding object %d to lease\n", object_id);
> +
> + if ((int) object_id < 0) {
> + ret = -EINVAL;
> + goto out_leases;
> + }
> +
> + obj = drm_mode_object_find(dev, lessor_priv, object_id,
> + DRM_MODE_OBJECT_ANY);
> + if (!obj) {
> + ret = -ENOENT;
> + goto out_leases;
> + }
> +
> + /* only allow leasing on crtc/plane/connector objects */
> + if (!drm_mode_object_lease_required(obj->type)) {
> + ret = -EINVAL;
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> +
> + /*
> + * We're using an IDR to hold the set of leased
> + * objects, but we don't need to point at the object's
> + * data structure from the lease as the main crtc_idr
> + * will be used to actually find that. Instead, all we
> + * really want is a 'leased/not-leased' result, for
> + * which any non-NULL pointer will work fine.
> + */
> + ret = idr_alloc(&leases, &drm_lease_idr_object , object_id, object_id + 1, GFP_KERNEL);
nit: space before ,
> + if (ret < 0) {
> + DRM_DEBUG_LEASE("Object %d cannot be inserted into leases (%d)\n",
> + object_id, ret);
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> + if (obj->type == DRM_MODE_OBJECT_CRTC && !lessor_priv->universal_planes) {
> + struct drm_crtc *crtc = obj_to_crtc(obj);
> + ret = idr_alloc(&leases, &drm_lease_idr_object, crtc->primary->base.id, crtc->primary->base.id + 1, GFP_KERNEL);
> + if (ret < 0) {
> + DRM_DEBUG_LEASE("Object primary plane %d cannot be inserted into leases (%d)\n",
> + object_id, ret);
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> + if (crtc->cursor) {
> + ret = idr_alloc(&leases, &drm_lease_idr_object, crtc->cursor->base.id, crtc->cursor->base.id + 1, GFP_KERNEL);
> + if (ret < 0) {
> + DRM_DEBUG_LEASE("Object cursor plane %d cannot be inserted into leases (%d)\n",
> + object_id, ret);
> + drm_mode_object_put(obj);
> + goto out_leases;
> + }
> + }
> + }
> + drm_mode_object_put(obj);
> + }
> +
> + mutex_lock(&dev->master_mutex);
> +
> + DRM_DEBUG_LEASE("Creating lease\n");
> + lessee = drm_lease_create(lessor, &leases);
> +
> + if (IS_ERR(lessee)) {
> + ret = PTR_ERR(lessee);
> + mutex_unlock(&dev->master_mutex);
> + goto out_leases;
> + }
> +
> + /* Clone the lessor file to create a new file for us */
> + DRM_DEBUG_LEASE("Allocating lease file\n");
> + path_get(&lessor_file->f_path);
Please forgive the stupid question, but where is this reference given up?
> + lessee_file = alloc_file(&lessor_file->f_path,
> + lessor_file->f_mode,
> + fops_get(lessor_file->f_inode->i_fop));
> + mutex_unlock(&dev->master_mutex);
> +
> + if (IS_ERR(lessee_file)) {
> + ret = PTR_ERR(lessee_file);
> + goto out_lessee;
> + }
> +
> + /* Initialize the new file for DRM */
> + DRM_DEBUG_LEASE("Initializing the file with %p\n", lessee_file->f_op->open);
> + ret = lessee_file->f_op->open(lessee_file->f_inode, lessee_file);
> + if (ret)
> + goto out_lessee_file;
> +
> + lessee_priv = lessee_file->private_data;
> +
> + /* Change the file to a master one */
> + drm_master_put(&lessee_priv->master);
> + lessee_priv->master = lessee;
> + lessee_priv->is_master = 1;
> + lessee_priv->authenticated = 1;
> +
> + /* Hook up the fd */
> + fd_install(fd, lessee_file);
> +
> + /* Pass fd back to userspace */
> + DRM_DEBUG_LEASE("Returning fd %d id %d\n", fd, lessee->lessee_id);
> + cl->fd = fd;
> + cl->lessee_id = lessee->lessee_id;
> +
> + DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n");
> + return 0;
> +
> +out_lessee_file:
> + fput(lessee_file);
> +
> +out_lessee:
> + drm_master_put(&lessee);
> +
> +out_leases:
> + idr_destroy(&leases);
> + put_unused_fd(fd);
> +
> + DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret);
> + return ret;
> +}
> +
> +/**
> + * drm_mode_list_lessees_ioctl - list lessee ids
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_list_lessees
> + * @lessor_priv: the file being manipulated
> + *
> + * Starting from the master associated with the specified file,
> + * the master with the provided lessee_id is found, and then
> + * an array of lessee ids associated with leases from that master
> + * are returned.
> + */
> +
> +int drm_mode_list_lessees_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessor_priv)
> +{
> + struct drm_mode_list_lessees *arg = data;
> + __u32 __user *lessee_ids = (__u32 __user *) (uintptr_t) (arg->lessees_ptr);
> + __u32 count_lessees = arg->count_lessees;
> + struct drm_master *lessor = lessor_priv->master, *lessee;
> + int count;
> + int ret = 0;
> +
> + DRM_DEBUG_LEASE("List lessees for %d\n", lessor->lessee_id);
> +
> + mutex_lock(&dev->master_mutex);
> +
> + count = 0;
> + drm_for_each_lessee(lessee, lessor) {
> + /* Only list un-revoked leases */
> + if (!idr_is_empty(&lessee->leases)) {
> + if (count_lessees > count) {
> + DRM_DEBUG_LEASE("Add lessee %d\n", lessee->lessee_id);
> + ret = put_user(lessee->lessee_id, lessee_ids + count);
> + if (ret)
> + break;
> + }
> + count++;
> + }
> + }
> +
> + DRM_DEBUG_LEASE("Lessor leases to %d\n", count);
> + if (ret == 0)
> + arg->count_lessees = count;
> +
> + mutex_unlock(&dev->master_mutex);
> +
> + return ret;
> +}
> +
> +/**
> + * drm_mode_get_lease_ioctl - list leased objects
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_get_lease
> + * @file_priv: the file being manipulated
> + *
> + * Return the list of leased objects for the specified lessee
> + */
> +
> +int drm_mode_get_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessee_priv)
> +{
> + struct drm_mode_get_lease *arg = data;
> + __u32 __user *object_ids = (__u32 __user *) (uintptr_t) (arg->objects_ptr);
> + __u32 count_objects = arg->count_objects;
> + struct drm_master *lessee = lessee_priv->master;
> + struct idr *object_idr;
> + int count;
> + void *entry;
> + int object;
> + int ret = 0;
> +
> + DRM_DEBUG_LEASE("get lease for %d\n", lessee->lessee_id);
> +
> + mutex_lock(&dev->master_mutex);
> +
> + if (lessee->lessor == NULL)
> + /* owner can use all objects */
> + object_idr = &lessee->dev->mode_config.crtc_idr;
What about other types of objects?
> + else
> + /* lessee can only use allowed object */
> + object_idr = &lessee->leases;
> +
> + count = 0;
> + idr_for_each_entry(object_idr, entry, object) {
> + if (count_objects > count) {
> + DRM_DEBUG_LEASE("adding object %d\n", object);
> + ret = put_user(object, object_ids + count);
> + if (ret)
> + break;
> + }
> + count++;
> + }
> +
> + DRM_DEBUG("lease holds %d objects\n", count);
> + if (ret == 0)
> + arg->count_objects = count;
> +
> + mutex_unlock(&dev->master_mutex);
> +
> + return ret;
> +}
> +
> +/**
> + * drm_mode_revoke_lease_ioctl - revoke lease
> + * @dev: the drm device
> + * @data: pointer to struct drm_mode_revoke_lease
> + * @file_priv: the file being manipulated
> + *
> + * This removes all of the objects from the lease without
> + * actually getting rid of the lease itself; that way all
> + * references to it still work correctly
> + */
> +int drm_mode_revoke_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *lessor_priv)
> +{
> + struct drm_mode_revoke_lease *arg = data;
> + struct drm_master *lessor = lessor_priv->master;
> + struct drm_master *lessee;
> + int ret = 0;
> +
> + DRM_DEBUG_LEASE("revoke lease for %d\n", arg->lessee_id);
> +
> + mutex_lock(&dev->master_mutex);
> +
> + lessee = _drm_find_lessee(lessor, arg->lessee_id);
> +
> + /* No such lessee */
> + if (!lessee) {
> + ret = -ENOENT;
> + goto fail;
> + }
> +
> + /* Lease is not held by lessor */
> + if (lessee->lessor != lessor) {
> + ret = -EACCES;
> + goto fail;
> + }
> +
> + _drm_lease_revoke(lessee);
> +
> +fail:
> + mutex_unlock(&dev->master_mutex);
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c
> index d1599f36b605..7c8b2698c6a7 100644
> --- a/drivers/gpu/drm/drm_mode_object.c
> +++ b/drivers/gpu/drm/drm_mode_object.c
> @@ -111,7 +111,7 @@ void drm_mode_object_unregister(struct drm_device *dev,
> * Returns whether the provided type of drm_mode_object must
> * be owned or leased to be used by a process.
> */
> -static bool drm_lease_required(uint32_t type)
> +bool drm_mode_object_lease_required(uint32_t type)
> {
> switch(type) {
> case DRM_MODE_OBJECT_CRTC:
> @@ -136,7 +136,8 @@ struct drm_mode_object *__drm_mode_object_find(struct drm_device *dev,
> if (obj && obj->id != id)
> obj = NULL;
>
> - if (obj && drm_lease_required(obj->type) && !_drm_lease_held(file_priv, obj->id))
> + if (obj && drm_mode_object_lease_required(obj->type) &&
> + !_drm_lease_held(file_priv, obj->id))
> obj = NULL;
>
> if (obj && obj->free_cb) {
> diff --git a/include/drm/drm_lease.h b/include/drm/drm_lease.h
> index 0981631b9aed..d2bcd1ea6cdc 100644
> --- a/include/drm/drm_lease.h
> +++ b/include/drm/drm_lease.h
> @@ -31,4 +31,16 @@ void _drm_lease_revoke(struct drm_master *master);
>
> uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs);
>
> +int drm_mode_create_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> +int drm_mode_list_lessees_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> +int drm_mode_get_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> +int drm_mode_revoke_lease_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> +
> #endif /* _DRM_LEASE_H_ */
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index c8155cb5a932..7ba3913f30b5 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -154,4 +154,6 @@ int drm_object_property_get_value(struct drm_mode_object *obj,
> void drm_object_attach_property(struct drm_mode_object *obj,
> struct drm_property *property,
> uint64_t init_val);
> +
> +bool drm_mode_object_lease_required(uint32_t type);
> #endif
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index a106f6a7a0f9..9c02d2125d07 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -888,6 +888,11 @@ extern "C" {
> #define DRM_IOCTL_SYNCOBJ_RESET DRM_IOWR(0xC4, struct drm_syncobj_array)
> #define DRM_IOCTL_SYNCOBJ_SIGNAL DRM_IOWR(0xC5, struct drm_syncobj_array)
>
> +#define DRM_IOCTL_MODE_CREATE_LEASE DRM_IOWR(0xC6, struct drm_mode_create_lease)
> +#define DRM_IOCTL_MODE_LIST_LESSEES DRM_IOWR(0xC7, struct drm_mode_list_lessees)
> +#define DRM_IOCTL_MODE_GET_LEASE DRM_IOWR(0xC8, struct drm_mode_get_lease)
> +#define DRM_IOCTL_MODE_REVOKE_LEASE DRM_IOWR(0xC9, struct drm_mode_revoke_lease)
> +
> /**
> * Device specific ioctls should only be in their respective headers
> * The device specific ioctl range is from 0x40 to 0x9f.
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 34b6bb34b002..5597a87154e5 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -782,6 +782,72 @@ struct drm_mode_destroy_blob {
> __u32 blob_id;
> };
>
> +/**
> + * Lease mode resources, creating another drm_master.
> + */
> +struct drm_mode_create_lease {
> + /** Pointer to array of object ids (__u32) */
> + __u64 object_ids;
> + /** Number of object ids */
> + __u32 object_count;
> + /** flags for new FD (O_CLOEXEC, etc) */
> + __u32 flags;
> +
> + /** Return: unique identifier for lessee. */
> + __u32 lessee_id;
> + /** Return: file descriptor to new drm_master file */
> + __u32 fd;
> +};
> +
> +/**
> + * List lesses from a drm_master
> + */
> +struct drm_mode_list_lessees {
> + /** Number of lessees.
> + * On input, provides length of the array.
> + * On output, provides total number. No
> + * more than the input number will be written
> + * back, so two calls can be used to get
> + * the size and then the data.
> + */
> + __u32 count_lessees;
> + __u32 pad;
> +
> + /** Pointer to lessees.
> + * pointer to __u64 array of lessee ids
> + */
> + __u64 lessees_ptr;
> +};
> +
> +/**
> + * Get leased objects
> + */
> +struct drm_mode_get_lease {
> + /** Number of leased objects.
> + * On input, provides length of the array.
> + * On output, provides total number. No
> + * more than the input number will be written
> + * back, so two calls can be used to get
> + * the size and then the data.
> + */
> + __u32 count_objects;
> + __u32 pad;
> +
> + /** Pointer to objects.
> + * pointer to __u32 array of object ids
> + */
> + __u64 objects_ptr;
> +};
> +
> +/**
> + * Revoke lease
> + */
> +struct drm_mode_revoke_lease {
> + /** Unique ID of lessee
> + */
> + __u32 lessee_id;
> +};
> +
> #if defined(__cplusplus)
> }
> #endif
> --
> 2.15.0.rc0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/5] drm: Add drm_object lease infrastructure [v4]
2017-10-16 20:42 ` Keith Packard
@ 2017-10-16 21:05 ` Sean Paul
0 siblings, 0 replies; 16+ messages in thread
From: Sean Paul @ 2017-10-16 21:05 UTC (permalink / raw)
To: Keith Packard
Cc: Sean Paul, linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
On Mon, Oct 16, 2017 at 01:42:46PM -0700, Keith Packard wrote:
> Sean Paul <seanpaul@chromium.org> writes:
>
>
> > With these nits fixed,
> > Reviewed-by: Sean Paul <seanpaul@chromium.org>
>
> Like this?
>
Perfect, thanks!
Sean
> From 0aa52dd5a0873831c79c14942075354c041e5bed Mon Sep 17 00:00:00 2001
> From: Keith Packard <keithp@keithp.com>
> Date: Mon, 16 Oct 2017 13:41:20 -0700
> Subject: [PATCH] drm: Mark functions requiring idr_mutex. Add lockdep to
> _drm_lease_revoke
>
> Reasonable suggestions by Sean Paul.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
> drivers/gpu/drm/drm_lease.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
> index 88c213f9c4ab..20694c77a2de 100644
> --- a/drivers/gpu/drm/drm_lease.c
> +++ b/drivers/gpu/drm/drm_lease.c
> @@ -58,7 +58,9 @@ _drm_find_lessee(struct drm_master *master, int lessee_id)
> }
>
> /**
> - * _drm_lease_held_master - check to see if an object is leased (or owned) by master
> + * _drm_lease_held_master - check to see if an object is leased (or
> + * owned) by master (idr_mutex held)
> + *
> * @master: the master to check the lease status of
> * @id: the id to check
> *
> @@ -77,7 +79,7 @@ static int _drm_lease_held_master(struct drm_master *master, int id)
> }
>
> /**
> - * _drm_has_leased - check to see if an object has been leased
> + * _drm_has_leased - check to see if an object has been leased (idr mutex held)
> * @master: the master to check the lease status of
> * @id: the id to check
> *
> @@ -300,8 +302,8 @@ void drm_lease_destroy(struct drm_master *master)
> }
>
> /**
> - * _drm_lease_revoke - revoke access to all leased objects
> - * @master: the master losing its lease
> + * _drm_lease_revoke - revoke access to all leased objects (idr_mutex held)
> + * @top: the master losing its lease
> */
>
> void _drm_lease_revoke(struct drm_master *top)
> @@ -310,6 +312,7 @@ void _drm_lease_revoke(struct drm_master *top)
> void *entry;
> struct drm_master *master = top;
>
> + lockdep_assert_held(&top->dev->mode_config.idr_mutex);
> /*
> * Walk the tree starting at 'top' emptying all leases. Because
> * the tree is fully connected, we can do this without recursing
> --
> 2.15.0.rc0
>
>
>
> --
> -keith
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6]
2017-10-16 21:03 ` Sean Paul
@ 2017-10-16 21:31 ` Keith Packard
0 siblings, 0 replies; 16+ messages in thread
From: Keith Packard @ 2017-10-16 21:31 UTC (permalink / raw)
To: Sean Paul; +Cc: linux-kernel, Dave Airlie, Daniel Vetter, dri-devel
[-- Attachment #1: Type: text/plain, Size: 1938 bytes --]
Sean Paul <seanpaul@chromium.org> writes:
> nit: space before ,
Thanks.
>> + /* Clone the lessor file to create a new file for us */
>> + DRM_DEBUG_LEASE("Allocating lease file\n");
>> + path_get(&lessor_file->f_path);
>
> Please forgive the stupid question, but where is this reference given
> up?
That's not a stupid question, it's a very subtle one which took me quite
a while to sort out. Here's path_get:
void path_get(const struct path *path)
{
mntget(path->mnt);
dget(path->dentry);
}
So, getting a reference on a 'path' actually gets a reference on two of
the things it points to.
alloc_file is passed the path and doesn't take an additional reference
on either of these fields, presumably because the normal path has the
caller taking a reference while looking up the object and handing that
reference off to alloc_file. In our case, we're creating a new file that
refers to the same path as an existing one, so we need another
reference.
When the file is finally freed in __fput, the two references are dropped
at the end of the function:
static void __fput(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct vfsmount *mnt = file->f_path.mnt;
...
dput(dentry);
mntput(mnt);
}
This was probably the twistiest part of creating a lease. All of the DRM
stuff was trivial; getting the core kernel object reference counts right
was a pain.
>> + if (lessee->lessor == NULL)
>> + /* owner can use all objects */
>> + object_idr = &lessee->dev->mode_config.crtc_idr;
>
> What about other types of objects?
If I understand your question correctly, the answer is that 'crtc_idr'
is misnamed -- it holds all of the mode setting objects.
Thanks for your review, let me know if you have more questions!
--
-keith
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-10-16 21:31 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-13 1:56 [PATCH 0/5]: drm: Add drm mode object leases Keith Packard
2017-10-13 1:56 ` [PATCH 1/5] drm/plane: drop num_overlay_planes (v2) Keith Packard
2017-10-16 19:23 ` Sean Paul
2017-10-13 1:56 ` [PATCH 2/5] drm: Add new LEASE debug level Keith Packard
2017-10-16 19:24 ` Sean Paul
2017-10-13 1:56 ` [PATCH 3/5] drm: Add drm_object lease infrastructure [v4] Keith Packard
2017-10-16 19:44 ` Sean Paul
2017-10-16 20:42 ` Keith Packard
2017-10-16 21:05 ` Sean Paul
2017-10-13 1:56 ` [PATCH 4/5] drm: Check mode object lease status in all master ioctl paths [v3] Keith Packard
2017-10-16 20:34 ` Sean Paul
2017-10-13 1:56 ` [PATCH 5/5] drm: Add four ioctls for managing drm mode object leases [v6] Keith Packard
2017-10-16 21:03 ` Sean Paul
2017-10-16 21:31 ` Keith Packard
2017-10-16 9:13 ` [PATCH 0/5]: drm: Add drm mode object leases Daniel Vetter
2017-10-16 17:52 ` 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).