diff mbox series

[v4,13/61] drm/i915: Reject more ioctls for userptr

Message ID 20201016104444.1492028-14-maarten.lankhorst@linux.intel.com
State New, archived
Headers show
Series drm/i915: Remove obj->mm.lock! | expand

Commit Message

Maarten Lankhorst Oct. 16, 2020, 10:43 a.m. UTC
Allow set_domain to fail silently, waiting for idle should be good enough.
set_tiling and set_caching are rejected with -ENXIO, there's no valid reason
to allow it.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 2 +-
 drivers/gpu/drm/i915/gem/i915_gem_domain.c   | 4 +++-
 drivers/gpu/drm/i915/gem/i915_gem_object.h   | 6 ++++++
 drivers/gpu/drm/i915/gem/i915_gem_userptr.c  | 3 ++-
 4 files changed, 12 insertions(+), 3 deletions(-)

Comments

Thomas Hellström (Intel) Oct. 30, 2020, 9:22 a.m. UTC | #1
On 10/16/20 12:43 PM, Maarten Lankhorst wrote:
> Allow set_domain to fail silently, waiting for idle should be good enough.
> set_tiling and set_caching are rejected with -ENXIO, there's no valid reason
> to allow it.

Please list all affected ioctls affected by the IS_PROXY flag. We also 
need userspace maintainer acks for this.


> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Maarten Lankhorst Oct. 30, 2020, 9:56 a.m. UTC | #2
Op 30-10-2020 om 10:22 schreef Thomas Hellström (Intel):
>
> On 10/16/20 12:43 PM, Maarten Lankhorst wrote:
>> Allow set_domain to fail silently, waiting for idle should be good enough.
>> set_tiling and set_caching are rejected with -ENXIO, there's no valid reason
>> to allow it.
>
> Please list all affected ioctls affected by the IS_PROXY flag. We also need userspace maintainer acks for this.

set_caching, set_domain and set_tiling. set_domain turns into a gem_wait for coherent userptr, since the other ioctl's that affect placement and caching are no longer allowed. :)

~Maarten

>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Thomas Hellström (Intel) Oct. 30, 2020, 2:14 p.m. UTC | #3
Hi, Maarten,

On 10/30/20 10:56 AM, Maarten Lankhorst wrote:
> Op 30-10-2020 om 10:22 schreef Thomas Hellström (Intel):
>> On 10/16/20 12:43 PM, Maarten Lankhorst wrote:
>>> Allow set_domain to fail silently, waiting for idle should be good enough.
>>> set_tiling and set_caching are rejected with -ENXIO, there's no valid reason
>>> to allow it.
>> Please list all affected ioctls affected by the IS_PROXY flag. We also need userspace maintainer acks for this.
> set_caching, set_domain and set_tiling. set_domain turns into a gem_wait for coherent userptr, since the other ioctl's that affect placement and caching are no longer allowed. :)

Sounds fine. The comment was about adding to the commit message.


> ~Maarten
>
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a02ca7de72de..5690e2ae2366 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -17365,7 +17365,7 @@  static int intel_user_framebuffer_create_handle(struct drm_framebuffer *fb,
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
-	if (obj->userptr.mm) {
+	if (i915_gem_object_is_userptr(obj)) {
 		drm_dbg(&i915->drm,
 			"attempting to use a userptr for a framebuffer, denied\n");
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 7c90a63c273d..43c22648b074 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -543,7 +543,9 @@  i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	 * considered to be outside of any cache domain.
 	 */
 	if (i915_gem_object_is_proxy(obj)) {
-		err = -ENXIO;
+		/* silently allow userptr to complete */
+		if (!i915_gem_object_is_userptr(obj))
+			err = -ENXIO;
 		goto out;
 	}
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e0c1e2817bee..436ff0d4951f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -528,6 +528,12 @@  void __i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj,
 void __i915_gem_object_invalidate_frontbuffer(struct drm_i915_gem_object *obj,
 					      enum fb_op_origin origin);
 
+static inline bool
+i915_gem_object_is_userptr(struct drm_i915_gem_object *obj)
+{
+	return obj->userptr.mm;
+}
+
 static inline void
 i915_gem_object_flush_frontbuffer(struct drm_i915_gem_object *obj,
 				  enum fb_op_origin origin)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
index 9c1293c99d88..3fd63fdd7466 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
@@ -721,7 +721,8 @@  static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
 	.name = "i915_gem_object_userptr",
 	.flags = I915_GEM_OBJECT_IS_SHRINKABLE |
 		 I915_GEM_OBJECT_NO_MMAP |
-		 I915_GEM_OBJECT_ASYNC_CANCEL,
+		 I915_GEM_OBJECT_ASYNC_CANCEL |
+		 I915_GEM_OBJECT_IS_PROXY,
 	.get_pages = i915_gem_userptr_get_pages,
 	.put_pages = i915_gem_userptr_put_pages,
 	.dmabuf_export = i915_gem_userptr_dmabuf_export,