[PATCH 00/03] Preventing zero GPU virtual address allocation

[PATCH 00/03] Preventing zero GPU virtual address allocation

[David Weinehall]

This patch series (one patch each for libdrm, the kernel, and beignet)
aims to provide a means to add a context-specific means to prevent
a mapping to GPU virtual address zero.  This is needed at least by
Beignet (possibly in other use-cases too, though I don't know of any
other) to allow use of address zero to represent NULL.

The kernel patch exports a new context parameter that can be set by
the context_{get,set}param ioctls, and uses this parameter to
decide whether or not a zero address mapping should be prevented.

The libdrm patch adds helper functions for the ioctls in question
and exports the context parameter.

The beignet patch uses the new libdrm function to disable zero
mappings if that functionality is available.

David Weinehall (3):
 drm/i915: add a context parameter to {en,dis}able zero address mapping
 libdrm: export context_{get,set}param and I915_CONTEXT_PARAM_NO_ZEROMAP
 beignet: set I915_CONTEXT_PARAM_NO_ZEROMAP when initializing context

drm-intel-nightly:
 drivers/gpu/drm/i915/i915_drv.h            |    5 +++++
 drivers/gpu/drm/i915/i915_gem_context.c    |   11 +++++++++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   13 +++++++++----
 include/uapi/drm/i915_drm.h                |    1 +
 4 files changed, 26 insertions(+), 4 deletions(-)

libdrm:
 configure.ac             |    2 -
 include/drm/i915_drm.h   |    1 
 intel/intel_bufmgr.h     |    4 +++
 intel/intel_bufmgr_gem.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 63 insertions(+), 1 deletion(-)

beignet:
 CMakeLists.txt           |    6 ++++++
 src/CMakeLists.txt       |    5 +++++
 src/intel/intel_driver.c |    4 ++++
 3 files changed, 15 insertions(+)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/03] drm/i915: add a context parameter to {en, dis}able zero address mapping

[David Weinehall]

Export a new context parameter that can be set/queried through the
context_{get,set}param ioctls.  This parameter is passed as a context
flag and decides whether or not a GPU address mapping is allowed to
be made at address zero.  The default is to allow such mappings.

Signed-off-by: David Weinehall <david.weinehall@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |    5 +++++
 drivers/gpu/drm/i915/i915_gem_context.c    |   11 +++++++++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   13 +++++++++----
 include/uapi/drm/i915_drm.h                |    1 +
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index acfa4fc93803..421640a53dd8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -781,11 +781,15 @@ struct i915_ctx_hang_stats {
 
 /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_HANDLE 0
+
+#define CONTEXT_NO_ZEROMAP (1<<0)
 /**
  * struct intel_context - as the name implies, represents a context.
  * @ref: reference count.
  * @user_handle: userspace tracking identity for this context.
  * @remap_slice: l3 row remapping information.
+ * @flags: context specific flags:
+ *         CONTEXT_NO_ZEROMAP: do not allow mapping things to page 0.
  * @file_priv: filp associated with this context (NULL for global default
  *	       context).
  * @hang_stats: information about the role of this context in possible GPU
@@ -802,6 +806,7 @@ struct intel_context {
 	struct kref ref;
 	int user_handle;
 	uint8_t remap_slice;
+	int flags;
 	struct drm_i915_file_private *file_priv;
 	struct i915_ctx_hang_stats hang_stats;
 	struct i915_hw_ppgtt *ppgtt;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 5a47eb5e3c5d..10bd0cfe34ad 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -902,6 +902,9 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 	case I915_CONTEXT_PARAM_BAN_PERIOD:
 		args->value = ctx->hang_stats.ban_period_seconds;
 		break;
+	case I915_CONTEXT_PARAM_NO_ZEROMAP:
+		args->value = ctx->flags & CONTEXT_NO_ZEROMAP;
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -939,6 +942,14 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 		else
 			ctx->hang_stats.ban_period_seconds = args->value;
 		break;
+	case I915_CONTEXT_PARAM_NO_ZEROMAP:
+		if (args->size) {
+			ret = -EINVAL;
+		} else {
+			ctx->flags &= ~CONTEXT_NO_ZEROMAP;
+			ctx->flags |= args->value ? CONTEXT_NO_ZEROMAP : 0;
+		}
+		break;
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 560c79a8a43d..6cfc716d7186 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -676,6 +676,7 @@ eb_vma_misplaced(struct i915_vma *vma)
 static int
 i915_gem_execbuffer_reserve(struct intel_engine_cs *ring,
 			    struct list_head *vmas,
+			    struct intel_context *ctx,
 			    bool *need_relocs)
 {
 	struct drm_i915_gem_object *obj;
@@ -698,6 +699,9 @@ i915_gem_execbuffer_reserve(struct intel_engine_cs *ring,
 		obj = vma->obj;
 		entry = vma->exec_entry;
 
+		if (ctx->flags & CONTEXT_NO_ZEROMAP)
+			entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
+
 		if (!has_fenced_gpu_access)
 			entry->flags &= ~EXEC_OBJECT_NEEDS_FENCE;
 		need_fence =
@@ -775,7 +779,8 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 				  struct drm_file *file,
 				  struct intel_engine_cs *ring,
 				  struct eb_vmas *eb,
-				  struct drm_i915_gem_exec_object2 *exec)
+				  struct drm_i915_gem_exec_object2 *exec,
+				  struct intel_context *ctx)
 {
 	struct drm_i915_gem_relocation_entry *reloc;
 	struct i915_address_space *vm;
@@ -861,7 +866,7 @@ i915_gem_execbuffer_relocate_slow(struct drm_device *dev,
 		goto err;
 
 	need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
-	ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, &need_relocs);
+	ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, ctx, &need_relocs);
 	if (ret)
 		goto err;
 
@@ -1515,7 +1520,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	/* Move the objects en-masse into the GTT, evicting if necessary. */
 	need_relocs = (args->flags & I915_EXEC_NO_RELOC) == 0;
-	ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, &need_relocs);
+	ret = i915_gem_execbuffer_reserve(ring, &eb->vmas, ctx, &need_relocs);
 	if (ret)
 		goto err;
 
@@ -1525,7 +1530,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (ret) {
 		if (ret == -EFAULT) {
 			ret = i915_gem_execbuffer_relocate_slow(dev, args, file, ring,
-								eb, exec);
+								eb, exec, ctx);
 			BUG_ON(!mutex_is_locked(&dev->struct_mutex));
 		}
 		if (ret)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 4851d660243c..5ae2b0f98999 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1102,6 +1102,7 @@ struct drm_i915_gem_context_param {
 	__u32 size;
 	__u64 param;
 #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
+#define I915_CONTEXT_PARAM_NO_ZEROMAP 0x2
 	__u64 value;
 };
 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/03] libdrm: export context_{get, set}param and I915_CONTEXT_PARAM_NO_ZEROMAP

[David Weinehall]

Provide helper functions for the context_{get,set}param ioctls,
as well as the I915_CONTEXT_PARAM_NO_ZEROMAP parameter.

Signed-off-by: David Weinehall <david.weinehall@intel.com>
---
 configure.ac             |    2 -
 include/drm/i915_drm.h   |    1 
 intel/intel_bufmgr.h     |    4 +++
 intel/intel_bufmgr_gem.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 78a0010d851f..e48fb7e87c52 100644
--- a/configure.ac
+++ b/configure.ac
@@ -20,7 +20,7 @@
 
 AC_PREREQ([2.63])
 AC_INIT([libdrm],
-        [2.4.61],
+        [2.4.62],
         [https://bugs.freedesktop.org/enter_bug.cgi?product=DRI],
         [libdrm])
 
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index ded43b1cb117..a658d1cc367a 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -1101,6 +1101,7 @@ struct drm_i915_gem_context_param {
 	__u32 size;
 	__u64 param;
 #define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
+#define I915_CONTEXT_PARAM_NO_ZEROMAP 0x2
 	__u64 value;
 };
 
diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
index 285919e4c40d..b9af2361735d 100644
--- a/intel/intel_bufmgr.h
+++ b/intel/intel_bufmgr.h
@@ -203,6 +203,10 @@ int drm_intel_gem_bo_wait(drm_intel_bo *bo, int64_t timeout_ns);
 
 drm_intel_context *drm_intel_gem_context_create(drm_intel_bufmgr *bufmgr);
 void drm_intel_gem_context_destroy(drm_intel_context *ctx);
+int drm_intel_gem_context_get_param(drm_intel_context *ctx,
+				    uint64_t param, uint64_t *value);
+int drm_intel_gem_context_set_param(drm_intel_context *ctx,
+				    uint64_t param, uint64_t value);
 int drm_intel_gem_bo_context_exec(drm_intel_bo *bo, drm_intel_context *ctx,
 				  int used, unsigned int flags);
 
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 60c06fccfb20..53378df2ecdf 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -3326,6 +3326,63 @@ drm_intel_gem_context_destroy(drm_intel_context *ctx)
 }
 
 int
+drm_intel_gem_context_get_param(drm_intel_context *ctx,
+				uint64_t param, uint64_t *value)
+{
+	drm_intel_bufmgr_gem *bufmgr_gem;
+	struct drm_i915_gem_context_param ctx_param;
+	int ret;
+
+	if (ctx == NULL)
+		return -EINVAL;
+
+	memclear(ctx_param);
+
+	bufmgr_gem = (drm_intel_bufmgr_gem *)ctx->bufmgr;
+
+	ctx_param.ctx_id = ctx->ctx_id;
+	ctx_param.param = param;
+
+	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM,
+		       &ctx_param);
+	if (ret != 0)
+		fprintf(stderr, "DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM failed: %s\n",
+			strerror(errno));
+	else
+		*value = ctx_param.value;
+
+	return ret;
+}
+
+int
+drm_intel_gem_context_set_param(drm_intel_context *ctx,
+				uint64_t param, uint64_t value)
+{
+	drm_intel_bufmgr_gem *bufmgr_gem;
+	struct drm_i915_gem_context_param ctx_param;
+	int ret;
+
+	if (ctx == NULL)
+		return -EINVAL;
+
+	memclear(ctx_param);
+
+	bufmgr_gem = (drm_intel_bufmgr_gem *)ctx->bufmgr;
+
+	ctx_param.ctx_id = ctx->ctx_id;
+	ctx_param.param = param;
+	ctx_param.value = value;
+
+	ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM,
+		       &ctx_param);
+	if (ret != 0)
+		fprintf(stderr, "DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM failed: %s (%d); %d\n",
+			strerror(errno), errno, ret);
+
+	return ret;
+}
+
+int
 drm_intel_get_reset_stats(drm_intel_context *ctx,
 			  uint32_t *reset_count,
 			  uint32_t *active,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/03] beignet: set I915_CONTEXT_PARAM_NO_ZEROMAP when initializing context

[David Weinehall]

Set the I915_CONTEXT_PARAM_NO_ZEROMAP context parameter to disable
zero mappings if libdrm is new enough to expose such functionality.

Signed-off-by: David Weinehall <david.weinehall@intel.com>
---
 CMakeLists.txt           |    6 ++++++
 src/CMakeLists.txt       |    5 +++++
 src/intel/intel_driver.c |    4 ++++
 3 files changed, 15 insertions(+)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5474447dbcf1..6c57a5346d65 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -146,6 +146,12 @@ IF(DRM_INTEL_FOUND)
     MESSAGE(STATUS "Disable EU total query support")
     MESSAGE(STATUS "Disable subslice total query support")
   ENDIF(DRM_INTEL_VERSION VERSION_GREATER 2.4.59)
+  IF(DRM_INTEL_VERSION VERSION_GREATER 2.4.61)
+    MESSAGE(STATUS "Enable no-zeromap support"
+    SET(DRM_INTEL_NOZEROMAP "enable")
+  ELSE(DRM_INTEL_VERSION VERSION_GREATER 2.4.61)
+    MESSAGE(STATUS "Disable no-zeromap support")
+  ENDIF(DRM_intEL_VERSION VERSION_GREATER 2.4.61)
 ELSE(DRM_INTEL_FOUND)
   MESSAGE(FATAL_ERROR "Looking for DRM Intel (>= 2.4.52) - not found")
 ENDIF(DRM_INTEL_FOUND)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index da695324829b..83c838ca20f6 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -119,6 +119,11 @@ SET(CMAKE_CXX_FLAGS "-DHAS_USERPTR ${CMAKE_CXX_FLAGS}")
 SET(CMAKE_C_FLAGS "-DHAS_USERPTR ${CMAKE_C_FLAGS}")
 endif (DRM_INTEL_USERPTR)
 
+if (DRM_INTEL_NOZEROMAP)
+SET(CMAKE_CXX_FLAGS "-DHAS_NOZEROMAP ${CMAKE_CXX_FLAGS}")
+SET(CMAKE_C_FLAGS "-DHAS_NOZEROMAP ${CMAKE_C_FLAGS}")
+endif (DRM_INTEL_NOZEROMAP)
+
 if (DRM_INTEL_EU_TOTAL)
 SET(CMAKE_CXX_FLAGS "-DHAS_EU_TOTAL ${CMAKE_CXX_FLAGS}")
 SET(CMAKE_C_FLAGS "-DHAS_EU_TOTAL ${CMAKE_C_FLAGS}")
diff --git a/src/intel/intel_driver.c b/src/intel/intel_driver.c
index 755ab6b9b4fd..a33c2605fcc8 100644
--- a/src/intel/intel_driver.c
+++ b/src/intel/intel_driver.c
@@ -140,6 +140,10 @@ intel_driver_context_init(intel_driver_t *driver)
 {
   driver->ctx = drm_intel_gem_context_create(driver->bufmgr);
   assert(driver->ctx);
+#ifdef HAS_ZEROMAP
+  drm_intel_gem_context_set_param(driver->ctx,
+				  I915_CONTEXT_PARAM_NO_ZEROMAP, 1);
+#endif /* HAS_ZEROMAP */
 }
 
 static void
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Chris Wilson]

On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> This patch series (one patch each for libdrm, the kernel, and beignet)
> aims to provide a means to add a context-specific means to prevent
> a mapping to GPU virtual address zero.  This is needed at least by
> Beignet (possibly in other use-cases too, though I don't know of any
> other) to allow use of address zero to represent NULL.

Urm, you cannot allow absolute addressing period. What happens to the
object at 0 when the user reads from it or writes to it? You have to
have an object at 0 for the user's NULL pointer access.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Chris Wilson]

On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > This patch series (one patch each for libdrm, the kernel, and beignet)
> > aims to provide a means to add a context-specific means to prevent
> > a mapping to GPU virtual address zero.  This is needed at least by
> > Beignet (possibly in other use-cases too, though I don't know of any
> > other) to allow use of address zero to represent NULL.
> 
> Urm, you cannot allow absolute addressing period. What happens to the
> object at 0 when the user reads from it or writes to it? You have to
> have an object at 0 for the user's NULL pointer access.

I'll mollify that: outside of full-ppgtt where you need to share the VM.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Daniel Vetter]

On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > aims to provide a means to add a context-specific means to prevent
> > > a mapping to GPU virtual address zero.  This is needed at least by
> > > Beignet (possibly in other use-cases too, though I don't know of any
> > > other) to allow use of address zero to represent NULL.
> > 
> > Urm, you cannot allow absolute addressing period. What happens to the
> > object at 0 when the user reads from it or writes to it? You have to
> > have an object at 0 for the user's NULL pointer access.
> 
> I'll mollify that: outside of full-ppgtt where you need to share the VM.

The description is misleading, the new flag doesn't prevent anything from
getting mapped at 0 but only prevents any bo submitted through execbuf on
the given context from being bound at address 0. If that would happen
compute kernels using NULL checks for some things would fall over.

Essentially it applies the PIN_BIAS for all execbuf objects, which works
even on ggtt execbufs.

Patches themselves look good, but we miss the igt to update the invalid
ctx flags testcase. And a bare minimal function testcase (which checks the
reloc offset with and without a ctx with this flag set) would be nice too.
With that and an r-b from the beignet developers I'll pull this in.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Chris Wilson]

On Wed, May 20, 2015 at 06:00:43PM +0200, Daniel Vetter wrote:
> On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > > aims to provide a means to add a context-specific means to prevent
> > > > a mapping to GPU virtual address zero.  This is needed at least by
> > > > Beignet (possibly in other use-cases too, though I don't know of any
> > > > other) to allow use of address zero to represent NULL.
> > > 
> > > Urm, you cannot allow absolute addressing period. What happens to the
> > > object at 0 when the user reads from it or writes to it? You have to
> > > have an object at 0 for the user's NULL pointer access.
> > 
> > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> 
> The description is misleading, the new flag doesn't prevent anything from
> getting mapped at 0 but only prevents any bo submitted through execbuf on
> the given context from being bound at address 0. If that would happen
> compute kernels using NULL checks for some things would fall over.

That does not address the issue that *0 is an untrapped runtime bug that
allows writing to other objects or reading from them.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[David Weinehall]

On Wed, May 20, 2015 at 06:00:43PM +0200, Daniel Vetter wrote:
> On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > > aims to provide a means to add a context-specific means to prevent
> > > > a mapping to GPU virtual address zero.  This is needed at least by
> > > > Beignet (possibly in other use-cases too, though I don't know of any
> > > > other) to allow use of address zero to represent NULL.
> > > 
> > > Urm, you cannot allow absolute addressing period. What happens to the
> > > object at 0 when the user reads from it or writes to it? You have to
> > > have an object at 0 for the user's NULL pointer access.
> > 
> > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> 
> The description is misleading, the new flag doesn't prevent anything from
> getting mapped at 0 but only prevents any bo submitted through execbuf on
> the given context from being bound at address 0. If that would happen
> compute kernels using NULL checks for some things would fall over.
> 
> Essentially it applies the PIN_BIAS for all execbuf objects, which works
> even on ggtt execbufs.
> 
> Patches themselves look good, but we miss the igt to update the invalid
> ctx flags testcase. And a bare minimal function testcase (which checks the
> reloc offset with and without a ctx with this flag set) would be nice too.
> With that and an r-b from the beignet developers I'll pull this in.
> -Daniel

Yeah, I'll submit the test cases; I have both of them laying around
somewhere already :)


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[David Weinehall]

On Wed, May 20, 2015 at 05:10:58PM +0100, Chris Wilson wrote:
> On Wed, May 20, 2015 at 06:00:43PM +0200, Daniel Vetter wrote:
> > On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > > > aims to provide a means to add a context-specific means to prevent
> > > > > a mapping to GPU virtual address zero.  This is needed at least by
> > > > > Beignet (possibly in other use-cases too, though I don't know of any
> > > > > other) to allow use of address zero to represent NULL.
> > > > 
> > > > Urm, you cannot allow absolute addressing period. What happens to the
> > > > object at 0 when the user reads from it or writes to it? You have to
> > > > have an object at 0 for the user's NULL pointer access.
> > > 
> > > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> > 
> > The description is misleading, the new flag doesn't prevent anything from
> > getting mapped at 0 but only prevents any bo submitted through execbuf on
> > the given context from being bound at address 0. If that would happen
> > compute kernels using NULL checks for some things would fall over.

Any suggestion for a better description?

> That does not address the issue that *0 is an untrapped runtime bug that
> allows writing to other objects or reading from them.

Not exactly sure what you suggest here?


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Chris Wilson]

On Thu, May 21, 2015 at 11:08:42AM +0300, David Weinehall wrote:
> On Wed, May 20, 2015 at 05:10:58PM +0100, Chris Wilson wrote:
> > On Wed, May 20, 2015 at 06:00:43PM +0200, Daniel Vetter wrote:
> > > On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > > > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > > > > aims to provide a means to add a context-specific means to prevent
> > > > > > a mapping to GPU virtual address zero.  This is needed at least by
> > > > > > Beignet (possibly in other use-cases too, though I don't know of any
> > > > > > other) to allow use of address zero to represent NULL.
> > > > > 
> > > > > Urm, you cannot allow absolute addressing period. What happens to the
> > > > > object at 0 when the user reads from it or writes to it? You have to
> > > > > have an object at 0 for the user's NULL pointer access.
> > > > 
> > > > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> > > 
> > > The description is misleading, the new flag doesn't prevent anything from
> > > getting mapped at 0 but only prevents any bo submitted through execbuf on
> > > the given context from being bound at address 0. If that would happen
> > > compute kernels using NULL checks for some things would fall over.
> 
> Any suggestion for a better description?
> 
> > That does not address the issue that *0 is an untrapped runtime bug that
> > allows writing to other objects or reading from them.
> 
> Not exactly sure what you suggest here?

That you have an unmitigated security hole in your design.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[David Weinehall]

On Thu, May 21, 2015 at 09:43:00AM +0100, Chris Wilson wrote:
> On Thu, May 21, 2015 at 11:08:42AM +0300, David Weinehall wrote:
[snip]
> > Not exactly sure what you suggest here?
> 
> That you have an unmitigated security hole in your design.

No, I meant what you suggest as a remedy.


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Daniel Vetter]

On Thu, May 21, 2015 at 09:43:00AM +0100, Chris Wilson wrote:
> On Thu, May 21, 2015 at 11:08:42AM +0300, David Weinehall wrote:
> > On Wed, May 20, 2015 at 05:10:58PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 06:00:43PM +0200, Daniel Vetter wrote:
> > > > On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > > > > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > > > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > > > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > > > > > aims to provide a means to add a context-specific means to prevent
> > > > > > > a mapping to GPU virtual address zero.  This is needed at least by
> > > > > > > Beignet (possibly in other use-cases too, though I don't know of any
> > > > > > > other) to allow use of address zero to represent NULL.
> > > > > > 
> > > > > > Urm, you cannot allow absolute addressing period. What happens to the
> > > > > > object at 0 when the user reads from it or writes to it? You have to
> > > > > > have an object at 0 for the user's NULL pointer access.
> > > > > 
> > > > > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> > > > 
> > > > The description is misleading, the new flag doesn't prevent anything from
> > > > getting mapped at 0 but only prevents any bo submitted through execbuf on
> > > > the given context from being bound at address 0. If that would happen
> > > > compute kernels using NULL checks for some things would fall over.
> > 
> > Any suggestion for a better description?
> > 
> > > That does not address the issue that *0 is an untrapped runtime bug that
> > > allows writing to other objects or reading from them.
> > 
> > Not exactly sure what you suggest here?
> 
> That you have an unmitigated security hole in your design.

There's no security issue afaics, there's only a correctness issue. opencl
kernels must be able to assume that NULL never points to a valid buffer
address of their own. It might point at other intersting stuff like batch
buffers or what else, but never about anything created by the ocl client
itself. This patch achieves that.

Ofc there's still the issue that anything else mapped can be stomped upon,
but that's just undefined behaviour in C-spec speak. And there's still the
isolation issues, but ocl doesn't make any new guarantees there afaik.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[David Weinehall]

tests/gem_ctx_param_basic: Expand ctx_param tests

Expand the context parameter tests to cover the
no-zeromap parameter.

Signed-off-by: David Weinehall <david.weinehall@intel.com>
---
 gem_ctx_param_basic.c |   24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
index b44b37cf0538..ba9366d1a679 100644
--- a/tests/gem_ctx_param_basic.c
+++ b/tests/gem_ctx_param_basic.c
@@ -98,7 +98,7 @@ igt_main
 		ctx_param.size = 0;
 	}
 
-	ctx_param.param  = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
+	ctx_param.param  = I915_CONTEXT_PARAM_NO_ZEROMAP + 1;
 
 	igt_subtest("invalid-param-get") {
 		ctx_param.context = ctx;
@@ -132,6 +132,28 @@ igt_main
 		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
 	}
 
+	ctx_param.param  = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
+
+	igt_subtest("non-root-set-no-zeromap") {
+		igt_fork(child, 1) {
+			igt_drop_root();
+
+			ctx_param.context = ctx;
+			TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+			ctx_param.value--;
+			TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
+		}
+
+		igt_waitchildren();
+	}
+
+	igt_subtest("root-set-no-zeromap") {
+		ctx_param.context = ctx;
+		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
+		ctx_param.value--;
+		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
+	}
+
 	igt_fixture
 		close(fd);
 }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Chris Wilson]

On Thu, May 21, 2015 at 11:44:11AM +0200, Daniel Vetter wrote:
> On Thu, May 21, 2015 at 09:43:00AM +0100, Chris Wilson wrote:
> > On Thu, May 21, 2015 at 11:08:42AM +0300, David Weinehall wrote:
> > > On Wed, May 20, 2015 at 05:10:58PM +0100, Chris Wilson wrote:
> > > > On Wed, May 20, 2015 at 06:00:43PM +0200, Daniel Vetter wrote:
> > > > > On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > > > > > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > > > > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > > > > > This patch series (one patch each for libdrm, the kernel, and beignet)
> > > > > > > > aims to provide a means to add a context-specific means to prevent
> > > > > > > > a mapping to GPU virtual address zero.  This is needed at least by
> > > > > > > > Beignet (possibly in other use-cases too, though I don't know of any
> > > > > > > > other) to allow use of address zero to represent NULL.
> > > > > > > 
> > > > > > > Urm, you cannot allow absolute addressing period. What happens to the
> > > > > > > object at 0 when the user reads from it or writes to it? You have to
> > > > > > > have an object at 0 for the user's NULL pointer access.
> > > > > > 
> > > > > > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> > > > > 
> > > > > The description is misleading, the new flag doesn't prevent anything from
> > > > > getting mapped at 0 but only prevents any bo submitted through execbuf on
> > > > > the given context from being bound at address 0. If that would happen
> > > > > compute kernels using NULL checks for some things would fall over.
> > > 
> > > Any suggestion for a better description?
> > > 
> > > > That does not address the issue that *0 is an untrapped runtime bug that
> > > > allows writing to other objects or reading from them.
> > > 
> > > Not exactly sure what you suggest here?
> > 
> > That you have an unmitigated security hole in your design.
> 
> There's no security issue afaics, there's only a correctness issue. opencl
> kernels must be able to assume that NULL never points to a valid buffer
> address of their own. It might point at other intersting stuff like batch
> buffers or what else, but never about anything created by the ocl client
> itself. This patch achieves that.

It also have just as much risk as reporting EBUSY due to the CL client
trying to use a pinned buffer.

However, it is a security hole because the same process can arrange to
have whatever buffer it likes at 0 then access it through the CL kernel.

> Ofc there's still the issue that anything else mapped can be stomped upon,
> but that's just undefined behaviour in C-spec speak. And there's still the
> isolation issues, but ocl doesn't make any new guarantees there afaik.

I disagree, it is removing the guarantees by providing an absolute
address which can be subverted by an external process.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Chris Wilson]

On Thu, May 21, 2015 at 12:38:45PM +0300, David Weinehall wrote:
> On Thu, May 21, 2015 at 09:43:00AM +0100, Chris Wilson wrote:
> > On Thu, May 21, 2015 at 11:08:42AM +0300, David Weinehall wrote:
> [snip]
> > > Not exactly sure what you suggest here?
> > 
> > That you have an unmitigated security hole in your design.
> 
> No, I meant what you suggest as a remedy.

You place your own buffer at address 0 i.e. you explicitly control the
layout and reject the batch if it is outside of your control.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Zou, Nanhai]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Daniel Vetter
> Sent: Thursday, May 21, 2015 12:01 AM
> To: Chris Wilson; intel-gfx
> Subject: Re: [Intel-gfx] [PATCH 00/03] Preventing zero GPU virtual address
> allocation
> 
> On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > This patch series (one patch each for libdrm, the kernel, and
> > > > beignet) aims to provide a means to add a context-specific means
> > > > to prevent a mapping to GPU virtual address zero.  This is needed
> > > > at least by Beignet (possibly in other use-cases too, though I
> > > > don't know of any
> > > > other) to allow use of address zero to represent NULL.
> > >
> > > Urm, you cannot allow absolute addressing period. What happens to
> > > the object at 0 when the user reads from it or writes to it? You
> > > have to have an object at 0 for the user's NULL pointer access.
> >
> > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> 
> The description is misleading, the new flag doesn't prevent anything from
> getting mapped at 0 but only prevents any bo submitted through execbuf on
> the given context from being bound at address 0. If that would happen
> compute kernels using NULL checks for some things would fall over.
> 
> Essentially it applies the PIN_BIAS for all execbuf objects, which works even on
> ggtt execbufs.
> 
> Patches themselves look good, but we miss the igt to update the invalid ctx
> flags testcase. And a bare minimal function testcase (which checks the reloc
> offset with and without a ctx with this flag set) would be nice too.
> With that and an r-b from the beignet developers I'll pull this in.
> -Daniel


Hi,
	We can verify the patches work for Beignet.

Thanks
Zou Nanhai

> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[David Weinehall]

On Thu, May 21, 2015 at 10:50:37AM +0100, Chris Wilson wrote:
> It also have just as much risk as reporting EBUSY due to the CL client
> trying to use a pinned buffer.
> 
> However, it is a security hole because the same process can arrange to
> have whatever buffer it likes at 0 then access it through the CL kernel.

I don't really understand what you're getting at here.  While it's true
that userland can have whatever buffer it likes at 0, there's nothing in
the current code that prevents this in the first place, so I cannot see
how this could be a regression.  This feature isn't intended as a
security measure; its sole purpose is to help implementations that
assume 0 = failure to avoid weird bugs.


Regards: David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Daniel Vetter]

On Wed, May 27, 2015 at 07:54:44AM +0000, Zou, Nanhai wrote:
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> > Daniel Vetter
> > Sent: Thursday, May 21, 2015 12:01 AM
> > To: Chris Wilson; intel-gfx
> > Subject: Re: [Intel-gfx] [PATCH 00/03] Preventing zero GPU virtual address
> > allocation
> > 
> > On Wed, May 20, 2015 at 03:14:06PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 03:09:43PM +0100, Chris Wilson wrote:
> > > > On Wed, May 20, 2015 at 04:54:19PM +0300, David Weinehall wrote:
> > > > > This patch series (one patch each for libdrm, the kernel, and
> > > > > beignet) aims to provide a means to add a context-specific means
> > > > > to prevent a mapping to GPU virtual address zero.  This is needed
> > > > > at least by Beignet (possibly in other use-cases too, though I
> > > > > don't know of any
> > > > > other) to allow use of address zero to represent NULL.
> > > >
> > > > Urm, you cannot allow absolute addressing period. What happens to
> > > > the object at 0 when the user reads from it or writes to it? You
> > > > have to have an object at 0 for the user's NULL pointer access.
> > >
> > > I'll mollify that: outside of full-ppgtt where you need to share the VM.
> > 
> > The description is misleading, the new flag doesn't prevent anything from
> > getting mapped at 0 but only prevents any bo submitted through execbuf on
> > the given context from being bound at address 0. If that would happen
> > compute kernels using NULL checks for some things would fall over.
> > 
> > Essentially it applies the PIN_BIAS for all execbuf objects, which works even on
> > ggtt execbufs.
> > 
> > Patches themselves look good, but we miss the igt to update the invalid ctx
> > flags testcase. And a bare minimal function testcase (which checks the reloc
> > offset with and without a ctx with this flag set) would be nice too.
> > With that and an r-b from the beignet developers I'll pull this in.
> > -Daniel
> 
> 
> Hi,
> 	We can verify the patches work for Beignet.

Thanks, I applied the kernel patch. Please push libdrm&beignet patches now
(with the libdrm release+depencies done ofc).

For the libdrm patch please make sure that any uapi header copies are
verbatim copies from the kernel generated with

$ make headers

Thanks, Daniel

> 
> Thanks
> Zou Nanhai
> 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[Daniel Vetter]

On Thu, May 21, 2015 at 12:44:53PM +0300, David Weinehall wrote:
> tests/gem_ctx_param_basic: Expand ctx_param tests
> 
> Expand the context parameter tests to cover the
> no-zeromap parameter.
> 
> Signed-off-by: David Weinehall <david.weinehall@intel.com>
> ---
>  gem_ctx_param_basic.c |   24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> index b44b37cf0538..ba9366d1a679 100644
> --- a/tests/gem_ctx_param_basic.c
> +++ b/tests/gem_ctx_param_basic.c
> @@ -98,7 +98,7 @@ igt_main
>  		ctx_param.size = 0;
>  	}
>  
> -	ctx_param.param  = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
> +	ctx_param.param  = I915_CONTEXT_PARAM_NO_ZEROMAP + 1;

Please respin this one with a LOCAL_ define for NO_ZEROMAP. We generally
don't want to have a hard coupling between the headers in libdrm and igt,
would mean a libdrm release roughly every week ;-)

>  
>  	igt_subtest("invalid-param-get") {
>  		ctx_param.context = ctx;
> @@ -132,6 +132,28 @@ igt_main
>  		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
>  	}
>  
> +	ctx_param.param  = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
> +
> +	igt_subtest("non-root-set-no-zeromap") {
> +		igt_fork(child, 1) {
> +			igt_drop_root();
> +
> +			ctx_param.context = ctx;
> +			TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> +			ctx_param.value--;
> +			TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
> +		}
> +
> +		igt_waitchildren();
> +	}
> +
> +	igt_subtest("root-set-no-zeromap") {
> +		ctx_param.context = ctx;
> +		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> +		ctx_param.value--;
> +		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> +	}

A simple functional test here which does:
a) an execbuf with just 1 batch. With full ppgtt you should get that one
at offset 0. If not, skip the testcase.
b) set the NO_ZEROMAP property.
c) re-run the same batch, assert that now the buffer is relocated to
something non-0.

Just to make sure we have a bare minimal testcase to make sure we don't
break this.

Thanks, Daniel

> +
>  	igt_fixture
>  		close(fd);
>  }
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[David Weinehall]

On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
> On Thu, May 21, 2015 at 12:44:53PM +0300, David Weinehall wrote:
> > tests/gem_ctx_param_basic: Expand ctx_param tests
> > 
> > Expand the context parameter tests to cover the
> > no-zeromap parameter.
> > 
> > Signed-off-by: David Weinehall <david.weinehall@intel.com>
> > ---
> >  gem_ctx_param_basic.c |   24 +++++++++++++++++++++++-
> >  1 file changed, 23 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/gem_ctx_param_basic.c b/tests/gem_ctx_param_basic.c
> > index b44b37cf0538..ba9366d1a679 100644
> > --- a/tests/gem_ctx_param_basic.c
> > +++ b/tests/gem_ctx_param_basic.c
> > @@ -98,7 +98,7 @@ igt_main
> >  		ctx_param.size = 0;
> >  	}
> >  
> > -	ctx_param.param  = LOCAL_CONTEXT_PARAM_BAN_PERIOD + 1;
> > +	ctx_param.param  = I915_CONTEXT_PARAM_NO_ZEROMAP + 1;
> 
> Please respin this one with a LOCAL_ define for NO_ZEROMAP. We generally
> don't want to have a hard coupling between the headers in libdrm and igt,
> would mean a libdrm release roughly every week ;-)

Oh, sorry, that was a typo, will fix.

> >  
> >  	igt_subtest("invalid-param-get") {
> >  		ctx_param.context = ctx;
> > @@ -132,6 +132,28 @@ igt_main
> >  		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> >  	}
> >  
> > +	ctx_param.param  = LOCAL_CONTEXT_PARAM_NO_ZEROMAP;
> > +
> > +	igt_subtest("non-root-set-no-zeromap") {
> > +		igt_fork(child, 1) {
> > +			igt_drop_root();
> > +
> > +			ctx_param.context = ctx;
> > +			TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> > +			ctx_param.value--;
> > +			TEST_FAIL(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM, EPERM);
> > +		}
> > +
> > +		igt_waitchildren();
> > +	}
> > +
> > +	igt_subtest("root-set-no-zeromap") {
> > +		ctx_param.context = ctx;
> > +		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_GETPARAM);
> > +		ctx_param.value--;
> > +		TEST_SUCCESS(LOCAL_IOCTL_I915_GEM_CONTEXT_SETPARAM);
> > +	}
> 
> A simple functional test here which does:
> a) an execbuf with just 1 batch. With full ppgtt you should get that one
> at offset 0. If not, skip the testcase.
> b) set the NO_ZEROMAP property.
> c) re-run the same batch, assert that now the buffer is relocated to
> something non-0.
> 
> Just to make sure we have a bare minimal testcase to make sure we don't
> break this.

OK, will add that -- thanks for the input.


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/03] drm/i915: add a context parameter to {en, dis}able zero address mapping

[Chris Wilson]

On Wed, May 20, 2015 at 05:00:13PM +0300, David Weinehall wrote:
> Export a new context parameter that can be set/queried through the
> context_{get,set}param ioctls.  This parameter is passed as a context
> flag and decides whether or not a GPU address mapping is allowed to
> be made at address zero.  The default is to allow such mappings.

Please revert this piece of unreviewed API.

The most obvious objection is what value of bias the kernel should be
using.

Then given that what value does this hold over and above userspace
specifying the layout they want?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[David Weinehall]

On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
> A simple functional test here which does:
> a) an execbuf with just 1 batch. With full ppgtt you should get that one
> at offset 0. If not, skip the testcase.
> b) set the NO_ZEROMAP property.
> c) re-run the same batch, assert that now the buffer is relocated to
> something non-0.
> 
> Just to make sure we have a bare minimal testcase to make sure we don't
> break this.

Maybe this should be added to another test rather than here?  This test
is described as a:

"Basic test for context set/get param input validation."

Somehow I feel that testing whether the *functionality* is correct
does not belong in this test, but rather in some test case that's
already related to execbufs, or even a dedicated test case.

But that might be over-engineering.  Opinions?


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/03] drm/i915: add a context parameter to {en, dis}able zero address mapping

[Daniel Vetter]

On Thu, May 28, 2015 at 03:39:26PM +0100, Chris Wilson wrote:
> On Wed, May 20, 2015 at 05:00:13PM +0300, David Weinehall wrote:
> > Export a new context parameter that can be set/queried through the
> > context_{get,set}param ioctls.  This parameter is passed as a context
> > flag and decides whether or not a GPU address mapping is allowed to
> > be made at address zero.  The default is to allow such mappings.
> 
> Please revert this piece of unreviewed API.
> 
> The most obvious objection is what value of bias the kernel should be
> using.
> 
> Then given that what value does this hold over and above userspace
> specifying the layout they want?

Yeah it would be redundant with softpinning. But that's only for ocl2, and
this is apparently needed for for ocl1.x already. Hence imo it's ok to
pull this ahead a bit, even if redundant.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/03] drm/i915: add a context parameter to {en, dis}able zero address mapping

[Chris Wilson]

On Thu, May 28, 2015 at 05:52:21PM +0200, Daniel Vetter wrote:
> On Thu, May 28, 2015 at 03:39:26PM +0100, Chris Wilson wrote:
> > On Wed, May 20, 2015 at 05:00:13PM +0300, David Weinehall wrote:
> > > Export a new context parameter that can be set/queried through the
> > > context_{get,set}param ioctls.  This parameter is passed as a context
> > > flag and decides whether or not a GPU address mapping is allowed to
> > > be made at address zero.  The default is to allow such mappings.
> > 
> > Please revert this piece of unreviewed API.
> > 
> > The most obvious objection is what value of bias the kernel should be
> > using.
> > 
> > Then given that what value does this hold over and above userspace
> > specifying the layout they want?
> 
> Yeah it would be redundant with softpinning. But that's only for ocl2, and
> this is apparently needed for for ocl1.x already. Hence imo it's ok to
> pull this ahead a bit, even if redundant.

Look at the interface:

CONTEXT_NO_ZEROMAP:
- it has nothing to do with mapping, it only concerns execbuf object
  location
- what is a sensible bias? I certainly do not wish for the hack we have
  right now to become permanent (as this patch makes it).
- it can still easily fail given a kernel that operates on pinned
  objects when not in full-ppgtt

For a context parameter, something like
  CONTEXT_MM_START, CONTEXT_MM_END
that limited the drm_mm range manager for the context (downside is that
such would only be available for full-ppgtt, but really full-ppgtt is
mandatory for this in the first place otherwise it *is* exploitable -
for it not requires a compiler that wouldn't need the NULL pointer to be
all 0). The advantage of that parameter is that it is useful in igt
testing, and even allows for limited rollout of 48bit full-ppgtt, vital
given the workarounds that userspace must buy into before enabling.

And then there is "int flags;". Are we really expecting to use the sign
bit on this bitmask?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[Daniel Vetter]

On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote:
> On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
> > A simple functional test here which does:
> > a) an execbuf with just 1 batch. With full ppgtt you should get that one
> > at offset 0. If not, skip the testcase.
> > b) set the NO_ZEROMAP property.
> > c) re-run the same batch, assert that now the buffer is relocated to
> > something non-0.
> > 
> > Just to make sure we have a bare minimal testcase to make sure we don't
> > break this.
> 
> Maybe this should be added to another test rather than here?  This test
> is described as a:
> 
> "Basic test for context set/get param input validation."
> 
> Somehow I feel that testing whether the *functionality* is correct
> does not belong in this test, but rather in some test case that's
> already related to execbufs, or even a dedicated test case.
> 
> But that might be over-engineering.  Opinions?

Yeah separate testcase would fit better, agreed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/03] drm/i915: add a context parameter to {en, dis}able zero address mapping

[David Weinehall]

On Thu, May 28, 2015 at 05:56:25PM +0100, Chris Wilson wrote:
> On Thu, May 28, 2015 at 05:52:21PM +0200, Daniel Vetter wrote:
> > On Thu, May 28, 2015 at 03:39:26PM +0100, Chris Wilson wrote:
> > > On Wed, May 20, 2015 at 05:00:13PM +0300, David Weinehall wrote:
> > > > Export a new context parameter that can be set/queried through the
> > > > context_{get,set}param ioctls.  This parameter is passed as a context
> > > > flag and decides whether or not a GPU address mapping is allowed to
> > > > be made at address zero.  The default is to allow such mappings.
> > > 
> > > Please revert this piece of unreviewed API.
> > > 
> > > The most obvious objection is what value of bias the kernel should be
> > > using.
> > > 
> > > Then given that what value does this hold over and above userspace
> > > specifying the layout they want?
> > 
> > Yeah it would be redundant with softpinning. But that's only for ocl2, and
> > this is apparently needed for for ocl1.x already. Hence imo it's ok to
> > pull this ahead a bit, even if redundant.
> 
> Look at the interface:
> 
> CONTEXT_NO_ZEROMAP:
> - it has nothing to do with mapping, it only concerns execbuf object
>   location

If you want the option renamed I'd be happy to do so, just suggest a
better name.

> - what is a sensible bias? I certainly do not wish for the hack we have
>   right now to become permanent (as this patch makes it).
> - it can still easily fail given a kernel that operates on pinned
>   objects when not in full-ppgtt
> 

I find it weird that you complain that this solution only works with
full ppgtt, then suggest an alternative solution that would only be
available for full ppgtt...

As far as solving the non-ppgtt case, I cannot see any obvious solution
except the original suggestion that was shot down quite quickly
(offsetting all allocations so none of them ever ends up at 0).

> For a context parameter, something like
>   CONTEXT_MM_START, CONTEXT_MM_END
> that limited the drm_mm range manager for the context (downside is that
> such would only be available for full-ppgtt, but really full-ppgtt is
> mandatory for this in the first place otherwise it *is* exploitable -
> for it not requires a compiler that wouldn't need the NULL pointer to be
> all 0). The advantage of that parameter is that it is useful in igt
> testing, and even allows for limited rollout of 48bit full-ppgtt, vital
> given the workarounds that userspace must buy into before enabling.
> 
I still don't see what attack vector you see that does not already
exist in the current code.

Also, what would be the use cases for a generic range limiting
interface, and wouldn't that force userspace to know unnecessarily much
about mm internals?

> And then there is "int flags;". Are we really expecting to use the sign
> bit on this bitmask?

No, that was simply a brainfart on my behalf; thanks for pointing it
out.


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/03] Preventing zero GPU virtual address allocation

[Dave Gordon]

On 27/05/15 10:17, David Weinehall wrote:
> On Thu, May 21, 2015 at 10:50:37AM +0100, Chris Wilson wrote:
>> It also have just as much risk as reporting EBUSY due to the CL client
>> trying to use a pinned buffer.
>>
>> However, it is a security hole because the same process can arrange to
>> have whatever buffer it likes at 0 then access it through the CL kernel.
> 
> I don't really understand what you're getting at here.  While it's true
> that userland can have whatever buffer it likes at 0, there's nothing in
> the current code that prevents this in the first place, so I cannot see
> how this could be a regression.  This feature isn't intended as a
> security measure; its sole purpose is to help implementations that
> assume 0 = failure to avoid weird bugs.
> 
> Regards: David
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

FWIW, GuC submission will require any object that has to be accessed by
the GuC itself to be mapped at an address that is *not* in the lowest
portion of the address range [0..GUC_WOPCM_SIZE_VALUE), currently 512K.

This mostly applies to GuC-specific objects such as the GuC client and
log objects, but also to 'intel_context' objects, as the GuC needs to
read and/or update certain parts of the saved context.

The GuC doesn't access batchbuffer objects, or any user-defined data
buffers that they operate on.

.Dave.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[Daniel Vetter]

On Fri, May 29, 2015 at 09:52:52AM +0200, Daniel Vetter wrote:
> On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote:
> > On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
> > > A simple functional test here which does:
> > > a) an execbuf with just 1 batch. With full ppgtt you should get that one
> > > at offset 0. If not, skip the testcase.
> > > b) set the NO_ZEROMAP property.
> > > c) re-run the same batch, assert that now the buffer is relocated to
> > > something non-0.
> > > 
> > > Just to make sure we have a bare minimal testcase to make sure we don't
> > > break this.
> > 
> > Maybe this should be added to another test rather than here?  This test
> > is described as a:
> > 
> > "Basic test for context set/get param input validation."
> > 
> > Somehow I feel that testing whether the *functionality* is correct
> > does not belong in this test, but rather in some test case that's
> > already related to execbufs, or even a dedicated test case.
> > 
> > But that might be over-engineering.  Opinions?
> 
> Yeah separate testcase would fit better, agreed.

Update version of this patch is still missing. I'll need to revert the
kernel side if this one doesn't show up soonish.

Also you're breaking the invalid-flags testcase (did you bother to run
them all and check for regressions?) which Jesse spotted, and with the new
basic set this will be a P1 "I'm going to block everything" bug.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[Daniel Vetter]

On Thu, Aug 06, 2015 at 11:30:00PM +0200, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 09:52:52AM +0200, Daniel Vetter wrote:
> > On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote:
> > > On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
> > > > A simple functional test here which does:
> > > > a) an execbuf with just 1 batch. With full ppgtt you should get that one
> > > > at offset 0. If not, skip the testcase.
> > > > b) set the NO_ZEROMAP property.
> > > > c) re-run the same batch, assert that now the buffer is relocated to
> > > > something non-0.
> > > > 
> > > > Just to make sure we have a bare minimal testcase to make sure we don't
> > > > break this.
> > > 
> > > Maybe this should be added to another test rather than here?  This test
> > > is described as a:
> > > 
> > > "Basic test for context set/get param input validation."
> > > 
> > > Somehow I feel that testing whether the *functionality* is correct
> > > does not belong in this test, but rather in some test case that's
> > > already related to execbufs, or even a dedicated test case.
> > > 
> > > But that might be over-engineering.  Opinions?
> > 
> > Yeah separate testcase would fit better, agreed.
> 
> Update version of this patch is still missing. I'll need to revert the
> kernel side if this one doesn't show up soonish.
> 
> Also you're breaking the invalid-flags testcase (did you bother to run
> them all and check for regressions?) which Jesse spotted, and with the new
> basic set this will be a P1 "I'm going to block everything" bug.

Forgot to add Jesse.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[Jesse Barnes]

On 08/06/2015 02:30 PM, Daniel Vetter wrote:
> On Fri, May 29, 2015 at 09:52:52AM +0200, Daniel Vetter wrote:
>> On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote:
>>> On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
>>>> A simple functional test here which does:
>>>> a) an execbuf with just 1 batch. With full ppgtt you should get that one
>>>> at offset 0. If not, skip the testcase.
>>>> b) set the NO_ZEROMAP property.
>>>> c) re-run the same batch, assert that now the buffer is relocated to
>>>> something non-0.
>>>>
>>>> Just to make sure we have a bare minimal testcase to make sure we don't
>>>> break this.
>>>
>>> Maybe this should be added to another test rather than here?  This test
>>> is described as a:
>>>
>>> "Basic test for context set/get param input validation."
>>>
>>> Somehow I feel that testing whether the *functionality* is correct
>>> does not belong in this test, but rather in some test case that's
>>> already related to execbufs, or even a dedicated test case.
>>>
>>> But that might be over-engineering.  Opinions?
>>
>> Yeah separate testcase would fit better, agreed.
> 
> Update version of this patch is still missing. I'll need to revert the
> kernel side if this one doesn't show up soonish.
> 
> Also you're breaking the invalid-flags testcase (did you bother to run
> them all and check for regressions?) which Jesse spotted, and with the new
> basic set this will be a P1 "I'm going to block everything" bug.

We really need man pages for new ioctls as well in libdrm.

Jesse

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[David Weinehall]

On Thu, Aug 06, 2015 at 02:33:31PM -0700, Jesse Barnes wrote:
> On 08/06/2015 02:30 PM, Daniel Vetter wrote:
> > On Fri, May 29, 2015 at 09:52:52AM +0200, Daniel Vetter wrote:
> >> On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote:
> >>> On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
> >>>> A simple functional test here which does:
> >>>> a) an execbuf with just 1 batch. With full ppgtt you should get that one
> >>>> at offset 0. If not, skip the testcase.
> >>>> b) set the NO_ZEROMAP property.
> >>>> c) re-run the same batch, assert that now the buffer is relocated to
> >>>> something non-0.
> >>>>
> >>>> Just to make sure we have a bare minimal testcase to make sure we don't
> >>>> break this.
> >>>
> >>> Maybe this should be added to another test rather than here?  This test
> >>> is described as a:
> >>>
> >>> "Basic test for context set/get param input validation."
> >>>
> >>> Somehow I feel that testing whether the *functionality* is correct
> >>> does not belong in this test, but rather in some test case that's
> >>> already related to execbufs, or even a dedicated test case.
> >>>
> >>> But that might be over-engineering.  Opinions?
> >>
> >> Yeah separate testcase would fit better, agreed.
> > 
> > Update version of this patch is still missing. I'll need to revert the
> > kernel side if this one doesn't show up soonish.
> > 
> > Also you're breaking the invalid-flags testcase (did you bother to run
> > them all and check for regressions?) which Jesse spotted, and with the new
> > basic set this will be a P1 "I'm going to block everything" bug.
> 
> We really need man pages for new ioctls as well in libdrm.

Hmmm, this isn't a new ioctl, just a context parameter that can be
set/queried using a pre-existing ioctl, but I can modify the existing
manual page (if there is one?) to include information about the new
parameter.


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[David Weinehall]

On Thu, Aug 06, 2015 at 11:30:00PM +0200, Daniel Vetter wrote:
[snip]
> Update version of this patch is still missing. I'll need to revert the
> kernel side if this one doesn't show up soonish.
> 
> Also you're breaking the invalid-flags testcase (did you bother to run
> them all and check for regressions?) which Jesse spotted, and with the new
> basic set this will be a P1 "I'm going to block everything" bug.

The patch I sent this Friday (the one I'd forgotten to send earlier)
should work for the invalid-flags case, yes.


Kind regards, David
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/3] tests/gem_ctx_param_basic: Expand ctx_param tests

[Jesse Barnes]

On 08/10/2015 07:15 AM, David Weinehall wrote:
> On Thu, Aug 06, 2015 at 02:33:31PM -0700, Jesse Barnes wrote:
>> On 08/06/2015 02:30 PM, Daniel Vetter wrote:
>>> On Fri, May 29, 2015 at 09:52:52AM +0200, Daniel Vetter wrote:
>>>> On Thu, May 28, 2015 at 05:53:17PM +0300, David Weinehall wrote:
>>>>> On Wed, May 27, 2015 at 01:32:10PM +0200, Daniel Vetter wrote:
>>>>>> A simple functional test here which does:
>>>>>> a) an execbuf with just 1 batch. With full ppgtt you should get that one
>>>>>> at offset 0. If not, skip the testcase.
>>>>>> b) set the NO_ZEROMAP property.
>>>>>> c) re-run the same batch, assert that now the buffer is relocated to
>>>>>> something non-0.
>>>>>>
>>>>>> Just to make sure we have a bare minimal testcase to make sure we don't
>>>>>> break this.
>>>>>
>>>>> Maybe this should be added to another test rather than here?  This test
>>>>> is described as a:
>>>>>
>>>>> "Basic test for context set/get param input validation."
>>>>>
>>>>> Somehow I feel that testing whether the *functionality* is correct
>>>>> does not belong in this test, but rather in some test case that's
>>>>> already related to execbufs, or even a dedicated test case.
>>>>>
>>>>> But that might be over-engineering.  Opinions?
>>>>
>>>> Yeah separate testcase would fit better, agreed.
>>>
>>> Update version of this patch is still missing. I'll need to revert the
>>> kernel side if this one doesn't show up soonish.
>>>
>>> Also you're breaking the invalid-flags testcase (did you bother to run
>>> them all and check for regressions?) which Jesse spotted, and with the new
>>> basic set this will be a P1 "I'm going to block everything" bug.
>>
>> We really need man pages for new ioctls as well in libdrm.
> 
> Hmmm, this isn't a new ioctl, just a context parameter that can be
> set/queried using a pre-existing ioctl, but I can modify the existing
> manual page (if there is one?) to include information about the new
> parameter.

Yeah we don't have one for this ioctl unfortunately.  There are examples of other ioctl man pages in the libdrm repo though; you could copy one of those and do one for the context get/set ioctl.  If you don't have any time, can you just file a JIRA instead?  We'll get to it eventually... :)

Thanks,
Jesse

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx