DRM: VMA Access Management and Render Nodes

PATCH In review David Herrmann 7 participants 25 messages Aug 23, 2013 – Aug 25, 2013

Subsystems (9)

DRM drivers Documentation DRM misc Intel i915 +5 more subsystems

lore

Share & tools

lore

David Herrmann

author

2013-08-23 11:13 UTC #0 lore raw

v2 cover DRM: VMA Access Management and Render Nodes

I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence, all gem drivers are safe now. For TTM drivers, I now use the already available verify_access() callback to get access to the underlying gem-object. Pretty simple.. Why hadn't I thought of that before?

Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But they do their own access-management, anyway.

The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes will not be created.

This allows us to test render-nodes and play with the API. I added FLINK for now so we can better test it. Not sure whether we should allow it in the end, though.

Maybe we can get this into 3.11?

Regards
David

David Herrmann (4):
drm/vma: add access management helpers
drm/gem: implement vma access management
drm: verify vma access in TTM+GEM drivers
drm: implement experimental render nodes

Kristian Høgsberg (1):
drm/i915: Support render nodes

Martin Peres (1):
drm/nouveau: Support render nodes

 Documentation/DocBook/drm.tmpl        |  71 ++++++++++++++++
 drivers/gpu/drm/ast/ast_ttm.c         |   4 +-
 drivers/gpu/drm/cirrus/cirrus_ttm.c   |   4 +-
 drivers/gpu/drm/drm_drv.c             |  15 ++--
 drivers/gpu/drm/drm_fops.c            |  14 +--
 drivers/gpu/drm/drm_gem.c             |  18 ++++
 drivers/gpu/drm/drm_pci.c             |   9 ++
 drivers/gpu/drm/drm_platform.c        |   9 ++
 drivers/gpu/drm/drm_stub.c            |  10 +++
 drivers/gpu/drm/drm_usb.c             |   9 ++
 drivers/gpu/drm/drm_vma_manager.c     | 155 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_dma.c       |  36 ++++----
 drivers/gpu/drm/i915/i915_drv.c       |   3 +-
 drivers/gpu/drm/mgag200/mgag200_ttm.c |   4 +-
 drivers/gpu/drm/nouveau/nouveau_bo.c  |   4 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c |  24 +++---
 drivers/gpu/drm/qxl/qxl_ttm.c         |   4 +-
 drivers/gpu/drm/radeon/radeon_ttm.c   |   4 +-
 include/drm/drmP.h                    |   9 ++
 include/drm/drm_vma_manager.h         |  21 ++++-
 20 files changed, 373 insertions(+), 54 deletions(-)

Signature · 6 lines

1.8.3.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

David Herrmann

author

2013-08-23 11:13 UTC #1 lore raw

PATCH v2 1/6 drm/vma: add access management helpers

The VMA offset manager uses a device-global address-space. Hence, any user can currently map any offset-node they want. They only need to guess the right offset. If we wanted per open-file offset spaces, we'd either need VM_NONLINEAR mappings or multiple "struct address_space" trees. As both doesn't really scale, we implement access management in the VMA manager itself.

We use an rb-tree to store open-files for each VMA node. On each mmap call, GEM, TTM or the drivers must check whether the current user is allowed to map this file.

We add a separate lock for each node as there is no generic lock available for the caller to protect the node easily.

As we currently don't know whether an object may be used for mmap(), we have to do access management for all objects. If it turns out to slow down handle creation/deletion significantly, we can optimize it in several ways:
- Most times only a single filp is added per bo so we could use a static "struct file *main_filp" which is checked/added/removed first before we fall back to the rbtree+drm_vma_offset_file.
This could be even done lockless with rcu.
- Let user-space pass a hint whether mmap() should be supported on the bo and avoid access-management if not.
- .. there are probably more ideas once we have benchmarks ..

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/drm_gem.c         |   1 +
 drivers/gpu/drm/drm_vma_manager.c | 155 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_vma_manager.h     |  21 +++++-
 3 files changed, 174 insertions(+), 3 deletions(-)

drivers/gpu/drm/drm_gem.c +1 -0

index 1ce88c3..d6122ae 100644
@@ -156,6 +156,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	kref_init(&obj->refcount);
 	obj->handle_count = 0;
 	obj->size = size;
+	drm_vma_node_reset(&obj->vma_node);
 }
 EXPORT_SYMBOL(drm_gem_private_object_init);

drivers/gpu/drm/drm_vma_manager.c +155 -0

170 lines not on this page · raw message

include/drm/drm_vma_manager.h +18 -3

index 22eedac..d66b6a2 100644
@@ -24,15 +24,24 @@
  */
 
 #include <drm/drm_mm.h>
+#include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/rbtree.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
+struct drm_vma_offset_file {
+	struct rb_node vm_rb;
+	struct file *vm_filp;
+	unsigned long vm_count;
+};
+
 struct drm_vma_offset_node {
+	rwlock_t vm_lock;
 	struct drm_mm_node vm_node;
 	struct rb_node vm_rb;
+	struct rb_root vm_files;
 };
 
 struct drm_vma_offset_manager {

@@ -56,6 +65,11 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
 void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
 			   struct drm_vma_offset_node *node);
 
+int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp);
+void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp);
+bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
+			     struct file *filp);
+
 /**
  * drm_vma_offset_exact_lookup() - Look up node by exact address
  * @mgr: Manager object

@@ -122,9 +136,8 @@ static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *m
  * drm_vma_node_reset() - Initialize or reset node object
  * @node: Node to initialize or reset
  *
- * Reset a node to its initial state. This must be called if @node isn't
- * already cleared (eg., via kzalloc) before using it with any VMA offset
- * manager.
+ * Reset a node to its initial state. This must be called before using it with
+ * any VMA offset manager.
  *
  * This must not be called on an already allocated node, or you will leak
  * memory.

@@ -132,6 +145,8 @@ static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *m
 static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
 {
 	memset(node, 0, sizeof(*node));
+	node->vm_files = RB_ROOT;
+	rwlock_init(&node->vm_lock);
 }
 
 /**

Signature · 1 line

1.8.3.4

David Herrmann

author

2013-08-23 11:13 UTC #2 lore raw

PATCH v2 2/6 drm/gem: implement vma access management

In reply to David Herrmann

We implement automatic vma mmap() access management for all drivers using gem_mmap. We use the vma manager to add each open-file that creates a gem-handle to the vma-node of the underlying gem object. Once the handle is destroyed, we drop the open-file again.

This allows us to use drm_vma_node_is_allowed() on _any_ gem object to see whether an open-file is granted access. In drm_gem_mmap() we use this to verify that unprivileged users cannot guess gem offsets and map arbitrary buffers.

Note that this manages access for _all_ gem users (also TTM+GEM), but the actual access checks are only done for drm_gem_mmap(). TTM drivers use the TTM mmap helpers, which need to do that separately.

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/drm_gem.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/gpu/drm/drm_gem.c +17 -0

index d6122ae..b2d59b2 100644
@@ -298,6 +298,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 	spin_unlock(&filp->table_lock);
 
 	drm_gem_remove_prime_handles(obj, filp);
+	drm_vma_node_revoke(&obj->vma_node, filp->filp);
 
 	if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, filp);

@@ -357,6 +358,11 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 	}
 	*handlep = ret;
 
+	ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp);
+	if (ret) {
+		drm_gem_handle_delete(file_priv, *handlep);
+		return ret;
+	}
 
 	if (dev->driver->gem_open_object) {
 		ret = dev->driver->gem_open_object(obj, file_priv);

@@ -701,6 +707,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
 	struct drm_device *dev = obj->dev;
 
 	drm_gem_remove_prime_handles(obj, file_priv);
+	drm_vma_node_revoke(&obj->vma_node, file_priv->filp);
 
 	if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, file_priv);

@@ -793,6 +800,10 @@ EXPORT_SYMBOL(drm_gem_vm_close);
  * the GEM object is not looked up based on its fake offset. To implement the
  * DRM mmap operation, drivers should use the drm_gem_mmap() function.
  *
+ * drm_gem_mmap_obj() assumes the user is granted access to the buffer while
+ * drm_gem_mmap() prevents unprivileged users from mapping random objects. So
+ * callers must verify access restrictions before calling this helper.
+ *
  * NOTE: This function has to be protected with dev->struct_mutex
  *
  * Return 0 or success or -EINVAL if the object size is smaller than the VMA

@@ -841,6 +852,9 @@ EXPORT_SYMBOL(drm_gem_mmap_obj);
  * Look up the GEM object based on the offset passed in (vma->vm_pgoff will
  * contain the fake offset we created when the GTT map ioctl was called on
  * the object) and map it with a call to drm_gem_mmap_obj().
+ *
+ * If the caller is not granted access to the buffer object, the mmap will fail
+ * with EACCES. Please see the vma manager for more information.
  */
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 {

@@ -861,6 +875,9 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	if (!node) {
 		mutex_unlock(&dev->struct_mutex);
 		return drm_mmap(filp, vma);
+	} else if (!drm_vma_node_is_allowed(node, filp)) {
+		mutex_unlock(&dev->struct_mutex);
+		return -EACCES;
 	}
 
 	obj = container_of(node, struct drm_gem_object, vma_node);

Signature · 1 line

1.8.3.4

David Herrmann

author

2013-08-23 11:13 UTC #3 lore raw

PATCH v2 3/6 drm: verify vma access in TTM+GEM drivers

In reply to David Herrmann

GEM does already a good job in tracking access to gem buffers via handles and drm_vma access management. However, TTM drivers currently do not verify this during mmap().

TTM provides the verify_access() callback to test this. So fix all drivers to actually call into gem+vma to verify access instead of always returning
0. (note that verify_access() returns 0 on success, unlike drm_vma_node_is_allowed() which returns "true").

All drivers assume that user-space can only get access to TTM buffers via GEM handles. So whenever the verify_access() callback is called from ttm_bo_mmap(), the buffer must have a valid embedded gem object. This is true for all TTM+GEM drivers. But that's why this patch doesn't touch pure TTM drivers (ie, vmwgfx).

Cc Dave Airlie <[email protected]>

Cc Alex Deucher <[email protected]>

Cc Ben Skeggs <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Cc Jerome Glisse <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/ast/ast_ttm.c         | 4 +++-
 drivers/gpu/drm/cirrus/cirrus_ttm.c   | 4 +++-
 drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 +++-
 drivers/gpu/drm/nouveau/nouveau_bo.c  | 4 +++-
 drivers/gpu/drm/qxl/qxl_ttm.c         | 4 +++-
 drivers/gpu/drm/radeon/radeon_ttm.c   | 4 +++-
 6 files changed, 18 insertions(+), 6 deletions(-)

drivers/gpu/drm/ast/ast_ttm.c +3 -1

index cf1c833..ae6c335 100644
@@ -148,7 +148,9 @@ ast_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
 
 static int ast_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct ast_bo *astbo = ast_bo(bo);
+
+	return !drm_vma_node_is_allowed(&astbo->gem.vma_node, filp);
 }
 
 static int ast_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/cirrus/cirrus_ttm.c +3 -1

index bf8a506..dd12b60 100644
@@ -148,7 +148,9 @@ cirrus_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
 
 static int cirrus_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct cirrus_bo *cirrusbo = cirrus_bo(bo);
+
+	return !drm_vma_node_is_allowed(&cirrusbo->gem.vma_node, filp);
 }
 
 static int cirrus_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/mgag200/mgag200_ttm.c +3 -1

index 6cf3fa0..a9f96b9 100644
@@ -148,7 +148,9 @@ mgag200_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
 
 static int mgag200_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct mgag200_bo *mgabo = mgag200_bo(bo);
+
+	return !drm_vma_node_is_allowed(&mgabo->gem.vma_node, filp);
 }
 
 static int mgag200_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/nouveau/nouveau_bo.c +3 -1

index 4e7ee5f..554a5af 100644
@@ -1260,7 +1260,9 @@ out:
 static int
 nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct nouveau_bo *nvbo = nouveau_bo(bo);
+
+	return !drm_vma_node_is_allowed(&nvbo->gem->vma_node, filp);
 }
 
 static int

drivers/gpu/drm/qxl/qxl_ttm.c +3 -1

index 1dfd84c..497fb6d 100644
@@ -212,7 +212,9 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
 
 static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct qxl_bo *qbo = to_qxl_bo(bo);
+
+	return !drm_vma_node_is_allowed(&qbo->gem_base.vma_node, filp);
 }
 
 static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/radeon/radeon_ttm.c +3 -1

index 6c0ce89..072e2d7 100644
@@ -203,7 +203,9 @@ static void radeon_evict_flags(struct ttm_buffer_object *bo,
 
 static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct radeon_bo *rbo = container_of(bo, struct radeon_bo, tbo);
+
+	return !drm_vma_node_is_allowed(&rbo->gem_base.vma_node, filp);
 }
 
 static void radeon_move_null(struct ttm_buffer_object *bo,

Signature · 1 line

1.8.3.4

David Herrmann

author

2013-08-23 11:13 UTC #4 lore raw

PATCH v2 4/6 drm: implement experimental render nodes

In reply to David Herrmann

Render nodes provide an API for userspace to use non-privileged GPU commands without any running DRM-Master. It is useful for offscreen rendering, GPGPU clients, and normal render clients which do not perform modesetting.

Compared to legacy clients, render clients no longer need any authentication to perform client ioctls. Instead, user-space controls render/client access to GPUs via filesystem access-modes on the render-node. Once a render-node was opened, a client has full access to the client/render operations on the GPU. However, no modesetting or ioctls that affect global state are allowed on render nodes.

To prevent privilege-escalation, drivers must explicitly state that they support render nodes. They must mark their render-only ioctls as DRM_RENDER_ALLOW so render clients can use them. Furthermore, they must support clients without any attached master.

If filesystem access-modes are not enough for fine-grained access control to render nodes (very unlikely, considering the versaitlity of FS-ACLs), you may still fall-back to fd-passing from server to client (which allows arbitrary access-control). However, note that revoking access is currently impossible and unlikely to get implemented.

Note: Render clients no longer have any associated DRM-Master as they are supposed to be independent of any server state. DRM core highly depends on file_priv->master to be non-NULL for modesetting/ctx/etc. commands. Therefore, drivers must be very careful to not require DRM-Master if they support DRIVER_RENDER.

So far render-nodes are protected by "drm_rnodes". As long as this module-parameter is not set to 1, a driver will not create render nodes. This allows us to experiment with the API a bit before we stabilize it.

Signed-off-by David Herrmann <[email protected]>

---
 Documentation/DocBook/drm.tmpl | 71 ++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_drv.c      | 15 ++++-----
 drivers/gpu/drm/drm_fops.c     | 14 ++++-----
 drivers/gpu/drm/drm_pci.c      |  9 ++++++
 drivers/gpu/drm/drm_platform.c |  9 ++++++
 drivers/gpu/drm/drm_stub.c     | 10 ++++++
 drivers/gpu/drm/drm_usb.c      |  9 ++++++
 include/drm/drmP.h             |  9 ++++++
 8 files changed, 132 insertions(+), 14 deletions(-)

Documentation/DocBook/drm.tmpl +71 -0

index 9fc8ed4..3a778bf 100644
@@ -205,6 +205,12 @@
               Driver implements DRM PRIME buffer sharing.
             </para></listitem>
           </varlistentry>
+          <varlistentry>
+            <term>DRIVER_RENDER</term>
+            <listitem><para>
+              Driver supports dedicated render nodes.
+            </para></listitem>
+          </varlistentry>
         </variablelist>
       </sect3>
       <sect3>

@@ -2644,6 +2650,71 @@ int (*resume) (struct drm_device *);</synopsis>
       info, since man pages should cover the rest.
     </para>
 
+  <!-- External: render nodes -->
+
+    <sect1>
+      <title>Render nodes</title>
+      <para>
+        DRM core provides multiple character-devices for user-space to use.
+        Depending on which device is opened, user-space can perform a different
+        set of operations (mainly ioctls). The primary node is always created
+        and called <term>card&lt;num&gt;</term>. Additionally, a currently
+        unused control node, called <term>controlD&lt;num&gt;</term> is also
+        created. The primary node provides all legacy operations and
+        historically was the only interface used by userspace. With KMS, the
+        control node was introduced. However, the planned KMS control interface
+        has never been written and so the control node stays unused to date.
+      </para>
+      <para>
+        With the increased use of offscreen renderers and GPGPU applications,
+        clients no longer require running compositors or graphics servers to
+        make use of a GPU. But the DRM API required unprivileged clients to
+        authenticate to a DRM-Master prior to getting GPU access. To avoid this
+        step and to grant clients GPU access without authenticating, render
+        nodes were introduced. Render nodes solely serve render clients, that
+        is, no modesetting or privileged ioctls can be issued on render nodes.
+        Only non-global rendering commands are allowed. If a driver supports
+        render nodes, it must advertise it via the <term>DRIVER_RENDER</term>
+        DRM driver capability. If not supported, the primary node must be used
+        for render clients together with the legacy drmAuth authentication
+        procedure.
+      </para>
+      <para>
+        If a driver advertises render node support, DRM core will create a
+        separate render node called <term>renderD&lt;num&gt;</term>. There will
+        be one render node per device. No ioctls except for
+        <term>GEM_CLOSE</term>, <term>GEM_FLINK</term> and PRIME-related ioctls
+        will be allowed on this node. Especially <term>GEM_OPEN</term> will be
+        explicitly prohibited. Render nodes are designed to avoid the
+        buffer-leaks, which occur if clients guess the flink names or mmap
+        offsets on the legacy interface. Additionally to this basic interface,
+        drivers must mark their driver-dependent render-only ioctls as
+        <term>DRM_RENDER_ALLOW</term> so render clients can use them. Driver
+        authors must be careful not to allow any privileged ioctls on render
+        nodes.
+      </para>
+      <para>
+        With render nodes, user-space can now control access to the render node
+        via basic file-system access-modes. A running graphics server which
+        authenticates clients on the privileged primary/legacy node is no longer
+        required. Instead, a client can open the render node and is immediately
+        granted GPU access. Communication between clients (or servers) is done
+        via PRIME. FLINK from render node to legacy node is also supported,
+        however, FLINK from legacy node to render node is prohibited. New
+        clients must not use the insecure FLINK interface.
+      </para>
+      <para>
+        Besides dropping all modeset/global ioctls, render nodes also drop the
+        DRM-Master concept. There is no reason to associate render clients with
+        a DRM-Master as they are independent of any graphics server. Besides,
+        they must work without any running master, anyway.
+        Drivers must be able to run without a master object if they support
+        render nodes. If, on the other hand, a driver requires shared state
+        between clients which is visible to user-space and accessible beyond
+        open-file boundaries, they cannot support render nodes.
+      </para>
+    </sect1>
+
   <!-- External: vblank handling -->
 
     <sect1>

drivers/gpu/drm/drm_drv.c +8 -7

index 288da3d..fde7447 100644
@@ -68,7 +68,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED),
-	DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),

@@ -130,14 +130,14 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 
 	DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED),

@@ -420,9 +420,10 @@ long drm_ioctl(struct file *filp,
 		DRM_DEBUG("no function\n");
 		retcode = -EINVAL;
 	} else if (((ioctl->flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)) ||
-		   ((ioctl->flags & DRM_AUTH) && !file_priv->authenticated) ||
+		   ((ioctl->flags & DRM_AUTH) && !drm_is_render_client(file_priv) && !file_priv->authenticated) ||
 		   ((ioctl->flags & DRM_MASTER) && !file_priv->is_master) ||
-		   (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL))) {
+		   (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL)) ||
+		   (!(ioctl->flags & DRM_RENDER_ALLOW) && drm_is_render_client(file_priv))) {
 		retcode = -EACCES;
 	} else {
 		if (cmd & (IOC_IN | IOC_OUT)) {

drivers/gpu/drm/drm_fops.c +7 -7

index 2d2401e..6d1726a 100644
@@ -262,10 +262,10 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 			goto out_prime_destroy;
 	}
 
-
-	/* if there is no current master make this fd it */
+	/* if there is no current master make this fd it, but do not create
+	 * any master object for render clients */
 	mutex_lock(&dev->struct_mutex);
-	if (!priv->minor->master) {
+	if (!priv->minor->master && !drm_is_render_client(priv)) {
 		/* create a new master */
 		priv->minor->master = drm_master_create(priv->minor);
 		if (!priv->minor->master) {

@@ -303,12 +303,11 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 				goto out_close;
 			}
 		}
-		mutex_unlock(&dev->struct_mutex);
-	} else {
+	} else if (!drm_is_render_client(priv)) {
 		/* get a reference to the master */
 		priv->master = drm_master_get(priv->minor->master);
-		mutex_unlock(&dev->struct_mutex);
 	}
+	mutex_unlock(&dev->struct_mutex);
 
 	mutex_lock(&dev->struct_mutex);
 	list_add(&priv->lhead, &dev->filelist);

@@ -478,7 +477,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	iput(container_of(dev->dev_mapping, struct inode, i_data));
 
 	/* drop the reference held my the file priv */
-	drm_master_put(&file_priv->master);
+	if (file_priv->master)
+		drm_master_put(&file_priv->master);
 	file_priv->is_master = 0;
 	list_del(&file_priv->lhead);
 	mutex_unlock(&dev->struct_mutex);

drivers/gpu/drm/drm_pci.c +9 -0

index 3fca2db..1f96cee 100644
@@ -354,6 +354,12 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 			goto err_g2;
 	}
 
+	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
+		ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
+		if (ret)
+			goto err_g21;
+	}
+
 	if ((ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY)))
 		goto err_g3;

@@ -383,6 +389,9 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 err_g4:
 	drm_put_minor(&dev->primary);
 err_g3:
+	if (dev->render)
+		drm_put_minor(&dev->render);
+err_g21:
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_put_minor(&dev->control);
 err_g2:

drivers/gpu/drm/drm_platform.c +9 -0

index 400024b..f7a18c6 100644
@@ -69,6 +69,12 @@ static int drm_get_platform_dev(struct platform_device *platdev,
 			goto err_g1;
 	}
 
+	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
+		ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
+		if (ret)
+			goto err_g11;
+	}
+
 	ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
 	if (ret)
 		goto err_g2;

@@ -100,6 +106,9 @@ static int drm_get_platform_dev(struct platform_device *platdev,
 err_g3:
 	drm_put_minor(&dev->primary);
 err_g2:
+	if (dev->render)
+		drm_put_minor(&dev->render);
+err_g11:
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_put_minor(&dev->control);
 err_g1:

drivers/gpu/drm/drm_stub.c +10 -0

index e30bb0d..e7eb027 100644
@@ -40,6 +40,9 @@
 unsigned int drm_debug = 0;	/* 1 to enable debug output */
 EXPORT_SYMBOL(drm_debug);
 
+unsigned int drm_rnodes = 0;	/* 1 to enable experimental render nodes API */
+EXPORT_SYMBOL(drm_rnodes);
+
 unsigned int drm_vblank_offdelay = 5000;    /* Default to 5000 msecs. */
 EXPORT_SYMBOL(drm_vblank_offdelay);

@@ -56,11 +59,13 @@ MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
 MODULE_PARM_DESC(debug, "Enable debug output");
+MODULE_PARM_DESC(rnodes, "Enable experimental render nodes API");
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
 MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic timestamps");
 
 module_param_named(debug, drm_debug, int, 0600);
+module_param_named(rnodes, drm_rnodes, int, 0600);
 module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
 module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600);

@@ -446,6 +451,9 @@ void drm_put_dev(struct drm_device *dev)
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_put_minor(&dev->control);
 
+	if (dev->render)
+		drm_put_minor(&dev->render);
+
 	if (driver->driver_features & DRIVER_GEM)
 		drm_gem_destroy(dev);

@@ -462,6 +470,8 @@ void drm_unplug_dev(struct drm_device *dev)
 	/* for a USB device */
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_unplug_minor(dev->control);
+	if (dev->render)
+		drm_unplug_minor(dev->render);
 	drm_unplug_minor(dev->primary);
 
 	mutex_lock(&drm_global_mutex);

drivers/gpu/drm/drm_usb.c +9 -0

index 34a156f..8766472 100644
@@ -33,6 +33,12 @@ int drm_get_usb_dev(struct usb_interface *interface,
 	if (ret)
 		goto err_g1;
 
+	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
+		ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
+		if (ret)
+			goto err_g11;
+	}
+
 	ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
 	if (ret)
 		goto err_g2;

@@ -62,6 +68,9 @@ int drm_get_usb_dev(struct usb_interface *interface,
 err_g3:
 	drm_put_minor(&dev->primary);
 err_g2:
+	if (dev->render)
+		drm_put_minor(&dev->render);
+err_g11:
 	drm_put_minor(&dev->control);
 err_g1:
 	kfree(dev);

include/drm/drmP.h +9 -0

index 90833dc..8fd48bd 100644
@@ -145,6 +145,7 @@ int drm_err(const char *func, const char *format, ...);
 #define DRIVER_GEM         0x1000
 #define DRIVER_MODESET     0x2000
 #define DRIVER_PRIME       0x4000
+#define DRIVER_RENDER      0x8000
 
 #define DRIVER_BUS_PCI 0x1
 #define DRIVER_BUS_PLATFORM 0x2

@@ -290,6 +291,7 @@ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
 #define DRM_ROOT_ONLY	0x4
 #define DRM_CONTROL_ALLOW 0x8
 #define DRM_UNLOCKED	0x10
+#define DRM_RENDER_ALLOW 0x20
 
 struct drm_ioctl_desc {
 	unsigned int cmd;

@@ -1204,6 +1206,7 @@ struct drm_device {
 	unsigned int agp_buffer_token;
 	struct drm_minor *control;		/**< Control node for card */
 	struct drm_minor *primary;		/**< render type primary screen head */
+	struct drm_minor *render;		/**< render node for card */
 
         struct drm_mode_config mode_config;	/**< Current mode config */

@@ -1250,6 +1253,11 @@ static inline bool drm_modeset_is_locked(struct drm_device *dev)
 	return mutex_is_locked(&dev->mode_config.mutex);
 }
 
+static inline bool drm_is_render_client(struct drm_file *file_priv)
+{
+	return file_priv->minor->type == DRM_MINOR_RENDER;
+}
+
 /******************************************************************/
 /** \name Internal function definitions */
 /*@{*/

@@ -1449,6 +1457,7 @@ extern void drm_put_dev(struct drm_device *dev);
 extern int drm_put_minor(struct drm_minor **minor);
 extern void drm_unplug_dev(struct drm_device *dev);
 extern unsigned int drm_debug;
+extern unsigned int drm_rnodes;
 
 extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;

Signature · 1 line

1.8.3.4

David Herrmann

author

2013-08-23 11:13 UTC #5 lore raw

PATCH v2 5/6 drm/i915: Support render nodes

In reply to David Herrmann

From: Kristian Høgsberg <[email protected]>

Enable support for drm render nodes for i915 by flagging the ioctls that are safe and just needed for rendering.

Cc Daniel Vetter <[email protected]>

Signed-off-by Kristian Høgsberg <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/i915/i915_dma.c | 36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/i915_drv.c |  3 ++-
 2 files changed, 20 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/i915_dma.c +18 ~~-18~~

index 0adfe40..ecf5ecd 100644
@@ -1840,7 +1840,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, i915_batchbuffer, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, i915_irq_emit, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, i915_irq_wait, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_SETPARAM, i915_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH),

@@ -1853,34 +1853,34 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, i915_set_status_page, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(I915_GEM_INIT, i915_gem_init_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_unpin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, i915_gem_entervt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, i915_gem_leavevt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GET_PIPE_FROM_CRTC_ID, intel_get_pipe_from_crtc_id, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
 };

drivers/gpu/drm/i915/i915_drv.c +2 -1

index eec47bd..5c0be0e 100644
@@ -1007,7 +1007,8 @@ static struct drm_driver driver = {
 	 */
 	.driver_features =
 	    DRIVER_USE_AGP | DRIVER_REQUIRE_AGP |
-	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
+	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
+	    DRIVER_RENDER,
 	.load = i915_driver_load,
 	.unload = i915_driver_unload,
 	.open = i915_driver_open,

Signature · 6 lines

1.8.3.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Chris Wilson

2013-08-23 11:29 UTC #6 lore raw

Re: [PATCH v2 5/6] drm/i915: Support render nodes

Quoted 27 lines · David Herrmann

From: Kristian Høgsberg <[email protected]>

Enable support for drm render nodes for i915 by flagging the ioctls that are safe and just needed for rendering.

Cc Daniel Vetter <[email protected]>

Signed-off-by Kristian Høgsberg <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/i915/i915_dma.c | 36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/i915_drv.c |  3 ++-
 2 files changed, 20 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/i915_dma.c +1 -1

index 0adfe40..ecf5ecd 100644
@@ -1840,7 +1840,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, i915_batchbuffer, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, i915_irq_emit, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, i915_irq_wait, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_SETPARAM, i915_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH),

drivers/gpu/drm/i915/i915_dma.c const struct drm_ioctl_desc i915_ioctls[] = { · line 1855

Quoted 3 lines

drivers/gpu/drm/i915/i915_dma.c +0 -0

@@ -1853,34 +1853,34 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, i915_set_status_page, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(I915_GEM_INIT, i915_gem_init_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer, DRM_AUTH|DRM_UNLOCKED),

execbuffer also

drivers/gpu/drm/i915/i915_dma.c const struct drm_ioctl_desc i915_ioctls[] = { · line 1861

Quoted 8 lines

drivers/gpu/drm/i915/i915_dma.c +2 -2

-	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_unpin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_UNLOCKED),

set/get caching

Quoted 28 lines

drivers/gpu/drm/i915/i915_dma.c +12 ~~-12~~

-	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, i915_gem_entervt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, i915_gem_leavevt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GET_PIPE_FROM_CRTC_ID, intel_get_pipe_from_crtc_id, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),

drivers/gpu/drm/i915/i915_dma.c const struct drm_ioctl_desc i915_ioctls[] = { · line 1884

Quoted 10 lines

drivers/gpu/drm/i915/i915_dma.c +3 -3

 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),

And reg read
-Chris

Signature · 1 line

Chris Wilson, Intel Open Source Technology Centre

Kristian Høgsberg

2013-08-23 21:13 UTC #7 lore raw

Re: [PATCH v2 5/6] drm/i915: Support render nodes

Quoted 81 lines · Chris Wilson

Quoted 27 lines · David Herrmann

From: Kristian Høgsberg <[email protected]>

Enable support for drm render nodes for i915 by flagging the ioctls that are safe and just needed for rendering.

Cc Daniel Vetter <[email protected]>

Signed-off-by Kristian Høgsberg <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/i915/i915_dma.c | 36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/i915_drv.c |  3 ++-
 2 files changed, 20 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/i915_dma.c +1 -1

index 0adfe40..ecf5ecd 100644
@@ -1840,7 +1840,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
      DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, i915_batchbuffer, DRM_AUTH),
      DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, i915_irq_emit, DRM_AUTH),
      DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, i915_irq_wait, DRM_AUTH),
-     DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH),
+     DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF_DRV(I915_SETPARAM, i915_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
      DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH),
      DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH),

drivers/gpu/drm/i915/i915_dma.c const struct drm_ioctl_desc i915_ioctls[] = { · line 1855

Quoted 3 lines

drivers/gpu/drm/i915/i915_dma.c +0 -0

@@ -1853,34 +1853,34 @@ const struct drm_ioctl_desc i915_ioctls[] = {
      DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, i915_set_status_page, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
      DRM_IOCTL_DEF_DRV(I915_GEM_INIT, i915_gem_init_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
      DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer, DRM_AUTH|DRM_UNLOCKED),

execbuffer also

drivers/gpu/drm/i915/i915_dma.c const struct drm_ioctl_desc i915_ioctls[] = { · line 1861

Quoted 8 lines

drivers/gpu/drm/i915/i915_dma.c +2 -2

-     DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED),
+     DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
      DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_unpin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED),
+     DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_UNLOCKED),
      DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_UNLOCKED),

set/get caching

Quoted 28 lines

drivers/gpu/drm/i915/i915_dma.c +12 ~~-12~~

-     DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+     DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, i915_gem_entervt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
      DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, i915_gem_leavevt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED),
+     DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF_DRV(I915_GET_PIPE_FROM_CRTC_ID, intel_get_pipe_from_crtc_id, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
+     DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),

drivers/gpu/drm/i915/i915_dma.c const struct drm_ioctl_desc i915_ioctls[] = { · line 1884

Quoted 10 lines

drivers/gpu/drm/i915/i915_dma.c +3 -3

      DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
      DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
      DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED),
-     DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
+     DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+     DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
      DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),

Quoted 1 line

And reg read

I defer to Chris on this.

Kristian
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Daniel Vetter

Reviewed-by on v2 5/6

2013-08-23 22:51 UTC #8 lore raw

Re: [PATCH v2 5/6] drm/i915: Support render nodes

In reply to David Herrmann

Quoted 28 lines · David Herrmann

From: Kristian Høgsberg <[email protected]>

Enable support for drm render nodes for i915 by flagging the ioctls that are safe and just needed for rendering.

Cc Daniel Vetter <[email protected]>

Signed-off-by Kristian Høgsberg <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/i915/i915_dma.c | 36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/i915_drv.c |  3 ++-
 2 files changed, 20 insertions(+), 19 deletions(-)

drivers/gpu/drm/i915/i915_dma.c +1 -1

index 0adfe40..ecf5ecd 100644
@@ -1840,7 +1840,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, i915_batchbuffer, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, i915_irq_emit, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, i915_irq_wait, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_SETPARAM, i915_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH),

@@ -1853,34 +1853,34 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, i915_set_status_page, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),

drivers/gpu/drm/i915/i915_dma.c const struct drm_ioctl_desc i915_ioctls[] = { · line 1861

Quoted 10 lines

drivers/gpu/drm/i915/i915_dma.c +2 -2

 	DRM_IOCTL_DEF_DRV(I915_GEM_INIT, i915_gem_init_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_unpin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_UNLOCKED),

Those two here also need to work on render nodes. With that and REG_READ also marked up like Chris said this is

Reviewed-by Daniel Vetter <[email protected]>

Quoted 57 lines

drivers/gpu/drm/i915/i915_dma.c +15 ~~-15~~

-	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, i915_gem_entervt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, i915_gem_leavevt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GET_PIPE_FROM_CRTC_ID, intel_get_pipe_from_crtc_id, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
 };

drivers/gpu/drm/i915/i915_drv.c +2 -1

index eec47bd..5c0be0e 100644
@@ -1007,7 +1007,8 @@ static struct drm_driver driver = {
 	 */
 	.driver_features =
 	    DRIVER_USE_AGP | DRIVER_REQUIRE_AGP |
-	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
+	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
+	    DRIVER_RENDER,
 	.load = i915_driver_load,
 	.unload = i915_driver_unload,
 	.open = i915_driver_open,

Signature · 1 line

1.8.3.4

Signature · 3 lines

Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

David Herrmann

author

2013-08-23 11:14 UTC #9 lore raw

PATCH v2 6/6 drm/nouveau: Support render nodes

In reply to David Herrmann

From: Martin Peres <[email protected]>

Enable support for drm render nodes for nouveau by flagging the ioctls that are safe and just needed for rendering.

Cc Ben Skeggs <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Signed-off-by Martin Peres <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

drivers/gpu/drm/nouveau/nouveau_drm.c +12 ~~-12~~

index b29d04b..ab42a25 100644
@@ -651,18 +651,18 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
 
 static const struct drm_ioctl_desc
 nouveau_ioctls[] = {
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_UNLOCKED|DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, nouveau_abi16_ioctl_setparam, DRM_UNLOCKED|DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 };
 
 static const struct file_operations

@@ -684,7 +684,7 @@ static struct drm_driver
 driver = {
 	.driver_features =
 		DRIVER_USE_AGP |
-		DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME,
+		DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER,
 
 	.load = nouveau_drm_load,
 	.unload = nouveau_drm_unload,

Signature · 1 line

1.8.3.4

Christian König

maintainer

2013-08-23 11:43 UTC #10 lore raw

In reply to David Herrmann

Hi David,

Am 23.08.2013 13:13, schrieb David Herrmann:

Quoted 16 lines

Hi
I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence, all gem drivers are safe now. For TTM drivers, I now use the already available verify_access() callback to get access to the underlying gem-object. Pretty simple.. Why hadn't I thought of that before?
Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But they do their own access-management, anyway.
The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes will not be created.

Quoted 5 lines

This allows us to test render-nodes and play with the API. I added FLINK for now so we can better test it. Not sure whether we should allow it in the end, though.
Maybe we can get this into 3.11?

A bit unlikely, but 3.12 should work fine.

I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway).

Cheers,
Christian.

Quoted 28 lines

Regards
David

David Herrmann (4):
drm/vma: add access management helpers
drm/gem: implement vma access management
drm: verify vma access in TTM+GEM drivers
drm: implement experimental render nodes

Kristian Høgsberg (1):
drm/i915: Support render nodes

Martin Peres (1):
drm/nouveau: Support render nodes

  Documentation/DocBook/drm.tmpl        |  71 ++++++++++++++++
  drivers/gpu/drm/ast/ast_ttm.c         |   4 +-
  drivers/gpu/drm/cirrus/cirrus_ttm.c   |   4 +-
  drivers/gpu/drm/drm_drv.c             |  15 ++--
  drivers/gpu/drm/drm_fops.c            |  14 +--
  drivers/gpu/drm/drm_gem.c             |  18 ++++
  drivers/gpu/drm/drm_pci.c             |   9 ++
  drivers/gpu/drm/drm_platform.c        |   9 ++
  drivers/gpu/drm/drm_stub.c            |  10 +++
  drivers/gpu/drm/drm_usb.c             |   9 ++
  drivers/gpu/drm/drm_vma_manager.c     | 155 ++++++++++++++++++++++++++++++++++
  drivers/gpu/drm/i915/i915_dma.c       |  36 ++++----
  drivers/gpu/drm/i915/i915_drv.c       |   3 +-

Quoted 9 lines

  drivers/gpu/drm/mgag200/mgag200_ttm.c |   4 +-
  drivers/gpu/drm/nouveau/nouveau_bo.c  |   4 +-
  drivers/gpu/drm/nouveau/nouveau_drm.c |  24 +++---
  drivers/gpu/drm/qxl/qxl_ttm.c         |   4 +-
  drivers/gpu/drm/radeon/radeon_ttm.c   |   4 +-
  include/drm/drmP.h                    |   9 ++
  include/drm/drm_vma_manager.h         |  21 ++++-
  20 files changed, 373 insertions(+), 54 deletions(-)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

David Herrmann

author

2013-08-23 12:31 UTC #11 lore raw

Quoted 31 lines · Christian König

Hi David,
Am 23.08.2013 13:13, schrieb David Herrmann:
Quoted 27 lines
Hi
I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence,
all gem drivers are safe now. For TTM drivers, I now use the already available
verify_access() callback to get access to the underlying gem-object. Pretty
simple.. Why hadn't I thought of that before?
Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But
they do their own access-management, anyway.
The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes
will not be created.
This allows us to test render-nodes and play with the API. I added FLINK for
now so we can better test it. Not sure whether we should allow it in the end,
though.

Quoted 4 lines

Quoted 1 line
Maybe we can get this into 3.11?
A bit unlikely, but 3.12 should work fine.

whoops, 3.12 of course.

Quoted 3 lines

I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway).

Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series.

Regards
David

Christian König

maintainer

2013-08-23 12:49 UTC #12 lore raw

Quoted 2 lines

Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series.

Do you have a branch somewhere I can grab?

That's a bit easier than applying the patchset from the list.

Thanks,
Christian.

Am 23.08.2013 14:31, schrieb David Herrmann:

Quoted 49 lines

Hi
Quoted 31 lines · Christian König
Hi David,
Am 23.08.2013 13:13, schrieb David Herrmann:
Quoted 27 lines
Hi
I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence,
all gem drivers are safe now. For TTM drivers, I now use the already available
verify_access() callback to get access to the underlying gem-object. Pretty
simple.. Why hadn't I thought of that before?
Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But
they do their own access-management, anyway.
The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes
will not be created.
This allows us to test render-nodes and play with the API. I added FLINK for
now so we can better test it. Not sure whether we should allow it in the end,
though.
Quoted 3 lines
Quoted 1 line
Maybe we can get this into 3.11?
A bit unlikely, but 3.12 should work fine.
whoops, 3.12 of course.
Quoted 3 lines
I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway).
Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series.
Regards
David

David Herrmann

author

2013-08-23 12:47 UTC #13 lore raw

Quoted 7 lines · Christian König

Quoted 2 lines
Feel free to send a patch to dri-devel or just let me know the ioctls and I will include it in this series.
Do you have a branch somewhere I can grab?
That's a bit easier than applying the patchset from the list.

https://github.com/dvdhrm/linux/commits/rnodes
I rebase it on update, so don't "git pull" it.

Cheers
David

Alex Deucher

2013-08-23 13:34 UTC #14 lore raw

In reply to Christian König

Quoted 31 lines · Christian König

Hi David,
Am 23.08.2013 13:13, schrieb David Herrmann:
Quoted 27 lines
Hi
I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence,
all gem drivers are safe now. For TTM drivers, I now use the already available
verify_access() callback to get access to the underlying gem-object. Pretty
simple.. Why hadn't I thought of that before?
Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But
they do their own access-management, anyway.
The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes
will not be created.
This allows us to test render-nodes and play with the API. I added FLINK for
now so we can better test it. Not sure whether we should allow it in the end,
though.

Quoted 9 lines

Quoted 1 line
Maybe we can get this into 3.11?
A bit unlikely, but 3.12 should work fine.
I'm working on a project that can make good use of this, so if Alex doesn't mind like to add the necessary radeon flags (should be only a few one liners anyway).

No objections from me.

Alex

Martin Peres

2013-08-23 12:00 UTC #15 lore raw

In reply to David Herrmann

Quoted 10 lines · David Herrmann

Hi
I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence, all gem drivers are safe now. For TTM drivers, I now use the already available verify_access() callback to get access to the underlying gem-object. Pretty simple.. Why hadn't I thought of that before?
Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But they do their own access-management, anyway.

Great! Thanks! Have you checked they are really safe with my "exploits"? I'll have another round of review even if it looked good the last time I checked.

Quoted 4 lines

The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes will not be created.

By default, having the render nodes doesn't change the way the userspace works at all. So, what is the point of protecting it behind a parameter?

Is it to make it clear this isn't part of the API yet? I would say that as long as libdrm hasn't been updated, this isn't part of the API anyway.

Quoted 3 lines

This allows us to test render-nodes and play with the API. I added FLINK for now so we can better test it. Not sure whether we should allow it in the end, though.

From a security point of view, I don't think we should keep it as applications shouldn't be trusted for not doing stupid things (because of code injection). So, unless we plan on adding access control to flink via LSM, we shouldn't expose the feature on render nodes.

From a dev point of view, keeping it means that the XServer doesn't have to know whether mesa supports render nodes or not. This is because the authentication dance isn't available on render nodes so the x-server has to tell mesa if it should authenticate or not. The other solution is to allow the authentication ioctls on render nodes and just return 0 if this is a render node. This way, we won't need any modification in mesa/xserver/dri2proto to pass the information that no authentication is needed. In this solution, only libdrm and the ddx should be modified to make use of the render node. That's not how I did it on my render node patchset, can't remember why...

What do you guys think?

Quoted 2 lines

Maybe we can get this into 3.11?

As long as we don't have to keep the interface stable (I don't want to expose flink on render nodes), I'm all for pushing the code now. Otherwise, a kernel branch somewhere is sufficient.

Do you plan on checking my userspace patches too? Those are enough to make use of the render nodes on X, but I haven't tested that all the combinations of version would still work. The libdrm work should be quite solid though (there are even libdrm tests for the added functionalities :)).

Thanks for your work David!
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

David Herrmann

author

2013-08-25 15:09 UTC #16 lore raw

Quoted 13 lines · Martin Peres

Quoted 11 lines · David Herrmann
Hi
I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence,
all gem drivers are safe now. For TTM drivers, I now use the already available
verify_access() callback to get access to the underlying gem-object. Pretty
simple.. Why hadn't I thought of that before?

Quoted 8 lines

Quoted 3 lines
Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But
they do their own access-management, anyway.
Great! Thanks! Have you checked they are really safe with my "exploits"? I'll have another round of review even if it looked good the last time I checked.

Good you asked. I tested whether it works, I didn't actually verify that it correctly fails in case of exploits. And in fact there is a small bug (I return "1" instead of -EACCES, stupid verify_access()) so user-space gets a segfault accessing the mmap when trying to exploit this. That actually doesn't sound that bad, does it? ;)

v2 is on its way.

Quoted 7 lines

Quoted 5 lines
The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes
will not be created.

Quoted 6 lines

By default, having the render nodes doesn't change the way the userspace works at all. So, what is the point of protecting it behind a parameter?
Is it to make it clear this isn't part of the API yet? I would say that as long
as libdrm hasn't been updated, this isn't part of the API anyway.

Hm, I wouldn't say so. Applications like weston and kmscon no longer use the legacy drmOpen() facility. They use udev+open(). So once it's upstream, it's part of the API regardless of libdrm. So the sole purpose of drm_rnodes is to mark it as "experimental".

Quoted 7 lines

Quoted 5 lines
This allows us to test render-nodes and play with the API. I added FLINK for
now so we can better test it. Not sure whether we should allow it in the end,
though.

Quoted 7 lines

From a security point of view, I don't think we should keep it as applications shouldn't
be trusted for not doing stupid things (because of code injection). So, unless we
plan on adding access control to flink via LSM, we shouldn't expose the feature
on render nodes.

This is also what I think. We have a chance to get rid of all legacy stuff, so maybe we should just drop it all.

Quoted 13 lines

From a dev point of view, keeping it means that the XServer doesn't have to know whether mesa supports render nodes or not. This is because the authentication dance isn't available on render nodes so the x-server has to tell mesa if it should authenticate or not. The other solution is to allow
the authentication ioctls on render nodes and just return 0 if this is a render node.
This way, we won't need any modification in mesa/xserver/dri2proto to pass the information that no authentication is needed. In this solution, only libdrm and
the ddx should be modified to make use of the render node. That's not how I did it on my render node patchset, can't remember why...

Quoted 1 line

What do you guys think?

We discussed that a bit on IRC. Of course, we can add a lot of wrappers and workarounds. We can make all the drmAuth stuff *just work*. But that means, we keep all the legacy. As said, we have the chance to introduce a new API and drop all the legacy. I think it is worth a shot. And we also notice quite fast which user-space programs need some rework.

Quoted 10 lines

Quoted 2 lines
Maybe we can get this into 3.11?
As long as we don't have to keep the interface stable (I don't want to expose flink
on render nodes), I'm all for pushing the code now. Otherwise, a kernel branch
somewhere is sufficient.

Quoted 7 lines

Do you plan on checking my userspace patches too? Those are enough to make use
of the render nodes on X, but I haven't tested that all the combinations of version
would still work. The libdrm work should be quite solid though (there are even libdrm
tests for the added functionalities :)).

I plan on having a working user-space for XDC. Most of your patches can be copied unchanged indeed. But servers other than Xorg don't use that, so they need separate fixes.

Cheers
David

Martin Peres

2013-08-25 18:22 UTC #17 lore raw

Quoted 20 lines · David Herrmann

Hi
Quoted 17 lines · Martin Peres
Quoted 14 lines · David Herrmann
Hi
I reduced the vma access-management patches to a minimum. I now do filp* tracking in gem unconditionally and force drm_gem_mmap() to check this. Hence,
all gem drivers are safe now. For TTM drivers, I now use the already available
verify_access() callback to get access to the underlying gem-object. Pretty
simple.. Why hadn't I thought of that before?
Long story short: All drivers using GEM are safe now. This leaves vmwgfx.. But
they do their own access-management, anyway.

Quoted 8 lines

Quoted 3 lines
Great! Thanks! Have you checked they are really safe with my "exploits"? I'll have another round of review even if it looked good the last time I checked.
Good you asked. I tested whether it works, I didn't actually verify that it correctly fails in case of exploits. And in fact there is a small bug (I return "1" instead of -EACCES, stupid verify_access()) so user-space gets a segfault accessing the mmap when trying to exploit this. That actually doesn't sound that bad, does it? ;)

Good to know I could contribute a little to your work. You're doing a great job!

Quoted 1 line

v2 is on its way.

Yep, saw it.

Quoted 10 lines

Quoted 10 lines
Quoted 6 lines
The 3 patches on top implement render-nodes. I added a "drm_rnodes" module parameter to core drm. You need to pass "drm.rnodes=1" on the kernel command-line or via sysfs _before_ loading a driver. Otherwise, render nodes
will not be created.
By default, having the render nodes doesn't change the way the userspace works at all. So, what is the point of protecting it behind a parameter?

Quoted 7 lines

Quoted 3 lines
Is it to make it clear this isn't part of the API yet? I would say that as long
as libdrm hasn't been updated, this isn't part of the API anyway.
Hm, I wouldn't say so. Applications like weston and kmscon no longer use the legacy drmOpen() facility. They use udev+open(). So once it's upstream, it's part of the API regardless of libdrm. So the sole purpose of drm_rnodes is to mark it as "experimental".

Ah, I guess I'll have to have a look at this. I basically got preempted from adding render node support to Weston and I didn't take the time to check it again, the vma patches were more important first. Thanks for saving me a giant headache with GEM/TTM, I spent two week ends trying to track a leak for radeon cards.

Quoted 13 lines

Quoted 6 lines
Quoted 5 lines
This allows us to test render-nodes and play with the API. I added FLINK for
now so we can better test it. Not sure whether we should allow it in the end,
though.
Quoted 7 lines
From a security point of view, I don't think we should keep it as
applications shouldn't
be trusted for not doing stupid things (because of code injection). So, unless we
plan on adding access control to flink via LSM, we shouldn't expose the feature
on render nodes.

Quoted 2 lines

This is also what I think. We have a chance to get rid of all legacy stuff, so maybe we should just drop it all.

Great!

Quoted 14 lines

Quoted 14 lines
From a dev point of view, keeping it means that the XServer doesn't
have to know whether mesa supports render nodes or not. This is because the authentication dance isn't available on render nodes so the x-server has to tell mesa if it should authenticate or not. The other solution is to allow
the authentication ioctls on render nodes and just return 0 if this is a render node.
This way, we won't need any modification in mesa/xserver/dri2proto to pass the information that no authentication is needed. In this solution, only libdrm and
the ddx should be modified to make use of the render node. That's not how I did it on my render node patchset, can't remember why...

Quoted 7 lines

Quoted 1 line
What do you guys think?
We discussed that a bit on IRC. Of course, we can add a lot of wrappers and workarounds. We can make all the drmAuth stuff *just work*. But that means, we keep all the legacy. As said, we have the chance to introduce a new API and drop all the legacy. I think it is worth a shot. And we also notice quite fast which user-space programs need some rework.

Well, if by "all the legacy", you mean the authentication-related functions then yes.

How do you plan on handling the case where the ddx has been updated and passes the render node to a not-yet-updated mesa? Mesa will try to authenticate and it will fail.

Keeping the authentication IOCTLs seem to me like a lesser evil, especially since they would basically do nothing.

Quoted 16 lines

Quoted 9 lines
Quoted 2 lines
Maybe we can get this into 3.11?
As long as we don't have to keep the interface stable (I don't want to expose flink
on render nodes), I'm all for pushing the code now. Otherwise, a kernel branch
somewhere is sufficient.
Quoted 7 lines
Do you plan on checking my userspace patches too? Those are enough to make use
of the render nodes on X, but I haven't tested that all the combinations of version
would still work. The libdrm work should be quite solid though (there are even libdrm
tests for the added functionalities :)).

Quoted 3 lines

I plan on having a working user-space for XDC. Most of your patches can be copied unchanged indeed. But servers other than Xorg don't use that, so they need separate fixes.

Brilliant :) Looking forward to it!

Martin

David Herrmann

author

2013-08-25 16:29 UTC #18 lore raw

PATCH 1/7 drm/vma: add access management helpers

In reply to David Herrmann

We use an rb-tree to store open-files for each VMA node. On each mmap call, GEM, TTM or the drivers must check whether the current user is allowed to map this file.

We add a separate lock for each node as there is no generic lock available for the caller to protect the node easily.

v2: add drm_vma_node_verify_access() helper

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/drm_gem.c         |   1 +
 drivers/gpu/drm/drm_vma_manager.c | 155 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_vma_manager.h     |  39 +++++++++-
 3 files changed, 192 insertions(+), 3 deletions(-)

drivers/gpu/drm/drm_gem.c +1 -0

index 1ce88c3..d6122ae 100644
@@ -156,6 +156,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	kref_init(&obj->refcount);
 	obj->handle_count = 0;
 	obj->size = size;
+	drm_vma_node_reset(&obj->vma_node);
 }
 EXPORT_SYMBOL(drm_gem_private_object_init);

drivers/gpu/drm/drm_vma_manager.c +155 -0

170 lines not on this page · raw message

include/drm/drm_vma_manager.h +36 -3

index 22eedac..c18a593 100644
@@ -24,15 +24,24 @@
  */
 
 #include <drm/drm_mm.h>
+#include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/rbtree.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 
+struct drm_vma_offset_file {
+	struct rb_node vm_rb;
+	struct file *vm_filp;
+	unsigned long vm_count;
+};
+
 struct drm_vma_offset_node {
+	rwlock_t vm_lock;
 	struct drm_mm_node vm_node;
 	struct rb_node vm_rb;
+	struct rb_root vm_files;
 };
 
 struct drm_vma_offset_manager {

@@ -56,6 +65,11 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
 void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
 			   struct drm_vma_offset_node *node);
 
+int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp);
+void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp);
+bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
+			     struct file *filp);
+
 /**
  * drm_vma_offset_exact_lookup() - Look up node by exact address
  * @mgr: Manager object

@@ -122,9 +136,8 @@ static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *m
  * drm_vma_node_reset() - Initialize or reset node object
  * @node: Node to initialize or reset
  *
- * Reset a node to its initial state. This must be called if @node isn't
- * already cleared (eg., via kzalloc) before using it with any VMA offset
- * manager.
+ * Reset a node to its initial state. This must be called before using it with
+ * any VMA offset manager.
  *
  * This must not be called on an already allocated node, or you will leak
  * memory.

@@ -132,6 +145,8 @@ static inline void drm_vma_offset_unlock_lookup(struct drm_vma_offset_manager *m
 static inline void drm_vma_node_reset(struct drm_vma_offset_node *node)
 {
 	memset(node, 0, sizeof(*node));
+	node->vm_files = RB_ROOT;
+	rwlock_init(&node->vm_lock);
 }
 
 /**

@@ -221,4 +236,22 @@ static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
 				    drm_vma_node_size(node) << PAGE_SHIFT, 1);
 }
 
+/**
+ * drm_vma_node_verify_access() - Access verification helper for TTM
+ * @node: Offset node
+ * @filp: Open-file
+ *
+ * This checks whether @filp is granted access to @node. It is the same as
+ * drm_vma_node_is_allowed() but suitable as drop-in helper for TTM
+ * verify_access() callbacks.
+ *
+ * RETURNS:
+ * 0 if access is granted, -EACCES otherwise.
+ */
+static inline int drm_vma_node_verify_access(struct drm_vma_offset_node *node,
+					     struct file *filp)
+{
+	return drm_vma_node_is_allowed(node, filp) ? 0 : -EACCES;
+}
+
 #endif /* __DRM_VMA_MANAGER_H__ */

Signature · 1 line

1.8.4

David Herrmann

author

2013-08-25 16:29 UTC #19 lore raw

PATCH 2/7 drm/gem: implement vma access management

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/drm_gem.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/gpu/drm/drm_gem.c +17 -0

index d6122ae..b2d59b2 100644
@@ -298,6 +298,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 	spin_unlock(&filp->table_lock);
 
 	drm_gem_remove_prime_handles(obj, filp);
+	drm_vma_node_revoke(&obj->vma_node, filp->filp);
 
 	if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, filp);

@@ -357,6 +358,11 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 	}
 	*handlep = ret;
 
+	ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp);
+	if (ret) {
+		drm_gem_handle_delete(file_priv, *handlep);
+		return ret;
+	}
 
 	if (dev->driver->gem_open_object) {
 		ret = dev->driver->gem_open_object(obj, file_priv);

@@ -701,6 +707,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
 	struct drm_device *dev = obj->dev;
 
 	drm_gem_remove_prime_handles(obj, file_priv);
+	drm_vma_node_revoke(&obj->vma_node, file_priv->filp);
 
 	if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, file_priv);

@@ -793,6 +800,10 @@ EXPORT_SYMBOL(drm_gem_vm_close);
  * the GEM object is not looked up based on its fake offset. To implement the
  * DRM mmap operation, drivers should use the drm_gem_mmap() function.
  *
+ * drm_gem_mmap_obj() assumes the user is granted access to the buffer while
+ * drm_gem_mmap() prevents unprivileged users from mapping random objects. So
+ * callers must verify access restrictions before calling this helper.
+ *
  * NOTE: This function has to be protected with dev->struct_mutex
  *
  * Return 0 or success or -EINVAL if the object size is smaller than the VMA

@@ -841,6 +852,9 @@ EXPORT_SYMBOL(drm_gem_mmap_obj);
  * Look up the GEM object based on the offset passed in (vma->vm_pgoff will
  * contain the fake offset we created when the GTT map ioctl was called on
  * the object) and map it with a call to drm_gem_mmap_obj().
+ *
+ * If the caller is not granted access to the buffer object, the mmap will fail
+ * with EACCES. Please see the vma manager for more information.
  */
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 {

@@ -861,6 +875,9 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	if (!node) {
 		mutex_unlock(&dev->struct_mutex);
 		return drm_mmap(filp, vma);
+	} else if (!drm_vma_node_is_allowed(node, filp)) {
+		mutex_unlock(&dev->struct_mutex);
+		return -EACCES;
 	}
 
 	obj = container_of(node, struct drm_gem_object, vma_node);

Signature · 1 line

1.8.4

David Herrmann

author

2013-08-25 16:29 UTC #20 lore raw

PATCH 3/7 drm: verify vma access in TTM+GEM drivers

In reply to David Herrmann

GEM does already a good job in tracking access to gem buffers via handles and drm_vma access management. However, TTM drivers currently do not verify this during mmap().

TTM provides the verify_access() callback to test this. So fix all drivers to actually call into gem+vma to verify access instead of always returning 0.

v2: Switch to drm_vma_node_verify_access() to correctly return -EACCES if
access was denied.

Cc Dave Airlie <[email protected]>

Cc Alex Deucher <[email protected]>

Cc Ben Skeggs <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Cc Jerome Glisse <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/ast/ast_ttm.c         | 4 +++-
 drivers/gpu/drm/cirrus/cirrus_ttm.c   | 4 +++-
 drivers/gpu/drm/mgag200/mgag200_ttm.c | 4 +++-
 drivers/gpu/drm/nouveau/nouveau_bo.c  | 4 +++-
 drivers/gpu/drm/qxl/qxl_ttm.c         | 4 +++-
 drivers/gpu/drm/radeon/radeon_ttm.c   | 4 +++-
 6 files changed, 18 insertions(+), 6 deletions(-)

drivers/gpu/drm/ast/ast_ttm.c +3 -1

index cf1c833..20fcf4e 100644
@@ -148,7 +148,9 @@ ast_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
 
 static int ast_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct ast_bo *astbo = ast_bo(bo);
+
+	return drm_vma_node_verify_access(&astbo->gem.vma_node, filp);
 }
 
 static int ast_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/cirrus/cirrus_ttm.c +3 -1

index bf8a506..ae2385c 100644
@@ -148,7 +148,9 @@ cirrus_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
 
 static int cirrus_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct cirrus_bo *cirrusbo = cirrus_bo(bo);
+
+	return drm_vma_node_verify_access(&cirrusbo->gem.vma_node, filp);
 }
 
 static int cirrus_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/mgag200/mgag200_ttm.c +3 -1

index 6cf3fa0..fd4539d 100644
@@ -148,7 +148,9 @@ mgag200_bo_evict_flags(struct ttm_buffer_object *bo, struct ttm_placement *pl)
 
 static int mgag200_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct mgag200_bo *mgabo = mgag200_bo(bo);
+
+	return drm_vma_node_verify_access(&mgabo->gem.vma_node, filp);
 }
 
 static int mgag200_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/nouveau/nouveau_bo.c +3 -1

index 4e7ee5f..e4444ba 100644
@@ -1260,7 +1260,9 @@ out:
 static int
 nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct nouveau_bo *nvbo = nouveau_bo(bo);
+
+	return drm_vma_node_verify_access(&nvbo->gem->vma_node, filp);
 }
 
 static int

drivers/gpu/drm/qxl/qxl_ttm.c +3 -1

index 1dfd84c..037786d 100644
@@ -212,7 +212,9 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
 
 static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct qxl_bo *qbo = to_qxl_bo(bo);
+
+	return drm_vma_node_verify_access(&qbo->gem_base.vma_node, filp);
 }
 
 static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,

drivers/gpu/drm/radeon/radeon_ttm.c +3 -1

index 6c0ce89..71245d6 100644
@@ -203,7 +203,9 @@ static void radeon_evict_flags(struct ttm_buffer_object *bo,
 
 static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
-	return 0;
+	struct radeon_bo *rbo = container_of(bo, struct radeon_bo, tbo);
+
+	return drm_vma_node_verify_access(&rbo->gem_base.vma_node, filp);
 }
 
 static void radeon_move_null(struct ttm_buffer_object *bo,

Signature · 1 line

1.8.4

David Herrmann

author

2013-08-25 16:29 UTC #21 lore raw

PATCH 4/7 drm: implement experimental render nodes

In reply to David Herrmann

v2: drop insecure GEM_FLINK to force use of dmabuf

Signed-off-by David Herrmann <[email protected]>

---
 Documentation/DocBook/drm.tmpl | 69 ++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_drv.c      | 13 ++++----
 drivers/gpu/drm/drm_fops.c     | 14 ++++-----
 drivers/gpu/drm/drm_pci.c      |  9 ++++++
 drivers/gpu/drm/drm_platform.c |  9 ++++++
 drivers/gpu/drm/drm_stub.c     | 10 ++++++
 drivers/gpu/drm/drm_usb.c      |  9 ++++++
 include/drm/drmP.h             |  9 ++++++
 8 files changed, 129 insertions(+), 13 deletions(-)

Documentation/DocBook/drm.tmpl +69 -0

index 9fc8ed4..ed1d6d2 100644
@@ -205,6 +205,12 @@
               Driver implements DRM PRIME buffer sharing.
             </para></listitem>
           </varlistentry>
+          <varlistentry>
+            <term>DRIVER_RENDER</term>
+            <listitem><para>
+              Driver supports dedicated render nodes.
+            </para></listitem>
+          </varlistentry>
         </variablelist>
       </sect3>
       <sect3>

@@ -2644,6 +2650,69 @@ int (*resume) (struct drm_device *);</synopsis>
       info, since man pages should cover the rest.
     </para>
 
+  <!-- External: render nodes -->
+
+    <sect1>
+      <title>Render nodes</title>
+      <para>
+        DRM core provides multiple character-devices for user-space to use.
+        Depending on which device is opened, user-space can perform a different
+        set of operations (mainly ioctls). The primary node is always created
+        and called <term>card&lt;num&gt;</term>. Additionally, a currently
+        unused control node, called <term>controlD&lt;num&gt;</term> is also
+        created. The primary node provides all legacy operations and
+        historically was the only interface used by userspace. With KMS, the
+        control node was introduced. However, the planned KMS control interface
+        has never been written and so the control node stays unused to date.
+      </para>
+      <para>
+        With the increased use of offscreen renderers and GPGPU applications,
+        clients no longer require running compositors or graphics servers to
+        make use of a GPU. But the DRM API required unprivileged clients to
+        authenticate to a DRM-Master prior to getting GPU access. To avoid this
+        step and to grant clients GPU access without authenticating, render
+        nodes were introduced. Render nodes solely serve render clients, that
+        is, no modesetting or privileged ioctls can be issued on render nodes.
+        Only non-global rendering commands are allowed. If a driver supports
+        render nodes, it must advertise it via the <term>DRIVER_RENDER</term>
+        DRM driver capability. If not supported, the primary node must be used
+        for render clients together with the legacy drmAuth authentication
+        procedure.
+      </para>
+      <para>
+        If a driver advertises render node support, DRM core will create a
+        separate render node called <term>renderD&lt;num&gt;</term>. There will
+        be one render node per device. No ioctls except  PRIME-related ioctls
+        will be allowed on this node. Especially <term>GEM_OPEN</term> will be
+        explicitly prohibited. Render nodes are designed to avoid the
+        buffer-leaks, which occur if clients guess the flink names or mmap
+        offsets on the legacy interface. Additionally to this basic interface,
+        drivers must mark their driver-dependent render-only ioctls as
+        <term>DRM_RENDER_ALLOW</term> so render clients can use them. Driver
+        authors must be careful not to allow any privileged ioctls on render
+        nodes.
+      </para>
+      <para>
+        With render nodes, user-space can now control access to the render node
+        via basic file-system access-modes. A running graphics server which
+        authenticates clients on the privileged primary/legacy node is no longer
+        required. Instead, a client can open the render node and is immediately
+        granted GPU access. Communication between clients (or servers) is done
+        via PRIME. FLINK from render node to legacy node is not supported. New
+        clients must not use the insecure FLINK interface.
+      </para>
+      <para>
+        Besides dropping all modeset/global ioctls, render nodes also drop the
+        DRM-Master concept. There is no reason to associate render clients with
+        a DRM-Master as they are independent of any graphics server. Besides,
+        they must work without any running master, anyway.
+        Drivers must be able to run without a master object if they support
+        render nodes. If, on the other hand, a driver requires shared state
+        between clients which is visible to user-space and accessible beyond
+        open-file boundaries, they cannot support render nodes.
+      </para>
+    </sect1>
+
   <!-- External: vblank handling -->
 
     <sect1>

drivers/gpu/drm/drm_drv.c +7 -6

index 288da3d..e572dd2 100644
@@ -68,7 +68,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED),
-	DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_VERSION, drm_setversion, DRM_MASTER),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_SET_UNIQUE, drm_setunique, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),

@@ -130,14 +130,14 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 
 	DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_GEM_OPEN, drm_gem_open_ioctl, DRM_AUTH|DRM_UNLOCKED),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETRESOURCES, drm_mode_getresources, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_HANDLE_TO_FD, drm_prime_handle_to_fd_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED),

@@ -420,9 +420,10 @@ long drm_ioctl(struct file *filp,
 		DRM_DEBUG("no function\n");
 		retcode = -EINVAL;
 	} else if (((ioctl->flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)) ||
-		   ((ioctl->flags & DRM_AUTH) && !file_priv->authenticated) ||
+		   ((ioctl->flags & DRM_AUTH) && !drm_is_render_client(file_priv) && !file_priv->authenticated) ||
 		   ((ioctl->flags & DRM_MASTER) && !file_priv->is_master) ||
-		   (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL))) {
+		   (!(ioctl->flags & DRM_CONTROL_ALLOW) && (file_priv->minor->type == DRM_MINOR_CONTROL)) ||
+		   (!(ioctl->flags & DRM_RENDER_ALLOW) && drm_is_render_client(file_priv))) {
 		retcode = -EACCES;
 	} else {
 		if (cmd & (IOC_IN | IOC_OUT)) {

drivers/gpu/drm/drm_fops.c +7 -7

index 2d2401e..6d1726a 100644
@@ -262,10 +262,10 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 			goto out_prime_destroy;
 	}
 
-
-	/* if there is no current master make this fd it */
+	/* if there is no current master make this fd it, but do not create
+	 * any master object for render clients */
 	mutex_lock(&dev->struct_mutex);
-	if (!priv->minor->master) {
+	if (!priv->minor->master && !drm_is_render_client(priv)) {
 		/* create a new master */
 		priv->minor->master = drm_master_create(priv->minor);
 		if (!priv->minor->master) {

@@ -303,12 +303,11 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
 				goto out_close;
 			}
 		}
-		mutex_unlock(&dev->struct_mutex);
-	} else {
+	} else if (!drm_is_render_client(priv)) {
 		/* get a reference to the master */
 		priv->master = drm_master_get(priv->minor->master);
-		mutex_unlock(&dev->struct_mutex);
 	}
+	mutex_unlock(&dev->struct_mutex);
 
 	mutex_lock(&dev->struct_mutex);
 	list_add(&priv->lhead, &dev->filelist);

@@ -478,7 +477,8 @@ int drm_release(struct inode *inode, struct file *filp)
 	iput(container_of(dev->dev_mapping, struct inode, i_data));
 
 	/* drop the reference held my the file priv */
-	drm_master_put(&file_priv->master);
+	if (file_priv->master)
+		drm_master_put(&file_priv->master);
 	file_priv->is_master = 0;
 	list_del(&file_priv->lhead);
 	mutex_unlock(&dev->struct_mutex);

drivers/gpu/drm/drm_pci.c +9 -0

index 3fca2db..1f96cee 100644
@@ -354,6 +354,12 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 			goto err_g2;
 	}
 
+	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
+		ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
+		if (ret)
+			goto err_g21;
+	}
+
 	if ((ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY)))
 		goto err_g3;

@@ -383,6 +389,9 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 err_g4:
 	drm_put_minor(&dev->primary);
 err_g3:
+	if (dev->render)
+		drm_put_minor(&dev->render);
+err_g21:
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_put_minor(&dev->control);
 err_g2:

drivers/gpu/drm/drm_platform.c +9 -0

index 400024b..f7a18c6 100644
@@ -69,6 +69,12 @@ static int drm_get_platform_dev(struct platform_device *platdev,
 			goto err_g1;
 	}
 
+	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
+		ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
+		if (ret)
+			goto err_g11;
+	}
+
 	ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
 	if (ret)
 		goto err_g2;

@@ -100,6 +106,9 @@ static int drm_get_platform_dev(struct platform_device *platdev,
 err_g3:
 	drm_put_minor(&dev->primary);
 err_g2:
+	if (dev->render)
+		drm_put_minor(&dev->render);
+err_g11:
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_put_minor(&dev->control);
 err_g1:

drivers/gpu/drm/drm_stub.c +10 -0

index e30bb0d..e7eb027 100644
@@ -40,6 +40,9 @@
 unsigned int drm_debug = 0;	/* 1 to enable debug output */
 EXPORT_SYMBOL(drm_debug);
 
+unsigned int drm_rnodes = 0;	/* 1 to enable experimental render nodes API */
+EXPORT_SYMBOL(drm_rnodes);
+
 unsigned int drm_vblank_offdelay = 5000;    /* Default to 5000 msecs. */
 EXPORT_SYMBOL(drm_vblank_offdelay);

@@ -56,11 +59,13 @@ MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
 MODULE_PARM_DESC(debug, "Enable debug output");
+MODULE_PARM_DESC(rnodes, "Enable experimental render nodes API");
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
 MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic timestamps");
 
 module_param_named(debug, drm_debug, int, 0600);
+module_param_named(rnodes, drm_rnodes, int, 0600);
 module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
 module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600);

@@ -446,6 +451,9 @@ void drm_put_dev(struct drm_device *dev)
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_put_minor(&dev->control);
 
+	if (dev->render)
+		drm_put_minor(&dev->render);
+
 	if (driver->driver_features & DRIVER_GEM)
 		drm_gem_destroy(dev);

@@ -462,6 +470,8 @@ void drm_unplug_dev(struct drm_device *dev)
 	/* for a USB device */
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_unplug_minor(dev->control);
+	if (dev->render)
+		drm_unplug_minor(dev->render);
 	drm_unplug_minor(dev->primary);
 
 	mutex_lock(&drm_global_mutex);

drivers/gpu/drm/drm_usb.c +9 -0

index 34a156f..8766472 100644
@@ -33,6 +33,12 @@ int drm_get_usb_dev(struct usb_interface *interface,
 	if (ret)
 		goto err_g1;
 
+	if (drm_core_check_feature(dev, DRIVER_RENDER) && drm_rnodes) {
+		ret = drm_get_minor(dev, &dev->render, DRM_MINOR_RENDER);
+		if (ret)
+			goto err_g11;
+	}
+
 	ret = drm_get_minor(dev, &dev->primary, DRM_MINOR_LEGACY);
 	if (ret)
 		goto err_g2;

@@ -62,6 +68,9 @@ int drm_get_usb_dev(struct usb_interface *interface,
 err_g3:
 	drm_put_minor(&dev->primary);
 err_g2:
+	if (dev->render)
+		drm_put_minor(&dev->render);
+err_g11:
 	drm_put_minor(&dev->control);
 err_g1:
 	kfree(dev);

include/drm/drmP.h +9 -0

index 90833dc..8fd48bd 100644
@@ -145,6 +145,7 @@ int drm_err(const char *func, const char *format, ...);
 #define DRIVER_GEM         0x1000
 #define DRIVER_MODESET     0x2000
 #define DRIVER_PRIME       0x4000
+#define DRIVER_RENDER      0x8000
 
 #define DRIVER_BUS_PCI 0x1
 #define DRIVER_BUS_PLATFORM 0x2

@@ -290,6 +291,7 @@ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
 #define DRM_ROOT_ONLY	0x4
 #define DRM_CONTROL_ALLOW 0x8
 #define DRM_UNLOCKED	0x10
+#define DRM_RENDER_ALLOW 0x20
 
 struct drm_ioctl_desc {
 	unsigned int cmd;

@@ -1204,6 +1206,7 @@ struct drm_device {
 	unsigned int agp_buffer_token;
 	struct drm_minor *control;		/**< Control node for card */
 	struct drm_minor *primary;		/**< render type primary screen head */
+	struct drm_minor *render;		/**< render node for card */
 
         struct drm_mode_config mode_config;	/**< Current mode config */

@@ -1250,6 +1253,11 @@ static inline bool drm_modeset_is_locked(struct drm_device *dev)
 	return mutex_is_locked(&dev->mode_config.mutex);
 }
 
+static inline bool drm_is_render_client(struct drm_file *file_priv)
+{
+	return file_priv->minor->type == DRM_MINOR_RENDER;
+}
+
 /******************************************************************/
 /** \name Internal function definitions */
 /*@{*/

@@ -1449,6 +1457,7 @@ extern void drm_put_dev(struct drm_device *dev);
 extern int drm_put_minor(struct drm_minor **minor);
 extern void drm_unplug_dev(struct drm_device *dev);
 extern unsigned int drm_debug;
+extern unsigned int drm_rnodes;
 
 extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;

Signature · 1 line

1.8.4

David Herrmann

author

2013-08-25 16:29 UTC #22 lore raw

PATCH 5/7 drm/i915: Support render nodes

In reply to David Herrmann

From: Kristian Høgsberg <[email protected]>

Enable support for drm render nodes for i915 by flagging the ioctls that are safe and just needed for rendering.

v2: mark reg_read, set_caching and get_caching (ickle, danvet)

Signed-off-by Kristian Høgsberg <[email protected]>

Signed-off-by David Herrmann <[email protected]>

Reviewed-by Daniel Vetter <[email protected]>

---
 drivers/gpu/drm/i915/i915_dma.c | 42 ++++++++++++++++++++---------------------
 drivers/gpu/drm/i915/i915_drv.c |  3 ++-
 2 files changed, 23 insertions(+), 22 deletions(-)

drivers/gpu/drm/i915/i915_dma.c +21 ~~-21~~

index 0adfe40..8fd814d 100644
@@ -1840,7 +1840,7 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_BATCHBUFFER, i915_batchbuffer, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_EMIT, i915_irq_emit, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_IRQ_WAIT, i915_irq_wait, DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(I915_GETPARAM, i915_getparam, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_SETPARAM, i915_setparam, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(I915_ALLOC, drm_noop, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(I915_FREE, drm_noop, DRM_AUTH),

@@ -1853,35 +1853,35 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_HWS_ADDR, i915_set_status_page, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(I915_GEM_INIT, i915_gem_init_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER, i915_gem_execbuffer, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_EXECBUFFER2, i915_gem_execbuffer2, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_PIN, i915_gem_pin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_UNPIN, i915_gem_unpin_ioctl, DRM_AUTH|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_BUSY, i915_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_CACHING, i915_gem_set_caching_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_CACHING, i915_gem_get_caching_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_THROTTLE, i915_gem_throttle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_ENTERVT, i915_gem_entervt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GEM_LEAVEVT, i915_gem_leavevt_ioctl, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CREATE, i915_gem_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PREAD, i915_gem_pread_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_PWRITE, i915_gem_pwrite_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP, i915_gem_mmap_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GET_PIPE_FROM_CRTC_ID, intel_get_pipe_from_crtc_id, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_PUT_IMAGE, intel_overlay_put_image, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_OVERLAY_ATTRS, intel_overlay_attrs, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_SET_SPRITE_COLORKEY, intel_sprite_set_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(I915_GET_SPRITE_COLORKEY, intel_sprite_get_colorkey, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(I915_GEM_WAIT, i915_gem_wait_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, i915_gem_context_create_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, i915_gem_context_destroy_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);

drivers/gpu/drm/i915/i915_drv.c +2 -1

index eec47bd..5c0be0e 100644
@@ -1007,7 +1007,8 @@ static struct drm_driver driver = {
 	 */
 	.driver_features =
 	    DRIVER_USE_AGP | DRIVER_REQUIRE_AGP |
-	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
+	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME |
+	    DRIVER_RENDER,
 	.load = i915_driver_load,
 	.unload = i915_driver_unload,
 	.open = i915_driver_open,

Signature · 6 lines

1.8.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

David Herrmann

author

2013-08-25 16:29 UTC #23 lore raw

PATCH 6/7 drm/nouveau: Support render nodes

In reply to David Herrmann

From: Martin Peres <[email protected]>

Enable support for drm render nodes for nouveau by flagging the ioctls that are safe and just needed for rendering.

Cc Ben Skeggs <[email protected]>

Cc Maarten Lankhorst <[email protected]>

Signed-off-by Martin Peres <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

drivers/gpu/drm/nouveau/nouveau_drm.c +12 ~~-12~~

index b29d04b..ab42a25 100644
@@ -651,18 +651,18 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
 
 static const struct drm_ioctl_desc
 nouveau_ioctls[] = {
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_UNLOCKED|DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GETPARAM, nouveau_abi16_ioctl_getparam, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(NOUVEAU_SETPARAM, nouveau_abi16_ioctl_setparam, DRM_UNLOCKED|DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH),
-	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_ALLOC, nouveau_abi16_ioctl_channel_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_CHANNEL_FREE, nouveau_abi16_ioctl_channel_free, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_UNLOCKED|DRM_AUTH|DRM_RENDER_ALLOW),
 };
 
 static const struct file_operations

@@ -684,7 +684,7 @@ static struct drm_driver
 driver = {
 	.driver_features =
 		DRIVER_USE_AGP |
-		DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME,
+		DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER,
 
 	.load = nouveau_drm_load,
 	.unload = nouveau_drm_unload,

Signature · 1 line

1.8.4

David Herrmann

author

2013-08-25 16:29 UTC #24 lore raw

PATCH 7/7 drm/radeon: support render nodes

In reply to David Herrmann

From: Christian König <[email protected]>

Enable support for drm render nodes for radeon by flagging the ioctls that are safe and just needed for rendering.

Signed-off-by Christian König <[email protected]>

Signed-off-by David Herrmann <[email protected]>

---
 drivers/gpu/drm/radeon/radeon_drv.c |  2 +-
 drivers/gpu/drm/radeon/radeon_kms.c | 22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

drivers/gpu/drm/radeon/radeon_drv.c +1 -1

index 1f93dd5..dffbb78 100644
@@ -384,7 +384,7 @@ static struct drm_driver kms_driver = {
 	.driver_features =
 	    DRIVER_USE_AGP |
 	    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
-	    DRIVER_PRIME,
+	    DRIVER_PRIME | DRIVER_RENDER,
 	.dev_priv_size = 0,
 	.load = radeon_driver_load_kms,
 	.open = radeon_driver_open_kms,

drivers/gpu/drm/radeon/radeon_kms.c +11 ~~-11~~

index b46a561..1593ee6 100644
@@ -738,18 +738,18 @@ const struct drm_ioctl_desc radeon_ioctls_kms[] = {
 	DRM_IOCTL_DEF_DRV(RADEON_SURF_ALLOC, radeon_surface_alloc_kms, DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(RADEON_SURF_FREE, radeon_surface_free_kms, DRM_AUTH),
 	/* KMS */
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_INFO, radeon_gem_info_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_CREATE, radeon_gem_create_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_MMAP, radeon_gem_mmap_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_DOMAIN, radeon_gem_set_domain_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_INFO, radeon_gem_info_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_CREATE, radeon_gem_create_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_MMAP, radeon_gem_mmap_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_DOMAIN, radeon_gem_set_domain_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(RADEON_GEM_PREAD, radeon_gem_pread_ioctl, DRM_AUTH|DRM_UNLOCKED),
 	DRM_IOCTL_DEF_DRV(RADEON_GEM_PWRITE, radeon_gem_pwrite_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_WAIT_IDLE, radeon_gem_wait_idle_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_CS, radeon_cs_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_INFO, radeon_info_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_TILING, radeon_gem_set_tiling_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_GET_TILING, radeon_gem_get_tiling_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_BUSY, radeon_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED),
-	DRM_IOCTL_DEF_DRV(RADEON_GEM_VA, radeon_gem_va_ioctl, DRM_AUTH|DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_WAIT_IDLE, radeon_gem_wait_idle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_CS, radeon_cs_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_INFO, radeon_info_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_SET_TILING, radeon_gem_set_tiling_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_GET_TILING, radeon_gem_get_tiling_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_BUSY, radeon_gem_busy_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(RADEON_GEM_VA, radeon_gem_va_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 int radeon_max_kms_ioctl = DRM_ARRAY_SIZE(radeon_ioctls_kms);

Signature · 6 lines

1.8.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel